Skip to content

Commit 17baefe

Browse files
Muhammad Faraz MaqsoodMuhammad Faraz Maqsood
Muhammad Faraz Maqsood
authored and
Muhammad Faraz Maqsood
committed
refactor: localize missing strings & add constants
In this commit, - localized missing strings - add constants for hardcoded values in constants.js file - add test coverage for AlertList
1 parent f3cbda9 commit 17baefe

File tree

8 files changed

+153
-36
lines changed

8 files changed

+153
-36
lines changed

‎src/CourseTeamManagement/CoursesChangesModal.jsx‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
} from '@openedx/paragon';
1010
import { CheckCircleOutline, SpinnerSimple } from '@openedx/paragon/icons';
1111
import { useIntl } from '@edx/frontend-platform/i18n';
12+
import { INSTRUCTOR_ROLE } from './constants';
1213
import messages from './messages';
1314

1415
export default function CoursesChangesModal({
@@ -105,7 +106,9 @@ export default function CoursesChangesModal({
105106
}) => (
106107
<p key={`added-${runId}`}>
107108
<span>{`${courseName} (${number} - ${runId})`}</span>{' '}
108-
{role === 'instructor' ? 'Instructor' : 'Staff'}
109+
{role === INSTRUCTOR_ROLE
110+
? intl.formatMessage(messages.instructorRole)
111+
: intl.formatMessage(messages.staffRole)}
109112
</p>
110113
),
111114
)}
@@ -119,7 +122,9 @@ export default function CoursesChangesModal({
119122
}) => (
120123
<p key={`removed-${runId}`}>
121124
<span>{`${courseName} (${number} - ${runId})`}</span>{' '}
122-
{role === 'instructor' ? 'Instructor' : 'Staff'}
125+
{role === INSTRUCTOR_ROLE
126+
? intl.formatMessage(messages.instructorRole)
127+
: intl.formatMessage(messages.staffRole)}
123128
</p>
124129
),
125130
)}
@@ -132,7 +137,7 @@ export default function CoursesChangesModal({
132137
courseName, number, runId, from, to,
133138
}) => (
134139
<p key={`role-${runId}`}>
135-
<span className="font-medium">{`${courseName} (${number} - ${runId}) ${from === 'instructor' ? 'Instructor' : 'Staff'} → ${to === 'instructor' ? 'Instructor' : 'Staff'}`}</span>
140+
<span className="font-medium">{`${courseName} (${number} - ${runId}) ${from === INSTRUCTOR_ROLE ? intl.formatMessage(messages.instructorRole) : intl.formatMessage(messages.staffRole)} → ${to === INSTRUCTOR_ROLE ? intl.formatMessage(messages.instructorRole) : intl.formatMessage(messages.staffRole)}`}</span>
136141
</p>
137142
),
138143
)}
@@ -158,9 +163,9 @@ export default function CoursesChangesModal({
158163
{!hasError && (
159164
<StatefulButton
160165
labels={{
161-
default: 'Save',
162-
pending: 'Saving',
163-
complete: 'Saved',
166+
default: intl.formatMessage(messages.saveChangesButtonText),
167+
pending: intl.formatMessage(messages.savingChangesButtonText),
168+
complete: intl.formatMessage(messages.savedChangesButtonText),
164169
}}
165170
data-testid="confirm-save-course-changes"
166171
variant="danger"

‎src/CourseTeamManagement/CoursesTable.jsx‎

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@ import TableActions from './TableActions';
1515
import { getChangedRows } from './utils';
1616
import CoursesChangesModal from './CoursesChangesModal';
1717
import { updateUserRolesInCourses } from './data/api';
18+
import {
19+
ACTIVE_COURSE_STATUS, INSTRUCTOR_ROLE, NULL_ROLE, STAFF_ROLE,
20+
} from './constants';
1821

1922
export default function CoursesTable({
2023
username,
@@ -37,14 +40,14 @@ export default function CoursesTable({
3740

3841
let userCoursesData = userCourses;
3942
const [originalRowRoles] = useState(() => userCoursesData.reduce((acc, row) => {
40-
acc[row.course_id] = row.role == null ? 'null' : row.role;
43+
acc[row.course_id] = row.role == null ? NULL_ROLE : row.role;
4144
return acc;
4245
}, {}));
4346

4447
const [originalCheckedRows] = useState(() => {
4548
const initial = {};
4649
userCoursesData.forEach((row) => {
47-
if (row.role === 'staff' || row.role === 'instructor') {
50+
if (row.role === STAFF_ROLE || row.role === INSTRUCTOR_ROLE) {
4851
initial[row.course_id] = true;
4952
}
5053
});
@@ -92,7 +95,7 @@ export default function CoursesTable({
9295
// As we are considering 'null' to appear as staff with disabled dropdown
9396
userCoursesData = userCoursesData.map((course) => ({
9497
...course,
95-
role: course.role === null ? 'null' : course.role,
98+
role: course.role === null ? NULL_ROLE : course.role,
9699
}));
97100
const [search, setSearch] = useState('');
98101
const [status, setStatus] = useState('');
@@ -111,7 +114,7 @@ export default function CoursesTable({
111114
const sortedAndFilteredData = React.useMemo(() => {
112115
let data = userCoursesData.map((row) => ({
113116
...row,
114-
role: rowRoles[row.course_id] !== undefined ? rowRoles[row.course_id] : 'null',
117+
role: rowRoles[row.course_id] !== undefined ? rowRoles[row.course_id] : NULL_ROLE,
115118
}));
116119

117120
// Manual Filtering for all columns in single search box
@@ -155,7 +158,7 @@ export default function CoursesTable({
155158
const [checkedRows, setCheckedRows] = useState(() => {
156159
const initial = {};
157160
userCoursesData.forEach((row) => {
158-
if (row.role === 'staff' || row.role === 'instructor') {
161+
if (row.role === STAFF_ROLE || row.role === INSTRUCTOR_ROLE) {
159162
initial[row.course_id] = true;
160163
}
161164
});
@@ -230,9 +233,9 @@ export default function CoursesTable({
230233
const formatStatus = ({ value }) => (
231234
<Badge
232235
className="course-team-management-course-status-badge"
233-
variant={value === 'active' ? 'success' : 'light'}
236+
variant={value === ACTIVE_COURSE_STATUS ? 'success' : 'light'}
234237
>
235-
{value === 'active'
238+
{value === ACTIVE_COURSE_STATUS
236239
? intl.formatMessage(messages.activeCoursesFilterLabel)
237240
: intl.formatMessage(messages.archivedCoursesFilterLabel)}
238241
</Badge>
@@ -252,9 +255,9 @@ export default function CoursesTable({
252255
const courseId = row.original.course_id;
253256
const value = rowRoles[courseId];
254257
// If role is 'null', default to staff for display only
255-
const displayValue = value === 'null' ? 'staff' : value;
256-
let title = 'Staff';
257-
if (displayValue === 'instructor') { title = 'Admin'; }
258+
const displayValue = value === NULL_ROLE ? STAFF_ROLE : value;
259+
let title = intl.formatMessage(messages.statusStaffFilterLabelChoice);
260+
if (displayValue === INSTRUCTOR_ROLE) { title = intl.formatMessage(messages.statusAdminFilterLabelChoice); }
258261
// Enable dropdown if checkbox is checked, otherwise disable
259262
const isChecked = !!checkedRows[courseId];
260263
const isDisabled = !isChecked;
@@ -274,17 +277,17 @@ export default function CoursesTable({
274277
<Dropdown.Menu placement="top">
275278
<Dropdown.Item
276279
data-testid={`role-dropdown-item-staff-${courseId}`}
277-
eventKey="staff"
278-
active={displayValue === 'staff'}
279-
onClick={() => setRowRoles((prev) => ({ ...prev, [courseId]: 'staff' }))}
280+
eventKey={STAFF_ROLE}
281+
active={displayValue === STAFF_ROLE}
282+
onClick={() => setRowRoles((prev) => ({ ...prev, [courseId]: STAFF_ROLE }))}
280283
>
281284
{intl.formatMessage(messages.statusStaffFilterLabelChoice)}
282285
</Dropdown.Item>
283286
<Dropdown.Item
284287
data-testid={`role-dropdown-item-instructor-${courseId}`}
285-
eventKey="instructor"
286-
active={displayValue === 'instructor'}
287-
onClick={() => setRowRoles((prev) => ({ ...prev, [courseId]: 'instructor' }))}
288+
eventKey={INSTRUCTOR_ROLE}
289+
active={displayValue === INSTRUCTOR_ROLE}
290+
onClick={() => setRowRoles((prev) => ({ ...prev, [courseId]: INSTRUCTOR_ROLE }))}
288291
>
289292
{intl.formatMessage(messages.statusAdminFilterLabelChoice)}
290293
</Dropdown.Item>

‎src/CourseTeamManagement/TableActions.jsx‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,14 @@ import { useIntl } from '@edx/frontend-platform/i18n';
1010

1111
import OrgDropdownWithSearch from './OrgDropdownWithSearch';
1212
import messages from './messages';
13+
import {
14+
ACTIVE_COURSE_STATUS,
15+
ARCHIVED_COURSE_STATUS,
16+
COURSE_STATUS_DROPDOWN_OPTIONS,
17+
INSTRUCTOR_ROLE,
18+
ROLE_DROPDOWN_OPTIONS,
19+
STAFF_ROLE,
20+
} from './constants';
1321

1422
const TableActions = ({
1523
search,
@@ -27,9 +35,9 @@ const TableActions = ({
2735
}) => {
2836
const intl = useIntl();
2937
let statusLabelMessage = messages.allCoursesFilterLabel;
30-
if (status === 'active') {
38+
if (status === ACTIVE_COURSE_STATUS) {
3139
statusLabelMessage = messages.activeCoursesFilterLabel;
32-
} else if (status === 'archived') {
40+
} else if (status === ARCHIVED_COURSE_STATUS) {
3341
statusLabelMessage = messages.archivedCoursesFilterLabel;
3442
}
3543

@@ -58,11 +66,7 @@ const TableActions = ({
5866
className="ml-2"
5967
variant="outline-primary"
6068
>
61-
{[
62-
{ value: '', label: messages.allCoursesFilterLabel },
63-
{ value: 'active', label: messages.activeCoursesFilterLabel },
64-
{ value: 'archived', label: messages.archivedCoursesFilterLabel },
65-
].map((option) => (
69+
{COURSE_STATUS_DROPDOWN_OPTIONS.map((option) => (
6670
<Dropdown.Item
6771
data-testid={`status-dropdown-item-${option.value}`}
6872
key={option.value}
@@ -92,7 +96,7 @@ const TableActions = ({
9296
)
9397
}
9498
onSelect={(eventKey) => {
95-
if (eventKey === 'staff' || eventKey === 'instructor') {
99+
if (eventKey === STAFF_ROLE || eventKey === INSTRUCTOR_ROLE) {
96100
const filteredCourseIds = new Set(sortedAndFilteredData.map((row) => row.course_id));
97101
setRowRoles((prev) => {
98102
const updated = { ...prev };
@@ -108,10 +112,7 @@ const TableActions = ({
108112
className="ml-2"
109113
variant="outline-primary"
110114
>
111-
{[
112-
{ value: 'staff', label: messages.statusStaffFilterLabel },
113-
{ value: 'instructor', label: messages.statusAdminFilterLabel },
114-
].map((option) => (
115+
{ROLE_DROPDOWN_OPTIONS.map((option) => (
115116
<Dropdown.Item key={option.value} eventKey={option.value}>
116117
{intl.formatMessage(option.label)}
117118
</Dropdown.Item>

‎src/CourseTeamManagement/constants.js‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import messages from './messages';
2+
3+
export const INSTRUCTOR_ROLE = 'instructor';
4+
export const STAFF_ROLE = 'staff';
5+
export const NULL_ROLE = 'null';
6+
export const ACTIVE_COURSE_STATUS = 'active';
7+
export const ARCHIVED_COURSE_STATUS = 'archived';
8+
9+
export const COURSE_STATUS_DROPDOWN_OPTIONS = [
10+
{ value: '', label: messages.allCoursesFilterLabel },
11+
{ value: ACTIVE_COURSE_STATUS, label: messages.activeCoursesFilterLabel },
12+
{ value: ARCHIVED_COURSE_STATUS, label: messages.archivedCoursesFilterLabel },
13+
];
14+
15+
export const ROLE_DROPDOWN_OPTIONS = [
16+
{ value: STAFF_ROLE, label: messages.statusStaffFilterLabel },
17+
{ value: INSTRUCTOR_ROLE, label: messages.statusAdminFilterLabel },
18+
];

‎src/CourseTeamManagement/messages.js‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,5 +251,35 @@ const messages = defineMessages({
251251
defaultMessage: 'Unexpected error occured while fetching user courses.',
252252
description: 'Error message for Get API error.',
253253
},
254+
staffRole: {
255+
id: 'courseTeamManagementStaffRole',
256+
defaultMessage: 'Staff',
257+
description: 'Course Team Management Staff role text',
258+
},
259+
instructorRole: {
260+
id: 'courseTeamManagementInstructorRole',
261+
defaultMessage: 'Instructor',
262+
description: 'Course Team Management Instructor role text',
263+
},
264+
saveChangesButtonText: {
265+
id: 'courseTeamManagementSaveChangesButtonText',
266+
defaultMessage: 'Save',
267+
description: 'Course Team Management Save changes button text',
268+
},
269+
savingChangesButtonText: {
270+
id: 'courseTeamManagementSavingChangesButtonText',
271+
defaultMessage: 'Saving',
272+
description: 'Course Team Management Saving changes button text',
273+
},
274+
savedChangesButtonText: {
275+
id: 'courseTeamManagementSavedChangesButtonText',
276+
defaultMessage: 'Saved',
277+
description: 'Course Team Management Saved changes button text',
278+
},
279+
alertDismissBtnText: {
280+
id: 'alertDismissBtnText',
281+
defaultMessage: 'Dismiss',
282+
description: 'Alert dismiss button text',
283+
},
254284
});
255285
export default messages;

‎src/CourseTeamManagement/utils.js‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { STAFF_ROLE, NULL_ROLE } from './constants';
2+
13
export function getChangedRows(checkedRows, originalCheckedRows, rowRoles, originalRowRoles, userCoursesData) {
24
const newlyCheckedWithRole = [];
35
const uncheckedWithRole = [];
@@ -25,7 +27,7 @@ export function getChangedRows(checkedRows, originalCheckedRows, rowRoles, origi
2527
if (!wasChecked && isChecked && currentRole) {
2628
newlyCheckedWithRole.push({
2729
runId,
28-
role: currentRole === 'null' ? 'staff' : currentRole,
30+
role: currentRole === NULL_ROLE ? STAFF_ROLE : currentRole,
2931
courseName,
3032
number,
3133
courseId,

‎src/userMessages/Alert.jsx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import {
66
} from '@fortawesome/free-solid-svg-icons';
77
import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
88
import { Button } from '@openedx/paragon';
9+
import { FormattedMessage } from '@edx/frontend-platform/i18n';
10+
import messages from '../CourseTeamManagement/messages';
911

1012
function getAlertClass(type) {
1113
if (type === 'error') {
@@ -45,7 +47,7 @@ function Alert({
4547
<div role="alert" className="flex-grow-1">
4648
{children}
4749
</div>
48-
{dismissible && <Button variant="link" size="inline" className="dismiss-button" onClick={onDismiss}>Dismiss</Button>}
50+
{dismissible && <Button variant="link" size="inline" className="dismiss-button" onClick={onDismiss}><FormattedMessage {...messages.alertDismissBtnText} /></Button>}
4951
</div>
5052
</div>
5153
);

‎src/userMessages/AlertList.test.jsx‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
import React from 'react';
2+
import { render, screen, fireEvent } from '@testing-library/react';
3+
import AlertList from './AlertList';
4+
import UserMessagesContext from './UserMessagesContext';
5+
6+
const MockAlert = ({ onDismiss, children }) => (
7+
<div>
8+
<span>{children}</span>
9+
<button onClick={onDismiss}>Dismiss</button>
10+
</div>
11+
);
12+
13+
describe('AlertList', () => {
14+
const mockRemove = jest.fn();
15+
const mockSetIsDismissed = jest.fn();
16+
17+
const renderWithContext = (props = {}) => {
18+
const defaultMessages = [
19+
{ id: '1', code: 'test', type: 'info', text: 'Test alert', dismissible: true, topic: 'general' },
20+
];
21+
22+
return render(
23+
<UserMessagesContext.Provider value={{ remove: mockRemove, messages: defaultMessages }}>
24+
<AlertList
25+
customAlerts={{ test: MockAlert }}
26+
isDismissed={false}
27+
setIsDismissed={mockSetIsDismissed}
28+
{...props}
29+
/>
30+
</UserMessagesContext.Provider>
31+
);
32+
};
33+
34+
beforeEach(() => {
35+
jest.clearAllMocks();
36+
});
37+
38+
it('calls remove and setIsDismissed on dismiss', () => {
39+
renderWithContext();
40+
41+
// find the dismiss button inside MockAlert
42+
fireEvent.click(screen.getByText('Dismiss'));
43+
44+
expect(mockRemove).toHaveBeenCalledWith('1');
45+
expect(mockSetIsDismissed).toHaveBeenCalledWith(true);
46+
});
47+
48+
it('does not call setIsDismissed if already dismissed', () => {
49+
renderWithContext({ isDismissed: true });
50+
51+
fireEvent.click(screen.getByText('Dismiss'));
52+
53+
expect(mockRemove).toHaveBeenCalledWith('1');
54+
expect(mockSetIsDismissed).not.toHaveBeenCalled();
55+
});
56+
});

0 commit comments

Comments
 (0)