Skip to content

Commit d4cb3aa

Browse files
committed
feat(gradebook): wire keepHighest through backend for keep-N feature
- Permit keepHighest in update_weights_params - Parse keepHighest from request and pass to bulk_update via keep_highest - Echo keepHighest in serialize_weight_updates response - Serialize keepHighest from contribution in index.json.jbuilder - Add controller tests: round-trip persist and index response - Add model tests: bulk_update persists keep_highest in equal mode
1 parent ecaab8f commit d4cb3aa

14 files changed

Lines changed: 752 additions & 21 deletions

File tree

app/controllers/course/gradebook_controller.rb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ def parse_weight_entry(entry)
4747
tab_id: entry[:tabId].to_i,
4848
weight: entry[:weight].to_f,
4949
weight_mode: entry[:weightMode] || 'equal',
50+
keep_highest: entry[:keepHighest].to_i,
5051
excluded_assessment_ids: (entry[:excludedAssessmentIds] || []).map(&:to_i),
5152
assessment_weights: (entry[:assessmentWeights] || []).map do |aw|
5253
{ assessment_id: aw[:assessmentId].to_i, weight: aw[:weight].to_f }
@@ -56,14 +57,15 @@ def parse_weight_entry(entry)
5657

5758
def update_weights_params
5859
params.permit(
59-
weights: [:tabId, :weight, :weightMode,
60+
weights: [:tabId, :weight, :weightMode, :keepHighest,
6061
excludedAssessmentIds: [], assessmentWeights: [:assessmentId, :weight]]
6162
)
6263
end
6364

6465
def serialize_weight_updates(updates)
6566
updates.map do |u|
6667
entry = { tabId: u[:tab_id], weight: u[:weight], weightMode: u[:weight_mode].to_s,
68+
keepHighest: u[:keep_highest],
6769
excludedAssessmentIds: u[:excluded_assessment_ids] }
6870
if u[:weight_mode].to_s == 'custom'
6971
entry[:assessmentWeights] = u[:assessment_weights].map do |aw|

app/views/course/gradebook/index.json.jbuilder

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ json.tabs @tabs do |tab|
1515
contribution = @tab_contributions[tab.id]
1616
json.gradebookWeight (contribution&.weight || 0).to_f
1717
json.weightMode(contribution&.weight_mode || 'equal')
18+
json.keepHighest(contribution&.keep_highest || 0)
1819
end
1920
end
2021

client/app/bundles/course/gradebook/__tests__/ConfigureWeightsPrompt.test.tsx

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@ describe('<ConfigureWeightsPrompt />', () => {
147147
tabId: 10,
148148
weight: 50,
149149
weightMode: 'custom',
150+
keepHighest: 0,
150151
excludedAssessmentIds: [],
151152
assessmentWeights: [
152153
{ assessmentId: 101, weight: 25 },
@@ -157,6 +158,7 @@ describe('<ConfigureWeightsPrompt />', () => {
157158
tabId: 11,
158159
weight: 50,
159160
weightMode: 'equal',
161+
keepHighest: 0,
160162
excludedAssessmentIds: [],
161163
},
162164
]);
@@ -404,6 +406,7 @@ describe('per-assessment exclusion', () => {
404406
expect(arg[0]).toMatchObject({
405407
tabId: 10,
406408
weight: 50,
409+
keepHighest: 0,
407410
excludedAssessmentIds: [101, 102],
408411
});
409412
});
@@ -477,4 +480,168 @@ describe('per-assessment exclusion', () => {
477480
expect(screen.queryByText(/no weights set yet/i)).not.toBeInTheDocument();
478481
});
479482
});
483+
484+
it('shows assessment weights sum footer in custom mode', () => {
485+
setup();
486+
fireEvent.click(
487+
within(modeGroup('Assignments')).getByRole('radio', { name: /custom/i }),
488+
);
489+
// Expand to see the footer
490+
// custom mode auto-expands, so just check the footer
491+
expect(screen.getByText(/Assessment weights:/i)).toBeInTheDocument();
492+
});
493+
});
494+
495+
describe('per-assessment exclusion (extended)', () => {
496+
beforeEach(() => jest.clearAllMocks());
497+
498+
it('shows "Excluded" label in custom mode for excluded assessment', async () => {
499+
setup({
500+
assessments: [
501+
{
502+
id: 101,
503+
title: A1,
504+
tabId: 10,
505+
maxGrade: 100,
506+
gradebookExcluded: true,
507+
},
508+
{ id: 102, title: A2, tabId: 10, maxGrade: 50 },
509+
],
510+
tabs: [
511+
{
512+
id: 10,
513+
title: 'Assignments',
514+
categoryId: 1,
515+
gradebookWeight: 50,
516+
weightMode: 'custom',
517+
},
518+
],
519+
});
520+
fireEvent.click(screen.getAllByRole('button', { name: '' })[0]);
521+
expect(await screen.findByText('Excluded')).toBeInTheDocument();
522+
});
523+
524+
it('does not show "Excluded" label in equal mode for excluded assessment', async () => {
525+
setup({
526+
assessments: [
527+
{
528+
id: 101,
529+
title: A1,
530+
tabId: 10,
531+
maxGrade: 100,
532+
gradebookExcluded: true,
533+
},
534+
{ id: 102, title: A2, tabId: 10, maxGrade: 50 },
535+
],
536+
});
537+
fireEvent.click(screen.getAllByRole('button', { name: '' })[0]);
538+
// In equal mode excluded shows "Excluded" text too
539+
expect(await screen.findByText('Excluded')).toBeInTheDocument();
540+
});
541+
});
542+
543+
describe('keep-highest control', () => {
544+
const TOGGLE = 'Enable keep highest for Assignments';
545+
const INPUT = 'Keep highest for Assignments';
546+
const three = [
547+
{ id: 101, title: A1, tabId: 10, maxGrade: 100 },
548+
{ id: 102, title: A2, tabId: 10, maxGrade: 50 },
549+
{ id: 103, title: 'Assignment 3', tabId: 10, maxGrade: 80 },
550+
];
551+
552+
beforeEach(() => jest.clearAllMocks());
553+
554+
it('renders a keep-highest checkbox; number field hidden until checked', () => {
555+
setup({ assessments: three });
556+
expect(screen.getByRole('checkbox', { name: TOGGLE })).toBeInTheDocument();
557+
expect(screen.queryByRole('spinbutton', { name: INPUT })).not.toBeInTheDocument();
558+
fireEvent.click(screen.getByRole('checkbox', { name: TOGGLE }));
559+
expect(screen.getByRole('spinbutton', { name: INPUT })).toBeInTheDocument();
560+
});
561+
562+
it('shows a visible "Keep highest" text label next to the checkbox', () => {
563+
setup({ assessments: three });
564+
expect(screen.getByText('Keep highest')).toBeInTheDocument();
565+
});
566+
567+
it('defaults the count to included − 1 when checked', () => {
568+
setup({ assessments: three }); // 3 assessments -> default to 2
569+
fireEvent.click(screen.getByRole('checkbox', { name: TOGGLE }));
570+
expect(screen.getByRole('spinbutton', { name: INPUT })).toHaveValue(2);
571+
});
572+
573+
it('hides checkbox + field in custom mode', () => {
574+
setup({ assessments: three });
575+
fireEvent.click(
576+
within(modeGroup('Assignments')).getByRole('radio', { name: /custom/i }),
577+
);
578+
expect(screen.queryByRole('checkbox', { name: TOGGLE })).not.toBeInTheDocument();
579+
expect(screen.queryByRole('spinbutton', { name: INPUT })).not.toBeInTheDocument();
580+
});
581+
582+
it('seeds the field (checkbox pre-checked) from tab.keepHighest', () => {
583+
setup({
584+
assessments: three,
585+
tabs: [
586+
{ id: 10, title: 'Assignments', categoryId: 1, gradebookWeight: 50, keepHighest: 2 },
587+
{ id: 11, title: 'Optional', categoryId: 1, gradebookWeight: 50 },
588+
],
589+
});
590+
expect(screen.getByRole('checkbox', { name: TOGGLE })).toBeChecked();
591+
expect(screen.getByRole('spinbutton', { name: INPUT })).toHaveValue(2);
592+
});
593+
594+
it('disables the checkbox when only one assessment is included', () => {
595+
setup({
596+
assessments: [{ id: 101, title: A1, tabId: 10, maxGrade: 100 }],
597+
});
598+
expect(screen.getByRole('checkbox', { name: TOGGLE })).toBeDisabled();
599+
});
600+
601+
it('sends keepHighest in the save payload', async () => {
602+
setup({ assessments: three });
603+
fireEvent.click(screen.getByRole('checkbox', { name: TOGGLE }));
604+
fireEvent.click(screen.getByRole('button', { name: /save/i }));
605+
await waitFor(() =>
606+
expect(operations.updateGradebookWeights).toHaveBeenCalled(),
607+
);
608+
const arg = (operations.updateGradebookWeights as jest.Mock).mock.calls[0][0];
609+
const tab10 = arg.find((e: { tabId: number }) => e.tabId === 10);
610+
expect(tab10.keepHighest).toBe(2);
611+
});
612+
613+
it('unchecking sends keepHighest 0', async () => {
614+
setup({
615+
assessments: three,
616+
tabs: [
617+
{ id: 10, title: 'Assignments', categoryId: 1, gradebookWeight: 50, keepHighest: 2 },
618+
{ id: 11, title: 'Optional', categoryId: 1, gradebookWeight: 50 },
619+
],
620+
});
621+
// checkbox is pre-checked; uncheck it
622+
fireEvent.click(screen.getByRole('checkbox', { name: TOGGLE }));
623+
fireEvent.click(screen.getByRole('button', { name: /save/i }));
624+
await waitFor(() =>
625+
expect(operations.updateGradebookWeights).toHaveBeenCalled(),
626+
);
627+
const arg = (operations.updateGradebookWeights as jest.Mock).mock.calls[0][0];
628+
const tab10 = arg.find((e: { tabId: number }) => e.tabId === 10);
629+
expect(tab10.keepHighest).toBe(0);
630+
});
631+
632+
it('blocks saving on non-integer input but not on keep > included (overflow)', async () => {
633+
setup({ assessments: three });
634+
fireEvent.click(screen.getByRole('checkbox', { name: TOGGLE }));
635+
const input = screen.getByRole('spinbutton', { name: INPUT });
636+
637+
// overflow: keep=5 > included=3 -> warning but save NOT blocked
638+
fireEvent.change(input, { target: { value: '5' } });
639+
expect(screen.getByRole('button', { name: /save/i })).not.toBeDisabled();
640+
expect(screen.getByText(/keeps more assessments than it contains/i)).toBeInTheDocument();
641+
642+
// non-integer (0): should block saving
643+
fireEvent.change(input, { target: { value: '0' } });
644+
expect(screen.getByRole('button', { name: /save/i })).toBeDisabled();
645+
expect(screen.getByText(/keep at least 1/i)).toBeInTheDocument();
646+
});
480647
});

client/app/bundles/course/gradebook/__tests__/GradebookWeightedTable.test.tsx

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1212,4 +1212,49 @@ describe('GradebookWeightedTable', () => {
12121212
}
12131213
});
12141214
});
1215+
1216+
describe('drop-lowest breakdown marker', () => {
1217+
// One equal tab, keepHighest=1, three graded assessments. The lowest (a1) drops.
1218+
const dropConfig = {
1219+
tabs: [{ ...makeTab(10, 'Missions', 1, 60), keepHighest: 1 }],
1220+
assessments: [
1221+
makeAssessment(1, 'Mission 1', 10, 100),
1222+
makeAssessment(2, 'Mission 2', 10, 100),
1223+
makeAssessment(3, 'Mission 3', 10, 100),
1224+
],
1225+
students: [makeStudent(1, 'Alice')],
1226+
submissions: [makeSub(1, 1, 30), makeSub(1, 2, 60), makeSub(1, 3, 90)],
1227+
};
1228+
1229+
it('labels the dropped assessment "Dropped (lowest)" (not "Excluded")', async () => {
1230+
const user = userEvent.setup();
1231+
renderWeighted(dropConfig);
1232+
await user.click(screen.getByRole('button', { name: /expand Alice/i }));
1233+
const detail = await screen.findByTestId(breakdownRowId(1, 10, 1));
1234+
expect(
1235+
within(detail).getByText(/Dropped \(lowest\)/),
1236+
).toBeInTheDocument();
1237+
expect(within(detail).queryByText(/Excluded/i)).not.toBeInTheDocument();
1238+
});
1239+
1240+
it('shows 0 (not —) in the dropped assessment cell in points mode', async () => {
1241+
const user = userEvent.setup();
1242+
renderWeighted(dropConfig);
1243+
await user.click(screen.getByRole('button', { name: /expand Alice/i }));
1244+
const detail = await screen.findByTestId(breakdownRowId(1, 10, 1));
1245+
// Dropped contributes 0 points — distinct from excluded's em dash.
1246+
expect(within(detail).getByText('0')).toBeInTheDocument();
1247+
expect(within(detail).queryByText('—')).not.toBeInTheDocument();
1248+
});
1249+
1250+
it("shows the dropped assessment's own grade % in percentage mode", async () => {
1251+
const user = userEvent.setup();
1252+
renderWeighted(dropConfig);
1253+
await user.click(screen.getByRole('radio', { name: /percentage/i }));
1254+
await user.click(screen.getByRole('button', { name: /expand Alice/i }));
1255+
const detail = await screen.findByTestId(breakdownRowId(1, 10, 1));
1256+
// a1 = 30/100 = 30% — visible so the instructor sees the dropped grade.
1257+
expect(within(detail).getByText('30%')).toBeInTheDocument();
1258+
});
1259+
});
12151260
});

0 commit comments

Comments
 (0)