Skip to content

Commit b66e27b

Browse files
fix: specify learner, warning message on grant beta tester role, ordering in special exams (#196)
* feat: add course info slot to header * fix: ordering special exams tables * feat: add warning message when grant beta tester * fix: error message on specify learner * test: update related tests
1 parent 8e501e6 commit b66e27b

19 files changed

Lines changed: 215 additions & 41 deletions

src/components/SpecifyLearnerField.test.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ describe('SpecifyLearnerField', () => {
214214
(useCourseInfo as jest.Mock).mockReturnValue({ data: { permissions: { admin: true, dataResearcher: false } } });
215215
(useLearner as jest.Mock).mockReturnValue({
216216
data: {},
217-
refetch: jest.fn().mockResolvedValue({ data: {} }),
217+
refetch: jest.fn().mockResolvedValue({ data: {}, error: { isAxiosError: true, response: { status: 404 } } }),
218218
error: { isAxiosError: true, response: { status: 404 } },
219219
});
220220
});

src/components/SpecifyLearnerField.tsx

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,24 @@ interface SpecifyLearnerFieldRef {
1818
reset: () => void,
1919
}
2020

21+
type ErrorState = 'not_enrolled' | 'not_found' | 'generic_error' | null;
22+
23+
const errorMessages = {
24+
not_found: messages.learnerNotFound,
25+
generic_error: messages.learnerGenericError,
26+
not_enrolled: messages.learnerNotEnrolled,
27+
};
28+
29+
const getErrorState = (error: Error | null, data?: SelectedLearner): ErrorState => {
30+
const isNotFoundError = isAxiosError(error)
31+
&& (error.response?.status === 404 || error.response?.status === 400);
32+
33+
if (isNotFoundError) return 'not_found';
34+
if (data && !data.isEnrolled) return 'not_enrolled';
35+
if (error && !isNotFoundError) return 'generic_error';
36+
return null;
37+
};
38+
2139
const initialLearnerState = {
2240
username: '',
2341
fullName: '',
@@ -30,18 +48,20 @@ const SpecifyLearnerField = forwardRef<SpecifyLearnerFieldRef, SpecifyLearnerFie
3048
const { courseId = '' } = useParams<{ courseId: string }>();
3149
const [identifier, setIdentifier] = useState('');
3250
const [showLearner, enableShowLearner, disableShowLearner] = useToggle(!!learner);
51+
const [errorState, setErrorState] = useState<ErrorState>(null);
3352
const { data: courseInfo } = useCourseInfo(courseId);
3453
const permissions = courseInfo?.permissions || { admin: false, dataResearcher: false };
3554
const { inputValue, handleChange, resetFilter } = useDebouncedFilter({
3655
filterValue: identifier,
3756
setFilter: setIdentifier,
3857
});
39-
const { data, refetch, error } = useLearner(courseId, inputValue);
58+
const { data, refetch } = useLearner(courseId, inputValue);
4059

4160
const resetState = () => {
4261
resetFilter();
4362
onClickSelect('');
4463
disableShowLearner();
64+
setErrorState(null);
4565
};
4666

4767
useImperativeHandle(ref, () => ({
@@ -53,6 +73,10 @@ const SpecifyLearnerField = forwardRef<SpecifyLearnerFieldRef, SpecifyLearnerFie
5373
const handleInputChange = (event: ChangeEvent<HTMLInputElement>) => {
5474
handleChange(event.target.value);
5575

76+
if (errorState) {
77+
setErrorState(null);
78+
}
79+
5680
if (showLearner) {
5781
disableShowLearner();
5882
}
@@ -64,6 +88,8 @@ const SpecifyLearnerField = forwardRef<SpecifyLearnerFieldRef, SpecifyLearnerFie
6488
// Need to pass empty value if learner is not valid to clear out any previously selected learner
6589
// We could have other conditions/fields depending on valid learner
6690
const formValue = !result.error && result.data?.isEnrolled ? inputValue : '';
91+
92+
setErrorState(getErrorState(result.error, result?.data));
6793
onClickSelect(formValue);
6894
enableShowLearner();
6995
});
@@ -102,20 +128,11 @@ const SpecifyLearnerField = forwardRef<SpecifyLearnerFieldRef, SpecifyLearnerFie
102128
<Button onClick={handleClickSelect} disabled={!inputValue}>{intl.formatMessage(messages.select)}</Button>
103129
)}
104130
</div>
105-
{showLearner && error
106-
&& isAxiosError(error)
107-
&& error.response?.status === 404 && (
131+
{showLearner && errorState && (
108132
<p className="text-danger-500 mb-0 x-small mt-2">
109-
{intl.formatMessage(messages.learnerNotFound, { identifier })}
133+
{intl.formatMessage(errorMessages[errorState], { identifier: inputValue })}
110134
</p>
111135
)}
112-
{
113-
showLearner && !error && !selectedLearner?.isEnrolled && (
114-
<p className="text-danger-500 mb-0 x-small mt-2">
115-
{intl.formatMessage(messages.learnerNotEnrolled, { identifier })}
116-
</p>
117-
)
118-
}
119136
</FormGroup>
120137
);
121138
});

src/components/SpecifyProblemField.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ const SpecifyProblemField = forwardRef<SpecifyProblemFieldRef, SpecifyProblemFie
127127
</div>
128128
{showSelectedLocation && error
129129
&& isAxiosError(error)
130-
&& (error.response?.status === 400) && (
130+
&& ((error.response?.status === 400) || (error.response?.status === 404)) && (
131131
<p className="text-danger-500 mb-0 x-small mt-2">
132132
{intl.formatMessage(messages.problemNotFound, { identifier: inputValue })}
133133
</p>

src/components/messages.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,11 @@ const messages = defineMessages({
156156
defaultMessage: 'Selected Learner:',
157157
description: 'Label for specify learner field when a learner has been selected',
158158
},
159+
learnerGenericError: {
160+
id: 'instruct.specifyLearner.learnerGenericError',
161+
defaultMessage: 'An error occurred while looking up {identifier}. Please try again.',
162+
description: 'Generic error message displayed when there is an error looking up a learner based on the provided identifier (email or username)',
163+
}
159164
});
160165

161166
export default messages;

src/enrollments/components/UpdateBetaTesterModal.test.tsx

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -181,8 +181,8 @@ describe('UpdateBetaTesterModal', () => {
181181
it('shows alert with failed usernames when some users fail (remove action)', async () => {
182182
const mockSuccessData = {
183183
results: [
184-
{ identifier: 'failed-user', userDoesNotExist: true },
185-
{ identifier: 'jane.doe', userDoesNotExist: false },
184+
{ identifier: 'failed-user', userDoesNotExist: true, isActive: true },
185+
{ identifier: 'jane.doe', userDoesNotExist: false, isActive: true },
186186
]
187187
};
188188

@@ -206,7 +206,7 @@ describe('UpdateBetaTesterModal', () => {
206206
it('shows alert with failed usernames when some users fail (add action)', async () => {
207207
const mockSuccessData = {
208208
results: [
209-
{ identifier: 'failed-user', userDoesNotExist: true },
209+
{ identifier: 'failed-user', userDoesNotExist: true, isActive: null },
210210
]
211211
};
212212

@@ -231,10 +231,39 @@ describe('UpdateBetaTesterModal', () => {
231231
});
232232
});
233233

234+
it('shows alert with inactive usernames when some users are inactive', async () => {
235+
const mockSuccessData = {
236+
results: [
237+
{ identifier: 'inactive-user', userDoesNotExist: false, isActive: false },
238+
{ identifier: 'jane.doe', userDoesNotExist: false, isActive: true },
239+
]
240+
};
241+
242+
const mutateWithSuccess = (_params: UpdateBetaTestersParams, callbacks: any) => {
243+
callbacks.onSuccess(mockSuccessData);
244+
};
245+
mutateMock.mockImplementation(mutateWithSuccess);
246+
247+
const nonBetaTesterProps = {
248+
...defaultProps,
249+
learner: learnerNonBetaTester,
250+
};
251+
252+
renderWithAlertAndIntl(
253+
<UpdateBetaTesterModal {...nonBetaTesterProps} />
254+
);
255+
256+
expect(mockAddAlert).toHaveBeenCalledWith({
257+
type: 'warning',
258+
message: messages.inactiveUsers.defaultMessage,
259+
extraContent: expect.any(Array),
260+
});
261+
});
262+
234263
it('does not show alert when all users succeed', async () => {
235264
const mockSuccessData = {
236265
results: [
237-
{ identifier: 'jane.doe', userDoesNotExist: false },
266+
{ identifier: 'jane.doe', userDoesNotExist: false, isActive: true },
238267
]
239268
};
240269

src/enrollments/components/UpdateBetaTesterModal.tsx

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ const UpdateBetaTesterModal = ({ learner, isOpen, onClose }: UpdateBetaTesterMod
2626
}, {
2727
onSuccess: (data) => {
2828
const failedUsernames = data.results?.filter(user => user.userDoesNotExist).map(user => user.identifier) || [];
29+
const inactiveUsernames = data.results?.filter(user => !user.isActive && user.isActive !== null && !user.userDoesNotExist).map(user => user.identifier) || [];
2930
if (failedUsernames.length > 0) {
3031
addAlert({
3132
type: 'danger',
@@ -37,6 +38,17 @@ const UpdateBetaTesterModal = ({ learner, isOpen, onClose }: UpdateBetaTesterMod
3738
)
3839
});
3940
}
41+
if (inactiveUsernames.length > 0) {
42+
addAlert({
43+
type: 'warning',
44+
message: intl.formatMessage(messages.inactiveUsers),
45+
extraContent: (
46+
inactiveUsernames.map((learner: string) => (
47+
<p key={learner} className="mb-0">{intl.formatMessage(messages.inactiveLearner, { learner })}</p>
48+
))
49+
)
50+
});
51+
}
4052
},
4153
onError: () => {
4254
showModal({

src/grading/components/GradingLearnerContent.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,9 @@ const GradingLearnerContent = ({ toolType, onShowTasks }: GradingLearnerContentP
165165

166166
const handleLearnerChange = (usernameOrEmail: string): void => {
167167
setUsernameOrEmail(usernameOrEmail);
168+
// Reset problem field when learner changes due to progress and attempts change for every learner
169+
setBlockId('');
170+
problemFieldRef.current?.reset();
168171
};
169172

170173
return (

src/slots.tsx

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,15 @@
1-
import { helpButtonSlotOperation, SlotOperation } from '@openedx/frontend-base';
2-
3-
import { appId, instructorDashboardRole } from './constants';
1+
import { helpButtonSlotOperation, SlotOperation, WidgetOperationTypes } from '@openedx/frontend-base';
2+
import CourseInfoSlot from '@src/slots/CourseInfoSlot/CourseInfoSlot';
3+
import { appId, instructorDashboardRole } from '@src/constants';
44

55
const slots: SlotOperation[] = [
66
helpButtonSlotOperation({ appId, role: instructorDashboardRole }),
7+
{
8+
slotId: 'org.openedx.frontend.slot.header.primaryLinks.v1',
9+
id: 'org.openedx.frontend.widget.slotShowcase.headerLink',
10+
op: WidgetOperationTypes.APPEND,
11+
element: <CourseInfoSlot />,
12+
},
713
];
814

915
export default slots;
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
import { screen } from '@testing-library/react';
2+
import CourseInfoSlot from '@src/slots/CourseInfoSlot/CourseInfoSlot';
3+
import { renderWithQueryClient } from '@src/testUtils';
4+
5+
jest.mock('@src/data/apiHook', () => ({
6+
useCourseInfo: () => ({
7+
data: {
8+
displayName: 'My Test Course',
9+
org: 'edX',
10+
courseNumber: '40-800',
11+
},
12+
}),
13+
}));
14+
15+
jest.mock('react-router-dom', () => ({
16+
useParams: () => ({
17+
courseId: 'course-v1:edX+Test+2023',
18+
}),
19+
}));
20+
21+
describe('CourseInfoSlot', () => {
22+
it('renders org and course name', () => {
23+
renderWithQueryClient(<CourseInfoSlot />);
24+
expect(screen.getByText('My Test Course')).toBeInTheDocument();
25+
expect(screen.getByText('edX 40-800')).toBeInTheDocument();
26+
});
27+
});
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
import { useParams } from 'react-router-dom';
2+
import { useCourseInfo } from '@src/data/apiHook';
3+
4+
const CourseInfoSlot = () => {
5+
const { courseId = '' } = useParams();
6+
const { data } = useCourseInfo(courseId);
7+
8+
if (!data) {
9+
return null;
10+
}
11+
12+
const { org = '', courseNumber = '', displayName = '' } = data;
13+
14+
return (
15+
<div style={{ minWidth: 0, lineHeight: '1' }}>
16+
<span className="d-block small m-0">{org} {courseNumber}</span>
17+
<span className="d-block m-0 font-weight-bold course-title">{displayName}</span>
18+
</div>
19+
);
20+
};
21+
22+
export default CourseInfoSlot;

0 commit comments

Comments
 (0)