Skip to content

Commit dd99aef

Browse files
navinkarkeranavinkarkera
committed
fix: Subsection settings data update
1 parent e919cf1 commit dd99aef

2 files changed

Lines changed: 125 additions & 103 deletions

File tree

‎src/course-outline/outline-sidebar/info-sidebar/SubsectionSettings.test.tsx‎

Lines changed: 80 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ jest.mock('@src/hooks', () => ({
1313
const wrappedSetState = (val: any) => {
1414
const newVal = typeof val === 'function' ? val(state) : val;
1515
setState(newVal);
16-
// call callback synchronously like the implementation would after debounce
1716
if (cb) { cb(newVal); }
1817
};
1918
return [state, wrappedSetState];
@@ -24,7 +23,11 @@ jest.mock('@src/hooks', () => ({
2423
jest.mock('@src/generic/datepicker-control', () => ({
2524
DATEPICKER_TYPES: { date: 'date', time: 'time' },
2625
DatepickerControl: ({ onChange, type, ...props }: any) => (
27-
<button type="button" data-testid={props['data-testid'] || type} onClick={() => onChange(type === 'date' ? '2025-12-31' : '12:00')}>
26+
<button
27+
type="button"
28+
data-testid={props['data-testid'] || type}
29+
onClick={() => onChange(type === 'date' ? '2025-12-31' : '12:00')}
30+
>
2831
{type}
2932
</button>
3033
),
@@ -73,119 +76,99 @@ jest.mock('@src/course-outline/outline-sidebar/OutlineSidebarContext', () => ({
7376

7477
const apiHooks = jest.requireMock('@src/course-outline/data/apiHooks') as any;
7578

79+
const baseItemData = {
80+
visibilityState: 'staff_only',
81+
start: '2022-01-01',
82+
format: null,
83+
due: null,
84+
isTimeLimited: false,
85+
isProctoredExam: false,
86+
isOnboardingExam: false,
87+
isPracticeExam: false,
88+
examReviewRules: null,
89+
defaultTimeLimitMinutes: null,
90+
hideAfterDue: undefined,
91+
showCorrectness: undefined,
92+
isPrereq: false,
93+
prereq: null,
94+
prereqMinScore: null,
95+
prereqMinCompletion: null,
96+
courseGraders: ['g1', 'g2'],
97+
graded: true,
98+
};
99+
76100
describe('SubsectionSettings', () => {
77101
beforeEach(() => {
78102
initializeMocks();
79103
mutate.mockReset();
80104
});
81105

82-
it('renders sections and calls mutate with combined payloads', async () => {
106+
it('renders core sections and calls mutate for release, visibility, grading, and special exam', async () => {
83107
apiHooks.useCourseDetails.mockReturnValue({ data: { selfPaced: false } });
84-
apiHooks.useCourseItemData.mockReturnValue({
85-
data: {
86-
visibilityState: 'staff_only',
87-
start: '2022-01-01',
88-
format: null,
89-
due: null,
90-
isTimeLimited: false,
91-
isProctoredExam: false,
92-
isOnboardingExam: false,
93-
isPracticeExam: false,
94-
examReviewRules: null,
95-
defaultTimeLimitMinutes: null,
96-
hideAfterDue: undefined,
97-
showCorrectness: undefined,
98-
isPrereq: false,
99-
prereq: null,
100-
prereqMinScore: null,
101-
prereqMinCompletion: null,
102-
courseGraders: ['g1', 'g2'],
103-
graded: true,
104-
},
105-
isPending: false,
106-
});
108+
apiHooks.useCourseItemData.mockReturnValue({ data: baseItemData, isPending: false });
107109

108110
const user = userEvent.setup();
109111
render(<SubsectionSettings subsectionId={subsectionId} />);
110112

111-
// Clicking Release should call mutate with releaseDate override
113+
// Release
112114
await user.click(await screen.findByRole('button', { name: 'Release' }));
113115
expect(mutate).toHaveBeenCalledWith(expect.objectContaining({ itemId: subsectionId, sectionId: 'section-abc', releaseDate: '2030-01-01' }));
114116

115-
// Clicking Visibility should call mutate with visibility change
117+
// Visibility
116118
await user.click(await screen.findByRole('button', { name: 'Visibility' }));
117119
expect(mutate).toHaveBeenCalledWith(expect.objectContaining({ itemId: subsectionId, sectionId: 'section-abc', visibility: 'v' }));
118120

119-
// Grading: clicking Ungraded button should call onChange via setUngraded
121+
// Grading -> Ungraded
120122
await user.click(await screen.findByRole('button', { name: 'Ungraded' }));
121123
expect(mutate).toHaveBeenCalledWith(expect.objectContaining({ itemId: subsectionId, graderType: 'notgraded' }));
122124

123-
// AdvancedTab mock: setFieldValue should call mutate through onChange
125+
// Special exam
124126
await user.click(await screen.findByRole('button', { name: 'Set Proctored' }));
125127
expect(mutate).toHaveBeenCalledWith(expect.objectContaining({ itemId: subsectionId, isProctoredExam: true }));
126128
});
127129

128-
it('handles grading select, assessment result toggles and prereq parsing', async () => {
129-
// Use a new itemData shape where graded is false so clicking Graded shows controls
130+
it('handles grading select and due date/time changes', async () => {
130131
apiHooks.useCourseDetails.mockReturnValue({ data: { selfPaced: false } });
131132
apiHooks.useCourseItemData.mockReturnValue({
132-
data: {
133-
visibilityState: 'staff_only',
134-
start: '2022-01-01',
135-
format: null,
136-
due: null,
137-
isTimeLimited: false,
138-
isProctoredExam: false,
139-
isOnboardingExam: false,
140-
isPracticeExam: false,
141-
examReviewRules: null,
142-
defaultTimeLimitMinutes: null,
143-
hideAfterDue: undefined,
144-
showCorrectness: undefined,
145-
isPrereq: false,
146-
prereq: null,
147-
prereqMinScore: '50',
148-
prereqMinCompletion: '75',
149-
courseGraders: ['g1', 'g2'],
150-
graded: false,
151-
},
133+
data: { ...baseItemData, graded: false, prereqMinScore: '50', prereqMinCompletion: '75' },
152134
isPending: false,
153135
});
154136

155137
const user = userEvent.setup();
156138
render(<SubsectionSettings subsectionId={subsectionId} />);
157139

158-
// Click Graded to show controls
159140
await user.click(await screen.findByRole('button', { name: 'Graded' }));
160-
161-
// Change grader select to 'g1'
162141
const select = await screen.findByTestId('grader-type-select');
163142
await user.selectOptions(select as HTMLSelectElement, 'g1');
164143
expect(mutate).toHaveBeenCalledWith(expect.objectContaining({ itemId: subsectionId, graderType: 'g1' }));
165144

166-
// Click date and time pickers to set dueDate
167145
await user.click(await screen.findByTestId('due-date-picker'));
168146
expect(mutate).toHaveBeenCalledWith(expect.objectContaining({ itemId: subsectionId, dueDate: '2025-12-31' }));
169147
await user.click(await screen.findByTestId('time'));
170148
expect(mutate).toHaveBeenCalledWith(expect.objectContaining({ itemId: subsectionId, dueDate: '12:00' }));
149+
});
150+
151+
it('toggles assessment result visibility', async () => {
152+
apiHooks.useCourseDetails.mockReturnValue({ data: { selfPaced: false } });
153+
apiHooks.useCourseItemData.mockReturnValue({ data: { ...baseItemData, graded: false }, isPending: false });
154+
155+
const user = userEvent.setup();
156+
render(<SubsectionSettings subsectionId={subsectionId} />);
171157

172-
// Assessment results: click Show -> should set 'always'
173158
await user.click(await screen.findByRole('button', { name: 'Show' }));
174159
expect(mutate).toHaveBeenCalledWith(expect.objectContaining({ itemId: subsectionId, showCorrectness: 'always' }));
175160

176-
// Click Hide when currently 'always' should change to 'never'
177161
await user.click(await screen.findByRole('button', { name: 'Hide' }));
178162
expect(mutate).toHaveBeenCalledWith(expect.objectContaining({ itemId: subsectionId, showCorrectness: 'never' }));
179163

180-
// Click checkbox to set past_due
181164
const checkbox = await screen.findByRole('checkbox');
182165
await user.click(checkbox);
183166
expect(mutate).toHaveBeenCalledWith(expect.objectContaining({ itemId: subsectionId, showCorrectness: 'past_due' }));
184167
});
185168

186169
it('does not render ReleaseSection when course is self paced', () => {
187170
apiHooks.useCourseDetails.mockReturnValue({ data: { selfPaced: true } });
188-
apiHooks.useCourseItemData.mockReturnValue({ data: { visibilityState: 'gated', start: null, graded: false }, isPending: false });
171+
apiHooks.useCourseItemData.mockReturnValue({ data: { ...baseItemData, start: null }, isPending: false });
189172

190173
render(<SubsectionSettings subsectionId={subsectionId} />);
191174

@@ -195,7 +178,7 @@ describe('SubsectionSettings', () => {
195178

196179
it('does not call mutate when item data is pending', async () => {
197180
apiHooks.useCourseDetails.mockReturnValue({ data: { selfPaced: false } });
198-
apiHooks.useCourseItemData.mockReturnValue({ data: { visibilityState: 'gated', start: null, graded: false }, isPending: true });
181+
apiHooks.useCourseItemData.mockReturnValue({ data: { ...baseItemData, start: null, graded: false }, isPending: true });
199182

200183
const user = userEvent.setup();
201184
render(<SubsectionSettings subsectionId={subsectionId} />);
@@ -204,51 +187,46 @@ describe('SubsectionSettings', () => {
204187
expect(mutate).not.toHaveBeenCalled();
205188
});
206189

207-
it('renders special exam section and sends only changed fields to api', async () => {
190+
it('resets grading local state when itemData changes', async () => {
208191
apiHooks.useCourseDetails.mockReturnValue({ data: { selfPaced: false } });
209-
apiHooks.useCourseItemData.mockReturnValue({
210-
data: {
211-
visibilityState: 'staff_only',
212-
start: '2022-01-01',
213-
format: null,
214-
due: null,
215-
isTimeLimited: false,
216-
isProctoredExam: false,
217-
isOnboardingExam: false,
218-
isPracticeExam: false,
219-
examReviewRules: null,
220-
defaultTimeLimitMinutes: null,
221-
hideAfterDue: undefined,
222-
showCorrectness: undefined,
223-
isPrereq: false,
224-
prereq: 'prereq-key',
225-
prereqMinScore: 'abc',
226-
prereqMinCompletion: 'xyz',
227-
courseGraders: ['g1', 'g2'],
228-
graded: true,
229-
},
230-
isPending: false,
231-
});
192+
const firstItemData = { ...baseItemData, format: 'g1', due: '2024-01-01', graded: true };
193+
const secondItemData = { ...firstItemData, format: 'g2', due: '2024-02-02' };
194+
195+
apiHooks.useCourseItemData.mockReturnValue({ data: firstItemData, isPending: false });
196+
197+
const { rerender } = render(<SubsectionSettings subsectionId={subsectionId} />);
198+
199+
mutate.mockClear();
200+
apiHooks.useCourseItemData.mockReturnValue({ data: secondItemData, isPending: false });
201+
rerender(<SubsectionSettings subsectionId={subsectionId} />);
202+
203+
expect(mutate).toHaveBeenCalledWith(expect.objectContaining({ graderType: 'g2', dueDate: '2024-02-02' }));
204+
});
205+
206+
it('resets assessment visibility local state when itemData changes', async () => {
207+
apiHooks.useCourseDetails.mockReturnValue({ data: { selfPaced: false } });
208+
const firstItemData = { ...baseItemData, graded: false, showCorrectness: 'always' };
209+
const secondItemData = { ...firstItemData, showCorrectness: 'never' };
210+
211+
apiHooks.useCourseItemData.mockReturnValue({ data: firstItemData, isPending: false });
212+
213+
const { rerender } = render(<SubsectionSettings subsectionId={subsectionId} />);
214+
215+
mutate.mockClear();
216+
apiHooks.useCourseItemData.mockReturnValue({ data: secondItemData, isPending: false });
217+
rerender(<SubsectionSettings subsectionId={subsectionId} />);
218+
219+
expect(mutate).toHaveBeenCalledWith(expect.objectContaining({ showCorrectness: 'never' }));
220+
});
221+
222+
it('does not call mutate when item data is absent', async () => {
223+
apiHooks.useCourseDetails.mockReturnValue({ data: { selfPaced: false } });
224+
apiHooks.useCourseItemData.mockReturnValue({ data: undefined, isPending: false });
232225

233226
const user = userEvent.setup();
234227
render(<SubsectionSettings subsectionId={subsectionId} />);
235228

236-
await user.click(await screen.findByRole('button', { name: 'Set Proctored' }));
237-
238-
expect(mutate).toHaveBeenCalledWith(expect.objectContaining({
239-
itemId: subsectionId,
240-
sectionId: 'section-abc',
241-
isProctoredExam: true,
242-
isTimeLimited: false,
243-
isOnboardingExam: false,
244-
isPracticeExam: false,
245-
defaultTimeLimitMinutes: null,
246-
examReviewRules: null,
247-
isPrereq: false,
248-
prereqUsageKey: 'prereq-key',
249-
prereqMinScore: 100,
250-
prereqMinCompletion: 100,
251-
}));
252-
expect(mutate).toHaveBeenCalledTimes(1);
229+
await user.click(screen.getByRole('button', { name: 'Visibility' }));
230+
expect(mutate).not.toHaveBeenCalled();
253231
});
254232
});

‎src/course-outline/outline-sidebar/info-sidebar/SubsectionSettings.tsx‎

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import AdvancedTab from '@src/generic/configure-modal/AdvancedTab';
1111
import { DatepickerControl, DATEPICKER_TYPES } from '@src/generic/datepicker-control';
1212
import { SidebarContent, SidebarSection } from '@src/generic/sidebar';
1313
import { useStateWithCallback } from '@src/hooks';
14-
import { useCallback, useState } from 'react';
14+
import { useCallback, useEffect, useRef, useState } from 'react';
1515
import { useSelector } from 'react-redux';
1616
import { ReleaseSection } from './sharedSettings/ReleaseSection';
1717
import messages from './messages';
@@ -46,6 +46,23 @@ const GradingSection = ({ subsectionId, onChange }: SubProps) => {
4646
},
4747
(val) => onChange(val || {}),
4848
);
49+
const didMountRef = useRef(false);
50+
51+
useEffect(() => {
52+
const nextState = {
53+
graderType: itemData?.format,
54+
dueDate: itemData?.due || '',
55+
};
56+
57+
if (!didMountRef.current) {
58+
didMountRef.current = true;
59+
return;
60+
}
61+
62+
if (localState?.graderType !== nextState.graderType || localState?.dueDate !== nextState.dueDate) {
63+
setLocalState(nextState);
64+
}
65+
}, [itemData?.format, itemData?.due]);
4966

5067
const setUngraded = () => {
5168
setGraded(false);
@@ -126,6 +143,18 @@ const AssessmentResultVisibilitySection = ({ subsectionId, onChange }: SubProps)
126143
},
127144
(val) => onChange(val || {}),
128145
);
146+
const didMountRef = useRef(false);
147+
148+
useEffect(() => {
149+
if (!didMountRef.current) {
150+
didMountRef.current = true;
151+
return;
152+
}
153+
154+
if (localState?.showCorrectness !== itemData?.showCorrectness) {
155+
setLocalState({ showCorrectness: itemData?.showCorrectness });
156+
}
157+
}, [itemData?.showCorrectness]);
129158

130159
return (
131160
<SidebarSection
@@ -182,6 +211,21 @@ const SpecialExamSection = ({ subsectionId, onChange }: SubProps) => {
182211
getLatestLocalState,
183212
(val) => onChange(val || {}),
184213
);
214+
const didMountRef = useRef(false);
215+
216+
useEffect(() => {
217+
if (!didMountRef.current) {
218+
didMountRef.current = true;
219+
return;
220+
}
221+
222+
const nextState = getLatestLocalState();
223+
const hasChanges = Object.keys(nextState).some((key) => (localState as any)?.[key] !== (nextState as any)[key]);
224+
225+
if (hasChanges) {
226+
setLocalState({ value: nextState, skipCallback: true });
227+
}
228+
}, [getLatestLocalState]);
185229

186230
const setFieldValue = (key: keyof ConfigureSubsectionData, value: any) => {
187231
setLocalState((prev) => ({

0 commit comments

Comments
 (0)