diff --git a/app/controllers/course/gradebook_controller.rb b/app/controllers/course/gradebook_controller.rb index b5ef6c71c2..e23e7c0a81 100644 --- a/app/controllers/course/gradebook_controller.rb +++ b/app/controllers/course/gradebook_controller.rb @@ -47,6 +47,7 @@ def parse_weight_entry(entry) tab_id: entry[:tabId].to_i, weight: entry[:weight].to_f, weight_mode: entry[:weightMode] || 'equal', + keep_highest: entry[:keepHighest].to_i, excluded_assessment_ids: (entry[:excludedAssessmentIds] || []).map(&:to_i), assessment_weights: (entry[:assessmentWeights] || []).map do |aw| { assessment_id: aw[:assessmentId].to_i, weight: aw[:weight].to_f } @@ -56,7 +57,7 @@ def parse_weight_entry(entry) def update_weights_params params.permit( - weights: [:tabId, :weight, :weightMode, + weights: [:tabId, :weight, :weightMode, :keepHighest, excludedAssessmentIds: [], assessmentWeights: [:assessmentId, :weight]] ) end @@ -64,6 +65,7 @@ def update_weights_params def serialize_weight_updates(updates) updates.map do |u| entry = { tabId: u[:tab_id], weight: u[:weight], weightMode: u[:weight_mode].to_s, + keepHighest: u[:keep_highest], excludedAssessmentIds: u[:excluded_assessment_ids] } if u[:weight_mode].to_s == 'custom' entry[:assessmentWeights] = u[:assessment_weights].map do |aw| diff --git a/app/views/course/gradebook/index.json.jbuilder b/app/views/course/gradebook/index.json.jbuilder index b3cc4ec4d0..9f30685096 100644 --- a/app/views/course/gradebook/index.json.jbuilder +++ b/app/views/course/gradebook/index.json.jbuilder @@ -15,6 +15,7 @@ json.tabs @tabs do |tab| contribution = @tab_contributions[tab.id] json.gradebookWeight (contribution&.weight || 0).to_f json.weightMode(contribution&.weight_mode || 'equal') + json.keepHighest(contribution&.keep_highest || 0) end end diff --git a/client/app/bundles/course/gradebook/__tests__/ConfigureWeightsPrompt.test.tsx b/client/app/bundles/course/gradebook/__tests__/ConfigureWeightsPrompt.test.tsx index cce652258c..3ea24d01d6 100644 --- a/client/app/bundles/course/gradebook/__tests__/ConfigureWeightsPrompt.test.tsx +++ b/client/app/bundles/course/gradebook/__tests__/ConfigureWeightsPrompt.test.tsx @@ -147,6 +147,7 @@ describe('', () => { tabId: 10, weight: 50, weightMode: 'custom', + keepHighest: 0, excludedAssessmentIds: [], assessmentWeights: [ { assessmentId: 101, weight: 25 }, @@ -157,6 +158,7 @@ describe('', () => { tabId: 11, weight: 50, weightMode: 'equal', + keepHighest: 0, excludedAssessmentIds: [], }, ]); @@ -404,6 +406,7 @@ describe('per-assessment exclusion', () => { expect(arg[0]).toMatchObject({ tabId: 10, weight: 50, + keepHighest: 0, excludedAssessmentIds: [101, 102], }); }); @@ -477,4 +480,190 @@ describe('per-assessment exclusion', () => { expect(screen.queryByText(/no weights set yet/i)).not.toBeInTheDocument(); }); }); + + it('shows assessment weights sum footer in custom mode', () => { + setup(); + fireEvent.click( + within(modeGroup('Assignments')).getByRole('radio', { name: /custom/i }), + ); + // Expand to see the footer + // custom mode auto-expands, so just check the footer + expect(screen.getByText(/Assessment weights:/i)).toBeInTheDocument(); + }); +}); + +describe('per-assessment exclusion (extended)', () => { + beforeEach(() => jest.clearAllMocks()); + + it('shows "Excluded" label in custom mode for excluded assessment', async () => { + setup({ + assessments: [ + { + id: 101, + title: A1, + tabId: 10, + maxGrade: 100, + gradebookExcluded: true, + }, + { id: 102, title: A2, tabId: 10, maxGrade: 50 }, + ], + tabs: [ + { + id: 10, + title: 'Assignments', + categoryId: 1, + gradebookWeight: 50, + weightMode: 'custom', + }, + ], + }); + fireEvent.click(screen.getAllByRole('button', { name: '' })[0]); + expect(await screen.findByText('Excluded')).toBeInTheDocument(); + }); + + it('does not show "Excluded" label in equal mode for excluded assessment', async () => { + setup({ + assessments: [ + { + id: 101, + title: A1, + tabId: 10, + maxGrade: 100, + gradebookExcluded: true, + }, + { id: 102, title: A2, tabId: 10, maxGrade: 50 }, + ], + }); + fireEvent.click(screen.getAllByRole('button', { name: '' })[0]); + // In equal mode excluded shows "Excluded" text too + expect(await screen.findByText('Excluded')).toBeInTheDocument(); + }); +}); + +describe('keep-highest control', () => { + const TOGGLE = 'Enable keep highest for Assignments'; + const INPUT = 'Keep highest for Assignments'; + const three = [ + { id: 101, title: A1, tabId: 10, maxGrade: 100 }, + { id: 102, title: A2, tabId: 10, maxGrade: 50 }, + { id: 103, title: 'Assignment 3', tabId: 10, maxGrade: 80 }, + ]; + + beforeEach(() => jest.clearAllMocks()); + + it('renders a keep-highest checkbox; number field hidden until checked', () => { + setup({ assessments: three }); + expect(screen.getByRole('checkbox', { name: TOGGLE })).toBeInTheDocument(); + expect( + screen.queryByRole('spinbutton', { name: INPUT }), + ).not.toBeInTheDocument(); + fireEvent.click(screen.getByRole('checkbox', { name: TOGGLE })); + expect(screen.getByRole('spinbutton', { name: INPUT })).toBeInTheDocument(); + }); + + it('shows a visible "Keep highest" text label next to the checkbox', () => { + setup({ assessments: three }); + expect(screen.getByText('Keep highest')).toBeInTheDocument(); + }); + + it('defaults the count to included − 1 when checked', () => { + setup({ assessments: three }); // 3 assessments -> default to 2 + fireEvent.click(screen.getByRole('checkbox', { name: TOGGLE })); + expect(screen.getByRole('spinbutton', { name: INPUT })).toHaveValue(2); + }); + + it('hides checkbox + field in custom mode', () => { + setup({ assessments: three }); + fireEvent.click( + within(modeGroup('Assignments')).getByRole('radio', { name: /custom/i }), + ); + expect( + screen.queryByRole('checkbox', { name: TOGGLE }), + ).not.toBeInTheDocument(); + expect( + screen.queryByRole('spinbutton', { name: INPUT }), + ).not.toBeInTheDocument(); + }); + + it('seeds the field (checkbox pre-checked) from tab.keepHighest', () => { + setup({ + assessments: three, + tabs: [ + { + id: 10, + title: 'Assignments', + categoryId: 1, + gradebookWeight: 50, + keepHighest: 2, + }, + { id: 11, title: 'Optional', categoryId: 1, gradebookWeight: 50 }, + ], + }); + expect(screen.getByRole('checkbox', { name: TOGGLE })).toBeChecked(); + expect(screen.getByRole('spinbutton', { name: INPUT })).toHaveValue(2); + }); + + it('disables the checkbox when only one assessment is included', () => { + setup({ + assessments: [{ id: 101, title: A1, tabId: 10, maxGrade: 100 }], + }); + expect(screen.getByRole('checkbox', { name: TOGGLE })).toBeDisabled(); + }); + + it('sends keepHighest in the save payload', async () => { + setup({ assessments: three }); + fireEvent.click(screen.getByRole('checkbox', { name: TOGGLE })); + fireEvent.click(screen.getByRole('button', { name: /save/i })); + await waitFor(() => + expect(operations.updateGradebookWeights).toHaveBeenCalled(), + ); + const arg = (operations.updateGradebookWeights as jest.Mock).mock + .calls[0][0]; + const tab10 = arg.find((e: { tabId: number }) => e.tabId === 10); + expect(tab10.keepHighest).toBe(2); + }); + + it('unchecking sends keepHighest 0', async () => { + setup({ + assessments: three, + tabs: [ + { + id: 10, + title: 'Assignments', + categoryId: 1, + gradebookWeight: 50, + keepHighest: 2, + }, + { id: 11, title: 'Optional', categoryId: 1, gradebookWeight: 50 }, + ], + }); + // checkbox is pre-checked; uncheck it + fireEvent.click(screen.getByRole('checkbox', { name: TOGGLE })); + fireEvent.click(screen.getByRole('button', { name: /save/i })); + await waitFor(() => + expect(operations.updateGradebookWeights).toHaveBeenCalled(), + ); + const arg = (operations.updateGradebookWeights as jest.Mock).mock + .calls[0][0]; + const tab10 = arg.find((e: { tabId: number }) => e.tabId === 10); + expect(tab10.keepHighest).toBe(0); + }); + + it('blocks saving on non-integer input but not on keep > included (overflow)', async () => { + setup({ assessments: three }); + fireEvent.click(screen.getByRole('checkbox', { name: TOGGLE })); + const input = screen.getByRole('spinbutton', { name: INPUT }); + + // overflow: keep=5 > included=3 -> warning but save NOT blocked + fireEvent.change(input, { target: { value: '5' } }); + expect(screen.getByRole('button', { name: /save/i })).not.toBeDisabled(); + expect( + screen.getByText(/keeps more assessments than it contains/i), + ).toBeInTheDocument(); + + // non-integer (0): should block saving + fireEvent.change(input, { target: { value: '0' } }); + expect(screen.getByRole('button', { name: /save/i })).toBeDisabled(); + expect(screen.getByText(/keep at least 1/i)).toBeInTheDocument(); + }); }); diff --git a/client/app/bundles/course/gradebook/__tests__/GradebookWeightedTable.test.tsx b/client/app/bundles/course/gradebook/__tests__/GradebookWeightedTable.test.tsx index 9bc42a310e..7fd5885854 100644 --- a/client/app/bundles/course/gradebook/__tests__/GradebookWeightedTable.test.tsx +++ b/client/app/bundles/course/gradebook/__tests__/GradebookWeightedTable.test.tsx @@ -1116,4 +1116,145 @@ describe('GradebookWeightedTable', () => { expect(cells[cells.length - 1]).toHaveTextContent('90%'); }); }); + + describe('single-open accordion', () => { + it('keeps only one student expanded at a time (opening another collapses the first)', async () => { + const user = userEvent.setup(); + renderWeighted({ + tabs: [makeTab(10, 'Missions', 1, 100)], + assessments: [makeAssessment(1, 'Mission 1', 10, 100)], + students: [makeStudent(1, 'Alice'), makeStudent(2, 'Bob')], + submissions: [makeSub(1, 1, 80), makeSub(2, 1, 60)], + }); + + await user.click(screen.getByRole('button', { name: /expand Alice/i })); + expect( + await screen.findByTestId(breakdownRowId(1, 10, 1)), + ).toBeInTheDocument(); + + // Opening Bob must collapse Alice. + await user.click(screen.getByRole('button', { name: /expand Bob/i })); + expect( + await screen.findByTestId(breakdownRowId(2, 10, 1)), + ).toBeInTheDocument(); + await waitFor(() => + expect( + screen.queryByTestId(breakdownRowId(1, 10, 1)), + ).not.toBeInTheDocument(), + ); + }); + + it('keeps focus on the toggle after expanding (keyboard users stay put)', async () => { + const user = userEvent.setup(); + renderWeighted({ + tabs: [makeTab(10, 'Missions', 1, 100)], + assessments: [makeAssessment(1, 'Mission 1', 10, 100)], + students: [makeStudent(1, 'Alice')], + submissions: [makeSub(1, 1, 80)], + }); + const toggle = screen.getByRole('button', { name: /expand Alice/i }); + await user.click(toggle); + // Same DOM button (label flips expand→collapse), so focus is retained. + expect( + screen.getByRole('button', { name: /collapse Alice/i }), + ).toHaveFocus(); + }); + }); + + describe('auto-scroll on expand', () => { + const one = { + tabs: [makeTab(10, 'Missions', 1, 100)], + assessments: [makeAssessment(1, 'Mission 1', 10, 100)], + students: [makeStudent(1, 'Alice')], + submissions: [makeSub(1, 1, 80)], + }; + + afterEach(() => { + // Restore the scrollTo we stub in (jsdom doesn't implement it). + delete (Element.prototype as unknown as { scrollTo?: unknown }).scrollTo; + }); + + it('smooth-scrolls the expanded row into view by default', async () => { + const scrollSpy = jest.fn(); + (Element.prototype as unknown as { scrollTo: unknown }).scrollTo = + scrollSpy; + const user = userEvent.setup(); + renderWeighted(one); + await user.click(screen.getByRole('button', { name: /expand Alice/i })); + await waitFor(() => expect(scrollSpy).toHaveBeenCalled()); + expect(scrollSpy.mock.calls[0][0]).toMatchObject({ behavior: 'smooth' }); + }); + + it('jumps instantly when the user prefers reduced motion', async () => { + const scrollSpy = jest.fn(); + (Element.prototype as unknown as { scrollTo: unknown }).scrollTo = + scrollSpy; + const originalMatchMedia = window.matchMedia; + window.matchMedia = ((query: string) => ({ + matches: query === '(prefers-reduced-motion: reduce)', + media: query, + onchange: null, + addEventListener: (): void => {}, + removeEventListener: (): void => {}, + addListener: (): void => {}, + removeListener: (): void => {}, + dispatchEvent: (): boolean => false, + })) as unknown as typeof window.matchMedia; + + try { + const user = userEvent.setup(); + renderWeighted(one); + await user.click(screen.getByRole('button', { name: /expand Alice/i })); + await waitFor(() => expect(scrollSpy).toHaveBeenCalled()); + expect(scrollSpy.mock.calls[0][0]).toMatchObject({ behavior: 'auto' }); + } finally { + window.matchMedia = originalMatchMedia; + } + }); + }); + + describe('drop-lowest breakdown marker', () => { + // One equal tab, keepHighest=1, three graded assessments. The lowest (a1) drops. + const dropConfig = { + tabs: [{ ...makeTab(10, 'Missions', 1, 60), keepHighest: 1 }], + assessments: [ + makeAssessment(1, 'Mission 1', 10, 100), + makeAssessment(2, 'Mission 2', 10, 100), + makeAssessment(3, 'Mission 3', 10, 100), + ], + students: [makeStudent(1, 'Alice')], + submissions: [makeSub(1, 1, 30), makeSub(1, 2, 60), makeSub(1, 3, 90)], + }; + + it('labels the dropped assessment "Dropped (lowest)" (not "Excluded")', async () => { + const user = userEvent.setup(); + renderWeighted(dropConfig); + await user.click(screen.getByRole('button', { name: /expand Alice/i })); + const detail = await screen.findByTestId(breakdownRowId(1, 10, 1)); + expect( + within(detail).getByText(/Dropped \(lowest\)/), + ).toBeInTheDocument(); + expect(within(detail).queryByText(/Excluded/i)).not.toBeInTheDocument(); + }); + + it('shows 0 (not —) in the dropped assessment cell in points mode', async () => { + const user = userEvent.setup(); + renderWeighted(dropConfig); + await user.click(screen.getByRole('button', { name: /expand Alice/i })); + const detail = await screen.findByTestId(breakdownRowId(1, 10, 1)); + // Dropped contributes 0 points — distinct from excluded's em dash. + expect(within(detail).getByText('0')).toBeInTheDocument(); + expect(within(detail).queryByText('—')).not.toBeInTheDocument(); + }); + + it("shows the dropped assessment's own grade % in percentage mode", async () => { + const user = userEvent.setup(); + renderWeighted(dropConfig); + await user.click(screen.getByRole('radio', { name: /percentage/i })); + await user.click(screen.getByRole('button', { name: /expand Alice/i })); + const detail = await screen.findByTestId(breakdownRowId(1, 10, 1)); + // a1 = 30/100 = 30% — visible so the instructor sees the dropped grade. + expect(within(detail).getByText('30%')).toBeInTheDocument(); + }); + }); }); diff --git a/client/app/bundles/course/gradebook/__tests__/computeWeighted.test.ts b/client/app/bundles/course/gradebook/__tests__/computeWeighted.test.ts index c4f2ed866f..dcd7cce5f6 100644 --- a/client/app/bundles/course/gradebook/__tests__/computeWeighted.test.ts +++ b/client/app/bundles/course/gradebook/__tests__/computeWeighted.test.ts @@ -686,3 +686,202 @@ describe('breakdown — exclusion', () => { expect(a.points).toBeCloseTo(60); }); }); + +// --------------------------------------------------------------------------- +// keep-N (keepHighest) — equal mode only +// --------------------------------------------------------------------------- + +// Three equal-mode assessments: a1=60/100=0.60, a2=80/100=0.80, a3=100/100=1.00 +// keepHighest:1 → keep top 1 (a3, ratio 1.0), drop a1 and a2 +const dropConfig = { + tab: { + id: 10, + title: 'Tab', + categoryId: 0, + gradebookWeight: 60, + keepHighest: 1, + }, + assessments: [ + { id: 1, tabId: 10, maxGrade: 100, title: 'A' }, + { id: 2, tabId: 10, maxGrade: 100, title: 'B' }, + { id: 3, tabId: 10, maxGrade: 100, title: 'C' }, + ], + submissions: subs([ + { studentId: 1, assessmentId: 1, grade: 60 }, + { studentId: 1, assessmentId: 2, grade: 80 }, + { studentId: 1, assessmentId: 3, grade: 100 }, + ]), +}; + +describe('equalSubtotal — keepHighest via computeTabSubtotal', () => { + it('keepHighest=0 (unset): keeps all assessments — same as default behavior', () => { + // (0.6 + 0.8 + 1.0) / 3 = 0.8 + expect( + computeTabSubtotal({ + studentId: 1, + tab: { id: 10, title: 'Tab', categoryId: 0, gradebookWeight: 60 }, + assessments: dropConfig.assessments, + submissions: dropConfig.submissions, + }), + ).toBeCloseTo(0.8); + }); + + it('keepHighest=1: averages only the single highest ratio', () => { + // top 1: a3 ratio=1.0 → subtotal = 1.0/1 = 1.0 + expect( + computeTabSubtotal({ + studentId: 1, + tab: dropConfig.tab, + assessments: dropConfig.assessments, + submissions: dropConfig.submissions, + }), + ).toBeCloseTo(1.0); + }); + + it('keepHighest > count: clamps to all assessments', () => { + // keepHighest=99 with 3 assessments → keep all 3 → (0.6+0.8+1.0)/3 = 0.8 + expect( + computeTabSubtotal({ + studentId: 1, + tab: { ...dropConfig.tab, keepHighest: 99 }, + assessments: dropConfig.assessments, + submissions: dropConfig.submissions, + }), + ).toBeCloseTo(0.8); + }); +}); + +describe('computeStudentBreakdown — keepHighest', () => { + it('dropped flag is true on lowest-ratio assessments, false on kept ones', () => { + const [tab] = computeStudentBreakdown({ + studentId: 1, + tabs: [dropConfig.tab], + assessments: dropConfig.assessments, + submissions: dropConfig.submissions, + }); + const a1 = tab.assessments.find((x) => x.assessmentId === 1)!; // ratio 0.60 — dropped + const a2 = tab.assessments.find((x) => x.assessmentId === 2)!; // ratio 0.80 — dropped + const a3 = tab.assessments.find((x) => x.assessmentId === 3)!; // ratio 1.00 — kept + + expect(a1.dropped).toBe(true); + expect(a2.dropped).toBe(true); + expect(a3.dropped).toBe(false); + }); + + it('dropped assessment has points=0 and effectiveWeight=0', () => { + const [tab] = computeStudentBreakdown({ + studentId: 1, + tabs: [dropConfig.tab], + assessments: dropConfig.assessments, + submissions: dropConfig.submissions, + }); + const a1 = tab.assessments.find((x) => x.assessmentId === 1)!; + const a2 = tab.assessments.find((x) => x.assessmentId === 2)!; + + expect(a1.points).toBe(0); + expect(a1.effectiveWeight).toBe(0); + expect(a2.points).toBe(0); + expect(a2.effectiveWeight).toBe(0); + }); + + it('kept assessment points/effectiveWeight are computed over keptCount=1', () => { + // tab weight=60, keptCount=1 → effectiveWeight=60/1=60; ratio=1.0 → points=(1.0/1)*60=60 + const [tab] = computeStudentBreakdown({ + studentId: 1, + tabs: [dropConfig.tab], + assessments: dropConfig.assessments, + submissions: dropConfig.submissions, + }); + const a3 = tab.assessments.find((x) => x.assessmentId === 3)!; + + expect(a3.effectiveWeight).toBeCloseTo(60); + expect(a3.points).toBeCloseTo(60); + }); + + it('excluded=false and dropped=false on kept assessment', () => { + const [tab] = computeStudentBreakdown({ + studentId: 1, + tabs: [dropConfig.tab], + assessments: dropConfig.assessments, + submissions: dropConfig.submissions, + }); + const a3 = tab.assessments.find((x) => x.assessmentId === 3)!; + expect(a3.excluded).toBe(false); + expect(a3.dropped).toBe(false); + }); + + it('no keepHighest (0): dropped=false for all assessments', () => { + const [tab] = computeStudentBreakdown({ + studentId: 1, + tabs: [{ id: 10, title: 'Tab', categoryId: 0, gradebookWeight: 60 }], + assessments: dropConfig.assessments, + submissions: dropConfig.submissions, + }); + tab.assessments.forEach((a) => expect(a.dropped).toBe(false)); + }); + + it('tie-break: equal ratios → lower id is dropped first', () => { + // a1 (id=1) and a2 (id=2) both have ratio 0.5; keepHighest=1. + // Tie-break ascending by id → a1 is ranked lower → a1 is dropped; a2 is kept. + const tieAssessments = [ + { id: 1, tabId: 10, maxGrade: 100, title: 'A' }, + { id: 2, tabId: 10, maxGrade: 100, title: 'B' }, + ]; + const [tab] = computeStudentBreakdown({ + studentId: 1, + tabs: [ + { + id: 10, + title: 'Tab', + categoryId: 0, + gradebookWeight: 60, + keepHighest: 1, + }, + ], + assessments: tieAssessments, + submissions: subs([ + { studentId: 1, assessmentId: 1, grade: 50 }, + { studentId: 1, assessmentId: 2, grade: 50 }, + ]), + }); + const a1 = tab.assessments.find((x) => x.assessmentId === 1)!; + const a2 = tab.assessments.find((x) => x.assessmentId === 2)!; + expect(a1.dropped).toBe(true); // lower id dropped + expect(a2.dropped).toBe(false); // higher id kept + }); +}); + +describe('computeWeightedRows — keepHighest', () => { + const keepStudent = [ + { + id: 1, + name: 'Alice', + email: 'alice@e.com', + externalId: null, + level: 1, + totalXp: 0, + }, + ]; + + it('subtotal for tab with keepHighest=1 equals average of the single top ratio', () => { + // top ratio among a1,a2,a3 = 1.0; subtotal = 1.0 + const rows = computeWeightedRows({ + students: keepStudent, + tabs: [dropConfig.tab], + assessments: dropConfig.assessments, + submissions: dropConfig.submissions, + }); + expect(rows[0].subtotals[0]).toBeCloseTo(1.0); + }); + + it('total reflects keepHighest subtotal multiplied by tab weight', () => { + // subtotal=1.0, tabWeight=60 → total=60 + const rows = computeWeightedRows({ + students: keepStudent, + tabs: [dropConfig.tab], + assessments: dropConfig.assessments, + submissions: dropConfig.submissions, + }); + expect(rows[0].total).toBeCloseTo(60); + }); +}); diff --git a/client/app/bundles/course/gradebook/__tests__/store.test.ts b/client/app/bundles/course/gradebook/__tests__/store.test.ts index 97143ade93..a1a2f1a18c 100644 --- a/client/app/bundles/course/gradebook/__tests__/store.test.ts +++ b/client/app/bundles/course/gradebook/__tests__/store.test.ts @@ -151,3 +151,70 @@ describe('UPDATE_TAB_WEIGHTS reducer', () => { ); }); }); + +describe('UPDATE_TAB_WEIGHTS — keepHighest', () => { + it('writes keepHighest onto the matching tab', () => { + const state = { + ...baseState, + tabs: [{ id: 10, title: 'T', categoryId: 1, gradebookWeight: 50 }], + }; + const next = reducer( + state, + actions.updateTabWeights({ + weights: [{ tabId: 10, weight: 50, keepHighest: 2 }], + }), + ); + expect(next.tabs[0].keepHighest).toBe(2); + }); + + it('defaults keepHighest to 0 when omitted from the payload', () => { + const state = { + ...baseState, + tabs: [ + { + id: 10, + title: 'T', + categoryId: 1, + gradebookWeight: 50, + keepHighest: 3, + }, + ], + }; + const next = reducer( + state, + actions.updateTabWeights({ + weights: [{ tabId: 10, weight: 50 }], + }), + ); + expect(next.tabs[0].keepHighest).toBe(0); + }); + + it('does not modify keepHighest on other tabs', () => { + const state = { + ...baseState, + tabs: [ + { + id: 10, + title: 'T1', + categoryId: 1, + gradebookWeight: 50, + keepHighest: 1, + }, + { + id: 11, + title: 'T2', + categoryId: 1, + gradebookWeight: 50, + keepHighest: 2, + }, + ], + }; + const next = reducer( + state, + actions.updateTabWeights({ + weights: [{ tabId: 10, weight: 50, keepHighest: 5 }], + }), + ); + expect(next.tabs[1].keepHighest).toBe(2); + }); +}); diff --git a/client/app/bundles/course/gradebook/components/ConfigureWeightsPrompt.tsx b/client/app/bundles/course/gradebook/components/ConfigureWeightsPrompt.tsx index 0783ab32f7..75d07fd63b 100644 --- a/client/app/bundles/course/gradebook/components/ConfigureWeightsPrompt.tsx +++ b/client/app/bundles/course/gradebook/components/ConfigureWeightsPrompt.tsx @@ -51,10 +51,36 @@ const translations = defineMessages({ defaultMessage: "Choose Equal (all assessments share the tab's weight) or Custom (set each assessment's share).", }, - descriptionDrop: { - id: 'course.gradebook.ConfigureWeightsPrompt.descriptionDrop', + keepHighestLabel: { + id: 'course.gradebook.ConfigureWeightsPrompt.keepHighestLabel', + defaultMessage: 'Keep highest', + }, + keepHighestAria: { + id: 'course.gradebook.ConfigureWeightsPrompt.keepHighestAria', + defaultMessage: 'Keep highest for {tab}', + }, + keepHighestToggleAria: { + id: 'course.gradebook.ConfigureWeightsPrompt.keepHighestToggleAria', + defaultMessage: 'Enable keep highest for {tab}', + }, + keepInvalid: { + id: 'course.gradebook.ConfigureWeightsPrompt.keepInvalid', + defaultMessage: 'Keep at least 1 (whole number)', + }, + keepSubtitle: { + id: 'course.gradebook.ConfigureWeightsPrompt.keepSubtitle', + defaultMessage: + '— keeps highest {keep} of {included} · each counts as {pct}%', + }, + keepOverflowWarning: { + id: 'course.gradebook.ConfigureWeightsPrompt.keepOverflowWarning', defaultMessage: - "In Equal mode, optionally drop each student's N lowest-scoring assessments before averaging.", + '"{tab}" keeps more assessments than it contains — all of them will count.', + }, + descriptionKeep: { + id: 'course.gradebook.ConfigureWeightsPrompt.descriptionKeep', + defaultMessage: + "In Equal mode, optionally keep only each student's N highest-scoring assessments.", }, total: { id: 'course.gradebook.ConfigureWeightsPrompt.total', @@ -194,6 +220,13 @@ const ConfigureWeightsPrompt: FC = ({ const seedExclusions = (): Record => Object.fromEntries(assessments.map((a) => [a.id, !!a.gradebookExcluded])); + const seedKeepHighest = (): Record => + Object.fromEntries(resolvedTabs.map((tb) => [tb.id, tb.keepHighest ?? 0])); + const seedKeepOn = (): Record => + Object.fromEntries( + resolvedTabs.map((tb) => [tb.id, (tb.keepHighest ?? 0) > 0]), + ); + const [weights, setWeights] = useState>(seedWeights); const [modes, setModes] = useState>(seedModes); const [assessmentWeights, setAssessmentWeights] = useState< @@ -201,6 +234,9 @@ const ConfigureWeightsPrompt: FC = ({ >(seedAssessmentWeights); const [excluded, setExcluded] = useState>(seedExclusions); + const [keepHighest, setKeepHighest] = + useState>(seedKeepHighest); + const [keepOn, setKeepOn] = useState>(seedKeepOn); const [expanded, setExpanded] = useState>({}); const [submitting, setSubmitting] = useState(false); @@ -210,6 +246,8 @@ const ConfigureWeightsPrompt: FC = ({ setModes(seedModes()); setAssessmentWeights(seedAssessmentWeights()); setExcluded(seedExclusions()); + setKeepHighest(seedKeepHighest()); + setKeepOn(seedKeepOn()); setExpanded({}); } }, [open]); @@ -239,12 +277,42 @@ const ConfigureWeightsPrompt: FC = ({ const effectiveWeight = (tabId: number): number => isAllExcluded(tabId) ? 0 : weights[tabId] ?? 0; + const validateKeep = (value: number): string | null => + Number.isInteger(value) && value >= 1 ? null : t(translations.keepInvalid); + + // Returns the effective keep-N value when the feature is active for a tab, + // or null when not applicable (mode != equal, checkbox off, or only 1 included). + const activeKeepValue = (tabId: number): number | null => { + if ((modes[tabId] ?? 'equal') !== 'equal') return null; + if (!keepOn[tabId]) return null; + return keepHighest[tabId] ?? 0; + }; + + const handleKeepChange = (tabId: number, raw: string): void => { + const parsed = raw === '' ? 0 : Number(raw); + setKeepHighest((prev) => ({ ...prev, [tabId]: parsed })); + }; + const sum = tabs.reduce((acc, tb) => acc + effectiveWeight(tb.id), 0); const hasInvalid = Object.values(weights).some((w) => validate(w) !== null) || - Object.values(assessmentWeights).some((w) => validate(w) !== null); + Object.values(assessmentWeights).some((w) => validate(w) !== null) || + tabs.some((tb) => { + if ((modes[tb.id] ?? 'equal') !== 'equal') return false; + if (!keepOn[tb.id]) return false; + const includedCount = tabIncludedIds(tb.id).length; + if (includedCount <= 1) return false; + return validateKeep(keepHighest[tb.id] ?? 0) !== null; + }); const hasUnbalanced = tabs.some((tb) => isUnbalanced(tb.id)); + const keepOverflowTabs = tabs.filter((tb) => { + const v = activeKeepValue(tb.id); + if (!v) return false; + const included = tabIncludedIds(tb.id).length; + return included > 1 && v > included; + }); + const handleChange = (tabId: number, raw: string): void => { const parsed = raw === '' ? 0 : Number(raw); setWeights((prev) => ({ ...prev, [tabId]: parsed })); @@ -289,6 +357,11 @@ const ConfigureWeightsPrompt: FC = ({ tabId: tb.id, weight: weights[tb.id] ?? 0, weightMode: mode, + keepHighest: + activeKeepValue(tb.id) !== null && + tabIncludedIds(tb.id).length > 1 + ? keepHighest[tb.id] ?? 0 + : 0, excludedAssessmentIds: tabAssessmentIds(tb.id).filter( (id) => excluded[id], ), @@ -336,6 +409,7 @@ const ConfigureWeightsPrompt: FC = ({ translations.descriptionWeights, translations.descriptionExclusion, translations.descriptionModes, + translations.descriptionKeep, ].map((key) => ( = ({ const excludedCount = tabAssessments.length - includedCount; const pct = includedCount > 0 ? r2(value / includedCount) : 0; + // keep-highest vars + const keepValue = keepHighest[tb.id] ?? 0; + const keepDisabled = includedCount <= 1; + const keepEnabled = !keepDisabled && !!keepOn[tb.id]; + const keepErr = keepEnabled ? validateKeep(keepValue) : null; + const kept = Math.min(keepValue, includedCount); + const keepPct = kept > 0 ? r2(value / kept) : 0; + const isKeepOverflow = keepOverflowTabs.some( + (ot) => ot.id === tb.id, + ); + return (
@@ -455,6 +540,104 @@ const ConfigureWeightsPrompt: FC = ({ {err} )} + {mode === 'equal' && !noAssessments && ( + <> +
+ { + if (keepEnabled) { + // Turn off: just toggle the on flag + setKeepOn((prev) => ({ + ...prev, + [tb.id]: false, + })); + } else { + // Turn on: seed default value if not already set + if (keepValue === 0) { + setKeepHighest((prev) => ({ + ...prev, + [tb.id]: Math.max(1, includedCount - 1), + })); + } + setKeepOn((prev) => ({ + ...prev, + [tb.id]: true, + })); + } + }} + size="small" + /> + + {t(translations.keepHighestLabel)} + + {keepEnabled && ( + + handleKeepChange(tb.id, e.target.value) + } + onKeyDown={(e) => { + if ( + ['.', ',', 'e', 'E', '-', '+'].includes( + e.key, + ) + ) + e.preventDefault(); + }} + size="small" + sx={{ width: 80, mx: 0.5 }} + type="number" + value={keepValue} + /> + )} + {keepEnabled && keepErr === null && ( + + {t(translations.keepSubtitle, { + keep: kept, + included: includedCount, + pct: keepPct.toFixed(2), + })} + + )} +
+ {keepEnabled && keepErr && ( + + {keepErr} + + )} + {isKeepOverflow && ( + + {t(translations.keepOverflowWarning, { + tab: tb.title, + })} + + )} + + )} {unbalanced && ( {t(translations.unbalanced, { tab: tb.title })} @@ -537,6 +720,14 @@ const ConfigureWeightsPrompt: FC = ({
); } + let caption = t(translations.ofGrade, { + pct: pct.toFixed(2), + }); + if (isExcluded) { + caption = t(translations.excluded); + } else if (keepEnabled) { + caption = '—'; + } return (
= ({ color="text.disabled" variant="caption" > - {isExcluded - ? t(translations.excluded) - : t(translations.ofGrade, { - pct: pct.toFixed(2), - })} + {caption}
); diff --git a/client/app/bundles/course/gradebook/components/GradebookWeightedTable.tsx b/client/app/bundles/course/gradebook/components/GradebookWeightedTable.tsx index 671a6c7fdb..07d73b5f8d 100644 --- a/client/app/bundles/course/gradebook/components/GradebookWeightedTable.tsx +++ b/client/app/bundles/course/gradebook/components/GradebookWeightedTable.tsx @@ -21,6 +21,8 @@ import { Tooltip, Typography, } from '@mui/material'; +import type { Theme } from '@mui/material/styles'; +import { lighten } from '@mui/material/styles'; import type { AssessmentData, CategoryData, @@ -40,7 +42,11 @@ import { DEFAULT_TABLE_ROWS_PER_PAGE } from 'lib/constants/sharedConstants'; import useTranslation from 'lib/hooks/useTranslation'; import tableTranslations from 'lib/translations/table'; -import type { AssessmentContribution, WeightedRow } from '../computeWeighted'; +import type { + AssessmentContribution, + TabBreakdown, + WeightedRow, +} from '../computeWeighted'; import { computeStudentBreakdown, computeWeightedRows, @@ -160,6 +166,10 @@ const translations = defineMessages({ id: 'course.gradebook.GradebookWeightedTable.excluded', defaultMessage: 'Excluded', }, + dropped: { + id: 'course.gradebook.GradebookWeightedTable.dropped', + defaultMessage: 'Dropped (lowest)', + }, total: { id: 'course.gradebook.GradebookWeightedTable.total', defaultMessage: 'Total', @@ -290,35 +300,59 @@ const GradebookWeightedTable = ({ return a.points; }; - const [expandedIds, setExpandedIds] = useState>(new Set()); + // Single-open accordion: auditing one student at a time. Replaces the former + // multi-expand Set so only the focused student's breakdown is on screen (and + // only one summary row ever pins under the header). + const [expandedId, setExpandedId] = useState(null); const toggleExpanded = (studentId: number): void => - setExpandedIds((prev) => { - const next = new Set(prev); - if (next.has(studentId)) next.delete(studentId); - else next.add(studentId); - return next; - }); + setExpandedId((prev) => (prev === studentId ? null : studentId)); - const breakdownsByStudent = useMemo( + const expandedBreakdown = useMemo( () => - new Map( - [...expandedIds].map((studentId) => [ - studentId, - computeStudentBreakdown({ - studentId, + expandedId === null + ? null + : computeStudentBreakdown({ + studentId: expandedId, tabs: resolvedTabs, assessments, submissions, }), - ]), - ), - [expandedIds, resolvedTabs, assessments, submissions], + [expandedId, resolvedTabs, assessments, submissions], ); + const containerRef = useRef(null); + const activeRowRef = useRef(null); const row1Ref = useRef(null); const row2Ref = useRef(null); + const row3Ref = useRef(null); const [row2Top, setRow2Top] = useState(0); const [row3Top, setRow3Top] = useState(0); + // Full header height = where the pinned summary row sticks, and the scroll + // offset that lands a focused row just beneath the header. + const [headerHeight, setHeaderHeight] = useState(0); + + // sx for a cell of the focused (expanded) student's summary row. `variant` + // selects the layering: 'checkbox' and 'name' are corner-sticky (top + the + // left freeze they already carry) and sit above the body's frozen column; + // 'data' cells pin on top only. The checkbox cell also carries the left + // accent bar that marks "this is the student you're auditing". + const pinnedCellSx = + (variant: 'checkbox' | 'name' | 'data') => + (theme: Theme): Record => { + const isLead = variant !== 'data'; + return { + position: 'sticky', + top: headerHeight, + zIndex: isLead ? 6 : 3, + // Opaque tint (not alpha) so scrolled body content can't bleed through + // the sticky cell. + backgroundColor: lighten(theme.palette.primary.light, 0.96), + // The checkbox cell carries the left accent bar marking the audited row. + ...(variant === 'checkbox' && { + boxShadow: `inset 3px 0 0 ${theme.palette.primary.main}`, + }), + }; + }; // Row-3 subheader for a tab: "Excluded" when the tab contributes nothing, // else the weight in the active lens ("/{w}" points / "{w}% of grade"). @@ -346,25 +380,54 @@ const GradebookWeightedTable = ({ useLayoutEffect(() => { const row1 = row1Ref.current; const row2 = row2Ref.current; - if (!row1 || !row2) return undefined; + const row3 = row3Ref.current; + if (!row1 || !row2 || !row3) return undefined; // Re-measure on every header-row resize, not just on mount. Expanding or // collapsing a row, switching display mode and showing/hiding columns all // reflow the header after mount; with a one-shot measurement rows 2–3 keep // a stale `top` and stay permanently dislodged from the rows above them. const measure = (): void => { - const h1 = row1.offsetHeight; + // getBoundingClientRect (subpixel) not offsetHeight (integer-rounded): + // rows are fractional (32px min + lineHeight content), and a rounded + // `top` lands the stuck rows 2-3 a fraction off — opening thin gaps + // between header rows (body bleeds through) and overshooting the single + // rowSpan=3 frozen-left cell so the right header reads a touch taller. + const h1 = row1.getBoundingClientRect().height; + const h2 = row2.getBoundingClientRect().height; + const h3 = row3.getBoundingClientRect().height; setRow2Top(h1); - setRow3Top(h1 + row2.offsetHeight); + setRow3Top(h1 + h2); + setHeaderHeight(h1 + h2 + h3); }; measure(); const observer = new ResizeObserver(measure); observer.observe(row1); observer.observe(row2); + observer.observe(row3); return () => observer.disconnect(); }, [visibleCategories, resolvedTabs]); + // On expand, glide the focused row to just beneath the header so its + // breakdown is guaranteed in view (even when the clicked row was near the + // bottom). getBoundingClientRect keeps this correct regardless of the row's + // offsetParent; scrollTo is optional-chained because jsdom lacks it. + useLayoutEffect(() => { + if (expandedId === null) return; + const container = containerRef.current; + const rowEl = activeRowRef.current; + if (!container || !rowEl) return; + const prefersReducedMotion = + window.matchMedia?.('(prefers-reduced-motion: reduce)').matches ?? false; + const delta = + rowEl.getBoundingClientRect().top - container.getBoundingClientRect().top; + container.scrollTo?.({ + top: container.scrollTop + delta - headerHeight, + behavior: prefersReducedMotion ? 'auto' : 'smooth', + }); + }, [expandedId, headerHeight]); + const rows = useMemo( () => computeWeightedRows({ @@ -622,6 +685,7 @@ const GradebookWeightedTable = ({ toolbar — plus the pagination below, so the table fills the remaining viewport; shorter classes shrink to fit (no whitespace). */} too — dropping the table's bottom edge and + // that row's right edge (the open bottom-right corner). The + // separator ABOVE it is owned by the previous row's borderBottom + // (that row isn't last-child, so it keeps it), so only the bottom + // and right edges need restoring here. Same (0,2,3) > (0,2,1). + '& tbody tr:last-of-type td': { + borderBottom: gridLine, + borderRight: gridLine, + }, + // The two frozen columns (checkbox + Name) are each their own + // sticky compositing layer; when scrolled, the next row's + // opaque sticky bg paints over the cell's borderBottom and the + // horizontal line vanishes in those columns only. Mirror the + // header fix (CLAUDE-tables.md): drop the coverable borderBottom + // and draw each separator as the lower cell's borderTop, which + // its own layer owns and always paints. Row 1's top line stays + // owned by the sticky header above it, so only rows 2+ get the + // borderTop — else both borders show at rest and read 2px thick. + '& tbody td:first-of-type, & tbody td:nth-of-type(2)': { + borderBottom: 'none', + }, + '& tbody tr:not(:first-of-type) td:first-of-type, & tbody tr:not(:first-of-type) td:nth-of-type(2)': + { + borderTop: gridLine, + }, }; }} > @@ -803,6 +893,7 @@ const GradebookWeightedTable = ({ {/* Row 3: Weight subheaders */} {resolvedTabs.map((tab) => ( @@ -842,10 +933,10 @@ const GradebookWeightedTable = ({ {body.rows.map((row, idx) => { const rowProps = body.forEachRow(row, idx); const studentId = row.original.studentId; - const isExpanded = expandedIds.has(studentId); + const isExpanded = expandedId === studentId; return ( - + {/* Body sticky-left cells sit at zIndex 1 — strictly below the header's sticky cells (MUI gives every stickyHeader cell zIndex 2). On a z-index tie the cell @@ -854,16 +945,19 @@ const GradebookWeightedTable = ({ column the frozen Name cell (z4) doesn't cover — i.e. the identity columns once they're toggled on. */} {showEmail && ( - {row.original.email} + + {row.original.email} + )} {showExternalId && ( - {row.original.externalId ?? ''} + + {row.original.externalId ?? ''} + )} {row.original.subtotals.map((subtotal, i) => { const weight = resolvedTabs[i].gradebookWeight ?? 0; return ( - + {fmtDisplay( tabDisplayValue(subtotal, weight), columnPrecisions.tabs[i], @@ -918,7 +1027,10 @@ const GradebookWeightedTable = ({ ); })} - + {fmtDisplay( totalDisplayValue(row.original.total), columnPrecisions.total, @@ -926,47 +1038,50 @@ const GradebookWeightedTable = ({ {isExpanded && - (breakdownsByStudent.get(studentId) ?? []).flatMap( - (tb, tabIdx) => - tb.assessments.map((a) => { - const isExcluded = a.excluded; - // Weightage is always "% of grade" — it never - // follows the points/percent lens. - const weightText = t( - translations.percentOfGrade, - { - weight: - Math.round(a.effectiveWeight * 100) / 100, - }, - ); - const gradeText = - a.grade === null - ? `—/${a.maxGrade}` - : `${a.grade}/${a.maxGrade}`; - return ( - + tb.assessments.map((a) => { + const isExcluded = a.excluded; + const isDropped = a.dropped; + const isInactive = isExcluded || isDropped; + // Weightage is always "% of grade" — it never + // follows the points/percent lens. + const weightText = t(translations.percentOfGrade, { + weight: Math.round(a.effectiveWeight * 100) / 100, + }); + const gradeText = + a.grade === null + ? `—/${a.maxGrade}` + : `${a.grade}/${a.maxGrade}`; + let statusText = weightText; + if (isExcluded) { + statusText = t(translations.excluded); + } else if (isDropped) { + statusText = t(translations.dropped); + } + return ( + + {/* Empty checkbox cell so the breakdown row + carries the same checkbox | name divider (the + universal cell borderRight) as the rows above. */} + - {/* Empty checkbox cell so the breakdown row - carries the same checkbox | name divider (the - universal cell borderRight) as the rows above. */} - - {/* Title over a muted "raw mark · weightage" + /> + {/* Title over a muted "raw mark · weightage" subtitle, stacked and confined to the (sticky) Name column. The breakdown row freezes the same checkbox | Name region as the student rows above — @@ -976,74 +1091,74 @@ const GradebookWeightedTable = ({ indent sits the title under the student name (past the expand chevron), signalling these are that student's assessments. */} - - {/* nowrap keeps the title on one line: its + + {/* nowrap keeps the title on one line: its max-content width then drives the table's auto layout, expanding the (frozen) Name column to fit the longest title. With the metadata line also nowrap, every breakdown row is exactly 2 lines — no fixed widths, no JS measurement. */} - - {a.title} - - {/* Muted metadata on its own line below the + + {a.title} + + {/* Muted metadata on its own line below the title: raw mark · effective weightage, kept on one line (nowrap). Weightage is always "% of grade" — never routed through the points/percent lens. */} - - {`${gradeText} · ${isExcluded ? t(translations.excluded) : weightText}`} - - - {/* One empty cell per visible identity column so + + {`${gradeText} · ${statusText}`} + + + {/* One empty cell per visible identity column so the grid lines stay aligned with the rows above. These scroll with the table (only checkbox + Name are frozen), matching the student rows. */} - {showEmail && } - {showExternalId && } - {resolvedTabs.map((tab, i) => { - const tabCellValue = isExcluded - ? '—' - : fmtDisplay( - breakdownDisplayValue(a), - columnPrecisions.tabs[i], - ); - return ( - - {i === tabIdx ? tabCellValue : ''} - - ); - })} - - - ); - }), + {showEmail && } + {showExternalId && } + {resolvedTabs.map((tab, i) => { + const tabCellValue = isExcluded + ? '—' + : fmtDisplay( + breakdownDisplayValue(a), + columnPrecisions.tabs[i], + ); + return ( + + {i === tabIdx ? tabCellValue : ''} + + ); + })} + + + ); + }), )} ); diff --git a/client/app/bundles/course/gradebook/computeWeighted.ts b/client/app/bundles/course/gradebook/computeWeighted.ts index d4bb0424f7..f490594d93 100644 --- a/client/app/bundles/course/gradebook/computeWeighted.ts +++ b/client/app/bundles/course/gradebook/computeWeighted.ts @@ -28,6 +28,7 @@ export interface AssessmentContribution { // Custom mode: the assessment's own configured weight. effectiveWeight: number; excluded: boolean; + dropped: boolean; // equal-mode keep-highest: ranked out for this student } export interface TabBreakdown { @@ -65,19 +66,25 @@ const buildAssessmentsByTab = ( // Equal-weight formula: average of (grade/maxGrade) ratios over INCLUDED assessments. // Excluded assessments are dropped from both numerator and count; ungraded included -// contribute 0. Returns null when no assessment is included. +// contribute 0. When keepN > 0, only the top keepN ratios are averaged. +// Returns null when no assessment is included. const equalSubtotal = ( studentId: number, + tab: TabData, tabAssessments: AssessmentData[], gradeLookup: GradeLookup, ): number | null => { const included = tabAssessments.filter((a) => !a.gradebookExcluded); if (included.length === 0) return null; + const keepN = tab.keepHighest ?? 0; const ratios = included.map((a) => { const grade = gradeLookup.get(gradeKey(studentId, a.id)); - return grade != null ? grade / a.maxGrade : 0; + return grade != null && a.maxGrade > 0 ? grade / a.maxGrade : 0; }); - return ratios.reduce((acc, r) => acc + r, 0) / ratios.length; + ratios.sort((x, y) => x - y); // ascending + const keep = keepN > 0 ? Math.min(keepN, included.length) : included.length; + const kept = ratios.slice(included.length - keep); // the `keep` highest + return kept.reduce((acc, r) => acc + r, 0) / kept.length; }; // Custom-weight formula: Σ(grade_i/maxGrade_i × assessmentWeight_i) / tabWeight over @@ -97,7 +104,8 @@ const customSubtotal = ( if (a.gradebookExcluded) return; const grade = gradeLookup.get(gradeKey(studentId, a.id)); const assessmentWeight = a.gradebookWeight ?? 0; - if (grade != null) numerator += (grade / a.maxGrade) * assessmentWeight; + if (grade != null && a.maxGrade > 0) + numerator += (grade / a.maxGrade) * assessmentWeight; hasContributing = true; }); return hasContributing ? numerator / tabWeight : null; @@ -114,7 +122,7 @@ const subtotalFromLookup = ( if (tab.weightMode === 'custom') { return customSubtotal(studentId, tab, tabAssessments, gradeLookup); } - return equalSubtotal(studentId, tabAssessments, gradeLookup); + return equalSubtotal(studentId, tab, tabAssessments, gradeLookup); }; // Weighted, additive total from already-computed subtotals. @@ -189,23 +197,47 @@ export const computeStudentBreakdown = ({ return tabs.map((tab) => { const list = assessmentsByTab.get(tab.id) ?? []; const weight = tab.gradebookWeight ?? 0; - const includedCount = list.filter((a) => !a.gradebookExcluded).length; + const included = list.filter((a) => !a.gradebookExcluded); + const includedCount = included.length; + + let droppedIds = new Set(); + let keptCount = includedCount; + if (tab.weightMode !== 'custom' && includedCount > 0) { + const keepN = tab.keepHighest ?? 0; + keptCount = keepN > 0 ? Math.min(keepN, includedCount) : includedCount; + if (keptCount < includedCount) { + const ranked = included + .map((a) => { + const grade = gradeLookup.get(gradeKey(studentId, a.id)); + return { + id: a.id, + ratio: grade != null && a.maxGrade > 0 ? grade / a.maxGrade : 0, + }; + }) + .sort((x, y) => x.ratio - y.ratio || x.id - y.id); // ascending: lowest first, tie-break by id + // Drop the lowest (includedCount − keptCount). + droppedIds = new Set( + ranked.slice(0, includedCount - keptCount).map((r) => r.id), + ); + } + } const contributions = list.map((a) => { const excluded = !!a.gradebookExcluded; + const dropped = droppedIds.has(a.id); const grade = gradeLookup.get(gradeKey(studentId, a.id)) ?? null; - const ratio = grade != null ? grade / a.maxGrade : 0; + const ratio = grade != null && a.maxGrade > 0 ? grade / a.maxGrade : 0; let points: number; let effectiveWeight: number; - if (excluded) { + if (excluded || dropped) { points = 0; effectiveWeight = 0; } else if (tab.weightMode === 'custom') { points = ratio * (a.gradebookWeight ?? 0); effectiveWeight = a.gradebookWeight ?? 0; } else { - points = includedCount > 0 ? (ratio / includedCount) * weight : 0; - effectiveWeight = includedCount > 0 ? weight / includedCount : 0; + points = keptCount > 0 ? (ratio / keptCount) * weight : 0; + effectiveWeight = keptCount > 0 ? weight / keptCount : 0; } return { assessmentId: a.id, @@ -215,6 +247,7 @@ export const computeStudentBreakdown = ({ points, effectiveWeight, excluded, + dropped, }; }); return { tabId: tab.id, assessments: contributions }; diff --git a/client/app/bundles/course/gradebook/store.ts b/client/app/bundles/course/gradebook/store.ts index 4777e16b3b..c8267ed793 100644 --- a/client/app/bundles/course/gradebook/store.ts +++ b/client/app/bundles/course/gradebook/store.ts @@ -70,6 +70,7 @@ const reducer = produce( tabId, weight, weightMode, + keepHighest, assessmentWeights, excludedAssessmentIds, }) => { @@ -77,6 +78,7 @@ const reducer = produce( if (tab) { tab.gradebookWeight = weight; tab.weightMode = weightMode; + tab.keepHighest = keepHighest ?? 0; } const excludedSet = new Set(excludedAssessmentIds ?? []); const tabAssessments = draft.assessments.filter( diff --git a/client/app/types/course/gradebook.ts b/client/app/types/course/gradebook.ts index c9009afbfe..2d72bcb1c0 100644 --- a/client/app/types/course/gradebook.ts +++ b/client/app/types/course/gradebook.ts @@ -9,6 +9,7 @@ export interface TabData { categoryId: number; gradebookWeight?: number; weightMode?: 'equal' | 'custom'; + keepHighest?: number; } export interface AssessmentData { @@ -52,6 +53,7 @@ export interface UpdateWeightsPayload { tabId: number; weight: number; weightMode?: 'equal' | 'custom'; + keepHighest?: number; excludedAssessmentIds?: number[]; assessmentWeights?: { assessmentId: number; weight: number }[]; }[]; diff --git a/client/locales/en.json b/client/locales/en.json index 1d9bf1cce6..bca35e87f7 100644 --- a/client/locales/en.json +++ b/client/locales/en.json @@ -9383,6 +9383,9 @@ "course.gradebook.GradebookWeightedTable.email": { "defaultMessage": "Email" }, + "course.gradebook.GradebookWeightedTable.dropped": { + "defaultMessage": "Dropped (lowest)" + }, "course.gradebook.GradebookWeightedTable.excluded": { "defaultMessage": "Excluded" }, diff --git a/db/schema.rb b/db/schema.rb index d1a22b40e8..75ec3ce3e2 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.2].define(version: 2026_06_11_000000) do +ActiveRecord::Schema[7.2].define(version: 2026_06_11_130000) 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.boolean "gradebook_excluded", default: false, 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/gradebook_controller_spec.rb b/spec/controllers/course/gradebook_controller_spec.rb index 59397f4120..aa94670ed1 100644 --- a/spec/controllers/course/gradebook_controller_spec.rb +++ b/spec/controllers/course/gradebook_controller_spec.rb @@ -297,6 +297,26 @@ def weight_for(tab) end end + describe '#update_weights keepHighest' do + render_views + + let(:category2) { create(:course_assessment_category, course: course) } + let(:tab) { create(:course_assessment_tab, category: category2) } + + before { controller_sign_in(controller, manager.user) } + + it 'persists keepHighest and echoes it back' do + post :update_weights, as: :json, params: { + course_id: course.id, + weights: [{ tabId: tab.id, weight: '50', weightMode: 'equal', keepHighest: 2 }] + } + expect(response).to have_http_status(:ok) + body = JSON.parse(response.body) + expect(body['weights'].first['keepHighest']).to eq(2) + expect(Course::Gradebook::Contribution.find_by(tab_id: tab.id).keep_highest).to eq(2) + end + end + describe '#update_weights with modes' do render_views @@ -421,6 +441,16 @@ def weight_for(tab) expect(body['assessments'].first).to have_key('gradebookExcluded') expect(body['assessments'].first['gradebookExcluded']).to eq(false) end + + it 'includes keepHighest in the weighted tabs response' do + contribution.update!(keep_highest: 3) + 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('keepHighest') + expect(tab_json['keepHighest']).to eq(3) + end end end end diff --git a/spec/models/course/gradebook/contribution_spec.rb b/spec/models/course/gradebook/contribution_spec.rb index d4d433e028..06e215a507 100644 --- a/spec/models/course/gradebook/contribution_spec.rb +++ b/spec/models/course/gradebook/contribution_spec.rb @@ -208,6 +208,29 @@ def excluded?(assessment) expect(excluded?(a1)).to eq(false) end end + + context 'with keep_highest' do + it 'persists keep_highest in equal mode' do + described_class.bulk_update( + course: course, + updates: [{ tab_id: tab1.id, weight: 50, weight_mode: 'equal', keep_highest: 3 }] + ) + expect(described_class.find_by(tab_id: tab1.id).keep_highest).to eq(3) + end + + it 'accepts 0 as a valid keep_highest value' do + described_class.bulk_update( + course: course, + updates: [{ tab_id: tab1.id, weight: 50, weight_mode: 'equal', keep_highest: 5 }] + ) + expect(described_class.find_by(tab_id: tab1.id).keep_highest).to eq(5) + described_class.bulk_update( + course: course, + updates: [{ tab_id: tab1.id, weight: 50, weight_mode: 'equal', keep_highest: 0 }] + ) + expect(described_class.find_by(tab_id: tab1.id).keep_highest).to eq(0) + end + end end end end