diff --git a/lms/static/scripts/frontend_apps/components/CheckpointSelector.tsx b/lms/static/scripts/frontend_apps/components/CheckpointSelector.tsx index 7a6f70c47e..4a8b75d06d 100644 --- a/lms/static/scripts/frontend_apps/components/CheckpointSelector.tsx +++ b/lms/static/scripts/frontend_apps/components/CheckpointSelector.tsx @@ -28,12 +28,6 @@ export default function CheckpointSelector({ }: CheckpointSelectorProps) { const headingId = useId(); - // The note below is specific to the "manual" reveal, so it only shows for that - // option. `selected` is currently always 'manual' (the only enabled option), - // but this keeps the association explicit for when more types are added. - // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition - const showManualNote = selected === 'manual'; - return (

@@ -52,18 +46,28 @@ export default function CheckpointSelector({ } }} > - Manual + + Instructor clicks a button to reveal student annotations + + } + > + Manual reveal + - More coming soon + More options coming - {showManualNote && ( - // No color class: inherits the base text color (black) per design. -

- Students will see when the settings have changed from - “Hide” to “Reveal” in their notifications. -

- )} + {/* No color class: inherits the base text color (black) per design. */} +

+ A Checkpoint is the moment when student annotations switch from hidden + to visible. Before the Checkpoint, students write without seeing each + other’s work. +

); } diff --git a/lms/static/scripts/frontend_apps/components/FilePickerApp.tsx b/lms/static/scripts/frontend_apps/components/FilePickerApp.tsx index b5b49abbf5..46825621e0 100644 --- a/lms/static/scripts/frontend_apps/components/FilePickerApp.tsx +++ b/lms/static/scripts/frontend_apps/components/FilePickerApp.tsx @@ -322,6 +322,7 @@ export default function FilePickerApp({ onSubmit }: FilePickerAppProps) { // from the legal "left blank", and a complete one can have fallen into // the past. Only the selector can tell; it shows the reason for any // rejection itself. + /* istanbul ignore next: unreachable while the due-date step is skipped */ if ( workflowStep === 'due-date' && dueDateSelectorRef.current && @@ -329,10 +330,18 @@ export default function FilePickerApp({ onSubmit }: FilePickerAppProps) { ) { return; } + // TEMPORARY: the due-date step is skipped because the + // date it collects has no effect yet, and offering it suggests the + // assignment does something it doesn't. Nothing else about the step was + // removed — to turn it back on, restore the commented-out line below in + // place of the one under it, and the blocks commented out in + // `FilePickerApp-test`. + // // From 'checkpoint' the next step is 'due-date'; from 'due-date' (the last // step) the workflow is done. The 'assignment-type' step has no "Next" — it // advances directly on selection (see `selectAssignmentType`). - setWorkflowStep(step => (step === 'checkpoint' ? 'due-date' : 'done')); + // setWorkflowStep(step => (step === 'checkpoint' ? 'due-date' : 'done')); + setWorkflowStep('done'); }; // Pick an assignment type in the first workflow step. Unlike the later steps, diff --git a/lms/static/scripts/frontend_apps/components/test/CheckpointSelector-test.js b/lms/static/scripts/frontend_apps/components/test/CheckpointSelector-test.js index 51a12eb35c..ada1d56b8a 100644 --- a/lms/static/scripts/frontend_apps/components/test/CheckpointSelector-test.js +++ b/lms/static/scripts/frontend_apps/components/test/CheckpointSelector-test.js @@ -47,23 +47,12 @@ describe('CheckpointSelector', () => { assert.calledWith(fakeOnChange, 'automatic'); }); - it('shows the reveal note when "manual" is selected', () => { - const wrapper = createComponent('manual'); + it('explains what a checkpoint is', () => { + const wrapper = createComponent(); assert.include( wrapper.text(), - 'Students will see when the settings have changed', - ); - }); - - it('hides the reveal note when a non-manual option is selected', () => { - // `selected` is typed `'manual'` today (the only option), so this exercises - // the conditional that will matter once more checkpoint types exist. - const wrapper = createComponent('more'); - - assert.notInclude( - wrapper.text(), - 'Students will see when the settings have changed', + 'A Checkpoint is the moment when student annotations switch from hidden to visible', ); }); diff --git a/lms/static/scripts/frontend_apps/components/test/FilePickerApp-test.js b/lms/static/scripts/frontend_apps/components/test/FilePickerApp-test.js index a63187c551..00f114f63f 100644 --- a/lms/static/scripts/frontend_apps/components/test/FilePickerApp-test.js +++ b/lms/static/scripts/frontend_apps/components/test/FilePickerApp-test.js @@ -144,39 +144,42 @@ describe('FilePickerApp', () => { }); } - function setDueDate(wrapper, date) { - interact(wrapper, () => { - wrapper.find('DueDateSelector').first().props().onChange(date); - }); - } - - /** - * Stand in for `DueDateSelector`'s `validate`. - * - * `DueDateSelector` is mocked here, so the real fields — and the checks - * the real component runs over them — never exist. The mock still - * receives `selectorRef`, so populating the handle decides what the - * parent hears when it validates the step. - */ - function setDueDateValidity(wrapper, valid) { - const { selectorRef } = wrapper.find('DueDateSelector').first().props(); - selectorRef.current = { validate: sinon.stub().returns(valid) }; - return selectorRef.current; - } - - /** - * Local `datetime-local` string (`YYYY-MM-DDTHH:MM`) `days` from now - * (negative for the past). - */ - function dueDateFromNow(days) { - const date = new Date(); - date.setDate(date.getDate() + days); - const pad = n => String(n).padStart(2, '0'); - return ( - `${date.getFullYear()}-${pad(date.getMonth() + 1)}-${pad(date.getDate())}` + - `T${pad(date.getHours())}:${pad(date.getMinutes())}` - ); - } + // Commented out while the due-date step is skipped (see + // `goToNextWorkflowStep`). Restore alongside the step. + // + // function setDueDate(wrapper, date) { + // interact(wrapper, () => { + // wrapper.find('DueDateSelector').first().props().onChange(date); + // }); + // } + // + // /** + // * Stand in for `DueDateSelector`'s `validate`. + // * + // * `DueDateSelector` is mocked here, so the real fields — and the checks + // * the real component runs over them — never exist. The mock still + // * receives `selectorRef`, so populating the handle decides what the + // * parent hears when it validates the step. + // */ + // function setDueDateValidity(wrapper, valid) { + // const { selectorRef } = wrapper.find('DueDateSelector').first().props(); + // selectorRef.current = { validate: sinon.stub().returns(valid) }; + // return selectorRef.current; + // } + // + // /** + // * Local `datetime-local` string (`YYYY-MM-DDTHH:MM`) `days` from now + // * (negative for the past). + // */ + // function dueDateFromNow(days) { + // const date = new Date(); + // date.setDate(date.getDate() + days); + // const pad = n => String(n).padStart(2, '0'); + // return ( + // `${date.getFullYear()}-${pad(date.getMonth() + 1)}-${pad(date.getDate())}` + + // `T${pad(date.getHours())}:${pad(date.getMinutes())}` + // ); + // } it('does not show the workflow when only one type is available', () => { fakeConfig.filePicker.assignmentTypes = ['reading']; @@ -223,7 +226,7 @@ describe('FilePickerApp', () => { assert.isTrue(wrapper.exists('ContentSelector')); }); - it('walks through checkpoint and due-date steps for "Hide & Reveal"', () => { + it('walks through the checkpoint step for "Hide & Reveal"', () => { fakeConfig.filePicker.assignmentTypes = ['reading', 'hide_and_reveal']; const wrapper = renderFilePicker(); @@ -235,64 +238,64 @@ describe('FilePickerApp', () => { assert.isFalse(wrapper.exists('ContentSelector')); clickNext(wrapper); - // Due-date step. - assert.isTrue(wrapper.exists('DueDateSelector')); - assert.isFalse(wrapper.exists('ContentSelector')); - clickNext(wrapper); - - // Regular flow takes over. + // Regular flow takes over: the due-date step that used to sit here is + // skipped for now (see `goToNextWorkflowStep`). assert.isFalse(wrapper.exists('DueDateSelector')); assert.isTrue(wrapper.exists('ContentSelector')); }); - it('blocks the due-date step while the selector reports an invalid value', () => { - fakeConfig.filePicker.assignmentTypes = ['reading', 'hide_and_reveal']; - const wrapper = renderFilePicker(); - - selectAssignmentType(wrapper, 'hide_and_reveal'); // -> checkpoint - clickNext(wrapper); // -> due-date - assert.isTrue(wrapper.exists('DueDateSelector')); - - // Whatever the reason — a half-entered value, or a complete one that is - // no longer in the future — the selector shows it inline; the parent - // only acts on the verdict. - const handle = setDueDateValidity(wrapper, false); - - clickNext(wrapper); - assert.isTrue(wrapper.exists('DueDateSelector')); - assert.isFalse(wrapper.exists('ContentSelector')); - assert.called(handle.validate); - }); - - it('anchors the earliest selectable due date to the present', () => { - fakeConfig.filePicker.assignmentTypes = ['reading', 'hide_and_reveal']; - const wrapper = renderFilePicker(); - - selectAssignmentType(wrapper, 'hide_and_reveal'); // -> checkpoint - clickNext(wrapper); // -> due-date - - // The selector rejects anything below `min` (past dates, past times on - // the current day); pinning `min` to "now" is the parent's half of the - // "no due dates in the past" rule. - const { min } = wrapper.find('DueDateSelector').first().props(); - assert.isTrue(min <= dueDateFromNow(0)); - assert.isTrue(min > dueDateFromNow(-1)); - }); - - it('leaves the due-date step when the selector reports a valid value', () => { - fakeConfig.filePicker.assignmentTypes = ['reading', 'hide_and_reveal']; - const wrapper = renderFilePicker(); - - selectAssignmentType(wrapper, 'hide_and_reveal'); // -> checkpoint - clickNext(wrapper); // -> due-date - - setDueDateValidity(wrapper, true); - setDueDate(wrapper, dueDateFromNow(7)); - - clickNext(wrapper); - assert.isFalse(wrapper.exists('DueDateSelector')); - assert.isTrue(wrapper.exists('ContentSelector')); - }); + // The three tests below only make sense while the due-date step is + // reachable. Restore them alongside the step, and put the due-date hop back + // into the test above. + // + // it('blocks the due-date step while the selector reports an invalid value', () => { + // fakeConfig.filePicker.assignmentTypes = ['reading', 'hide_and_reveal']; + // const wrapper = renderFilePicker(); + // + // selectAssignmentType(wrapper, 'hide_and_reveal'); // -> checkpoint + // clickNext(wrapper); // -> due-date + // assert.isTrue(wrapper.exists('DueDateSelector')); + // + // // Whatever the reason — a half-entered value, or a complete one that is + // // no longer in the future — the selector shows it inline; the parent + // // only acts on the verdict. + // const handle = setDueDateValidity(wrapper, false); + // + // clickNext(wrapper); + // assert.isTrue(wrapper.exists('DueDateSelector')); + // assert.isFalse(wrapper.exists('ContentSelector')); + // assert.called(handle.validate); + // }); + // + // it('anchors the earliest selectable due date to the present', () => { + // fakeConfig.filePicker.assignmentTypes = ['reading', 'hide_and_reveal']; + // const wrapper = renderFilePicker(); + // + // selectAssignmentType(wrapper, 'hide_and_reveal'); // -> checkpoint + // clickNext(wrapper); // -> due-date + // + // // The selector rejects anything below `min` (past dates, past times on + // // the current day); pinning `min` to "now" is the parent's half of the + // // "no due dates in the past" rule. + // const { min } = wrapper.find('DueDateSelector').first().props(); + // assert.isTrue(min <= dueDateFromNow(0)); + // assert.isTrue(min > dueDateFromNow(-1)); + // }); + // + // it('leaves the due-date step when the selector reports a valid value', () => { + // fakeConfig.filePicker.assignmentTypes = ['reading', 'hide_and_reveal']; + // const wrapper = renderFilePicker(); + // + // selectAssignmentType(wrapper, 'hide_and_reveal'); // -> checkpoint + // clickNext(wrapper); // -> due-date + // + // setDueDateValidity(wrapper, true); + // setDueDate(wrapper, dueDateFromNow(7)); + // + // clickNext(wrapper); + // assert.isFalse(wrapper.exists('DueDateSelector')); + // assert.isTrue(wrapper.exists('ContentSelector')); + // }); it('does not offer a "Back" button on the first step', () => { fakeConfig.filePicker.assignmentTypes = ['reading', 'hide_and_reveal']; @@ -308,13 +311,9 @@ describe('FilePickerApp', () => { fakeConfig.filePicker.assignmentTypes = ['reading', 'hide_and_reveal']; const wrapper = renderFilePicker(); + // The due-date step this test used to walk back from is skipped for now. selectAssignmentType(wrapper, 'hide_and_reveal'); // -> checkpoint - clickNext(wrapper); // -> due-date - assert.isTrue(wrapper.exists('DueDateSelector')); - - clickBack(wrapper); // -> checkpoint assert.isTrue(wrapper.exists('CheckpointSelector')); - assert.isFalse(wrapper.exists('DueDateSelector')); clickBack(wrapper); // -> assignment-type assert.isTrue(wrapper.exists('AssignmentTypeSelector')); @@ -332,9 +331,7 @@ describe('FilePickerApp', () => { selectAssignmentType(wrapper, 'hide_and_reveal'); // -> checkpoint assert.equal(cardTitle(), 'Paced Social Annotation'); - clickNext(wrapper); // -> due-date - assert.equal(cardTitle(), 'Paced Social Annotation'); - + // The skipped due-date step shared this title. clickNext(wrapper); // -> regular flow assert.equal(cardTitle(), 'Assignment details'); }); @@ -346,9 +343,9 @@ describe('FilePickerApp', () => { // The mode-selection step itself offers no close button. assert.isNotOk(wrapper.find('CardHeader').prop('onClose')); + // Checked from the checkpoint step while the due-date step is skipped. selectAssignmentType(wrapper, 'hide_and_reveal'); // -> checkpoint - clickNext(wrapper); // -> due-date - assert.isTrue(wrapper.exists('DueDateSelector')); + assert.isTrue(wrapper.exists('CheckpointSelector')); // The header exposes a close handler during the Paced sub-steps. const onClose = wrapper.find('CardHeader').prop('onClose'); @@ -356,7 +353,7 @@ describe('FilePickerApp', () => { interact(wrapper, () => onClose()); assert.isTrue(wrapper.exists('AssignmentTypeSelector')); - assert.isFalse(wrapper.exists('DueDateSelector')); + assert.isFalse(wrapper.exists('CheckpointSelector')); }); it('recomputes the branch when the type is changed after going back', () => { @@ -509,53 +506,57 @@ describe('FilePickerApp', () => { }); }); - it('sends the due date as a UTC datetime for "Hide & Reveal"', async () => { - fakeConfig.filePicker.assignmentTypes = ['reading', 'hide_and_reveal']; - const onSubmit = sinon.stub().callsFake(e => e.preventDefault()); - const wrapper = renderFilePicker({ onSubmit }); - - const clickNext = () => - interact(wrapper, () => { - wrapper - .find('Button[data-testid="workflow-next-button"]') - .props() - .onClick(); - }); - - // Walk the Hide & Reveal workflow, picking a future due date. The picker - // value is local wall-clock time; the backend receives it as UTC. - interact(wrapper, () => { - // Selecting the type advances straight to the checkpoint step. - wrapper - .find('AssignmentTypeSelector') - .props() - .onSelect('hide_and_reveal'); - }); - clickNext(); // -> due-date - const localDueDate = '2035-01-15T10:30'; - interact(wrapper, () => { - wrapper.find('DueDateSelector').first().props().onChange(localDueDate); - }); - clickNext(); // -> content selection - - selectContent(wrapper, 'https://example.com'); - - await waitFor(() => fakeAPICall.called); - assert.calledWith(fakeAPICall, { - authToken: 'DUMMY_AUTH_TOKEN', - path: deepLinkingAPIPath, - data: { - ...deepLinkingAPIData, - content: { type: 'url', url: 'https://example.com' }, - title: null, - group_set: null, - auto_grading_config: null, - assignment_gradable_max_points: null, - checkpoint_enabled: true, - due_date: new Date(localDueDate).toISOString(), - }, - }); - }); + // No due date can be picked while the step is skipped, so the payload + // always carries `due_date: null` (asserted by the tests above). Restore + // alongside the step. + // + // it('sends the due date as a UTC datetime for "Hide & Reveal"', async () => { + // fakeConfig.filePicker.assignmentTypes = ['reading', 'hide_and_reveal']; + // const onSubmit = sinon.stub().callsFake(e => e.preventDefault()); + // const wrapper = renderFilePicker({ onSubmit }); + // + // const clickNext = () => + // interact(wrapper, () => { + // wrapper + // .find('Button[data-testid="workflow-next-button"]') + // .props() + // .onClick(); + // }); + // + // // Walk the Hide & Reveal workflow, picking a future due date. The picker + // // value is local wall-clock time; the backend receives it as UTC. + // interact(wrapper, () => { + // // Selecting the type advances straight to the checkpoint step. + // wrapper + // .find('AssignmentTypeSelector') + // .props() + // .onSelect('hide_and_reveal'); + // }); + // clickNext(); // -> due-date + // const localDueDate = '2035-01-15T10:30'; + // interact(wrapper, () => { + // wrapper.find('DueDateSelector').first().props().onChange(localDueDate); + // }); + // clickNext(); // -> content selection + // + // selectContent(wrapper, 'https://example.com'); + // + // await waitFor(() => fakeAPICall.called); + // assert.calledWith(fakeAPICall, { + // authToken: 'DUMMY_AUTH_TOKEN', + // path: deepLinkingAPIPath, + // data: { + // ...deepLinkingAPIData, + // content: { type: 'url', url: 'https://example.com' }, + // title: null, + // group_set: null, + // auto_grading_config: null, + // assignment_gradable_max_points: null, + // checkpoint_enabled: true, + // due_date: new Date(localDueDate).toISOString(), + // }, + // }); + // }); it('fetches form field values via deep linking API on implicit form submission', async () => { // Enable title field, which is a field where the user could trigger an