Skip to content

Commit c03eff7

Browse files
Address CodeRabbit PR expertiza#169: modal deps, async add user, a11y, quiz flag, remove errors
Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 99a2344 commit c03eff7

4 files changed

Lines changed: 75 additions & 41 deletions

File tree

src/pages/AssignmentParticipants/AssignmentParticipants.tsx

Lines changed: 53 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import { useParams } from 'react-router-dom';
1616
import { IAssignmentResponse } from 'utils/interfaces';
1717
import { HttpMethod } from 'utils/httpMethods';
1818
import { FaCheck, FaInfoCircle, FaTimes } from 'react-icons/fa';
19-
import { Form, Button } from 'react-bootstrap';
19+
import { Form, Button, OverlayTrigger, Tooltip } from 'react-bootstrap';
2020

2121
const UI_ROLE_TO_API_ROLE_NAME: Record<string, string> = {
2222
admin: 'administrator',
@@ -50,6 +50,7 @@ function AssignmentParticipants() {
5050
const [selectedParticipant, setSelectedParticipant] = useState<Participant | null>(null);
5151
const [error, setError] = useState<string | null>(null);
5252
const [saveError, setSaveError] = useState<string | null>(null);
53+
const [removeError, setRemoveError] = useState<string | null>(null);
5354

5455
const { assignmentId } = useParams();
5556

@@ -82,6 +83,8 @@ function AssignmentParticipants() {
8283
}
8384
}, [
8485
assignmentId,
86+
modalShow.edit,
87+
modalShow.remove,
8588
addParticipantResponse,
8689
updateParticipantResponse,
8790
updateUserResponse,
@@ -153,14 +156,14 @@ function AssignmentParticipants() {
153156
url: `/participants/${selectedParticipant.id}`,
154157
method: 'DELETE',
155158
});
156-
setError(null);
159+
setRemoveError(null);
157160
setModalShow({ edit: false, remove: false });
158161
} catch (err: any) {
159162
const message =
160163
err?.response?.data?.message ??
161164
err?.message ??
162165
'Failed to remove participant. Please try again.';
163-
setError(message);
166+
setRemoveError(message);
164167
}
165168
};
166169

@@ -208,7 +211,7 @@ function AssignmentParticipants() {
208211
};
209212

210213
/** Validates lookup input and adds a matched user as an assignment participant. */
211-
const handleAddUser = () => {
214+
const handleAddUser = async () => {
212215
if (!newUserName.trim()) {
213216
setError('Name must not be empty.');
214217
return;
@@ -226,18 +229,24 @@ function AssignmentParticipants() {
226229
return;
227230
}
228231

229-
setError(null);
230-
231-
addParticipant({
232-
url: `/participants/${selectedRole}`,
233-
method: 'POST',
234-
data: {
235-
user_id: user.id,
236-
assignment_id: Number(assignmentId),
237-
}
238-
});
239-
240-
setNewUserName('');
232+
try {
233+
await addParticipant({
234+
url: `/participants/${selectedRole}`,
235+
method: 'POST',
236+
data: {
237+
user_id: user.id,
238+
assignment_id: Number(assignmentId),
239+
},
240+
});
241+
setError(null);
242+
setNewUserName('');
243+
} catch (err: any) {
244+
const message =
245+
err?.response?.data?.message ??
246+
err?.message ??
247+
'Failed to add participant. Please try again.';
248+
setError(message);
249+
}
241250
};
242251

243252
return (
@@ -275,12 +284,14 @@ function AssignmentParticipants() {
275284
onChange={() => setSelectedRole(role as ParticipantRole)}
276285
/>
277286
{role}
278-
<FaInfoCircle
279-
className="ms-2 text-secondary"
280-
size={16}
281-
aria-label={participantRoleInfo(role)}
282-
title={participantRoleInfo(role)}
283-
/>
287+
<OverlayTrigger
288+
placement="top"
289+
overlay={<Tooltip id={`role-help-${role}`}>{participantRoleInfo(role)}</Tooltip>}
290+
>
291+
<span className="ms-2 d-inline-flex" tabIndex={0} role="button" aria-label={participantRoleInfo(role)}>
292+
<FaInfoCircle className="text-secondary" size={16} aria-hidden />
293+
</span>
294+
</OverlayTrigger>
284295
</label>
285296
))}
286297
</div>
@@ -333,19 +344,33 @@ function AssignmentParticipants() {
333344
)}
334345
<ConfirmRemoveModal
335346
show={modalShow.remove}
336-
onHide={() => setModalShow({ ...modalShow, remove: false })}
347+
errorMessage={removeError}
348+
onHide={() => {
349+
setRemoveError(null);
350+
setModalShow({ ...modalShow, remove: false });
351+
}}
337352
onConfirm={handleRemove}
338353
/>
339354
</div>
340355
);
341356
}
342357

343-
/** Returns the enabled/disabled permission icon for a permission cell. */
344-
export function permissionIcon(permission: IsEnabled) {
345-
return permission === IsEnabled.Yes ? (
346-
<FaCheck size={20} className="text-success" aria-label="Yes" />
358+
/**
359+
* Renders an accessible icon for a permission cell.
360+
* @param permission Whether the permission is granted.
361+
* @param permissionLabel Short column label for screen readers, e.g. "Review permission".
362+
*/
363+
export function permissionIcon(permission: IsEnabled, permissionLabel?: string) {
364+
const yes = permission === IsEnabled.Yes;
365+
const ariaLabel = permissionLabel
366+
? `${permissionLabel}: ${yes ? "Yes" : "No"}`
367+
: yes
368+
? "Permission enabled"
369+
: "Permission disabled";
370+
return yes ? (
371+
<FaCheck size={20} className="text-success" aria-label={ariaLabel} role="img" />
347372
) : (
348-
<FaTimes size={20} className="text-secondary" aria-label="No" />
373+
<FaTimes size={20} className="text-secondary" aria-label={ariaLabel} role="img" />
349374
);
350375
}
351376

src/pages/AssignmentParticipants/AssignmentParticipantsUtil.tsx

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,8 @@ export function findUserByIdentifier<T extends { id: number; name?: string | nul
3737
/** Table column visibility for quiz / mentor from GET /assignments/:id (snake_case from API). */
3838
export function assignmentTableFlagsFromResponse(assignment: IAssignmentResponse): AssignmentProperties {
3939
return {
40-
hasQuiz: Boolean(assignment.require_quiz) || Boolean(assignment.has_quizzes),
40+
// Require both flags so we do not show "Take quiz" when quizzes exist but are not required.
41+
hasQuiz: Boolean(assignment.require_quiz) && Boolean(assignment.has_quizzes),
4142
hasMentor: Boolean(assignment.has_mentors),
4243
};
4344
}
@@ -61,9 +62,8 @@ export function classForRole(role: Role): string {
6162
}
6263
}
6364

64-
export function iconForRole(role: Role): JSX.Element {
65-
// Role icons are intentionally omitted to follow the project icon guidelines
66-
// (use the standardized icon assets under public/assets/icons for actions).
65+
/** Intentionally empty: role row uses text styling only (project icon guidelines). */
66+
export function iconForRole(_role: Role): JSX.Element {
6767
return <></>;
6868
}
6969

@@ -90,6 +90,7 @@ export function participantRoleInfo(role: ParticipantRole): string {
9090
}
9191
}
9292

93+
/** Reads a dotted path from an object; only stops on null/undefined intermediates (not on false/0/""). */
9394
export function getNestedValue<T>(obj: T, path: string): any {
94-
return path.split('.').reduce((acc: any, key: string) => (acc ? acc[key as keyof typeof acc] : undefined), obj);
95+
return path.split(".").reduce((acc: any, key: string) => (acc == null ? undefined : acc[key]), obj);
9596
}

src/pages/AssignmentParticipants/ConfirmRemoveModal.tsx

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,21 +4,29 @@ import Button from 'react-bootstrap/Button';
44
interface ConfirmRemoveModalProps {
55
show: boolean;
66
onHide: () => void;
7-
onConfirm: () => void;
7+
onConfirm: () => void | Promise<void>;
8+
errorMessage?: string | null;
89
}
910

10-
{/* Changed the buttons for Cancel and Confirm according to the design standards */}
11-
function ConfirmRemoveModal({ show, onHide, onConfirm }: ConfirmRemoveModalProps) {
11+
function ConfirmRemoveModal({ show, onHide, onConfirm, errorMessage }: ConfirmRemoveModalProps) {
1212
return (
1313
<Modal show={show} onHide={onHide} centered>
1414
<Modal.Header closeButton>
1515
<Modal.Title>Confirm Removal</Modal.Title>
1616
</Modal.Header>
1717
<Modal.Body>
18-
Are you sure you want to remove this participant?
18+
<p className="mb-2">Are you sure you want to remove this participant?</p>
19+
{errorMessage ? (
20+
<div className="flash_note alert alert-danger error-message" role="alert">
21+
{errorMessage}
22+
</div>
23+
) : null}
1924
</Modal.Body>
2025
<Modal.Footer>
21-
<Button className= "btn btn-md" variant="danger" onClick={onConfirm}>
26+
<Button className="btn btn-md" variant="secondary" onClick={onHide}>
27+
Cancel
28+
</Button>
29+
<Button className="btn btn-md" variant="danger" onClick={() => void onConfirm()}>
2230
Confirm
2331
</Button>
2432
</Modal.Footer>

src/pages/AssignmentParticipants/ParticipantsTable.tsx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ function ParticipantTable({
4545
enableSorting: true,
4646
cell: ({ row }: { row: any }) => (
4747
<div className={`permission-column ${classForStatus(row.original.permissions.review)}`}>
48-
{permissionIcon(row.original.permissions.review)}
48+
{permissionIcon(row.original.permissions.review, "Review permission")}
4949
</div>
5050
),
5151
},
@@ -56,7 +56,7 @@ function ParticipantTable({
5656
enableSorting: true,
5757
cell: ({ row }: { row: any }) => (
5858
<div className={`permission-column ${classForStatus(row.original.permissions.submit)}`}>
59-
{permissionIcon(row.original.permissions.submit)}
59+
{permissionIcon(row.original.permissions.submit, "Submit permission")}
6060
</div>
6161
),
6262
},
@@ -78,7 +78,7 @@ function ParticipantTable({
7878
enableSorting: true,
7979
cell: ({ row }: { row: any }) => (
8080
<div className={`permission-column ${classForStatus(row.original.permissions.mentor)}`}>
81-
{permissionIcon(row.original.permissions.mentor)}
81+
{permissionIcon(row.original.permissions.mentor, "Mentor permission")}
8282
</div>
8383
),
8484
},

0 commit comments

Comments
 (0)