Skip to content

Commit 4913b10

Browse files
diana-villalvazo-wgudiana-villalvazo-wgu
authored
fix: improve error handling in cohorts (#211)
1 parent f5919f0 commit 4913b10

7 files changed

Lines changed: 250 additions & 12 deletions

File tree

‎src/cohorts/CohortsPage.test.tsx‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,17 @@ import { useCohorts, useCohortStatus, useToggleCohorts } from './data/apiHook';
55
import { renderWithAlertAndIntl } from '@src/testUtils';
66
import messages from './messages';
77
import { CohortProvider } from './components/CohortContext';
8+
import * as AlertProvider from '@src/providers/AlertProvider';
89

910
jest.mock('react-router-dom', () => ({
1011
...jest.requireActual('react-router-dom'),
1112
useParams: () => ({ courseId: 'course-v1:edX+Test+2024' }),
1213
}));
1314

15+
jest.mock('axios', () => ({
16+
isAxiosError: (error: any) => error?.isAxiosError === true,
17+
}));
18+
1419
jest.mock('./data/apiHook', () => ({
1520
useCohorts: jest.fn(),
1621
useCohortStatus: jest.fn(),
@@ -82,4 +87,111 @@ describe('CohortsPage', () => {
8287
await user.click(screen.getByRole('button', { name: messages.disableLabel.defaultMessage }));
8388
expect(disableMock).toHaveBeenCalled();
8489
});
90+
91+
it('shows error modal when enable cohorts fails', async () => {
92+
const showModalMock = jest.fn();
93+
jest.spyOn(AlertProvider, 'useAlert').mockReturnValue({
94+
alerts: [],
95+
addAlert: jest.fn(),
96+
removeAlert: jest.fn(),
97+
clearAlerts: jest.fn(),
98+
showToast: jest.fn(),
99+
showModal: showModalMock,
100+
showInlineAlert: jest.fn(),
101+
dismissInlineAlert: jest.fn(),
102+
inlineAlerts: [],
103+
});
104+
105+
const enableMock = jest.fn();
106+
(useCohorts as jest.Mock).mockReturnValue({ data: [] });
107+
(useCohortStatus as jest.Mock).mockReturnValue({ data: { isCohorted: false } });
108+
(useToggleCohorts as jest.Mock).mockReturnValue({ mutate: enableMock });
109+
110+
renderWithCohortsProvider();
111+
const user = userEvent.setup();
112+
await user.click(screen.getByRole('button', { name: messages.enableCohorts.defaultMessage }));
113+
114+
// Simulate error callback
115+
const callArgs = enableMock.mock.calls[0][1];
116+
callArgs.onError(new Error('Enable failed'));
117+
118+
expect(showModalMock).toHaveBeenCalledWith({
119+
confirmText: messages.closeButton.defaultMessage,
120+
message: messages.enableCohortError.defaultMessage,
121+
variant: 'danger',
122+
});
123+
});
124+
125+
it('shows error modal with API message when enable cohorts fails with axios error', async () => {
126+
const showModalMock = jest.fn();
127+
jest.spyOn(AlertProvider, 'useAlert').mockReturnValue({
128+
alerts: [],
129+
addAlert: jest.fn(),
130+
removeAlert: jest.fn(),
131+
clearAlerts: jest.fn(),
132+
showToast: jest.fn(),
133+
showModal: showModalMock,
134+
showInlineAlert: jest.fn(),
135+
dismissInlineAlert: jest.fn(),
136+
inlineAlerts: [],
137+
});
138+
139+
const enableMock = jest.fn();
140+
(useCohorts as jest.Mock).mockReturnValue({ data: [] });
141+
(useCohortStatus as jest.Mock).mockReturnValue({ data: { isCohorted: false } });
142+
(useToggleCohorts as jest.Mock).mockReturnValue({ mutate: enableMock });
143+
144+
renderWithCohortsProvider();
145+
const user = userEvent.setup();
146+
await user.click(screen.getByRole('button', { name: messages.enableCohorts.defaultMessage }));
147+
148+
// Simulate axios error with developer_message
149+
const axiosError = {
150+
isAxiosError: true,
151+
response: { data: { developer_message: 'API specific error' } },
152+
};
153+
const callArgs = enableMock.mock.calls[0][1];
154+
callArgs.onError(axiosError);
155+
156+
expect(showModalMock).toHaveBeenCalledWith({
157+
confirmText: messages.closeButton.defaultMessage,
158+
message: 'API specific error',
159+
variant: 'danger',
160+
});
161+
});
162+
163+
it('shows error modal when disable cohorts fails', async () => {
164+
const showModalMock = jest.fn();
165+
jest.spyOn(AlertProvider, 'useAlert').mockReturnValue({
166+
alerts: [],
167+
addAlert: jest.fn(),
168+
removeAlert: jest.fn(),
169+
clearAlerts: jest.fn(),
170+
showToast: jest.fn(),
171+
showModal: showModalMock,
172+
showInlineAlert: jest.fn(),
173+
dismissInlineAlert: jest.fn(),
174+
inlineAlerts: [],
175+
});
176+
177+
const disableMock = jest.fn();
178+
(useCohorts as jest.Mock).mockReturnValue({ data: [{ id: '1', name: 'Cohort 1' }] });
179+
(useCohortStatus as jest.Mock).mockReturnValue({ data: { isCohorted: true } });
180+
(useToggleCohorts as jest.Mock).mockReturnValue({ mutate: disableMock });
181+
182+
renderWithCohortsProvider();
183+
const user = userEvent.setup();
184+
await user.click(screen.getByRole('button', { name: messages.disableCohorts.defaultMessage }));
185+
await user.click(screen.getByRole('button', { name: messages.disableLabel.defaultMessage }));
186+
187+
// Simulate error callback
188+
const callArgs = disableMock.mock.calls[0][1];
189+
callArgs.onError(new Error('Disable failed'));
190+
191+
expect(showModalMock).toHaveBeenCalledWith({
192+
confirmText: messages.closeButton.defaultMessage,
193+
message: messages.disableCohortError.defaultMessage,
194+
variant: 'danger',
195+
});
196+
});
85197
});

‎src/cohorts/CohortsPage.tsx‎

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,16 @@
1+
import { useState } from 'react';
2+
import { useParams } from 'react-router-dom';
3+
import { isAxiosError } from 'axios';
14
import { useIntl } from '@openedx/frontend-base';
25
import { IconButton } from '@openedx/paragon';
36
import { Settings } from '@openedx/paragon/icons';
4-
import { useParams } from 'react-router-dom';
5-
import { useState } from 'react';
67
import { CohortProvider, useCohortContext } from '@src/cohorts/components/CohortContext';
78
import DisableCohortsModal from '@src/cohorts/components/DisableCohortsModal';
89
import DisabledCohortsView from '@src/cohorts/components/DisabledCohortsView';
910
import EnabledCohortsView from '@src/cohorts/components/EnabledCohortsView';
1011
import { useCohortStatus, useToggleCohorts } from '@src/cohorts/data/apiHook';
1112
import messages from '@src/cohorts/messages';
13+
import { useAlert } from '@src/providers/AlertProvider';
1214
import './CohortsPage.scss';
1315

1416
const CohortsPageContent = () => {
@@ -19,19 +21,34 @@ const CohortsPageContent = () => {
1921
const [isOpenDisableModal, setIsOpenDisableModal] = useState(false);
2022
const { clearSelectedCohort } = useCohortContext();
2123
const { isCohorted = false } = cohortStatus ?? {};
24+
const { showModal } = useAlert();
2225

2326
const handleEnableCohorts = () => {
2427
toggleCohortsMutate({ isCohorted: true },
2528
{
26-
onError: (error) => console.log(error)
29+
onError: (error) => {
30+
const errorMessage = (isAxiosError(error) && error?.response?.data?.developer_message) || intl.formatMessage(messages.enableCohortError);
31+
showModal({
32+
confirmText: intl.formatMessage(messages.closeButton),
33+
message: errorMessage,
34+
variant: 'danger',
35+
});
36+
}
2737
});
2838
};
2939

3040
const handleDisableCohorts = () => {
3141
toggleCohortsMutate({ isCohorted: false },
3242
{
3343
onSuccess: () => clearSelectedCohort(),
34-
onError: (error) => console.log(error)
44+
onError: (error) => {
45+
const errorMessage = (isAxiosError(error) && error?.response?.data?.developer_message) || intl.formatMessage(messages.disableCohortError);
46+
showModal({
47+
confirmText: intl.formatMessage(messages.closeButton),
48+
message: errorMessage,
49+
variant: 'danger',
50+
});
51+
}
3552
});
3653
setIsOpenDisableModal(false);
3754
};

‎src/cohorts/components/CohortCard.test.tsx‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ jest.mock('react-router-dom', () => ({
1919
useParams: jest.fn(),
2020
}));
2121

22+
jest.mock('axios', () => ({
23+
isAxiosError: (error: any) => error?.isAxiosError === true,
24+
}));
25+
2226
jest.mock('@src/cohorts/data/apiHook', () => ({
2327
usePatchCohort: jest.fn(),
2428
useAddLearnersToCohort: jest.fn(),
@@ -212,4 +216,64 @@ describe('CohortCard', () => {
212216
expect(screen.getByText(/0 students/)).toBeInTheDocument();
213217
});
214218
});
219+
220+
describe('Error Handling', () => {
221+
it('should show error modal when edit cohort fails with generic error', async () => {
222+
const showModalMock = jest.fn();
223+
mockUseAlert.mockReturnValue({ clearAlerts: mockClearAlerts, showModal: showModalMock });
224+
225+
const user = userEvent.setup();
226+
renderCohortCard();
227+
228+
const settingsTab = screen.getByText(messages.settings.defaultMessage);
229+
await user.click(settingsTab);
230+
await waitFor(() => {
231+
expect(screen.getByText(messages.cohortAssignmentMethod.defaultMessage)).toBeInTheDocument();
232+
});
233+
234+
mockMutate.mockImplementation((_options: any, callbacks: { onError: (error: any) => void }) => {
235+
callbacks.onError(new Error('Generic error'));
236+
});
237+
238+
const submitBtn = screen.getByRole('button', { name: messages.saveLabel.defaultMessage });
239+
await user.click(submitBtn);
240+
241+
expect(showModalMock).toHaveBeenCalledWith({
242+
confirmText: messages.closeButton.defaultMessage,
243+
message: messages.editCohortError.defaultMessage,
244+
variant: 'danger',
245+
});
246+
});
247+
248+
it('should show error modal with API message when edit cohort fails with axios error', async () => {
249+
const showModalMock = jest.fn();
250+
mockUseAlert.mockReturnValue({ clearAlerts: mockClearAlerts, showModal: showModalMock });
251+
252+
const user = userEvent.setup();
253+
renderCohortCard();
254+
255+
const settingsTab = screen.getByText(messages.settings.defaultMessage);
256+
await user.click(settingsTab);
257+
await waitFor(() => {
258+
expect(screen.getByText(messages.cohortAssignmentMethod.defaultMessage)).toBeInTheDocument();
259+
});
260+
261+
const axiosError = {
262+
isAxiosError: true,
263+
response: { data: { developer_message: 'Cohort name already exists' } },
264+
};
265+
mockMutate.mockImplementation((_options: any, callbacks: { onError: (error: any) => void }) => {
266+
callbacks.onError(axiosError);
267+
});
268+
269+
const submitBtn = screen.getByRole('button', { name: messages.saveLabel.defaultMessage });
270+
await user.click(submitBtn);
271+
272+
expect(showModalMock).toHaveBeenCalledWith({
273+
confirmText: messages.closeButton.defaultMessage,
274+
message: 'Cohort name already exists',
275+
variant: 'danger',
276+
});
277+
});
278+
});
215279
});

‎src/cohorts/components/CohortCard.tsx‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1-
import { useParams } from 'react-router-dom';
21
import { useRef, useState } from 'react';
2+
import { useParams } from 'react-router-dom';
3+
import { isAxiosError } from 'axios';
34
import { FormattedMessage, getExternalLinkUrl, useIntl } from '@openedx/frontend-base';
45
import { Card, Hyperlink, Tab, Tabs, Toast } from '@openedx/paragon';
56
import messages from '@src/cohorts/messages';
@@ -41,9 +42,12 @@ const CohortCard = () => {
4142
setShowSuccessMessage(true);
4243
setSelectedCohort({ ...selectedCohort, ...updatedCohort });
4344
},
44-
onError: (error: Error) => {
45+
onError: (error) => {
46+
const errorMessage = (isAxiosError(error) && error?.response?.data?.developer_message) || intl.formatMessage(messages.editCohortError);
4547
showModal({
46-
message: error.message,
48+
confirmText: intl.formatMessage(messages.closeButton),
49+
message: errorMessage,
50+
variant: 'danger',
4751
});
4852
}
4953
}

‎src/cohorts/components/EnabledCohortsView.test.tsx‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,19 @@ describe('EnabledCohortsView', () => {
154154
});
155155

156156
it('handles cohort creation error', async () => {
157+
const showModalMock = jest.fn();
158+
useAlertSpy.mockReturnValue({
159+
alerts: [],
160+
addAlert: addAlertMock,
161+
removeAlert: removeAlertMock,
162+
clearAlerts: clearAlertsMock,
163+
showToast: jest.fn(),
164+
showModal: showModalMock,
165+
showInlineAlert: jest.fn(),
166+
dismissInlineAlert: jest.fn(),
167+
inlineAlerts: []
168+
});
169+
157170
(useCohorts as jest.Mock).mockReturnValue({ data: [] });
158171
renderWithCohortProvider();
159172
const user = userEvent.setup();
@@ -169,11 +182,13 @@ describe('EnabledCohortsView', () => {
169182

170183
// Simulate error callback
171184
const createArgs = createCohortMock.mock.calls[0][1];
172-
const consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
173185
createArgs.onError('Creation failed');
174186

175-
expect(consoleErrorSpy).toHaveBeenCalledWith('Creation failed');
176-
consoleErrorSpy.mockRestore();
187+
expect(showModalMock).toHaveBeenCalledWith({
188+
confirmText: messages.closeButton.defaultMessage,
189+
message: messages.enableCohortError.defaultMessage,
190+
variant: 'danger',
191+
});
177192
});
178193

179194
it('handles successful cohort creation', async () => {

‎src/cohorts/components/EnabledCohortsView.tsx‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { useState, useEffect } from 'react';
22
import { useParams } from 'react-router-dom';
3+
import { isAxiosError } from 'axios';
34
import { useIntl } from '@openedx/frontend-base';
45
import { FormControl, Button, Card, Alert } from '@openedx/paragon';
56
import { CheckCircle, Error, WarningFilled } from '@openedx/paragon/icons';
@@ -25,7 +26,7 @@ const EnabledCohortsView = () => {
2526
const { mutate: createCohort } = useCreateCohort(courseId);
2627
const { clearSelectedCohort, selectedCohort, setSelectedCohort } = useCohortContext();
2728
const [displayAddForm, setDisplayAddForm] = useState(false);
28-
const { alerts, addAlert, removeAlert, clearAlerts } = useAlert();
29+
const { alerts, addAlert, removeAlert, clearAlerts, showModal } = useAlert();
2930

3031
const cohortsList = [{ id: 'null', name: intl.formatMessage(messages.selectCohortPlaceholder) }, ...data];
3132

@@ -89,7 +90,12 @@ const EnabledCohortsView = () => {
8990
hideAddForm();
9091
},
9192
onError: (error) => {
92-
console.error(error);
93+
const errorMessage = (isAxiosError(error) && error?.response?.data?.developer_message) || intl.formatMessage(messages.enableCohortError);
94+
showModal({
95+
confirmText: intl.formatMessage(messages.closeButton),
96+
message: errorMessage,
97+
variant: 'danger',
98+
});
9399
}
94100
});
95101
};

‎src/cohorts/messages.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,26 @@ const messages = defineMessages({
231231
defaultMessage: 'No file found in upload data. Please try again.',
232232
description: 'Error message displayed when no file is found in the uploaded data'
233233
},
234+
closeButton: {
235+
id: 'instruct.cohorts.closeButton',
236+
defaultMessage: 'Close',
237+
description: 'Label for the close button'
238+
},
239+
enableCohortError: {
240+
id: 'instruct.cohorts.enableCohortError',
241+
defaultMessage: 'An error occurred while enabling cohorts. Please try again later.',
242+
description: 'Error message displayed when enabling cohorts fails'
243+
},
244+
disableCohortError: {
245+
id: 'instruct.cohorts.disableCohortError',
246+
defaultMessage: 'An error occurred while disabling cohorts. Please try again later.',
247+
description: 'Error message displayed when disabling cohorts fails'
248+
},
249+
editCohortError: {
250+
id: 'instruct.cohorts.editCohortError',
251+
defaultMessage: 'An error occurred while saving your changes. Please try again later.',
252+
description: 'Error message displayed when editing a cohort fails'
253+
}
234254
});
235255

236256
export default messages;

0 commit comments

Comments
 (0)