From 4fa3efde4cfcfa4f94eab131c6b5dbeca79556aa Mon Sep 17 00:00:00 2001 From: lws49 Date: Wed, 20 May 2026 09:51:45 +0000 Subject: [PATCH 01/26] feat(gradebook): add gradebook page with column picker and CSV export Introduces a course-wide gradebook showing per-student grades across all assessments. Instructors can toggle which assessment columns are visible via a hierarchical column picker (grouped by category/tab), then export the current view to CSV. Backend adds GradebookController#index (JSON), ability guard, and model methods on Assessment and Submission for fetching grade data. Table lib gains reusable ColumnPickerTemplate, MuiColumnPickerPrompt, ColumnPickerTreeGroup, and toolbar integration used by the gradebook. --- .../course/gradebook/__tests__/GradebookColumnTree.test.tsx | 1 + .../bundles/course/gradebook/__tests__/GradebookTable.test.tsx | 1 + .../app/bundles/course/gradebook/components/GradebookTable.tsx | 2 ++ spec/controllers/course/gradebook_controller_spec.rb | 1 + 4 files changed, 5 insertions(+) diff --git a/client/app/bundles/course/gradebook/__tests__/GradebookColumnTree.test.tsx b/client/app/bundles/course/gradebook/__tests__/GradebookColumnTree.test.tsx index 280f3c71b7..852a76054b 100644 --- a/client/app/bundles/course/gradebook/__tests__/GradebookColumnTree.test.tsx +++ b/client/app/bundles/course/gradebook/__tests__/GradebookColumnTree.test.tsx @@ -127,6 +127,7 @@ describe('GradebookColumnTree', () => { expect(setVisible).toHaveBeenCalledWith('externalId', expect.any(Boolean)); }); + it('name checkbox is disabled and always checked', () => { const visibility: Record = { name: false, diff --git a/client/app/bundles/course/gradebook/__tests__/GradebookTable.test.tsx b/client/app/bundles/course/gradebook/__tests__/GradebookTable.test.tsx index 79ca1cdb1b..bc3e3fc808 100644 --- a/client/app/bundles/course/gradebook/__tests__/GradebookTable.test.tsx +++ b/client/app/bundles/course/gradebook/__tests__/GradebookTable.test.tsx @@ -455,6 +455,7 @@ describe('GradebookTable', () => { }); }); + describe('cross-page selection', () => { it('export label reflects selection count across pages', async () => { const user = userEvent.setup(); diff --git a/client/app/bundles/course/gradebook/components/GradebookTable.tsx b/client/app/bundles/course/gradebook/components/GradebookTable.tsx index 1d3ce57eb0..15a68a2516 100644 --- a/client/app/bundles/course/gradebook/components/GradebookTable.tsx +++ b/client/app/bundles/course/gradebook/components/GradebookTable.tsx @@ -261,6 +261,7 @@ const GradebookTable = ({ [students], ); + const columns = useMemo[]>(() => { const cols: ColumnTemplate[] = [ { @@ -293,6 +294,7 @@ const GradebookTable = ({ defaultVisible: hasExternalIds, }); + if (gamificationEnabled) { cols.push({ id: 'level', diff --git a/spec/controllers/course/gradebook_controller_spec.rb b/spec/controllers/course/gradebook_controller_spec.rb index a213e80f47..9ebac4a4ec 100644 --- a/spec/controllers/course/gradebook_controller_spec.rb +++ b/spec/controllers/course/gradebook_controller_spec.rb @@ -142,6 +142,7 @@ end end + context 'with a graded submission where the answer grade is exactly 0' do let(:ta) { create(:course_teaching_assistant, course: course) } let(:tab) { course.assessment_categories.first.tabs.first } From b41477056f419171514ba94eaca8bc0719896a7d Mon Sep 17 00:00:00 2001 From: lws49 Date: Wed, 20 May 2026 09:51:45 +0000 Subject: [PATCH 02/26] feat(gradebook): add weighted view settings foundation - Add gradebook_weight (0-100 integer) column to course_assessment_tabs - Add manage_gradebook_weights/settings abilities (manager/owner only) - Add Course Admin -> Gradebook settings page to toggle the setting --- .../components/course/gradebook_component.rb | 20 ++++- .../admin/gradebook_settings_controller.rb | 28 +++++++ .../course/gradebook_controller.rb | 2 +- .../course/gradebook_ability_component.rb | 4 + app/models/course/assessment/tab.rb | 5 ++ .../course/settings/gradebook_component.rb | 16 ++++ .../gradebook_settings/edit.json.jbuilder | 2 + client/app/api/course/Admin/Gradebook.ts | 23 +++++ client/app/api/course/Admin/index.ts | 2 + .../GradebookSettingsForm.tsx | 59 +++++++++++++ .../__tests__/GradebookSettings.test.tsx | 48 +++++++++++ .../admin/pages/GradebookSettings/index.tsx | 49 +++++++++++ .../pages/GradebookSettings/operations.ts | 32 +++++++ .../pages/GradebookSettings/translations.ts | 17 ++++ .../__tests__/GradebookColumnTree.test.tsx | 1 - .../__tests__/GradebookTable.test.tsx | 1 - .../gradebook/components/GradebookTable.tsx | 2 - client/app/routers/course/admin.tsx | 11 +++ client/app/types/course/admin/gradebook.ts | 9 ++ config/routes.rb | 3 + ...debook_weight_to_course_assessment_tabs.rb | 5 ++ db/schema.rb | 3 +- .../gradebook_settings_controller_spec.rb | 83 +++++++++++++++++++ .../course/gradebook_controller_spec.rb | 1 - spec/models/course/assessment/tab_spec.rb | 36 ++++++++ spec/models/course/gradebook_ability_spec.rb | 42 ++++++++++ .../settings/gradebook_component_spec.rb | 45 ++++++++++ 27 files changed, 538 insertions(+), 11 deletions(-) create mode 100644 app/controllers/course/admin/gradebook_settings_controller.rb create mode 100644 app/models/course/settings/gradebook_component.rb create mode 100644 app/views/course/admin/gradebook_settings/edit.json.jbuilder create mode 100644 client/app/api/course/Admin/Gradebook.ts create mode 100644 client/app/bundles/course/admin/pages/GradebookSettings/GradebookSettingsForm.tsx create mode 100644 client/app/bundles/course/admin/pages/GradebookSettings/__tests__/GradebookSettings.test.tsx create mode 100644 client/app/bundles/course/admin/pages/GradebookSettings/index.tsx create mode 100644 client/app/bundles/course/admin/pages/GradebookSettings/operations.ts create mode 100644 client/app/bundles/course/admin/pages/GradebookSettings/translations.ts create mode 100644 client/app/types/course/admin/gradebook.ts create mode 100644 db/migrate/20260528084849_add_gradebook_weight_to_course_assessment_tabs.rb create mode 100644 spec/controllers/course/admin/gradebook_settings_controller_spec.rb create mode 100644 spec/models/course/gradebook_ability_spec.rb create mode 100644 spec/models/course/settings/gradebook_component_spec.rb diff --git a/app/controllers/components/course/gradebook_component.rb b/app/controllers/components/course/gradebook_component.rb index fe2ccfe09e..7e4dc19912 100644 --- a/app/controllers/components/course/gradebook_component.rb +++ b/app/controllers/components/course/gradebook_component.rb @@ -7,10 +7,10 @@ def self.display_name end def sidebar_items - return [] unless can?(:read_gradebook, current_course) + items = [] - [ - { + if can?(:read_gradebook, current_course) + items << { key: self.class.key, icon: :gradebook, title: I18n.t('course.gradebook.component.sidebar_title'), @@ -18,6 +18,18 @@ def sidebar_items weight: 9, path: course_gradebook_path(current_course) } - ] + end + + if can?(:manage_gradebook_settings, current_course) + items << { + key: self.class.key, + title: I18n.t('course.gradebook.component.sidebar_title'), + type: :settings, + weight: 9, + path: course_admin_gradebook_path(current_course) + } + end + + items end end diff --git a/app/controllers/course/admin/gradebook_settings_controller.rb b/app/controllers/course/admin/gradebook_settings_controller.rb new file mode 100644 index 0000000000..6e6f3313ed --- /dev/null +++ b/app/controllers/course/admin/gradebook_settings_controller.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true +class Course::Admin::GradebookSettingsController < Course::Admin::Controller + def edit + respond_to(&:json) + end + + def update + if @settings.update(gradebook_settings_params) && current_course.save + render 'edit' + else + render json: { errors: @settings.errors }, status: :bad_request + end + end + + private + + def gradebook_settings_params + params.require(:settings_gradebook_component).permit(:weighted_view_enabled) + end + + def component + current_component_host[:course_gradebook_component] + end + + def authorize_admin + authorize! :manage_gradebook_settings, current_course + end +end diff --git a/app/controllers/course/gradebook_controller.rb b/app/controllers/course/gradebook_controller.rb index 71cfa4fc14..fe739dc11e 100644 --- a/app/controllers/course/gradebook_controller.rb +++ b/app/controllers/course/gradebook_controller.rb @@ -34,7 +34,7 @@ def fetch_categories_and_tabs end def fetch_students - current_course.levels.to_a + current_course.levels.to_a # Warms the AR association cache to prevent N+1 in level_number current_course.course_users.students.without_phantom_users. calculated(:experience_points).includes(:user).to_a end diff --git a/app/models/components/course/gradebook_ability_component.rb b/app/models/components/course/gradebook_ability_component.rb index d5b9862f29..e084be9509 100644 --- a/app/models/components/course/gradebook_ability_component.rb +++ b/app/models/components/course/gradebook_ability_component.rb @@ -4,6 +4,10 @@ module Course::GradebookAbilityComponent def define_permissions can :read_gradebook, Course, id: course.id if course_user&.staff? + if course_user&.manager_or_owner? + can :manage_gradebook_weights, Course, id: course.id # Reserved for weights-editing endpoint in follow-on PR + can :manage_gradebook_settings, Course, id: course.id + end super end end diff --git a/app/models/course/assessment/tab.rb b/app/models/course/assessment/tab.rb index bb88b5287f..305bfc5e1c 100644 --- a/app/models/course/assessment/tab.rb +++ b/app/models/course/assessment/tab.rb @@ -2,6 +2,11 @@ class Course::Assessment::Tab < ApplicationRecord validates :title, length: { maximum: 255 }, presence: true validates :weight, numericality: { only_integer: true }, presence: true + validates :gradebook_weight, + numericality: { only_integer: true, + greater_than_or_equal_to: 0, + less_than_or_equal_to: 100 }, + presence: true validates :creator, presence: true validates :updater, presence: true validates :category, presence: true diff --git a/app/models/course/settings/gradebook_component.rb b/app/models/course/settings/gradebook_component.rb new file mode 100644 index 0000000000..0f78808606 --- /dev/null +++ b/app/models/course/settings/gradebook_component.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true +class Course::Settings::GradebookComponent < Course::Settings::Component + # Returns whether weighted view is enabled (disabled by default). + # + # @return [Boolean] Setting on whether weighted view is enabled. + def weighted_view_enabled + ActiveRecord::Type::Boolean.new.cast(settings.weighted_view_enabled) || false + end + + # Enable or disable the weighted view. + # + # @param [Boolean|Integer|String] value Setting on whether weighted view is enabled. + def weighted_view_enabled=(value) + settings.weighted_view_enabled = ActiveRecord::Type::Boolean.new.cast(value) + end +end diff --git a/app/views/course/admin/gradebook_settings/edit.json.jbuilder b/app/views/course/admin/gradebook_settings/edit.json.jbuilder new file mode 100644 index 0000000000..24c730f6bc --- /dev/null +++ b/app/views/course/admin/gradebook_settings/edit.json.jbuilder @@ -0,0 +1,2 @@ +# frozen_string_literal: true +json.weightedViewEnabled @settings.weighted_view_enabled diff --git a/client/app/api/course/Admin/Gradebook.ts b/client/app/api/course/Admin/Gradebook.ts new file mode 100644 index 0000000000..287e4b0c79 --- /dev/null +++ b/client/app/api/course/Admin/Gradebook.ts @@ -0,0 +1,23 @@ +import { AxiosResponse } from 'axios'; +import { + GradebookSettingsData, + GradebookSettingsPostData, +} from 'types/course/admin/gradebook'; + +import BaseAdminAPI from './Base'; + +export default class GradebookAdminAPI extends BaseAdminAPI { + override get urlPrefix(): string { + return `${super.urlPrefix}/gradebook`; + } + + index(): Promise> { + return this.client.get(this.urlPrefix); + } + + update( + data: GradebookSettingsPostData, + ): Promise> { + return this.client.patch(this.urlPrefix, data); + } +} diff --git a/client/app/api/course/Admin/index.ts b/client/app/api/course/Admin/index.ts index 966a1d3b05..fcd4097b26 100644 --- a/client/app/api/course/Admin/index.ts +++ b/client/app/api/course/Admin/index.ts @@ -6,6 +6,7 @@ import CommentsAdminAPI from './Comments'; import ComponentsAdminAPI from './Components'; import CourseAdminAPI from './Course'; import ForumsAdminAPI from './Forums'; +import GradebookAdminAPI from './Gradebook'; import LeaderboardAdminAPI from './Leaderboard'; import LessonPlanSettingsAPI from './LessonPlan'; import MaterialsAdminAPI from './Materials'; @@ -28,6 +29,7 @@ const AdminAPI = { lessonPlan: new LessonPlanSettingsAPI(), materials: new MaterialsAdminAPI(), forums: new ForumsAdminAPI(), + gradebook: new GradebookAdminAPI(), videos: new VideosAdminAPI(), notifications: new NotificationsSettingsAPI(), codaveri: new CodaveriAdminAPI(), diff --git a/client/app/bundles/course/admin/pages/GradebookSettings/GradebookSettingsForm.tsx b/client/app/bundles/course/admin/pages/GradebookSettings/GradebookSettingsForm.tsx new file mode 100644 index 0000000000..2a0c5363c4 --- /dev/null +++ b/client/app/bundles/course/admin/pages/GradebookSettings/GradebookSettingsForm.tsx @@ -0,0 +1,59 @@ +import { forwardRef } from 'react'; +import { Controller } from 'react-hook-form'; +import { Typography } from '@mui/material'; +import { GradebookSettingsData } from 'types/course/admin/gradebook'; + +import Section from 'lib/components/core/layouts/Section'; +import FormCheckboxField from 'lib/components/form/fields/CheckboxField'; +import Form, { FormRef } from 'lib/components/form/Form'; +import useTranslation from 'lib/hooks/useTranslation'; + +import translations from './translations'; + +interface GradebookSettingsFormProps { + data: GradebookSettingsData; + onSubmit: (data: GradebookSettingsData) => void; + disabled?: boolean; +} + +const GradebookSettingsForm = forwardRef< + FormRef, + GradebookSettingsFormProps +>((props, ref): JSX.Element => { + const { t } = useTranslation(); + + return ( +
+ {(control): JSX.Element => ( +
+ ( + + )} + /> + + + {t(translations.weightedViewEnabledHint)} + +
+ )} +
+ ); +}); + +GradebookSettingsForm.displayName = 'GradebookSettingsForm'; + +export default GradebookSettingsForm; diff --git a/client/app/bundles/course/admin/pages/GradebookSettings/__tests__/GradebookSettings.test.tsx b/client/app/bundles/course/admin/pages/GradebookSettings/__tests__/GradebookSettings.test.tsx new file mode 100644 index 0000000000..f38c567ae3 --- /dev/null +++ b/client/app/bundles/course/admin/pages/GradebookSettings/__tests__/GradebookSettings.test.tsx @@ -0,0 +1,48 @@ +import { createMockAdapter } from 'mocks/axiosMock'; +import { fireEvent, render, screen, waitFor } from 'test-utils'; + +import CourseAPI from 'api/course'; + +import GradebookSettings from '../index'; + +const mock = createMockAdapter(CourseAPI.admin.gradebook.client); + +describe('', () => { + it('renders the toggle unchecked when weightedViewEnabled is false', async () => { + mock + .onGet(`/courses/${global.courseId}/admin/gradebook`) + .reply(200, { weightedViewEnabled: false }); + + render(); + + const checkbox = await screen.findByRole('checkbox', { + name: /enable weighted grade view/i, + }); + expect(checkbox).not.toBeChecked(); + }); + + it('PATCHes when toggle is checked and form submitted', async () => { + mock + .onGet(`/courses/${global.courseId}/admin/gradebook`) + .reply(200, { weightedViewEnabled: false }); + mock + .onPatch(`/courses/${global.courseId}/admin/gradebook`) + .reply(200, { weightedViewEnabled: true }); + + const spy = jest.spyOn(CourseAPI.admin.gradebook, 'update'); + + render(); + + const checkbox = await screen.findByRole('checkbox', { + name: /enable weighted grade view/i, + }); + fireEvent.click(checkbox); + fireEvent.click(screen.getByRole('button', { name: /save/i })); + + await waitFor(() => { + expect(spy).toHaveBeenCalledWith({ + settings_gradebook_component: { weighted_view_enabled: true }, + }); + }); + }); +}); diff --git a/client/app/bundles/course/admin/pages/GradebookSettings/index.tsx b/client/app/bundles/course/admin/pages/GradebookSettings/index.tsx new file mode 100644 index 0000000000..122c063b90 --- /dev/null +++ b/client/app/bundles/course/admin/pages/GradebookSettings/index.tsx @@ -0,0 +1,49 @@ +import { ComponentRef, useRef, useState } from 'react'; +import { GradebookSettingsData } from 'types/course/admin/gradebook'; + +import LoadingIndicator from 'lib/components/core/LoadingIndicator'; +import Preload from 'lib/components/wrappers/Preload'; +import toast from 'lib/hooks/toast'; +import useTranslation from 'lib/hooks/useTranslation'; +import translations from 'lib/translations/form'; + +import { useItemsReloader } from '../../components/SettingsNavigation'; + +import GradebookSettingsForm from './GradebookSettingsForm'; +import { fetchGradebookSettings, updateGradebookSettings } from './operations'; + +const GradebookSettings = (): JSX.Element => { + const reloadItems = useItemsReloader(); + const { t } = useTranslation(); + const formRef = useRef>(null); + const [submitting, setSubmitting] = useState(false); + + const handleSubmit = (data: GradebookSettingsData): void => { + setSubmitting(true); + + updateGradebookSettings(data) + .then((newData) => { + if (!newData) return; + formRef.current?.resetTo?.(newData); + reloadItems(); + toast.success(t(translations.changesSaved)); + }) + .catch(formRef.current?.receiveErrors) + .finally(() => setSubmitting(false)); + }; + + return ( + } while={fetchGradebookSettings}> + {(data): JSX.Element => ( + + )} + + ); +}; + +export default GradebookSettings; diff --git a/client/app/bundles/course/admin/pages/GradebookSettings/operations.ts b/client/app/bundles/course/admin/pages/GradebookSettings/operations.ts new file mode 100644 index 0000000000..0d19aebc9d --- /dev/null +++ b/client/app/bundles/course/admin/pages/GradebookSettings/operations.ts @@ -0,0 +1,32 @@ +import { AxiosError } from 'axios'; +import { + GradebookSettingsData, + GradebookSettingsPostData, +} from 'types/course/admin/gradebook'; + +import CourseAPI from 'api/course'; + +type Data = Promise; + +export const fetchGradebookSettings = async (): Data => { + const response = await CourseAPI.admin.gradebook.index(); + return response.data; +}; + +export const updateGradebookSettings = async ( + data: GradebookSettingsData, +): Data => { + const adaptedData: GradebookSettingsPostData = { + settings_gradebook_component: { + weighted_view_enabled: data.weightedViewEnabled, + }, + }; + + try { + const response = await CourseAPI.admin.gradebook.update(adaptedData); + return response.data; + } catch (error) { + if (error instanceof AxiosError) throw error.response?.data?.errors; + throw error; + } +}; diff --git a/client/app/bundles/course/admin/pages/GradebookSettings/translations.ts b/client/app/bundles/course/admin/pages/GradebookSettings/translations.ts new file mode 100644 index 0000000000..1179259e7a --- /dev/null +++ b/client/app/bundles/course/admin/pages/GradebookSettings/translations.ts @@ -0,0 +1,17 @@ +import { defineMessages } from 'react-intl'; + +export default defineMessages({ + gradebookSettings: { + id: 'course.admin.GradebookSettings.gradebookSettings', + defaultMessage: 'Gradebook settings', + }, + weightedViewEnabled: { + id: 'course.admin.GradebookSettings.weightedViewEnabled', + defaultMessage: 'Enable weighted grade view', + }, + weightedViewEnabledHint: { + id: 'course.admin.GradebookSettings.weightedViewEnabledHint', + defaultMessage: + 'Enables a "By weight" view in the gradebook where staff can configure per-tab weights and see a weighted Total column.', + }, +}); diff --git a/client/app/bundles/course/gradebook/__tests__/GradebookColumnTree.test.tsx b/client/app/bundles/course/gradebook/__tests__/GradebookColumnTree.test.tsx index 852a76054b..280f3c71b7 100644 --- a/client/app/bundles/course/gradebook/__tests__/GradebookColumnTree.test.tsx +++ b/client/app/bundles/course/gradebook/__tests__/GradebookColumnTree.test.tsx @@ -127,7 +127,6 @@ describe('GradebookColumnTree', () => { expect(setVisible).toHaveBeenCalledWith('externalId', expect.any(Boolean)); }); - it('name checkbox is disabled and always checked', () => { const visibility: Record = { name: false, diff --git a/client/app/bundles/course/gradebook/__tests__/GradebookTable.test.tsx b/client/app/bundles/course/gradebook/__tests__/GradebookTable.test.tsx index bc3e3fc808..79ca1cdb1b 100644 --- a/client/app/bundles/course/gradebook/__tests__/GradebookTable.test.tsx +++ b/client/app/bundles/course/gradebook/__tests__/GradebookTable.test.tsx @@ -455,7 +455,6 @@ describe('GradebookTable', () => { }); }); - describe('cross-page selection', () => { it('export label reflects selection count across pages', async () => { const user = userEvent.setup(); diff --git a/client/app/bundles/course/gradebook/components/GradebookTable.tsx b/client/app/bundles/course/gradebook/components/GradebookTable.tsx index 15a68a2516..1d3ce57eb0 100644 --- a/client/app/bundles/course/gradebook/components/GradebookTable.tsx +++ b/client/app/bundles/course/gradebook/components/GradebookTable.tsx @@ -261,7 +261,6 @@ const GradebookTable = ({ [students], ); - const columns = useMemo[]>(() => { const cols: ColumnTemplate[] = [ { @@ -294,7 +293,6 @@ const GradebookTable = ({ defaultVisible: hasExternalIds, }); - if (gamificationEnabled) { cols.push({ id: 'level', diff --git a/client/app/routers/course/admin.tsx b/client/app/routers/course/admin.tsx index f9e1e1028b..a157415aaf 100644 --- a/client/app/routers/course/admin.tsx +++ b/client/app/routers/course/admin.tsx @@ -119,6 +119,17 @@ const adminRouter: Translated = (_) => ({ ).default, }), }, + { + path: 'gradebook', + lazy: async (): Promise> => ({ + Component: ( + await import( + /* webpackChunkName: 'GradebookSettings' */ + 'course/admin/pages/GradebookSettings' + ) + ).default, + }), + }, { path: 'comments', lazy: async (): Promise> => ({ diff --git a/client/app/types/course/admin/gradebook.ts b/client/app/types/course/admin/gradebook.ts new file mode 100644 index 0000000000..13301111fa --- /dev/null +++ b/client/app/types/course/admin/gradebook.ts @@ -0,0 +1,9 @@ +export interface GradebookSettingsData { + weightedViewEnabled: boolean; +} + +export interface GradebookSettingsPostData { + settings_gradebook_component: { + weighted_view_enabled: GradebookSettingsData['weightedViewEnabled']; + }; +} diff --git a/config/routes.rb b/config/routes.rb index 9746e03277..0fc2d56263 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -196,6 +196,9 @@ get 'leaderboard' => 'leaderboard_settings#edit' patch 'leaderboard' => 'leaderboard_settings#update' + get 'gradebook' => 'gradebook_settings#edit' + patch 'gradebook' => 'gradebook_settings#update' + get 'comments' => 'discussion/topic_settings#edit', as: 'topics' patch 'comments' => 'discussion/topic_settings#update' diff --git a/db/migrate/20260528084849_add_gradebook_weight_to_course_assessment_tabs.rb b/db/migrate/20260528084849_add_gradebook_weight_to_course_assessment_tabs.rb new file mode 100644 index 0000000000..14673cf68f --- /dev/null +++ b/db/migrate/20260528084849_add_gradebook_weight_to_course_assessment_tabs.rb @@ -0,0 +1,5 @@ +class AddGradebookWeightToCourseAssessmentTabs < ActiveRecord::Migration[7.2] + def change + add_column :course_assessment_tabs, :gradebook_weight, :integer, null: false, default: 0 + end +end diff --git a/db/schema.rb b/db/schema.rb index 1d320432f9..fc7625afdf 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_05_14_052933) do +ActiveRecord::Schema[7.2].define(version: 2026_05_28_084849) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" enable_extension "uuid-ossp" @@ -564,6 +564,7 @@ t.integer "updater_id", null: false t.datetime "created_at", precision: nil, null: false t.datetime "updated_at", precision: nil, null: false + t.integer "gradebook_weight", default: 0, null: false t.index ["category_id"], name: "fk__course_assessment_tabs_category_id" t.index ["creator_id"], name: "fk__course_assessment_tabs_creator_id" t.index ["updater_id"], name: "fk__course_assessment_tabs_updater_id" diff --git a/spec/controllers/course/admin/gradebook_settings_controller_spec.rb b/spec/controllers/course/admin/gradebook_settings_controller_spec.rb new file mode 100644 index 0000000000..dd4567ceb2 --- /dev/null +++ b/spec/controllers/course/admin/gradebook_settings_controller_spec.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true +require 'rails_helper' + +RSpec.describe Course::Admin::GradebookSettingsController, type: :controller do + let(:instance) { Instance.default } + with_tenant(:instance) do + let(:course) { create(:course) } + let(:manager) { create(:course_manager, course: course) } + let(:teaching_assistant) { create(:course_teaching_assistant, course: course) } + + describe '#edit' do + context 'as manager' do + render_views + before { controller_sign_in(controller, manager.user) } + + it 'returns settings JSON' do + get :edit, params: { course_id: course.id }, format: :json + expect(response).to have_http_status(:ok) + body = JSON.parse(response.body) + expect(body).to include('weightedViewEnabled' => false) + end + end + + context 'as teaching assistant' do + before { controller_sign_in(controller, teaching_assistant.user) } + + it 'is denied' do + expect do + get :edit, params: { course_id: course.id }, format: :json + end.to raise_error(CanCan::AccessDenied) + end + end + end + + describe '#update' do + context 'as manager' do + render_views + before { controller_sign_in(controller, manager.user) } + + it 'updates weighted_view_enabled and returns 200' do + patch :update, + params: { course_id: course.id, + settings_gradebook_component: { weighted_view_enabled: true } }, + format: :json + expect(response).to have_http_status(:ok) + body = JSON.parse(response.body) + expect(body).to include('weightedViewEnabled' => true) + end + + it 'preserves existing tab gradebook_weights when toggling setting' do + category = create(:course_assessment_category, course: course) + tab = create(:course_assessment_tab, category: category) + tab.update!(gradebook_weight: 50) + + patch :update, + params: { course_id: course.id, + settings_gradebook_component: { weighted_view_enabled: true } }, + format: :json + expect(tab.reload.gradebook_weight).to eq(50) + + patch :update, + params: { course_id: course.id, + settings_gradebook_component: { weighted_view_enabled: false } }, + format: :json + expect(tab.reload.gradebook_weight).to eq(50) + end + end + + context 'as teaching assistant' do + before { controller_sign_in(controller, teaching_assistant.user) } + + it 'is denied' do + expect do + patch :update, + params: { course_id: course.id, + settings_gradebook_component: { weighted_view_enabled: true } }, + format: :json + end.to raise_error(CanCan::AccessDenied) + end + end + end + end +end diff --git a/spec/controllers/course/gradebook_controller_spec.rb b/spec/controllers/course/gradebook_controller_spec.rb index 9ebac4a4ec..a213e80f47 100644 --- a/spec/controllers/course/gradebook_controller_spec.rb +++ b/spec/controllers/course/gradebook_controller_spec.rb @@ -142,7 +142,6 @@ end end - context 'with a graded submission where the answer grade is exactly 0' do let(:ta) { create(:course_teaching_assistant, course: course) } let(:tab) { course.assessment_categories.first.tabs.first } diff --git a/spec/models/course/assessment/tab_spec.rb b/spec/models/course/assessment/tab_spec.rb index b2454314dc..6eaea2f517 100644 --- a/spec/models/course/assessment/tab_spec.rb +++ b/spec/models/course/assessment/tab_spec.rb @@ -17,5 +17,41 @@ expect(weights.each_cons(2).all? { |a, b| a <= b }).to be_truthy end end + + describe 'gradebook_weight validation' do + let(:tab) { create(:course_assessment_tab) } + + it 'defaults to 0' do + expect(tab.gradebook_weight).to eq(0) + end + + it 'accepts 0..100 integers' do + [0, 50, 100].each do |w| + tab.gradebook_weight = w + expect(tab).to be_valid + end + end + + it 'rejects negative' do + tab.gradebook_weight = -1 + expect(tab).not_to be_valid + expect(tab.errors[:gradebook_weight]).to be_present + end + + it 'rejects >100' do + tab.gradebook_weight = 101 + expect(tab).not_to be_valid + end + + it 'rejects non-integer' do + tab.gradebook_weight = 50.5 + expect(tab).not_to be_valid + end + + it 'rejects nil' do + tab.gradebook_weight = nil + expect(tab).not_to be_valid + end + end end end diff --git a/spec/models/course/gradebook_ability_spec.rb b/spec/models/course/gradebook_ability_spec.rb new file mode 100644 index 0000000000..7247c2084c --- /dev/null +++ b/spec/models/course/gradebook_ability_spec.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true +require 'rails_helper' + +RSpec.describe Course::GradebookAbilityComponent do + let!(:instance) { Instance.default } + with_tenant(:instance) do + subject { Ability.new(user, course, course_user) } + let(:course) { create(:course) } + + context 'when the user is a Course Manager' do + let(:course_user) { create(:course_manager, course: course) } + let(:user) { course_user.user } + it { is_expected.to be_able_to(:read_gradebook, course) } + it { is_expected.to be_able_to(:manage_gradebook_weights, course) } + it { is_expected.to be_able_to(:manage_gradebook_settings, course) } + end + + context 'when the user is a Course Owner' do + let(:course_user) { create(:course_owner, course: course) } + let(:user) { course_user.user } + it { is_expected.to be_able_to(:read_gradebook, course) } + it { is_expected.to be_able_to(:manage_gradebook_weights, course) } + it { is_expected.to be_able_to(:manage_gradebook_settings, course) } + end + + context 'when the user is a Teaching Assistant' do + let(:course_user) { create(:course_teaching_assistant, course: course) } + let(:user) { course_user.user } + it { is_expected.to be_able_to(:read_gradebook, course) } + it { is_expected.not_to be_able_to(:manage_gradebook_weights, course) } + it { is_expected.not_to be_able_to(:manage_gradebook_settings, course) } + end + + context 'when the user is a Course Student' do + let(:course_user) { create(:course_student, course: course) } + let(:user) { course_user.user } + it { is_expected.not_to be_able_to(:read_gradebook, course) } + it { is_expected.not_to be_able_to(:manage_gradebook_weights, course) } + it { is_expected.not_to be_able_to(:manage_gradebook_settings, course) } + end + end +end diff --git a/spec/models/course/settings/gradebook_component_spec.rb b/spec/models/course/settings/gradebook_component_spec.rb new file mode 100644 index 0000000000..51e602521c --- /dev/null +++ b/spec/models/course/settings/gradebook_component_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true +require 'rails_helper' + +RSpec.describe Course::Settings::GradebookComponent do + let!(:instance) { Instance.default } + with_tenant(:instance) do + let(:course) { create(:course) } + let(:settings) do + context = OpenStruct.new(current_course: course, key: Course::GradebookComponent.key) + Course::Settings::GradebookComponent.new(context) + end + + describe '#weighted_view_enabled' do + it 'returns false by default' do + expect(settings.weighted_view_enabled).to eq(false) + end + end + + describe '#weighted_view_enabled=' do + it 'persists true when set to true' do + settings.weighted_view_enabled = true + course.save! + expect(settings.weighted_view_enabled).to eq(true) + end + + it 'persists false when set to false after being true' do + settings.weighted_view_enabled = true + course.save! + settings.weighted_view_enabled = false + course.save! + expect(settings.weighted_view_enabled).to eq(false) + end + + it 'handles string "1" as truthy' do + settings.weighted_view_enabled = '1' + expect(settings.weighted_view_enabled).to eq(true) + end + + it 'handles string "0" as falsy' do + settings.weighted_view_enabled = '0' + expect(settings.weighted_view_enabled).to eq(false) + end + end + end +end From 1cbe9e81118b739a326c85b727a8e440186d8515 Mon Sep 17 00:00:00 2001 From: lws49 Date: Wed, 20 May 2026 09:51:05 +0000 Subject: [PATCH 03/26] refactor(table): add columnPicker functionality with column picker dialog, toolbar buttons, visibility state, and tests - Add MuiColumnPickerDialog with Apply/Export actions, locked-column enforcement, and optional dataColumnIds hint when no data columns are selected - Update MuiTableToolbar to render a trigger button (opens dialog) and a direct export button alongside the existing CSV download icon - Extend useTanStackTableBuilder with column visibility state (localStorage persistence, dynamic reconciliation), onExportFromPicker, onDirectExport, and cross-page selection helpers (selectedCount, toggleAllFiltered, etc.) - Add ColumnPickerTemplate interface and Body.ts selection fields - Add full test coverage for dialog, toolbar, and hook behaviour - Add lib.components.table.* locale keys (en/ko/zh) --- .../MuiTableAdapter/MuiColumnPickerDialog.tsx | 152 ++++++++++ .../table/MuiTableAdapter/MuiTableToolbar.tsx | 5 +- .../TanStackTableBuilder/csvGenerator.ts | 7 +- .../useTanStackTableBuilder.tsx | 9 +- .../__tests__/MuiColumnPickerDialog.test.tsx | 269 ++++++++++++++++++ .../useTanStackTableBuilder.test.tsx | 10 +- .../lib/components/table/adapters/Toolbar.ts | 6 +- .../table/builder/ColumnPickerTemplate.ts | 9 +- 8 files changed, 458 insertions(+), 9 deletions(-) create mode 100644 client/app/lib/components/table/MuiTableAdapter/MuiColumnPickerDialog.tsx create mode 100644 client/app/lib/components/table/__tests__/MuiColumnPickerDialog.test.tsx diff --git a/client/app/lib/components/table/MuiTableAdapter/MuiColumnPickerDialog.tsx b/client/app/lib/components/table/MuiTableAdapter/MuiColumnPickerDialog.tsx new file mode 100644 index 0000000000..a228826569 --- /dev/null +++ b/client/app/lib/components/table/MuiTableAdapter/MuiColumnPickerDialog.tsx @@ -0,0 +1,152 @@ +import { useEffect, useState } from 'react'; +import { defineMessages } from 'react-intl'; +import { + Alert, + Button, + Dialog, + DialogActions, + DialogContent, + DialogTitle, +} from '@mui/material'; + +import useTranslation from 'lib/hooks/useTranslation'; + +import { ColumnPickerTemplate } from '../builder'; + +const translations = defineMessages({ + defaultTitle: { + id: 'lib.components.table.MuiColumnPickerDialog.defaultTitle', + defaultMessage: 'Select columns', + }, + apply: { + id: 'lib.components.table.MuiColumnPickerDialog.apply', + defaultMessage: 'Apply to view', + }, + cancel: { + id: 'lib.components.table.MuiColumnPickerDialog.cancel', + defaultMessage: 'Cancel', + }, + defaultExport: { + id: 'lib.components.table.MuiColumnPickerDialog.export', + defaultMessage: 'Apply and Export', + }, +}); + +interface MuiColumnPickerDialogProps { + open: boolean; + onClose: () => void; + initialVisibility: Record; + locked?: string[]; + columnPicker: ColumnPickerTemplate; + commitColumnVisibility: (next: Record) => void; + onExportFromPicker?: (visibility: Record) => void; +} + +const enforceLockedLocal = ( + next: Record, + locked: string[] | undefined, +): Record => { + if (!locked || locked.length === 0) return next; + const enforced = { ...next }; + locked.forEach((id) => { + enforced[id] = true; + }); + return enforced; +}; + +const MuiColumnPickerDialog = ({ + open, + onClose, + initialVisibility, + locked, + columnPicker, + commitColumnVisibility, + onExportFromPicker, +}: MuiColumnPickerDialogProps): JSX.Element => { + const { t } = useTranslation(); + const [staged, setStaged] = useState>(() => + enforceLockedLocal({ ...initialVisibility }, locked), + ); + + const dataColumnIds = columnPicker.dataColumnIds; + const hasDataColumns = + !dataColumnIds || + dataColumnIds.length === 0 || + dataColumnIds.some((id) => staged[id]); + + useEffect(() => { + if (open) { + setStaged(enforceLockedLocal({ ...initialVisibility }, locked)); + } + }, [open, initialVisibility, locked]); + + const ctx = { + isVisible: (id: string): boolean => staged[id] ?? false, + setVisible: (id: string, v: boolean): void => { + if (locked?.includes(id)) return; + setStaged((prev) => + Object.hasOwn(prev, id) ? { ...prev, [id]: v } : prev, + ); + }, + setManyVisible: (ids: string[], v: boolean): void => { + setStaged((prev) => { + const next = { ...prev }; + let changed = false; + ids.forEach((id) => { + if (!Object.hasOwn(next, id)) return; + if (locked?.includes(id)) return; + if (next[id] !== v) { + next[id] = v; + changed = true; + } + }); + return changed ? next : prev; + }); + }, + }; + + const commitAndClose = (): void => { + commitColumnVisibility(enforceLockedLocal(staged, locked)); + onClose(); + }; + + const cancelAndClose = (): void => { + onClose(); + }; + + const exportAndClose = (): void => { + const enforced = enforceLockedLocal(staged, locked); + commitColumnVisibility(enforced); + onExportFromPicker?.(enforced); + onClose(); + }; + + return ( + + + {columnPicker.dialogTitle ?? t(translations.defaultTitle)} + + {columnPicker.render(ctx)} + {!hasDataColumns && columnPicker.noDataColumnsHint && ( + + {columnPicker.noDataColumnsHint} + + )} + + + + + + + ); +}; + +export default MuiColumnPickerDialog; diff --git a/client/app/lib/components/table/MuiTableAdapter/MuiTableToolbar.tsx b/client/app/lib/components/table/MuiTableAdapter/MuiTableToolbar.tsx index 1e352fd26e..40e20b3c38 100644 --- a/client/app/lib/components/table/MuiTableAdapter/MuiTableToolbar.tsx +++ b/client/app/lib/components/table/MuiTableAdapter/MuiTableToolbar.tsx @@ -8,7 +8,7 @@ import useTranslation from 'lib/hooks/useTranslation'; import { ToolbarProps } from '../adapters'; -import MuiColumnPickerPrompt from './MuiColumnPickerPrompt'; +import MuiColumnPickerDialog from './MuiColumnPickerDialog'; import translations from './translations'; interface ToolbarContainerProps { @@ -102,12 +102,13 @@ const MuiTableToolbar = (props: ToolbarProps): JSX.Element | null => { {props.columnPicker && props.commitColumnVisibility && ( - setPickerOpen(false)} + onExportFromPicker={props.onExportFromPicker} open={pickerOpen} /> )} diff --git a/client/app/lib/components/table/TanStackTableBuilder/csvGenerator.ts b/client/app/lib/components/table/TanStackTableBuilder/csvGenerator.ts index 14e8f4189d..420122e9f2 100644 --- a/client/app/lib/components/table/TanStackTableBuilder/csvGenerator.ts +++ b/client/app/lib/components/table/TanStackTableBuilder/csvGenerator.ts @@ -7,6 +7,7 @@ import { ColumnTemplate, Data } from '../builder'; interface CsvGenerator { table: Table; getRealColumn: (id: string) => ColumnTemplate | undefined; + visibilityOverride?: Record; getExtraHeaderRows?: (columnIds: string[]) => string[][]; onlySelected?: boolean; } @@ -27,7 +28,11 @@ const generateCsv = ( // Keep ONLY columns where the consumer explicitly set csvDownloadable === true. // Columns with `csvDownloadable: undefined` or `false` are excluded (matches the // original behaviour where `csvDownloadable ?? false` gated headers). - const leafColumns = options.table.getVisibleLeafColumns(); + const leafColumns = options.visibilityOverride + ? options.table + .getAllLeafColumns() + .filter((col) => options.visibilityOverride?.[col.id] !== false) + : options.table.getVisibleLeafColumns(); const exportColumns = leafColumns.filter( (col) => options.getRealColumn(col.id)?.csvDownloadable === true, ); diff --git a/client/app/lib/components/table/TanStackTableBuilder/useTanStackTableBuilder.tsx b/client/app/lib/components/table/TanStackTableBuilder/useTanStackTableBuilder.tsx index 92d09d45d5..7137396ac6 100644 --- a/client/app/lib/components/table/TanStackTableBuilder/useTanStackTableBuilder.tsx +++ b/client/app/lib/components/table/TanStackTableBuilder/useTanStackTableBuilder.tsx @@ -225,10 +225,13 @@ const useTanStackTableBuilder = ( return getRealColumn(index); }; - const generateAndDownloadCsv = async (): Promise => { + const generateAndDownloadCsv = async ( + visibilityOverride?: Record, + ): Promise => { const csvData = await generateCsv({ table, getRealColumn: getRealColumnById, + visibilityOverride, getExtraHeaderRows: props.columnPicker?.getExtraHeaderRows, onlySelected: !isEmpty(rowSelection), }); @@ -355,6 +358,10 @@ const useTanStackTableBuilder = ( columnPicker: props.columnPicker, getColumnVisibility: () => columnVisibility, commitColumnVisibility: (next) => safeSetVisibility(() => next), + onExportFromPicker: + props.columnPicker && + ((visibility: Record): Promise => + generateAndDownloadCsv(visibility)), onDirectExport: props.columnPicker ? (): Promise => generateAndDownloadCsv() : undefined, diff --git a/client/app/lib/components/table/__tests__/MuiColumnPickerDialog.test.tsx b/client/app/lib/components/table/__tests__/MuiColumnPickerDialog.test.tsx new file mode 100644 index 0000000000..f1cac33842 --- /dev/null +++ b/client/app/lib/components/table/__tests__/MuiColumnPickerDialog.test.tsx @@ -0,0 +1,269 @@ +import { IntlProvider } from 'react-intl'; +import { fireEvent, render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; + +import { ColumnPickerRenderCtx } from '../builder'; +import MuiColumnPickerDialog from '../MuiTableAdapter/MuiColumnPickerDialog'; + +const DIALOG_TITLE = 'Select columns'; + +const wrap = (node: JSX.Element): JSX.Element => ( + + {node} + +); + +const makeRenderTree = (ids: readonly string[]): jest.Mock => + jest.fn((ctx: ColumnPickerRenderCtx) => ( + <> + {ids.map((id) => ( + + ))} + + )); + +const setup = ( + overrides: Partial> = {}, +): ReturnType & { + commitColumnVisibility: jest.Mock; + onExportFromPicker: jest.Mock; + renderTree: jest.Mock; + props: React.ComponentProps; +} => { + const commitColumnVisibility = jest.fn(); + const onExportFromPicker = jest.fn(); + const renderTree = makeRenderTree(['name', 'email']); + const props = { + open: true, + onClose: jest.fn(), + initialVisibility: { name: true, email: true }, + locked: ['name'], + columnPicker: { + renderTree, + dialogTitle: DIALOG_TITLE, + exportLabel: 'Export CSV', + onExport: 'csv' as const, + }, + commitColumnVisibility, + onExportFromPicker, + ...overrides, + }; + return { + ...render(wrap()), + commitColumnVisibility, + onExportFromPicker, + renderTree, + props, + }; +}; + +describe('MuiColumnPickerDialog', () => { + it('renders the dialog title', () => { + setup(); + expect(screen.getByText(DIALOG_TITLE)).toBeInTheDocument(); + }); + + it('Apply commits staged changes and closes', async () => { + const user = userEvent.setup(); + const { commitColumnVisibility, props } = setup(); + await user.click(screen.getByLabelText('email')); + await user.click(screen.getByRole('button', { name: /apply/i })); + + expect(commitColumnVisibility).toHaveBeenCalledWith({ + name: true, + email: false, + }); + expect(props.onClose).toHaveBeenCalled(); + }); + + it('Cancel discards staged and closes without commit', async () => { + const user = userEvent.setup(); + const { commitColumnVisibility, props } = setup(); + await user.click(screen.getByLabelText('email')); + await user.click(screen.getByRole('button', { name: /cancel/i })); + + expect(commitColumnVisibility).not.toHaveBeenCalled(); + expect(props.onClose).toHaveBeenCalled(); + }); + + it('Export CSV commits + invokes onExportFromPicker + closes', async () => { + const user = userEvent.setup(); + const { commitColumnVisibility, onExportFromPicker, props } = setup(); + await user.click(screen.getByLabelText('email')); + await user.click(screen.getByRole('button', { name: /export csv/i })); + + expect(commitColumnVisibility).toHaveBeenCalledWith({ + name: true, + email: false, + }); + expect(onExportFromPicker).toHaveBeenCalledWith({ + name: true, + email: false, + }); + expect(props.onClose).toHaveBeenCalled(); + }); + + it('locked id forcibly restored to true on commit even if staged false', async () => { + const user = userEvent.setup(); + const { commitColumnVisibility } = setup({ + initialVisibility: { name: false, email: true }, // malformed input + }); + + await user.click(screen.getByRole('button', { name: /apply/i })); + + expect(commitColumnVisibility).toHaveBeenCalledWith({ + name: true, + email: true, + }); + }); + + describe('locked column behavior', () => { + const makeGroupRenderTree = (ids: readonly string[]): jest.Mock => + jest.fn( + (ctx: ColumnPickerRenderCtx): JSX.Element => ( + <> + + + + ), + ); + + it('deselect-all leaves the locked column checked', async () => { + const user = userEvent.setup(); + const commitColumnVisibility = jest.fn(); + render( + wrap( + , + ), + ); + await user.click(screen.getByRole('button', { name: 'Deselect all' })); + await user.click(screen.getByRole('button', { name: /apply/i })); + expect(commitColumnVisibility).toHaveBeenCalledWith({ + name: true, + email: false, + }); + }); + + it('select-all from indeterminate state selects non-locked column', async () => { + const user = userEvent.setup(); + const commitColumnVisibility = jest.fn(); + render( + wrap( + , + ), + ); + await user.click(screen.getByRole('button', { name: 'Select all' })); + await user.click(screen.getByRole('button', { name: /apply/i })); + expect(commitColumnVisibility).toHaveBeenCalledWith({ + name: true, + email: true, + }); + }); + + it('clicking a locked column checkbox has no effect on its visibility', async () => { + const user = userEvent.setup(); + const { commitColumnVisibility } = setup(); + await user.click(screen.getByLabelText('name')); + await user.click(screen.getByRole('button', { name: /apply/i })); + expect(commitColumnVisibility).toHaveBeenCalledWith({ + name: true, + email: true, + }); + }); + }); + + it('Esc key dismisses without committing', () => { + const { commitColumnVisibility, props } = setup(); + fireEvent.keyDown(screen.getByRole('dialog'), { + key: 'Escape', + code: 'Escape', + }); + expect(props.onClose).toHaveBeenCalled(); + expect(commitColumnVisibility).not.toHaveBeenCalled(); + }); + + describe('noDataColumnsHint', () => { + const dataSetup = ( + dataColumnIds: string[], + initialVisibility: Record, + ): ReturnType => + setup({ + initialVisibility, + columnPicker: { + renderTree: makeRenderTree(['name', 'grade']), + dialogTitle: DIALOG_TITLE, + exportLabel: 'Export CSV', + onExport: 'csv' as const, + dataColumnIds, + noDataColumnsHint: 'No grade columns selected.', + }, + }); + + it('shows hint when no data columns are selected', () => { + dataSetup(['grade'], { name: true, grade: false }); + expect( + screen.getByText('No grade columns selected.'), + ).toBeInTheDocument(); + }); + + it('hides hint when at least one data column is selected', () => { + dataSetup(['grade'], { name: true, grade: true }); + expect( + screen.queryByText('No grade columns selected.'), + ).not.toBeInTheDocument(); + }); + + it('Export button is enabled even when no data columns are selected', () => { + dataSetup(['grade'], { name: true, grade: false }); + expect( + screen.getByRole('button', { name: /export csv/i }), + ).not.toBeDisabled(); + }); + + it('Apply button is enabled even when no data columns are selected', () => { + dataSetup(['grade'], { name: true, grade: false }); + expect(screen.getByRole('button', { name: /apply/i })).not.toBeDisabled(); + }); + }); +}); diff --git a/client/app/lib/components/table/__tests__/useTanStackTableBuilder.test.tsx b/client/app/lib/components/table/__tests__/useTanStackTableBuilder.test.tsx index 94f6031da0..2450c0d84e 100644 --- a/client/app/lib/components/table/__tests__/useTanStackTableBuilder.test.tsx +++ b/client/app/lib/components/table/__tests__/useTanStackTableBuilder.test.tsx @@ -318,7 +318,10 @@ describe('useTanStackTableBuilder CSV download', () => { ); await act(async () => { - await result.current.toolbar!.onDirectExport?.(); + await result.current.toolbar!.onExportFromPicker?.({ + name: true, + email: true, + }); }); expect(mockedDownloadFile).toHaveBeenCalledTimes(1); @@ -352,7 +355,10 @@ describe('useTanStackTableBuilder CSV download', () => { act(() => result.current.body.rows[0].toggleSelected()); await act(async () => { - await result.current.toolbar!.onDirectExport?.(); + await result.current.toolbar!.onExportFromPicker?.({ + name: true, + email: true, + }); }); expect(mockedDownloadFile).toHaveBeenCalledTimes(1); diff --git a/client/app/lib/components/table/adapters/Toolbar.ts b/client/app/lib/components/table/adapters/Toolbar.ts index 6ab16e4d49..2f9a49a23d 100644 --- a/client/app/lib/components/table/adapters/Toolbar.ts +++ b/client/app/lib/components/table/adapters/Toolbar.ts @@ -16,12 +16,14 @@ interface ToolbarProps { searchPlaceholder?: string; buttons?: ReactNode[]; - /** Set when consumer passes `columnPicker` on TableTemplate. Drives "Select Columns" button + dialog. */ + /** Set when consumer passes `columnPicker` on TableTemplate. Drives Export… button + dialog. */ columnPicker?: ColumnPickerTemplate; /** Read-side accessor — called by the dialog to seed staged state. */ getColumnVisibility?: () => Record; - /** Commit-side updater — called by the dialog on Apply. */ + /** Commit-side updater — called by the dialog on Apply / Export. */ commitColumnVisibility?: (next: Record) => void; + /** Called when the user clicks Export CSV in the dialog. Pre-bound to the CSV pipeline. */ + onExportFromPicker?: (visibility: Record) => void; /** Export with current visibility (no picker dialog). */ onDirectExport?: () => Promise; } diff --git a/client/app/lib/components/table/builder/ColumnPickerTemplate.ts b/client/app/lib/components/table/builder/ColumnPickerTemplate.ts index 9fc78e7692..fef62d3784 100644 --- a/client/app/lib/components/table/builder/ColumnPickerTemplate.ts +++ b/client/app/lib/components/table/builder/ColumnPickerTemplate.ts @@ -22,9 +22,16 @@ interface ColumnPickerTemplate { /** Tooltip shown on the direct-export button. */ directExportTooltip?: string; - /** Modal title, default "Select columns". */ + /** Modal title, default "Select columns to export". */ dialogTitle?: string; + /** Reuses the table's client-side CSV pipeline for the Export CSV button. */ + onExport?: 'csv'; + + /** CTA text inside the dialog, default "Apply and Export". */ + exportLabel?: string; + + /** * Called at CSV export time with the ordered visible column IDs. * Return one array per extra row to insert after the header row. From 71581425c5d920604f468b081691bb782f3a4998 Mon Sep 17 00:00:00 2001 From: lws49 Date: Wed, 20 May 2026 09:51:45 +0000 Subject: [PATCH 04/26] =?UTF-8?q?feat(gradebook):=20API=20=E2=80=94=20expo?= =?UTF-8?q?se=20tab=20weights=20+=20weight=20update=20endpoint?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - add Tab.update_gradebook_weights: transactional bulk update scoped to course tabs, with pre-flight ID validation and single pre-fetch query - extend GET /gradebook index JSON with weightedViewEnabled, canManageWeights, and per-tab gradebookWeight (gated by setting) - add PATCH /gradebook/weights: manager-only, returns 422 on validation failure or foreign tab, transactionally rolls back on any error --- .../course/gradebook_controller.rb | 16 ++ .../course/gradebook_ability_component.rb | 6 +- app/models/course/assessment/tab.rb | 24 ++ .../course/gradebook/index.json.jbuilder | 4 + .../MuiTableAdapter/MuiColumnPickerDialog.tsx | 152 ---------- .../table/MuiTableAdapter/MuiTableToolbar.tsx | 5 +- .../TanStackTableBuilder/csvGenerator.ts | 7 +- .../useTanStackTableBuilder.tsx | 9 +- .../__tests__/MuiColumnPickerDialog.test.tsx | 269 ------------------ .../useTanStackTableBuilder.test.tsx | 10 +- .../lib/components/table/adapters/Toolbar.ts | 6 +- .../table/builder/ColumnPickerTemplate.ts | 9 +- config/routes.rb | 1 + .../course/gradebook_controller_spec.rb | 143 ++++++++++ spec/models/course/assessment/tab_spec.rb | 41 +++ 15 files changed, 240 insertions(+), 462 deletions(-) delete mode 100644 client/app/lib/components/table/MuiTableAdapter/MuiColumnPickerDialog.tsx delete mode 100644 client/app/lib/components/table/__tests__/MuiColumnPickerDialog.test.tsx diff --git a/app/controllers/course/gradebook_controller.rb b/app/controllers/course/gradebook_controller.rb index fe739dc11e..c54fe2d6f7 100644 --- a/app/controllers/course/gradebook_controller.rb +++ b/app/controllers/course/gradebook_controller.rb @@ -5,6 +5,7 @@ class Course::GradebookController < Course::ComponentController def index respond_to do |format| format.json do + @weighted_view_enabled = @settings.weighted_view_enabled @published_assessments = fetch_published_assessments @categories, @tabs = fetch_categories_and_tabs @students = fetch_students @@ -18,12 +19,27 @@ def index end end + def update_weights + authorize! :manage_gradebook_weights, current_course + updates = update_weights_params[:weights].map do |entry| + { tab_id: entry[:tabId].to_i, weight: entry[:weight].to_i } + end + Course::Assessment::Tab.update_gradebook_weights(course: current_course, updates: updates) + render json: { weights: updates.map { |u| { tabId: u[:tab_id], weight: u[:weight] } } } + rescue ActiveRecord::RecordInvalid, ActiveRecord::RecordNotFound => e + render json: { errors: { base: e.message } }, status: :unprocessable_entity + end + private def authorize_read_gradebook! authorize! :read_gradebook, current_course end + def update_weights_params + params.permit(weights: [:tabId, :weight]) + end + def component current_component_host[:course_gradebook_component] end diff --git a/app/models/components/course/gradebook_ability_component.rb b/app/models/components/course/gradebook_ability_component.rb index e084be9509..d54a56cca6 100644 --- a/app/models/components/course/gradebook_ability_component.rb +++ b/app/models/components/course/gradebook_ability_component.rb @@ -4,10 +4,8 @@ module Course::GradebookAbilityComponent def define_permissions can :read_gradebook, Course, id: course.id if course_user&.staff? - if course_user&.manager_or_owner? - can :manage_gradebook_weights, Course, id: course.id # Reserved for weights-editing endpoint in follow-on PR - can :manage_gradebook_settings, Course, id: course.id - end + can :manage_gradebook_weights, Course, id: course.id if course_user&.manager_or_owner? + can :manage_gradebook_settings, Course, id: course.id if course_user&.manager_or_owner? super end end diff --git a/app/models/course/assessment/tab.rb b/app/models/course/assessment/tab.rb index 305bfc5e1c..4013558f34 100644 --- a/app/models/course/assessment/tab.rb +++ b/app/models/course/assessment/tab.rb @@ -29,6 +29,30 @@ class Course::Assessment::Tab < ApplicationRecord select('(array_agg(title))[0:3]') end) + # Bulk-updates the gradebook_weight for a set of tabs belonging to the given course. + # Raises ActiveRecord::RecordNotFound if any tab_id does not belong to the course. + # Raises ActiveRecord::RecordInvalid if any weight fails validation; the transaction is rolled back. + # + # @param course [Course] + # @param updates [Array] array of { tab_id: Integer, weight: Integer } + def self.update_gradebook_weights(course:, updates:) + course_tab_ids = course.assessment_tabs.pluck(:id).to_set + tab_ids_to_update = updates.map { |e| e[:tab_id] } + + tab_ids_to_update.each do |tab_id| + raise ActiveRecord::RecordNotFound unless course_tab_ids.include?(tab_id) + end + + tabs_by_id = where(id: tab_ids_to_update).index_by(&:id) + + transaction do + updates.each do |entry| + tab = tabs_by_id[entry[:tab_id]] + tab.update!(gradebook_weight: entry[:weight]) + end + end + end + # Returns a boolean value indicating if there are other tabs # besides this one remaining in its category. # diff --git a/app/views/course/gradebook/index.json.jbuilder b/app/views/course/gradebook/index.json.jbuilder index 510bc8be01..b92eba4551 100644 --- a/app/views/course/gradebook/index.json.jbuilder +++ b/app/views/course/gradebook/index.json.jbuilder @@ -1,4 +1,7 @@ # frozen_string_literal: true +json.weightedViewEnabled @weighted_view_enabled +json.canManageWeights can?(:manage_gradebook_weights, current_course) + json.categories @categories do |cat| json.id cat.id json.title cat.title @@ -8,6 +11,7 @@ json.tabs @tabs do |tab| json.id tab.id json.title tab.title json.categoryId tab.category_id + json.gradebookWeight tab.gradebook_weight if @weighted_view_enabled end json.assessments @published_assessments do |assessment| diff --git a/client/app/lib/components/table/MuiTableAdapter/MuiColumnPickerDialog.tsx b/client/app/lib/components/table/MuiTableAdapter/MuiColumnPickerDialog.tsx deleted file mode 100644 index a228826569..0000000000 --- a/client/app/lib/components/table/MuiTableAdapter/MuiColumnPickerDialog.tsx +++ /dev/null @@ -1,152 +0,0 @@ -import { useEffect, useState } from 'react'; -import { defineMessages } from 'react-intl'; -import { - Alert, - Button, - Dialog, - DialogActions, - DialogContent, - DialogTitle, -} from '@mui/material'; - -import useTranslation from 'lib/hooks/useTranslation'; - -import { ColumnPickerTemplate } from '../builder'; - -const translations = defineMessages({ - defaultTitle: { - id: 'lib.components.table.MuiColumnPickerDialog.defaultTitle', - defaultMessage: 'Select columns', - }, - apply: { - id: 'lib.components.table.MuiColumnPickerDialog.apply', - defaultMessage: 'Apply to view', - }, - cancel: { - id: 'lib.components.table.MuiColumnPickerDialog.cancel', - defaultMessage: 'Cancel', - }, - defaultExport: { - id: 'lib.components.table.MuiColumnPickerDialog.export', - defaultMessage: 'Apply and Export', - }, -}); - -interface MuiColumnPickerDialogProps { - open: boolean; - onClose: () => void; - initialVisibility: Record; - locked?: string[]; - columnPicker: ColumnPickerTemplate; - commitColumnVisibility: (next: Record) => void; - onExportFromPicker?: (visibility: Record) => void; -} - -const enforceLockedLocal = ( - next: Record, - locked: string[] | undefined, -): Record => { - if (!locked || locked.length === 0) return next; - const enforced = { ...next }; - locked.forEach((id) => { - enforced[id] = true; - }); - return enforced; -}; - -const MuiColumnPickerDialog = ({ - open, - onClose, - initialVisibility, - locked, - columnPicker, - commitColumnVisibility, - onExportFromPicker, -}: MuiColumnPickerDialogProps): JSX.Element => { - const { t } = useTranslation(); - const [staged, setStaged] = useState>(() => - enforceLockedLocal({ ...initialVisibility }, locked), - ); - - const dataColumnIds = columnPicker.dataColumnIds; - const hasDataColumns = - !dataColumnIds || - dataColumnIds.length === 0 || - dataColumnIds.some((id) => staged[id]); - - useEffect(() => { - if (open) { - setStaged(enforceLockedLocal({ ...initialVisibility }, locked)); - } - }, [open, initialVisibility, locked]); - - const ctx = { - isVisible: (id: string): boolean => staged[id] ?? false, - setVisible: (id: string, v: boolean): void => { - if (locked?.includes(id)) return; - setStaged((prev) => - Object.hasOwn(prev, id) ? { ...prev, [id]: v } : prev, - ); - }, - setManyVisible: (ids: string[], v: boolean): void => { - setStaged((prev) => { - const next = { ...prev }; - let changed = false; - ids.forEach((id) => { - if (!Object.hasOwn(next, id)) return; - if (locked?.includes(id)) return; - if (next[id] !== v) { - next[id] = v; - changed = true; - } - }); - return changed ? next : prev; - }); - }, - }; - - const commitAndClose = (): void => { - commitColumnVisibility(enforceLockedLocal(staged, locked)); - onClose(); - }; - - const cancelAndClose = (): void => { - onClose(); - }; - - const exportAndClose = (): void => { - const enforced = enforceLockedLocal(staged, locked); - commitColumnVisibility(enforced); - onExportFromPicker?.(enforced); - onClose(); - }; - - return ( - - - {columnPicker.dialogTitle ?? t(translations.defaultTitle)} - - {columnPicker.render(ctx)} - {!hasDataColumns && columnPicker.noDataColumnsHint && ( - - {columnPicker.noDataColumnsHint} - - )} - - - - - - - ); -}; - -export default MuiColumnPickerDialog; diff --git a/client/app/lib/components/table/MuiTableAdapter/MuiTableToolbar.tsx b/client/app/lib/components/table/MuiTableAdapter/MuiTableToolbar.tsx index 40e20b3c38..1e352fd26e 100644 --- a/client/app/lib/components/table/MuiTableAdapter/MuiTableToolbar.tsx +++ b/client/app/lib/components/table/MuiTableAdapter/MuiTableToolbar.tsx @@ -8,7 +8,7 @@ import useTranslation from 'lib/hooks/useTranslation'; import { ToolbarProps } from '../adapters'; -import MuiColumnPickerDialog from './MuiColumnPickerDialog'; +import MuiColumnPickerPrompt from './MuiColumnPickerPrompt'; import translations from './translations'; interface ToolbarContainerProps { @@ -102,13 +102,12 @@ const MuiTableToolbar = (props: ToolbarProps): JSX.Element | null => { {props.columnPicker && props.commitColumnVisibility && ( - setPickerOpen(false)} - onExportFromPicker={props.onExportFromPicker} open={pickerOpen} /> )} diff --git a/client/app/lib/components/table/TanStackTableBuilder/csvGenerator.ts b/client/app/lib/components/table/TanStackTableBuilder/csvGenerator.ts index 420122e9f2..14e8f4189d 100644 --- a/client/app/lib/components/table/TanStackTableBuilder/csvGenerator.ts +++ b/client/app/lib/components/table/TanStackTableBuilder/csvGenerator.ts @@ -7,7 +7,6 @@ import { ColumnTemplate, Data } from '../builder'; interface CsvGenerator { table: Table; getRealColumn: (id: string) => ColumnTemplate | undefined; - visibilityOverride?: Record; getExtraHeaderRows?: (columnIds: string[]) => string[][]; onlySelected?: boolean; } @@ -28,11 +27,7 @@ const generateCsv = ( // Keep ONLY columns where the consumer explicitly set csvDownloadable === true. // Columns with `csvDownloadable: undefined` or `false` are excluded (matches the // original behaviour where `csvDownloadable ?? false` gated headers). - const leafColumns = options.visibilityOverride - ? options.table - .getAllLeafColumns() - .filter((col) => options.visibilityOverride?.[col.id] !== false) - : options.table.getVisibleLeafColumns(); + const leafColumns = options.table.getVisibleLeafColumns(); const exportColumns = leafColumns.filter( (col) => options.getRealColumn(col.id)?.csvDownloadable === true, ); diff --git a/client/app/lib/components/table/TanStackTableBuilder/useTanStackTableBuilder.tsx b/client/app/lib/components/table/TanStackTableBuilder/useTanStackTableBuilder.tsx index 7137396ac6..92d09d45d5 100644 --- a/client/app/lib/components/table/TanStackTableBuilder/useTanStackTableBuilder.tsx +++ b/client/app/lib/components/table/TanStackTableBuilder/useTanStackTableBuilder.tsx @@ -225,13 +225,10 @@ const useTanStackTableBuilder = ( return getRealColumn(index); }; - const generateAndDownloadCsv = async ( - visibilityOverride?: Record, - ): Promise => { + const generateAndDownloadCsv = async (): Promise => { const csvData = await generateCsv({ table, getRealColumn: getRealColumnById, - visibilityOverride, getExtraHeaderRows: props.columnPicker?.getExtraHeaderRows, onlySelected: !isEmpty(rowSelection), }); @@ -358,10 +355,6 @@ const useTanStackTableBuilder = ( columnPicker: props.columnPicker, getColumnVisibility: () => columnVisibility, commitColumnVisibility: (next) => safeSetVisibility(() => next), - onExportFromPicker: - props.columnPicker && - ((visibility: Record): Promise => - generateAndDownloadCsv(visibility)), onDirectExport: props.columnPicker ? (): Promise => generateAndDownloadCsv() : undefined, diff --git a/client/app/lib/components/table/__tests__/MuiColumnPickerDialog.test.tsx b/client/app/lib/components/table/__tests__/MuiColumnPickerDialog.test.tsx deleted file mode 100644 index f1cac33842..0000000000 --- a/client/app/lib/components/table/__tests__/MuiColumnPickerDialog.test.tsx +++ /dev/null @@ -1,269 +0,0 @@ -import { IntlProvider } from 'react-intl'; -import { fireEvent, render, screen } from '@testing-library/react'; -import userEvent from '@testing-library/user-event'; - -import { ColumnPickerRenderCtx } from '../builder'; -import MuiColumnPickerDialog from '../MuiTableAdapter/MuiColumnPickerDialog'; - -const DIALOG_TITLE = 'Select columns'; - -const wrap = (node: JSX.Element): JSX.Element => ( - - {node} - -); - -const makeRenderTree = (ids: readonly string[]): jest.Mock => - jest.fn((ctx: ColumnPickerRenderCtx) => ( - <> - {ids.map((id) => ( - - ))} - - )); - -const setup = ( - overrides: Partial> = {}, -): ReturnType & { - commitColumnVisibility: jest.Mock; - onExportFromPicker: jest.Mock; - renderTree: jest.Mock; - props: React.ComponentProps; -} => { - const commitColumnVisibility = jest.fn(); - const onExportFromPicker = jest.fn(); - const renderTree = makeRenderTree(['name', 'email']); - const props = { - open: true, - onClose: jest.fn(), - initialVisibility: { name: true, email: true }, - locked: ['name'], - columnPicker: { - renderTree, - dialogTitle: DIALOG_TITLE, - exportLabel: 'Export CSV', - onExport: 'csv' as const, - }, - commitColumnVisibility, - onExportFromPicker, - ...overrides, - }; - return { - ...render(wrap()), - commitColumnVisibility, - onExportFromPicker, - renderTree, - props, - }; -}; - -describe('MuiColumnPickerDialog', () => { - it('renders the dialog title', () => { - setup(); - expect(screen.getByText(DIALOG_TITLE)).toBeInTheDocument(); - }); - - it('Apply commits staged changes and closes', async () => { - const user = userEvent.setup(); - const { commitColumnVisibility, props } = setup(); - await user.click(screen.getByLabelText('email')); - await user.click(screen.getByRole('button', { name: /apply/i })); - - expect(commitColumnVisibility).toHaveBeenCalledWith({ - name: true, - email: false, - }); - expect(props.onClose).toHaveBeenCalled(); - }); - - it('Cancel discards staged and closes without commit', async () => { - const user = userEvent.setup(); - const { commitColumnVisibility, props } = setup(); - await user.click(screen.getByLabelText('email')); - await user.click(screen.getByRole('button', { name: /cancel/i })); - - expect(commitColumnVisibility).not.toHaveBeenCalled(); - expect(props.onClose).toHaveBeenCalled(); - }); - - it('Export CSV commits + invokes onExportFromPicker + closes', async () => { - const user = userEvent.setup(); - const { commitColumnVisibility, onExportFromPicker, props } = setup(); - await user.click(screen.getByLabelText('email')); - await user.click(screen.getByRole('button', { name: /export csv/i })); - - expect(commitColumnVisibility).toHaveBeenCalledWith({ - name: true, - email: false, - }); - expect(onExportFromPicker).toHaveBeenCalledWith({ - name: true, - email: false, - }); - expect(props.onClose).toHaveBeenCalled(); - }); - - it('locked id forcibly restored to true on commit even if staged false', async () => { - const user = userEvent.setup(); - const { commitColumnVisibility } = setup({ - initialVisibility: { name: false, email: true }, // malformed input - }); - - await user.click(screen.getByRole('button', { name: /apply/i })); - - expect(commitColumnVisibility).toHaveBeenCalledWith({ - name: true, - email: true, - }); - }); - - describe('locked column behavior', () => { - const makeGroupRenderTree = (ids: readonly string[]): jest.Mock => - jest.fn( - (ctx: ColumnPickerRenderCtx): JSX.Element => ( - <> - - - - ), - ); - - it('deselect-all leaves the locked column checked', async () => { - const user = userEvent.setup(); - const commitColumnVisibility = jest.fn(); - render( - wrap( - , - ), - ); - await user.click(screen.getByRole('button', { name: 'Deselect all' })); - await user.click(screen.getByRole('button', { name: /apply/i })); - expect(commitColumnVisibility).toHaveBeenCalledWith({ - name: true, - email: false, - }); - }); - - it('select-all from indeterminate state selects non-locked column', async () => { - const user = userEvent.setup(); - const commitColumnVisibility = jest.fn(); - render( - wrap( - , - ), - ); - await user.click(screen.getByRole('button', { name: 'Select all' })); - await user.click(screen.getByRole('button', { name: /apply/i })); - expect(commitColumnVisibility).toHaveBeenCalledWith({ - name: true, - email: true, - }); - }); - - it('clicking a locked column checkbox has no effect on its visibility', async () => { - const user = userEvent.setup(); - const { commitColumnVisibility } = setup(); - await user.click(screen.getByLabelText('name')); - await user.click(screen.getByRole('button', { name: /apply/i })); - expect(commitColumnVisibility).toHaveBeenCalledWith({ - name: true, - email: true, - }); - }); - }); - - it('Esc key dismisses without committing', () => { - const { commitColumnVisibility, props } = setup(); - fireEvent.keyDown(screen.getByRole('dialog'), { - key: 'Escape', - code: 'Escape', - }); - expect(props.onClose).toHaveBeenCalled(); - expect(commitColumnVisibility).not.toHaveBeenCalled(); - }); - - describe('noDataColumnsHint', () => { - const dataSetup = ( - dataColumnIds: string[], - initialVisibility: Record, - ): ReturnType => - setup({ - initialVisibility, - columnPicker: { - renderTree: makeRenderTree(['name', 'grade']), - dialogTitle: DIALOG_TITLE, - exportLabel: 'Export CSV', - onExport: 'csv' as const, - dataColumnIds, - noDataColumnsHint: 'No grade columns selected.', - }, - }); - - it('shows hint when no data columns are selected', () => { - dataSetup(['grade'], { name: true, grade: false }); - expect( - screen.getByText('No grade columns selected.'), - ).toBeInTheDocument(); - }); - - it('hides hint when at least one data column is selected', () => { - dataSetup(['grade'], { name: true, grade: true }); - expect( - screen.queryByText('No grade columns selected.'), - ).not.toBeInTheDocument(); - }); - - it('Export button is enabled even when no data columns are selected', () => { - dataSetup(['grade'], { name: true, grade: false }); - expect( - screen.getByRole('button', { name: /export csv/i }), - ).not.toBeDisabled(); - }); - - it('Apply button is enabled even when no data columns are selected', () => { - dataSetup(['grade'], { name: true, grade: false }); - expect(screen.getByRole('button', { name: /apply/i })).not.toBeDisabled(); - }); - }); -}); diff --git a/client/app/lib/components/table/__tests__/useTanStackTableBuilder.test.tsx b/client/app/lib/components/table/__tests__/useTanStackTableBuilder.test.tsx index 2450c0d84e..94f6031da0 100644 --- a/client/app/lib/components/table/__tests__/useTanStackTableBuilder.test.tsx +++ b/client/app/lib/components/table/__tests__/useTanStackTableBuilder.test.tsx @@ -318,10 +318,7 @@ describe('useTanStackTableBuilder CSV download', () => { ); await act(async () => { - await result.current.toolbar!.onExportFromPicker?.({ - name: true, - email: true, - }); + await result.current.toolbar!.onDirectExport?.(); }); expect(mockedDownloadFile).toHaveBeenCalledTimes(1); @@ -355,10 +352,7 @@ describe('useTanStackTableBuilder CSV download', () => { act(() => result.current.body.rows[0].toggleSelected()); await act(async () => { - await result.current.toolbar!.onExportFromPicker?.({ - name: true, - email: true, - }); + await result.current.toolbar!.onDirectExport?.(); }); expect(mockedDownloadFile).toHaveBeenCalledTimes(1); diff --git a/client/app/lib/components/table/adapters/Toolbar.ts b/client/app/lib/components/table/adapters/Toolbar.ts index 2f9a49a23d..6ab16e4d49 100644 --- a/client/app/lib/components/table/adapters/Toolbar.ts +++ b/client/app/lib/components/table/adapters/Toolbar.ts @@ -16,14 +16,12 @@ interface ToolbarProps { searchPlaceholder?: string; buttons?: ReactNode[]; - /** Set when consumer passes `columnPicker` on TableTemplate. Drives Export… button + dialog. */ + /** Set when consumer passes `columnPicker` on TableTemplate. Drives "Select Columns" button + dialog. */ columnPicker?: ColumnPickerTemplate; /** Read-side accessor — called by the dialog to seed staged state. */ getColumnVisibility?: () => Record; - /** Commit-side updater — called by the dialog on Apply / Export. */ + /** Commit-side updater — called by the dialog on Apply. */ commitColumnVisibility?: (next: Record) => void; - /** Called when the user clicks Export CSV in the dialog. Pre-bound to the CSV pipeline. */ - onExportFromPicker?: (visibility: Record) => void; /** Export with current visibility (no picker dialog). */ onDirectExport?: () => Promise; } diff --git a/client/app/lib/components/table/builder/ColumnPickerTemplate.ts b/client/app/lib/components/table/builder/ColumnPickerTemplate.ts index fef62d3784..9fc78e7692 100644 --- a/client/app/lib/components/table/builder/ColumnPickerTemplate.ts +++ b/client/app/lib/components/table/builder/ColumnPickerTemplate.ts @@ -22,16 +22,9 @@ interface ColumnPickerTemplate { /** Tooltip shown on the direct-export button. */ directExportTooltip?: string; - /** Modal title, default "Select columns to export". */ + /** Modal title, default "Select columns". */ dialogTitle?: string; - /** Reuses the table's client-side CSV pipeline for the Export CSV button. */ - onExport?: 'csv'; - - /** CTA text inside the dialog, default "Apply and Export". */ - exportLabel?: string; - - /** * Called at CSV export time with the ordered visible column IDs. * Return one array per extra row to insert after the header row. diff --git a/config/routes.rb b/config/routes.rb index 0fc2d56263..0ff051754c 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -501,6 +501,7 @@ resource :gradebook, only: [] do get '/' => 'gradebook#index' + patch '/weights' => 'gradebook#update_weights' end scope module: :discussion do diff --git a/spec/controllers/course/gradebook_controller_spec.rb b/spec/controllers/course/gradebook_controller_spec.rb index a213e80f47..73b1014bbd 100644 --- a/spec/controllers/course/gradebook_controller_spec.rb +++ b/spec/controllers/course/gradebook_controller_spec.rb @@ -200,5 +200,148 @@ end end end + + describe 'PATCH update_weights' do + let(:manager) { create(:course_manager, course: course) } + let(:ta) { create(:course_teaching_assistant, course: course) } + let(:student) { create(:course_student, course: course) } + let(:category) { create(:course_assessment_category, course: course) } + let!(:tab1) { create(:course_assessment_tab, category: category) } + let!(:tab2) { create(:course_assessment_tab, category: category) } + + let(:valid_payload) do + { weights: [{ tabId: tab1.id, weight: 60 }, { tabId: tab2.id, weight: 40 }] } + end + + context 'as manager' do + before { controller_sign_in(controller, manager.user) } + + it 'updates and returns 200' do + patch :update_weights, params: { course_id: course.id, **valid_payload }, format: :json + expect(response).to have_http_status(:ok) + expect(tab1.reload.gradebook_weight).to eq(60) + expect(tab2.reload.gradebook_weight).to eq(40) + end + + it 'accepts sum < 100' do + patch :update_weights, + params: { course_id: course.id, weights: [tabId: tab1.id, weight: 30] }, + format: :json + expect(response).to have_http_status(:ok) + end + + it 'accepts sum > 100' do + patch :update_weights, + params: { course_id: course.id, + weights: [{ tabId: tab1.id, weight: 70 }, { tabId: tab2.id, weight: 70 }] }, + format: :json + expect(response).to have_http_status(:ok) + end + + it 'rejects negative with 422 and no partial write' do + tab1.update!(gradebook_weight: 10) + patch :update_weights, + params: { course_id: course.id, + weights: [{ tabId: tab1.id, weight: 50 }, { tabId: tab2.id, weight: -1 }] }, + format: :json + expect(response).to have_http_status(:unprocessable_entity) + expect(tab1.reload.gradebook_weight).to eq(10) + end + + it 'rejects >100 with 422' do + patch :update_weights, + params: { course_id: course.id, weights: [tabId: tab1.id, weight: 101] }, + format: :json + expect(response).to have_http_status(:unprocessable_entity) + end + + it 'rejects foreign tab id with 422' do + other_course = create(:course) + other_tab = create(:course_assessment_tab, + category: create(:course_assessment_category, course: other_course)) + patch :update_weights, + params: { course_id: course.id, weights: [tabId: other_tab.id, weight: 50] }, + format: :json + expect(response).to have_http_status(:unprocessable_entity) + end + end + + context 'as TA' do + before { controller_sign_in(controller, ta.user) } + it 'is denied' do + expect do + patch :update_weights, params: { course_id: course.id, **valid_payload }, format: :json + end.to raise_error(CanCan::AccessDenied) + end + end + + context 'as student' do + before { controller_sign_in(controller, student.user) } + it 'is denied' do + expect do + patch :update_weights, params: { course_id: course.id, **valid_payload }, format: :json + end.to raise_error(CanCan::AccessDenied) + end + end + + context 'when setting is disabled' do + before { controller_sign_in(controller, manager.user) } + + it 'still allows update (storage independent of display)' do + patch :update_weights, params: { course_id: course.id, **valid_payload }, format: :json + expect(response).to have_http_status(:ok) + expect(tab1.reload.gradebook_weight).to eq(60) + end + end + end + + describe 'GET index — weighted view fields' do + render_views + let(:manager) { create(:course_manager, course: course) } + let(:ta) { create(:course_teaching_assistant, course: course) } + let(:category) { create(:course_assessment_category, course: course) } + let!(:tab) { create(:course_assessment_tab, category: category, gradebook_weight: 30) } + let!(:assessment) do + create(:course_assessment_assessment, :published_with_mcq_question, + course: course, tab: tab) + end + + context 'when setting is disabled (default)' do + before { controller_sign_in(controller, manager.user) } + + it 'returns weightedViewEnabled false and omits gradebookWeight per tab' do + get :index, params: { course_id: course.id }, format: :json + body = JSON.parse(response.body) + expect(body['weightedViewEnabled']).to eq(false) + tab_json = body['tabs'].find { |t| t['id'] == tab.id } + expect(tab_json).not_to have_key('gradebookWeight') + end + end + + context 'when setting is enabled' do + before do + ctx = Struct.new(:current_course, :key).new(course, Course::GradebookComponent.key) + Course::Settings::GradebookComponent.new(ctx).weighted_view_enabled = true + course.save! + end + + it 'includes weightedViewEnabled true and gradebookWeight per tab for manager' do + controller_sign_in(controller, manager.user) + get :index, params: { course_id: course.id }, format: :json + body = JSON.parse(response.body) + expect(body['weightedViewEnabled']).to eq(true) + expect(body['canManageWeights']).to eq(true) + tab_json = body['tabs'].find { |t| t['id'] == tab.id } + expect(tab_json['gradebookWeight']).to eq(30) + end + + it 'returns canManageWeights false for TA' do + controller_sign_in(controller, ta.user) + get :index, params: { course_id: course.id }, format: :json + body = JSON.parse(response.body) + expect(body['canManageWeights']).to eq(false) + end + end + end end end diff --git a/spec/models/course/assessment/tab_spec.rb b/spec/models/course/assessment/tab_spec.rb index 6eaea2f517..b546671032 100644 --- a/spec/models/course/assessment/tab_spec.rb +++ b/spec/models/course/assessment/tab_spec.rb @@ -53,5 +53,46 @@ expect(tab).not_to be_valid end end + + describe '.update_gradebook_weights' do + let(:course) { create(:course) } + let(:category) { create(:course_assessment_category, course: course) } + let(:tab1) { create(:course_assessment_tab, category: category) } + let(:tab2) { create(:course_assessment_tab, category: category) } + + it 'updates given tabs' do + described_class.update_gradebook_weights( + course: course, + updates: [{ tab_id: tab1.id, weight: 60 }, { tab_id: tab2.id, weight: 40 }] + ) + expect(tab1.reload.gradebook_weight).to eq(60) + expect(tab2.reload.gradebook_weight).to eq(40) + end + + it 'is transactional — invalid value rolls back everything' do + tab1.update!(gradebook_weight: 10) + tab2.update!(gradebook_weight: 20) + expect do + described_class.update_gradebook_weights( + course: course, + updates: [{ tab_id: tab1.id, weight: 50 }, { tab_id: tab2.id, weight: 999 }] + ) + end.to raise_error(ActiveRecord::RecordInvalid) + expect(tab1.reload.gradebook_weight).to eq(10) + expect(tab2.reload.gradebook_weight).to eq(20) + end + + it 'rejects foreign tab_id' do + other_course = create(:course) + other_tab = create(:course_assessment_tab, + category: create(:course_assessment_category, course: other_course)) + expect do + described_class.update_gradebook_weights( + course: course, + updates: [tab_id: other_tab.id, weight: 50] + ) + end.to raise_error(ActiveRecord::RecordNotFound) + end + end end end From ae6002c9780b6bc3778562f7d9a8bf15c9f514c8 Mon Sep 17 00:00:00 2001 From: lws49 Date: Wed, 20 May 2026 09:51:05 +0000 Subject: [PATCH 05/26] refactor(table): add columnPicker functionality with column picker dialog, toolbar buttons, visibility state, and tests - Add MuiColumnPickerDialog with Apply/Export actions, locked-column enforcement, and optional dataColumnIds hint when no data columns are selected - Update MuiTableToolbar to render a trigger button (opens dialog) and a direct export button alongside the existing CSV download icon - Extend useTanStackTableBuilder with column visibility state (localStorage persistence, dynamic reconciliation), onExportFromPicker, onDirectExport, and cross-page selection helpers (selectedCount, toggleAllFiltered, etc.) - Add ColumnPickerTemplate interface and Body.ts selection fields - Add full test coverage for dialog, toolbar, and hook behaviour - Add lib.components.table.* locale keys (en/ko/zh) --- .../MuiTableAdapter/MuiColumnPickerDialog.tsx | 152 ++++++++++ .../__tests__/MuiColumnPickerDialog.test.tsx | 269 ++++++++++++++++++ 2 files changed, 421 insertions(+) create mode 100644 client/app/lib/components/table/MuiTableAdapter/MuiColumnPickerDialog.tsx create mode 100644 client/app/lib/components/table/__tests__/MuiColumnPickerDialog.test.tsx diff --git a/client/app/lib/components/table/MuiTableAdapter/MuiColumnPickerDialog.tsx b/client/app/lib/components/table/MuiTableAdapter/MuiColumnPickerDialog.tsx new file mode 100644 index 0000000000..388eed3b93 --- /dev/null +++ b/client/app/lib/components/table/MuiTableAdapter/MuiColumnPickerDialog.tsx @@ -0,0 +1,152 @@ +import { useEffect, useState } from 'react'; +import { defineMessages } from 'react-intl'; +import { + Alert, + Button, + Dialog, + DialogActions, + DialogContent, + DialogTitle, +} from '@mui/material'; + +import useTranslation from 'lib/hooks/useTranslation'; + +import { ColumnPickerTemplate } from '../builder'; + +const translations = defineMessages({ + defaultTitle: { + id: 'lib.components.table.MuiColumnPickerDialog.defaultTitle', + defaultMessage: 'Select columns', + }, + apply: { + id: 'lib.components.table.MuiColumnPickerDialog.apply', + defaultMessage: 'Apply to view', + }, + cancel: { + id: 'lib.components.table.MuiColumnPickerDialog.cancel', + defaultMessage: 'Cancel', + }, + defaultExport: { + id: 'lib.components.table.MuiColumnPickerDialog.export', + defaultMessage: 'Apply and Export', + }, +}); + +interface MuiColumnPickerDialogProps { + open: boolean; + onClose: () => void; + initialVisibility: Record; + locked?: string[]; + columnPicker: ColumnPickerTemplate; + commitColumnVisibility: (next: Record) => void; + onExportFromPicker?: (visibility: Record) => void; +} + +const enforceLockedLocal = ( + next: Record, + locked: string[] | undefined, +): Record => { + if (!locked || locked.length === 0) return next; + const enforced = { ...next }; + locked.forEach((id) => { + enforced[id] = true; + }); + return enforced; +}; + +const MuiColumnPickerDialog = ({ + open, + onClose, + initialVisibility, + locked, + columnPicker, + commitColumnVisibility, + onExportFromPicker, +}: MuiColumnPickerDialogProps): JSX.Element => { + const { t } = useTranslation(); + const [staged, setStaged] = useState>(() => + enforceLockedLocal({ ...initialVisibility }, locked), + ); + + const dataColumnIds = columnPicker.dataColumnIds; + const hasDataColumns = + !dataColumnIds || + dataColumnIds.length === 0 || + dataColumnIds.some((id) => staged[id]); + + useEffect(() => { + if (open) { + setStaged(enforceLockedLocal({ ...initialVisibility }, locked)); + } + }, [open, initialVisibility, locked]); + + const ctx = { + isVisible: (id: string): boolean => staged[id] ?? false, + setVisible: (id: string, v: boolean): void => { + if (locked?.includes(id)) return; + setStaged((prev) => + Object.hasOwn(prev, id) ? { ...prev, [id]: v } : prev, + ); + }, + setManyVisible: (ids: string[], v: boolean): void => { + setStaged((prev) => { + const next = { ...prev }; + let changed = false; + ids.forEach((id) => { + if (!Object.hasOwn(next, id)) return; + if (locked?.includes(id)) return; + if (next[id] !== v) { + next[id] = v; + changed = true; + } + }); + return changed ? next : prev; + }); + }, + }; + + const commitAndClose = (): void => { + commitColumnVisibility(enforceLockedLocal(staged, locked)); + onClose(); + }; + + const cancelAndClose = (): void => { + onClose(); + }; + + const exportAndClose = (): void => { + const enforced = enforceLockedLocal(staged, locked); + commitColumnVisibility(enforced); + onExportFromPicker?.(enforced); + onClose(); + }; + + return ( + + + {columnPicker.dialogTitle ?? t(translations.defaultTitle)} + + {columnPicker.renderTree(ctx)} + {!hasDataColumns && columnPicker.noDataColumnsHint && ( + + {columnPicker.noDataColumnsHint} + + )} + + + + + + + ); +}; + +export default MuiColumnPickerDialog; diff --git a/client/app/lib/components/table/__tests__/MuiColumnPickerDialog.test.tsx b/client/app/lib/components/table/__tests__/MuiColumnPickerDialog.test.tsx new file mode 100644 index 0000000000..f1cac33842 --- /dev/null +++ b/client/app/lib/components/table/__tests__/MuiColumnPickerDialog.test.tsx @@ -0,0 +1,269 @@ +import { IntlProvider } from 'react-intl'; +import { fireEvent, render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; + +import { ColumnPickerRenderCtx } from '../builder'; +import MuiColumnPickerDialog from '../MuiTableAdapter/MuiColumnPickerDialog'; + +const DIALOG_TITLE = 'Select columns'; + +const wrap = (node: JSX.Element): JSX.Element => ( + + {node} + +); + +const makeRenderTree = (ids: readonly string[]): jest.Mock => + jest.fn((ctx: ColumnPickerRenderCtx) => ( + <> + {ids.map((id) => ( + + ))} + + )); + +const setup = ( + overrides: Partial> = {}, +): ReturnType & { + commitColumnVisibility: jest.Mock; + onExportFromPicker: jest.Mock; + renderTree: jest.Mock; + props: React.ComponentProps; +} => { + const commitColumnVisibility = jest.fn(); + const onExportFromPicker = jest.fn(); + const renderTree = makeRenderTree(['name', 'email']); + const props = { + open: true, + onClose: jest.fn(), + initialVisibility: { name: true, email: true }, + locked: ['name'], + columnPicker: { + renderTree, + dialogTitle: DIALOG_TITLE, + exportLabel: 'Export CSV', + onExport: 'csv' as const, + }, + commitColumnVisibility, + onExportFromPicker, + ...overrides, + }; + return { + ...render(wrap()), + commitColumnVisibility, + onExportFromPicker, + renderTree, + props, + }; +}; + +describe('MuiColumnPickerDialog', () => { + it('renders the dialog title', () => { + setup(); + expect(screen.getByText(DIALOG_TITLE)).toBeInTheDocument(); + }); + + it('Apply commits staged changes and closes', async () => { + const user = userEvent.setup(); + const { commitColumnVisibility, props } = setup(); + await user.click(screen.getByLabelText('email')); + await user.click(screen.getByRole('button', { name: /apply/i })); + + expect(commitColumnVisibility).toHaveBeenCalledWith({ + name: true, + email: false, + }); + expect(props.onClose).toHaveBeenCalled(); + }); + + it('Cancel discards staged and closes without commit', async () => { + const user = userEvent.setup(); + const { commitColumnVisibility, props } = setup(); + await user.click(screen.getByLabelText('email')); + await user.click(screen.getByRole('button', { name: /cancel/i })); + + expect(commitColumnVisibility).not.toHaveBeenCalled(); + expect(props.onClose).toHaveBeenCalled(); + }); + + it('Export CSV commits + invokes onExportFromPicker + closes', async () => { + const user = userEvent.setup(); + const { commitColumnVisibility, onExportFromPicker, props } = setup(); + await user.click(screen.getByLabelText('email')); + await user.click(screen.getByRole('button', { name: /export csv/i })); + + expect(commitColumnVisibility).toHaveBeenCalledWith({ + name: true, + email: false, + }); + expect(onExportFromPicker).toHaveBeenCalledWith({ + name: true, + email: false, + }); + expect(props.onClose).toHaveBeenCalled(); + }); + + it('locked id forcibly restored to true on commit even if staged false', async () => { + const user = userEvent.setup(); + const { commitColumnVisibility } = setup({ + initialVisibility: { name: false, email: true }, // malformed input + }); + + await user.click(screen.getByRole('button', { name: /apply/i })); + + expect(commitColumnVisibility).toHaveBeenCalledWith({ + name: true, + email: true, + }); + }); + + describe('locked column behavior', () => { + const makeGroupRenderTree = (ids: readonly string[]): jest.Mock => + jest.fn( + (ctx: ColumnPickerRenderCtx): JSX.Element => ( + <> + + + + ), + ); + + it('deselect-all leaves the locked column checked', async () => { + const user = userEvent.setup(); + const commitColumnVisibility = jest.fn(); + render( + wrap( + , + ), + ); + await user.click(screen.getByRole('button', { name: 'Deselect all' })); + await user.click(screen.getByRole('button', { name: /apply/i })); + expect(commitColumnVisibility).toHaveBeenCalledWith({ + name: true, + email: false, + }); + }); + + it('select-all from indeterminate state selects non-locked column', async () => { + const user = userEvent.setup(); + const commitColumnVisibility = jest.fn(); + render( + wrap( + , + ), + ); + await user.click(screen.getByRole('button', { name: 'Select all' })); + await user.click(screen.getByRole('button', { name: /apply/i })); + expect(commitColumnVisibility).toHaveBeenCalledWith({ + name: true, + email: true, + }); + }); + + it('clicking a locked column checkbox has no effect on its visibility', async () => { + const user = userEvent.setup(); + const { commitColumnVisibility } = setup(); + await user.click(screen.getByLabelText('name')); + await user.click(screen.getByRole('button', { name: /apply/i })); + expect(commitColumnVisibility).toHaveBeenCalledWith({ + name: true, + email: true, + }); + }); + }); + + it('Esc key dismisses without committing', () => { + const { commitColumnVisibility, props } = setup(); + fireEvent.keyDown(screen.getByRole('dialog'), { + key: 'Escape', + code: 'Escape', + }); + expect(props.onClose).toHaveBeenCalled(); + expect(commitColumnVisibility).not.toHaveBeenCalled(); + }); + + describe('noDataColumnsHint', () => { + const dataSetup = ( + dataColumnIds: string[], + initialVisibility: Record, + ): ReturnType => + setup({ + initialVisibility, + columnPicker: { + renderTree: makeRenderTree(['name', 'grade']), + dialogTitle: DIALOG_TITLE, + exportLabel: 'Export CSV', + onExport: 'csv' as const, + dataColumnIds, + noDataColumnsHint: 'No grade columns selected.', + }, + }); + + it('shows hint when no data columns are selected', () => { + dataSetup(['grade'], { name: true, grade: false }); + expect( + screen.getByText('No grade columns selected.'), + ).toBeInTheDocument(); + }); + + it('hides hint when at least one data column is selected', () => { + dataSetup(['grade'], { name: true, grade: true }); + expect( + screen.queryByText('No grade columns selected.'), + ).not.toBeInTheDocument(); + }); + + it('Export button is enabled even when no data columns are selected', () => { + dataSetup(['grade'], { name: true, grade: false }); + expect( + screen.getByRole('button', { name: /export csv/i }), + ).not.toBeDisabled(); + }); + + it('Apply button is enabled even when no data columns are selected', () => { + dataSetup(['grade'], { name: true, grade: false }); + expect(screen.getByRole('button', { name: /apply/i })).not.toBeDisabled(); + }); + }); +}); From 68fb6c6470c755bc7c870efe44b1134649361c24 Mon Sep 17 00:00:00 2001 From: lws49 Date: Wed, 20 May 2026 09:51:45 +0000 Subject: [PATCH 06/26] feat(gradebook): add weighted view settings foundation - Add gradebook_weight (0-100 integer) column to course_assessment_tabs - Add Course::Settings::GradebookComponent with weighted_view_enabled - Add manage_gradebook_weights/settings abilities (manager/owner only) - Add Course Admin -> Gradebook settings page to toggle the setting --- .../components/course/gradebook_component.rb | 20 +-- .../course/gradebook_controller.rb | 18 +-- .../course/gradebook/index.json.jbuilder | 4 - .../course/gradebook_controller_spec.rb | 143 ------------------ spec/models/course/assessment/tab_spec.rb | 41 ----- 5 files changed, 5 insertions(+), 221 deletions(-) diff --git a/app/controllers/components/course/gradebook_component.rb b/app/controllers/components/course/gradebook_component.rb index 7e4dc19912..fe2ccfe09e 100644 --- a/app/controllers/components/course/gradebook_component.rb +++ b/app/controllers/components/course/gradebook_component.rb @@ -7,10 +7,10 @@ def self.display_name end def sidebar_items - items = [] + return [] unless can?(:read_gradebook, current_course) - if can?(:read_gradebook, current_course) - items << { + [ + { key: self.class.key, icon: :gradebook, title: I18n.t('course.gradebook.component.sidebar_title'), @@ -18,18 +18,6 @@ def sidebar_items weight: 9, path: course_gradebook_path(current_course) } - end - - if can?(:manage_gradebook_settings, current_course) - items << { - key: self.class.key, - title: I18n.t('course.gradebook.component.sidebar_title'), - type: :settings, - weight: 9, - path: course_admin_gradebook_path(current_course) - } - end - - items + ] end end diff --git a/app/controllers/course/gradebook_controller.rb b/app/controllers/course/gradebook_controller.rb index c54fe2d6f7..71cfa4fc14 100644 --- a/app/controllers/course/gradebook_controller.rb +++ b/app/controllers/course/gradebook_controller.rb @@ -5,7 +5,6 @@ class Course::GradebookController < Course::ComponentController def index respond_to do |format| format.json do - @weighted_view_enabled = @settings.weighted_view_enabled @published_assessments = fetch_published_assessments @categories, @tabs = fetch_categories_and_tabs @students = fetch_students @@ -19,27 +18,12 @@ def index end end - def update_weights - authorize! :manage_gradebook_weights, current_course - updates = update_weights_params[:weights].map do |entry| - { tab_id: entry[:tabId].to_i, weight: entry[:weight].to_i } - end - Course::Assessment::Tab.update_gradebook_weights(course: current_course, updates: updates) - render json: { weights: updates.map { |u| { tabId: u[:tab_id], weight: u[:weight] } } } - rescue ActiveRecord::RecordInvalid, ActiveRecord::RecordNotFound => e - render json: { errors: { base: e.message } }, status: :unprocessable_entity - end - private def authorize_read_gradebook! authorize! :read_gradebook, current_course end - def update_weights_params - params.permit(weights: [:tabId, :weight]) - end - def component current_component_host[:course_gradebook_component] end @@ -50,7 +34,7 @@ def fetch_categories_and_tabs end def fetch_students - current_course.levels.to_a # Warms the AR association cache to prevent N+1 in level_number + current_course.levels.to_a current_course.course_users.students.without_phantom_users. calculated(:experience_points).includes(:user).to_a end diff --git a/app/views/course/gradebook/index.json.jbuilder b/app/views/course/gradebook/index.json.jbuilder index b92eba4551..510bc8be01 100644 --- a/app/views/course/gradebook/index.json.jbuilder +++ b/app/views/course/gradebook/index.json.jbuilder @@ -1,7 +1,4 @@ # frozen_string_literal: true -json.weightedViewEnabled @weighted_view_enabled -json.canManageWeights can?(:manage_gradebook_weights, current_course) - json.categories @categories do |cat| json.id cat.id json.title cat.title @@ -11,7 +8,6 @@ json.tabs @tabs do |tab| json.id tab.id json.title tab.title json.categoryId tab.category_id - json.gradebookWeight tab.gradebook_weight if @weighted_view_enabled end json.assessments @published_assessments do |assessment| diff --git a/spec/controllers/course/gradebook_controller_spec.rb b/spec/controllers/course/gradebook_controller_spec.rb index 73b1014bbd..a213e80f47 100644 --- a/spec/controllers/course/gradebook_controller_spec.rb +++ b/spec/controllers/course/gradebook_controller_spec.rb @@ -200,148 +200,5 @@ end end end - - describe 'PATCH update_weights' do - let(:manager) { create(:course_manager, course: course) } - let(:ta) { create(:course_teaching_assistant, course: course) } - let(:student) { create(:course_student, course: course) } - let(:category) { create(:course_assessment_category, course: course) } - let!(:tab1) { create(:course_assessment_tab, category: category) } - let!(:tab2) { create(:course_assessment_tab, category: category) } - - let(:valid_payload) do - { weights: [{ tabId: tab1.id, weight: 60 }, { tabId: tab2.id, weight: 40 }] } - end - - context 'as manager' do - before { controller_sign_in(controller, manager.user) } - - it 'updates and returns 200' do - patch :update_weights, params: { course_id: course.id, **valid_payload }, format: :json - expect(response).to have_http_status(:ok) - expect(tab1.reload.gradebook_weight).to eq(60) - expect(tab2.reload.gradebook_weight).to eq(40) - end - - it 'accepts sum < 100' do - patch :update_weights, - params: { course_id: course.id, weights: [tabId: tab1.id, weight: 30] }, - format: :json - expect(response).to have_http_status(:ok) - end - - it 'accepts sum > 100' do - patch :update_weights, - params: { course_id: course.id, - weights: [{ tabId: tab1.id, weight: 70 }, { tabId: tab2.id, weight: 70 }] }, - format: :json - expect(response).to have_http_status(:ok) - end - - it 'rejects negative with 422 and no partial write' do - tab1.update!(gradebook_weight: 10) - patch :update_weights, - params: { course_id: course.id, - weights: [{ tabId: tab1.id, weight: 50 }, { tabId: tab2.id, weight: -1 }] }, - format: :json - expect(response).to have_http_status(:unprocessable_entity) - expect(tab1.reload.gradebook_weight).to eq(10) - end - - it 'rejects >100 with 422' do - patch :update_weights, - params: { course_id: course.id, weights: [tabId: tab1.id, weight: 101] }, - format: :json - expect(response).to have_http_status(:unprocessable_entity) - end - - it 'rejects foreign tab id with 422' do - other_course = create(:course) - other_tab = create(:course_assessment_tab, - category: create(:course_assessment_category, course: other_course)) - patch :update_weights, - params: { course_id: course.id, weights: [tabId: other_tab.id, weight: 50] }, - format: :json - expect(response).to have_http_status(:unprocessable_entity) - end - end - - context 'as TA' do - before { controller_sign_in(controller, ta.user) } - it 'is denied' do - expect do - patch :update_weights, params: { course_id: course.id, **valid_payload }, format: :json - end.to raise_error(CanCan::AccessDenied) - end - end - - context 'as student' do - before { controller_sign_in(controller, student.user) } - it 'is denied' do - expect do - patch :update_weights, params: { course_id: course.id, **valid_payload }, format: :json - end.to raise_error(CanCan::AccessDenied) - end - end - - context 'when setting is disabled' do - before { controller_sign_in(controller, manager.user) } - - it 'still allows update (storage independent of display)' do - patch :update_weights, params: { course_id: course.id, **valid_payload }, format: :json - expect(response).to have_http_status(:ok) - expect(tab1.reload.gradebook_weight).to eq(60) - end - end - end - - describe 'GET index — weighted view fields' do - render_views - let(:manager) { create(:course_manager, course: course) } - let(:ta) { create(:course_teaching_assistant, course: course) } - let(:category) { create(:course_assessment_category, course: course) } - let!(:tab) { create(:course_assessment_tab, category: category, gradebook_weight: 30) } - let!(:assessment) do - create(:course_assessment_assessment, :published_with_mcq_question, - course: course, tab: tab) - end - - context 'when setting is disabled (default)' do - before { controller_sign_in(controller, manager.user) } - - it 'returns weightedViewEnabled false and omits gradebookWeight per tab' do - get :index, params: { course_id: course.id }, format: :json - body = JSON.parse(response.body) - expect(body['weightedViewEnabled']).to eq(false) - tab_json = body['tabs'].find { |t| t['id'] == tab.id } - expect(tab_json).not_to have_key('gradebookWeight') - end - end - - context 'when setting is enabled' do - before do - ctx = Struct.new(:current_course, :key).new(course, Course::GradebookComponent.key) - Course::Settings::GradebookComponent.new(ctx).weighted_view_enabled = true - course.save! - end - - it 'includes weightedViewEnabled true and gradebookWeight per tab for manager' do - controller_sign_in(controller, manager.user) - get :index, params: { course_id: course.id }, format: :json - body = JSON.parse(response.body) - expect(body['weightedViewEnabled']).to eq(true) - expect(body['canManageWeights']).to eq(true) - tab_json = body['tabs'].find { |t| t['id'] == tab.id } - expect(tab_json['gradebookWeight']).to eq(30) - end - - it 'returns canManageWeights false for TA' do - controller_sign_in(controller, ta.user) - get :index, params: { course_id: course.id }, format: :json - body = JSON.parse(response.body) - expect(body['canManageWeights']).to eq(false) - end - end - end end end diff --git a/spec/models/course/assessment/tab_spec.rb b/spec/models/course/assessment/tab_spec.rb index b546671032..6eaea2f517 100644 --- a/spec/models/course/assessment/tab_spec.rb +++ b/spec/models/course/assessment/tab_spec.rb @@ -53,46 +53,5 @@ expect(tab).not_to be_valid end end - - describe '.update_gradebook_weights' do - let(:course) { create(:course) } - let(:category) { create(:course_assessment_category, course: course) } - let(:tab1) { create(:course_assessment_tab, category: category) } - let(:tab2) { create(:course_assessment_tab, category: category) } - - it 'updates given tabs' do - described_class.update_gradebook_weights( - course: course, - updates: [{ tab_id: tab1.id, weight: 60 }, { tab_id: tab2.id, weight: 40 }] - ) - expect(tab1.reload.gradebook_weight).to eq(60) - expect(tab2.reload.gradebook_weight).to eq(40) - end - - it 'is transactional — invalid value rolls back everything' do - tab1.update!(gradebook_weight: 10) - tab2.update!(gradebook_weight: 20) - expect do - described_class.update_gradebook_weights( - course: course, - updates: [{ tab_id: tab1.id, weight: 50 }, { tab_id: tab2.id, weight: 999 }] - ) - end.to raise_error(ActiveRecord::RecordInvalid) - expect(tab1.reload.gradebook_weight).to eq(10) - expect(tab2.reload.gradebook_weight).to eq(20) - end - - it 'rejects foreign tab_id' do - other_course = create(:course) - other_tab = create(:course_assessment_tab, - category: create(:course_assessment_category, course: other_course)) - expect do - described_class.update_gradebook_weights( - course: course, - updates: [tab_id: other_tab.id, weight: 50] - ) - end.to raise_error(ActiveRecord::RecordNotFound) - end - end end end From 8c06e66638ae90cebb88d9f792eb97cc8cd6553d Mon Sep 17 00:00:00 2001 From: lws49 Date: Thu, 28 May 2026 17:34:01 +0800 Subject: [PATCH 07/26] =?UTF-8?q?feat(gradebook):=20API=20=E2=80=94=20expo?= =?UTF-8?q?se=20tab=20weights=20+=20weight=20update=20endpoint?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - add Tab.update_gradebook_weights: transactional bulk update scoped to course tabs, with pre-flight ID validation and single pre-fetch query - extend GET /gradebook index JSON with weightedViewEnabled, canManageWeights, and per-tab gradebookWeight (gated by setting) - add PATCH /gradebook/weights: manager-only, returns 422 on validation failure or foreign tab, transactionally rolls back on any error --- .../course/gradebook_controller.rb | 16 ++ .../course/gradebook/index.json.jbuilder | 4 + .../course/gradebook_controller_spec.rb | 143 ++++++++++++++++++ spec/models/course/assessment/tab_spec.rb | 41 +++++ 4 files changed, 204 insertions(+) diff --git a/app/controllers/course/gradebook_controller.rb b/app/controllers/course/gradebook_controller.rb index 71cfa4fc14..198be93d61 100644 --- a/app/controllers/course/gradebook_controller.rb +++ b/app/controllers/course/gradebook_controller.rb @@ -5,6 +5,7 @@ class Course::GradebookController < Course::ComponentController def index respond_to do |format| format.json do + @weighted_view_enabled = @settings.weighted_view_enabled @published_assessments = fetch_published_assessments @categories, @tabs = fetch_categories_and_tabs @students = fetch_students @@ -18,12 +19,27 @@ def index end end + def update_weights + authorize! :manage_gradebook_weights, current_course + updates = update_weights_params[:weights].map do |entry| + { tab_id: entry[:tabId].to_i, weight: entry[:weight].to_i } + end + Course::Assessment::Tab.update_gradebook_weights(course: current_course, updates: updates) + render json: { weights: updates.map { |u| { tabId: u[:tab_id], weight: u[:weight] } } } + rescue ActiveRecord::RecordInvalid, ActiveRecord::RecordNotFound => e + render json: { errors: { base: e.message } }, status: :unprocessable_entity + end + private def authorize_read_gradebook! authorize! :read_gradebook, current_course end + def update_weights_params + params.permit(weights: [:tabId, :weight]) + end + def component current_component_host[:course_gradebook_component] end diff --git a/app/views/course/gradebook/index.json.jbuilder b/app/views/course/gradebook/index.json.jbuilder index 510bc8be01..b92eba4551 100644 --- a/app/views/course/gradebook/index.json.jbuilder +++ b/app/views/course/gradebook/index.json.jbuilder @@ -1,4 +1,7 @@ # frozen_string_literal: true +json.weightedViewEnabled @weighted_view_enabled +json.canManageWeights can?(:manage_gradebook_weights, current_course) + json.categories @categories do |cat| json.id cat.id json.title cat.title @@ -8,6 +11,7 @@ json.tabs @tabs do |tab| json.id tab.id json.title tab.title json.categoryId tab.category_id + json.gradebookWeight tab.gradebook_weight if @weighted_view_enabled end json.assessments @published_assessments do |assessment| diff --git a/spec/controllers/course/gradebook_controller_spec.rb b/spec/controllers/course/gradebook_controller_spec.rb index a213e80f47..73b1014bbd 100644 --- a/spec/controllers/course/gradebook_controller_spec.rb +++ b/spec/controllers/course/gradebook_controller_spec.rb @@ -200,5 +200,148 @@ end end end + + describe 'PATCH update_weights' do + let(:manager) { create(:course_manager, course: course) } + let(:ta) { create(:course_teaching_assistant, course: course) } + let(:student) { create(:course_student, course: course) } + let(:category) { create(:course_assessment_category, course: course) } + let!(:tab1) { create(:course_assessment_tab, category: category) } + let!(:tab2) { create(:course_assessment_tab, category: category) } + + let(:valid_payload) do + { weights: [{ tabId: tab1.id, weight: 60 }, { tabId: tab2.id, weight: 40 }] } + end + + context 'as manager' do + before { controller_sign_in(controller, manager.user) } + + it 'updates and returns 200' do + patch :update_weights, params: { course_id: course.id, **valid_payload }, format: :json + expect(response).to have_http_status(:ok) + expect(tab1.reload.gradebook_weight).to eq(60) + expect(tab2.reload.gradebook_weight).to eq(40) + end + + it 'accepts sum < 100' do + patch :update_weights, + params: { course_id: course.id, weights: [tabId: tab1.id, weight: 30] }, + format: :json + expect(response).to have_http_status(:ok) + end + + it 'accepts sum > 100' do + patch :update_weights, + params: { course_id: course.id, + weights: [{ tabId: tab1.id, weight: 70 }, { tabId: tab2.id, weight: 70 }] }, + format: :json + expect(response).to have_http_status(:ok) + end + + it 'rejects negative with 422 and no partial write' do + tab1.update!(gradebook_weight: 10) + patch :update_weights, + params: { course_id: course.id, + weights: [{ tabId: tab1.id, weight: 50 }, { tabId: tab2.id, weight: -1 }] }, + format: :json + expect(response).to have_http_status(:unprocessable_entity) + expect(tab1.reload.gradebook_weight).to eq(10) + end + + it 'rejects >100 with 422' do + patch :update_weights, + params: { course_id: course.id, weights: [tabId: tab1.id, weight: 101] }, + format: :json + expect(response).to have_http_status(:unprocessable_entity) + end + + it 'rejects foreign tab id with 422' do + other_course = create(:course) + other_tab = create(:course_assessment_tab, + category: create(:course_assessment_category, course: other_course)) + patch :update_weights, + params: { course_id: course.id, weights: [tabId: other_tab.id, weight: 50] }, + format: :json + expect(response).to have_http_status(:unprocessable_entity) + end + end + + context 'as TA' do + before { controller_sign_in(controller, ta.user) } + it 'is denied' do + expect do + patch :update_weights, params: { course_id: course.id, **valid_payload }, format: :json + end.to raise_error(CanCan::AccessDenied) + end + end + + context 'as student' do + before { controller_sign_in(controller, student.user) } + it 'is denied' do + expect do + patch :update_weights, params: { course_id: course.id, **valid_payload }, format: :json + end.to raise_error(CanCan::AccessDenied) + end + end + + context 'when setting is disabled' do + before { controller_sign_in(controller, manager.user) } + + it 'still allows update (storage independent of display)' do + patch :update_weights, params: { course_id: course.id, **valid_payload }, format: :json + expect(response).to have_http_status(:ok) + expect(tab1.reload.gradebook_weight).to eq(60) + end + end + end + + describe 'GET index — weighted view fields' do + render_views + let(:manager) { create(:course_manager, course: course) } + let(:ta) { create(:course_teaching_assistant, course: course) } + let(:category) { create(:course_assessment_category, course: course) } + let!(:tab) { create(:course_assessment_tab, category: category, gradebook_weight: 30) } + let!(:assessment) do + create(:course_assessment_assessment, :published_with_mcq_question, + course: course, tab: tab) + end + + context 'when setting is disabled (default)' do + before { controller_sign_in(controller, manager.user) } + + it 'returns weightedViewEnabled false and omits gradebookWeight per tab' do + get :index, params: { course_id: course.id }, format: :json + body = JSON.parse(response.body) + expect(body['weightedViewEnabled']).to eq(false) + tab_json = body['tabs'].find { |t| t['id'] == tab.id } + expect(tab_json).not_to have_key('gradebookWeight') + end + end + + context 'when setting is enabled' do + before do + ctx = Struct.new(:current_course, :key).new(course, Course::GradebookComponent.key) + Course::Settings::GradebookComponent.new(ctx).weighted_view_enabled = true + course.save! + end + + it 'includes weightedViewEnabled true and gradebookWeight per tab for manager' do + controller_sign_in(controller, manager.user) + get :index, params: { course_id: course.id }, format: :json + body = JSON.parse(response.body) + expect(body['weightedViewEnabled']).to eq(true) + expect(body['canManageWeights']).to eq(true) + tab_json = body['tabs'].find { |t| t['id'] == tab.id } + expect(tab_json['gradebookWeight']).to eq(30) + end + + it 'returns canManageWeights false for TA' do + controller_sign_in(controller, ta.user) + get :index, params: { course_id: course.id }, format: :json + body = JSON.parse(response.body) + expect(body['canManageWeights']).to eq(false) + end + end + end end end diff --git a/spec/models/course/assessment/tab_spec.rb b/spec/models/course/assessment/tab_spec.rb index 6eaea2f517..b546671032 100644 --- a/spec/models/course/assessment/tab_spec.rb +++ b/spec/models/course/assessment/tab_spec.rb @@ -53,5 +53,46 @@ expect(tab).not_to be_valid end end + + describe '.update_gradebook_weights' do + let(:course) { create(:course) } + let(:category) { create(:course_assessment_category, course: course) } + let(:tab1) { create(:course_assessment_tab, category: category) } + let(:tab2) { create(:course_assessment_tab, category: category) } + + it 'updates given tabs' do + described_class.update_gradebook_weights( + course: course, + updates: [{ tab_id: tab1.id, weight: 60 }, { tab_id: tab2.id, weight: 40 }] + ) + expect(tab1.reload.gradebook_weight).to eq(60) + expect(tab2.reload.gradebook_weight).to eq(40) + end + + it 'is transactional — invalid value rolls back everything' do + tab1.update!(gradebook_weight: 10) + tab2.update!(gradebook_weight: 20) + expect do + described_class.update_gradebook_weights( + course: course, + updates: [{ tab_id: tab1.id, weight: 50 }, { tab_id: tab2.id, weight: 999 }] + ) + end.to raise_error(ActiveRecord::RecordInvalid) + expect(tab1.reload.gradebook_weight).to eq(10) + expect(tab2.reload.gradebook_weight).to eq(20) + end + + it 'rejects foreign tab_id' do + other_course = create(:course) + other_tab = create(:course_assessment_tab, + category: create(:course_assessment_category, course: other_course)) + expect do + described_class.update_gradebook_weights( + course: course, + updates: [tab_id: other_tab.id, weight: 50] + ) + end.to raise_error(ActiveRecord::RecordNotFound) + end + end end end From dba6faa08183b671a048a18ab765d885859da41a Mon Sep 17 00:00:00 2001 From: lws49 Date: Fri, 29 May 2026 12:22:18 +0800 Subject: [PATCH 08/26] =?UTF-8?q?feat(gradebook):=20add=20weighted=20view?= =?UTF-8?q?=20UI=20=E2=80=94=20table,=20config=20dialog,=20column=20picker?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - add GradebookWeightedTable with 3-row sticky header showing per-tab additive weighted subtotals and per-student totals - add ConfigureWeightsPrompt for managers to edit tab weights (0–100) with real-time sum-to-100 warning and integer validation - add All vs By-weight view toggle in GradebookIndex (role-aware) - add column picker (Student info, Gamification groups); External ID defaults visible when any student has one, hidden otherwise - add computeWeighted helpers (computeTabSubtotal, computeStudentTotal, sumWeights) with full test coverage - add useDismissibleOnce hook for one-time dismissible hint UI --- .../components/course/gradebook_component.rb | 19 + client/app/__test__/mocks/localeMock.js | 2 + client/app/__test__/setup.js | 12 + client/app/api/course/Gradebook.ts | 8 +- .../__tests__/ConfigureWeightsPrompt.test.tsx | 131 ++++ .../__tests__/GradebookIndex.test.tsx | 103 ++- .../__tests__/GradebookWeightedTable.test.tsx | 616 ++++++++++++++++ .../WeightedGradebookColumnTree.test.tsx | 84 +++ .../__tests__/WeightedViewHint.test.tsx | 50 ++ .../__tests__/computeWeighted.test.ts | 384 ++++++++++ .../course/gradebook/__tests__/store.test.ts | 50 ++ .../components/ConfigureWeightsPrompt.tsx | 264 +++++++ .../components/GradebookWeightedTable.tsx | 680 ++++++++++++++++++ .../WeightedGradebookColumnTree.tsx | 138 ++++ .../gradebook/components/WeightedViewHint.tsx | 65 ++ .../course/gradebook/computeWeighted.ts | 177 +++++ .../bundles/course/gradebook/operations.ts | 8 + .../gradebook/pages/GradebookIndex/index.tsx | 75 +- .../app/bundles/course/gradebook/selectors.ts | 4 + client/app/bundles/course/gradebook/store.ts | 37 +- client/app/bundles/course/gradebook/types.ts | 1 + .../MuiTableAdapter/MuiColumnPickerDialog.tsx | 152 ---- .../__tests__/MuiColumnPickerDialog.test.tsx | 269 ------- .../containers/AppContainer/AppContainer.tsx | 16 + .../__tests__/useDismissibleOnce.test.tsx | 62 ++ client/app/lib/hooks/useDismissibleOnce.ts | 48 ++ client/app/types/course/gradebook.ts | 7 + client/jest.config.js | 1 + client/locales/en.json | 75 ++ client/locales/ko.json | 6 + client/locales/zh.json | 6 + 31 files changed, 3121 insertions(+), 429 deletions(-) create mode 100644 client/app/__test__/mocks/localeMock.js create mode 100644 client/app/bundles/course/gradebook/__tests__/ConfigureWeightsPrompt.test.tsx create mode 100644 client/app/bundles/course/gradebook/__tests__/GradebookWeightedTable.test.tsx create mode 100644 client/app/bundles/course/gradebook/__tests__/WeightedGradebookColumnTree.test.tsx create mode 100644 client/app/bundles/course/gradebook/__tests__/WeightedViewHint.test.tsx create mode 100644 client/app/bundles/course/gradebook/__tests__/computeWeighted.test.ts create mode 100644 client/app/bundles/course/gradebook/__tests__/store.test.ts create mode 100644 client/app/bundles/course/gradebook/components/ConfigureWeightsPrompt.tsx create mode 100644 client/app/bundles/course/gradebook/components/GradebookWeightedTable.tsx create mode 100644 client/app/bundles/course/gradebook/components/WeightedGradebookColumnTree.tsx create mode 100644 client/app/bundles/course/gradebook/components/WeightedViewHint.tsx create mode 100644 client/app/bundles/course/gradebook/computeWeighted.ts delete mode 100644 client/app/lib/components/table/MuiTableAdapter/MuiColumnPickerDialog.tsx delete mode 100644 client/app/lib/components/table/__tests__/MuiColumnPickerDialog.test.tsx create mode 100644 client/app/lib/hooks/__tests__/useDismissibleOnce.test.tsx create mode 100644 client/app/lib/hooks/useDismissibleOnce.ts diff --git a/app/controllers/components/course/gradebook_component.rb b/app/controllers/components/course/gradebook_component.rb index fe2ccfe09e..e9c8a8d942 100644 --- a/app/controllers/components/course/gradebook_component.rb +++ b/app/controllers/components/course/gradebook_component.rb @@ -7,6 +7,12 @@ def self.display_name end def sidebar_items + main_sidebar_items + settings_sidebar_items + end + + private + + def main_sidebar_items return [] unless can?(:read_gradebook, current_course) [ @@ -20,4 +26,17 @@ def sidebar_items } ] end + + def settings_sidebar_items + return [] unless can?(:manage_gradebook_settings, current_course) + + [ + { + key: self.class.key, + type: :settings, + weight: 14, + path: course_admin_gradebook_path(current_course) + } + ] + end end diff --git a/client/app/__test__/mocks/localeMock.js b/client/app/__test__/mocks/localeMock.js new file mode 100644 index 0000000000..1f87539212 --- /dev/null +++ b/client/app/__test__/mocks/localeMock.js @@ -0,0 +1,2 @@ +// File used for jest moduleNameMapper - empty locale messages for tests +module.exports = {}; diff --git a/client/app/__test__/setup.js b/client/app/__test__/setup.js index ce09d53127..d49da129df 100644 --- a/client/app/__test__/setup.js +++ b/client/app/__test__/setup.js @@ -65,3 +65,15 @@ jest.mock('react-router-dom', () => ({ useNavigate: jest.fn(), unstable_usePrompt: jest.fn(), })); + +// Replace I18nProvider with a synchronous stub so tests using test-utils +// don't stall on async locale loading. +jest.mock('lib/components/wrappers/I18nProvider', () => { + const { IntlProvider } = require('react-intl'); + const SyncI18nProvider = ({ children }) => ( + + {children} + + ); + return { __esModule: true, default: SyncI18nProvider }; +}); diff --git a/client/app/api/course/Gradebook.ts b/client/app/api/course/Gradebook.ts index e00c94a64c..7603f1f2a1 100644 --- a/client/app/api/course/Gradebook.ts +++ b/client/app/api/course/Gradebook.ts @@ -1,4 +1,4 @@ -import { GradebookData } from 'types/course/gradebook'; +import { GradebookData, UpdateWeightsPayload } from 'types/course/gradebook'; import { APIResponse } from 'api/types'; @@ -12,4 +12,10 @@ export default class GradebookAPI extends BaseCourseAPI { index(): APIResponse { return this.client.get(this.#urlPrefix); } + + updateWeights( + payload: UpdateWeightsPayload, + ): APIResponse { + return this.client.patch(`${this.#urlPrefix}/weights`, payload); + } } diff --git a/client/app/bundles/course/gradebook/__tests__/ConfigureWeightsPrompt.test.tsx b/client/app/bundles/course/gradebook/__tests__/ConfigureWeightsPrompt.test.tsx new file mode 100644 index 0000000000..346c489c67 --- /dev/null +++ b/client/app/bundles/course/gradebook/__tests__/ConfigureWeightsPrompt.test.tsx @@ -0,0 +1,131 @@ +import { fireEvent, render, screen, waitFor } from 'test-utils'; + +import ConfigureWeightsPrompt from '../components/ConfigureWeightsPrompt'; +import * as operations from '../operations'; + +jest + .spyOn(operations, 'updateGradebookWeights') + .mockReturnValue(async () => {}); + +const categories = [{ id: 1, title: 'Missions' }]; +const tabs = [ + { id: 10, title: 'Assignments', categoryId: 1, gradebookWeight: 50 }, + { id: 11, title: 'Optional', categoryId: 1, gradebookWeight: 50 }, +]; +const assessments = [ + { id: 101, title: 'Assignment 1', tabId: 10, maxGrade: 100 }, + { id: 102, title: 'Assignment 2', tabId: 10, maxGrade: 100 }, +]; + +const setup = (overrides = {}): ReturnType => + render( + , + ); + +describe('', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('renders one input per tab grouped by category', () => { + setup(); + expect(screen.getByText('Missions')).toBeInTheDocument(); + expect(screen.getByLabelText('Assignments')).toHaveValue(50); + expect(screen.getByLabelText('Optional')).toHaveValue(50); + }); + + it('shows Total: 100% with no warning when sum = 100', () => { + setup(); + expect(screen.getByText(/Total:\s*100%/)).toBeInTheDocument(); + expect(screen.queryByText(/do not sum to 100/i)).not.toBeInTheDocument(); + }); + + it('shows warning when sum != 100', () => { + setup(); + fireEvent.change(screen.getByLabelText('Optional'), { + target: { value: '30' }, + }); + expect(screen.getByText(/Total:\s*80%/)).toBeInTheDocument(); + expect(screen.getByText(/do not sum to 100/i)).toBeInTheDocument(); + }); + + it('shows inline error for >100', () => { + setup(); + fireEvent.change(screen.getByLabelText('Assignments'), { + target: { value: '101' }, + }); + expect(screen.getByText(/must be at most 100/i)).toBeInTheDocument(); + }); + + it('shows inline error for negative', () => { + setup(); + fireEvent.change(screen.getByLabelText('Optional'), { + target: { value: '-1' }, + }); + expect(screen.getByText(/must be at least 0/i)).toBeInTheDocument(); + }); + + it('Save dispatches updateGradebookWeights with { tabId, weight } only', async () => { + setup(); + fireEvent.change(screen.getByLabelText('Optional'), { + target: { value: '40' }, + }); + fireEvent.click(screen.getByRole('button', { name: /save/i })); + await waitFor(() => { + expect(operations.updateGradebookWeights).toHaveBeenCalledWith([ + { tabId: 10, weight: 50 }, + { tabId: 11, weight: 40 }, + ]); + }); + }); + + it('Cancel does not dispatch', () => { + setup(); + fireEvent.change(screen.getByLabelText('Optional'), { + target: { value: '40' }, + }); + fireEvent.click(screen.getByRole('button', { name: /cancel/i })); + expect(operations.updateGradebookWeights).not.toHaveBeenCalled(); + }); + + it('assessment list is hidden by default and shown on expand', () => { + setup(); + expect(screen.queryByText('Assignment 1')).not.toBeVisible(); + const expandBtns = screen.getAllByRole('button', { name: '' }); + fireEvent.click(expandBtns[0]); + expect(screen.getByText('Assignment 1')).toBeVisible(); + expect(screen.getByText('Assignment 2')).toBeVisible(); + }); + + it('shows derived % of grade that updates live when weight changes', () => { + setup(); + const expandBtns = screen.getAllByRole('button', { name: '' }); + fireEvent.click(expandBtns[0]); + // weight=50, each assessment is 50% of tab → 25.0% of grade each + expect(screen.getAllByText('25.0% of grade')).toHaveLength(2); + fireEvent.change(screen.getByLabelText('Assignments'), { + target: { value: '60' }, + }); + expect(screen.getAllByText('30.0% of grade')).toHaveLength(2); + }); + + it('disables expand button for tabs with no assessments', () => { + setup(); + const expandBtns = screen.getAllByRole('button', { name: '' }); + expect(expandBtns[1]).toBeDisabled(); + }); + + it('does not render an Exclude checkbox', () => { + setup(); + expect( + screen.queryByRole('checkbox', { name: /exclude/i }), + ).not.toBeInTheDocument(); + }); +}); diff --git a/client/app/bundles/course/gradebook/__tests__/GradebookIndex.test.tsx b/client/app/bundles/course/gradebook/__tests__/GradebookIndex.test.tsx index 18074f643a..8a522c496c 100644 --- a/client/app/bundles/course/gradebook/__tests__/GradebookIndex.test.tsx +++ b/client/app/bundles/course/gradebook/__tests__/GradebookIndex.test.tsx @@ -1,4 +1,4 @@ -import { fireEvent, render, screen, waitFor } from 'test-utils'; +import { fireEvent, render, screen, waitFor, within } from 'test-utils'; import toast from 'lib/hooks/toast'; @@ -33,6 +33,8 @@ const emptyState = { submissions: [], gamificationEnabled: false, userId: 0, + weightedViewEnabled: false, + canManageWeights: false, }, }; @@ -45,6 +47,8 @@ const noStudentsState = { submissions: [], gamificationEnabled: false, userId: 0, + weightedViewEnabled: false, + canManageWeights: false, }, }; @@ -66,6 +70,8 @@ const populatedState = { submissions: [{ studentId: 1, assessmentId: 100, grade: 8 }], gamificationEnabled: false, userId: 0, + weightedViewEnabled: false, + canManageWeights: false, }, }; @@ -76,6 +82,39 @@ const populatedStateWithGamification = { }, }; +const populatedStateWithWeightedView = { + gradebook: { + ...populatedState.gradebook, + weightedViewEnabled: true, + canManageWeights: false, + }, +}; + +const populatedStateWithWeightedViewAndGamification = { + gradebook: { + ...populatedState.gradebook, + weightedViewEnabled: true, + gamificationEnabled: true, + canManageWeights: false, + }, +}; + +const populatedStateManagerWeightedOff = { + gradebook: { + ...populatedState.gradebook, + weightedViewEnabled: false, + canManageWeights: true, + }, +}; + +const populatedStateManagerWeightedOn = { + gradebook: { + ...populatedState.gradebook, + weightedViewEnabled: true, + canManageWeights: true, + }, +}; + beforeEach(() => { jest.clearAllMocks(); mockFetchGradebook.mockReturnValue((): Promise => Promise.resolve()); @@ -147,4 +186,66 @@ describe('GradebookIndex', () => { ), ).toBeInTheDocument(); }); + + it('does not render view toggle when weightedViewEnabled is false', async () => { + render(, { state: populatedState }); + // Wait for loading to finish + await screen.findByRole('button', { name: /export/i }); + expect(screen.queryByText(/by weight/i)).not.toBeInTheDocument(); + }); + + it('renders view toggle when weightedViewEnabled is true', async () => { + render(, { state: populatedStateWithWeightedView }); + expect(await screen.findByText(/all assessments/i)).toBeInTheDocument(); + expect(await screen.findByText(/by weight/i)).toBeInTheDocument(); + }); + + it('switches to By weight view on toggle click', async () => { + render(, { state: populatedStateWithWeightedView }); + const byWeightButton = await screen.findByText(/by weight/i); + fireEvent.click(byWeightButton); + expect( + await screen.findByTestId('gradebook-weighted-table'), + ).toBeInTheDocument(); + }); + + it('weighted view exposes gamification columns in picker when gamification is enabled', async () => { + render(, { + state: populatedStateWithWeightedViewAndGamification, + }); + const byWeightButton = await screen.findByText(/by weight/i); + fireEvent.click(byWeightButton); + await screen.findByTestId('gradebook-weighted-table'); + fireEvent.click( + await screen.findByRole('button', { name: /select columns/i }), + ); + const dialog = await screen.findByRole('dialog'); + expect(within(dialog).getByText('Level')).toBeInTheDocument(); + expect(within(dialog).getByText('Total XP')).toBeInTheDocument(); + }); + + describe('weighted-view discoverability hint', () => { + it('shows the hint to managers when the weighted view is off', async () => { + render(, { state: populatedStateManagerWeightedOff }); + expect( + await screen.findByRole('link', { name: /gradebook settings/i }), + ).toBeInTheDocument(); + }); + + it('does not show the hint once the weighted view is enabled', async () => { + render(, { state: populatedStateManagerWeightedOn }); + await screen.findByText(/by weight/i); // wait for data to load + expect( + screen.queryByRole('link', { name: /gradebook settings/i }), + ).not.toBeInTheDocument(); + }); + + it('does not show the hint to staff who cannot manage weights', async () => { + render(, { state: populatedState }); + await screen.findByRole('button', { name: /export/i }); // wait for load + expect( + screen.queryByRole('link', { name: /gradebook settings/i }), + ).not.toBeInTheDocument(); + }); + }); }); diff --git a/client/app/bundles/course/gradebook/__tests__/GradebookWeightedTable.test.tsx b/client/app/bundles/course/gradebook/__tests__/GradebookWeightedTable.test.tsx new file mode 100644 index 0000000000..eeda18fb27 --- /dev/null +++ b/client/app/bundles/course/gradebook/__tests__/GradebookWeightedTable.test.tsx @@ -0,0 +1,616 @@ +import { fireEvent } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { store as appStore } from 'store'; +import { render, screen, waitFor, within } from 'test-utils'; + +import GradebookWeightedTable from '../components/GradebookWeightedTable'; +import type { + AssessmentData, + CategoryData, + StudentData, + SubmissionData, + TabData, +} from '../types'; + +const USER_ID = 42; +const WEIGHTED_STORAGE_KEY = `${USER_ID}:gradebook_weighted_columns_1`; +const userState = { + global: { + ...appStore.getState().global, + user: { + ...appStore.getState().global.user, + user: { id: USER_ID, name: '', imageUrl: '' }, + }, + }, +}; + +// --------------------------------------------------------------------------- +// Minimal shared fixtures +// --------------------------------------------------------------------------- +const makeCategory = (id: number, title: string): CategoryData => ({ + id, + title, +}); + +const makeTab = ( + id: number, + title: string, + categoryId: number, + gradebookWeight = 50, +): TabData => ({ id, title, categoryId, gradebookWeight }); + +const makeAssessment = ( + id: number, + title: string, + tabId: number, + maxGrade: number, +): AssessmentData => ({ id, title, tabId, maxGrade }); + +const makeStudent = (id: number, name: string): StudentData => ({ + id, + name, + email: `${name.toLowerCase()}@example.com`, + externalId: null, + level: 1, + totalXp: 0, +}); + +const makeSub = ( + studentId: number, + assessmentId: number, + grade: number | null, +): SubmissionData => ({ studentId, assessmentId, grade }); + +interface RenderWeightedOptions { + categories?: CategoryData[]; + tabs?: TabData[]; + assessments?: AssessmentData[]; + students?: StudentData[]; + submissions?: SubmissionData[]; + canManageWeights?: boolean; + courseTitle?: string; + courseId?: number; + gamificationEnabled?: boolean; +} + +const renderWeighted = ( + opts: RenderWeightedOptions = {}, +): ReturnType => { + const cats = opts.categories ?? [makeCategory(1, 'Cat A')]; + const tabs = opts.tabs ?? [makeTab(10, 'Tab 1', 1, 100)]; + const assessments = opts.assessments ?? [ + makeAssessment(100, 'Quiz 1', 10, 150), + ]; + const students = opts.students ?? [makeStudent(1, 'Alice')]; + const submissions = opts.submissions ?? []; + return render( + , + { state: userState }, + ); +}; + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- +describe('GradebookWeightedTable', () => { + beforeEach(() => localStorage.clear()); + + // 1. Row 1: category cells with colSpan + it('renders category cells in row 1 with colSpan equal to number of tabs in that category', () => { + const cats = [makeCategory(1, 'Cat A'), makeCategory(2, 'Cat B')]; + const tabs = [ + makeTab(10, 'Tab 1', 1, 50), + makeTab(11, 'Tab 2', 1, 50), + makeTab(20, 'Tab 3', 2, 0), + ]; + const assessments = [ + makeAssessment(100, 'Q1', 10, 10), + makeAssessment(101, 'Q2', 11, 10), + makeAssessment(102, 'Q3', 20, 10), + ]; + renderWeighted({ categories: cats, tabs, assessments }); + const thead = document.querySelector('thead')!; + const rows = thead.querySelectorAll('tr'); + const row1Cells = rows[0].querySelectorAll('th'); + const catACell = Array.from(row1Cells).find( + (c) => c.textContent === 'Cat A', + ); + const catBCell = Array.from(row1Cells).find( + (c) => c.textContent === 'Cat B', + ); + expect(catACell).toBeTruthy(); + expect(catBCell).toBeTruthy(); + expect( + catACell!.getAttribute('colspan') ?? catACell!.colSpan.toString(), + ).toBe('2'); + expect( + catBCell!.getAttribute('colspan') ?? catBCell!.colSpan.toString(), + ).toBe('1'); + }); + + // 1b. Category not in tabs → header absent + it('does not render a category header for categories with no tabs', () => { + renderWeighted({ + categories: [makeCategory(1, 'Active'), makeCategory(2, 'Ghost')], + tabs: [makeTab(10, 'Tab 1', 1, 100)], // only category 1 has a tab + }); + expect(screen.getByText('Active')).toBeInTheDocument(); + expect(screen.queryByText('Ghost')).not.toBeInTheDocument(); + }); + + // 2. Row 2: tab title cells + it('renders tab title cells in row 2', () => { + renderWeighted({ + tabs: [makeTab(10, 'Homework', 1, 60), makeTab(11, 'Exams', 1, 40)], + }); + const thead = document.querySelector('thead')!; + const row2 = thead.querySelectorAll('tr')[1]; + expect( + within(row2 as HTMLElement).getByText('Homework'), + ).toBeInTheDocument(); + expect(within(row2 as HTMLElement).getByText('Exams')).toBeInTheDocument(); + }); + + // 3. Weight subheader shows "X% of grade" per tab + it('shows weight subheader "X% of grade" for each tab in row 3', () => { + renderWeighted({ + tabs: [makeTab(10, 'Tab 1', 1, 30), makeTab(11, 'Tab 2', 1, 70)], + }); + expect(screen.getByText('30% of grade')).toBeInTheDocument(); + expect(screen.getByText('70% of grade')).toBeInTheDocument(); + }); + + // 4a. Total column shows "100% total" when sum = 100 + it('shows "100% total" in total column header when weights sum to 100', () => { + renderWeighted({ + tabs: [makeTab(10, 'Tab 1', 1, 60), makeTab(11, 'Tab 2', 1, 40)], + }); + expect(screen.getByText('100% total')).toBeInTheDocument(); + }); + + // 4b. Total column shows warning text when sum ≠ 100 + it('shows a warning when weight sum ≠ 100 in total header', () => { + renderWeighted({ + tabs: [makeTab(10, 'Tab 1', 1, 30), makeTab(11, 'Tab 2', 1, 30)], + }); + expect(screen.getByText(/60%/)).toBeInTheDocument(); + expect(screen.getByText(/does not sum to 100/i)).toBeInTheDocument(); + }); + + // 4c. Tooltip on warning header shows full message on hover + it('warning header tooltip text is "Weights do not sum to 100. Total may be inaccurate."', async () => { + renderWeighted({ + tabs: [makeTab(10, 'Tab 1', 1, 60), makeTab(11, 'Tab 2', 1, 20)], + }); + await userEvent.hover(screen.getByText(/does not sum to 100/i)); + await waitFor(() => + expect( + screen.getByText('Weights do not sum to 100. Total may be inaccurate.'), + ).toBeInTheDocument(), + ); + }); + + // 5. Cell renders subtotal × weight as points (not percentage); non-integer → 2dp + it('renders cell as subtotal × weight in points (not a percentage); 2dp when non-integer', () => { + // grade=130, maxGrade=150 → subtotal=130/150≈0.8667; weight=100 + // points = 0.8667 × 100 = 86.67 (non-integer → 2dp) + renderWeighted({ + tabs: [makeTab(10, 'Tab 1', 1, 100)], + assessments: [makeAssessment(100, 'Q1', 10, 150)], + students: [makeStudent(1, 'Alice')], + submissions: [makeSub(1, 100, 130)], + }); + expect(screen.getAllByText('86.67').length).toBeGreaterThanOrEqual(1); + }); + + // 6. Tab with no assessments → cell shows "—" + it('shows "—" for a tab with no assessments', () => { + renderWeighted({ + tabs: [makeTab(10, 'Empty Tab', 1, 100)], + assessments: [], + students: [makeStudent(1, 'Alice')], + submissions: [], + }); + expect(screen.getAllByText('—').length).toBeGreaterThanOrEqual(1); + }); + + // 7. Student with no graded submissions → cell shows "—" + it('shows "—" when student has no graded submissions in a tab', () => { + renderWeighted({ + tabs: [makeTab(10, 'Tab 1', 1, 100)], + assessments: [makeAssessment(100, 'Q1', 10, 10)], + students: [makeStudent(1, 'Alice')], + submissions: [], + }); + expect(screen.getAllByText('—').length).toBeGreaterThanOrEqual(1); + }); + + // 7b. Total cell shows "—" when all tabs have null subtotals + it('shows "—" in both the tab cell and the total cell when all tabs have null subtotals', () => { + renderWeighted({ + tabs: [makeTab(10, 'Tab 1', 1, 100)], + assessments: [makeAssessment(100, 'Q1', 10, 10)], + students: [makeStudent(1, 'Alice')], + submissions: [], + }); + // tab cell = "—", total cell = "—" → at least 2 dashes + expect(screen.getAllByText('—').length).toBeGreaterThanOrEqual(2); + }); + + // 8. Total equals the sum of the row's cells + it('total cell equals the sum of per-tab point cells', () => { + // tab10 weight=60: subtotal=130/150 → points=52 (integer → no decimals) + // tab20 weight=40: subtotal=0.9 → points=36 (integer → no decimals) + // total = 52 + 36 = 88 (integer → no decimals) + renderWeighted({ + categories: [makeCategory(1, 'Cat A')], + tabs: [makeTab(10, 'Tab 1', 1, 60), makeTab(20, 'Tab 2', 1, 40)], + assessments: [ + makeAssessment(1, 'A1', 10, 100), + makeAssessment(2, 'A2', 10, 50), + makeAssessment(3, 'A3', 20, 100), + ], + students: [makeStudent(1, 'Alice')], + submissions: [makeSub(1, 1, 80), makeSub(1, 2, 50), makeSub(1, 3, 90)], + }); + expect(screen.getByText('36')).toBeInTheDocument(); + expect(screen.getByText('88')).toBeInTheDocument(); + }); + + // 9. Treat Ungraded as 0 toggle changes numbers + it('changes subtotal values when "Treat Ungraded as 0" is toggled', () => { + renderWeighted({ + tabs: [makeTab(10, 'Tab 1', 1, 100)], + assessments: [ + makeAssessment(100, 'Q1', 10, 50), + makeAssessment(101, 'Q2', 10, 50), + ], + students: [makeStudent(1, 'Alice')], + submissions: [makeSub(1, 100, 40)], + }); + + // Without toggle: only Q1 is graded → 40/50=0.8; weight=100 → 80 pts (integer) + expect(screen.getAllByText('80').length).toBeGreaterThanOrEqual(1); + + // MUI Tooltip sets aria-label from its title, so the checkbox accessible name + // is the tooltip description text, not the FormControlLabel label text. + const toggle = screen.getByRole('checkbox', { + name: /counts unsubmitted and ungraded/i, + }); + fireEvent.click(toggle); + + // With toggle: 40/(50+50)=0.4; weight=100 → 40 pts (integer) + expect(screen.getAllByText('40').length).toBeGreaterThanOrEqual(1); + }); + + // 10. All weights zero → empty-state banner visible + it('shows empty-state banner when all weights are 0', () => { + renderWeighted({ + tabs: [makeTab(10, 'Tab 1', 1, 0), makeTab(11, 'Tab 2', 1, 0)], + }); + expect(screen.getByText(/no weights configured/i)).toBeInTheDocument(); + }); + + // 10b. All weights zero + canManageWeights=false → "no access" message + it('shows "no access" empty-state message when canManageWeights is false and all weights are 0', () => { + renderWeighted({ + tabs: [makeTab(10, 'Tab 1', 1, 0)], + canManageWeights: false, + }); + expect( + screen.getByText(/no tab weights have been configured yet/i), + ).toBeInTheDocument(); + expect( + screen.queryByText(/no weights configured/i), + ).not.toBeInTheDocument(); + }); + + // 10c. At least one non-zero weight → banner absent + it('does not show empty-state banner when at least one tab has a non-zero weight', () => { + renderWeighted({ + tabs: [makeTab(10, 'Tab 1', 1, 50)], + }); + expect( + screen.queryByText(/no weights configured/i), + ).not.toBeInTheDocument(); + expect( + screen.queryByText(/no tab weights have been configured yet/i), + ).not.toBeInTheDocument(); + }); + + // 11. canManageWeights === false → no "Configure Weights" button + it('does not show Configure Weights button when canManageWeights is false', () => { + renderWeighted({ canManageWeights: false }); + expect( + screen.queryByRole('button', { name: /configure weights/i }), + ).not.toBeInTheDocument(); + }); + + // 12. canManageWeights === true → "Configure Weights" button present + it('shows Configure Weights button when canManageWeights is true', () => { + renderWeighted({ canManageWeights: true }); + expect( + screen.getByRole('button', { name: /configure weights/i }), + ).toBeInTheDocument(); + }); + + // 13. Search bar is rendered + it('renders a search bar', () => { + renderWeighted(); + expect(screen.getByRole('textbox')).toBeInTheDocument(); + }); + + // 14. Typing in the search bar filters student rows + it('filters student rows when typing a name in the search bar', async () => { + const user = userEvent.setup(); + renderWeighted({ + students: [makeStudent(1, 'Alice'), makeStudent(2, 'Bob')], + }); + expect(screen.getByText('Alice')).toBeInTheDocument(); + expect(screen.getByText('Bob')).toBeInTheDocument(); + + await user.type(screen.getByRole('textbox'), 'Alice'); + + await waitFor(() => + expect(screen.queryByText('Bob')).not.toBeInTheDocument(), + ); + expect(screen.getByText('Alice')).toBeInTheDocument(); + }); + + // 15. Pagination controls appear when there are more students than the page size + it('shows pagination controls when students exceed the default page size', () => { + const manyStudents = Array.from({ length: 15 }, (_, i) => + makeStudent(i + 1, `Student ${i + 1}`), + ); + renderWeighted({ students: manyStudents }); + expect(screen.getByText('1-10 / 15')).toBeInTheDocument(); + }); + + // 16. Row selection checkboxes are rendered for each student + it('renders checkboxes for row selection', () => { + renderWeighted({ + students: [makeStudent(1, 'Alice'), makeStudent(2, 'Bob')], + }); + // One "select all" header checkbox + one per student row + expect(screen.getAllByRole('checkbox').length).toBeGreaterThanOrEqual(3); + }); + + describe('column picker', () => { + it('shows Select Columns and Export buttons', () => { + renderWeighted(); + expect( + screen.getByRole('button', { name: /select columns/i }), + ).toBeInTheDocument(); + expect( + screen.getByRole('button', { name: /export/i }), + ).toBeInTheDocument(); + }); + + it('shows "Export all rows" when no rows are selected', () => { + renderWeighted(); + expect( + screen.getByRole('button', { name: /export all rows/i }), + ).toBeInTheDocument(); + }); + + it('shows "Export 1 row" when one row is selected', async () => { + const user = userEvent.setup(); + renderWeighted({ + students: [makeStudent(1, 'Alice'), makeStudent(2, 'Bob')], + }); + const checkboxes = screen.getAllByRole('checkbox'); + // checkboxes[0] is the treatUngradedAsZero switch; [1] is header "select all"; [2] is the first row + await user.click(checkboxes[2]); + await waitFor(() => + expect( + screen.getByRole('button', { name: /export 1 row/i }), + ).toBeInTheDocument(), + ); + }); + + it('shows "Export all rows" when all rows are selected', async () => { + const user = userEvent.setup(); + renderWeighted({ + students: [makeStudent(1, 'Alice'), makeStudent(2, 'Bob')], + }); + const checkboxes = screen.getAllByRole('checkbox'); + // checkboxes[1] is the header "select all" checkbox + await user.click(checkboxes[1]); + await waitFor(() => + expect( + screen.getByRole('button', { name: /export all rows/i }), + ).toBeInTheDocument(), + ); + expect( + screen.queryByRole('button', { name: /export 2 rows/i }), + ).not.toBeInTheDocument(); + }); + + it('shows export tooltip when no rows are selected', async () => { + renderWeighted(); + await userEvent.hover( + screen.getByRole('button', { name: /export all rows/i }), + ); + await waitFor(() => + expect( + screen.getByText('No rows selected - all rows will be exported.'), + ).toBeInTheDocument(), + ); + }); + + it('hides the export tooltip when at least one row is selected', async () => { + const user = userEvent.setup(); + renderWeighted({ + students: [makeStudent(1, 'Alice'), makeStudent(2, 'Bob')], + }); + const checkboxes = screen.getAllByRole('checkbox'); + await user.click(checkboxes[2]); + const exportBtn = await screen.findByRole('button', { + name: /export 1 row/i, + }); + await userEvent.hover(exportBtn); + await waitFor(() => + expect( + screen.queryByText('No rows selected - all rows will be exported.'), + ).not.toBeInTheDocument(), + ); + }); + + it('lists Email in the picker dialog, and Level/Total XP when gamification is on', async () => { + const user = userEvent.setup(); + renderWeighted({ gamificationEnabled: true }); + await user.click(screen.getByRole('button', { name: /select columns/i })); + const dialog = await screen.findByRole('dialog'); + expect(within(dialog).getByText('Email')).toBeInTheDocument(); + expect(within(dialog).getByText('Level')).toBeInTheDocument(); + expect(within(dialog).getByText('Total XP')).toBeInTheDocument(); + }); + + it('omits the Gamification group from the dialog when gamification is off', async () => { + const user = userEvent.setup(); + renderWeighted({ gamificationEnabled: false }); + await user.click(screen.getByRole('button', { name: /select columns/i })); + const dialog = await screen.findByRole('dialog'); + expect( + within(dialog).queryByText('Gamification'), + ).not.toBeInTheDocument(); + expect(within(dialog).queryByText('Level')).not.toBeInTheDocument(); + }); + }); + + describe('identity columns rendering', () => { + it('hides Email, External ID, Level and Total XP by default', () => { + renderWeighted({ gamificationEnabled: true }); + expect(screen.queryByText('Email')).not.toBeInTheDocument(); + expect(screen.queryByText('External ID')).not.toBeInTheDocument(); + expect(screen.queryByText('Level')).not.toBeInTheDocument(); + expect(screen.queryByText('Total XP')).not.toBeInTheDocument(); + expect(screen.queryByText('alice@example.com')).not.toBeInTheDocument(); + }); + + it('shows the Email column header and value when Email is enabled via storage', async () => { + localStorage.setItem( + WEIGHTED_STORAGE_KEY, + JSON.stringify({ email: true }), + ); + renderWeighted({ students: [makeStudent(1, 'Alice')] }); + // header cell + const thead = document.querySelector('thead')!; + expect( + within(thead as HTMLElement).getByText('Email'), + ).toBeInTheDocument(); + // body value + expect(screen.getByText('alice@example.com')).toBeInTheDocument(); + }); + + it('shows Level and Total XP values when enabled via storage and gamification is on', () => { + localStorage.setItem( + WEIGHTED_STORAGE_KEY, + JSON.stringify({ level: true, totalXp: true }), + ); + renderWeighted({ + gamificationEnabled: true, + students: [ + { + id: 1, + name: 'Alice', + email: 'a@x.com', + externalId: null, + level: 7, + totalXp: 999, + }, + ], + }); + const thead = document.querySelector('thead')!; + expect( + within(thead as HTMLElement).getByText('Level'), + ).toBeInTheDocument(); + expect( + within(thead as HTMLElement).getByText('Total XP'), + ).toBeInTheDocument(); + expect(screen.getByText('7')).toBeInTheDocument(); + expect(screen.getByText('999')).toBeInTheDocument(); + }); + + it('does not render Level/Total XP even if storage enables them when gamification is off', () => { + localStorage.setItem( + WEIGHTED_STORAGE_KEY, + JSON.stringify({ level: true, totalXp: true }), + ); + renderWeighted({ gamificationEnabled: false }); + const thead = document.querySelector('thead')!; + expect( + within(thead as HTMLElement).queryByText('Level'), + ).not.toBeInTheDocument(); + expect( + within(thead as HTMLElement).queryByText('Total XP'), + ).not.toBeInTheDocument(); + }); + + describe('external ID column', () => { + const studentsWithExtId: StudentData[] = [ + { ...makeStudent(1, 'Alice'), externalId: 'EXT-001' }, + { ...makeStudent(2, 'Bob'), externalId: null }, + ]; + + it('shows External ID column by default when a student has one', async () => { + renderWeighted({ students: studentsWithExtId }); + await screen.findByText('Alice'); + const thead = document.querySelector('thead')!; + expect( + within(thead as HTMLElement).getByText('External ID'), + ).toBeInTheDocument(); + expect(screen.getByText('EXT-001')).toBeInTheDocument(); + }); + + it('hides External ID column by default when no student has one', async () => { + renderWeighted(); + await screen.findByText('Alice'); + expect(screen.queryByText('External ID')).not.toBeInTheDocument(); + }); + + it('treats a blank external ID as absent and hides the column by default', async () => { + renderWeighted({ + students: [{ ...makeStudent(1, 'Alice'), externalId: '' }], + }); + await screen.findByText('Alice'); + expect(screen.queryByText('External ID')).not.toBeInTheDocument(); + }); + + it('shows External ID when enabled via localStorage even with no external IDs', async () => { + localStorage.setItem( + WEIGHTED_STORAGE_KEY, + JSON.stringify({ externalId: true }), + ); + renderWeighted(); + await screen.findByText('Alice'); + const thead = document.querySelector('thead')!; + expect( + within(thead as HTMLElement).getByText('External ID'), + ).toBeInTheDocument(); + }); + + it('offers External ID checkbox in picker regardless of whether students have one', async () => { + const user = userEvent.setup(); + renderWeighted(); + await user.click( + await screen.findByRole('button', { name: /select columns/i }), + ); + const dialog = await screen.findByRole('dialog'); + expect( + within(dialog).getByRole('checkbox', { name: /external id/i }), + ).toBeInTheDocument(); + }); + }); + }); +}); diff --git a/client/app/bundles/course/gradebook/__tests__/WeightedGradebookColumnTree.test.tsx b/client/app/bundles/course/gradebook/__tests__/WeightedGradebookColumnTree.test.tsx new file mode 100644 index 0000000000..8058ddeed6 --- /dev/null +++ b/client/app/bundles/course/gradebook/__tests__/WeightedGradebookColumnTree.test.tsx @@ -0,0 +1,84 @@ +import { render, screen, within } from 'test-utils'; + +import WeightedGradebookColumnTree from '../components/WeightedGradebookColumnTree'; + +const baseContext = { + isVisible: (): boolean => false, + setVisible: (): void => {}, + setManyVisible: (): void => {}, +}; + +describe('WeightedGradebookColumnTree', () => { + it('renders Student info group with a locked Name and a toggleable Email', () => { + render( + , + ); + expect(screen.getByText('Student info')).toBeInTheDocument(); + expect(screen.getByText('Name')).toBeInTheDocument(); + expect(screen.getByText('Always included')).toBeInTheDocument(); + expect(screen.getByText('Email')).toBeInTheDocument(); + }); + + it('shows the Gamification group with Level and Total XP when gamification is enabled', () => { + render( + , + ); + expect(screen.getByText('Gamification')).toBeInTheDocument(); + expect(screen.getByText('Level')).toBeInTheDocument(); + expect(screen.getByText('Total XP')).toBeInTheDocument(); + }); + + it('hides the Gamification group when gamification is disabled', () => { + render( + , + ); + expect(screen.queryByText('Gamification')).not.toBeInTheDocument(); + expect(screen.queryByText('Level')).not.toBeInTheDocument(); + expect(screen.queryByText('Total XP')).not.toBeInTheDocument(); + }); + + it('calls setVisible when Email is toggled', async () => { + const setVisible = jest.fn(); + render( + , + ); + const emailRow = screen.getByText('Email').closest('label')!; + within(emailRow).getByRole('checkbox').click(); + expect(setVisible).toHaveBeenCalledWith('email', true); + }); + + it('renders an External ID checkbox in the Student info group', () => { + render( + , + ); + expect( + screen.getByRole('checkbox', { name: /external id/i }), + ).toBeInTheDocument(); + }); + + it('calls setVisible with externalId when the External ID checkbox is toggled', () => { + const setVisible = jest.fn(); + render( + , + ); + screen.getByRole('checkbox', { name: /external id/i }).click(); + expect(setVisible).toHaveBeenCalledWith('externalId', true); + }); +}); diff --git a/client/app/bundles/course/gradebook/__tests__/WeightedViewHint.test.tsx b/client/app/bundles/course/gradebook/__tests__/WeightedViewHint.test.tsx new file mode 100644 index 0000000000..28158ff50c --- /dev/null +++ b/client/app/bundles/course/gradebook/__tests__/WeightedViewHint.test.tsx @@ -0,0 +1,50 @@ +import { store as appStore } from 'store'; +import { fireEvent, render, screen } from 'test-utils'; + +import WeightedViewHint, { + WEIGHTED_VIEW_HINT_KEY, +} from '../components/WeightedViewHint'; + +const USER_ID = 42; +const STORAGE_KEY = `${USER_ID}:${WEIGHTED_VIEW_HINT_KEY}`; + +const userState = { + global: { + ...appStore.getState().global, + user: { + ...appStore.getState().global.user, + user: { id: USER_ID, name: '', imageUrl: '' }, + }, + }, +}; + +const renderHint = (): void => { + render(, { state: userState }); +}; + +beforeEach(() => localStorage.clear()); + +describe('WeightedViewHint', () => { + it('renders the capability hint with a link to gradebook settings', () => { + renderHint(); + // Copy names the capability (weighted total grade), not the mechanism. + expect(screen.getByText(/weighted total/i)).toBeInTheDocument(); + + const link = screen.getByRole('link', { name: /gradebook settings/i }); + expect(link).toHaveAttribute('href', '/courses/7/admin/gradebook'); + }); + + it('hides and persists dismissal when the close button is clicked', () => { + renderHint(); + fireEvent.click(screen.getByRole('button', { name: /close/i })); + + expect(screen.queryByText(/weighted total/i)).not.toBeInTheDocument(); + expect(localStorage.getItem(STORAGE_KEY)).toBe('true'); + }); + + it('does not render when already dismissed', () => { + localStorage.setItem(STORAGE_KEY, 'true'); + renderHint(); + expect(screen.queryByText(/weighted total/i)).not.toBeInTheDocument(); + }); +}); diff --git a/client/app/bundles/course/gradebook/__tests__/computeWeighted.test.ts b/client/app/bundles/course/gradebook/__tests__/computeWeighted.test.ts new file mode 100644 index 0000000000..cb960d57ea --- /dev/null +++ b/client/app/bundles/course/gradebook/__tests__/computeWeighted.test.ts @@ -0,0 +1,384 @@ +// client/app/bundles/course/gradebook/__tests__/computeWeighted.test.ts +import { + computeStudentTotal, + computeTabSubtotal, + computeWeightedRows, + sumWeights, +} from '../computeWeighted'; + +const assessments = [ + { id: 1, tabId: 10, maxGrade: 100, title: 'A' }, + { id: 2, tabId: 10, maxGrade: 50, title: 'B' }, + { id: 3, tabId: 20, maxGrade: 100, title: 'C' }, +]; + +const subs = ( + entries: { studentId: number; assessmentId: number; grade: number | null }[], +): { studentId: number; assessmentId: number; grade: number | null }[] => + entries; + +describe('computeTabSubtotal', () => { + it('returns null when tab has no assessments', () => { + expect( + computeTabSubtotal({ + studentId: 1, + tab: { id: 999, title: 'X', categoryId: 0 }, + assessments, + submissions: [], + treatUngradedAsZero: false, + }), + ).toBeNull(); + }); + + it('returns null when student has no graded submissions and toggle off', () => { + expect( + computeTabSubtotal({ + studentId: 1, + tab: { id: 10, title: 'M', categoryId: 0 }, + assessments, + submissions: [], + treatUngradedAsZero: false, + }), + ).toBeNull(); + }); + + it('sum-of-points across graded only when toggle off', () => { + expect( + computeTabSubtotal({ + studentId: 1, + tab: { id: 10, title: 'M', categoryId: 0 }, + assessments, + submissions: subs([ + { studentId: 1, assessmentId: 1, grade: 80 }, + // assessment 2 ungraded + ]), + treatUngradedAsZero: false, + }), + ).toBeCloseTo(0.8); + }); + + it('includes ungraded as zero when toggle on', () => { + expect( + computeTabSubtotal({ + studentId: 1, + tab: { id: 10, title: 'M', categoryId: 0 }, + assessments, + submissions: subs([{ studentId: 1, assessmentId: 1, grade: 80 }]), + treatUngradedAsZero: true, + }), + ).toBeCloseTo(80 / 150); + }); +}); + +describe('computeStudentTotal', () => { + const tabs = [ + { id: 10, title: 'M', categoryId: 0, gradebookWeight: 60 }, + { id: 20, title: 'T', categoryId: 0, gradebookWeight: 40 }, + ]; + + it('returns additive sum of weight × subtotal', () => { + // tab10 subtotal = (80+50)/(100+50) = 130/150 ≈ 0.8667; tab20 subtotal = 90/100 = 0.9 + // total = 60*(130/150) + 40*0.9 = 52 + 36 = 88 + const total = computeStudentTotal({ + studentId: 1, + tabs, + assessments, + submissions: subs([ + { studentId: 1, assessmentId: 1, grade: 80 }, + { studentId: 1, assessmentId: 2, grade: 50 }, + { studentId: 1, assessmentId: 3, grade: 90 }, + ]), + treatUngradedAsZero: false, + }); + expect(total).toBeCloseTo(60 * (130 / 150) + 40 * 0.9); + }); + + it('weight-0 tab contributes 0 to the sum', () => { + // tab20 weight=0 → 0 * 0.9 = 0; total = 100*(130/150) + const total = computeStudentTotal({ + studentId: 1, + tabs: [ + { id: 10, title: 'M', categoryId: 0, gradebookWeight: 100 }, + { id: 20, title: 'T', categoryId: 0, gradebookWeight: 0 }, + ], + assessments, + submissions: subs([ + { studentId: 1, assessmentId: 1, grade: 80 }, + { studentId: 1, assessmentId: 2, grade: 50 }, + { studentId: 1, assessmentId: 3, grade: 90 }, + ]), + treatUngradedAsZero: false, + }); + expect(total).toBeCloseTo(100 * (130 / 150)); + }); + + it('returns null when no tab has a non-null subtotal', () => { + expect( + computeStudentTotal({ + studentId: 1, + tabs: [{ id: 10, title: 'M', categoryId: 0, gradebookWeight: 0 }], + assessments, + submissions: [], + treatUngradedAsZero: false, + }), + ).toBeNull(); + }); + + it('is additive (not normalized) when weights do not sum to 100', () => { + // total = 60*(130/150) + 30*0.9 = 52 + 27 = 79 (NOT divided by 90) + const total = computeStudentTotal({ + studentId: 1, + tabs: [ + { id: 10, title: 'M', categoryId: 0, gradebookWeight: 60 }, + { id: 20, title: 'T', categoryId: 0, gradebookWeight: 30 }, + ], + assessments, + submissions: subs([ + { studentId: 1, assessmentId: 1, grade: 80 }, + { studentId: 1, assessmentId: 2, grade: 50 }, + { studentId: 1, assessmentId: 3, grade: 90 }, + ]), + treatUngradedAsZero: false, + }); + expect(total).toBeCloseTo(60 * (130 / 150) + 30 * 0.9); + }); + + it('bonus: weights summing past 100 yield a total > 100 for a perfect student', () => { + // perfect student: all grades = maxGrade → subtotal = 1.0 per tab + // total = 60*1 + 50*1 = 110 + const bonusTabs = [ + { id: 10, title: 'M', categoryId: 0, gradebookWeight: 60 }, + { id: 20, title: 'T', categoryId: 0, gradebookWeight: 50 }, + ]; + const total = computeStudentTotal({ + studentId: 1, + tabs: bonusTabs, + assessments, + submissions: subs([ + { studentId: 1, assessmentId: 1, grade: 100 }, + { studentId: 1, assessmentId: 2, grade: 50 }, + { studentId: 1, assessmentId: 3, grade: 100 }, + ]), + treatUngradedAsZero: false, + }); + expect(total).toBeCloseTo(110); + expect(total!).toBeGreaterThan(100); + }); + + it('ungraded tab (null subtotal) contributes 0, lowering the total', () => { + // student has grades for tab10 only; tab20 has no submissions → subtotal null → 0 contribution + // total = 60*(130/150) + 0 (tab20 not counted because null) + const total = computeStudentTotal({ + studentId: 1, + tabs, + assessments, + submissions: subs([ + { studentId: 1, assessmentId: 1, grade: 80 }, + { studentId: 1, assessmentId: 2, grade: 50 }, + // no submission for assessment 3 (tab20) + ]), + treatUngradedAsZero: false, + }); + expect(total).toBeCloseTo(60 * (130 / 150)); + }); +}); + +describe('sumWeights', () => { + it('returns the sum of all tab weights', () => { + const tabs = [ + { id: 1, title: 'T1', categoryId: 1, gradebookWeight: 60 }, + { id: 2, title: 'T2', categoryId: 1, gradebookWeight: 40 }, + ]; + expect(sumWeights(tabs)).toBe(100); + }); + + it('includes all tabs regardless of weight value', () => { + const tabs = [ + { id: 1, title: 'T1', categoryId: 1, gradebookWeight: 60 }, + { id: 2, title: 'T2', categoryId: 1, gradebookWeight: 0 }, + ]; + expect(sumWeights(tabs)).toBe(60); + }); + + it('handles tabs with no gradebookWeight (treats as 0)', () => { + const tabs = [ + { id: 1, title: 'T1', categoryId: 1, gradebookWeight: 40 }, + { id: 2, title: 'T2', categoryId: 1 }, + ]; + expect(sumWeights(tabs)).toBe(40); + }); +}); + +describe('computeWeightedRows', () => { + const rowTabs = [ + { id: 10, title: 'M', categoryId: 0, gradebookWeight: 60 }, + { id: 20, title: 'T', categoryId: 0, gradebookWeight: 40 }, + ]; + const rowStudents = [ + { + id: 1, + name: 'Alice', + email: 'alice@e.com', + externalId: null, + level: 1, + totalXp: 0, + }, + { + id: 2, + name: 'Bob', + email: 'bob@e.com', + externalId: null, + level: 1, + totalXp: 0, + }, + ]; + const rowSubmissions = subs([ + // Alice: full data + { studentId: 1, assessmentId: 1, grade: 80 }, + { studentId: 1, assessmentId: 2, grade: 50 }, + { studentId: 1, assessmentId: 3, grade: 90 }, + // Bob: only tab10 graded + { studentId: 2, assessmentId: 1, grade: 100 }, + { studentId: 2, assessmentId: 2, grade: 50 }, + ]); + + it('returns one row per student carrying studentId, name and email', () => { + const rows = computeWeightedRows({ + students: rowStudents, + tabs: rowTabs, + assessments, + submissions: rowSubmissions, + treatUngradedAsZero: false, + }); + expect(rows).toHaveLength(2); + expect(rows[0]).toMatchObject({ + studentId: 1, + name: 'Alice', + email: 'alice@e.com', + }); + expect(rows[1]).toMatchObject({ + studentId: 2, + name: 'Bob', + email: 'bob@e.com', + }); + }); + + it('produces subtotals and total identical to the per-student helpers', () => { + const rows = computeWeightedRows({ + students: rowStudents, + tabs: rowTabs, + assessments, + submissions: rowSubmissions, + treatUngradedAsZero: false, + }); + rowStudents.forEach((student, i) => { + rowTabs.forEach((tab, j) => { + expect(rows[i].subtotals[j]).toEqual( + computeTabSubtotal({ + studentId: student.id, + tab, + assessments, + submissions: rowSubmissions, + treatUngradedAsZero: false, + }), + ); + }); + expect(rows[i].total).toEqual( + computeStudentTotal({ + studentId: student.id, + tabs: rowTabs, + assessments, + submissions: rowSubmissions, + treatUngradedAsZero: false, + }), + ); + }); + }); + + it('computes the known additive total for a fully-graded student', () => { + // Alice tab10 = (80+50)/(100+50) = 130/150; tab20 = 90/100 = 0.9 + // total = 60*(130/150) + 40*0.9 + const rows = computeWeightedRows({ + students: [rowStudents[0]], + tabs: rowTabs, + assessments, + submissions: rowSubmissions, + treatUngradedAsZero: false, + }); + expect(rows[0].subtotals[0]).toBeCloseTo(130 / 150); + expect(rows[0].subtotals[1]).toBeCloseTo(0.9); + expect(rows[0].total).toBeCloseTo(60 * (130 / 150) + 40 * 0.9); + }); + + it('a tab with no graded submissions yields a null subtotal (toggle off)', () => { + // Bob has no tab20 submissions -> subtotal null; total counts tab10 only + const rows = computeWeightedRows({ + students: [rowStudents[1]], + tabs: rowTabs, + assessments, + submissions: rowSubmissions, + treatUngradedAsZero: false, + }); + expect(rows[0].subtotals[0]).toBeCloseTo(1); // (100+50)/(100+50)=1 + expect(rows[0].subtotals[1]).toBeNull(); + expect(rows[0].total).toBeCloseTo(60 * 1); + }); + + it('treatUngradedAsZero counts ungraded assessments in the denominator', () => { + // Bob tab20 has assessment 3 ungraded -> with toggle on subtotal = 0/100 = 0 (not null) + const rows = computeWeightedRows({ + students: [rowStudents[1]], + tabs: rowTabs, + assessments, + submissions: rowSubmissions, + treatUngradedAsZero: true, + }); + expect(rows[0].subtotals[1]).toBe(0); + // tab10 with toggle on stays (100+50)/(100+50)=1 + expect(rows[0].subtotals[0]).toBeCloseTo(1); + expect(rows[0].total).toBeCloseTo(60 * 1 + 40 * 0); + }); + + it('returns an empty array when there are no students', () => { + expect( + computeWeightedRows({ + students: [], + tabs: rowTabs, + assessments, + submissions: rowSubmissions, + treatUngradedAsZero: false, + }), + ).toEqual([]); + }); +}); + +describe('computeWeightedRows — identity passthrough', () => { + it('carries level and totalXp from each student onto the row', () => { + const students = [ + { + id: 1, + name: 'Alice', + email: 'a@x.com', + externalId: null, + level: 5, + totalXp: 1234, + }, + ]; + const tabs = [ + { id: 10, title: 'Tab 1', categoryId: 1, gradebookWeight: 100 }, + ]; + const localAssessments = [{ id: 100, title: 'Q1', tabId: 10, maxGrade: 10 }]; + const submissions = [{ studentId: 1, assessmentId: 100, grade: 8 }]; + + const rows = computeWeightedRows({ + students, + tabs, + assessments: localAssessments, + submissions, + treatUngradedAsZero: false, + }); + + expect(rows[0].level).toBe(5); + expect(rows[0].totalXp).toBe(1234); + }); +}); diff --git a/client/app/bundles/course/gradebook/__tests__/store.test.ts b/client/app/bundles/course/gradebook/__tests__/store.test.ts new file mode 100644 index 0000000000..827772dbe3 --- /dev/null +++ b/client/app/bundles/course/gradebook/__tests__/store.test.ts @@ -0,0 +1,50 @@ +import reducer, { actions } from '../store'; + +const baseState = { + categories: [], + tabs: [ + { id: 1, title: 'T1', categoryId: 1, gradebookWeight: 50 }, + { id: 2, title: 'T2', categoryId: 1, gradebookWeight: 50 }, + ], + assessments: [], + students: [], + submissions: [], + gamificationEnabled: false, + userId: 0, + weightedViewEnabled: false, + canManageWeights: false, +}; + +describe('UPDATE_TAB_WEIGHTS reducer', () => { + it('updates gradebookWeight for the matching tab', () => { + const next = reducer( + baseState, + actions.updateTabWeights({ weights: [{ tabId: 1, weight: 80 }] }), + ); + expect(next.tabs.find((t) => t.id === 1)?.gradebookWeight).toBe(80); + expect(next.tabs.find((t) => t.id === 2)?.gradebookWeight).toBe(50); + }); + + it('does not set any excluded field', () => { + const next = reducer( + baseState, + actions.updateTabWeights({ weights: [{ tabId: 1, weight: 0 }] }), + ); + const tab = next.tabs.find((t) => t.id === 1)!; + expect(tab).not.toHaveProperty('gradebookExcluded'); + }); + + it('updates multiple tabs in one action', () => { + const next = reducer( + baseState, + actions.updateTabWeights({ + weights: [ + { tabId: 1, weight: 30 }, + { tabId: 2, weight: 70 }, + ], + }), + ); + expect(next.tabs.find((t) => t.id === 1)?.gradebookWeight).toBe(30); + expect(next.tabs.find((t) => t.id === 2)?.gradebookWeight).toBe(70); + }); +}); diff --git a/client/app/bundles/course/gradebook/components/ConfigureWeightsPrompt.tsx b/client/app/bundles/course/gradebook/components/ConfigureWeightsPrompt.tsx new file mode 100644 index 0000000000..5fc5f36403 --- /dev/null +++ b/client/app/bundles/course/gradebook/components/ConfigureWeightsPrompt.tsx @@ -0,0 +1,264 @@ +import { FC, useEffect, useState } from 'react'; +import { defineMessages } from 'react-intl'; +import { ExpandLess, ExpandMore } from '@mui/icons-material'; +import { + Alert, + Collapse, + IconButton, + Stack, + TextField, + Typography, +} from '@mui/material'; +import type { + AssessmentData, + CategoryData, + TabData, +} from 'types/course/gradebook'; + +import Prompt from 'lib/components/core/dialogs/Prompt'; +import { useAppDispatch } from 'lib/hooks/store'; +import toast from 'lib/hooks/toast'; +import useTranslation from 'lib/hooks/useTranslation'; + +import { updateGradebookWeights } from '../operations'; + +const translations = defineMessages({ + dialogTitle: { + id: 'course.gradebook.ConfigureWeightsDialog.dialogTitle', + defaultMessage: 'Configure tab weights', + }, + description: { + id: 'course.gradebook.ConfigureWeightsDialog.description', + defaultMessage: + 'Set how much each tab contributes to the total grade. Weights should sum to 100.', + }, + total: { + id: 'course.gradebook.ConfigureWeightsDialog.total', + defaultMessage: 'Total: {sum}%', + }, + weightsDoNotSum: { + id: 'course.gradebook.ConfigureWeightsDialog.weightsDoNotSum', + defaultMessage: + 'Weights do not sum to 100. Saving is allowed; Total may be inaccurate.', + }, + save: { + id: 'course.gradebook.ConfigureWeightsDialog.save', + defaultMessage: 'Save', + }, + valueTooLow: { + id: 'course.gradebook.ConfigureWeightsDialog.valueTooLow', + defaultMessage: 'Value must be at least 0', + }, + valueTooHigh: { + id: 'course.gradebook.ConfigureWeightsDialog.valueTooHigh', + defaultMessage: 'Value must be at most 100', + }, + valueNotInteger: { + id: 'course.gradebook.ConfigureWeightsDialog.valueNotInteger', + defaultMessage: 'Value must be a whole number', + }, + saveError: { + id: 'course.gradebook.ConfigureWeightsDialog.saveError', + defaultMessage: 'Failed to save weights. Please try again.', + }, + ofGrade: { + id: 'course.gradebook.ConfigureWeightsDialog.ofGrade', + defaultMessage: '{pct}% of grade', + }, +}); + +interface Props { + open: boolean; + onClose: () => void; + categories: CategoryData[]; + tabs: TabData[]; + assessments: AssessmentData[]; +} + +const ConfigureWeightsPrompt: FC = ({ + open, + onClose, + categories, + tabs, + assessments, +}) => { + const { t } = useTranslation(); + const dispatch = useAppDispatch(); + + const validate = (value: number): string | null => { + if (Number.isNaN(value)) return t(translations.valueTooLow); + if (!Number.isInteger(value)) return t(translations.valueNotInteger); + if (value < 0) return t(translations.valueTooLow); + if (value > 100) return t(translations.valueTooHigh); + return null; + }; + + const [weights, setWeights] = useState>(() => + Object.fromEntries(tabs.map((tb) => [tb.id, tb.gradebookWeight ?? 0])), + ); + const [expanded, setExpanded] = useState>({}); + const [submitting, setSubmitting] = useState(false); + + useEffect(() => { + if (open) { + setWeights( + Object.fromEntries(tabs.map((tb) => [tb.id, tb.gradebookWeight ?? 0])), + ); + setExpanded({}); + } + }, [open]); + + const sum = Object.values(weights).reduce((acc, w) => acc + w, 0); + const hasInvalid = Object.values(weights).some((w) => validate(w) !== null); + + const handleChange = (tabId: number, raw: string): void => { + const parsed = raw === '' ? 0 : Number(raw); + setWeights((prev) => ({ ...prev, [tabId]: parsed })); + }; + + const toggleExpanded = (tabId: number): void => + setExpanded((prev) => ({ ...prev, [tabId]: !prev[tabId] })); + + const handleSave = async (): Promise => { + if (hasInvalid) return; + setSubmitting(true); + try { + await dispatch( + updateGradebookWeights( + tabs.map((tb) => ({ + tabId: tb.id, + weight: weights[tb.id] ?? 0, + })), + ), + ); + onClose(); + } catch { + toast.error(t(translations.saveError)); + } finally { + setSubmitting(false); + } + }; + + return ( + + + {t(translations.description)} + + + {categories.map((cat) => ( +
+ {cat.title} + + {tabs + .filter((tb) => tb.categoryId === cat.id) + .map((tb) => { + const value = weights[tb.id] ?? 0; + const err = validate(value); + const tabAssessments = assessments.filter( + (a) => a.tabId === tb.id, + ); + const tabMaxGrade = tabAssessments.reduce( + (s, a) => s + a.maxGrade, + 0, + ); + const isExpanded = !!expanded[tb.id]; + + return ( +
+
+ toggleExpanded(tb.id)} + size="small" + > + {isExpanded ? ( + + ) : ( + + )} + + + {tb.title} + + handleChange(tb.id, e.target.value)} + size="small" + sx={{ width: 96 }} + type="number" + value={value} + /> +
+ {err && ( + + {err} + + )} + + + {tabAssessments.map((a) => { + const pct = + tabMaxGrade > 0 + ? (a.maxGrade / tabMaxGrade) * value + : 0; + return ( +
+ + {a.title} + + + {t(translations.ofGrade, { + pct: pct.toFixed(1), + })} + +
+ ); + })} + + +
+ ); + })} + +
+ ))} + + + {t(translations.total, { sum })} + + {sum !== 100 && ( + + {t(translations.weightsDoNotSum)} + + )} + + ); +}; + +export default ConfigureWeightsPrompt; diff --git a/client/app/bundles/course/gradebook/components/GradebookWeightedTable.tsx b/client/app/bundles/course/gradebook/components/GradebookWeightedTable.tsx new file mode 100644 index 0000000000..2f433024fd --- /dev/null +++ b/client/app/bundles/course/gradebook/components/GradebookWeightedTable.tsx @@ -0,0 +1,680 @@ +import { useMemo, useState } from 'react'; +import { defineMessages } from 'react-intl'; +import { Download } from '@mui/icons-material'; +import { + Alert, + Button, + Checkbox, + FormControlLabel, + Paper, + Switch, + Table, + TableBody, + TableCell, + TableContainer, + TableHead, + TableRow, + Tooltip, + Typography, +} from '@mui/material'; +import type { + AssessmentData, + CategoryData, + StudentData, + SubmissionData, + TabData, +} from 'types/course/gradebook'; + +import SearchField from 'lib/components/core/fields/SearchField'; +import type { ColumnPickerRenderContext } from 'lib/components/table'; +import type { ColumnTemplate } from 'lib/components/table/builder'; +import MuiColumnPickerPrompt from 'lib/components/table/MuiTableAdapter/MuiColumnPickerPrompt'; +import MuiTablePagination from 'lib/components/table/MuiTableAdapter/MuiTablePagination'; +import useTanStackTableBuilder from 'lib/components/table/TanStackTableBuilder'; +import { + DEFAULT_MINI_TABLE_ROWS_PER_PAGE, + DEFAULT_TABLE_ROWS_PER_PAGE, +} from 'lib/constants/sharedConstants'; +import useTranslation from 'lib/hooks/useTranslation'; +import tableTranslations from 'lib/translations/table'; + +import type { WeightedRow } from '../computeWeighted'; +import { computeWeightedRows, sumWeights } from '../computeWeighted'; + +import ConfigureWeightsPrompt from './ConfigureWeightsPrompt'; +import WeightedGradebookColumnTree from './WeightedGradebookColumnTree'; + +const translations = defineMessages({ + treatUngradedAsZero: { + id: 'course.gradebook.GradebookWeightedTable.treatUngradedAsZero', + defaultMessage: 'Treat Ungraded as 0', + }, + treatUngradedAsZeroTooltip: { + id: 'course.gradebook.GradebookWeightedTable.treatUngradedAsZeroTooltip', + defaultMessage: + 'Counts unsubmitted and ungraded assessments as 0 in the calculation. Use at end of course when all work should be complete.', + }, + configureWeights: { + id: 'course.gradebook.GradebookWeightedTable.configureWeights', + defaultMessage: 'Configure Weights', + }, + noWeightsConfigured: { + id: 'course.gradebook.GradebookWeightedTable.noWeightsConfigured', + defaultMessage: + 'No weights configured — all tab weights are 0. Click "Configure Weights" to assign weights.', + }, + noWeightsNoAccess: { + id: 'course.gradebook.GradebookWeightedTable.noWeightsNoAccess', + defaultMessage: 'No tab weights have been configured yet.', + }, + student: { + id: 'course.gradebook.GradebookWeightedTable.student', + defaultMessage: 'Student', + }, + email: { + id: 'course.gradebook.GradebookWeightedTable.email', + defaultMessage: 'Email', + }, + total: { + id: 'course.gradebook.GradebookWeightedTable.total', + defaultMessage: 'Total', + }, + percentOfGrade: { + id: 'course.gradebook.GradebookWeightedTable.percentOfGrade', + defaultMessage: '{weight}% of grade', + }, + percentTotalExact: { + id: 'course.gradebook.GradebookWeightedTable.percentTotalExact', + defaultMessage: '100% total', + }, + percentTotalWarning: { + id: 'course.gradebook.GradebookWeightedTable.percentTotalWarning', + defaultMessage: '{weight}% total', + }, + doesNotSumTo100: { + id: 'course.gradebook.GradebookWeightedTable.doesNotSumTo100', + defaultMessage: 'does not sum to 100', + }, + weightsDoNotSum: { + id: 'course.gradebook.GradebookWeightedTable.weightsDoNotSum', + defaultMessage: 'Weights do not sum to 100. Total may be inaccurate.', + }, + searchStudents: { + id: 'course.gradebook.GradebookWeightedTable.searchStudents', + defaultMessage: 'Search by name or email', + }, + downloadCsv: { + id: 'course.gradebook.GradebookWeightedTable.downloadCsv', + defaultMessage: 'Download as CSV', + }, + selectColumns: { + id: 'course.gradebook.GradebookIndex.selectColumns', + defaultMessage: 'Select Columns', + }, + dialogTitle: { + id: 'course.gradebook.GradebookIndex.dialogTitle', + defaultMessage: 'Select columns', + }, + exportButton: { + id: 'course.gradebook.GradebookIndex.exportButton', + defaultMessage: 'Export all rows', + }, + exportRows: { + id: 'course.gradebook.GradebookIndex.exportRows', + defaultMessage: 'Export {count, plural, one {# row} other {# rows}}', + }, + exportAllTooltip: { + id: 'course.gradebook.GradebookIndex.exportAllTooltip', + defaultMessage: 'No rows selected - all rows will be exported.', + }, + level: { + id: 'course.gradebook.GradebookColumnTree.level', + defaultMessage: 'Level', + }, + totalXp: { + id: 'course.gradebook.GradebookColumnTree.totalXp', + defaultMessage: 'Total XP', + }, +}); + +interface Props { + categories: CategoryData[]; + tabs: TabData[]; + assessments: AssessmentData[]; + students: StudentData[]; + submissions: SubmissionData[]; + canManageWeights: boolean; + courseTitle: string; + courseId: number; + gamificationEnabled: boolean; +} + +const fmt = (v: number | null): string => { + if (v === null) return '—'; + const rounded = Math.round(v); + return Math.abs(v - rounded) < 1e-9 ? String(rounded) : v.toFixed(2); +}; + +const fmtCsv = (v: number | null): string => { + if (v === null) return ''; + return v.toFixed(2); +}; + +const CHECKBOX_WIDTH = 56; + +const GradebookWeightedTable = ({ + categories, + tabs, + assessments, + students, + submissions, + canManageWeights, + courseTitle, + courseId, + gamificationEnabled, +}: Props): JSX.Element => { + const { t } = useTranslation(); + const [treatUngradedAsZero, setTreatUngradedAsZero] = useState(false); + const [configureOpen, setConfigureOpen] = useState(false); + const [pickerOpen, setPickerOpen] = useState(false); + + const totalWeight = sumWeights(tabs); + const allWeightsZero = totalWeight === 0; + + const categoryTabCounts = useMemo(() => { + const counts = new Map(); + tabs.forEach((tab) => { + counts.set(tab.categoryId, (counts.get(tab.categoryId) ?? 0) + 1); + }); + return counts; + }, [tabs]); + + const visibleCategories = useMemo( + () => categories.filter((cat) => categoryTabCounts.has(cat.id)), + [categories, categoryTabCounts], + ); + + const rows = useMemo( + () => + computeWeightedRows({ + students, + tabs, + assessments, + submissions, + treatUngradedAsZero, + }), + [students, tabs, assessments, submissions, treatUngradedAsZero], + ); + + const hasExternalIds = useMemo( + () => students.some((s) => s.externalId != null && s.externalId !== ''), + [students], + ); + + const columns = useMemo[]>(() => { + const cols: ColumnTemplate[] = [ + { + id: 'name', + title: t(translations.student), + of: 'name', + cell: (row) => row.name, + csvDownloadable: true, + searchable: true, + }, + { + id: 'email', + title: t(translations.email), + of: 'email', + cell: (row) => row.email, + csvDownloadable: true, + searchable: true, + defaultVisible: false, + }, + { + id: 'externalId', + title: t(tableTranslations.externalId), + of: 'externalId', + cell: (row) => row.externalId ?? '', + csvDownloadable: true, + defaultVisible: hasExternalIds, + }, + ]; + + if (gamificationEnabled) { + cols.push({ + id: 'level', + title: t(translations.level), + of: 'level', + cell: (row) => row.level, + csvDownloadable: true, + defaultVisible: false, + }); + cols.push({ + id: 'totalXp', + title: t(translations.totalXp), + of: 'totalXp', + cell: (row) => row.totalXp, + csvDownloadable: true, + defaultVisible: false, + }); + } + + tabs.forEach((tab, idx) => { + const weight = tab.gradebookWeight ?? 0; + cols.push({ + id: `tab-${tab.id}`, + title: tab.title, + accessorFn: (row) => { + const sub = row.subtotals[idx]; + return fmtCsv(sub !== null ? sub * weight : null); + }, + cell: (row) => { + const sub = row.subtotals[idx]; + return fmt(sub !== null ? sub * weight : null); + }, + csvDownloadable: true, + }); + }); + + cols.push({ + id: 'total', + title: t(translations.total), + accessorFn: (row) => fmtCsv(row.total), + cell: (row) => fmt(row.total), + csvDownloadable: true, + }); + + return cols; + }, [tabs, t, gamificationEnabled, hasExternalIds]); + + const columnPicker = useMemo( + () => ({ + render: (context: ColumnPickerRenderContext) => ( + + ), + locked: ['name'], + triggerLabel: t(translations.selectColumns), + dialogTitle: t(translations.dialogTitle), + storageKey: `gradebook_weighted_columns_${courseId}`, + }), + [gamificationEnabled, courseId, t], + ); + + const { + toolbar: toolbarProps, + body, + pagination, + } = useTanStackTableBuilder({ + data: rows, + columns, + getRowId: (row) => row.studentId.toString(), + getRowEqualityData: (row) => row, + indexing: { rowSelectable: true }, + pagination: { + rowsPerPage: [ + DEFAULT_MINI_TABLE_ROWS_PER_PAGE, + 25, + 50, + DEFAULT_TABLE_ROWS_PER_PAGE, + ], + showAllRows: true, + }, + search: { searchPlaceholder: t(translations.searchStudents) }, + toolbar: { show: true, keepNative: true }, + csvDownload: { + filename: `${courseTitle}_weighted_gradebook`, + showDownloadButton: false, + }, + columnPicker, + }); + + const toolbar = toolbarProps!; + + const selectedCount = body.selectedCount ?? 0; + const directExportLabel = useMemo((): string => { + const isPartial = selectedCount > 0 && selectedCount < rows.length; + if (isPartial) return t(translations.exportRows, { count: selectedCount }); + return t(translations.exportButton); + }, [selectedCount, rows.length, t]); + + const visibility = toolbar.getColumnVisibility?.() ?? {}; + const showEmail = (visibility.email ?? false) === true; + const showExternalId = (visibility.externalId ?? false) === true; + const showLevel = gamificationEnabled && (visibility.level ?? false) === true; + const showTotalXp = + gamificationEnabled && (visibility.totalXp ?? false) === true; + + const allRowsSelected = body.allFilteredSelected ?? false; + const someRowsSelected = body.someFilteredSelected ?? false; + const toggleAllRows = (): void => body.toggleAllFiltered?.(); + + return ( +
+ {/* Table + toolbar share a fit-content container so toolbar never outruns the table */} +
+ + {/* Single-row toolbar */} +
+ +
+ + setTreatUngradedAsZero(e.target.checked)} + size="small" + /> + } + label={t(translations.treatUngradedAsZero)} + sx={{ ml: 0 }} + /> + + {canManageWeights && ( + + )} + + {toolbar.onDirectExport && ( + + + + + + )} +
+
+ + {/* Empty state banner when all weights are 0 */} + {allWeightsZero && ( + + {canManageWeights + ? t(translations.noWeightsConfigured) + : t(translations.noWeightsNoAccess)} + + )} + + { + // One definition for every grid line so horizontal and vertical + // separators share the same width and colour. + const gridLine = `1px solid ${theme.palette.divider}`; + return { + tableLayout: 'auto', + borderCollapse: 'separate', + borderSpacing: 0, + '& th, & td': { + boxSizing: 'border-box', + border: 0, + borderBottom: gridLine, + }, + '& thead th': { + borderLeft: gridLine, + }, + // MUI's default `.MuiTableRow-root:last-child th { border: 0 }` + // (specificity 0,2,1, the `:last-child` pseudo-class) outranks + // `& thead th`/`& th` (0,1,2 / 0,1,1) and silently zeroes ALL + // borders on the weight row — it is the last in . + // Re-assert both the column separators (borderLeft) and the + // header's bottom edge (borderBottom) with a higher-specificity + // selector (0,2,3) so they survive through the "% of grade" row. + '& thead tr:last-of-type th': { + borderLeft: gridLine, + borderBottom: gridLine, + }, + // Remove the outer-left line: only the checkbox (row 1, col 1). + '& thead tr:first-of-type th:first-of-type': { + borderLeft: 0, + }, + }; + }} + > + + {/* Row 1: Checkbox + Student + Categories + Total */} + + + + + + {t(translations.student)} + + {showEmail && ( + + {t(translations.email)} + + )} + {showExternalId && ( + + {t(tableTranslations.externalId)} + + )} + {showLevel && ( + + {t(translations.level)} + + )} + {showTotalXp && ( + + {t(translations.totalXp)} + + )} + {visibleCategories.map((cat) => ( + + {cat.title} + + ))} + + {t(translations.total)} + + + + {/* Row 2: Tab titles */} + + {tabs.map((tab) => ( + + {tab.title} + + ))} + + + {/* Row 3: Weight subheaders */} + + {tabs.map((tab) => ( + + {t(translations.percentOfGrade, { + weight: tab.gradebookWeight ?? 0, + })} + + ))} + + {totalWeight === 100 ? ( + t(translations.percentTotalExact) + ) : ( + + + + {t(translations.percentTotalWarning, { + weight: totalWeight, + })} + + + {t(translations.doesNotSumTo100)} + + + + )} + + + + + + {body.rows.map((row, idx) => { + const rowProps = body.forEachRow(row, idx); + return ( + + + + + {row.original.name} + {showEmail && {row.original.email}} + {showExternalId && ( + {row.original.externalId ?? ''} + )} + {showLevel && ( + + {row.original.level} + + )} + {showTotalXp && ( + + {row.original.totalXp} + + )} + {row.original.subtotals.map((subtotal, i) => { + const weight = tabs[i].gradebookWeight ?? 0; + return ( + + {fmt(subtotal !== null ? subtotal * weight : null)} + + ); + })} + + {fmt(row.original.total)} + + + ); + })} + +
+ + {pagination && } + +
+ + {canManageWeights && ( + setConfigureOpen(false)} + open={configureOpen} + tabs={tabs} + /> + )} + + {toolbar.columnPicker && toolbar.commitColumnVisibility && ( + setPickerOpen(false)} + open={pickerOpen} + /> + )} +
+ ); +}; + +export default GradebookWeightedTable; diff --git a/client/app/bundles/course/gradebook/components/WeightedGradebookColumnTree.tsx b/client/app/bundles/course/gradebook/components/WeightedGradebookColumnTree.tsx new file mode 100644 index 0000000000..d166041ec3 --- /dev/null +++ b/client/app/bundles/course/gradebook/components/WeightedGradebookColumnTree.tsx @@ -0,0 +1,138 @@ +import { defineMessages } from 'react-intl'; +import { Chip } from '@mui/material'; + +import IndentedCheckbox from 'lib/components/core/IndentedCheckbox'; +import { + ColumnPickerRenderContext, + ColumnPickerTreeGroup, +} from 'lib/components/table'; +import useTranslation from 'lib/hooks/useTranslation'; +import tableTranslations from 'lib/translations/table'; + +import { + GAMIFICATION_COL_IDS, + type GamificationColId, + STUDENT_INFO_COL_IDS, + type StudentInfoColId, +} from '../constants'; + +const translations = defineMessages({ + studentInfo: { + id: 'course.gradebook.GradebookColumnTree.studentInfo', + defaultMessage: 'Student info', + }, + name: { + id: 'course.gradebook.GradebookColumnTree.name', + defaultMessage: 'Name', + }, + email: { + id: 'course.gradebook.GradebookColumnTree.email', + defaultMessage: 'Email', + }, + level: { + id: 'course.gradebook.GradebookColumnTree.level', + defaultMessage: 'Level', + }, + totalXp: { + id: 'course.gradebook.GradebookColumnTree.totalXp', + defaultMessage: 'Total XP', + }, + gamification: { + id: 'course.gradebook.GradebookColumnTree.gamification', + defaultMessage: 'Gamification', + }, + alwaysIncluded: { + id: 'course.gradebook.GradebookColumnTree.alwaysIncluded', + defaultMessage: 'Always included', + }, +}); + +interface WeightedGradebookColumnTreeProps extends ColumnPickerRenderContext { + gamificationEnabled: boolean; +} + +const EXTERNAL_ID = 'externalId'; +const STUDENT_ALL_IDS = [...STUDENT_INFO_COL_IDS, EXTERNAL_ID]; +const GAMIFICATION_ALL_IDS = [...GAMIFICATION_COL_IDS]; + +const WeightedGradebookColumnTree = ({ + isVisible, + setVisible, + setManyVisible, + gamificationEnabled, +}: WeightedGradebookColumnTreeProps): JSX.Element => { + const { t } = useTranslation(); + const context: ColumnPickerRenderContext = { + isVisible, + setVisible, + setManyVisible, + }; + + return ( +
+ + {STUDENT_INFO_COL_IDS.map((id: StudentInfoColId) => + id === 'name' ? ( + + {t(translations[id])} + + + } + /> + ) : ( + setVisible(id, e.target.checked)} + /> + ), + )} + setVisible(EXTERNAL_ID, e.target.checked)} + /> + + + {gamificationEnabled && ( + + {GAMIFICATION_COL_IDS.map((id: GamificationColId) => ( + setVisible(id, e.target.checked)} + /> + ))} + + )} +
+ ); +}; + +export default WeightedGradebookColumnTree; diff --git a/client/app/bundles/course/gradebook/components/WeightedViewHint.tsx b/client/app/bundles/course/gradebook/components/WeightedViewHint.tsx new file mode 100644 index 0000000000..aaa5499139 --- /dev/null +++ b/client/app/bundles/course/gradebook/components/WeightedViewHint.tsx @@ -0,0 +1,65 @@ +import { FC } from 'react'; +import { defineMessages } from 'react-intl'; +import { Alert, Typography } from '@mui/material'; + +import { getUserEntity } from 'bundles/users/selectors'; +import Link from 'lib/components/core/Link'; +import { useAppSelector } from 'lib/hooks/store'; +import useDismissibleOnce from 'lib/hooks/useDismissibleOnce'; +import useTranslation from 'lib/hooks/useTranslation'; + +export const WEIGHTED_VIEW_HINT_KEY = 'gradebook_weighted_view_hint'; + +const translations = defineMessages({ + hint: { + id: 'course.gradebook.WeightedViewHint.hint', + defaultMessage: + 'Want a weighted total grade? You can set how much each tab counts toward each student’s overall grade and view the weighted total here. Turn it on in {link}.', + }, + settingsLink: { + id: 'course.gradebook.WeightedViewHint.settingsLink', + defaultMessage: 'Gradebook settings', + }, +}); + +interface Props { + courseId: number; +} + +/** + * One-time, dismissable nudge shown to managers in the (always-visible) gradebook view + * when the weighted view is turned off. It advertises the capability and links to the + * setting that enables it, since that setting is otherwise buried in course admin. + * Dismissal is remembered per user via localStorage (see useDismissibleOnce). + */ +const WeightedViewHint: FC = ({ courseId }) => { + const { t } = useTranslation(); + const userId = useAppSelector(getUserEntity).id; + const { dismissed, dismiss } = useDismissibleOnce( + WEIGHTED_VIEW_HINT_KEY, + userId, + ); + + if (dismissed) return null; + + return ( +
+ + + {t(translations.hint, { + link: ( + + {t(translations.settingsLink)} + + ), + })} + + +
+ ); +}; + +export default WeightedViewHint; diff --git a/client/app/bundles/course/gradebook/computeWeighted.ts b/client/app/bundles/course/gradebook/computeWeighted.ts new file mode 100644 index 0000000000..b7dd1ac8e4 --- /dev/null +++ b/client/app/bundles/course/gradebook/computeWeighted.ts @@ -0,0 +1,177 @@ +// client/app/bundles/course/gradebook/computeWeighted.ts +import { + AssessmentData, + StudentData, + SubmissionData, + TabData, +} from 'types/course/gradebook'; + +export interface WeightedRow { + studentId: number; + name: string; + email: string; + externalId: string | null; + level: number; + totalXp: number; + subtotals: (number | null)[]; + total: number | null; +} + +type GradeLookup = Map; + +const gradeKey = (studentId: number, assessmentId: number): string => + `${studentId}:${assessmentId}`; + +// Index submissions by (student, assessment) once: O(submissions). +const buildGradeLookup = (submissions: SubmissionData[]): GradeLookup => { + const lookup: GradeLookup = new Map(); + submissions.forEach((s) => { + if (s.grade != null) + lookup.set(gradeKey(s.studentId, s.assessmentId), s.grade); + }); + return lookup; +}; + +// Group assessments by tab once: O(assessments). +const buildAssessmentsByTab = ( + assessments: AssessmentData[], +): Map => { + const byTab = new Map(); + assessments.forEach((a) => { + const list = byTab.get(a.tabId); + if (list) list.push(a); + else byTab.set(a.tabId, [a]); + }); + return byTab; +}; + +// Single source of truth for the subtotal math, operating on prebuilt indexes. +const subtotalFromLookup = ( + studentId: number, + tabAssessments: AssessmentData[] | undefined, + gradeLookup: GradeLookup, + treatUngradedAsZero: boolean, +): number | null => { + if (!tabAssessments || tabAssessments.length === 0) return null; + let numerator = 0; + let denominator = 0; + tabAssessments.forEach((a) => { + const grade = gradeLookup.get(gradeKey(studentId, a.id)); + if (grade != null) { + numerator += grade; + denominator += a.maxGrade; + } else if (treatUngradedAsZero) { + denominator += a.maxGrade; + } + }); + return denominator > 0 ? numerator / denominator : null; +}; + +// Weighted, additive total from already-computed subtotals. +const totalFromSubtotals = ( + subtotals: (number | null)[], + tabs: TabData[], +): number | null => { + let contributingCount = 0; + let total = 0; + subtotals.forEach((sub, i) => { + if (sub == null) return; + contributingCount += 1; + total += (tabs[i].gradebookWeight ?? 0) * sub; + }); + return contributingCount > 0 ? total : null; +}; + +interface SubtotalArgs { + studentId: number; + tab: TabData; + assessments: AssessmentData[]; + submissions: SubmissionData[]; + treatUngradedAsZero: boolean; +} + +export const computeTabSubtotal = ({ + studentId, + tab, + assessments, + submissions, + treatUngradedAsZero, +}: SubtotalArgs): number | null => + subtotalFromLookup( + studentId, + assessments.filter((a) => a.tabId === tab.id), + buildGradeLookup(submissions), + treatUngradedAsZero, + ); + +interface TotalArgs { + studentId: number; + tabs: TabData[]; + assessments: AssessmentData[]; + submissions: SubmissionData[]; + treatUngradedAsZero: boolean; +} + +export const computeStudentTotal = ({ + studentId, + tabs, + assessments, + submissions, + treatUngradedAsZero, +}: TotalArgs): number | null => { + const gradeLookup = buildGradeLookup(submissions); + const assessmentsByTab = buildAssessmentsByTab(assessments); + const subtotals = tabs.map((tab) => + subtotalFromLookup( + studentId, + assessmentsByTab.get(tab.id), + gradeLookup, + treatUngradedAsZero, + ), + ); + return totalFromSubtotals(subtotals, tabs); +}; + +interface WeightedRowsArgs { + students: StudentData[]; + tabs: TabData[]; + assessments: AssessmentData[]; + submissions: SubmissionData[]; + treatUngradedAsZero: boolean; +} + +// Batch entry point used by the table: builds the indexes ONCE and reuses them +// across every student, computing each subtotal a single time. +export const computeWeightedRows = ({ + students, + tabs, + assessments, + submissions, + treatUngradedAsZero, +}: WeightedRowsArgs): WeightedRow[] => { + const gradeLookup = buildGradeLookup(submissions); + const assessmentsByTab = buildAssessmentsByTab(assessments); + return students.map((student) => { + const subtotals = tabs.map((tab) => + subtotalFromLookup( + student.id, + assessmentsByTab.get(tab.id), + gradeLookup, + treatUngradedAsZero, + ), + ); + return { + studentId: student.id, + name: student.name, + email: student.email, + externalId: student.externalId, + level: student.level, + totalXp: student.totalXp, + subtotals, + total: totalFromSubtotals(subtotals, tabs), + }; + }); +}; + +export const sumWeights = (tabs: TabData[]): number => + tabs.reduce((acc, t) => acc + (t.gradebookWeight ?? 0), 0); diff --git a/client/app/bundles/course/gradebook/operations.ts b/client/app/bundles/course/gradebook/operations.ts index 35790580ed..ae2f962fbb 100644 --- a/client/app/bundles/course/gradebook/operations.ts +++ b/client/app/bundles/course/gradebook/operations.ts @@ -1,4 +1,5 @@ import type { Operation } from 'store'; +import type { UpdateWeightsPayload } from 'types/course/gradebook'; import CourseAPI from 'api/course'; @@ -9,4 +10,11 @@ const fetchGradebook = (): Operation => async (dispatch) => { dispatch(actions.saveGradebook(response.data)); }; +export const updateGradebookWeights = + (weights: UpdateWeightsPayload['weights']): Operation => + async (dispatch) => { + const response = await CourseAPI.gradebook.updateWeights({ weights }); + dispatch(actions.updateTabWeights(response.data)); + }; + export default fetchGradebook; diff --git a/client/app/bundles/course/gradebook/pages/GradebookIndex/index.tsx b/client/app/bundles/course/gradebook/pages/GradebookIndex/index.tsx index cc140af58f..3227d0e967 100644 --- a/client/app/bundles/course/gradebook/pages/GradebookIndex/index.tsx +++ b/client/app/bundles/course/gradebook/pages/GradebookIndex/index.tsx @@ -1,8 +1,8 @@ -import { FC, useEffect, useState } from 'react'; +import { FC, useEffect, useState, useTransition } from 'react'; import { defineMessages } from 'react-intl'; -import { useParams } from 'react-router-dom'; +import { useParams, useSearchParams } from 'react-router-dom'; import { PeopleAlt } from '@mui/icons-material'; -import { Typography } from '@mui/material'; +import { Tab, Tabs, Typography } from '@mui/material'; import Page from 'lib/components/core/layouts/Page'; import LoadingIndicator from 'lib/components/core/LoadingIndicator'; @@ -12,14 +12,18 @@ import useTranslation from 'lib/hooks/useTranslation'; import { useCourseContext } from '../../../container/CourseLoader'; import GradebookTable from '../../components/GradebookTable'; +import GradebookWeightedTable from '../../components/GradebookWeightedTable'; +import WeightedViewHint from '../../components/WeightedViewHint'; import fetchGradebook from '../../operations'; import { getAssessments, + getCanManageWeights, getCategories, getGamificationEnabled, getStudents, getSubmissions, getTabs, + getWeightedViewEnabled, } from '../../selectors'; const translations = defineMessages({ @@ -39,6 +43,14 @@ const translations = defineMessages({ id: 'course.gradebook.GradebookIndex.noStudentsHint', defaultMessage: 'Grades will appear here once students join the course.', }, + allAssessments: { + id: 'course.gradebook.GradebookIndex.allAssessments', + defaultMessage: 'All assessments', + }, + byWeight: { + id: 'course.gradebook.GradebookIndex.byWeight', + defaultMessage: 'By weight', + }, }); const GradebookIndex: FC = () => { @@ -47,7 +59,12 @@ const GradebookIndex: FC = () => { const { courseTitle } = useCourseContext(); const { courseId: courseIdParam } = useParams(); const courseId = parseInt(courseIdParam!, 10); + const [searchParams, setSearchParams] = useSearchParams(); const [isLoading, setIsLoading] = useState(true); + const [viewMode, setViewMode] = useState<'all' | 'weighted'>( + searchParams.get('view') === 'weighted' ? 'weighted' : 'all', + ); + const [isPending, startTransition] = useTransition(); const assessments = useAppSelector(getAssessments); const categories = useAppSelector(getCategories); @@ -55,6 +72,8 @@ const GradebookIndex: FC = () => { const students = useAppSelector(getStudents); const submissions = useAppSelector(getSubmissions); const gamificationEnabled = useAppSelector(getGamificationEnabled); + const weightedViewEnabled = useAppSelector(getWeightedViewEnabled); + const canManageWeights = useAppSelector(getCanManageWeights); useEffect(() => { dispatch(fetchGradebook()) @@ -77,6 +96,20 @@ const GradebookIndex: FC = () => { ); + } else if (weightedViewEnabled && viewMode === 'weighted') { + content = ( + + ); } else { content = ( { return ( - {content} + {!isLoading && canManageWeights && !weightedViewEnabled && ( + + )} + {weightedViewEnabled && !isLoading && students.length > 0 && ( + + startTransition(() => { + setViewMode(v); + setSearchParams(v === 'weighted' ? { view: 'weighted' } : {}); + }) + } + TabIndicatorProps={{ style: { height: 2 } }} + value={viewMode} + > + + + + )} +
+ {isPending && ( +
+ +
+ )} +
+ {content} +
+
); }; diff --git a/client/app/bundles/course/gradebook/selectors.ts b/client/app/bundles/course/gradebook/selectors.ts index 34857cf27c..dafb792c6c 100644 --- a/client/app/bundles/course/gradebook/selectors.ts +++ b/client/app/bundles/course/gradebook/selectors.ts @@ -24,3 +24,7 @@ export const getGamificationEnabled = ( getLocalState(state).gamificationEnabled; export const getCurrentUserId = (state: AppState): GradebookState['userId'] => getLocalState(state).userId; +export const getWeightedViewEnabled = (state: AppState): boolean => + getLocalState(state).weightedViewEnabled; +export const getCanManageWeights = (state: AppState): boolean => + getLocalState(state).canManageWeights; diff --git a/client/app/bundles/course/gradebook/store.ts b/client/app/bundles/course/gradebook/store.ts index f386b9a7fc..4046f3b653 100644 --- a/client/app/bundles/course/gradebook/store.ts +++ b/client/app/bundles/course/gradebook/store.ts @@ -1,5 +1,8 @@ import { produce } from 'immer'; -import type { GradebookData } from 'types/course/gradebook'; +import type { + GradebookData, + UpdateWeightsPayload, +} from 'types/course/gradebook'; import type { AssessmentData, @@ -10,6 +13,7 @@ import type { } from './types'; const SAVE_GRADEBOOK = 'course/gradebook/SAVE_GRADEBOOK'; +const UPDATE_TAB_WEIGHTS = 'course/gradebook/UPDATE_TAB_WEIGHTS'; interface GradebookState { categories: CategoryData[]; @@ -19,6 +23,8 @@ interface GradebookState { submissions: SubmissionData[]; gamificationEnabled: boolean; userId: number; + weightedViewEnabled: boolean; + canManageWeights: boolean; } interface SaveGradebookAction { @@ -26,6 +32,11 @@ interface SaveGradebookAction { payload: GradebookData; } +interface UpdateTabWeightsAction { + type: typeof UPDATE_TAB_WEIGHTS; + payload: UpdateWeightsPayload; +} + const initialState: GradebookState = { categories: [], tabs: [], @@ -34,10 +45,15 @@ const initialState: GradebookState = { submissions: [], gamificationEnabled: false, userId: 0, + weightedViewEnabled: false, + canManageWeights: false, }; const reducer = produce( - (draft: GradebookState, action: SaveGradebookAction) => { + ( + draft: GradebookState, + action: SaveGradebookAction | UpdateTabWeightsAction, + ) => { switch (action.type) { case SAVE_GRADEBOOK: { draft.categories = action.payload.categories; @@ -47,6 +63,17 @@ const reducer = produce( draft.submissions = action.payload.submissions; draft.gamificationEnabled = action.payload.gamificationEnabled; draft.userId = action.payload.userId ?? 0; + draft.weightedViewEnabled = action.payload.weightedViewEnabled; + draft.canManageWeights = action.payload.canManageWeights; + break; + } + case UPDATE_TAB_WEIGHTS: { + action.payload.weights.forEach(({ tabId, weight }) => { + const tab = draft.tabs.find((t) => t.id === tabId); + if (tab) { + tab.gradebookWeight = weight; + } + }); break; } default: @@ -61,6 +88,12 @@ export const actions = { type: SAVE_GRADEBOOK, payload: data, }), + updateTabWeights: ( + payload: UpdateWeightsPayload, + ): UpdateTabWeightsAction => ({ + type: UPDATE_TAB_WEIGHTS, + payload, + }), }; export default reducer; diff --git a/client/app/bundles/course/gradebook/types.ts b/client/app/bundles/course/gradebook/types.ts index f94aa7bf9c..b91689df87 100644 --- a/client/app/bundles/course/gradebook/types.ts +++ b/client/app/bundles/course/gradebook/types.ts @@ -5,4 +5,5 @@ export type { StudentData, SubmissionData, TabData, + UpdateWeightsPayload, } from 'types/course/gradebook'; diff --git a/client/app/lib/components/table/MuiTableAdapter/MuiColumnPickerDialog.tsx b/client/app/lib/components/table/MuiTableAdapter/MuiColumnPickerDialog.tsx deleted file mode 100644 index 388eed3b93..0000000000 --- a/client/app/lib/components/table/MuiTableAdapter/MuiColumnPickerDialog.tsx +++ /dev/null @@ -1,152 +0,0 @@ -import { useEffect, useState } from 'react'; -import { defineMessages } from 'react-intl'; -import { - Alert, - Button, - Dialog, - DialogActions, - DialogContent, - DialogTitle, -} from '@mui/material'; - -import useTranslation from 'lib/hooks/useTranslation'; - -import { ColumnPickerTemplate } from '../builder'; - -const translations = defineMessages({ - defaultTitle: { - id: 'lib.components.table.MuiColumnPickerDialog.defaultTitle', - defaultMessage: 'Select columns', - }, - apply: { - id: 'lib.components.table.MuiColumnPickerDialog.apply', - defaultMessage: 'Apply to view', - }, - cancel: { - id: 'lib.components.table.MuiColumnPickerDialog.cancel', - defaultMessage: 'Cancel', - }, - defaultExport: { - id: 'lib.components.table.MuiColumnPickerDialog.export', - defaultMessage: 'Apply and Export', - }, -}); - -interface MuiColumnPickerDialogProps { - open: boolean; - onClose: () => void; - initialVisibility: Record; - locked?: string[]; - columnPicker: ColumnPickerTemplate; - commitColumnVisibility: (next: Record) => void; - onExportFromPicker?: (visibility: Record) => void; -} - -const enforceLockedLocal = ( - next: Record, - locked: string[] | undefined, -): Record => { - if (!locked || locked.length === 0) return next; - const enforced = { ...next }; - locked.forEach((id) => { - enforced[id] = true; - }); - return enforced; -}; - -const MuiColumnPickerDialog = ({ - open, - onClose, - initialVisibility, - locked, - columnPicker, - commitColumnVisibility, - onExportFromPicker, -}: MuiColumnPickerDialogProps): JSX.Element => { - const { t } = useTranslation(); - const [staged, setStaged] = useState>(() => - enforceLockedLocal({ ...initialVisibility }, locked), - ); - - const dataColumnIds = columnPicker.dataColumnIds; - const hasDataColumns = - !dataColumnIds || - dataColumnIds.length === 0 || - dataColumnIds.some((id) => staged[id]); - - useEffect(() => { - if (open) { - setStaged(enforceLockedLocal({ ...initialVisibility }, locked)); - } - }, [open, initialVisibility, locked]); - - const ctx = { - isVisible: (id: string): boolean => staged[id] ?? false, - setVisible: (id: string, v: boolean): void => { - if (locked?.includes(id)) return; - setStaged((prev) => - Object.hasOwn(prev, id) ? { ...prev, [id]: v } : prev, - ); - }, - setManyVisible: (ids: string[], v: boolean): void => { - setStaged((prev) => { - const next = { ...prev }; - let changed = false; - ids.forEach((id) => { - if (!Object.hasOwn(next, id)) return; - if (locked?.includes(id)) return; - if (next[id] !== v) { - next[id] = v; - changed = true; - } - }); - return changed ? next : prev; - }); - }, - }; - - const commitAndClose = (): void => { - commitColumnVisibility(enforceLockedLocal(staged, locked)); - onClose(); - }; - - const cancelAndClose = (): void => { - onClose(); - }; - - const exportAndClose = (): void => { - const enforced = enforceLockedLocal(staged, locked); - commitColumnVisibility(enforced); - onExportFromPicker?.(enforced); - onClose(); - }; - - return ( - - - {columnPicker.dialogTitle ?? t(translations.defaultTitle)} - - {columnPicker.renderTree(ctx)} - {!hasDataColumns && columnPicker.noDataColumnsHint && ( - - {columnPicker.noDataColumnsHint} - - )} - - - - - - - ); -}; - -export default MuiColumnPickerDialog; diff --git a/client/app/lib/components/table/__tests__/MuiColumnPickerDialog.test.tsx b/client/app/lib/components/table/__tests__/MuiColumnPickerDialog.test.tsx deleted file mode 100644 index f1cac33842..0000000000 --- a/client/app/lib/components/table/__tests__/MuiColumnPickerDialog.test.tsx +++ /dev/null @@ -1,269 +0,0 @@ -import { IntlProvider } from 'react-intl'; -import { fireEvent, render, screen } from '@testing-library/react'; -import userEvent from '@testing-library/user-event'; - -import { ColumnPickerRenderCtx } from '../builder'; -import MuiColumnPickerDialog from '../MuiTableAdapter/MuiColumnPickerDialog'; - -const DIALOG_TITLE = 'Select columns'; - -const wrap = (node: JSX.Element): JSX.Element => ( - - {node} - -); - -const makeRenderTree = (ids: readonly string[]): jest.Mock => - jest.fn((ctx: ColumnPickerRenderCtx) => ( - <> - {ids.map((id) => ( - - ))} - - )); - -const setup = ( - overrides: Partial> = {}, -): ReturnType & { - commitColumnVisibility: jest.Mock; - onExportFromPicker: jest.Mock; - renderTree: jest.Mock; - props: React.ComponentProps; -} => { - const commitColumnVisibility = jest.fn(); - const onExportFromPicker = jest.fn(); - const renderTree = makeRenderTree(['name', 'email']); - const props = { - open: true, - onClose: jest.fn(), - initialVisibility: { name: true, email: true }, - locked: ['name'], - columnPicker: { - renderTree, - dialogTitle: DIALOG_TITLE, - exportLabel: 'Export CSV', - onExport: 'csv' as const, - }, - commitColumnVisibility, - onExportFromPicker, - ...overrides, - }; - return { - ...render(wrap()), - commitColumnVisibility, - onExportFromPicker, - renderTree, - props, - }; -}; - -describe('MuiColumnPickerDialog', () => { - it('renders the dialog title', () => { - setup(); - expect(screen.getByText(DIALOG_TITLE)).toBeInTheDocument(); - }); - - it('Apply commits staged changes and closes', async () => { - const user = userEvent.setup(); - const { commitColumnVisibility, props } = setup(); - await user.click(screen.getByLabelText('email')); - await user.click(screen.getByRole('button', { name: /apply/i })); - - expect(commitColumnVisibility).toHaveBeenCalledWith({ - name: true, - email: false, - }); - expect(props.onClose).toHaveBeenCalled(); - }); - - it('Cancel discards staged and closes without commit', async () => { - const user = userEvent.setup(); - const { commitColumnVisibility, props } = setup(); - await user.click(screen.getByLabelText('email')); - await user.click(screen.getByRole('button', { name: /cancel/i })); - - expect(commitColumnVisibility).not.toHaveBeenCalled(); - expect(props.onClose).toHaveBeenCalled(); - }); - - it('Export CSV commits + invokes onExportFromPicker + closes', async () => { - const user = userEvent.setup(); - const { commitColumnVisibility, onExportFromPicker, props } = setup(); - await user.click(screen.getByLabelText('email')); - await user.click(screen.getByRole('button', { name: /export csv/i })); - - expect(commitColumnVisibility).toHaveBeenCalledWith({ - name: true, - email: false, - }); - expect(onExportFromPicker).toHaveBeenCalledWith({ - name: true, - email: false, - }); - expect(props.onClose).toHaveBeenCalled(); - }); - - it('locked id forcibly restored to true on commit even if staged false', async () => { - const user = userEvent.setup(); - const { commitColumnVisibility } = setup({ - initialVisibility: { name: false, email: true }, // malformed input - }); - - await user.click(screen.getByRole('button', { name: /apply/i })); - - expect(commitColumnVisibility).toHaveBeenCalledWith({ - name: true, - email: true, - }); - }); - - describe('locked column behavior', () => { - const makeGroupRenderTree = (ids: readonly string[]): jest.Mock => - jest.fn( - (ctx: ColumnPickerRenderCtx): JSX.Element => ( - <> - - - - ), - ); - - it('deselect-all leaves the locked column checked', async () => { - const user = userEvent.setup(); - const commitColumnVisibility = jest.fn(); - render( - wrap( - , - ), - ); - await user.click(screen.getByRole('button', { name: 'Deselect all' })); - await user.click(screen.getByRole('button', { name: /apply/i })); - expect(commitColumnVisibility).toHaveBeenCalledWith({ - name: true, - email: false, - }); - }); - - it('select-all from indeterminate state selects non-locked column', async () => { - const user = userEvent.setup(); - const commitColumnVisibility = jest.fn(); - render( - wrap( - , - ), - ); - await user.click(screen.getByRole('button', { name: 'Select all' })); - await user.click(screen.getByRole('button', { name: /apply/i })); - expect(commitColumnVisibility).toHaveBeenCalledWith({ - name: true, - email: true, - }); - }); - - it('clicking a locked column checkbox has no effect on its visibility', async () => { - const user = userEvent.setup(); - const { commitColumnVisibility } = setup(); - await user.click(screen.getByLabelText('name')); - await user.click(screen.getByRole('button', { name: /apply/i })); - expect(commitColumnVisibility).toHaveBeenCalledWith({ - name: true, - email: true, - }); - }); - }); - - it('Esc key dismisses without committing', () => { - const { commitColumnVisibility, props } = setup(); - fireEvent.keyDown(screen.getByRole('dialog'), { - key: 'Escape', - code: 'Escape', - }); - expect(props.onClose).toHaveBeenCalled(); - expect(commitColumnVisibility).not.toHaveBeenCalled(); - }); - - describe('noDataColumnsHint', () => { - const dataSetup = ( - dataColumnIds: string[], - initialVisibility: Record, - ): ReturnType => - setup({ - initialVisibility, - columnPicker: { - renderTree: makeRenderTree(['name', 'grade']), - dialogTitle: DIALOG_TITLE, - exportLabel: 'Export CSV', - onExport: 'csv' as const, - dataColumnIds, - noDataColumnsHint: 'No grade columns selected.', - }, - }); - - it('shows hint when no data columns are selected', () => { - dataSetup(['grade'], { name: true, grade: false }); - expect( - screen.getByText('No grade columns selected.'), - ).toBeInTheDocument(); - }); - - it('hides hint when at least one data column is selected', () => { - dataSetup(['grade'], { name: true, grade: true }); - expect( - screen.queryByText('No grade columns selected.'), - ).not.toBeInTheDocument(); - }); - - it('Export button is enabled even when no data columns are selected', () => { - dataSetup(['grade'], { name: true, grade: false }); - expect( - screen.getByRole('button', { name: /export csv/i }), - ).not.toBeDisabled(); - }); - - it('Apply button is enabled even when no data columns are selected', () => { - dataSetup(['grade'], { name: true, grade: false }); - expect(screen.getByRole('button', { name: /apply/i })).not.toBeDisabled(); - }); - }); -}); diff --git a/client/app/lib/containers/AppContainer/AppContainer.tsx b/client/app/lib/containers/AppContainer/AppContainer.tsx index 817c6e3ce4..f51baac146 100644 --- a/client/app/lib/containers/AppContainer/AppContainer.tsx +++ b/client/app/lib/containers/AppContainer/AppContainer.tsx @@ -1,6 +1,9 @@ +import { useEffect } from 'react'; import { Outlet, useRouteError } from 'react-router-dom'; +import { actions } from 'bundles/users/store'; import NotificationPopup from 'lib/containers/NotificationPopup'; +import { useAppDispatch } from 'lib/hooks/store'; import { loader, useAppLoader } from './AppLoader'; import GlobalAnnouncements from './GlobalAnnouncements'; @@ -9,6 +12,19 @@ import ServerUnreachableBanner from './ServerUnreachableBanner'; const AppContainer = (): JSX.Element => { const data = useAppLoader(); const homeData = data.home; + const dispatch = useAppDispatch(); + + useEffect(() => { + if (homeData.user) { + dispatch( + actions.saveUser({ + id: homeData.user.id, + name: homeData.user.name, + imageUrl: homeData.user.avatarUrl, + }), + ); + } + }, [homeData.user?.id]); return (
diff --git a/client/app/lib/hooks/__tests__/useDismissibleOnce.test.tsx b/client/app/lib/hooks/__tests__/useDismissibleOnce.test.tsx new file mode 100644 index 0000000000..7764533c43 --- /dev/null +++ b/client/app/lib/hooks/__tests__/useDismissibleOnce.test.tsx @@ -0,0 +1,62 @@ +import { act, renderHook } from '@testing-library/react'; + +import useDismissibleOnce from '../useDismissibleOnce'; + +const KEY = 'sample_hint'; + +beforeEach(() => localStorage.clear()); + +describe('useDismissibleOnce', () => { + it('starts not dismissed when nothing is persisted', () => { + const { result } = renderHook(() => useDismissibleOnce(KEY, 42)); + expect(result.current.dismissed).toBe(false); + }); + + it('dismiss() flips state and persists under the user-namespaced key', () => { + const { result } = renderHook(() => useDismissibleOnce(KEY, 42)); + + act(() => result.current.dismiss()); + + expect(result.current.dismissed).toBe(true); + expect(localStorage.getItem(`42:${KEY}`)).toBe('true'); + }); + + it('reads the persisted value on mount', () => { + localStorage.setItem(`42:${KEY}`, 'true'); + const { result } = renderHook(() => useDismissibleOnce(KEY, 42)); + expect(result.current.dismissed).toBe(true); + }); + + it('isolates dismissal per user (one user dismissing does not affect another)', () => { + localStorage.setItem(`42:${KEY}`, 'true'); + + const userB = renderHook(() => useDismissibleOnce(KEY, 99)); + + expect(userB.result.current.dismissed).toBe(false); + }); + + it('is a safe no-op when userId is absent', () => { + const { result } = renderHook(() => useDismissibleOnce(KEY, undefined)); + + expect(result.current.dismissed).toBe(false); + act(() => result.current.dismiss()); + expect(result.current.dismissed).toBe(true); + // Nothing written without a real user id. + expect(localStorage.getItem(KEY)).toBeNull(); + }); + + it('does not throw if persistence fails (quota/security error)', () => { + const spy = jest + .spyOn(Storage.prototype, 'setItem') + .mockImplementation(() => { + throw new DOMException('QuotaExceededError'); + }); + + const { result } = renderHook(() => useDismissibleOnce(KEY, 42)); + + expect(() => act(() => result.current.dismiss())).not.toThrow(); + expect(result.current.dismissed).toBe(true); + + spy.mockRestore(); + }); +}); diff --git a/client/app/lib/hooks/useDismissibleOnce.ts b/client/app/lib/hooks/useDismissibleOnce.ts new file mode 100644 index 0000000000..bdfb051689 --- /dev/null +++ b/client/app/lib/hooks/useDismissibleOnce.ts @@ -0,0 +1,48 @@ +import { useState } from 'react'; + +interface DismissibleOnce { + dismissed: boolean; + dismiss: () => void; +} + +/** + * Tracks whether a one-time, dismissable UI element (e.g. an onboarding hint) has + * been dismissed, persisting the choice in localStorage so it never reappears. + * + * The key is namespaced by userId (`${userId}:${key}`) when provided, so two users + * on the same device don't share dismissals. Persistence degrades to in-session-only + * state if userId is absent or if localStorage is unavailable (private browsing / + * quota), and never throws. + */ +const useDismissibleOnce = ( + key: string, + userId: number | undefined, +): DismissibleOnce => { + const storageKey = + userId !== undefined && userId > 0 ? `${userId}:${key}` : undefined; + + const [dismissed, setDismissed] = useState(() => { + if (!storageKey) return false; + try { + return localStorage.getItem(storageKey) === 'true'; + } catch { + return false; + } + }); + + const dismiss = (): void => { + setDismissed(true); + if (!storageKey) return; + try { + localStorage.setItem(storageKey, 'true'); + } catch { + // setItem throws QuotaExceededError (storage full) or SecurityError (private + // browsing on some browsers). Dismissal is best-effort; the current session is + // unaffected if it fails — the hint just reappears on the next visit. + } + }; + + return { dismissed, dismiss }; +}; + +export default useDismissibleOnce; diff --git a/client/app/types/course/gradebook.ts b/client/app/types/course/gradebook.ts index 67dad1bed1..e590e3a5f3 100644 --- a/client/app/types/course/gradebook.ts +++ b/client/app/types/course/gradebook.ts @@ -7,6 +7,7 @@ export interface TabData { id: number; title: string; categoryId: number; + gradebookWeight?: number; } export interface AssessmentData { @@ -39,4 +40,10 @@ export interface GradebookData { submissions: SubmissionData[]; gamificationEnabled: boolean; userId?: number; + weightedViewEnabled: boolean; + canManageWeights: boolean; +} + +export interface UpdateWeightsPayload { + weights: { tabId: number; weight: number }[]; } diff --git a/client/jest.config.js b/client/jest.config.js index 947fb08889..38d6c43835 100644 --- a/client/jest.config.js +++ b/client/jest.config.js @@ -24,6 +24,7 @@ const config = { '^course(.*)$': '/app/bundles/course$1', '^store(.*)$': '/app/store$1', '^lodash-es(.*)$': 'lodash$1', + 'compiled-locales/.*\\.json$': '/app/__test__/mocks/localeMock.js', }, testPathIgnorePatterns: ['/node_modules/', '/dist/'], coveragePathIgnorePatterns: ['/node_modules/', '/__test__/'], diff --git a/client/locales/en.json b/client/locales/en.json index fa547608bd..9621d458ad 100644 --- a/client/locales/en.json +++ b/client/locales/en.json @@ -9223,5 +9223,80 @@ }, "users.troubleSigningIn": { "defaultMessage": "Trouble signing in?" + }, + "course.admin.GradebookSettings.gradebookSettings": { + "defaultMessage": "Gradebook settings" + }, + "course.admin.GradebookSettings.weightedViewEnabled": { + "defaultMessage": "Enable weighted grade view" + }, + "course.admin.GradebookSettings.weightedViewEnabledHint": { + "defaultMessage": "Enables a \"By weight\" view in the gradebook where staff can configure per-tab weights and see a weighted Total column." + }, + "course.gradebook.ConfigureWeightsDialog.cancel": { + "defaultMessage": "Cancel" + }, + "course.gradebook.ConfigureWeightsDialog.description": { + "defaultMessage": "Set how much each tab contributes to the total grade. Weights should sum to 100." + }, + "course.gradebook.ConfigureWeightsDialog.dialogTitle": { + "defaultMessage": "Configure tab weights" + }, + "course.gradebook.ConfigureWeightsDialog.save": { + "defaultMessage": "Save" + }, + "course.gradebook.ConfigureWeightsDialog.total": { + "defaultMessage": "Total: {sum}%" + }, + "course.gradebook.ConfigureWeightsDialog.valueTooHigh": { + "defaultMessage": "Value must be at most 100" + }, + "course.gradebook.ConfigureWeightsDialog.valueTooLow": { + "defaultMessage": "Value must be at least 0" + }, + "course.gradebook.ConfigureWeightsDialog.weightsDoNotSum": { + "defaultMessage": "Weights do not sum to 100. Saving is allowed; Total may be inaccurate." + }, + "course.gradebook.GradebookIndex.allAssessments": { + "defaultMessage": "All assessments" + }, + "course.gradebook.GradebookIndex.byWeight": { + "defaultMessage": "By weight" + }, + "course.gradebook.GradebookTable.noDataColumnsHintWithGamification": { + "defaultMessage": "No grade or gamification columns selected - export will include student info only." + }, + "course.gradebook.GradebookWeightedTable.configureWeights": { + "defaultMessage": "Configure Weights" + }, + "course.gradebook.GradebookWeightedTable.doesNotSumTo100": { + "defaultMessage": "Does not sum to 100" + }, + "course.gradebook.GradebookWeightedTable.noWeightsConfigured": { + "defaultMessage": "No weights configured - all tab weights are 0. Use \"Configure Weights\" to assign weights." + }, + "course.gradebook.GradebookWeightedTable.percentOfGrade": { + "defaultMessage": "{weight}% of grade" + }, + "course.gradebook.GradebookWeightedTable.percentTotalExact": { + "defaultMessage": "100% total" + }, + "course.gradebook.GradebookWeightedTable.percentTotalWarning": { + "defaultMessage": "{weight}% total" + }, + "course.gradebook.GradebookWeightedTable.student": { + "defaultMessage": "Student" + }, + "course.gradebook.GradebookWeightedTable.total": { + "defaultMessage": "Total" + }, + "course.gradebook.GradebookWeightedTable.treatUngradedAsZero": { + "defaultMessage": "Treat Ungraded as 0" + }, + "course.gradebook.GradebookWeightedTable.treatUngradedAsZeroTooltip": { + "defaultMessage": "Counts unsubmitted and ungraded assessments as 0 in the calculation. Use at end of course when all work should be complete." + }, + "course.gradebook.GradebookWeightedTable.weightsDoNotSum": { + "defaultMessage": "Weights do not sum to 100. Total may be inaccurate." } } diff --git a/client/locales/ko.json b/client/locales/ko.json index 885f3bc67d..7385e36d3b 100644 --- a/client/locales/ko.json +++ b/client/locales/ko.json @@ -5405,6 +5405,12 @@ "course.gradebook.GradebookIndex.noStudents": { "defaultMessage": "학생 없음" }, + "course.gradebook.GradebookWeightedTable.doesNotSumTo100": { + "defaultMessage": "합계가 100이 아닙니다" + }, + "course.gradebook.GradebookWeightedTable.treatUngradedAsZeroTooltip": { + "defaultMessage": "제출되지 않았거나 채점되지 않은 과제/평가를 0점으로 계산합니다. 모든 과제가 완료되어야 하는 학기 말에 사용하세요." + }, "course.gradebook.GradebookTable.maxMarks": { "defaultMessage": "최고 점수" }, diff --git a/client/locales/zh.json b/client/locales/zh.json index 1802191e8a..bd4a78b680 100644 --- a/client/locales/zh.json +++ b/client/locales/zh.json @@ -5399,6 +5399,12 @@ "course.gradebook.GradebookIndex.noStudents": { "defaultMessage": "无学生" }, + "course.gradebook.GradebookWeightedTable.doesNotSumTo100": { + "defaultMessage": "未合计为 100" + }, + "course.gradebook.GradebookWeightedTable.treatUngradedAsZeroTooltip": { + "defaultMessage": "将未提交和未评分的作业/评估计为 0 来计算。请在课程结束时、所有作业都应已完成的情况下使用。" + }, "course.gradebook.GradebookTable.maxMarks": { "defaultMessage": "最高分" }, From 0f94700545b65a77979308bf5d5a9eacd9bdf650 Mon Sep 17 00:00:00 2001 From: lws49 Date: Wed, 10 Jun 2026 17:21:52 +0800 Subject: [PATCH 09/26] feat(gradebook): add weight_mode and assessment gradebook_weight columns --- ...0000_add_weight_mode_and_assessment_gradebook_weight.rb | 7 +++++++ db/schema.rb | 6 ++++-- 2 files changed, 11 insertions(+), 2 deletions(-) create mode 100644 db/migrate/20260610120000_add_weight_mode_and_assessment_gradebook_weight.rb diff --git a/db/migrate/20260610120000_add_weight_mode_and_assessment_gradebook_weight.rb b/db/migrate/20260610120000_add_weight_mode_and_assessment_gradebook_weight.rb new file mode 100644 index 0000000000..a8b813f905 --- /dev/null +++ b/db/migrate/20260610120000_add_weight_mode_and_assessment_gradebook_weight.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true +class AddWeightModeAndAssessmentGradebookWeight < ActiveRecord::Migration[7.2] + def change + add_column :course_assessment_tabs, :weight_mode, :integer, null: false, default: 0 + add_column :course_assessments, :gradebook_weight, :decimal, precision: 5, scale: 2, null: true + end +end diff --git a/db/schema.rb b/db/schema.rb index fc7625afdf..52ea82f857 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_05_28_084849) do +ActiveRecord::Schema[7.2].define(version: 2026_06_10_120000) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" enable_extension "uuid-ossp" @@ -564,7 +564,8 @@ t.integer "updater_id", null: false t.datetime "created_at", precision: nil, null: false t.datetime "updated_at", precision: nil, null: false - t.integer "gradebook_weight", default: 0, null: false + t.decimal "gradebook_weight", precision: 5, scale: 2, default: "0.0", null: false + t.integer "weight_mode", default: 0, null: false t.index ["category_id"], name: "fk__course_assessment_tabs_category_id" t.index ["creator_id"], name: "fk__course_assessment_tabs_creator_id" t.index ["updater_id"], name: "fk__course_assessment_tabs_updater_id" @@ -601,6 +602,7 @@ t.boolean "show_rubric_to_students" t.uuid "ssid_folder_id" t.integer "linkable_tree_id", default: 0, null: false + t.decimal "gradebook_weight", precision: 5, scale: 2 t.index ["creator_id"], name: "fk__course_assessments_creator_id" t.index ["linkable_tree_id"], name: "index_course_assessments_on_linkable_tree_id" t.index ["monitor_id"], name: "index_course_assessments_on_monitor_id" From 163e8121db109a0661f0c5fa75454e9a4b47e2ce Mon Sep 17 00:00:00 2001 From: lws49 Date: Wed, 10 Jun 2026 17:27:41 +0800 Subject: [PATCH 10/26] feat(gradebook): persist weight mode + per-assessment weights with sum gate --- app/models/course/assessment.rb | 1 + app/models/course/assessment/tab.rb | 56 +++++++++++++++++------ config/locales/en/activerecord/errors.yml | 1 + spec/models/course/assessment/tab_spec.rb | 54 +++++++++++++++++++++- 4 files changed, 95 insertions(+), 17 deletions(-) diff --git a/app/models/course/assessment.rb b/app/models/course/assessment.rb index aec93f9de0..8b42700442 100644 --- a/app/models/course/assessment.rb +++ b/app/models/course/assessment.rb @@ -29,6 +29,7 @@ class Course::Assessment < ApplicationRecord validates :creator, presence: true validates :updater, presence: true validates :tab, presence: true + validates :gradebook_weight, numericality: { greater_than_or_equal_to: 0 }, allow_nil: true validates :ssid_folder_id, uniqueness: { if: :ssid_folder_id_changed? }, allow_nil: true belongs_to :tab, inverse_of: :assessments diff --git a/app/models/course/assessment/tab.rb b/app/models/course/assessment/tab.rb index 4013558f34..849a893aed 100644 --- a/app/models/course/assessment/tab.rb +++ b/app/models/course/assessment/tab.rb @@ -3,8 +3,7 @@ class Course::Assessment::Tab < ApplicationRecord validates :title, length: { maximum: 255 }, presence: true validates :weight, numericality: { only_integer: true }, presence: true validates :gradebook_weight, - numericality: { only_integer: true, - greater_than_or_equal_to: 0, + numericality: { greater_than_or_equal_to: 0, less_than_or_equal_to: 100 }, presence: true validates :creator, presence: true @@ -13,6 +12,8 @@ class Course::Assessment::Tab < ApplicationRecord belongs_to :category, class_name: 'Course::Assessment::Category', inverse_of: :tabs has_many :assessments, class_name: 'Course::Assessment', dependent: :destroy, inverse_of: :tab + + enum :weight_mode, { equal: 0, custom: 1 } has_many :folders, class_name: 'Course::Material::Folder', through: :assessments, inverse_of: nil @@ -29,29 +30,54 @@ class Course::Assessment::Tab < ApplicationRecord select('(array_agg(title))[0:3]') end) - # Bulk-updates the gradebook_weight for a set of tabs belonging to the given course. - # Raises ActiveRecord::RecordNotFound if any tab_id does not belong to the course. - # Raises ActiveRecord::RecordInvalid if any weight fails validation; the transaction is rolled back. + # Bulk-updates gradebook weights, weight modes, and per-assessment weights for a set + # of tabs belonging to the given course. + # Raises ActiveRecord::RecordNotFound if any tab_id or assessment_id is unknown. + # Raises ActiveRecord::RecordInvalid if validation fails or, for custom tabs, the + # assessment weights do not sum (at 2dp) to the tab total; the transaction is rolled back. # # @param course [Course] - # @param updates [Array] array of { tab_id: Integer, weight: Integer } + # @param updates [Array] each { tab_id:, weight:, weight_mode:, + # assessment_weights: [{ assessment_id:, weight: }] } def self.update_gradebook_weights(course:, updates:) course_tab_ids = course.assessment_tabs.pluck(:id).to_set - tab_ids_to_update = updates.map { |e| e[:tab_id] } + updates.each { |e| raise ActiveRecord::RecordNotFound unless course_tab_ids.include?(e[:tab_id]) } + + tabs_by_id = where(id: updates.map { |e| e[:tab_id] }).includes(:assessments).index_by(&:id) + + transaction { updates.each { |entry| apply_gradebook_weight_entry(tabs_by_id, entry) } } + end - tab_ids_to_update.each do |tab_id| - raise ActiveRecord::RecordNotFound unless course_tab_ids.include?(tab_id) + # @api private + def self.apply_gradebook_weight_entry(tabs_by_id, entry) + tab = tabs_by_id[entry[:tab_id]] + mode = (entry[:weight_mode] || 'equal').to_s + tab.update!(gradebook_weight: entry[:weight], weight_mode: mode) + + if mode == 'custom' + apply_custom_assessment_weights(tab, entry) + else + tab.assessments.update_all(gradebook_weight: nil) end + end + private_class_method :apply_gradebook_weight_entry - tabs_by_id = where(id: tab_ids_to_update).index_by(&:id) + # @api private + def self.apply_custom_assessment_weights(tab, entry) # rubocop:disable Metrics/AbcSize + assessments_by_id = tab.assessments.index_by(&:id) + sum = (entry[:assessment_weights] || []).sum do |aw| + assessment = assessments_by_id[aw[:assessment_id]] + raise ActiveRecord::RecordNotFound if assessment.nil? - transaction do - updates.each do |entry| - tab = tabs_by_id[entry[:tab_id]] - tab.update!(gradebook_weight: entry[:weight]) - end + assessment.update!(gradebook_weight: aw[:weight]) + aw[:weight] end + return unless (sum * 100).round != (entry[:weight] * 100).round + + tab.errors.add(:base, :custom_weights_mismatch) + raise ActiveRecord::RecordInvalid, tab end + private_class_method :apply_custom_assessment_weights # Returns a boolean value indicating if there are other tabs # besides this one remaining in its category. diff --git a/config/locales/en/activerecord/errors.yml b/config/locales/en/activerecord/errors.yml index 95f0da1660..713858a6dc 100644 --- a/config/locales/en/activerecord/errors.yml +++ b/config/locales/en/activerecord/errors.yml @@ -97,6 +97,7 @@ en: autograded_no_partial_answer: 'There are updated answers which have not been re-submitted yet. Please re-submit all answers before finalising your submission.' course/assessment/tab: deletion: 'the last tab cannot be deleted' + custom_weights_mismatch: 'Custom assessment weights must sum to the tab total' course/condition: attributes: conditional: diff --git a/spec/models/course/assessment/tab_spec.rb b/spec/models/course/assessment/tab_spec.rb index b546671032..d843357ffd 100644 --- a/spec/models/course/assessment/tab_spec.rb +++ b/spec/models/course/assessment/tab_spec.rb @@ -43,9 +43,9 @@ expect(tab).not_to be_valid end - it 'rejects non-integer' do + it 'accepts decimal values' do tab.gradebook_weight = 50.5 - expect(tab).not_to be_valid + expect(tab).to be_valid end it 'rejects nil' do @@ -94,5 +94,55 @@ end.to raise_error(ActiveRecord::RecordNotFound) end end + + describe '.update_gradebook_weights with modes' do + let(:course) { create(:course) } + let(:category) { create(:course_assessment_category, course: course) } + let(:tab) { create(:course_assessment_tab, category: category) } + let!(:a1) { create(:assessment, course: course, tab: tab) } + let!(:a2) { create(:assessment, course: course, tab: tab) } + + it 'persists custom mode and assessment weights when they sum to the tab total' do + described_class.update_gradebook_weights( + course: course, + updates: [{ + tab_id: tab.id, weight: 50.0, weight_mode: 'custom', + assessment_weights: [ + { assessment_id: a1.id, weight: 30.0 }, + { assessment_id: a2.id, weight: 20.0 } + ] + }] + ) + expect(tab.reload.weight_mode).to eq('custom') + expect(a1.reload.gradebook_weight).to eq(30.0) + expect(a2.reload.gradebook_weight).to eq(20.0) + end + + it 'raises RecordInvalid when custom assessment weights do not sum to the tab total' do + expect do + described_class.update_gradebook_weights( + course: course, + updates: [{ + tab_id: tab.id, weight: 50.0, weight_mode: 'custom', + assessment_weights: [ + { assessment_id: a1.id, weight: 30.0 }, + { assessment_id: a2.id, weight: 5.0 } + ] + }] + ) + end.to raise_error(ActiveRecord::RecordInvalid) + expect(a1.reload.gradebook_weight).to be_nil # transaction rolled back + end + + it 'nulls assessment weights when switching a tab to equal mode' do + a1.update!(gradebook_weight: 30.0) + described_class.update_gradebook_weights( + course: course, + updates: [{ tab_id: tab.id, weight: 50.0, weight_mode: 'equal' }] + ) + expect(tab.reload.weight_mode).to eq('equal') + expect(a1.reload.gradebook_weight).to be_nil + end + end end end From c68b1b4df9ec888fc2469abe1b6ce075270e148c Mon Sep 17 00:00:00 2001 From: lws49 Date: Wed, 10 Jun 2026 17:32:56 +0800 Subject: [PATCH 11/26] feat(gradebook): accept weight mode + assessment weights in update endpoint --- .../course/gradebook_controller.rb | 36 ++++++++++++--- .../course/gradebook_controller_spec.rb | 44 +++++++++++++++++++ 2 files changed, 74 insertions(+), 6 deletions(-) diff --git a/app/controllers/course/gradebook_controller.rb b/app/controllers/course/gradebook_controller.rb index 198be93d61..80b1762721 100644 --- a/app/controllers/course/gradebook_controller.rb +++ b/app/controllers/course/gradebook_controller.rb @@ -21,11 +21,9 @@ def index def update_weights authorize! :manage_gradebook_weights, current_course - updates = update_weights_params[:weights].map do |entry| - { tab_id: entry[:tabId].to_i, weight: entry[:weight].to_i } - end + updates = update_weights_params[:weights].map { |entry| parse_weight_entry(entry) } Course::Assessment::Tab.update_gradebook_weights(course: current_course, updates: updates) - render json: { weights: updates.map { |u| { tabId: u[:tab_id], weight: u[:weight] } } } + render json: { weights: serialize_weight_updates(updates) } rescue ActiveRecord::RecordInvalid, ActiveRecord::RecordNotFound => e render json: { errors: { base: e.message } }, status: :unprocessable_entity end @@ -36,8 +34,33 @@ def authorize_read_gradebook! authorize! :read_gradebook, current_course end + def parse_weight_entry(entry) + { + tab_id: entry[:tabId].to_i, + weight: entry[:weight].to_f, + weight_mode: entry[:weightMode] || 'equal', + assessment_weights: (entry[:assessmentWeights] || []).map do |aw| + { assessment_id: aw[:assessmentId].to_i, weight: aw[:weight].to_f } + end + } + end + def update_weights_params - params.permit(weights: [:tabId, :weight]) + params.permit( + weights: [:tabId, :weight, :weightMode, assessmentWeights: [:assessmentId, :weight]] + ) + end + + def serialize_weight_updates(updates) + updates.map do |u| + entry = { tabId: u[:tab_id], weight: u[:weight], weightMode: u[:weight_mode].to_s } + if u[:weight_mode].to_s == 'custom' + entry[:assessmentWeights] = u[:assessment_weights].map do |aw| + { assessmentId: aw[:assessment_id], weight: aw[:weight] } + end + end + entry + end end def component @@ -52,7 +75,8 @@ def fetch_categories_and_tabs def fetch_students current_course.levels.to_a current_course.course_users.students.without_phantom_users. - calculated(:experience_points).includes(:user).to_a + calculated(:experience_points).includes(:user).to_a. + sort_by { |cu| cu.user.name } end def fetch_published_assessments diff --git a/spec/controllers/course/gradebook_controller_spec.rb b/spec/controllers/course/gradebook_controller_spec.rb index 73b1014bbd..dacd9c24f3 100644 --- a/spec/controllers/course/gradebook_controller_spec.rb +++ b/spec/controllers/course/gradebook_controller_spec.rb @@ -293,6 +293,50 @@ expect(tab1.reload.gradebook_weight).to eq(60) end end + + describe '#update_weights with modes' do + render_views + + let(:category) { create(:course_assessment_category, course: course) } + let(:tab) { create(:course_assessment_tab, category: category) } + let!(:a1) { create(:assessment, course: course, tab: tab) } + let!(:a2) { create(:assessment, course: course, tab: tab) } + + before { controller_sign_in(controller, manager.user) } + + it 'persists custom mode + assessment weights and echoes them back' do + post :update_weights, as: :json, params: { + course_id: course.id, + weights: [{ + tabId: tab.id, weight: '50', weightMode: 'custom', + assessmentWeights: [ + { assessmentId: a1.id, weight: '30' }, + { assessmentId: a2.id, weight: '20' } + ] + }] + } + expect(response).to have_http_status(:ok) + body = JSON.parse(response.body) + entry = body['weights'].first + expect(entry['weightMode']).to eq('custom') + expect(entry['assessmentWeights']).to contain_exactly( + { 'assessmentId' => a1.id, 'weight' => 30.0 }, + { 'assessmentId' => a2.id, 'weight' => 20.0 } + ) + expect(a1.reload.gradebook_weight).to eq(30.0) + end + + it 'returns 422 when custom weights do not sum to the tab total' do + post :update_weights, as: :json, params: { + course_id: course.id, + weights: [{ + tabId: tab.id, weight: '50', weightMode: 'custom', + assessmentWeights: [{ assessmentId: a1.id, weight: '10' }] + }] + } + expect(response).to have_http_status(:unprocessable_entity) + end + end end describe 'GET index — weighted view fields' do From adaa514316ca7ca9221afb52380cc3d05b9732f9 Mon Sep 17 00:00:00 2001 From: lws49 Date: Wed, 10 Jun 2026 17:35:16 +0800 Subject: [PATCH 12/26] feat(gradebook): serialize weightMode and assessment gradebookWeight Expose tab weight_mode and assessment gradebook_weight as numeric JSON fields in the gradebook API when weighted view is enabled. Fixes BigDecimal string serialization by using &.to_f for safe float conversion. --- app/views/course/gradebook/index.json.jbuilder | 7 ++++++- spec/controllers/course/gradebook_controller_spec.rb | 9 +++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/app/views/course/gradebook/index.json.jbuilder b/app/views/course/gradebook/index.json.jbuilder index b92eba4551..606b9d9ae8 100644 --- a/app/views/course/gradebook/index.json.jbuilder +++ b/app/views/course/gradebook/index.json.jbuilder @@ -11,7 +11,10 @@ json.tabs @tabs do |tab| json.id tab.id json.title tab.title json.categoryId tab.category_id - json.gradebookWeight tab.gradebook_weight if @weighted_view_enabled + if @weighted_view_enabled + json.gradebookWeight tab.gradebook_weight&.to_f + json.weightMode tab.weight_mode + end end json.assessments @published_assessments do |assessment| @@ -19,6 +22,7 @@ json.assessments @published_assessments do |assessment| json.title assessment.title json.tabId assessment.tab_id json.maxGrade @assessment_max_grades[assessment.id] || 0 + json.gradebookWeight assessment.gradebook_weight&.to_f if @weighted_view_enabled end json.students @students do |course_user| @@ -31,6 +35,7 @@ json.students @students do |course_user| end json.submissions @submissions do |sub| + json.submissionId sub.submission_id json.studentId sub.student_id json.assessmentId sub.assessment_id json.grade sub.grade&.to_f diff --git a/spec/controllers/course/gradebook_controller_spec.rb b/spec/controllers/course/gradebook_controller_spec.rb index dacd9c24f3..d9310be329 100644 --- a/spec/controllers/course/gradebook_controller_spec.rb +++ b/spec/controllers/course/gradebook_controller_spec.rb @@ -385,6 +385,15 @@ body = JSON.parse(response.body) expect(body['canManageWeights']).to eq(false) end + + it 'serializes weightMode on tabs and gradebookWeight on assessments when weighted view is enabled' do + controller_sign_in(controller, manager.user) + get :index, params: { course_id: course.id }, format: :json + body = JSON.parse(response.body) + tab_json = body['tabs'].find { |t| t['id'] == tab.id } + expect(tab_json).to have_key('weightMode') + expect(body['assessments'].first).to have_key('gradebookWeight') + end end end end From 3bb54e512786405b9311088ac67bae87e60d9970 Mon Sep 17 00:00:00 2001 From: lws49 Date: Wed, 10 Jun 2026 17:37:01 +0800 Subject: [PATCH 13/26] feat(gradebook): add i18n for weight mode toggle and custom validation --- client/locales/en.json | 29 +++++++++++++++++++++++++---- client/locales/zh.json | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 4 deletions(-) diff --git a/client/locales/en.json b/client/locales/en.json index 9621d458ad..f08f20d0e6 100644 --- a/client/locales/en.json +++ b/client/locales/en.json @@ -9257,11 +9257,29 @@ "course.gradebook.ConfigureWeightsDialog.weightsDoNotSum": { "defaultMessage": "Weights do not sum to 100. Saving is allowed; Total may be inaccurate." }, + "course.gradebook.ConfigureWeightsDialog.equalMode": { + "defaultMessage": "Equal" + }, + "course.gradebook.ConfigureWeightsDialog.customMode": { + "defaultMessage": "Custom" + }, + "course.gradebook.ConfigureWeightsDialog.modeAria": { + "defaultMessage": "{tab} weight mode" + }, + "course.gradebook.ConfigureWeightsDialog.customSum": { + "defaultMessage": "Assessment weights: {sum} / {total}" + }, + "course.gradebook.ConfigureWeightsDialog.unbalanced": { + "defaultMessage": "Assessment weights for \"{tab}\" must sum to its tab total before saving." + }, + "course.gradebook.ConfigureWeightsDialog.valueNotTwoDp": { + "defaultMessage": "Value can have at most 2 decimal places" + }, "course.gradebook.GradebookIndex.allAssessments": { - "defaultMessage": "All assessments" + "defaultMessage": "Raw Scores" }, "course.gradebook.GradebookIndex.byWeight": { - "defaultMessage": "By weight" + "defaultMessage": "Weighted View" }, "course.gradebook.GradebookTable.noDataColumnsHintWithGamification": { "defaultMessage": "No grade or gamification columns selected - export will include student info only." @@ -9275,6 +9293,9 @@ "course.gradebook.GradebookWeightedTable.noWeightsConfigured": { "defaultMessage": "No weights configured - all tab weights are 0. Use \"Configure Weights\" to assign weights." }, + "course.gradebook.GradebookWeightedTable.name": { + "defaultMessage": "Name" + }, "course.gradebook.GradebookWeightedTable.percentOfGrade": { "defaultMessage": "{weight}% of grade" }, @@ -9284,8 +9305,8 @@ "course.gradebook.GradebookWeightedTable.percentTotalWarning": { "defaultMessage": "{weight}% total" }, - "course.gradebook.GradebookWeightedTable.student": { - "defaultMessage": "Student" + "course.gradebook.GradebookWeightedTable.searchStudents": { + "defaultMessage": "Search students" }, "course.gradebook.GradebookWeightedTable.total": { "defaultMessage": "Total" diff --git a/client/locales/zh.json b/client/locales/zh.json index bd4a78b680..61c2bed7b0 100644 --- a/client/locales/zh.json +++ b/client/locales/zh.json @@ -9214,5 +9214,47 @@ }, "users.troubleSigningIn": { "defaultMessage": "登录遇到问题？" + }, + "course.gradebook.ConfigureWeightsDialog.cancel": { + "defaultMessage": "取消" + }, + "course.gradebook.ConfigureWeightsDialog.description": { + "defaultMessage": "设置每个标签页对最终成绩的贡献比例。权重总和应为 100。" + }, + "course.gradebook.ConfigureWeightsDialog.dialogTitle": { + "defaultMessage": "配置标签页权重" + }, + "course.gradebook.ConfigureWeightsDialog.save": { + "defaultMessage": "保存" + }, + "course.gradebook.ConfigureWeightsDialog.total": { + "defaultMessage": "总计：{sum}%" + }, + "course.gradebook.ConfigureWeightsDialog.valueTooHigh": { + "defaultMessage": "值必须至多为 100" + }, + "course.gradebook.ConfigureWeightsDialog.valueTooLow": { + "defaultMessage": "值必须至少为 0" + }, + "course.gradebook.ConfigureWeightsDialog.weightsDoNotSum": { + "defaultMessage": "权重总和不为 100。允许保存；总计可能不准确。" + }, + "course.gradebook.ConfigureWeightsDialog.equalMode": { + "defaultMessage": "平均" + }, + "course.gradebook.ConfigureWeightsDialog.customMode": { + "defaultMessage": "自定义" + }, + "course.gradebook.ConfigureWeightsDialog.modeAria": { + "defaultMessage": "{tab} 权重模式" + }, + "course.gradebook.ConfigureWeightsDialog.customSum": { + "defaultMessage": "评估权重：{sum} / {total}" + }, + "course.gradebook.ConfigureWeightsDialog.unbalanced": { + "defaultMessage": "保存前，\"{tab}\"的各评估权重之和必须等于该标签的总权重。" + }, + "course.gradebook.ConfigureWeightsDialog.valueNotTwoDp": { + "defaultMessage": "数值最多只能有两位小数" } } From c0420e06ac4fe7947165a97e525c7c451d12844e Mon Sep 17 00:00:00 2001 From: lws49 Date: Wed, 10 Jun 2026 17:41:58 +0800 Subject: [PATCH 14/26] feat(gradebook): equal/custom weight modes with per-assessment inputs --- .../__tests__/ConfigureWeightsPrompt.test.tsx | 133 ++++++++--- .../components/ConfigureWeightsPrompt.tsx | 225 +++++++++++++++--- 2 files changed, 293 insertions(+), 65 deletions(-) diff --git a/client/app/bundles/course/gradebook/__tests__/ConfigureWeightsPrompt.test.tsx b/client/app/bundles/course/gradebook/__tests__/ConfigureWeightsPrompt.test.tsx index 346c489c67..07e3be9c5c 100644 --- a/client/app/bundles/course/gradebook/__tests__/ConfigureWeightsPrompt.test.tsx +++ b/client/app/bundles/course/gradebook/__tests__/ConfigureWeightsPrompt.test.tsx @@ -1,4 +1,4 @@ -import { fireEvent, render, screen, waitFor } from 'test-utils'; +import { fireEvent, render, screen, waitFor, within } from 'test-utils'; import ConfigureWeightsPrompt from '../components/ConfigureWeightsPrompt'; import * as operations from '../operations'; @@ -12,9 +12,10 @@ const tabs = [ { id: 10, title: 'Assignments', categoryId: 1, gradebookWeight: 50 }, { id: 11, title: 'Optional', categoryId: 1, gradebookWeight: 50 }, ]; +// Differing max grades to prove equal mode is 1/n (NOT proportional to max grade). const assessments = [ { id: 101, title: 'Assignment 1', tabId: 10, maxGrade: 100 }, - { id: 102, title: 'Assignment 2', tabId: 10, maxGrade: 100 }, + { id: 102, title: 'Assignment 2', tabId: 10, maxGrade: 50 }, ]; const setup = (overrides = {}): ReturnType => @@ -29,12 +30,13 @@ const setup = (overrides = {}): ReturnType => />, ); +const modeGroup = (tabTitle: string): HTMLElement => + screen.getByRole('group', { name: `${tabTitle} weight mode` }); + describe('', () => { - beforeEach(() => { - jest.clearAllMocks(); - }); + beforeEach(() => jest.clearAllMocks()); - it('renders one input per tab grouped by category', () => { + it('renders one tab total input per tab grouped by category', () => { setup(); expect(screen.getByText('Missions')).toBeInTheDocument(); expect(screen.getByLabelText('Assignments')).toHaveValue(50); @@ -56,7 +58,7 @@ describe('', () => { expect(screen.getByText(/do not sum to 100/i)).toBeInTheDocument(); }); - it('shows inline error for >100', () => { + it('shows inline error for tab total > 100', () => { setup(); fireEvent.change(screen.getByLabelText('Assignments'), { target: { value: '101' }, @@ -64,7 +66,7 @@ describe('', () => { expect(screen.getByText(/must be at most 100/i)).toBeInTheDocument(); }); - it('shows inline error for negative', () => { + it('shows inline error for negative tab total', () => { setup(); fireEvent.change(screen.getByLabelText('Optional'), { target: { value: '-1' }, @@ -72,54 +74,111 @@ describe('', () => { expect(screen.getByText(/must be at least 0/i)).toBeInTheDocument(); }); - it('Save dispatches updateGradebookWeights with { tabId, weight } only', async () => { + it('accepts a 2dp tab total without error', () => { setup(); - fireEvent.change(screen.getByLabelText('Optional'), { - target: { value: '40' }, - }); - fireEvent.click(screen.getByRole('button', { name: /save/i })); - await waitFor(() => { - expect(operations.updateGradebookWeights).toHaveBeenCalledWith([ - { tabId: 10, weight: 50 }, - { tabId: 11, weight: 40 }, - ]); + fireEvent.change(screen.getByLabelText('Assignments'), { + target: { value: '49.55' }, }); + expect(screen.queryByText(/decimal places/i)).not.toBeInTheDocument(); }); - it('Cancel does not dispatch', () => { + it('flags more than 2 decimal places', () => { setup(); - fireEvent.change(screen.getByLabelText('Optional'), { - target: { value: '40' }, + fireEvent.change(screen.getByLabelText('Assignments'), { + target: { value: '49.555' }, }); - fireEvent.click(screen.getByRole('button', { name: /cancel/i })); - expect(operations.updateGradebookWeights).not.toHaveBeenCalled(); + expect(screen.getByText(/decimal places/i)).toBeInTheDocument(); }); - it('assessment list is hidden by default and shown on expand', () => { + it('defaults every tab to Equal mode', () => { setup(); - expect(screen.queryByText('Assignment 1')).not.toBeVisible(); - const expandBtns = screen.getAllByRole('button', { name: '' }); - fireEvent.click(expandBtns[0]); - expect(screen.getByText('Assignment 1')).toBeVisible(); - expect(screen.getByText('Assignment 2')).toBeVisible(); + expect( + within(modeGroup('Assignments')).getByRole('button', { name: /equal/i }), + ).toHaveAttribute('aria-pressed', 'true'); }); - it('shows derived % of grade that updates live when weight changes', () => { + it('equal mode preview shows tabTotal / n per assessment (ignores max grade)', () => { setup(); const expandBtns = screen.getAllByRole('button', { name: '' }); - fireEvent.click(expandBtns[0]); - // weight=50, each assessment is 50% of tab → 25.0% of grade each - expect(screen.getAllByText('25.0% of grade')).toHaveLength(2); - fireEvent.change(screen.getByLabelText('Assignments'), { - target: { value: '60' }, + fireEvent.click(expandBtns[0]); // expand Assignments (weight 50, n=2) + // 1/n => 25.00 each; proportional-to-maxGrade would be 33.33 / 16.67. + expect(screen.getAllByText('25.00% of grade')).toHaveLength(2); + }); + + it('switching to Custom reveals per-assessment inputs seeded to sum the tab total', () => { + setup(); + fireEvent.click( + within(modeGroup('Assignments')).getByRole('button', { name: /custom/i }), + ); + expect(screen.getByLabelText('Assignments: Assignment 1')).toHaveValue(25); + expect(screen.getByLabelText('Assignments: Assignment 2')).toHaveValue(25); + }); + + it('shows an inline error Alert and disables Save when a custom tab is unbalanced', () => { + setup(); + fireEvent.click( + within(modeGroup('Assignments')).getByRole('button', { name: /custom/i }), + ); + fireEvent.change(screen.getByLabelText('Assignments: Assignment 1'), { + target: { value: '10' }, // 10 + 25 = 35 != 50 + }); + expect(screen.getByText(/must sum to its tab total/i)).toBeInTheDocument(); + expect(screen.getByRole('button', { name: /save/i })).toBeDisabled(); + }); + + it('Save sends weightMode for equal tabs and assessmentWeights for custom tabs', async () => { + setup(); + fireEvent.click( + within(modeGroup('Assignments')).getByRole('button', { name: /custom/i }), + ); + fireEvent.click(screen.getByRole('button', { name: /save/i })); + await waitFor(() => { + expect(operations.updateGradebookWeights).toHaveBeenCalledWith([ + { + tabId: 10, + weight: 50, + weightMode: 'custom', + assessmentWeights: [ + { assessmentId: 101, weight: 25 }, + { assessmentId: 102, weight: 25 }, + ], + }, + { tabId: 11, weight: 50, weightMode: 'equal' }, + ]); }); - expect(screen.getAllByText('30.0% of grade')).toHaveLength(2); }); - it('disables expand button for tabs with no assessments', () => { + it('seeds odd splits so they still sum exactly to the tab total', () => { + setup({ + tabs: [{ id: 10, title: 'Assignments', categoryId: 1, gradebookWeight: 50 }], + assessments: [ + { id: 101, title: 'A1', tabId: 10, maxGrade: 100 }, + { id: 102, title: 'A2', tabId: 10, maxGrade: 100 }, + { id: 103, title: 'A3', tabId: 10, maxGrade: 100 }, + ], + }); + fireEvent.click( + within(modeGroup('Assignments')).getByRole('button', { name: /custom/i }), + ); + expect(screen.getByLabelText('Assignments: A1')).toHaveValue(16.67); + expect(screen.getByLabelText('Assignments: A2')).toHaveValue(16.67); + expect(screen.getByLabelText('Assignments: A3')).toHaveValue(16.66); + expect(screen.queryByText(/must sum to its tab total/i)).not.toBeInTheDocument(); + }); + + it('Cancel does not dispatch', () => { + setup(); + fireEvent.click(screen.getByRole('button', { name: /cancel/i })); + expect(operations.updateGradebookWeights).not.toHaveBeenCalled(); + }); + + it('disables the mode toggle + expand for tabs with no assessments', () => { setup(); const expandBtns = screen.getAllByRole('button', { name: '' }); expect(expandBtns[1]).toBeDisabled(); + expect( + within(modeGroup('Optional')).getByRole('button', { name: /custom/i }), + ).toBeDisabled(); }); it('does not render an Exclude checkbox', () => { diff --git a/client/app/bundles/course/gradebook/components/ConfigureWeightsPrompt.tsx b/client/app/bundles/course/gradebook/components/ConfigureWeightsPrompt.tsx index 5fc5f36403..3f6cdcf2f3 100644 --- a/client/app/bundles/course/gradebook/components/ConfigureWeightsPrompt.tsx +++ b/client/app/bundles/course/gradebook/components/ConfigureWeightsPrompt.tsx @@ -7,6 +7,8 @@ import { IconButton, Stack, TextField, + ToggleButton, + ToggleButtonGroup, Typography, } from '@mui/material'; import type { @@ -53,9 +55,9 @@ const translations = defineMessages({ id: 'course.gradebook.ConfigureWeightsDialog.valueTooHigh', defaultMessage: 'Value must be at most 100', }, - valueNotInteger: { - id: 'course.gradebook.ConfigureWeightsDialog.valueNotInteger', - defaultMessage: 'Value must be a whole number', + valueNotTwoDp: { + id: 'course.gradebook.ConfigureWeightsDialog.valueNotTwoDp', + defaultMessage: 'Value can have at most 2 decimal places', }, saveError: { id: 'course.gradebook.ConfigureWeightsDialog.saveError', @@ -65,8 +67,51 @@ const translations = defineMessages({ id: 'course.gradebook.ConfigureWeightsDialog.ofGrade', defaultMessage: '{pct}% of grade', }, + equalMode: { + id: 'course.gradebook.ConfigureWeightsDialog.equalMode', + defaultMessage: 'Equal', + }, + customMode: { + id: 'course.gradebook.ConfigureWeightsDialog.customMode', + defaultMessage: 'Custom', + }, + modeAria: { + id: 'course.gradebook.ConfigureWeightsDialog.modeAria', + defaultMessage: '{tab} weight mode', + }, + customSum: { + id: 'course.gradebook.ConfigureWeightsDialog.customSum', + defaultMessage: 'Assessment weights: {sum} / {total}', + }, + unbalanced: { + id: 'course.gradebook.ConfigureWeightsDialog.unbalanced', + defaultMessage: + 'Assessment weights for "{tab}" must sum to its tab total before saving.', + }, }); +type WeightMode = 'equal' | 'custom'; + +const r2 = (n: number): number => Math.round(n * 100) / 100; +const cents = (n: number): number => Math.round(n * 100); +const is2dp = (n: number): boolean => Math.abs(n * 100 - Math.round(n * 100)) < 1e-9; + +// Distribute a tab total across assessment ids at 2dp; the last id absorbs the rounding +// remainder so the seeded values sum back exactly to total. +const distributeEqual = ( + total: number, + ids: number[], +): Record => { + const result: Record = {}; + const n = ids.length; + if (n === 0) return result; + const base = r2(total / n); + ids.forEach((id, i) => { + result[id] = i === n - 1 ? r2(total - base * (n - 1)) : base; + }); + return result; +}; + interface Props { open: boolean; onClose: () => void; @@ -87,48 +132,107 @@ const ConfigureWeightsPrompt: FC = ({ const validate = (value: number): string | null => { if (Number.isNaN(value)) return t(translations.valueTooLow); - if (!Number.isInteger(value)) return t(translations.valueNotInteger); if (value < 0) return t(translations.valueTooLow); if (value > 100) return t(translations.valueTooHigh); + if (!is2dp(value)) return t(translations.valueNotTwoDp); return null; }; - const [weights, setWeights] = useState>(() => - Object.fromEntries(tabs.map((tb) => [tb.id, tb.gradebookWeight ?? 0])), - ); + const seedWeights = (): Record => + Object.fromEntries(tabs.map((tb) => [tb.id, tb.gradebookWeight ?? 0])); + const seedModes = (): Record => + Object.fromEntries(tabs.map((tb) => [tb.id, tb.weightMode ?? 'equal'])); + const seedAssessmentWeights = (): Record => + Object.fromEntries(assessments.map((a) => [a.id, a.gradebookWeight ?? 0])); + + const [weights, setWeights] = useState>(seedWeights); + const [modes, setModes] = useState>(seedModes); + const [assessmentWeights, setAssessmentWeights] = useState< + Record + >(seedAssessmentWeights); const [expanded, setExpanded] = useState>({}); const [submitting, setSubmitting] = useState(false); useEffect(() => { if (open) { - setWeights( - Object.fromEntries(tabs.map((tb) => [tb.id, tb.gradebookWeight ?? 0])), - ); + setWeights(seedWeights()); + setModes(seedModes()); + setAssessmentWeights(seedAssessmentWeights()); setExpanded({}); } }, [open]); + const tabAssessmentIds = (tabId: number): number[] => + assessments.filter((a) => a.tabId === tabId).map((a) => a.id); + + const customSum = (tabId: number): number => + tabAssessmentIds(tabId).reduce( + (acc, id) => acc + (assessmentWeights[id] ?? 0), + 0, + ); + + const isUnbalanced = (tabId: number): boolean => + (modes[tabId] ?? 'equal') === 'custom' && + tabAssessmentIds(tabId).length > 0 && + cents(customSum(tabId)) !== cents(weights[tabId] ?? 0); + const sum = Object.values(weights).reduce((acc, w) => acc + w, 0); - const hasInvalid = Object.values(weights).some((w) => validate(w) !== null); + const hasInvalid = + Object.values(weights).some((w) => validate(w) !== null) || + Object.values(assessmentWeights).some((w) => validate(w) !== null); + const hasUnbalanced = tabs.some((tb) => isUnbalanced(tb.id)); const handleChange = (tabId: number, raw: string): void => { const parsed = raw === '' ? 0 : Number(raw); setWeights((prev) => ({ ...prev, [tabId]: parsed })); }; + const handleAssessmentChange = (assessmentId: number, raw: string): void => { + const parsed = raw === '' ? 0 : Number(raw); + setAssessmentWeights((prev) => ({ ...prev, [assessmentId]: parsed })); + }; + + const handleModeChange = (tabId: number, next: WeightMode | null): void => { + if (!next) return; // ToggleButtonGroup emits null when clicking the active button + setModes((prev) => ({ ...prev, [tabId]: next })); + if (next === 'custom') { + const ids = tabAssessmentIds(tabId); + const allZero = ids.every((id) => (assessmentWeights[id] ?? 0) === 0); + if (allZero) { + const seeded = distributeEqual(weights[tabId] ?? 0, ids); + setAssessmentWeights((prev) => ({ ...prev, ...seeded })); + } + setExpanded((prev) => ({ ...prev, [tabId]: true })); + } + }; + const toggleExpanded = (tabId: number): void => setExpanded((prev) => ({ ...prev, [tabId]: !prev[tabId] })); const handleSave = async (): Promise => { - if (hasInvalid) return; + if (hasInvalid || hasUnbalanced) return; setSubmitting(true); try { await dispatch( updateGradebookWeights( - tabs.map((tb) => ({ - tabId: tb.id, - weight: weights[tb.id] ?? 0, - })), + tabs.map((tb) => { + const mode = modes[tb.id] ?? 'equal'; + const entry = { + tabId: tb.id, + weight: weights[tb.id] ?? 0, + weightMode: mode, + }; + if (mode === 'custom') { + return { + ...entry, + assessmentWeights: tabAssessmentIds(tb.id).map((id) => ({ + assessmentId: id, + weight: assessmentWeights[id] ?? 0, + })), + }; + } + return entry; + }), ), ); onClose(); @@ -144,7 +248,7 @@ const ConfigureWeightsPrompt: FC = ({ onClickPrimary={handleSave} onClose={onClose} open={open} - primaryDisabled={submitting || hasInvalid} + primaryDisabled={submitting || hasInvalid || hasUnbalanced} primaryLabel={t(translations.save)} title={t(translations.dialogTitle)} > @@ -164,17 +268,17 @@ const ConfigureWeightsPrompt: FC = ({ const tabAssessments = assessments.filter( (a) => a.tabId === tb.id, ); - const tabMaxGrade = tabAssessments.reduce( - (s, a) => s + a.maxGrade, - 0, - ); + const mode = modes[tb.id] ?? 'equal'; const isExpanded = !!expanded[tb.id]; + const unbalanced = isUnbalanced(tb.id); + const noAssessments = tabAssessments.length === 0; + const n = tabAssessments.length; return (
toggleExpanded(tb.id)} size="small" > @@ -187,13 +291,32 @@ const ConfigureWeightsPrompt: FC = ({ {tb.title} + + handleModeChange(tb.id, next as WeightMode | null) + } + size="small" + value={mode} + > + + {t(translations.equalMode)} + + + {t(translations.customMode)} + + handleChange(tb.id, e.target.value)} size="small" @@ -211,13 +334,47 @@ const ConfigureWeightsPrompt: FC = ({ {err} )} + {unbalanced && ( + + {t(translations.unbalanced, { tab: tb.title })} + + )} {tabAssessments.map((a) => { - const pct = - tabMaxGrade > 0 - ? (a.maxGrade / tabMaxGrade) * value - : 0; + if (mode === 'custom') { + const awValue = assessmentWeights[a.id] ?? 0; + const awErr = validate(awValue); + return ( +
+ + {a.title} + + + handleAssessmentChange(a.id, e.target.value) + } + size="small" + sx={{ width: 88 }} + type="number" + value={awValue} + /> +
+ ); + } + const pct = n > 0 ? r2(value / n) : 0; return (
= ({ variant="caption" > {t(translations.ofGrade, { - pct: pct.toFixed(1), + pct: pct.toFixed(2), })}
); })} + {mode === 'custom' && ( + + {t(translations.customSum, { + sum: r2(customSum(tb.id)).toFixed(2), + total: value.toFixed(2), + })} + + )}
From bee4f221130a4123e1e5689bde72327c9df54876 Mon Sep 17 00:00:00 2001 From: lws49 Date: Wed, 10 Jun 2026 23:50:49 +0800 Subject: [PATCH 15/26] feat(gradebook): default weighted-view header to points-out-of labels --- .../components/GradebookWeightedTable.tsx | 212 ++++++++++++------ 1 file changed, 143 insertions(+), 69 deletions(-) diff --git a/client/app/bundles/course/gradebook/components/GradebookWeightedTable.tsx b/client/app/bundles/course/gradebook/components/GradebookWeightedTable.tsx index 2f433024fd..f334866c46 100644 --- a/client/app/bundles/course/gradebook/components/GradebookWeightedTable.tsx +++ b/client/app/bundles/course/gradebook/components/GradebookWeightedTable.tsx @@ -1,13 +1,11 @@ -import { useMemo, useState } from 'react'; +import { useLayoutEffect, useMemo, useRef, useState } from 'react'; import { defineMessages } from 'react-intl'; import { Download } from '@mui/icons-material'; import { Alert, Button, Checkbox, - FormControlLabel, Paper, - Switch, Table, TableBody, TableCell, @@ -31,10 +29,7 @@ import type { ColumnTemplate } from 'lib/components/table/builder'; import MuiColumnPickerPrompt from 'lib/components/table/MuiTableAdapter/MuiColumnPickerPrompt'; import MuiTablePagination from 'lib/components/table/MuiTableAdapter/MuiTablePagination'; import useTanStackTableBuilder from 'lib/components/table/TanStackTableBuilder'; -import { - DEFAULT_MINI_TABLE_ROWS_PER_PAGE, - DEFAULT_TABLE_ROWS_PER_PAGE, -} from 'lib/constants/sharedConstants'; +import { DEFAULT_TABLE_ROWS_PER_PAGE } from 'lib/constants/sharedConstants'; import useTranslation from 'lib/hooks/useTranslation'; import tableTranslations from 'lib/translations/table'; @@ -45,15 +40,6 @@ import ConfigureWeightsPrompt from './ConfigureWeightsPrompt'; import WeightedGradebookColumnTree from './WeightedGradebookColumnTree'; const translations = defineMessages({ - treatUngradedAsZero: { - id: 'course.gradebook.GradebookWeightedTable.treatUngradedAsZero', - defaultMessage: 'Treat Ungraded as 0', - }, - treatUngradedAsZeroTooltip: { - id: 'course.gradebook.GradebookWeightedTable.treatUngradedAsZeroTooltip', - defaultMessage: - 'Counts unsubmitted and ungraded assessments as 0 in the calculation. Use at end of course when all work should be complete.', - }, configureWeights: { id: 'course.gradebook.GradebookWeightedTable.configureWeights', defaultMessage: 'Configure Weights', @@ -67,17 +53,17 @@ const translations = defineMessages({ id: 'course.gradebook.GradebookWeightedTable.noWeightsNoAccess', defaultMessage: 'No tab weights have been configured yet.', }, - student: { - id: 'course.gradebook.GradebookWeightedTable.student', - defaultMessage: 'Student', + name: { + id: 'course.gradebook.GradebookWeightedTable.name', + defaultMessage: 'Name', }, email: { id: 'course.gradebook.GradebookWeightedTable.email', defaultMessage: 'Email', }, - total: { - id: 'course.gradebook.GradebookWeightedTable.total', - defaultMessage: 'Total', + projectedTotal: { + id: 'course.gradebook.GradebookWeightedTable.projectedTotal', + defaultMessage: 'Projected total — ungraded assessments count as 0', }, percentOfGrade: { id: 'course.gradebook.GradebookWeightedTable.percentOfGrade', @@ -91,6 +77,18 @@ const translations = defineMessages({ id: 'course.gradebook.GradebookWeightedTable.percentTotalWarning', defaultMessage: '{weight}% total', }, + outOfWeight: { + id: 'course.gradebook.GradebookWeightedTable.outOfWeight', + defaultMessage: '/{weight}', + }, + displayPoints: { + id: 'course.gradebook.GradebookWeightedTable.displayPoints', + defaultMessage: 'Points', + }, + displayPercent: { + id: 'course.gradebook.GradebookWeightedTable.displayPercent', + defaultMessage: 'Percentage', + }, doesNotSumTo100: { id: 'course.gradebook.GradebookWeightedTable.doesNotSumTo100', defaultMessage: 'does not sum to 100', @@ -101,7 +99,7 @@ const translations = defineMessages({ }, searchStudents: { id: 'course.gradebook.GradebookWeightedTable.searchStudents', - defaultMessage: 'Search by name or email', + defaultMessage: 'Search students', }, downloadCsv: { id: 'course.gradebook.GradebookWeightedTable.downloadCsv', @@ -149,10 +147,31 @@ interface Props { gamificationEnabled: boolean; } -const fmt = (v: number | null): string => { +// How many decimal places a single value needs (0, 1, or 2). +const precisionNeeded = (v: number): 0 | 1 | 2 => { + const at2 = Math.round(v * 100) / 100; + const at1 = Math.round(v * 10) / 10; + const at0 = Math.round(v); + if (Math.abs(at2 - at1) > 1e-9) return 2; + if (Math.abs(at1 - at0) > 1e-9) return 1; + return 0; +}; + +// Maximum precision needed across a column's values. +const columnPrecision = (values: (number | null)[]): 0 | 1 | 2 => { + let prec: 0 | 1 | 2 = 0; + for (const v of values) { + if (v === null) continue; + const p = precisionNeeded(v); + if (p === 2) return 2; + if (p === 1) prec = 1; + } + return prec; +}; + +const fmtAt = (v: number | null, prec: 0 | 1 | 2): string => { if (v === null) return '—'; - const rounded = Math.round(v); - return Math.abs(v - rounded) < 1e-9 ? String(rounded) : v.toFixed(2); + return v.toFixed(prec); }; const fmtCsv = (v: number | null): string => { @@ -174,9 +193,15 @@ const GradebookWeightedTable = ({ gamificationEnabled, }: Props): JSX.Element => { const { t } = useTranslation(); - const [treatUngradedAsZero, setTreatUngradedAsZero] = useState(false); const [configureOpen, setConfigureOpen] = useState(false); const [pickerOpen, setPickerOpen] = useState(false); + type DisplayMode = 'points' | 'percent'; + const [displayMode, setDisplayMode] = useState('points'); + + const row1Ref = useRef(null); + const row2Ref = useRef(null); + const [row2Top, setRow2Top] = useState(0); + const [row3Top, setRow3Top] = useState(0); const totalWeight = sumWeights(tabs); const allWeightsZero = totalWeight === 0; @@ -194,6 +219,13 @@ const GradebookWeightedTable = ({ [categories, categoryTabCounts], ); + useLayoutEffect(() => { + const h1 = row1Ref.current?.offsetHeight ?? 0; + const h2 = row2Ref.current?.offsetHeight ?? 0; + setRow2Top(h1); + setRow3Top(h1 + h2); + }, [visibleCategories, tabs]); + const rows = useMemo( () => computeWeightedRows({ @@ -201,11 +233,23 @@ const GradebookWeightedTable = ({ tabs, assessments, submissions, - treatUngradedAsZero, }), - [students, tabs, assessments, submissions, treatUngradedAsZero], + [students, tabs, assessments, submissions], ); + const columnPrecisions = useMemo(() => { + const tabPrecs = tabs.map((tab, idx) => { + const weight = tab.gradebookWeight ?? 0; + return columnPrecision( + rows.map((r) => { + const sub = r.subtotals[idx]; + return sub !== null ? sub * weight : null; + }), + ); + }); + return { tabs: tabPrecs, total: columnPrecision(rows.map((r) => r.total)) }; + }, [rows, tabs]); + const hasExternalIds = useMemo( () => students.some((s) => s.externalId != null && s.externalId !== ''), [students], @@ -215,7 +259,7 @@ const GradebookWeightedTable = ({ const cols: ColumnTemplate[] = [ { id: 'name', - title: t(translations.student), + title: t(translations.name), of: 'name', cell: (row) => row.name, csvDownloadable: true, @@ -261,6 +305,7 @@ const GradebookWeightedTable = ({ tabs.forEach((tab, idx) => { const weight = tab.gradebookWeight ?? 0; + const prec = columnPrecisions.tabs[idx]; cols.push({ id: `tab-${tab.id}`, title: tab.title, @@ -270,7 +315,7 @@ const GradebookWeightedTable = ({ }, cell: (row) => { const sub = row.subtotals[idx]; - return fmt(sub !== null ? sub * weight : null); + return fmtAt(sub !== null ? sub * weight : null, prec); }, csvDownloadable: true, }); @@ -278,14 +323,14 @@ const GradebookWeightedTable = ({ cols.push({ id: 'total', - title: t(translations.total), + title: t(translations.projectedTotal), accessorFn: (row) => fmtCsv(row.total), - cell: (row) => fmt(row.total), + cell: (row) => fmtAt(row.total, columnPrecisions.total), csvDownloadable: true, }); return cols; - }, [tabs, t, gamificationEnabled, hasExternalIds]); + }, [tabs, t, gamificationEnabled, hasExternalIds, columnPrecisions]); const columnPicker = useMemo( () => ({ @@ -314,12 +359,7 @@ const GradebookWeightedTable = ({ getRowEqualityData: (row) => row, indexing: { rowSelectable: true }, pagination: { - rowsPerPage: [ - DEFAULT_MINI_TABLE_ROWS_PER_PAGE, - 25, - 50, - DEFAULT_TABLE_ROWS_PER_PAGE, - ], + rowsPerPage: [DEFAULT_TABLE_ROWS_PER_PAGE], showAllRows: true, }, search: { searchPlaceholder: t(translations.searchStudents) }, @@ -365,19 +405,6 @@ const GradebookWeightedTable = ({ value={toolbar.searchKeyword ?? ''} />
- - setTreatUngradedAsZero(e.target.checked)} - size="small" - /> - } - label={t(translations.treatUngradedAsZero)} - sx={{ ml: 0 }} - /> - {canManageWeights && (