Repository navigation
Bulk-deleting remote assignments reports success when the server could not delete them #1685
Description
Deleting remote assignments in bulk shows a success notification and clears the selection even when the API could not delete some or all of them. POST /v1/assignments/bulk/delete deletes each assignment separately. For an outstanding one it first calls the gateway, and it catches each failure and reports that id in failedIds, still answering 200. useDeleteBulkAssignmentsMutation parses that result, and its own test says the ids are returned "so the caller can report them". But the hook's onSuccess shows a generic success notification, and DeleteRemoteAssignments.handleDelete ignores the result entirely. When the gateway is unreachable, the user is told the delete succeeded while every outstanding assignment survives, and its link still works for the subject.
Where
apps/web/src/components/DeleteRemoteAssignments/DeleteRemoteAssignments.tsx:98-109:
const handleDelete = () => {
deleteMutation.mutate(
{ ids: [...selectedIds] },
{
onSuccess() {
setSelectedIds(new Set());
setIsConfirmOpen(false);
onBack?.();
}
}
);
};apps/web/src/hooks/useDeleteBulkAssignmentsMutation.ts:16-20:
onSuccess(result) {
addNotification({ type: 'success' });
void queryClient.invalidateQueries({ queryKey: [ASSIGNMENTS_QUERY_KEY_PREFIX] });
return result;
}The server side is apps/api/src/assignments/assignments.service.ts:146-176 (deleteBulk collects failedIds and returns { deletedCount, failedIds }).
Reproduce
- With the gateway stopped (or unreachable from the API), open Group → Remote Assignments → Delete Assignments.
- Select one or more Outstanding assignments and confirm the delete.
Actual: a success notification appears and the selection clears. After the list refetches, the same assignments are still there, and nothing tells the user why.
Expected: when failedIds is non-empty, the user is told how many assignments could not be deleted (an error notification, not a success one), and the failed ones stay selected so they can retry.
Tests
apps/web/src/components/DeleteRemoteAssignments/__tests__/DeleteRemoteAssignments.test.tsx:it('should report the assignments the server could not delete, so a partial failure is not shown as success'), with the mutation resolving to{ deletedCount: 0, failedIds: ['a1'] }.testing/src/specs/remote-assignments-wizard.spec.ts: route**/v1/assignments/bulk/deleteto a200with a non-emptyfailedIds, and assert an error message is shown instead of the success toast.
Suggested fix
Show the success notification only when failedIds is empty, moving that check out of the hook or making it conditional there. In handleDelete, read the resolved failedIds: when there are any, show an error naming the count and keep those ids selected. Clear the selection only for the ids that were deleted.
Activity
joshunrau commentedon Oct 26, 2026
Still reproduces on
main(d86740e). No commit since this was filed touches the hook, the component ordeleteBulk, and no open PR fixes it.The unit tests currently encode the bug, so they change with the fix:
apps/web/src/hooks/__tests__/useDeleteBulkAssignmentsMutation.test.ts:30resolves every test to{ deletedCount: 1, failedIds: ['assignment-2'] }, and:53-57asserts{ type: 'success' }against that result. The success test needs a consistent{ deletedCount: 1, failedIds: [] }forids: ['assignment-1'].DeleteRemoteAssignments.test.tsx:13types the per-callonSuccessas() => void, and the tests at:247-266call it with no argument. OncehandleDeletereads the result, they need to pass aDeleteBulkAssignmentsResult.Two gaps in the suggested fix
failedIdsdoes not cover every assignment that was not deleted.deleteBulkonly loads rows that are stillOUTSTANDINGorEXPIRED(apps/api/src/assignments/assignments.service.ts:147-153). If the subject completes an assignment after the list loaded, or someone cancels or deletes it, its id is in neitherdeletedCountnorfailedIds, and the fix as written still shows success. Treat the delete as a success only whendeletedCount === ids.length, and reportids.length - deletedCount. KeepfailedIdsselected. The skipped ids leave the list when it refetches.apps/gateway/src/routers/api.router.ts:165-167). The API's axios client rejects that 404 (gateway.http.tssets novalidateStatus), sodeleteBulkreports the id as failed. The API is left with anOUTSTANDINGrow and no gateway row when the gateway delete succeeds and the Mongo write after it fails (assignments.service.ts:161-163, and cancel at:203-206). The same happens when the gateway's SQLite database is recreated. Such a row then fails every delete and every cancel. Counting the gateway's own 404 as deleted fixes this. That is an API change, and it also reaches the synchronizer (gateway.synchronizer.ts:92,:216), so do it with Bulk assignment delete makes one gateway call and one database delete per assignment, serially, with no cap on the batch #1749. That issue's gateway bulk route "reports the ids it did not find", and those ids should count as deleted, not go intofailedIds.Keep the
200. Mutations default tothrowOnError: true(apps/web/AGENTS.md), so an error status would replace the page with the error boundary, as in #1671.Plan
useDeleteBulkAssignmentsMutation.ts:onSuccess(result, { ids })always invalidates. It adds{ type: 'success' }only whendeletedCount === ids.length. Otherwise it adds{ type: 'error', message: t({ en, es, fr }) }with the count.DeleteRemoteAssignments.tsxhandleDelete: always close the dialog. On full success, clear the selection and callonBackas today. Otherwise, set the selection tonew Set(result.failedIds)and skiponBack, so Delete Selected (N) becomes the retry.Tests
it('should announce an error with the count when the server could not delete every assignment').it('should keep only the failed assignments selected and stay on the page, so Delete Selected retries them').testing/exercisesbulk/delete. The header ofremote-assignments-wizard.spec.tssays no assignment is created, so add a newtesting/src/specs/remote-assignments-delete.spec.ts. Seed an assignment in a dedicated group withapi.createAssignment(testing/src/support/api-client.ts:54), open/group/remote-assignments?mode=delete, and route**/v1/assignments/bulk/deleteto{ deletedCount: 0, failedIds: [id] }. Assert that the error shows and thatdelete-select-assignment-<id>stays checked. Cover gap 1 against the real server: tick the assignment, cancel it withPATCH /v1/assignments/:id{ status: 'CANCELED' }(api-client.tsneeds a helper for this), confirm, and assert the error. Put the delete-mode locators inRemoteAssignmentsPage, because Bulk assignment delete makes one gateway call and one database delete per assignment, serially, with no cap on the batch #1749's planned e2e needs them too.Landing
#1679, #1682 and #1757 also change
DeleteRemoteAssignments.tsx, so land them together or in sequence. #1811 dropsrootStylefrom the sameDataTableand adds members toremote-assignments.page.ts. Expect textual conflicts there, not behavioural ones. #1749 keeps the{ deletedCount, failedIds }shape, so this can land first.Triage note written with Claude Code. Code references point at
main@d86740e.