Skip to content

Commit aa1db13

Browse files
fix: adding attempt validation before edit/remove allowance (#200)
1 parent 4ea580e commit aa1db13

5 files changed

Lines changed: 248 additions & 10 deletions

File tree

src/specialExams/components/Allowances.test.tsx

Lines changed: 148 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import userEvent from '@testing-library/user-event';
33
import Allowances from '@src/specialExams/components/Allowances';
44
import { renderWithAlertAndIntl } from '@src/testUtils';
55
import messages from '@src/specialExams/messages';
6-
import { useAddAllowance, useAllowances, useSpecialExams } from '@src/specialExams/data/apiHook';
6+
import { useAddAllowance, useAllowances, useAttempts, useSpecialExams } from '@src/specialExams/data/apiHook';
77
import { useLearner } from '@src/data/apiHook';
88

99
const mockLearnerData = {
@@ -15,13 +15,16 @@ const mockLearnerData = {
1515
};
1616

1717
const mockAllowance = {
18+
id: 1,
1819
user: {
1920
username: 'john_doe',
2021
email: 'john.doe@hotmail.com',
22+
id: 5,
2123
},
2224
proctoredExam: {
2325
examName: 'Midterm Exam',
2426
examType: 'proctored',
27+
id: 101,
2528
},
2629
value: '30 minutes',
2730
key: 'additional_time_granted',
@@ -47,14 +50,17 @@ jest.mock('@src/specialExams/data/apiHook', () => ({
4750
useAddAllowance: jest.fn(),
4851
useSpecialExams: jest.fn(),
4952
useDeleteAllowance: jest.fn().mockReturnValue({ mutate: jest.fn() }),
53+
useAttempts: jest.fn(),
5054
}));
5155

5256
describe('Allowances', () => {
5357
let mockAddAllowance: jest.Mock;
58+
let mockRefetchAttempts: jest.Mock;
5459

5560
beforeEach(() => {
5661
jest.clearAllMocks();
5762
mockAddAllowance = jest.fn();
63+
mockRefetchAttempts = jest.fn();
5864
(useLearner as jest.Mock).mockReturnValue({
5965
data: mockLearnerData,
6066
refetch: jest.fn().mockResolvedValue({ data: mockLearnerData }),
@@ -69,6 +75,10 @@ describe('Allowances', () => {
6975
data: mockSpecialExams,
7076
refetch: jest.fn().mockResolvedValue({ data: mockSpecialExams }),
7177
});
78+
(useAttempts as jest.Mock).mockReturnValue({
79+
data: { results: [], count: 0, numPages: 0 },
80+
refetch: mockRefetchAttempts.mockResolvedValue({ data: { results: [], count: 0, numPages: 0 } }),
81+
});
7282
});
7383

7484
it('renders AllowancesList and AddAllowanceModal', () => {
@@ -273,4 +283,141 @@ describe('Allowances', () => {
273283
// DeleteAllowanceModal should not render without selectedAllowance
274284
expect(screen.queryByRole('dialog', { name: messages.deleteAllowance.defaultMessage })).not.toBeInTheDocument();
275285
});
286+
287+
describe('checkAttemptAndProceed', () => {
288+
it('shows warning modal when user has already attempted the exam on edit', async () => {
289+
const mockAttemptData = {
290+
results: [
291+
{
292+
id: 1,
293+
examId: 101,
294+
user: { username: 'john_doe' },
295+
examName: 'Midterm Exam',
296+
allowedTimeLimitMins: 60,
297+
type: 'proctored',
298+
startTime: '2023-01-01T10:00:00Z',
299+
endTime: '2023-01-01T11:00:00Z',
300+
status: 'completed',
301+
},
302+
],
303+
count: 1,
304+
numPages: 1,
305+
};
306+
(useAllowances as jest.Mock).mockReturnValue({
307+
data: { results: [mockAllowance], count: 1, numPages: 1 },
308+
isLoading: false,
309+
});
310+
(useAttempts as jest.Mock).mockReturnValue({
311+
data: mockAttemptData,
312+
refetch: jest.fn().mockResolvedValue({ data: mockAttemptData }),
313+
});
314+
const user = userEvent.setup();
315+
renderWithAlertAndIntl(<Allowances />);
316+
await user.click(screen.getByRole('button', { name: messages.actions.defaultMessage }));
317+
const editBtn = await screen.findByRole('button', { name: /edit/i });
318+
await user.click(editBtn);
319+
await waitFor(() => {
320+
expect(screen.getByText(/Cannot edit allowance: john_doe has already attempted the exam/i)).toBeInTheDocument();
321+
});
322+
expect(screen.queryByRole('dialog', { name: messages.editAllowance.defaultMessage })).not.toBeInTheDocument();
323+
});
324+
325+
it('shows warning modal when user has already attempted the exam on delete', async () => {
326+
const mockAttemptData = {
327+
results: [
328+
{
329+
id: 1,
330+
examId: 101,
331+
user: { username: 'john_doe' },
332+
examName: 'Midterm Exam',
333+
allowedTimeLimitMins: 60,
334+
type: 'proctored',
335+
startTime: '2023-01-01T10:00:00Z',
336+
endTime: '2023-01-01T11:00:00Z',
337+
status: 'completed',
338+
},
339+
],
340+
count: 1,
341+
numPages: 1,
342+
};
343+
(useAllowances as jest.Mock).mockReturnValue({
344+
data: { results: [mockAllowance], count: 1, numPages: 1 },
345+
isLoading: false,
346+
});
347+
(useAttempts as jest.Mock).mockReturnValue({
348+
data: mockAttemptData,
349+
refetch: jest.fn().mockResolvedValue({ data: mockAttemptData }),
350+
});
351+
const user = userEvent.setup();
352+
renderWithAlertAndIntl(<Allowances />);
353+
await user.click(screen.getByRole('button', { name: messages.actions.defaultMessage }));
354+
const deleteBtn = await screen.findByRole('button', { name: /delete/i });
355+
await user.click(deleteBtn);
356+
await waitFor(() => {
357+
expect(screen.getByText(/Cannot delete allowance: john_doe has already attempted the exam/i)).toBeInTheDocument();
358+
});
359+
expect(screen.queryByRole('dialog', { name: messages.deleteAllowance.defaultMessage })).not.toBeInTheDocument();
360+
});
361+
362+
it('opens edit modal when user has not attempted the exam', async () => {
363+
const mockAttemptData = {
364+
results: [
365+
{
366+
id: 1,
367+
examId: 999, // different exam, not the one in the allowance
368+
user: { username: 'john_doe' },
369+
examName: 'Other Exam',
370+
allowedTimeLimitMins: 60,
371+
type: 'proctored',
372+
startTime: '2023-01-01T10:00:00Z',
373+
endTime: '2023-01-01T11:00:00Z',
374+
status: 'completed',
375+
},
376+
],
377+
count: 1,
378+
numPages: 1,
379+
};
380+
(useAllowances as jest.Mock).mockReturnValue({
381+
data: { results: [mockAllowance], count: 1, numPages: 1 },
382+
isLoading: false,
383+
});
384+
(useAttempts as jest.Mock).mockReturnValue({
385+
data: mockAttemptData,
386+
refetch: jest.fn().mockResolvedValue({ data: mockAttemptData }),
387+
});
388+
const user = userEvent.setup();
389+
renderWithAlertAndIntl(<Allowances />);
390+
await user.click(screen.getByRole('button', { name: messages.actions.defaultMessage }));
391+
const editBtn = await screen.findByRole('button', { name: /edit/i });
392+
await user.click(editBtn);
393+
await waitFor(() => {
394+
expect(screen.getByRole('dialog', { name: messages.editAllowance.defaultMessage })).toBeInTheDocument();
395+
});
396+
});
397+
398+
it('opens delete modal when user has not attempted the exam', async () => {
399+
const mockAttemptData = {
400+
results: [], // no attempts
401+
count: 0,
402+
numPages: 0,
403+
};
404+
405+
(useAllowances as jest.Mock).mockReturnValue({
406+
data: { results: [mockAllowance], count: 1, numPages: 1 },
407+
isLoading: false,
408+
});
409+
(useAttempts as jest.Mock).mockReturnValue({
410+
data: mockAttemptData,
411+
refetch: jest.fn().mockResolvedValue({ data: mockAttemptData }),
412+
});
413+
const user = userEvent.setup();
414+
renderWithAlertAndIntl(<Allowances />);
415+
await user.click(screen.getByRole('button', { name: messages.actions.defaultMessage }));
416+
const deleteBtn = await screen.findByRole('button', { name: /delete/i });
417+
await user.click(deleteBtn);
418+
await waitFor(() => {
419+
expect(screen.getByRole('dialog', { name: messages.deleteAllowance.defaultMessage })).toBeInTheDocument();
420+
});
421+
});
422+
});
276423
});

src/specialExams/components/Allowances.tsx

Lines changed: 59 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,86 @@
1-
import { useState } from 'react';
1+
import { useState, useEffect } from 'react';
2+
import { useParams } from 'react-router-dom';
3+
import { useIntl } from '@openedx/frontend-base';
24
import { useToggle } from '@openedx/paragon';
35
import AddAllowanceModal from '@src/specialExams/components/AddAllowanceModal';
46
import AllowancesList from '@src/specialExams/components/AllowancesList';
57
import EditAllowanceModal from '@src/specialExams/components/EditAllowanceModal';
68
import DeleteAllowanceModal from '@src/specialExams/components/DeleteAllowanceModal';
9+
import { useAttempts } from '@src/specialExams/data/apiHook';
10+
import messages from '@src/specialExams/messages';
711
import { Allowance } from '@src/specialExams/types';
12+
import { useAlert } from '@src/providers/AlertProvider';
813

914
const Allowances = () => {
15+
const intl = useIntl();
16+
const { courseId = '' } = useParams<{ courseId: string }>();
17+
const { showModal } = useAlert();
1018
const [isAddModalOpen, openAddModal, closeAddModal] = useToggle(false);
1119
const [isEditModalOpen, openEditModal, closeEditModal] = useToggle(false);
1220
const [isDeleteModalOpen, openDeleteModal, closeDeleteModal] = useToggle(false);
1321
const [selectedAllowance, setSelectedAllowance] = useState<Allowance | null>(null);
22+
const [pendingAction, setPendingAction] = useState<'edit' | 'delete' | null>(null);
23+
24+
const { refetch } = useAttempts(courseId, {
25+
page: 0,
26+
pageSize: 100,
27+
emailOrUsername: selectedAllowance?.user.username ?? '',
28+
ordering: '',
29+
}, false); // disabled by default — only runs on refetch
30+
31+
// Handle the attempt check after selectedAllowance and pendingAction are set
32+
useEffect(() => {
33+
if (!selectedAllowance || !pendingAction) {
34+
return;
35+
}
36+
37+
const checkAttempt = async () => {
38+
const { data } = await refetch();
39+
const hasAttempt = data?.results.some(
40+
(attempt) => attempt.examId === selectedAllowance.proctoredExam.id
41+
);
42+
43+
if (hasAttempt) {
44+
showModal({
45+
message: intl.formatMessage(messages.cannotModifyAllowance, {
46+
action: pendingAction,
47+
username: selectedAllowance.user.username,
48+
}),
49+
variant: 'warning',
50+
});
51+
setSelectedAllowance(null);
52+
setPendingAction(null);
53+
} else if (pendingAction === 'edit') {
54+
openEditModal();
55+
setPendingAction(null);
56+
} else {
57+
openDeleteModal();
58+
setPendingAction(null);
59+
}
60+
};
61+
62+
checkAttempt();
63+
}, [selectedAllowance, pendingAction, refetch, showModal, intl, openEditModal, openDeleteModal]);
1464

1565
const handleEditAllowance = (allowance: Allowance) => {
1666
setSelectedAllowance(allowance);
17-
openEditModal();
67+
setPendingAction('edit');
68+
};
69+
70+
const handleDeleteAllowance = (allowance: Allowance) => {
71+
setSelectedAllowance(allowance);
72+
setPendingAction('delete');
1873
};
1974

2075
const handleCloseEditModal = () => {
2176
setSelectedAllowance(null);
77+
setPendingAction(null);
2278
closeEditModal();
2379
};
2480

25-
const handleDeleteAllowance = (allowance: Allowance) => {
26-
setSelectedAllowance(allowance);
27-
openDeleteModal();
28-
};
29-
3081
const handleCloseDeleteModal = () => {
3182
setSelectedAllowance(null);
83+
setPendingAction(null);
3284
closeDeleteModal();
3385
};
3486

src/specialExams/data/apiHook.test.tsx

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,40 @@ describe('specialExams api hooks', () => {
241241

242242
expect(mockGetAttempts).toHaveBeenLastCalledWith(newCourseId, params);
243243
});
244+
245+
it('does not fetch when enabled is false', () => {
246+
const { result } = renderHook(() => useAttempts(courseId, params, false), {
247+
wrapper: createWrapper(),
248+
});
249+
expect(result.current.isLoading).toBe(false);
250+
expect(result.current.fetchStatus).toBe('idle');
251+
expect(mockGetAttempts).not.toHaveBeenCalled();
252+
});
253+
254+
it('fetches when enabled is true', async () => {
255+
mockGetAttempts.mockResolvedValue(mockAttemptsData);
256+
const { result } = renderHook(() => useAttempts(courseId, params, true), {
257+
wrapper: createWrapper(),
258+
});
259+
expect(result.current.isLoading).toBe(true);
260+
await waitFor(() => {
261+
expect(result.current.isSuccess).toBe(true);
262+
});
263+
expect(mockGetAttempts).toHaveBeenCalledWith(courseId, params);
264+
expect(result.current.data).toBe(mockAttemptsData);
265+
});
266+
267+
it('can be manually refetched when enabled is false', async () => {
268+
mockGetAttempts.mockResolvedValue(mockAttemptsData);
269+
const { result } = renderHook(() => useAttempts(courseId, params, false), {
270+
wrapper: createWrapper(),
271+
});
272+
expect(mockGetAttempts).not.toHaveBeenCalled();
273+
expect(result.current.fetchStatus).toBe('idle');
274+
const refetchResult = await result.current.refetch();
275+
expect(mockGetAttempts).toHaveBeenCalledWith(courseId, params);
276+
expect(refetchResult.data).toBe(mockAttemptsData);
277+
});
244278
});
245279

246280
describe('useAllowances', () => {

src/specialExams/data/apiHook.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,11 @@ import { addAllowance, deleteAllowance, getAllowances, getAttempts, getSpecialEx
33
import { specialExamsQueryKeys } from './queryKeys';
44
import { AddAllowanceParams, AttemptsParams, DeleteAllowanceParams } from '../types';
55

6-
export const useAttempts = (courseId: string, params: AttemptsParams) => (
6+
export const useAttempts = (courseId: string, params: AttemptsParams, enabled = true) => (
77
useQuery({
88
queryKey: specialExamsQueryKeys.attempts(courseId, params),
99
queryFn: () => getAttempts(courseId, params),
10-
enabled: !!courseId,
10+
enabled: !!courseId && enabled,
1111
})
1212
);
1313

src/specialExams/messages.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,11 @@ const messages = defineMessages({
220220
id: 'instruct.specialExams.editAllowanceError',
221221
defaultMessage: 'An error occurred while editing the allowance. Please try again.',
222222
description: 'Error message displayed when there is an issue editing an allowance'
223+
},
224+
cannotModifyAllowance: {
225+
id: 'instruct.specialExams.cannotModifyAllowance',
226+
defaultMessage: 'Cannot {action} allowance: {username} has already attempted the exam',
227+
description: 'Warning message shown when trying to edit or delete an allowance for a student who has already attempted the exam'
223228
}
224229
});
225230

0 commit comments

Comments
 (0)