From 556c3666f93ea3d1ebbd852dbb0538265a48da0a Mon Sep 17 00:00:00 2001 From: lws49 Date: Wed, 20 May 2026 09:51:05 +0000 Subject: [PATCH 1/2] 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 0c5fbed237..0b8746193b 100644 --- a/client/app/lib/components/table/TanStackTableBuilder/useTanStackTableBuilder.tsx +++ b/client/app/lib/components/table/TanStackTableBuilder/useTanStackTableBuilder.tsx @@ -221,10 +221,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), }); @@ -351,6 +354,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 b8899dbf67..ec99ec3ff7 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 8b79b302e474604894f535be2f2517b367717e6f Mon Sep 17 00:00:00 2001 From: lws49 Date: Wed, 20 May 2026 09:51:45 +0000 Subject: [PATCH 2/2] =?UTF-8?q?feat(gradebook):=20API=20=E2=80=94=20expose?= =?UTF-8?q?=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 8c67f0a070..edd6daa4d1 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 0b8746193b..0c5fbed237 100644 --- a/client/app/lib/components/table/TanStackTableBuilder/useTanStackTableBuilder.tsx +++ b/client/app/lib/components/table/TanStackTableBuilder/useTanStackTableBuilder.tsx @@ -221,13 +221,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), }); @@ -354,10 +351,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 ec99ec3ff7..b8899dbf67 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 0b6f16f368..6838d77f24 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 4b21113c31..01fc03c37d 100644 --- a/spec/controllers/course/gradebook_controller_spec.rb +++ b/spec/controllers/course/gradebook_controller_spec.rb @@ -185,5 +185,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