From 27967173cd1a6cd88453c26bd0bc5c69129a643c Mon Sep 17 00:00:00 2001 From: lws49 Date: Sun, 28 Jun 2026 12:39:41 +0800 Subject: [PATCH] =?UTF-8?q?feat(gradebook):=20external=20assessment=20mana?= =?UTF-8?q?gement=20=E2=80=94=20panel,=20add/edit/delete,=20reorder,=20wei?= =?UTF-8?q?ght=20integration?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the management surface on top of the read-only foundation: - Manage panel (launched from the gradebook + weighted toolbars) with add, edit, and delete prompts; drag-and-drop reordering of external columns. - Weighted-view integration: external rows in Configure Weights (name + single weight, no mode/expand), and flat external grouping in the column picker. - Store reducers + thunks for create/update/delete/reorder; API + types. - Backend: create/update/destroy/reorder actions with jbuilders; a per-course `position` column (append-on-create, reorder! rewrites order); routes. --- .../course/external_assessments_controller.rb | 78 +++- .../course/gradebook_controller.rb | 2 +- app/models/course/external_assessment.rb | 32 ++ .../_external_assessment.json.jbuilder | 9 + .../external_assessments/create.json.jbuilder | 18 + .../external_assessments/update.json.jbuilder | 14 + client/app/api/course/Gradebook.ts | 40 ++ .../AddExternalColumnPrompt.test.tsx | 195 +++++++++ .../DeleteExternalColumnPrompt.test.tsx | 101 +++++ .../EditExternalAssessmentPrompt.test.tsx | 307 ++++++++++++++ .../ManageExternalAssessmentsButton.test.tsx | 37 ++ .../ManageExternalAssessmentsPanel.test.tsx | 360 +++++++++++++++++ .../__tests__/WeightedGradebookTable.test.tsx | 26 +- .../__tests__/computeWeighted.test.ts | 54 +++ .../gradebook/__tests__/operations.test.ts | 378 +++++++++++++++++- .../course/gradebook/__tests__/store.test.ts | 357 ++++++++++++++++- .../components/AddExternalColumnPrompt.tsx | 207 ++++++++++ .../components/ConfigureWeightsPrompt.tsx | 57 ++- .../components/DeleteExternalColumnPrompt.tsx | 95 +++++ .../components/GradebookColumnTree.tsx | 60 +-- .../gradebook/components/GradebookTable.tsx | 28 +- .../components/WeightedGradebookTable.tsx | 4 + .../manage/EditExternalAssessmentPrompt.tsx | 212 ++++++++++ .../ManageExternalAssessmentsButton.tsx | 38 ++ .../manage/ManageExternalAssessmentsPanel.tsx | 319 +++++++++++++++ .../course/gradebook/computeWeighted.ts | 4 +- .../app/bundles/course/gradebook/constants.ts | 4 + .../bundles/course/gradebook/operations.ts | 66 +++ .../gradebook/pages/GradebookIndex/index.tsx | 9 + client/app/bundles/course/gradebook/store.ts | 131 ++++++ client/app/types/course/gradebook.ts | 11 + client/locales/en.json | 132 ++++++ client/locales/ko.json | 138 +++++++ client/locales/zh.json | 138 +++++++ config/routes.rb | 5 +- ...position_to_course_external_assessments.rb | 23 ++ db/schema.rb | 7 +- .../external_assessments_controller_spec.rb | 318 ++++++++++++++- .../course/gradebook_controller_spec.rb | 21 + .../models/course/external_assessment_spec.rb | 33 ++ 40 files changed, 3995 insertions(+), 73 deletions(-) create mode 100644 app/views/course/external_assessments/_external_assessment.json.jbuilder create mode 100644 app/views/course/external_assessments/create.json.jbuilder create mode 100644 app/views/course/external_assessments/update.json.jbuilder create mode 100644 client/app/bundles/course/gradebook/__tests__/AddExternalColumnPrompt.test.tsx create mode 100644 client/app/bundles/course/gradebook/__tests__/DeleteExternalColumnPrompt.test.tsx create mode 100644 client/app/bundles/course/gradebook/__tests__/EditExternalAssessmentPrompt.test.tsx create mode 100644 client/app/bundles/course/gradebook/__tests__/ManageExternalAssessmentsButton.test.tsx create mode 100644 client/app/bundles/course/gradebook/__tests__/ManageExternalAssessmentsPanel.test.tsx create mode 100644 client/app/bundles/course/gradebook/components/AddExternalColumnPrompt.tsx create mode 100644 client/app/bundles/course/gradebook/components/DeleteExternalColumnPrompt.tsx create mode 100644 client/app/bundles/course/gradebook/components/manage/EditExternalAssessmentPrompt.tsx create mode 100644 client/app/bundles/course/gradebook/components/manage/ManageExternalAssessmentsButton.tsx create mode 100644 client/app/bundles/course/gradebook/components/manage/ManageExternalAssessmentsPanel.tsx create mode 100644 db/migrate/20260625000000_add_position_to_course_external_assessments.rb diff --git a/app/controllers/course/external_assessments_controller.rb b/app/controllers/course/external_assessments_controller.rb index cf2c02fdad..fa3b69486e 100644 --- a/app/controllers/course/external_assessments_controller.rb +++ b/app/controllers/course/external_assessments_controller.rb @@ -1,6 +1,46 @@ # frozen_string_literal: true class Course::ExternalAssessmentsController < Course::ComponentController - before_action :load_external_assessment, only: [:grades] + before_action :load_external_assessment, only: [:update, :destroy, :grades] + + def create + authorize! :manage_gradebook_weights, current_course + @weighted_view_enabled = gradebook_settings.weighted_view_enabled + @external_assessment = Course::ExternalAssessment.create_for_course!( + course: current_course, + title: create_params[:title], + maximum_grade: create_params[:maximumGrade], + weight: create_weight, + floor_at_zero: bound_flag(:floorAtZero, default: true), + cap_at_maximum: bound_flag(:capAtMaximum, default: true) + ) + render 'create' + rescue ActiveRecord::RecordInvalid => e + render json: { errors: { base: e.message } }, status: :unprocessable_entity + end + + def update + authorize! :manage_gradebook_weights, current_course + @weighted_view_enabled = gradebook_settings.weighted_view_enabled + @external_assessment.update!(update_params_attrs) + update_weight if @weighted_view_enabled && params.key?(:weight) + render 'update' + rescue ActiveRecord::RecordInvalid => e + render json: { errors: { base: e.message } }, status: :unprocessable_entity + end + + def destroy + authorize! :manage_gradebook_weights, current_course + @external_assessment.destroy! + head :ok + end + + def reorder + authorize! :manage_gradebook_weights, current_course + Course::ExternalAssessment.reorder!(course: current_course, ordered_ids: reorder_params) + head :ok + rescue ArgumentError + head :unprocessable_entity + end def grades authorize! :manage_gradebook_weights, current_course @@ -26,12 +66,44 @@ def component current_component_host[:course_gradebook_component] end + def gradebook_settings + @gradebook_settings ||= Course::Settings::GradebookComponent.new(component) + end + def load_external_assessment @external_assessment = Course::ExternalAssessment.for_course(current_course).find(params[:id]) rescue ActiveRecord::RecordNotFound head :not_found end + def create_params + params.permit(:title, :maximumGrade, :weight, :floorAtZero, :capAtMaximum) + end + + def create_weight + @weighted_view_enabled ? (create_params[:weight].presence || 0).to_f : 0 + end + + def update_weight + @external_assessment.gradebook_contribution&.update!(weight: (params[:weight].presence || 0).to_f) + end + + def update_params_attrs + attrs = {} + attrs[:title] = params[:title] if params.key?(:title) + attrs[:maximum_grade] = params[:maximumGrade] if params.key?(:maximumGrade) + attrs[:floor_at_zero] = bound_flag(:floorAtZero, default: true) if params.key?(:floorAtZero) + attrs[:cap_at_maximum] = bound_flag(:capAtMaximum, default: true) if params.key?(:capAtMaximum) + attrs + end + + # Coerce a string/bool HTTP param into a Ruby boolean (defaults when absent). + def bound_flag(key, default:) + return default unless params.key?(key) + + ActiveRecord::Type::Boolean.new.cast(params[key]) + end + def grade_params params.permit(:studentId, :grade) end @@ -40,4 +112,8 @@ def grade_params def normalized_grade(value) value.blank? ? nil : value end + + def reorder_params + params.require(:orderedIds).map(&:to_i) + end end diff --git a/app/controllers/course/gradebook_controller.rb b/app/controllers/course/gradebook_controller.rb index 0964fe6539..d632592d45 100644 --- a/app/controllers/course/gradebook_controller.rb +++ b/app/controllers/course/gradebook_controller.rb @@ -160,7 +160,7 @@ def fetch_categories_and_tabs end def load_externals - @external_assessments = Course::ExternalAssessment.for_course(current_course). + @external_assessments = Course::ExternalAssessment.for_course(current_course).order(:position). includes(:gradebook_contribution, external_assessment_grades: :course_user).to_a @external_grades = @external_assessments.flat_map(&:external_assessment_grades) @external_contributions = @external_assessments. diff --git a/app/models/course/external_assessment.rb b/app/models/course/external_assessment.rb index 329247c014..820f53e086 100644 --- a/app/models/course/external_assessment.rb +++ b/app/models/course/external_assessment.rb @@ -20,11 +20,37 @@ class Course::ExternalAssessment < ApplicationRecord belongs_to :course, inverse_of: :external_assessments has_one :gradebook_contribution, class_name: 'Course::Gradebook::ExternalContribution', inverse_of: :external_assessment, dependent: :destroy + # delete_all (not destroy): grades carry no destroy callbacks, so destroying the + # parent would otherwise fire one SELECT+DELETE per grade (N+1). delete_all removes + # them in a single statement. Rails must issue it — the DB FK has no ON DELETE CASCADE. has_many :external_assessment_grades, class_name: 'Course::ExternalAssessmentGrade', inverse_of: :external_assessment, dependent: :delete_all scope :for_course, ->(course) { where(course_id: course.id) } + before_create :assign_default_position + + # Next free position for the course (positions are 0-based, per course, not + # required to be unique). Drives append-at-end for both manual add and import. + def self.next_position(course) + (for_course(course).maximum(:position) || -1) + 1 + end + + # Rewrites positions to match ordered_ids (the canonical gradebook order). The + # id set must match the course's externals exactly, else the order would be + # corrupted by a stale/partial payload. + def self.reorder!(course:, ordered_ids:) + scope = for_course(course) + raise ArgumentError, 'ordered_ids must match the course externals' unless + ordered_ids.map(&:to_i).sort == scope.pluck(:id).sort + + transaction do + ordered_ids.each_with_index do |id, index| + scope.where(id: id).update_all(position: index) + end + end + end + # The negative serialized id used by the synthetic tab AND the leaf assessment. def synthetic_tab_id -id @@ -46,4 +72,10 @@ def self.create_for_course!(course:, title:, maximum_grade:, weight: 0, end end # rubocop:enable Metrics/ParameterLists + + private + + def assign_default_position + self.position ||= self.class.next_position(course) + end end diff --git a/app/views/course/external_assessments/_external_assessment.json.jbuilder b/app/views/course/external_assessments/_external_assessment.json.jbuilder new file mode 100644 index 0000000000..4c062d9b38 --- /dev/null +++ b/app/views/course/external_assessments/_external_assessment.json.jbuilder @@ -0,0 +1,9 @@ +# frozen_string_literal: true +json.id(-external_assessment.id) +json.title external_assessment.title +json.tabId external_assessment.synthetic_tab_id +json.maxGrade external_assessment.maximum_grade.to_f +json.external true +json.floorAtZero external_assessment.floor_at_zero +json.capAtMaximum external_assessment.cap_at_maximum +json.gradebookExcluded false diff --git a/app/views/course/external_assessments/create.json.jbuilder b/app/views/course/external_assessments/create.json.jbuilder new file mode 100644 index 0000000000..badc35f856 --- /dev/null +++ b/app/views/course/external_assessments/create.json.jbuilder @@ -0,0 +1,18 @@ +# frozen_string_literal: true +json.assessment do + json.partial! 'external_assessment', external_assessment: @external_assessment + json.gradebookWeight(@external_assessment.gradebook_contribution&.weight&.to_f || 0) if @weighted_view_enabled +end +json.tab do + json.id @external_assessment.synthetic_tab_id + json.title @external_assessment.title + json.categoryId Course::ExternalAssessment::SYNTHETIC_CATEGORY_ID + if @weighted_view_enabled + json.gradebookWeight(@external_assessment.gradebook_contribution&.weight&.to_f || 0) + json.weightMode 'equal' + end +end +json.category do + json.id Course::ExternalAssessment::SYNTHETIC_CATEGORY_ID + json.title Course::ExternalAssessment::SYNTHETIC_CATEGORY_TITLE +end diff --git a/app/views/course/external_assessments/update.json.jbuilder b/app/views/course/external_assessments/update.json.jbuilder new file mode 100644 index 0000000000..f5aeb89ca0 --- /dev/null +++ b/app/views/course/external_assessments/update.json.jbuilder @@ -0,0 +1,14 @@ +# frozen_string_literal: true +json.assessment do + json.partial! 'external_assessment', external_assessment: @external_assessment + json.gradebookWeight(@external_assessment.gradebook_contribution&.weight&.to_f || 0) if @weighted_view_enabled +end +json.tab do + json.id @external_assessment.synthetic_tab_id + json.title @external_assessment.title + json.categoryId Course::ExternalAssessment::SYNTHETIC_CATEGORY_ID + if @weighted_view_enabled + json.gradebookWeight(@external_assessment.gradebook_contribution&.weight&.to_f || 0) + json.weightMode 'equal' + end +end diff --git a/client/app/api/course/Gradebook.ts b/client/app/api/course/Gradebook.ts index 4fe79086b9..161b1afc4e 100644 --- a/client/app/api/course/Gradebook.ts +++ b/client/app/api/course/Gradebook.ts @@ -1,4 +1,6 @@ import { + ExternalAssessmentNode, + ExternalAssessmentUpdate, ExternalGradePayload, GradebookData, UpdateWeightsPayload, @@ -23,6 +25,44 @@ export default class GradebookAPI extends BaseCourseAPI { return this.client.patch(`${this.#urlPrefix}/weights`, payload); } + createExternal(payload: { + title: string; + maximumGrade: number; + floorAtZero: boolean; + capAtMaximum: boolean; + weight?: number; + }): APIResponse { + return this.client.post(`${this.#urlPrefix}/external_assessments`, payload); + } + + // `id` is the positive external id (negate the negative serialized id before calling). + updateExternal( + id: number, + payload: { + title?: string; + maximumGrade?: number; + floorAtZero?: boolean; + capAtMaximum?: boolean; + weight?: number; + }, + ): APIResponse { + return this.client.patch( + `${this.#urlPrefix}/external_assessments/${id}`, + payload, + ); + } + + deleteExternal(id: number): APIResponse { + return this.client.delete(`${this.#urlPrefix}/external_assessments/${id}`); + } + + reorderExternals(payload: { orderedIds: number[] }): APIResponse { + return this.client.put( + `${this.#urlPrefix}/external_assessments/reorder`, + payload, + ); + } + setExternalGrade( id: number, payload: { studentId: number; grade: number | null }, diff --git a/client/app/bundles/course/gradebook/__tests__/AddExternalColumnPrompt.test.tsx b/client/app/bundles/course/gradebook/__tests__/AddExternalColumnPrompt.test.tsx new file mode 100644 index 0000000000..cd8fd577fe --- /dev/null +++ b/client/app/bundles/course/gradebook/__tests__/AddExternalColumnPrompt.test.tsx @@ -0,0 +1,195 @@ +import { fireEvent, render, screen, waitFor } from 'test-utils'; + +import AddExternalColumnPrompt from '../components/AddExternalColumnPrompt'; +import { createExternalAssessment } from '../operations'; + +jest.mock('../operations', () => ({ + createExternalAssessment: jest.fn( + () => (): Promise => Promise.resolve(), + ), +})); + +afterEach(() => { + jest.clearAllMocks(); +}); + +it('submits with both bound flags on by default', async () => { + render(); + // Wait for the Dialog to render (i18n loading) + await waitFor(() => screen.getByLabelText('Name')); + fireEvent.change(screen.getByLabelText('Name'), { + target: { value: 'Midterm' }, + }); + fireEvent.change(screen.getByLabelText('Max marks'), { + target: { value: '50' }, + }); + fireEvent.click(screen.getByRole('button', { name: 'Create' })); + await waitFor(() => + expect(createExternalAssessment).toHaveBeenCalledWith( + 'Midterm', + 50, + true, + true, + undefined, + ), + ); +}); + +it('submits floorAtZero false when the floor toggle is switched off', async () => { + render(); + await waitFor(() => screen.getByLabelText('Name')); + fireEvent.change(screen.getByLabelText('Name'), { + target: { value: 'Midterm' }, + }); + fireEvent.change(screen.getByLabelText('Max marks'), { + target: { value: '50' }, + }); + fireEvent.click(screen.getByRole('checkbox', { name: 'Floor grades at 0' })); + fireEvent.click(screen.getByRole('button', { name: 'Create' })); + await waitFor(() => + expect(createExternalAssessment).toHaveBeenCalledWith( + 'Midterm', + 50, + false, + true, + undefined, + ), + ); +}); + +it('submits capAtMaximum false when the cap toggle is switched off', async () => { + render(); + await waitFor(() => screen.getByLabelText('Name')); + fireEvent.change(screen.getByLabelText('Name'), { + target: { value: 'Midterm' }, + }); + fireEvent.change(screen.getByLabelText('Max marks'), { + target: { value: '50' }, + }); + fireEvent.click(screen.getByRole('checkbox', { name: 'Cap grades at max' })); + fireEvent.click(screen.getByRole('button', { name: 'Create' })); + await waitFor(() => + expect(createExternalAssessment).toHaveBeenCalledWith( + 'Midterm', + 50, + true, + false, + undefined, + ), + ); +}); + +it('disables Create when the name is blank', async () => { + render(); + await waitFor(() => screen.getByLabelText('Name')); + fireEvent.change(screen.getByLabelText('Max marks'), { + target: { value: '50' }, + }); + expect(screen.getByRole('button', { name: 'Create' })).toBeDisabled(); +}); + +it('disables Create when max marks is blank', async () => { + render(); + await waitFor(() => screen.getByLabelText('Name')); + fireEvent.change(screen.getByLabelText('Name'), { + target: { value: 'Midterm' }, + }); + expect(screen.getByRole('button', { name: 'Create' })).toBeDisabled(); +}); + +it('disables Create when max marks is negative', async () => { + render(); + await waitFor(() => screen.getByLabelText('Name')); + fireEvent.change(screen.getByLabelText('Name'), { + target: { value: 'Midterm' }, + }); + fireEvent.change(screen.getByLabelText('Max marks'), { + target: { value: '-5' }, + }); + expect(screen.getByRole('button', { name: 'Create' })).toBeDisabled(); +}); + +it('closes the dialog after a successful create', async () => { + const onClose = jest.fn(); + render(); + await waitFor(() => screen.getByLabelText('Name')); + fireEvent.change(screen.getByLabelText('Name'), { + target: { value: 'Midterm' }, + }); + fireEvent.change(screen.getByLabelText('Max marks'), { + target: { value: '50' }, + }); + fireEvent.click(screen.getByRole('button', { name: 'Create' })); + await waitFor(() => expect(onClose).toHaveBeenCalled()); +}); + +it('keeps the dialog open when create fails', async () => { + (createExternalAssessment as jest.Mock).mockReturnValueOnce(() => + Promise.reject(new Error('boom')), + ); + const onClose = jest.fn(); + render(); + await waitFor(() => screen.getByLabelText('Name')); + fireEvent.change(screen.getByLabelText('Name'), { + target: { value: 'Midterm' }, + }); + fireEvent.change(screen.getByLabelText('Max marks'), { + target: { value: '50' }, + }); + fireEvent.click(screen.getByRole('button', { name: 'Create' })); + await waitFor(() => expect(createExternalAssessment).toHaveBeenCalled()); + expect(onClose).not.toHaveBeenCalled(); +}); + +it('explains the floor and cap toggles, stressing the grade is unchanged', async () => { + render(); + await waitFor(() => screen.getByLabelText('Name')); + expect( + screen.getByLabelText( + /Counts negative grades as 0 when computing the weighted total. The actual grade is unchanged./i, + ), + ).toBeInTheDocument(); + expect( + screen.getByLabelText( + /Counts grades above the maximum as the maximum when computing the weighted total. The actual grade is unchanged./i, + ), + ).toBeInTheDocument(); +}); + +it('passes the typed weightage when weighted view is on', async () => { + render( + , + ); + await waitFor(() => screen.getByLabelText('Name')); + fireEvent.change(screen.getByLabelText('Name'), { + target: { value: 'Midterm' }, + }); + fireEvent.change(screen.getByLabelText('Max marks'), { + target: { value: '50' }, + }); + fireEvent.change(screen.getByLabelText('Weightage'), { + target: { value: '20' }, + }); + fireEvent.click(screen.getByRole('button', { name: 'Create' })); + await waitFor(() => + expect(createExternalAssessment).toHaveBeenCalledWith( + 'Midterm', + 50, + true, + true, + 20, + ), + ); +}); + +it('hides the weightage field when weighted view is off', async () => { + render( + , + ); + await waitFor(() => screen.getByLabelText('Name')); + expect(screen.queryByLabelText('Weightage')).not.toBeInTheDocument(); +}); diff --git a/client/app/bundles/course/gradebook/__tests__/DeleteExternalColumnPrompt.test.tsx b/client/app/bundles/course/gradebook/__tests__/DeleteExternalColumnPrompt.test.tsx new file mode 100644 index 0000000000..68cd985889 --- /dev/null +++ b/client/app/bundles/course/gradebook/__tests__/DeleteExternalColumnPrompt.test.tsx @@ -0,0 +1,101 @@ +import userEvent from '@testing-library/user-event'; +import { render, screen, waitFor } from 'test-utils'; + +import toast from 'lib/hooks/toast'; + +import DeleteExternalColumnPrompt from '../components/DeleteExternalColumnPrompt'; +import { deleteExternalAssessment } from '../operations'; + +jest.mock('../operations', () => ({ + __esModule: true, + ...jest.requireActual('../operations'), + deleteExternalAssessment: jest.fn(), +})); + +jest.mock('lib/hooks/toast', () => ({ + __esModule: true, + default: { error: jest.fn() }, +})); + +const mockDelete = deleteExternalAssessment as jest.Mock; +const mockToast = toast as jest.Mocked; + +const baseProps = { + open: true, + assessmentId: -1, + title: 'Quiz 2', + onClose: jest.fn(), +}; + +afterEach(() => jest.clearAllMocks()); + +it('renders nothing when closed', () => { + render(); + + expect( + screen.queryByRole('button', { name: 'Delete' }), + ).not.toBeInTheDocument(); +}); + +it('shows a loading spinner on the delete button while the deletion is in flight', async () => { + let resolveDelete: () => void = () => {}; + mockDelete.mockReturnValue( + () => + new Promise((resolve) => { + resolveDelete = resolve; + }), + ); + + render(); + + await userEvent.click(await screen.findByRole('button', { name: 'Delete' })); + + // While the request is pending, the delete button shows its loading state. + expect(await screen.findByRole('progressbar')).toBeInTheDocument(); + + resolveDelete(); + await waitFor(() => expect(baseProps.onClose).toHaveBeenCalled()); +}); + +it('dispatches the delete operation with the assessment id on confirm', async () => { + mockDelete.mockReturnValue(() => Promise.resolve()); + + render(); + + await userEvent.click(await screen.findByRole('button', { name: 'Delete' })); + + await waitFor(() => + expect(mockDelete).toHaveBeenCalledWith(baseProps.assessmentId), + ); + await waitFor(() => expect(baseProps.onClose).toHaveBeenCalled()); +}); + +it('toasts an error and keeps the dialog open when the delete fails', async () => { + mockDelete.mockReturnValue(() => Promise.reject(new Error('boom'))); + + render(); + + await userEvent.click(await screen.findByRole('button', { name: 'Delete' })); + + await waitFor(() => expect(mockToast.error).toHaveBeenCalled()); + expect(baseProps.onClose).not.toHaveBeenCalled(); + // finally{} resets saving → confirm button leaves its loading state. + await waitFor(() => + expect(screen.queryByRole('progressbar')).not.toBeInTheDocument(), + ); +}); + +it('closes without deleting when Cancel is clicked', async () => { + render(); + + await userEvent.click(await screen.findByRole('button', { name: 'Cancel' })); + + expect(baseProps.onClose).toHaveBeenCalled(); + expect(mockDelete).not.toHaveBeenCalled(); +}); + +it('names the assessment being deleted in the confirmation body', async () => { + render(); + + expect(await screen.findByText(/Delete "Quiz 2"\?/)).toBeInTheDocument(); +}); diff --git a/client/app/bundles/course/gradebook/__tests__/EditExternalAssessmentPrompt.test.tsx b/client/app/bundles/course/gradebook/__tests__/EditExternalAssessmentPrompt.test.tsx new file mode 100644 index 0000000000..1b76a774c0 --- /dev/null +++ b/client/app/bundles/course/gradebook/__tests__/EditExternalAssessmentPrompt.test.tsx @@ -0,0 +1,307 @@ +import userEvent from '@testing-library/user-event'; +import { fireEvent, render, screen, waitFor } from 'test-utils'; + +import toast from 'lib/hooks/toast'; + +import EditExternalAssessmentPrompt from '../components/manage/EditExternalAssessmentPrompt'; +import { editExternalAssessment } from '../operations'; + +jest.mock('../operations', () => ({ + editExternalAssessment: jest.fn(() => (): Promise => Promise.resolve()), +})); +jest.mock('lib/hooks/toast', () => ({ error: jest.fn() })); + +const assessment = { + id: -3, + title: 'Quiz', + tabId: -3, + maxGrade: 20, + external: true, + floorAtZero: true, + capAtMaximum: true, +}; + +beforeEach(() => { + jest.clearAllMocks(); +}); + +it('saves the edited name and a toggled cap flag', async () => { + render( + , + ); + const name = await screen.findByLabelText('Name'); + await userEvent.clear(name); + await userEvent.type(name, 'Quiz 1'); + fireEvent.click(screen.getByRole('checkbox', { name: 'Cap grades at max' })); + fireEvent.click(screen.getByRole('button', { name: 'Save' })); + await waitFor(() => + expect(editExternalAssessment).toHaveBeenCalledWith(-3, { + title: 'Quiz 1', + maximumGrade: 20, + floorAtZero: true, + capAtMaximum: false, + }), + ); +}, 10000); + +it('trims surrounding whitespace from the saved name', async () => { + render( + , + ); + const name = await screen.findByLabelText('Name'); + await userEvent.clear(name); + await userEvent.type(name, ' Quiz 2 '); + fireEvent.click(screen.getByRole('button', { name: 'Save' })); + await waitFor(() => + expect(editExternalAssessment).toHaveBeenCalledWith(-3, { + title: 'Quiz 2', + maximumGrade: 20, + floorAtZero: true, + capAtMaximum: true, + }), + ); +}); + +it('saves a toggled floor flag', async () => { + render( + , + ); + await screen.findByLabelText('Name'); + fireEvent.click(screen.getByRole('checkbox', { name: 'Floor grades at 0' })); + fireEvent.click(screen.getByRole('button', { name: 'Save' })); + await waitFor(() => + expect(editExternalAssessment).toHaveBeenCalledWith(-3, { + title: 'Quiz', + maximumGrade: 20, + floorAtZero: false, + capAtMaximum: true, + }), + ); +}); + +it('saves an edited max marks value', async () => { + render( + , + ); + fireEvent.change(await screen.findByLabelText('Max marks'), { + target: { value: '50' }, + }); + fireEvent.click(screen.getByRole('button', { name: 'Save' })); + await waitFor(() => + expect(editExternalAssessment).toHaveBeenCalledWith(-3, { + title: 'Quiz', + maximumGrade: 50, + floorAtZero: true, + capAtMaximum: true, + }), + ); +}); + +it('defaults floor and cap switches to checked when the assessment omits them', async () => { + render( + , + ); + await screen.findByLabelText('Name'); + expect( + screen.getByRole('checkbox', { name: 'Floor grades at 0' }), + ).toBeChecked(); + expect( + screen.getByRole('checkbox', { name: 'Cap grades at max' }), + ).toBeChecked(); +}); + +it('explains the floor and cap toggles, explaining the grade is unchanged', async () => { + render( + , + ); + await screen.findByLabelText('Name'); + expect( + screen.getByLabelText( + /Counts negative grades as 0 when computing the weighted total. The actual grade is unchanged./i, + ), + ).toBeInTheDocument(); + expect( + screen.getByLabelText( + /Counts grades above the maximum as the maximum when computing the weighted total. The actual grade is unchanged./i, + ), + ).toBeInTheDocument(); +}); + +it('shows a weightage field seeded from the current weight and includes it when saving (weighted view on)', async () => { + render( + , + ); + const weight = await screen.findByLabelText('Weightage'); + expect(weight).toHaveValue(30); + fireEvent.change(weight, { target: { value: '45' } }); + fireEvent.click(screen.getByRole('button', { name: 'Save' })); + await waitFor(() => + expect(editExternalAssessment).toHaveBeenCalledWith(-3, { + title: 'Quiz', + maximumGrade: 20, + floorAtZero: true, + capAtMaximum: true, + weight: 45, + }), + ); +}); + +it('defaults weightage to 0 when no currentWeight is provided (weighted view on)', async () => { + render( + , + ); + const weight = await screen.findByLabelText('Weightage'); + expect(weight).toHaveValue(0); + fireEvent.click(screen.getByRole('button', { name: 'Save' })); + await waitFor(() => + expect(editExternalAssessment).toHaveBeenCalledWith(-3, { + title: 'Quiz', + maximumGrade: 20, + floorAtZero: true, + capAtMaximum: true, + weight: 0, + }), + ); +}); + +it('omits the weightage field when weighted view is off', async () => { + render( + , + ); + await screen.findByLabelText('Name'); + expect(screen.queryByLabelText('Weightage')).not.toBeInTheDocument(); +}); + +it('disables Save when the name is blank', async () => { + render( + , + ); + const name = await screen.findByLabelText('Name'); + await userEvent.clear(name); + await userEvent.type(name, ' '); + expect(screen.getByRole('button', { name: 'Save' })).toBeDisabled(); +}); + +it('disables Save when max marks is blank', async () => { + render( + , + ); + fireEvent.change(await screen.findByLabelText('Max marks'), { + target: { value: '' }, + }); + expect(screen.getByRole('button', { name: 'Save' })).toBeDisabled(); +}); + +it('disables Save when max marks is negative', async () => { + render( + , + ); + fireEvent.change(await screen.findByLabelText('Max marks'), { + target: { value: '-5' }, + }); + expect(screen.getByRole('button', { name: 'Save' })).toBeDisabled(); +}); + +it('calls onClose when Cancel is clicked', async () => { + const onClose = jest.fn(); + render( + , + ); + await screen.findByLabelText('Name'); + fireEvent.click(screen.getByRole('button', { name: 'Cancel' })); + expect(onClose).toHaveBeenCalled(); +}); + +it('closes the dialog after a successful save', async () => { + const onClose = jest.fn(); + render( + , + ); + await screen.findByLabelText('Name'); + fireEvent.click(screen.getByRole('button', { name: 'Save' })); + await waitFor(() => expect(onClose).toHaveBeenCalled()); +}); + +it('shows an error toast and keeps the dialog open when saving fails', async () => { + (editExternalAssessment as jest.Mock).mockImplementationOnce( + () => (): Promise => Promise.reject(new Error('boom')), + ); + const onClose = jest.fn(); + render( + , + ); + await screen.findByLabelText('Name'); + fireEvent.click(screen.getByRole('button', { name: 'Save' })); + await waitFor(() => expect(toast.error).toHaveBeenCalled()); + expect(onClose).not.toHaveBeenCalled(); +}); diff --git a/client/app/bundles/course/gradebook/__tests__/ManageExternalAssessmentsButton.test.tsx b/client/app/bundles/course/gradebook/__tests__/ManageExternalAssessmentsButton.test.tsx new file mode 100644 index 0000000000..c66d7593ce --- /dev/null +++ b/client/app/bundles/course/gradebook/__tests__/ManageExternalAssessmentsButton.test.tsx @@ -0,0 +1,37 @@ +import { fireEvent, render, screen } from 'test-utils'; + +import ManageExternalAssessmentsButton from '../components/manage/ManageExternalAssessmentsButton'; + +const state = { + gradebook: { + categories: [], + tabs: [], + students: [], + submissions: [], + assessments: [], + gamificationEnabled: false, + weightedViewEnabled: false, + canManageWeights: true, + courseMaxLevel: 0, + levelContribution: { + enabled: false, + formula: '', + weight: 0, + show: false, + clamp: true, + }, + }, +}; + +it('opens the panel on click', async () => { + render(, { state }); + fireEvent.click( + await screen.findByRole('button', { name: 'Manage external assessments' }), + ); + expect(await screen.findByText('External assessments')).toBeVisible(); +}); + +it('does not open the external assessments panel by default', () => { + render(, { state }); + expect(screen.queryByText('External assessments')).not.toBeInTheDocument(); +}); diff --git a/client/app/bundles/course/gradebook/__tests__/ManageExternalAssessmentsPanel.test.tsx b/client/app/bundles/course/gradebook/__tests__/ManageExternalAssessmentsPanel.test.tsx new file mode 100644 index 0000000000..7033339d00 --- /dev/null +++ b/client/app/bundles/course/gradebook/__tests__/ManageExternalAssessmentsPanel.test.tsx @@ -0,0 +1,360 @@ +import type { DropResult } from '@hello-pangea/dnd'; +import userEvent from '@testing-library/user-event'; +import type { AppDispatch } from 'store'; +import { render, screen, waitFor } from 'test-utils'; + +import ManageExternalAssessmentsPanel, { + handleDragEnd, + moveItem, +} from '../components/manage/ManageExternalAssessmentsPanel'; +import { reorderExternalAssessments } from '../operations'; + +const dropResult = (from: number, to: number | null): DropResult => + ({ + draggableId: 'x', + type: 'DEFAULT', + mode: 'FLUID', + reason: 'DROP', + combine: null, + source: { index: from, droppableId: 'external-assessments' }, + destination: + to === null ? null : { index: to, droppableId: 'external-assessments' }, + }) as DropResult; + +jest.mock('../operations', () => ({ + __esModule: true, + ...jest.requireActual('../operations'), + reorderExternalAssessments: jest.fn( + () => (): Promise => Promise.resolve(), + ), +})); + +const preloadedState = { + gradebook: { + categories: [], + students: [], + submissions: [], + gamificationEnabled: false, + weightedViewEnabled: false, + canManageWeights: true, + courseMaxLevel: 0, + levelContribution: { + enabled: false, + formula: '', + weight: 0, + show: false, + clamp: true, + }, + tabs: [ + { id: -1, title: 'Midterm', categoryId: -1, gradebookWeight: 25 }, + { id: -2, title: 'Final', categoryId: -1, gradebookWeight: 50 }, + ], + assessments: [ + { + id: -1, + title: 'Midterm', + tabId: -1, + maxGrade: 50, + external: true, + floorAtZero: true, + capAtMaximum: true, + }, + { + id: -2, + title: 'Final', + tabId: -2, + maxGrade: 100, + external: true, + floorAtZero: true, + capAtMaximum: true, + }, + { id: 7, title: 'Native quiz', tabId: 3, maxGrade: 10 }, + ], + }, +}; + +const externalWith = (overrides: { + floorAtZero: boolean; + capAtMaximum: boolean; +}): typeof preloadedState => ({ + gradebook: { + ...preloadedState.gradebook, + tabs: [{ id: -1, title: 'Midterm', categoryId: -1, gradebookWeight: 25 }], + assessments: [ + { + id: -1, + title: 'Midterm', + tabId: -1, + maxGrade: 50, + external: true, + ...overrides, + }, + ], + }, +}); + +describe('handleDragEnd', () => { + beforeEach(() => { + (reorderExternalAssessments as jest.Mock).mockClear(); + }); + + it('does nothing when the row is dropped outside the list', () => { + const dispatch = jest.fn() as unknown as AppDispatch; + handleDragEnd([-1, -2, -3], dropResult(0, null), dispatch, jest.fn()); + expect(reorderExternalAssessments).not.toHaveBeenCalled(); + expect(dispatch).not.toHaveBeenCalled(); + }); + + it('does nothing when the row is dropped back in its original position', () => { + const dispatch = jest.fn() as unknown as AppDispatch; + handleDragEnd([-1, -2, -3], dropResult(1, 1), dispatch, jest.fn()); + expect(reorderExternalAssessments).not.toHaveBeenCalled(); + expect(dispatch).not.toHaveBeenCalled(); + }); + + it('dispatches the reordered ids on a valid move', () => { + const dispatch = jest.fn(() => Promise.resolve()) as unknown as AppDispatch; + handleDragEnd([-1, -2, -3], dropResult(2, 0), dispatch, jest.fn()); + expect(reorderExternalAssessments).toHaveBeenCalledWith([-3, -1, -2]); + expect(dispatch).toHaveBeenCalledTimes(1); + }); + + it('invokes the error callback when the reorder dispatch rejects', async () => { + const onError = jest.fn(); + const dispatch = jest.fn(() => + Promise.reject(new Error('nope')), + ) as unknown as AppDispatch; + handleDragEnd([-1, -2, -3], dropResult(0, 2), dispatch, onError); + await waitFor(() => expect(onError).toHaveBeenCalledTimes(1)); + }); +}); + +it('shows an empty state when there are no externals', async () => { + render(, { + state: { gradebook: { ...preloadedState.gradebook, assessments: [] } }, + }); + expect(await screen.findByText('No external assessments yet')).toBeVisible(); + expect(screen.queryByRole('table')).not.toBeInTheDocument(); +}); + +it('lists only external assessments', async () => { + render(, { + state: preloadedState, + }); + expect(await screen.findByText('Name')).toBeVisible(); + expect(await screen.findByText('Midterm')).toBeVisible(); + expect(screen.queryByText('Native quiz')).not.toBeInTheDocument(); +}); + +it('opens the add dialog', async () => { + render(, { + state: preloadedState, + }); + await userEvent.click(await screen.findByRole('button', { name: 'Add' })); + await waitFor(() => + expect(screen.getByText('Add external assessment')).toBeVisible(), + ); +}); + +it('opens the edit prompt for a row', async () => { + render(, { + state: preloadedState, + }); + await userEvent.click( + await screen.findByRole('button', { name: 'edit Midterm' }), + ); + await waitFor(() => + expect(screen.getByText('Edit external assessment')).toBeVisible(), + ); +}); + +it('opens the delete prompt for a row', async () => { + render(, { + state: preloadedState, + }); + await userEvent.click( + await screen.findByRole('button', { name: 'delete Midterm' }), + ); + await waitFor(() => expect(screen.getByRole('dialog')).toBeVisible()); +}); + +it('invokes onClose when the close button is clicked', async () => { + const onClose = jest.fn(); + render(, { + state: preloadedState, + }); + await userEvent.click(await screen.findByRole('button', { name: 'Close' })); + expect(onClose).toHaveBeenCalledTimes(1); +}); + +it('opens the edit dialog for a row', async () => { + render(, { + state: preloadedState, + }); + + await userEvent.click(await screen.findByLabelText('edit Midterm')); + expect(await screen.findByText('Edit external assessment')).toBeVisible(); +}); + +it('seeds the edit dialog weightage from the tab weight (weighted view on)', async () => { + render(, { + state: { + gradebook: { + ...preloadedState.gradebook, + weightedViewEnabled: true, + }, + }, + }); + + await userEvent.click(await screen.findByLabelText('edit Midterm')); + expect(await screen.findByText('Edit external assessment')).toBeVisible(); + expect(await screen.findByLabelText('Weightage')).toHaveValue(25); +}); + +it('opens the delete confirmation for a row', async () => { + render(, { + state: preloadedState, + }); + + await userEvent.click(await screen.findByLabelText('delete Midterm')); + expect(await screen.findByText('Delete external assessment')).toBeVisible(); + // the confirmation body names the assessment being deleted + expect( + screen.getByText( + /Delete "Midterm"\? This permanently removes the column and every student grade in it\. This cannot be undone\./, + ), + ).toBeVisible(); +}); + +it('hides weights when weighted view is disabled', async () => { + render(, { + state: preloadedState, + }); + + await screen.findByText('Midterm'); + expect(screen.queryByText('Weight')).not.toBeInTheDocument(); + expect(screen.queryByText('25')).not.toBeInTheDocument(); +}); + +it('shows external assessment weights when weighted view is enabled', async () => { + render(, { + state: { + gradebook: { + ...preloadedState.gradebook, + weightedViewEnabled: true, + }, + }, + }); + + expect(await screen.findByText('Weight')).toBeVisible(); + expect(screen.getByText('25')).toBeVisible(); +}); + +it('renders bounds chips per the floor/cap flags', async () => { + const stateWithMixedBounds = { + gradebook: { + ...preloadedState.gradebook, + assessments: [ + { + id: -1, + title: 'Midterm', + tabId: -1, + maxGrade: 50, + external: true, + floorAtZero: true, + capAtMaximum: false, + }, + ], + }, + }; + render(, { + state: stateWithMixedBounds, + }); + + expect(await screen.findByText('≥ 0')).toBeVisible(); + expect(screen.queryByText('≤ max')).not.toBeInTheDocument(); +}); + +it('shows both bounds chips by default when flags are absent', async () => { + const stateWithDefaultBounds = { + gradebook: { + ...preloadedState.gradebook, + assessments: [ + { id: -1, title: 'Midterm', tabId: -1, maxGrade: 50, external: true }, + ], + }, + }; + render(, { + state: stateWithDefaultBounds, + }); + + expect(await screen.findByText('≥ 0')).toBeVisible(); + expect(screen.getByText('≤ max')).toBeVisible(); +}); + +it('shows the ≥ 0 and ≤ max chips for a floored and capped external', async () => { + render(, { + state: externalWith({ floorAtZero: true, capAtMaximum: true }), + }); + + expect(await screen.findByText('≥ 0')).toBeVisible(); + expect(screen.getByText('≤ max')).toBeVisible(); + expect(screen.getByText('50')).toBeVisible(); +}); + +it('hides both bound chips when an external is neither floored nor capped', async () => { + render(, { + state: externalWith({ floorAtZero: false, capAtMaximum: false }), + }); + + await screen.findByText('Midterm'); + expect(screen.queryByText('≥ 0')).not.toBeInTheDocument(); + expect(screen.queryByText('≤ max')).not.toBeInTheDocument(); +}); + +it('renders a drag handle per external assessment', async () => { + const page = render( + , + { + state: preloadedState, + }, + ); + expect(await page.findByLabelText('reorder Midterm')).toBeVisible(); + expect(await page.findByLabelText('reorder Final')).toBeVisible(); +}); + +it('hides the drag handle when there is only one external assessment', async () => { + const page = render( + , + { + state: { + gradebook: { + ...preloadedState.gradebook, + assessments: [ + { + id: -1, + title: 'Midterm', + tabId: -1, + maxGrade: 50, + external: true, + floorAtZero: true, + capAtMaximum: true, + }, + ], + }, + }, + }, + ); + // The row still renders, with its grade in place... + expect(await page.findByText('Midterm')).toBeVisible(); + expect(page.getByText('50')).toBeVisible(); + // ...but the lone row has no reorder handle, so it cannot be dragged. + expect(page.queryByLabelText('reorder Midterm')).not.toBeInTheDocument(); +}); + +describe('moveItem', () => { + it('moves an item from one index to another, preserving the rest', () => { + expect(moveItem([-1, -2, -3], 2, 0)).toEqual([-3, -1, -2]); + expect(moveItem([-1, -2, -3], 0, 2)).toEqual([-2, -3, -1]); + }); +}); diff --git a/client/app/bundles/course/gradebook/__tests__/WeightedGradebookTable.test.tsx b/client/app/bundles/course/gradebook/__tests__/WeightedGradebookTable.test.tsx index 262b195788..1ee1086468 100644 --- a/client/app/bundles/course/gradebook/__tests__/WeightedGradebookTable.test.tsx +++ b/client/app/bundles/course/gradebook/__tests__/WeightedGradebookTable.test.tsx @@ -4,6 +4,7 @@ import { render, screen, waitFor, within } from 'test-utils'; import TestApp from 'utilities/TestApp'; import WeightedGradebookTable from '../components/WeightedGradebookTable'; +import { LEVEL_ASSESSMENT_ID, LEVEL_TAB_ID } from '../computeWeighted'; import type { AssessmentData, CategoryData, @@ -957,9 +958,11 @@ describe('WeightedGradebookTable', () => { }); await user.click(screen.getByRole('button', { name: /expand Alice/i })); - // Synthetic Level breakdown row (tabId -1, assessmentId -1): its + // Synthetic Level breakdown row (LEVEL_TAB_ID/LEVEL_ASSESSMENT_ID): its // contribution (15) sits under Level Contribution, tab columns stay empty. - const levelRow = await screen.findByTestId(breakdownRowId(1, -1, -1)); + const levelRow = await screen.findByTestId( + breakdownRowId(1, LEVEL_TAB_ID, LEVEL_ASSESSMENT_ID), + ); const cells = within(levelRow).getAllByRole('cell'); expect(cells).toHaveLength(7); expect(cells[3]).toHaveTextContent('15'); // Level Contribution @@ -1786,10 +1789,11 @@ describe('level contribution columns', () => { }); await user.click(screen.getByRole('button', { name: /expand Alice/i })); const bdRows = await screen.findAllByTestId(/^breakdown-row-/); - // Synthetic level tab uses LEVEL_TAB_ID (-1); its row must come first, before - // the real tab (id 10) rows — mirroring the column order. + // Synthetic level tab uses LEVEL_TAB_ID (0, disjoint from positive real tab + // ids and negative external-assessment pseudo-tab ids); its row must come + // first, before the real tab (id 10) rows — mirroring the column order. expect(bdRows[0].getAttribute('data-testid')).toMatch( - /^breakdown-row-1--1-/, + new RegExp(`^breakdown-row-1-${LEVEL_TAB_ID}-${LEVEL_ASSESSMENT_ID}$`), ); expect( bdRows.some((r) => r.getAttribute('data-testid')?.includes('-10-')), @@ -1813,7 +1817,11 @@ describe('level contribution columns', () => { submissions: [makeSub(1, 100, 8)], }); await user.click(screen.getByRole('button', { name: /expand Alice/i })); - const levelRow = (await screen.findAllByTestId(/^breakdown-row-1--1-/))[0]; + const levelRow = ( + await screen.findAllByTestId( + new RegExp(`^breakdown-row-1-${LEVEL_TAB_ID}-`), + ) + )[0]; expect(within(levelRow).getByText(/Level 14/)).toBeInTheDocument(); // The misleading "14/20" fraction must NOT appear. expect( @@ -1857,7 +1865,11 @@ describe('level contribution columns', () => { }); await user.click(screen.getByRole('radio', { name: /percentage/i })); await user.click(screen.getByRole('button', { name: /expand Alice/i })); - const levelRow = (await screen.findAllByTestId(/^breakdown-row-1--1-/))[0]; + const levelRow = ( + await screen.findAllByTestId( + new RegExp(`^breakdown-row-1-${LEVEL_TAB_ID}-`), + ) + )[0]; // The breakdown's level cell mirrors the summary cell: 15 / 30 → 50%. expect(within(levelRow).getByText('50%')).toBeInTheDocument(); }); diff --git a/client/app/bundles/course/gradebook/__tests__/computeWeighted.test.ts b/client/app/bundles/course/gradebook/__tests__/computeWeighted.test.ts index ef94ec6a4c..b0ba83e5ce 100644 --- a/client/app/bundles/course/gradebook/__tests__/computeWeighted.test.ts +++ b/client/app/bundles/course/gradebook/__tests__/computeWeighted.test.ts @@ -890,6 +890,60 @@ const lc = ( ...over, }); +describe('computeStudentBreakdown - level/external synthetic-id collision', () => { + // An external assessment whose primary key is 1 is rendered as a negative-id + // pseudo-tab (synthetic_tab_id = -id = -1), with its assessment id likewise + // negated (-1). The Level-contribution row historically also used -1 for both + // its tabId and assessmentId, so the two collided: indistinguishable by tabId + // and producing duplicate `bd---1--1` React keys/testids. + it('keeps the level row distinct from external assessment #1', () => { + // External #1 as a pseudo-tab: tabId -1, single assessment id -1. + const externalTab = { + id: -1, + title: 'Midterm (external)', + categoryId: 99, + gradebookWeight: 20, + }; + const externalAssessment = { + id: -1, + tabId: -1, + maxGrade: 100, + title: 'Midterm (external)', + }; + + const breakdown = computeStudentBreakdown({ + studentId: 1, + tabs: [externalTab], + assessments: [externalAssessment], + submissions: subs([{ studentId: 1, assessmentId: -1, grade: 90 }]), + level: 15, + levelContribution: lc(), // enabled: true + levelContributionPoints: 4, // non-null → the level row is pushed + courseMaxLevel: 30, + }); + + // One external pseudo-tab row + one level row = two DISTINCT rows. + expect(breakdown).toHaveLength(2); + + // The LEVEL_TAB_ID filter must select exactly the level row — and only it. + const levelRows = breakdown.filter((tb) => tb.tabId === LEVEL_TAB_ID); + expect(levelRows).toHaveLength(1); + expect(levelRows[0].assessments[0].title).toBe('Level'); + + // The external row must NOT be classified as the level row. + const externalRow = breakdown.find( + (tb) => tb.assessments[0]?.title === 'Midterm (external)', + )!; + expect(externalRow.tabId).not.toBe(LEVEL_TAB_ID); + + // Row identity keys `${tabId}-${assessmentId}` must be unique (no dup React keys). + const keys = breakdown.map( + (tb) => `${tb.tabId}-${tb.assessments[0].assessmentId}`, + ); + expect(new Set(keys).size).toBe(2); + }); +}); + describe('computeWeightedRows - level contribution', () => { const baseStudent = { id: 1, diff --git a/client/app/bundles/course/gradebook/__tests__/operations.test.ts b/client/app/bundles/course/gradebook/__tests__/operations.test.ts index e1a3fca281..1e9813cd42 100644 --- a/client/app/bundles/course/gradebook/__tests__/operations.test.ts +++ b/client/app/bundles/course/gradebook/__tests__/operations.test.ts @@ -1,32 +1,367 @@ +import type { AppDispatch } from 'store'; + import CourseAPI from 'api/course'; -import { updateGradebookWeights } from '../operations'; +import fetchGradebook, { + createExternalAssessment, + deleteExternalAssessment, + editExternalAssessment, + reorderExternalAssessments, + setExternalGrade, + updateGradebookWeights, +} from '../operations'; + +const REORDER = 'course/gradebook/REORDER_EXTERNAL_ASSESSMENTS'; + +const externals = [ + { id: -1, title: 'A', tabId: -1, maxGrade: 10, external: true }, + { id: -2, title: 'B', tabId: -2, maxGrade: 10, external: true }, +]; + +// Minimal thunk harness: record plain actions, stub getState. +const harness = (): { + dispatched: { type: string; payload: unknown }[]; + dispatch: AppDispatch; + getState: () => never; +} => { + const dispatched: { type: string; payload: unknown }[] = []; + return { + dispatched, + // The operation only dispatches plain { type, payload } actions, which we + // record; cast to AppDispatch so it satisfies the thunk's dispatch param. + dispatch: ((a: { type: string; payload: unknown }) => { + dispatched.push(a); + return a; + }) as unknown as AppDispatch, + getState: () => ({ gradebook: { assessments: externals } }) as never, + }; +}; + +describe('reorderExternalAssessments', () => { + afterEach(() => jest.restoreAllMocks()); + + it('applies optimistically, then reverts and rethrows when the PUT fails', async () => { + const { dispatched, dispatch, getState } = harness(); + jest + .spyOn(CourseAPI.gradebook, 'reorderExternals') + .mockRejectedValue(new Error('network')); + + await expect( + reorderExternalAssessments([-2, -1])(dispatch, getState, {}), + ).rejects.toThrow('network'); + + const reorders = dispatched.filter((a) => a.type === REORDER); + expect(reorders[0].payload).toEqual([-2, -1]); // optimistic apply + expect(reorders[reorders.length - 1].payload).toEqual([-1, -2]); // rolled back to original + }); + + it('does not roll back on success', async () => { + const { dispatched, dispatch, getState } = harness(); + jest + .spyOn(CourseAPI.gradebook, 'reorderExternals') + .mockResolvedValue({ data: undefined } as never); + + await reorderExternalAssessments([-2, -1])(dispatch, getState, {}); + + const reorders = dispatched.filter((a) => a.type === REORDER); + expect(reorders).toHaveLength(1); + expect(reorders[0].payload).toEqual([-2, -1]); + }); + + it('calls the API with positive external ids (negated store ids)', async () => { + const { dispatch, getState } = harness(); + const spy = jest + .spyOn(CourseAPI.gradebook, 'reorderExternals') + .mockResolvedValue({ data: undefined } as never); + + await reorderExternalAssessments([-2, -1])(dispatch, getState, {}); + + expect(spy).toHaveBeenCalledWith({ orderedIds: [2, 1] }); + }); + + it('rolls back only external assessments in their stored order', async () => { + const dispatched: { type: string; payload: unknown }[] = []; + const dispatch = ((a: { type: string; payload: unknown }) => { + dispatched.push(a); + return a; + }) as unknown as AppDispatch; + const getState = (() => ({ + gradebook: { + assessments: [ + { id: 5, external: false }, + ...externals, // ids -1, -2 + ], + }, + })) as unknown as () => never; + jest + .spyOn(CourseAPI.gradebook, 'reorderExternals') + .mockRejectedValue(new Error('network')); + + await expect( + reorderExternalAssessments([-2, -1])(dispatch, getState, {}), + ).rejects.toThrow('network'); + + const reorders = dispatched.filter((a) => a.type === REORDER); + expect(reorders[reorders.length - 1].payload).toEqual([-1, -2]); + }); +}); + +describe('setExternalGrade', () => { + afterEach(() => jest.restoreAllMocks()); + + const gradeHarness = ( + submissions: { + studentId: number; + assessmentId: number; + grade: number | null; + }[], + ): { + dispatched: { type: string; payload: unknown }[]; + dispatch: AppDispatch; + getState: () => never; + } => { + const dispatched: { type: string; payload: unknown }[] = []; + return { + dispatched, + dispatch: ((a: { type: string; payload: unknown }) => { + dispatched.push(a); + return a; + }) as unknown as AppDispatch, + getState: () => ({ gradebook: { submissions } }) as never, + }; + }; + + const SET = 'course/gradebook/SET_EXTERNAL_GRADE'; + + it('applies optimistically, calls the API with the negated id, then reconciles', async () => { + const { dispatched, dispatch, getState } = gradeHarness([ + { studentId: 7, assessmentId: -1, grade: 3 }, + ]); + const spy = jest + .spyOn(CourseAPI.gradebook, 'setExternalGrade') + .mockResolvedValue({ + data: { studentId: 7, assessmentId: -1, grade: 9 }, + } as never); + + await setExternalGrade(-1, 7, 9)(dispatch, getState, {}); + + expect(spy).toHaveBeenCalledWith(1, { studentId: 7, grade: 9 }); + const sets = dispatched.filter((a) => a.type === SET); + expect(sets[0].payload).toEqual({ + studentId: 7, + assessmentId: -1, + grade: 9, + }); // optimistic + expect(sets[sets.length - 1].payload).toEqual({ + studentId: 7, + assessmentId: -1, + grade: 9, + }); // reconciled from response.data + }); + + it('restores the prior grade and rethrows when the PUT fails', async () => { + const { dispatched, dispatch, getState } = gradeHarness([ + { studentId: 7, assessmentId: -1, grade: 3 }, + ]); + jest + .spyOn(CourseAPI.gradebook, 'setExternalGrade') + .mockRejectedValue(new Error('network')); + + await expect( + setExternalGrade(-1, 7, 9)(dispatch, getState, {}), + ).rejects.toThrow('network'); + + const sets = dispatched.filter((a) => a.type === SET); + expect(sets[0].payload).toEqual({ + studentId: 7, + assessmentId: -1, + grade: 9, + }); // optimistic + expect(sets[sets.length - 1].payload).toEqual({ + studentId: 7, + assessmentId: -1, + grade: 3, + }); // rolled back to prev + }); + + it('rolls back to null when no prior submission exists', async () => { + const { dispatched, dispatch, getState } = gradeHarness([]); + jest + .spyOn(CourseAPI.gradebook, 'setExternalGrade') + .mockRejectedValue(new Error('network')); + + await expect( + setExternalGrade(-1, 7, 9)(dispatch, getState, {}), + ).rejects.toThrow('network'); + + const sets = dispatched.filter((a) => a.type === SET); + expect(sets[sets.length - 1].payload).toEqual({ + studentId: 7, + assessmentId: -1, + grade: null, + }); + }); +}); + +describe('createExternalAssessment', () => { + afterEach(() => jest.restoreAllMocks()); + + const createHarness = (): { + dispatch: AppDispatch; + getState: () => never; + } => ({ + dispatch: ((a: unknown) => a) as unknown as AppDispatch, + getState: (() => ({})) as unknown as () => never, + }); + + it('omits weight from the payload when undefined', async () => { + const { dispatch, getState } = createHarness(); + const spy = jest + .spyOn(CourseAPI.gradebook, 'createExternal') + .mockResolvedValue({ data: {} } as never); + + await createExternalAssessment( + 'A', + 10, + true, + false, + )(dispatch, getState, {}); + + expect(spy).toHaveBeenCalledWith({ + title: 'A', + maximumGrade: 10, + floorAtZero: true, + capAtMaximum: false, + }); + }); + + it('includes weight in the payload when provided', async () => { + const { dispatch, getState } = createHarness(); + const spy = jest + .spyOn(CourseAPI.gradebook, 'createExternal') + .mockResolvedValue({ data: {} } as never); + + await createExternalAssessment( + 'A', + 10, + true, + false, + 2, + )(dispatch, getState, {}); + + expect(spy).toHaveBeenCalledWith({ + title: 'A', + maximumGrade: 10, + floorAtZero: true, + capAtMaximum: false, + weight: 2, + }); + }); +}); + +describe('id-negating thunks', () => { + afterEach(() => jest.restoreAllMocks()); + + const noopHarness = (): { dispatch: AppDispatch; getState: () => never } => ({ + dispatch: ((a: unknown) => a) as unknown as AppDispatch, + getState: (() => ({})) as unknown as () => never, + }); + + it('editExternalAssessment negates the id and forwards the patch', async () => { + const { dispatch, getState } = noopHarness(); + const spy = jest + .spyOn(CourseAPI.gradebook, 'updateExternal') + .mockResolvedValue({ data: {} } as never); + + await editExternalAssessment(-3, { maximumGrade: 20 })( + dispatch, + getState, + {}, + ); -jest.mock('api/course', () => ({ - gradebook: { - updateWeights: jest.fn(), - }, -})); + expect(spy).toHaveBeenCalledWith(3, { maximumGrade: 20 }); + }); + + it('deleteExternalAssessment negates the id for the API but dispatches the original id', async () => { + const dispatched: { type: string; payload: unknown }[] = []; + const dispatch = ((a: { type: string; payload: unknown }) => { + dispatched.push(a); + return a; + }) as unknown as AppDispatch; + const spy = jest + .spyOn(CourseAPI.gradebook, 'deleteExternal') + .mockResolvedValue({ data: undefined } as never); + + await deleteExternalAssessment(-3)(dispatch, (() => ({})) as never, {}); + + expect(spy).toHaveBeenCalledWith(3); + expect(dispatched).toContainEqual({ + type: 'course/gradebook/DELETE_EXTERNAL_ASSESSMENT', + payload: -3, + }); + }); +}); + +describe('simple pass-through thunks', () => { + afterEach(() => jest.restoreAllMocks()); + + const passHarness = (): { + dispatched: { type: string; payload: unknown }[]; + dispatch: AppDispatch; + getState: () => never; + } => { + const dispatched: { type: string; payload: unknown }[] = []; + return { + dispatched, + dispatch: ((a: { type: string; payload: unknown }) => { + dispatched.push(a); + return a; + }) as unknown as AppDispatch, + getState: (() => ({})) as unknown as () => never, + }; + }; -const mockedUpdateWeights = CourseAPI.gradebook.updateWeights as jest.Mock; + it('fetchGradebook dispatches saveGradebook with the index response', async () => { + const { dispatched, dispatch, getState } = passHarness(); + jest + .spyOn(CourseAPI.gradebook, 'index') + .mockResolvedValue({ data: { loaded: true } } as never); -describe('updateGradebookWeights', () => { - beforeEach(() => { - mockedUpdateWeights.mockReset(); + await fetchGradebook()(dispatch, getState, {}); + + expect(dispatched).toContainEqual({ + type: 'course/gradebook/SAVE_GRADEBOOK', + payload: { loaded: true }, + }); + }); + + it('updateGradebookWeights wraps weights and dispatches updateTabWeights', async () => { + const { dispatched, dispatch, getState } = passHarness(); + const spy = jest + .spyOn(CourseAPI.gradebook, 'updateWeights') + .mockResolvedValue({ data: { weights: [] } } as never); + + await updateGradebookWeights([])(dispatch, getState, {}); + + expect(spy).toHaveBeenCalledWith({ weights: [] }); + expect(dispatched).toContainEqual({ + type: 'course/gradebook/UPDATE_TAB_WEIGHTS', + payload: { weights: [] }, + }); }); // The backend response does NOT echo formulaAst. The operation must merge the // client-side formulaAst back into the dispatched payload so the reducer can // optimistically recompute each student's levelContribution. Dispatching the // raw response strands formulaAst undefined → reducer nulls every student. - it('merges formulaAst back into the dispatched action', async () => { + it('merges formulaAst back into the dispatched updateTabWeights action', async () => { const formulaAst = { type: 'call2' as const, fn: 'min' as const, a: { type: 'var' as const, name: 'level' as const }, b: { type: 'num' as const, value: 30 }, }; - mockedUpdateWeights.mockResolvedValue({ + jest.spyOn(CourseAPI.gradebook, 'updateWeights').mockResolvedValue({ data: { weights: [], levelContribution: { @@ -36,9 +371,14 @@ describe('updateGradebookWeights', () => { show: false, }, }, - }); + } as never); + + const dispatched: { type: string; payload: unknown }[] = []; + const dispatch = ((a: { type: string; payload: unknown }) => { + dispatched.push(a); + return a; + }) as unknown as AppDispatch; - const dispatch = jest.fn(); await updateGradebookWeights([], { enabled: true, formula: 'min(level, 30)', @@ -46,10 +386,12 @@ describe('updateGradebookWeights', () => { weight: 10, show: false, clamp: true, - })(dispatch, jest.fn(), {}); + })(dispatch, (() => ({})) as never, {}); - expect(dispatch).toHaveBeenCalledTimes(1); - const dispatched = dispatch.mock.calls[0][0]; - expect(dispatched.payload.levelContribution.formulaAst).toEqual(formulaAst); + expect(dispatched).toHaveLength(1); + const action = dispatched[0] as { + payload: { levelContribution: { formulaAst: unknown } }; + }; + expect(action.payload.levelContribution.formulaAst).toEqual(formulaAst); }); }); diff --git a/client/app/bundles/course/gradebook/__tests__/store.test.ts b/client/app/bundles/course/gradebook/__tests__/store.test.ts index cde8e561a3..6eeb7e83ab 100644 --- a/client/app/bundles/course/gradebook/__tests__/store.test.ts +++ b/client/app/bundles/course/gradebook/__tests__/store.test.ts @@ -1,5 +1,7 @@ import reducer, { actions } from '../store'; +const EXTERNAL_ASSESSMENTS = 'External Assessments'; + const baseState = { categories: [], tabs: [ @@ -25,6 +27,61 @@ const baseState = { }, }; +describe('SAVE_GRADEBOOK reducer', () => { + it('returns the initial state for an unknown action', () => { + const next = reducer(undefined, { type: 'unknown' } as never); + expect(next).toEqual({ + categories: [], + tabs: [], + assessments: [], + students: [], + submissions: [], + gamificationEnabled: false, + weightedViewEnabled: false, + canManageWeights: false, + courseMaxLevel: 0, + levelContribution: { + enabled: false, + formula: '', + weight: 0, + show: false, + clamp: true, + }, + }); + }); + + it('hydrates every field from the payload', () => { + const next = reducer( + undefined, + actions.saveGradebook({ + categories: [{ id: 1, title: 'C' }], + tabs: [{ id: 10, title: 'T', categoryId: 1, gradebookWeight: 50 }], + assessments: [{ id: 100, title: 'A', tabId: 10, maxGrade: 100 }], + students: [], + submissions: [{ studentId: 1, assessmentId: 100, grade: 8 }], + gamificationEnabled: true, + weightedViewEnabled: true, + canManageWeights: true, + courseMaxLevel: 0, + levelContribution: { + enabled: false, + formula: '', + weight: 0, + show: false, + clamp: true, + }, + }), + ); + expect(next.categories).toHaveLength(1); + expect(next.tabs[0].gradebookWeight).toBe(50); + expect(next.assessments[0].id).toBe(100); + expect(next.submissions[0].grade).toBe(8); + expect(next.gamificationEnabled).toBe(true); + expect(next.weightedViewEnabled).toBe(true); + expect(next.canManageWeights).toBe(true); + }); +}); + describe('UPDATE_TAB_WEIGHTS reducer', () => { it('updates gradebookWeight and weightMode for the matching tab', () => { const next = reducer( @@ -38,7 +95,7 @@ describe('UPDATE_TAB_WEIGHTS reducer', () => { expect(next.tabs.find((t) => t.id === 2)?.gradebookWeight).toBe(50); }); - it('does not set any excluded field', () => { + it('does not set any excluded field in tabs', () => { const next = reducer( baseState, actions.updateTabWeights({ @@ -166,6 +223,40 @@ describe('UPDATE_TAB_WEIGHTS reducer', () => { false, ); }); + + it('clears assessment exclusion when excludedAssessmentIds is omitted', () => { + const seeded = { + ...baseState, + assessments: [ + { + id: 101, + title: 'A1', + tabId: 1, + maxGrade: 100, + gradebookExcluded: true, + }, + { + id: 102, + title: 'A2', + tabId: 1, + maxGrade: 100, + gradebookExcluded: true, + }, + ], + }; + const next = reducer( + seeded, + actions.updateTabWeights({ + weights: [{ tabId: 1, weight: 50, weightMode: 'equal' }], + }), + ); + expect(next.assessments.find((a) => a.id === 101)?.gradebookExcluded).toBe( + false, + ); + expect(next.assessments.find((a) => a.id === 102)?.gradebookExcluded).toBe( + false, + ); + }); }); describe('level contribution', () => { @@ -540,6 +631,179 @@ describe('external assessment reducers', () => { courseMaxLevel: 0, }; + it('applyCreatedExternal adds category, tab and assessment', () => { + const next = reducer( + state, + actions.applyCreatedExternal({ + assessment: { + id: -5, + title: 'Midterm', + tabId: 200, + maxGrade: 50, + external: true, + }, + tab: { id: 200, title: 'Midterm', categoryId: 2 }, + category: { id: 2, title: EXTERNAL_ASSESSMENTS }, + }), + ); + expect(next.categories.find((c) => c.id === 2)?.title).toBe( + EXTERNAL_ASSESSMENTS, + ); + expect(next.tabs.find((t) => t.id === 200)?.title).toBe('Midterm'); + expect(next.assessments.find((a) => a.id === -5)?.external).toBe(true); + }); + + it('applyCreatedExternal does not duplicate existing category, tab, or assessment', () => { + const seeded = { + ...state, + categories: [...state.categories, { id: 2, title: EXTERNAL_ASSESSMENTS }], + tabs: [...state.tabs, { id: 200, title: 'Midterm', categoryId: 2 }], + }; + const next = reducer( + seeded, + actions.applyCreatedExternal({ + assessment: { + id: -6, + title: 'Final', + tabId: 200, + maxGrade: 100, + external: true, + }, + tab: { id: 200, title: 'Midterm', categoryId: 2 }, + category: { id: 2, title: EXTERNAL_ASSESSMENTS }, + }), + ); + expect(next.categories.filter((c) => c.id === 2)).toHaveLength(1); + expect(next.tabs.filter((t) => t.id === 200)).toHaveLength(1); + expect(next.assessments.filter((a) => a.id === -6)).toHaveLength(1); + }); + + it('updateExternalAssessment changes title and maxGrade and syncs tab title', () => { + const seeded = { + ...state, + assessments: [ + ...state.assessments, + { id: -5, title: 'Midterm', tabId: 200, maxGrade: 50, external: true }, + ], + tabs: [...state.tabs, { id: 200, title: 'Midterm', categoryId: 2 }], + }; + const next = reducer( + seeded, + actions.updateExternalAssessment({ + assessment: { + id: -5, + title: 'Midterm Exam', + tabId: 200, + maxGrade: 60, + external: true, + }, + tab: { id: 200, title: 'Midterm Exam', categoryId: 2 }, + }), + ); + expect(next.assessments.find((a) => a.id === -5)?.title).toBe( + 'Midterm Exam', + ); + expect(next.assessments.find((a) => a.id === -5)?.maxGrade).toBe(60); + expect(next.tabs.find((t) => t.id === 200)?.title).toBe('Midterm Exam'); + }); + + it('updateExternalAssessment writes tab.gradebookWeight when provided and preserves it when omitted', () => { + const seeded = { + ...state, + assessments: [ + ...state.assessments, + { id: -5, title: 'Midterm', tabId: 200, maxGrade: 50, external: true }, + ], + tabs: [ + ...state.tabs, + { id: 200, title: 'Midterm', categoryId: 2, gradebookWeight: 40 }, + ], + }; + const withWeight = reducer( + seeded, + actions.updateExternalAssessment({ + assessment: { + id: -5, + title: 'M', + tabId: 200, + maxGrade: 50, + external: true, + }, + tab: { id: 200, title: 'M', categoryId: 2, gradebookWeight: 75 }, + }), + ); + expect(withWeight.tabs.find((t) => t.id === 200)?.gradebookWeight).toBe(75); + + const noWeight = reducer( + seeded, + actions.updateExternalAssessment({ + assessment: { + id: -5, + title: 'M', + tabId: 200, + maxGrade: 50, + external: true, + }, + tab: { id: 200, title: 'M', categoryId: 2 }, + }), + ); + expect(noWeight.tabs.find((t) => t.id === 200)?.gradebookWeight).toBe(40); + }); + + it('deleteExternalAssessment removes the assessment and its now-empty tab', () => { + const seeded = { + ...state, + assessments: [ + ...state.assessments, + { id: -5, title: 'Midterm', tabId: 200, maxGrade: 50, external: true }, + ], + tabs: [...state.tabs, { id: 200, title: 'Midterm', categoryId: 2 }], + submissions: [ + ...state.submissions, + { studentId: 1, assessmentId: -5, grade: 30 }, + ], + }; + const next = reducer(seeded, actions.deleteExternalAssessment(-5)); + expect(next.assessments.find((a) => a.id === -5)).toBeUndefined(); + expect(next.tabs.find((t) => t.id === 200)).toBeUndefined(); + expect(next.submissions.find((s) => s.assessmentId === -5)).toBeUndefined(); + }); + + it('deleteExternalAssessment drops the synthetic category once its last external is gone', () => { + const seeded = { + ...state, + categories: [...state.categories, { id: 2, title: EXTERNAL_ASSESSMENTS }], + assessments: [ + ...state.assessments, + { id: -5, title: 'Midterm', tabId: 200, maxGrade: 50, external: true }, + ], + tabs: [...state.tabs, { id: 200, title: 'Midterm', categoryId: 2 }], + }; + const next = reducer(seeded, actions.deleteExternalAssessment(-5)); + expect(next.categories.find((c) => c.id === 2)).toBeUndefined(); + expect(next.categories.find((c) => c.id === 1)).toBeDefined(); + }); + + it('deleteExternalAssessment keeps the category while other externals remain', () => { + const seeded = { + ...state, + categories: [...state.categories, { id: 2, title: EXTERNAL_ASSESSMENTS }], + assessments: [ + ...state.assessments, + { id: -5, title: 'Midterm', tabId: 200, maxGrade: 50, external: true }, + { id: -6, title: 'Final', tabId: 201, maxGrade: 100, external: true }, + ], + tabs: [ + ...state.tabs, + { id: 200, title: 'Midterm', categoryId: 2 }, + { id: 201, title: 'Final', categoryId: 2 }, + ], + }; + const next = reducer(seeded, actions.deleteExternalAssessment(-5)); + expect(next.categories.find((c) => c.id === 2)).toBeDefined(); + expect(next.tabs.find((t) => t.id === 201)).toBeDefined(); + }); + it('setExternalGrade upserts a submission row by (studentId, assessmentId)', () => { const inserted = reducer( state, @@ -577,4 +841,95 @@ describe('external assessment reducers', () => { ?.grade, ).toBeNull(); }); + + it('UPDATE_EXTERNAL_ASSESSMENT copies bound flags', () => { + const seed = (overrides = {}): ReturnType => + reducer( + undefined, + actions.saveGradebook({ + categories: [], + tabs: [{ id: -1, title: 'Ext', categoryId: -1 }], + assessments: [ + { + id: -1, + title: 'Ext', + tabId: -1, + maxGrade: 50, + external: true, + floorAtZero: true, + capAtMaximum: true, + }, + ], + students: [], + submissions: [], + gamificationEnabled: false, + weightedViewEnabled: true, + canManageWeights: true, + courseMaxLevel: 0, + levelContribution: { + enabled: false, + formula: '', + weight: 0, + show: false, + clamp: true, + }, + ...overrides, + }), + ); + + const next = reducer( + seed(), + actions.updateExternalAssessment({ + assessment: { + id: -1, + title: 'Ext', + tabId: -1, + maxGrade: 50, + external: true, + floorAtZero: false, + capAtMaximum: false, + }, + tab: { id: -1, title: 'Ext', categoryId: -1 }, + }), + ); + const a = next.assessments.find((x) => x.id === -1)!; + expect(a.floorAtZero).toBe(false); + expect(a.capAtMaximum).toBe(false); + }); + + it('REORDER_EXTERNAL_ASSESSMENTS permutes external assessments and their synthetic tabs', () => { + const externalState = { + ...baseState, + tabs: [ + { id: -1, title: 'Quiz', categoryId: -1 }, + { id: -2, title: 'Exam', categoryId: -1 }, + ], + assessments: [ + { id: -1, title: 'Quiz', tabId: -1, maxGrade: 10, external: true }, + { id: -2, title: 'Exam', tabId: -2, maxGrade: 20, external: true }, + ], + }; + const next = reducer( + externalState, + actions.reorderExternalAssessments([-2, -1]), + ); + expect(next.assessments.map((a) => a.id)).toEqual([-2, -1]); + expect(next.tabs.map((t) => t.id)).toEqual([-2, -1]); + }); + + it('deleteExternalAssessment is a no-op for an unknown id', () => { + const seeded = { + ...state, + categories: [...state.categories, { id: 2, title: EXTERNAL_ASSESSMENTS }], + assessments: [ + ...state.assessments, + { id: -5, title: 'Midterm', tabId: 200, maxGrade: 50, external: true }, + ], + tabs: [...state.tabs, { id: 200, title: 'Midterm', categoryId: 2 }], + }; + const next = reducer(seeded, actions.deleteExternalAssessment(-999)); + expect(next.assessments.find((a) => a.id === -5)).toBeDefined(); + expect(next.tabs.find((t) => t.id === 200)).toBeDefined(); + expect(next.categories.find((c) => c.id === 2)).toBeDefined(); + }); }); diff --git a/client/app/bundles/course/gradebook/components/AddExternalColumnPrompt.tsx b/client/app/bundles/course/gradebook/components/AddExternalColumnPrompt.tsx new file mode 100644 index 0000000000..f872da82f8 --- /dev/null +++ b/client/app/bundles/course/gradebook/components/AddExternalColumnPrompt.tsx @@ -0,0 +1,207 @@ +import { FC, useState } from 'react'; +import { defineMessages } from 'react-intl'; +import InfoOutlined from '@mui/icons-material/InfoOutlined'; +import { + Button, + Dialog, + DialogActions, + DialogContent, + DialogTitle, + FormControlLabel, + Switch, + TextField, + Tooltip, +} from '@mui/material'; + +import { useAppDispatch } from 'lib/hooks/store'; +import toast from 'lib/hooks/toast'; +import useTranslation from 'lib/hooks/useTranslation'; + +import { createExternalAssessment } from '../operations'; + +const translations = defineMessages({ + title: { + id: 'course.gradebook.AddExternalColumnPrompt.title', + defaultMessage: 'Add external assessment', + }, + nameLabel: { + id: 'course.gradebook.AddExternalColumnPrompt.nameLabel', + defaultMessage: 'Name', + }, + maxLabel: { + id: 'course.gradebook.AddExternalColumnPrompt.maxLabel', + defaultMessage: 'Max marks', + }, + weightLabel: { + id: 'course.gradebook.AddExternalColumnPrompt.weightLabel', + defaultMessage: 'Weightage', + }, + floorLabel: { + id: 'course.gradebook.AddExternalColumnPrompt.floorLabel', + defaultMessage: 'Floor grades at 0', + }, + capLabel: { + id: 'course.gradebook.AddExternalColumnPrompt.capLabel', + defaultMessage: 'Cap grades at max', + }, + floorHint: { + id: 'course.gradebook.AddExternalColumnPrompt.floorHint', + defaultMessage: + 'Counts negative grades as 0 when computing the weighted total. The actual grade is unchanged.', + }, + capHint: { + id: 'course.gradebook.AddExternalColumnPrompt.capHint', + defaultMessage: + 'Counts grades above the maximum as the maximum when computing the weighted total. The actual grade is unchanged.', + }, + cancel: { + id: 'course.gradebook.AddExternalColumnPrompt.cancel', + defaultMessage: 'Cancel', + }, + create: { + id: 'course.gradebook.AddExternalColumnPrompt.create', + defaultMessage: 'Create', + }, + error: { + id: 'course.gradebook.AddExternalColumnPrompt.error', + defaultMessage: 'Could not create the external assessment.', + }, + success: { + id: 'course.gradebook.AddExternalColumnPrompt.success', + defaultMessage: 'External assessment created.', + }, +}); + +interface Props { + open: boolean; + weightedViewEnabled?: boolean; + onClose: () => void; +} + +const AddExternalColumnPrompt: FC = ({ + open, + weightedViewEnabled = false, + onClose, +}) => { + const { t } = useTranslation(); + const dispatch = useAppDispatch(); + const [name, setName] = useState(''); + const [max, setMax] = useState(''); + const [floorAtZero, setFloorAtZero] = useState(true); + const [capAtMaximum, setCapAtMaximum] = useState(true); + const [saving, setSaving] = useState(false); + const [weight, setWeight] = useState('0'); + + const reset = (): void => { + setName(''); + setMax(''); + setFloorAtZero(true); + setCapAtMaximum(true); + setWeight('0'); + }; + + const canSave = + name.trim() !== '' && max.trim() !== '' && Number(max) >= 0 && !saving; + + const submit = async (): Promise => { + setSaving(true); + try { + await dispatch( + createExternalAssessment( + name.trim(), + Number(max), + floorAtZero, + capAtMaximum, + weightedViewEnabled ? Number(weight) : undefined, + ), + ); + toast.success(t(translations.success)); + reset(); + onClose(); + } catch { + toast.error(t(translations.error)); + } finally { + setSaving(false); + } + }; + + return ( + + {t(translations.title)} + + setName(e.target.value)} + value={name} + /> + setMax(e.target.value)} + type="number" + value={max} + /> + {weightedViewEnabled && ( + setWeight(e.target.value)} + type="number" + value={weight} + /> + )} +
+ setFloorAtZero(e.target.checked)} + /> + } + label={t(translations.floorLabel)} + /> + + + +
+
+ setCapAtMaximum(e.target.checked)} + /> + } + label={t(translations.capLabel)} + /> + + + +
+ + + + + + + ); +}; + +export default AddExternalColumnPrompt; diff --git a/client/app/bundles/course/gradebook/components/ConfigureWeightsPrompt.tsx b/client/app/bundles/course/gradebook/components/ConfigureWeightsPrompt.tsx index 95563f2591..1a865ff582 100644 --- a/client/app/bundles/course/gradebook/components/ConfigureWeightsPrompt.tsx +++ b/client/app/bundles/course/gradebook/components/ConfigureWeightsPrompt.tsx @@ -426,6 +426,12 @@ const ConfigureWeightsPrompt: FC = ({ setKeepHighest((prev) => ({ ...prev, [tabId]: parsed })); }; + // Skip a category with no tabs — e.g. the synthetic "External Assessments" + // group once its last external is deleted — so no bare header lingers. + const visibleCategories = categories.filter((cat) => + tabs.some((tb) => tb.categoryId === cat.id), + ); + // Gamification off hides the level section, so a persisted enabled level must be // treated as disabled — it contributes nothing to the Total (matches the table's // showLevelContribution = gamificationEnabled && enabled). @@ -574,7 +580,7 @@ const ConfigureWeightsPrompt: FC = ({ ))} - {categories.map((cat) => ( + {visibleCategories.map((cat) => (
{cat.title} @@ -586,6 +592,55 @@ const ConfigureWeightsPrompt: FC = ({ const tabAssessments = assessments.filter( (a) => a.tabId === tb.id, ); + // An external assessment is always one-per-tab; its tab has + // no internal structure, so render just name + weight (no + // mode toggle, no expand, no per-assessment exclusion). + const isExternal = + tabAssessments.length > 0 && + tabAssessments.every((a) => a.external); + if (isExternal) { + return ( +
+
+
+ + {tb.title} + + + setWeights((prev) => ({ + ...prev, + [tb.id]: r2(prev[tb.id] ?? 0), + })) + } + onChange={(e) => + handleChange(tb.id, e.target.value) + } + size="small" + sx={{ width: 96 }} + type="number" + value={value} + /> +
+ {err && ( + + {err} + + )} +
+ ); + } const mode = modes[tb.id] ?? 'equal'; const isExpanded = !!expanded[tb.id]; const unbalanced = isUnbalanced(tb.id); diff --git a/client/app/bundles/course/gradebook/components/DeleteExternalColumnPrompt.tsx b/client/app/bundles/course/gradebook/components/DeleteExternalColumnPrompt.tsx new file mode 100644 index 0000000000..3d8ab0b44b --- /dev/null +++ b/client/app/bundles/course/gradebook/components/DeleteExternalColumnPrompt.tsx @@ -0,0 +1,95 @@ +import { FC, useState } from 'react'; +import { defineMessages } from 'react-intl'; +import { LoadingButton } from '@mui/lab'; +import { + Button, + Dialog, + DialogActions, + DialogContent, + DialogContentText, + DialogTitle, +} from '@mui/material'; + +import { useAppDispatch } from 'lib/hooks/store'; +import toast from 'lib/hooks/toast'; +import useTranslation from 'lib/hooks/useTranslation'; + +import { deleteExternalAssessment } from '../operations'; + +const translations = defineMessages({ + title: { + id: 'course.gradebook.DeleteExternalColumnPrompt.title', + defaultMessage: 'Delete external assessment', + }, + body: { + id: 'course.gradebook.DeleteExternalColumnPrompt.body', + defaultMessage: + 'Delete "{title}"? This permanently removes the column and every student grade in it. This cannot be undone.', + }, + cancel: { + id: 'course.gradebook.DeleteExternalColumnPrompt.cancel', + defaultMessage: 'Cancel', + }, + confirm: { + id: 'course.gradebook.DeleteExternalColumnPrompt.confirm', + defaultMessage: 'Delete', + }, + error: { + id: 'course.gradebook.DeleteExternalColumnPrompt.error', + defaultMessage: 'Could not delete the external assessment.', + }, +}); + +interface Props { + open: boolean; + assessmentId: number; + title: string; + onClose: () => void; +} + +const DeleteExternalColumnPrompt: FC = ({ + open, + assessmentId, + title, + onClose, +}) => { + const { t } = useTranslation(); + const dispatch = useAppDispatch(); + const [saving, setSaving] = useState(false); + + const submit = async (): Promise => { + setSaving(true); + try { + await dispatch(deleteExternalAssessment(assessmentId)); + onClose(); + } catch { + toast.error(t(translations.error)); + } finally { + setSaving(false); + } + }; + + return ( + + {t(translations.title)} + + {t(translations.body, { title })} + + + + + {t(translations.confirm)} + + + + ); +}; + +export default DeleteExternalColumnPrompt; diff --git a/client/app/bundles/course/gradebook/components/GradebookColumnTree.tsx b/client/app/bundles/course/gradebook/components/GradebookColumnTree.tsx index e2b5d52648..e42d629dfc 100644 --- a/client/app/bundles/course/gradebook/components/GradebookColumnTree.tsx +++ b/client/app/bundles/course/gradebook/components/GradebookColumnTree.tsx @@ -11,6 +11,7 @@ import useTranslation from 'lib/hooks/useTranslation'; import tableTranslations from 'lib/translations/table'; import { + EXTERNAL_CATEGORY_ID, GAMIFICATION_COL_IDS, type GamificationColId, STUDENT_INFO_COL_IDS, @@ -105,6 +106,21 @@ const GradebookColumnTree = ({ return map; }, [tabs, tabAsnIds]); + const renderLeaf = (id: string, indentLevel: number): JSX.Element | null => { + const asnId = parseAssessmentColumnId(id); + const asn = asnId !== null ? asnById.get(asnId) : undefined; + if (!asn) return null; + return ( + setVisible(id, e.target.checked)} + /> + ); + }; + return (
- {thisCatTabs.map((tab) => { - const tabIds = tabAsnIds.get(tab.id) ?? []; - return ( - - {tabIds.map((id) => { - const asnId = parseAssessmentColumnId(id); - const asn = - asnId !== null ? asnById.get(asnId) : undefined; - if (!asn) return null; - return ( - setVisible(id, e.target.checked)} - /> - ); - })} - - ); - })} + {cat.id === EXTERNAL_CATEGORY_ID + ? catIds.map((id) => renderLeaf(id, 2)) + : thisCatTabs.map((tab) => { + const tabIds = tabAsnIds.get(tab.id) ?? []; + return ( + + {tabIds.map((id) => renderLeaf(id, 3))} + + ); + })} ); })} diff --git a/client/app/bundles/course/gradebook/components/GradebookTable.tsx b/client/app/bundles/course/gradebook/components/GradebookTable.tsx index 3cc71f803b..dc430d3a8d 100644 --- a/client/app/bundles/course/gradebook/components/GradebookTable.tsx +++ b/client/app/bundles/course/gradebook/components/GradebookTable.tsx @@ -1,4 +1,5 @@ import { + cloneElement, forwardRef, useCallback, useLayoutEffect, @@ -314,6 +315,8 @@ interface GradebookTableProps { courseTitle: string; courseId: number; gamificationEnabled: boolean; + /** Optional action rendered in the toolbar, left of the column picker. */ + toolbarAction?: JSX.Element; } const GradebookTable = ({ @@ -325,6 +328,7 @@ const GradebookTable = ({ courseTitle, courseId, gamificationEnabled, + toolbarAction, }: GradebookTableProps): JSX.Element => { const { t } = useTranslation(); @@ -598,15 +602,25 @@ const GradebookTable = ({ return t(translations.exportButton); }, [selectedCount, rows.length, t]); - const toolbarWithLabel = toolbar?.columnPicker + const toolbarWithLabel = toolbar ? { ...toolbar, - columnPicker: { - ...toolbar.columnPicker, - directExportLabel, - directExportTooltip: - selectedCount === 0 ? t(translations.exportAllTooltip) : undefined, - }, + buttons: toolbarAction + ? [ + ...(toolbar.buttons ?? []), + cloneElement(toolbarAction, { key: 'toolbar-action' }), + ] + : toolbar.buttons, + ...(toolbar.columnPicker && { + columnPicker: { + ...toolbar.columnPicker, + directExportLabel, + directExportTooltip: + selectedCount === 0 + ? t(translations.exportAllTooltip) + : undefined, + }, + }), } : toolbar; diff --git a/client/app/bundles/course/gradebook/components/WeightedGradebookTable.tsx b/client/app/bundles/course/gradebook/components/WeightedGradebookTable.tsx index ffdc9e6a24..8a35678992 100644 --- a/client/app/bundles/course/gradebook/components/WeightedGradebookTable.tsx +++ b/client/app/bundles/course/gradebook/components/WeightedGradebookTable.tsx @@ -245,6 +245,8 @@ interface Props { gamificationEnabled: boolean; courseMaxLevel: number; levelContribution: LevelContributionData; + /** Optional action rendered in the toolbar, left of the column picker. */ + toolbarAction?: JSX.Element; } const r2 = (n: number): number => Math.round(n * 100) / 100; @@ -294,6 +296,7 @@ const WeightedGradebookTable = ({ gamificationEnabled, courseMaxLevel, levelContribution, + toolbarAction, }: Props): JSX.Element => { const { t } = useTranslation(); const [configureOpen, setConfigureOpen] = useState(false); @@ -869,6 +872,7 @@ const WeightedGradebookTable = ({ ]} value={displayMode} /> + {toolbarAction} {canManageWeights && ( + + + + ); +}; + +export default EditExternalAssessmentPrompt; diff --git a/client/app/bundles/course/gradebook/components/manage/ManageExternalAssessmentsButton.tsx b/client/app/bundles/course/gradebook/components/manage/ManageExternalAssessmentsButton.tsx new file mode 100644 index 0000000000..01870a3034 --- /dev/null +++ b/client/app/bundles/course/gradebook/components/manage/ManageExternalAssessmentsButton.tsx @@ -0,0 +1,38 @@ +import { FC, useState } from 'react'; +import { defineMessages } from 'react-intl'; +import { Button } from '@mui/material'; + +import useTranslation from 'lib/hooks/useTranslation'; + +import ManageExternalAssessmentsPanel from './ManageExternalAssessmentsPanel'; + +const translations = defineMessages({ + manage: { + id: 'course.gradebook.ManageExternalAssessmentsButton.label', + defaultMessage: 'Manage external assessments', + }, +}); + +interface Props { + /** Match the host toolbar's button size. Defaults to MUI's `medium`. */ + size?: 'small' | 'medium'; +} + +const ManageExternalAssessmentsButton: FC = ({ size }) => { + const { t } = useTranslation(); + const [open, setOpen] = useState(false); + + return ( + <> + + setOpen(false)} + open={open} + /> + + ); +}; + +export default ManageExternalAssessmentsButton; diff --git a/client/app/bundles/course/gradebook/components/manage/ManageExternalAssessmentsPanel.tsx b/client/app/bundles/course/gradebook/components/manage/ManageExternalAssessmentsPanel.tsx new file mode 100644 index 0000000000..18a80e7bfa --- /dev/null +++ b/client/app/bundles/course/gradebook/components/manage/ManageExternalAssessmentsPanel.tsx @@ -0,0 +1,319 @@ +import { FC, useState } from 'react'; +import { defineMessages } from 'react-intl'; +import { + DragDropContext, + Draggable, + Droppable, + DropResult, +} from '@hello-pangea/dnd'; +import { Add, Delete, DragIndicator, Edit } from '@mui/icons-material'; +import { + Button, + Chip, + Dialog, + DialogActions, + DialogContent, + DialogTitle, + IconButton, + Stack, + Typography, +} from '@mui/material'; +import type { AppDispatch } from 'store'; +import type { AssessmentData } from 'types/course/gradebook'; + +import { useAppDispatch, useAppSelector } from 'lib/hooks/store'; +import toast from 'lib/hooks/toast'; +import useTranslation from 'lib/hooks/useTranslation'; + +import { reorderExternalAssessments } from '../../operations'; +import { + getExternalAssessments, + getTabs, + getWeightedViewEnabled, +} from '../../selectors'; +import AddExternalColumnPrompt from '../AddExternalColumnPrompt'; +import DeleteExternalColumnPrompt from '../DeleteExternalColumnPrompt'; + +import EditExternalAssessmentPrompt from './EditExternalAssessmentPrompt'; + +const translations = defineMessages({ + title: { + id: 'course.gradebook.ManageExternalPanel.title', + defaultMessage: 'External assessments', + }, + add: { + id: 'course.gradebook.ManageExternalPanel.add', + defaultMessage: 'Add', + }, + name: { + id: 'course.gradebook.ManageExternalPanel.name', + defaultMessage: 'Name', + }, + max: { + id: 'course.gradebook.ManageExternalPanel.max', + defaultMessage: 'Max', + }, + weight: { + id: 'course.gradebook.ManageExternalPanel.weight', + defaultMessage: 'Weight', + }, + bounds: { + id: 'course.gradebook.ManageExternalPanel.bounds', + defaultMessage: 'Bounds', + }, + floored: { + id: 'course.gradebook.ManageExternalPanel.floored', + defaultMessage: '≥ 0', + }, + capped: { + id: 'course.gradebook.ManageExternalPanel.capped', + defaultMessage: '≤ max', + }, + actions: { + id: 'course.gradebook.ManageExternalPanel.actions', + defaultMessage: 'Actions', + }, + empty: { + id: 'course.gradebook.ManageExternalPanel.empty', + defaultMessage: 'No external assessments yet', + }, + emptyHint: { + id: 'course.gradebook.ManageExternalPanel.emptyHint', + defaultMessage: 'Add one to track grades earned outside Coursemology.', + }, + close: { + id: 'course.gradebook.ManageExternalPanel.close', + defaultMessage: 'Close', + }, + reorderError: { + id: 'course.gradebook.ManageExternalPanel.reorderError', + defaultMessage: 'Could not save the new order. Please try again.', + }, +}); + +interface Props { + open: boolean; + onClose: () => void; +} + +// Returns a new array with the item at `from` moved to `to`. +export const moveItem = (ids: number[], from: number, to: number): number[] => { + const next = [...ids]; + const [moved] = next.splice(from, 1); + next.splice(to, 0, moved); + return next; +}; + +// Builds the new external order from a drag result and persists it. +// Exported so the no-op / dispatch / failure branches stay unit-testable +// without driving the drag-and-drop library in jsdom. +export const handleDragEnd = ( + externalIds: number[], + result: DropResult, + dispatch: AppDispatch, + onError: () => void, +): void => { + if (!result.destination || result.destination.index === result.source.index) { + return; + } + const order = moveItem( + externalIds, + result.source.index, + result.destination.index, + ); + dispatch(reorderExternalAssessments(order)).catch(onError); +}; + +const ManageExternalAssessmentsPanel: FC = ({ open, onClose }) => { + const { t } = useTranslation(); + const externals = useAppSelector(getExternalAssessments); + const tabs = useAppSelector(getTabs); + const weightedViewEnabled = useAppSelector(getWeightedViewEnabled); + const dispatch = useAppDispatch(); + const [addOpen, setAddOpen] = useState(false); + const [editing, setEditing] = useState(null); + const [deleting, setDeleting] = useState(null); + + const tabWeights = Object.fromEntries( + tabs.map((tab) => [tab.id, tab.gradebookWeight ?? 0]), + ); + + const onDragEnd = (result: DropResult): void => + handleDragEnd( + externals.map((a) => a.id), + result, + dispatch, + () => toast.error(t(translations.reorderError)), + ); + + const gridCols = weightedViewEnabled + ? '2.5rem minmax(10rem,1fr) 5rem 5rem 9.5rem 6rem' + : '2.5rem minmax(10rem,1fr) 5rem 9.5rem 6rem'; + + // A lone assessment has nothing to reorder against, so dragging is disabled + // and the handle is hidden. Its grid column is kept (empty) so the rest of + // the row stays in identical placement. + const reorderable = externals.length > 1; + + return ( + + {t(translations.title)} + + + + + + {externals.length === 0 ? ( +
+ + {t(translations.empty)} + + + {t(translations.emptyHint)} + +
+ ) : ( + <> +
+ + {t(translations.name)} + {t(translations.max)} + {weightedViewEnabled && {t(translations.weight)}} + {t(translations.bounds)} + {t(translations.actions)} +
+ + + + {(dropProvided) => ( +
+ {externals.map((a, index) => ( + + {(dragProvided, { isDragging }) => ( +
+ {reorderable ? ( + + + + ) : ( + + )} + + {a.title} + + {a.maxGrade} + {weightedViewEnabled && ( + {tabWeights[a.tabId] ?? 0} + )} + + + {(a.floorAtZero ?? true) && ( + + )} + {(a.capAtMaximum ?? true) && ( + + )} + + + + setEditing(a)} + size="small" + > + + + setDeleting(a)} + size="small" + > + + + +
+ )} + + ))} + {dropProvided.placeholder} +
+ )} + + + + )} + + + + + + setAddOpen(false)} + open={addOpen} + weightedViewEnabled={weightedViewEnabled} + /> + {editing && ( + setEditing(null)} + open={Boolean(editing)} + weightedViewEnabled={weightedViewEnabled} + /> + )} + {deleting && ( + setDeleting(null)} + open={Boolean(deleting)} + title={deleting.title} + /> + )} + + ); +}; + +export default ManageExternalAssessmentsPanel; diff --git a/client/app/bundles/course/gradebook/computeWeighted.ts b/client/app/bundles/course/gradebook/computeWeighted.ts index 3c651bb8d2..f58c793d9c 100644 --- a/client/app/bundles/course/gradebook/computeWeighted.ts +++ b/client/app/bundles/course/gradebook/computeWeighted.ts @@ -12,8 +12,8 @@ import { ParsedFormula } from './levelFormula'; type GradeEntry = Pick; // Synthetic ids for the Level term — disjoint from real (positive) tab/assessment ids. -export const LEVEL_TAB_ID = -1; -export const LEVEL_ASSESSMENT_ID = -1; +export const LEVEL_TAB_ID = 0; +export const LEVEL_ASSESSMENT_ID = 0; export interface WeightedRow { studentId: number; diff --git a/client/app/bundles/course/gradebook/constants.ts b/client/app/bundles/course/gradebook/constants.ts index 4c61d2723f..c5e07ff06f 100644 --- a/client/app/bundles/course/gradebook/constants.ts +++ b/client/app/bundles/course/gradebook/constants.ts @@ -3,3 +3,7 @@ export type StudentInfoColId = (typeof STUDENT_INFO_COL_IDS)[number]; export const GAMIFICATION_COL_IDS = ['level', 'totalXp'] as const; export type GamificationColId = (typeof GAMIFICATION_COL_IDS)[number]; + +/** Synthetic category id for external assessments (mirrors backend + * Course::ExternalAssessment::SYNTHETIC_CATEGORY_ID). */ +export const EXTERNAL_CATEGORY_ID = -1; diff --git a/client/app/bundles/course/gradebook/operations.ts b/client/app/bundles/course/gradebook/operations.ts index 55ad9e6827..7b65c4100c 100644 --- a/client/app/bundles/course/gradebook/operations.ts +++ b/client/app/bundles/course/gradebook/operations.ts @@ -34,6 +34,72 @@ export const updateGradebookWeights = dispatch(actions.updateTabWeights(responseData)); }; +export const createExternalAssessment = + ( + title: string, + maximumGrade: number, + floorAtZero: boolean, + capAtMaximum: boolean, + weight?: number, + ): Operation => + async (dispatch) => { + const response = await CourseAPI.gradebook.createExternal({ + title, + maximumGrade, + floorAtZero, + capAtMaximum, + ...(weight === undefined ? {} : { weight }), + }); + dispatch(actions.applyCreatedExternal(response.data)); + }; + +export const editExternalAssessment = + ( + assessmentId: number, + patch: { + title?: string; + maximumGrade?: number; + floorAtZero?: boolean; + capAtMaximum?: boolean; + weight?: number; + }, + ): Operation => + async (dispatch) => { + const response = await CourseAPI.gradebook.updateExternal( + -assessmentId, + patch, + ); + dispatch(actions.updateExternalAssessment(response.data)); + }; + +export const deleteExternalAssessment = + (assessmentId: number): Operation => + async (dispatch) => { + await CourseAPI.gradebook.deleteExternal(-assessmentId); + dispatch(actions.deleteExternalAssessment(assessmentId)); + }; + +// Optimistic: apply the new external order immediately, persist via one PUT. +// On failure, restore the previous order and rethrow so the caller can toast. +// `orderedAssessmentIds` are the negative serialized ids (store ids); the API +// wants the positive external ids, so we negate them. +export const reorderExternalAssessments = + (orderedAssessmentIds: number[]): Operation => + async (dispatch, getState) => { + const prevOrder = getState() + .gradebook.assessments.filter((a) => a.external) + .map((a) => a.id); + dispatch(actions.reorderExternalAssessments(orderedAssessmentIds)); + try { + await CourseAPI.gradebook.reorderExternals({ + orderedIds: orderedAssessmentIds.map((id) => -id), + }); + } catch (error) { + dispatch(actions.reorderExternalAssessments(prevOrder)); + throw error; + } + }; + // Optimistic: apply the new grade immediately, then reconcile with the server. // On failure, restore the previous value and rethrow so the caller can toast. export const setExternalGrade = diff --git a/client/app/bundles/course/gradebook/pages/GradebookIndex/index.tsx b/client/app/bundles/course/gradebook/pages/GradebookIndex/index.tsx index 03f36e4174..0d08b3a302 100644 --- a/client/app/bundles/course/gradebook/pages/GradebookIndex/index.tsx +++ b/client/app/bundles/course/gradebook/pages/GradebookIndex/index.tsx @@ -13,6 +13,7 @@ import useTranslation from 'lib/hooks/useTranslation'; import { useCourseContext } from '../../../container/CourseLoader'; import GradebookTable from '../../components/GradebookTable'; import GradeLinkHint from '../../components/GradeLinkHint'; +import ManageExternalAssessmentsButton from '../../components/manage/ManageExternalAssessmentsButton'; import WeightedGradebookTable from '../../components/WeightedGradebookTable'; import WeightedViewHint from '../../components/WeightedViewHint'; import fetchGradebook from '../../operations'; @@ -115,6 +116,11 @@ const GradebookIndex: FC = () => { students={students} submissions={submissions} tabs={tabs} + toolbarAction={ + canManageWeights ? ( + + ) : undefined + } /> ); } else { @@ -128,6 +134,9 @@ const GradebookIndex: FC = () => { students={students} submissions={submissions} tabs={tabs} + toolbarAction={ + canManageWeights ? : undefined + } /> ); } diff --git a/client/app/bundles/course/gradebook/store.ts b/client/app/bundles/course/gradebook/store.ts index dbd74a8342..113a9adf2e 100644 --- a/client/app/bundles/course/gradebook/store.ts +++ b/client/app/bundles/course/gradebook/store.ts @@ -1,5 +1,7 @@ import { produce } from 'immer'; import type { + ExternalAssessmentNode, + ExternalAssessmentUpdate, ExternalGradePayload, GradebookData, LevelContributionData, @@ -17,6 +19,13 @@ import type { const SAVE_GRADEBOOK = 'course/gradebook/SAVE_GRADEBOOK'; const UPDATE_TAB_WEIGHTS = 'course/gradebook/UPDATE_TAB_WEIGHTS'; +const APPLY_CREATED_EXTERNAL = 'course/gradebook/APPLY_CREATED_EXTERNAL'; +const UPDATE_EXTERNAL_ASSESSMENT = + 'course/gradebook/UPDATE_EXTERNAL_ASSESSMENT'; +const DELETE_EXTERNAL_ASSESSMENT = + 'course/gradebook/DELETE_EXTERNAL_ASSESSMENT'; +const REORDER_EXTERNAL_ASSESSMENTS = + 'course/gradebook/REORDER_EXTERNAL_ASSESSMENTS'; const SET_EXTERNAL_GRADE = 'course/gradebook/SET_EXTERNAL_GRADE'; interface GradebookState { @@ -42,6 +51,22 @@ interface UpdateTabWeightsAction { payload: UpdateWeightsPayload; } +interface ApplyCreatedExternalAction { + type: typeof APPLY_CREATED_EXTERNAL; + payload: ExternalAssessmentNode; +} +interface UpdateExternalAssessmentAction { + type: typeof UPDATE_EXTERNAL_ASSESSMENT; + payload: ExternalAssessmentUpdate; +} +interface DeleteExternalAssessmentAction { + type: typeof DELETE_EXTERNAL_ASSESSMENT; + payload: number; // negative serialized assessment id +} +interface ReorderExternalAssessmentsAction { + type: typeof REORDER_EXTERNAL_ASSESSMENTS; + payload: number[]; // negative serialized assessment ids, new order +} interface SetExternalGradeAction { type: typeof SET_EXTERNAL_GRADE; payload: ExternalGradePayload; @@ -72,6 +97,10 @@ const reducer = produce( action: | SaveGradebookAction | UpdateTabWeightsAction + | ApplyCreatedExternalAction + | UpdateExternalAssessmentAction + | DeleteExternalAssessmentAction + | ReorderExternalAssessmentsAction | SetExternalGradeAction, ) => { switch (action.type) { @@ -146,6 +175,89 @@ const reducer = produce( } break; } + case APPLY_CREATED_EXTERNAL: { + const { assessment, tab, category } = action.payload; + if (!draft.categories.some((c) => c.id === category.id)) { + draft.categories.push(category); + } + if (!draft.tabs.some((t) => t.id === tab.id)) { + draft.tabs.push(tab); + } + if (!draft.assessments.some((a) => a.id === assessment.id)) { + draft.assessments.push(assessment); + } + break; + } + case UPDATE_EXTERNAL_ASSESSMENT: { + const { assessment, tab } = action.payload; + const a = draft.assessments.find((x) => x.id === assessment.id); + if (a) { + a.title = assessment.title; + a.maxGrade = assessment.maxGrade; + a.floorAtZero = assessment.floorAtZero; + a.capAtMaximum = assessment.capAtMaximum; + } + const t = draft.tabs.find((x) => x.id === tab.id); + if (t) { + t.title = tab.title; + if (tab.gradebookWeight !== undefined) { + t.gradebookWeight = tab.gradebookWeight; + } + } + break; + } + case DELETE_EXTERNAL_ASSESSMENT: { + const id = action.payload; + const removed = draft.assessments.find((a) => a.id === id); + draft.assessments = draft.assessments.filter((a) => a.id !== id); + draft.submissions = draft.submissions.filter( + (s) => s.assessmentId !== id, + ); + if ( + removed && + !draft.assessments.some((a) => a.tabId === removed.tabId) + ) { + const removedTab = draft.tabs.find((t) => t.id === removed.tabId); + draft.tabs = draft.tabs.filter((t) => t.id !== removed.tabId); + // Drop the now-empty synthetic "External Assessments" category so its + // header doesn't linger after the last external is deleted (the + // backend omits it entirely once no externals remain). + if ( + removedTab && + !draft.tabs.some((t) => t.categoryId === removedTab.categoryId) + ) { + draft.categories = draft.categories.filter( + (c) => c.id !== removedTab.categoryId, + ); + } + } + break; + } + case REORDER_EXTERNAL_ASSESSMENTS: { + const rank = new Map(action.payload.map((id, i) => [id, i])); + const externalsSorted = draft.assessments + .filter((a) => a.external) + .sort((x, y) => (rank.get(x.id) ?? 0) - (rank.get(y.id) ?? 0)); + let ai = 0; + draft.assessments = draft.assessments.map((a) => { + if (!a.external) return a; + const next = externalsSorted[ai]; + ai += 1; + return next; + }); + const tabRank = new Map(externalsSorted.map((a, i) => [a.tabId, i])); + const tabsSorted = draft.tabs + .filter((t) => tabRank.has(t.id)) + .sort((x, y) => (tabRank.get(x.id) ?? 0) - (tabRank.get(y.id) ?? 0)); + let ti = 0; + draft.tabs = draft.tabs.map((t) => { + if (!tabRank.has(t.id)) return t; + const next = tabsSorted[ti]; + ti += 1; + return next; + }); + break; + } case SET_EXTERNAL_GRADE: { const { studentId, assessmentId, grade } = action.payload; const existing = draft.submissions.find( @@ -173,6 +285,25 @@ export const actions = { type: UPDATE_TAB_WEIGHTS, payload, }), + applyCreatedExternal: ( + payload: ExternalAssessmentNode, + ): ApplyCreatedExternalAction => ({ type: APPLY_CREATED_EXTERNAL, payload }), + updateExternalAssessment: ( + payload: ExternalAssessmentUpdate, + ): UpdateExternalAssessmentAction => ({ + type: UPDATE_EXTERNAL_ASSESSMENT, + payload, + }), + deleteExternalAssessment: (id: number): DeleteExternalAssessmentAction => ({ + type: DELETE_EXTERNAL_ASSESSMENT, + payload: id, + }), + reorderExternalAssessments: ( + payload: number[], + ): ReorderExternalAssessmentsAction => ({ + type: REORDER_EXTERNAL_ASSESSMENTS, + payload, + }), setExternalGrade: ( payload: ExternalGradePayload, ): SetExternalGradeAction => ({ type: SET_EXTERNAL_GRADE, payload }), diff --git a/client/app/types/course/gradebook.ts b/client/app/types/course/gradebook.ts index 37eca4530a..ec970289bc 100644 --- a/client/app/types/course/gradebook.ts +++ b/client/app/types/course/gradebook.ts @@ -91,6 +91,17 @@ export interface LevelContributionSaveData extends LevelContributionData { formulaAst: FormulaNode | null; } +export interface ExternalAssessmentNode { + assessment: AssessmentData; + tab: TabData; + category: CategoryData; +} + +export interface ExternalAssessmentUpdate { + assessment: AssessmentData; + tab: Pick; +} + export interface ExternalGradePayload { studentId: number; assessmentId: number; diff --git a/client/locales/en.json b/client/locales/en.json index 22d42bc456..a66f6ec601 100644 --- a/client/locales/en.json +++ b/client/locales/en.json @@ -9676,5 +9676,137 @@ }, "course.gradebook.GradebookTable.gradeSaved": { "defaultMessage": "Grade saved. {title} · {name}: {oldGrade} → {newGrade}" + }, + "course.gradebook.AddExternalColumnPrompt.cancel": { + "defaultMessage": "Cancel" + }, + "course.gradebook.AddExternalColumnPrompt.capHint": { + "defaultMessage": "Counts grades above the maximum as the maximum when computing the weighted total. The actual grade is unchanged." + }, + "course.gradebook.AddExternalColumnPrompt.capLabel": { + "defaultMessage": "Cap grades at max" + }, + "course.gradebook.AddExternalColumnPrompt.create": { + "defaultMessage": "Create" + }, + "course.gradebook.AddExternalColumnPrompt.error": { + "defaultMessage": "Could not create the external assessment." + }, + "course.gradebook.AddExternalColumnPrompt.floorHint": { + "defaultMessage": "Counts negative grades as 0 when computing the weighted total. The actual grade is unchanged." + }, + "course.gradebook.AddExternalColumnPrompt.floorLabel": { + "defaultMessage": "Floor grades at 0" + }, + "course.gradebook.AddExternalColumnPrompt.maxLabel": { + "defaultMessage": "Max marks" + }, + "course.gradebook.AddExternalColumnPrompt.nameLabel": { + "defaultMessage": "Name" + }, + "course.gradebook.AddExternalColumnPrompt.success": { + "defaultMessage": "External assessment created." + }, + "course.gradebook.AddExternalColumnPrompt.title": { + "defaultMessage": "Add external assessment" + }, + "course.gradebook.AddExternalColumnPrompt.weightLabel": { + "defaultMessage": "Weightage" + }, + "course.gradebook.DeleteExternalColumnPrompt.body": { + "defaultMessage": "Delete \"{title}\"? This permanently removes the column and every student grade in it. This cannot be undone." + }, + "course.gradebook.DeleteExternalColumnPrompt.cancel": { + "defaultMessage": "Cancel" + }, + "course.gradebook.DeleteExternalColumnPrompt.confirm": { + "defaultMessage": "Delete" + }, + "course.gradebook.DeleteExternalColumnPrompt.error": { + "defaultMessage": "Could not delete the external assessment." + }, + "course.gradebook.DeleteExternalColumnPrompt.title": { + "defaultMessage": "Delete external assessment" + }, + "course.gradebook.EditExternalAssessmentPrompt.cancel": { + "defaultMessage": "Cancel" + }, + "course.gradebook.EditExternalAssessmentPrompt.capHint": { + "defaultMessage": "Counts grades above the maximum as the maximum when computing the weighted total. The actual grade is unchanged." + }, + "course.gradebook.EditExternalAssessmentPrompt.capLabel": { + "defaultMessage": "Cap grades at max" + }, + "course.gradebook.EditExternalAssessmentPrompt.error": { + "defaultMessage": "Could not save the external assessment." + }, + "course.gradebook.EditExternalAssessmentPrompt.floorHint": { + "defaultMessage": "Counts negative grades as 0 when computing the weighted total. The actual grade is unchanged." + }, + "course.gradebook.EditExternalAssessmentPrompt.floorLabel": { + "defaultMessage": "Floor grades at 0" + }, + "course.gradebook.EditExternalAssessmentPrompt.maxLabel": { + "defaultMessage": "Max marks" + }, + "course.gradebook.EditExternalAssessmentPrompt.nameLabel": { + "defaultMessage": "Name" + }, + "course.gradebook.EditExternalAssessmentPrompt.save": { + "defaultMessage": "Save" + }, + "course.gradebook.EditExternalAssessmentPrompt.title": { + "defaultMessage": "Edit external assessment" + }, + "course.gradebook.EditExternalAssessmentPrompt.weightLabel": { + "defaultMessage": "Weightage" + }, + "course.gradebook.GradebookWeightedTable.displayMode": { + "defaultMessage": "Display mode" + }, + "course.gradebook.ManageExternalAssessmentsButton.label": { + "defaultMessage": "Manage external assessments" + }, + "course.gradebook.ManageExternalPanel.actions": { + "defaultMessage": "Actions" + }, + "course.gradebook.ManageExternalPanel.add": { + "defaultMessage": "Add" + }, + "course.gradebook.ManageExternalPanel.bounds": { + "defaultMessage": "Bounds" + }, + "course.gradebook.ManageExternalPanel.capped": { + "defaultMessage": "≤ max" + }, + "course.gradebook.ManageExternalPanel.close": { + "defaultMessage": "Close" + }, + "course.gradebook.ManageExternalPanel.empty": { + "defaultMessage": "No external assessments yet" + }, + "course.gradebook.ManageExternalPanel.emptyHint": { + "defaultMessage": "Add one to track grades earned outside Coursemology." + }, + "course.gradebook.ManageExternalPanel.floored": { + "defaultMessage": "≥ 0" + }, + "course.gradebook.ManageExternalPanel.max": { + "defaultMessage": "Max" + }, + "course.gradebook.ManageExternalPanel.name": { + "defaultMessage": "Name" + }, + "course.gradebook.ManageExternalPanel.reorderError": { + "defaultMessage": "Could not save the new order. Please try again." + }, + "course.gradebook.ManageExternalPanel.title": { + "defaultMessage": "External assessments" + }, + "course.gradebook.ManageExternalPanel.weight": { + "defaultMessage": "Weight" + }, + "course.gradebook.ProjectedTotalHint.policy": { + "defaultMessage": "Totals count ungraded assessments as 0." } } diff --git a/client/locales/ko.json b/client/locales/ko.json index 07fff21080..ad2e910451 100644 --- a/client/locales/ko.json +++ b/client/locales/ko.json @@ -9667,5 +9667,143 @@ }, "course.gradebook.GradebookTable.gradeSaved": { "defaultMessage": "성적이 저장되었습니다. {title} · {name}: {oldGrade} → {newGrade}" + }, + "course.gradebook.AddExternalColumnPrompt.cancel": { + "defaultMessage": "취소" + }, + "course.gradebook.AddExternalColumnPrompt.create": { + "defaultMessage": "만들기" + }, + "course.gradebook.AddExternalColumnPrompt.error": { + "defaultMessage": "외부 평가를 생성할 수 없습니다." + }, + "course.gradebook.AddExternalColumnPrompt.maxLabel": { + "defaultMessage": "최대 점수" + }, + "course.gradebook.AddExternalColumnPrompt.nameLabel": { + "defaultMessage": "이름" + }, + "course.gradebook.AddExternalColumnPrompt.success": { + "defaultMessage": "외부 평가가 생성되었습니다." + }, + "course.gradebook.AddExternalColumnPrompt.title": { + "defaultMessage": "외부 평가 추가" + }, + "course.gradebook.DeleteExternalColumnPrompt.body": { + "defaultMessage": "\"{title}\"을(를) 삭제하시겠습니까? 이 작업은 해당 열과 그 안의 모든 학생 성적을 영구적으로 제거합니다. 이 작업은 되돌릴 수 없습니다." + }, + "course.gradebook.DeleteExternalColumnPrompt.cancel": { + "defaultMessage": "취소" + }, + "course.gradebook.DeleteExternalColumnPrompt.confirm": { + "defaultMessage": "삭제" + }, + "course.gradebook.DeleteExternalColumnPrompt.error": { + "defaultMessage": "외부 평가를 삭제할 수 없습니다." + }, + "course.gradebook.DeleteExternalColumnPrompt.title": { + "defaultMessage": "외부 평가 삭제" + }, + "course.gradebook.GradeLinkHint.hint": { + "defaultMessage": "각 성적은 학생 제출물의 점수 합계입니다. 성적을 클릭하면 해당 제출물을 열고 점수를 조정할 수 있습니다." + }, + "course.gradebook.GradebookIndex.noStudentsHint": { + "defaultMessage": "학생이 강좌에 참여하면 여기에 성적이 표시됩니다." + }, + "course.gradebook.GradebookWeightedTable.displayMode": { + "defaultMessage": "표시 모드" + }, + "course.gradebook.ProjectedTotalHint.policy": { + "defaultMessage": "총점은 채점되지 않은 평가를 0점으로 계산합니다." + }, + "course.gradebook.AddExternalColumnPrompt.capHint": { + "defaultMessage": "가중 총점을 계산할 때 최대 점수를 초과하는 성적을 최대 점수로 계산합니다. 실제 성적은 변경되지 않습니다." + }, + "course.gradebook.AddExternalColumnPrompt.capLabel": { + "defaultMessage": "최대 점수로 상한 적용" + }, + "course.gradebook.AddExternalColumnPrompt.floorHint": { + "defaultMessage": "가중 총점을 계산할 때 음수 성적을 0으로 계산합니다. 실제 성적은 변경되지 않습니다." + }, + "course.gradebook.AddExternalColumnPrompt.floorLabel": { + "defaultMessage": "0으로 하한 적용" + }, + "course.gradebook.AddExternalColumnPrompt.weightLabel": { + "defaultMessage": "가중치" + }, + "course.gradebook.EditExternalAssessmentPrompt.cancel": { + "defaultMessage": "취소" + }, + "course.gradebook.EditExternalAssessmentPrompt.capHint": { + "defaultMessage": "가중 총점을 계산할 때 최대 점수를 초과하는 성적을 최대 점수로 계산합니다. 실제 성적은 변경되지 않습니다." + }, + "course.gradebook.EditExternalAssessmentPrompt.capLabel": { + "defaultMessage": "최대 점수로 상한 적용" + }, + "course.gradebook.EditExternalAssessmentPrompt.error": { + "defaultMessage": "외부 평가를 저장할 수 없습니다." + }, + "course.gradebook.EditExternalAssessmentPrompt.floorHint": { + "defaultMessage": "가중 총점을 계산할 때 음수 성적을 0으로 계산합니다. 실제 성적은 변경되지 않습니다." + }, + "course.gradebook.EditExternalAssessmentPrompt.floorLabel": { + "defaultMessage": "0으로 하한 적용" + }, + "course.gradebook.EditExternalAssessmentPrompt.maxLabel": { + "defaultMessage": "최대 점수" + }, + "course.gradebook.EditExternalAssessmentPrompt.nameLabel": { + "defaultMessage": "이름" + }, + "course.gradebook.EditExternalAssessmentPrompt.save": { + "defaultMessage": "저장" + }, + "course.gradebook.EditExternalAssessmentPrompt.title": { + "defaultMessage": "외부 평가 편집" + }, + "course.gradebook.EditExternalAssessmentPrompt.weightLabel": { + "defaultMessage": "가중치" + }, + "course.gradebook.ManageExternalAssessmentsButton.label": { + "defaultMessage": "외부 평가 관리" + }, + "course.gradebook.ManageExternalPanel.actions": { + "defaultMessage": "작업" + }, + "course.gradebook.ManageExternalPanel.add": { + "defaultMessage": "추가" + }, + "course.gradebook.ManageExternalPanel.bounds": { + "defaultMessage": "범위" + }, + "course.gradebook.ManageExternalPanel.capped": { + "defaultMessage": "≤ 최대" + }, + "course.gradebook.ManageExternalPanel.close": { + "defaultMessage": "닫기" + }, + "course.gradebook.ManageExternalPanel.empty": { + "defaultMessage": "아직 외부 평가가 없습니다" + }, + "course.gradebook.ManageExternalPanel.emptyHint": { + "defaultMessage": "Coursemology 외부에서 받은 성적을 추적하려면 하나를 추가하세요." + }, + "course.gradebook.ManageExternalPanel.floored": { + "defaultMessage": "≥ 0" + }, + "course.gradebook.ManageExternalPanel.max": { + "defaultMessage": "최대 점수" + }, + "course.gradebook.ManageExternalPanel.name": { + "defaultMessage": "이름" + }, + "course.gradebook.ManageExternalPanel.reorderError": { + "defaultMessage": "새 순서를 저장할 수 없습니다. 다시 시도해 주세요." + }, + "course.gradebook.ManageExternalPanel.title": { + "defaultMessage": "외부 평가" + }, + "course.gradebook.ManageExternalPanel.weight": { + "defaultMessage": "가중치" } } diff --git a/client/locales/zh.json b/client/locales/zh.json index f46c1576a1..69841e9524 100644 --- a/client/locales/zh.json +++ b/client/locales/zh.json @@ -9661,5 +9661,143 @@ }, "course.gradebook.GradebookTable.gradeSaved": { "defaultMessage": "成绩已保存。{title} · {name}：{oldGrade} → {newGrade}" + }, + "course.gradebook.AddExternalColumnPrompt.cancel": { + "defaultMessage": "取消" + }, + "course.gradebook.AddExternalColumnPrompt.create": { + "defaultMessage": "创建" + }, + "course.gradebook.AddExternalColumnPrompt.error": { + "defaultMessage": "无法创建外部评估。" + }, + "course.gradebook.AddExternalColumnPrompt.maxLabel": { + "defaultMessage": "最高分" + }, + "course.gradebook.AddExternalColumnPrompt.nameLabel": { + "defaultMessage": "名称" + }, + "course.gradebook.AddExternalColumnPrompt.success": { + "defaultMessage": "外部评估已创建。" + }, + "course.gradebook.AddExternalColumnPrompt.title": { + "defaultMessage": "添加外部评估" + }, + "course.gradebook.DeleteExternalColumnPrompt.body": { + "defaultMessage": "要删除\"{title}\"吗？这将永久移除该列及其中每位学生的成绩。此操作无法撤销。" + }, + "course.gradebook.DeleteExternalColumnPrompt.cancel": { + "defaultMessage": "取消" + }, + "course.gradebook.DeleteExternalColumnPrompt.confirm": { + "defaultMessage": "删除" + }, + "course.gradebook.DeleteExternalColumnPrompt.error": { + "defaultMessage": "无法删除外部评估。" + }, + "course.gradebook.DeleteExternalColumnPrompt.title": { + "defaultMessage": "删除外部评估" + }, + "course.gradebook.GradeLinkHint.hint": { + "defaultMessage": "每个成绩都是学生提交内容中各项分数的总和。点击任一成绩即可打开该提交内容并调整分数。" + }, + "course.gradebook.GradebookIndex.noStudentsHint": { + "defaultMessage": "学生加入课程后，成绩将显示在这里。" + }, + "course.gradebook.GradebookWeightedTable.displayMode": { + "defaultMessage": "显示模式" + }, + "course.gradebook.ProjectedTotalHint.policy": { + "defaultMessage": "总分会将未评分的评估计为 0。" + }, + "course.gradebook.AddExternalColumnPrompt.capHint": { + "defaultMessage": "计算加权总分时，将超过最高分的成绩按最高分计算。实际成绩保持不变。" + }, + "course.gradebook.AddExternalColumnPrompt.capLabel": { + "defaultMessage": "成绩上限设为最高分" + }, + "course.gradebook.AddExternalColumnPrompt.floorHint": { + "defaultMessage": "计算加权总分时，将负数成绩按 0 计算。实际成绩保持不变。" + }, + "course.gradebook.AddExternalColumnPrompt.floorLabel": { + "defaultMessage": "成绩下限设为 0" + }, + "course.gradebook.AddExternalColumnPrompt.weightLabel": { + "defaultMessage": "权重" + }, + "course.gradebook.EditExternalAssessmentPrompt.cancel": { + "defaultMessage": "取消" + }, + "course.gradebook.EditExternalAssessmentPrompt.capHint": { + "defaultMessage": "计算加权总分时，将超过最高分的成绩按最高分计算。实际成绩保持不变。" + }, + "course.gradebook.EditExternalAssessmentPrompt.capLabel": { + "defaultMessage": "成绩上限设为最高分" + }, + "course.gradebook.EditExternalAssessmentPrompt.error": { + "defaultMessage": "无法保存外部评估。" + }, + "course.gradebook.EditExternalAssessmentPrompt.floorHint": { + "defaultMessage": "计算加权总分时，将负数成绩按 0 计算。实际成绩保持不变。" + }, + "course.gradebook.EditExternalAssessmentPrompt.floorLabel": { + "defaultMessage": "成绩下限设为 0" + }, + "course.gradebook.EditExternalAssessmentPrompt.maxLabel": { + "defaultMessage": "最高分" + }, + "course.gradebook.EditExternalAssessmentPrompt.nameLabel": { + "defaultMessage": "名称" + }, + "course.gradebook.EditExternalAssessmentPrompt.save": { + "defaultMessage": "保存" + }, + "course.gradebook.EditExternalAssessmentPrompt.title": { + "defaultMessage": "编辑外部评估" + }, + "course.gradebook.EditExternalAssessmentPrompt.weightLabel": { + "defaultMessage": "权重" + }, + "course.gradebook.ManageExternalAssessmentsButton.label": { + "defaultMessage": "管理外部评估" + }, + "course.gradebook.ManageExternalPanel.actions": { + "defaultMessage": "操作" + }, + "course.gradebook.ManageExternalPanel.add": { + "defaultMessage": "添加" + }, + "course.gradebook.ManageExternalPanel.bounds": { + "defaultMessage": "范围" + }, + "course.gradebook.ManageExternalPanel.capped": { + "defaultMessage": "≤ 最高分" + }, + "course.gradebook.ManageExternalPanel.close": { + "defaultMessage": "关闭" + }, + "course.gradebook.ManageExternalPanel.empty": { + "defaultMessage": "暂无外部评估" + }, + "course.gradebook.ManageExternalPanel.emptyHint": { + "defaultMessage": "添加一个以跟踪在 Coursemology 之外获得的成绩。" + }, + "course.gradebook.ManageExternalPanel.floored": { + "defaultMessage": "≥ 0" + }, + "course.gradebook.ManageExternalPanel.max": { + "defaultMessage": "最高分" + }, + "course.gradebook.ManageExternalPanel.name": { + "defaultMessage": "名称" + }, + "course.gradebook.ManageExternalPanel.reorderError": { + "defaultMessage": "无法保存新的顺序。请重试。" + }, + "course.gradebook.ManageExternalPanel.title": { + "defaultMessage": "外部评估" + }, + "course.gradebook.ManageExternalPanel.weight": { + "defaultMessage": "权重" } } diff --git a/config/routes.rb b/config/routes.rb index e04fd4d813..7648e09363 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -501,10 +501,13 @@ resource :gradebook, only: [] do get '/' => 'gradebook#index' patch '/weights' => 'gradebook#update_weights' - resources :external_assessments, only: [] do + resources :external_assessments, only: [:create, :update, :destroy] do member do put 'grades' => 'external_assessments#grades' end + collection do + put 'reorder' => 'external_assessments#reorder' + end end end diff --git a/db/migrate/20260625000000_add_position_to_course_external_assessments.rb b/db/migrate/20260625000000_add_position_to_course_external_assessments.rb new file mode 100644 index 0000000000..2976609196 --- /dev/null +++ b/db/migrate/20260625000000_add_position_to_course_external_assessments.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true +class AddPositionToCourseExternalAssessments < ActiveRecord::Migration[7.2] + def up + add_column :course_external_assessments, :position, :integer + + # Backfill existing rows deterministically: per course, 0-based by id. + execute <<~SQL.squish + UPDATE course_external_assessments AS e + SET position = sub.rn - 1 + FROM ( + SELECT id, ROW_NUMBER() OVER (PARTITION BY course_id ORDER BY id) AS rn + FROM course_external_assessments + ) AS sub + WHERE e.id = sub.id + SQL + + change_column_null :course_external_assessments, :position, false + end + + def down + remove_column :course_external_assessments, :position + end +end diff --git a/db/schema.rb b/db/schema.rb index bb3a423813..28385a064f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.2].define(version: 2026_06_24_000000) do +ActiveRecord::Schema[7.2].define(version: 2026_06_25_000000) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" enable_extension "uuid-ossp" @@ -784,12 +784,13 @@ t.bigint "course_id", null: false t.string "title", null: false t.decimal "maximum_grade", precision: 5, scale: 2, null: false + t.boolean "floor_at_zero", default: true, null: false + t.boolean "cap_at_maximum", default: true, null: false t.bigint "creator_id", null: false t.bigint "updater_id", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false - t.boolean "floor_at_zero", default: true, null: false - t.boolean "cap_at_maximum", default: true, null: false + t.integer "position", null: false t.index ["course_id", "title"], name: "index_course_external_assessments_on_course_id_and_title", unique: true t.index ["course_id"], name: "fk__course_external_assessments_course_id" t.index ["creator_id"], name: "fk__course_external_assessments_creator_id" diff --git a/spec/controllers/course/external_assessments_controller_spec.rb b/spec/controllers/course/external_assessments_controller_spec.rb index c235b0e628..de1e474358 100644 --- a/spec/controllers/course/external_assessments_controller_spec.rb +++ b/spec/controllers/course/external_assessments_controller_spec.rb @@ -8,6 +8,296 @@ let(:course) { create(:course) } let(:manager) { create(:course_manager, course: course) } let(:ta) { create(:course_teaching_assistant, course: course) } + let(:observer) { create(:course_observer, course: course) } + + describe '#create' do + render_views + let(:params) do + { course_id: course.id, format: :json, title: 'Final', maximumGrade: 100 } + end + + context 'as a manager' do + before { controller_sign_in(controller, manager.user) } + + it 'creates an external assessment with a contribution and no tab/category' do + expect do + post :create, params: params + end.to change(Course::ExternalAssessment, :count).by(1). + and change(Course::Gradebook::ExternalContribution, :count).by(1). + and not_change(Course::Assessment::Tab, :count) + expect(response).to be_successful + body = JSON.parse(response.body) + created = Course::ExternalAssessment.last + expect(body['assessment']['id']).to eq(-created.id) + expect(body['assessment']['id']).to be < 0 + expect(body['assessment']['title']).to eq('Final') + expect(body['assessment']['maxGrade']).to eq(100.0) + expect(body['assessment']['external']).to be(true) + expect(body['assessment']['gradebookExcluded']).to be(false) + expect(body['tab']['id']).to eq(body['assessment']['tabId']) + expect(body['tab']['id']).to be < 0 + expect(body['tab']['categoryId']).to eq(Course::ExternalAssessment::SYNTHETIC_CATEGORY_ID) + expect(body['category']['id']).to eq(Course::ExternalAssessment::SYNTHETIC_CATEGORY_ID) + expect(body['category']['title']).to eq(Course::ExternalAssessment::SYNTHETIC_CATEGORY_TITLE) + end + + it 'serializes the bound flags' do + post :create, as: :json, params: { + course_id: course, title: 'Midterm', maximumGrade: 50 + } + json = JSON.parse(response.body) + expect(json['assessment']).to include('floorAtZero' => true, 'capAtMaximum' => true) + end + + it 'persists explicit bound flags on create' do + post :create, as: :json, params: { + course_id: course, title: 'Bonus', maximumGrade: 10, + floorAtZero: false, capAtMaximum: false + } + json = JSON.parse(response.body) + expect(json['assessment']).to include('floorAtZero' => false, 'capAtMaximum' => false) + end + + it 'persists a weight when weighted view is enabled' do + context = OpenStruct.new(current_course: course, key: Course::GradebookComponent.key) + Course::Settings::GradebookComponent.new(context).weighted_view_enabled = true + course.save! + + post :create, as: :json, params: { + course_id: course, title: 'Presentation', maximumGrade: 10, + weight: 25 + } + + created = Course::ExternalAssessment.last + json = JSON.parse(response.body) + expect(created.gradebook_contribution.weight).to eq(25) + expect(json['assessment']['gradebookWeight']).to eq(25.0) + expect(json['tab']['gradebookWeight']).to eq(25.0) + end + + it 'ignores a weight when weighted view is disabled' do + post :create, as: :json, params: { + course_id: course, title: 'Presentation', maximumGrade: 10, + weight: 25 + } + + created = Course::ExternalAssessment.last + json = JSON.parse(response.body) + expect(created.gradebook_contribution.weight).to eq(0) + expect(json['assessment']).not_to have_key('gradebookWeight') + expect(json['tab']).not_to have_key('gradebookWeight') + end + + it 'returns 422 on a blank title' do + post :create, params: params.merge(title: '') + expect(response).to have_http_status(:unprocessable_content) + end + + it 'returns 422 on a blank maximumGrade' do + post :create, params: params.merge(maximumGrade: '') + expect(response).to have_http_status(:unprocessable_content) + end + + it "serializes the tab weightMode as 'equal' when weighted" do + context = OpenStruct.new(current_course: course, key: Course::GradebookComponent.key) + Course::Settings::GradebookComponent.new(context).weighted_view_enabled = true + course.save! + + post :create, as: :json, params: { + course_id: course, title: 'Presentation', maximumGrade: 10, weight: 25 + } + json = JSON.parse(response.body) + expect(json['tab']['weightMode']).to eq('equal') + end + end + + context 'as a teaching assistant' do + before { controller_sign_in(controller, ta.user) } + + it 'is denied' do + expect { post :create, params: params }.to raise_error(CanCan::AccessDenied) + end + end + + context 'as an observer' do + before { controller_sign_in(controller, observer.user) } + + it 'is denied' do + expect { post :create, params: params }.to raise_error(CanCan::AccessDenied) + end + end + end + + describe '#update' do + render_views + let!(:external) { create(:course_external_assessment, course: course, title: 'Mid') } + let(:gb_student) { create(:course_student, course: course) } + + context 'as a manager' do + before { controller_sign_in(controller, manager.user) } + + it 'renames and changes the maximum grade without touching any tab' do + expect do + patch :update, params: { course_id: course.id, id: external.id, format: :json, + title: 'Midterm', maximumGrade: 60 } + end.not_to change(Course::Assessment::Tab, :count) + expect(response).to be_successful + body = JSON.parse(response.body) + expect(body['assessment']['id']).to eq(-external.id) + expect(body['assessment']['title']).to eq('Midterm') + expect(body['assessment']['maxGrade']).to eq(60.0) + expect(body['assessment']['external']).to be(true) + expect(body['assessment']['gradebookExcluded']).to be(false) + expect(body['tab']['id']).to eq(-external.id) + expect(body['tab']['title']).to eq('Midterm') + expect(body['tab']['categoryId']).to eq(Course::ExternalAssessment::SYNTHETIC_CATEGORY_ID) + expect(external.reload.title).to eq('Midterm') + expect(external.maximum_grade).to eq(60) + end + + it 'stores a maximum grade with two decimal places without rounding' do + patch :update, params: { course_id: course.id, id: external.id, format: :json, + maximumGrade: '99.25' } + expect(response).to be_successful + expect(external.reload.maximum_grade).to eq(BigDecimal('99.25')) + end + + it 'returns 404 when the external belongs to another course' do + other_external = create(:course_external_assessment) + patch :update, params: { course_id: course.id, id: other_external.id, format: :json, title: 'X' } + expect(response).to have_http_status(:not_found) + end + + it 'returns 404 when grading an external that belongs to another course' do + other_external = create(:course_external_assessment) + put :grades, params: { course_id: course.id, id: other_external.id, format: :json, + studentId: gb_student.user_id, grade: 50 } + expect(response).to have_http_status(:not_found) + end + + it 'returns 422 on a blank title' do + patch :update, params: { course_id: course.id, id: external.id, format: :json, + title: '' } + expect(response).to have_http_status(:unprocessable_content) + end + + it 'updates a bound flag' do + external = create(:course_external_assessment, + course: course, title: 'Quiz', maximum_grade: 20) + patch :update, as: :json, params: { + course_id: course, id: external.id, capAtMaximum: false + } + expect(external.reload.cap_at_maximum).to be(false) + expect(external.floor_at_zero).to be(true) + end + + context 'when weighted view is enabled' do + let!(:weighted_external) do + Course::ExternalAssessment.create_for_course!( + course: course, title: 'Final', maximum_grade: 100, weight: 10 + ) + end + + before do + context = OpenStruct.new(current_course: course, key: Course::GradebookComponent.key) + Course::Settings::GradebookComponent.new(context).weighted_view_enabled = true + course.save! + end + + it 'updates the contribution weight and echoes it back' do + patch :update, as: :json, params: { + course_id: course, id: weighted_external.id, weight: 40 + } + expect(response).to be_successful + expect(weighted_external.gradebook_contribution.reload.weight).to eq(40) + body = JSON.parse(response.body) + expect(body['assessment']['gradebookWeight']).to eq(40.0) + expect(body['tab']['gradebookWeight']).to eq(40.0) + end + + it "serializes the tab weightMode as 'equal'" do + patch :update, as: :json, params: { + course_id: course, id: weighted_external.id, weight: 40 + } + body = JSON.parse(response.body) + expect(body['tab']['weightMode']).to eq('equal') + end + + it 'leaves the contribution weight untouched when no weight param is sent' do + patch :update, as: :json, params: { + course_id: course, id: weighted_external.id, title: 'Renamed' + } + expect(response).to be_successful + expect(weighted_external.gradebook_contribution.reload.weight).to eq(10) + end + end + + it 'ignores a weight when weighted view is disabled' do + weighted_external = Course::ExternalAssessment.create_for_course!( + course: course, title: 'Final', maximum_grade: 100, weight: 10 + ) + patch :update, as: :json, params: { + course_id: course, id: weighted_external.id, weight: 40 + } + expect(weighted_external.gradebook_contribution.reload.weight).to eq(10) + body = JSON.parse(response.body) + expect(body['assessment']).not_to have_key('gradebookWeight') + expect(body['tab']).not_to have_key('gradebookWeight') + end + + it 'inserts a grade for a student who has none' do + expect do + put :grades, params: { course_id: course.id, id: external.id, format: :json, + studentId: gb_student.user_id, grade: 70 } + end.to change { Course::ExternalAssessmentGrade.count }.by(1) + expect(response).to be_successful + end + end + + context 'as a teaching assistant' do + before { controller_sign_in(controller, ta.user) } + + it 'is denied' do + expect do + patch :update, params: { course_id: course.id, id: external.id, format: :json, title: 'X' } + end.to raise_error(CanCan::AccessDenied) + end + end + end + + describe '#destroy' do + let!(:external) { create(:course_external_assessment, course: course) } + + context 'as a manager' do + before { controller_sign_in(controller, manager.user) } + + it 'deletes the external and cascades grades' do + create(:course_external_assessment_grade, external_assessment: external) + expect do + delete :destroy, params: { course_id: course.id, id: external.id, format: :json } + end.to change { Course::ExternalAssessment.count }.by(-1). + and change { Course::ExternalAssessmentGrade.count }.by(-1) + expect(response).to be_successful + end + + it 'returns 404 when the external belongs to another course' do + other_external = create(:course_external_assessment) + expect do + delete :destroy, params: { course_id: course.id, id: other_external.id, format: :json } + end.not_to(change { Course::ExternalAssessment.count }) + expect(response).to have_http_status(:not_found) + end + end + + context 'as a teaching assistant' do + before { controller_sign_in(controller, ta.user) } + + it 'is denied' do + expect { delete :destroy, params: { course_id: course.id, id: external.id, format: :json } }. + to raise_error(CanCan::AccessDenied) + end + end + end describe '#grades' do render_views @@ -76,9 +366,8 @@ end end - context 'as a student' do - let(:viewer) { create(:course_student, course: course) } - before { controller_sign_in(controller, viewer.user) } + context 'as an observer' do + before { controller_sign_in(controller, observer.user) } it 'is denied' do expect do @@ -88,5 +377,28 @@ end end end + + describe '#reorder' do + let!(:a) { create(:course_external_assessment, course: course) } + let!(:b) { create(:course_external_assessment, course: course) } + let!(:c) { create(:course_external_assessment, course: course) } + + context 'as a manager' do + before { controller_sign_in(controller, manager.user) } + + it 'rewrites positions to the given order' do + put :reorder, params: { course_id: course.id, format: :json, + orderedIds: [c.id, a.id, b.id] } + expect(response).to have_http_status(:ok) + expect([a.reload.position, b.reload.position, c.reload.position]).to eq([1, 2, 0]) + end + + it 'rejects a payload whose id set does not match' do + put :reorder, params: { course_id: course.id, format: :json, + orderedIds: [a.id, b.id] } + expect(response).to have_http_status(:unprocessable_content) + end + end + end end end diff --git a/spec/controllers/course/gradebook_controller_spec.rb b/spec/controllers/course/gradebook_controller_spec.rb index 9c8defbe73..b5d45f2e53 100644 --- a/spec/controllers/course/gradebook_controller_spec.rb +++ b/spec/controllers/course/gradebook_controller_spec.rb @@ -800,5 +800,26 @@ def weight_for(tab) end end end + + describe 'external assessment ordering' do + render_views + let(:manager) { create(:course_manager, course: course) } + + it 'serializes externals in position order, not creation order' do + first = create(:course_external_assessment, course: course, title: 'Zeta') + second = create(:course_external_assessment, course: course, title: 'Alpha') + # Make Alpha come first by position. + Course::ExternalAssessment.reorder!(course: course, ordered_ids: [second.id, first.id]) + + controller_sign_in(controller, manager.user) + get :index, params: { course_id: course.id, format: :json } + + body = JSON.parse(response.body) + external_titles = body['tabs']. + select { |t| t['categoryId'] == Course::ExternalAssessment::SYNTHETIC_CATEGORY_ID }. + map { |t| t['title'] } + expect(external_titles).to eq(%w[Alpha Zeta]) + end + end end end diff --git a/spec/models/course/external_assessment_spec.rb b/spec/models/course/external_assessment_spec.rb index 21451a2cc3..fa461cd1fd 100644 --- a/spec/models/course/external_assessment_spec.rb +++ b/spec/models/course/external_assessment_spec.rb @@ -124,5 +124,38 @@ expect(external.cap_at_maximum).to be(false) end end + + describe 'positioning' do + let(:course) { create(:course) } + + it 'appends new assessments at the end of the course' do + first = create(:course_external_assessment, course: course) + second = create(:course_external_assessment, course: course) + expect([first.reload.position, second.reload.position]).to eq([0, 1]) + end + + it 'scopes positions per course' do + other = create(:course) + a = create(:course_external_assessment, course: course) + b = create(:course_external_assessment, course: other) + expect([a.reload.position, b.reload.position]).to eq([0, 0]) + end + + describe '.reorder!' do + it 'rewrites positions to the given order' do + a = create(:course_external_assessment, course: course) + b = create(:course_external_assessment, course: course) + c = create(:course_external_assessment, course: course) + described_class.reorder!(course: course, ordered_ids: [c.id, a.id, b.id]) + expect([a.reload.position, b.reload.position, c.reload.position]).to eq([1, 2, 0]) + end + + it 'raises when the id set does not match the course externals' do + a = create(:course_external_assessment, course: course) + expect { described_class.reorder!(course: course, ordered_ids: [a.id, a.id + 999]) }. + to raise_error(ArgumentError) + end + end + end end end