Skip to content

feat: bulk unenroll UI for support tools (LP-860) - #18

Open
djoseph-apphelix wants to merge 1 commit into
masterfrom
djoseph/LP-860
Open

feat: bulk unenroll UI for support tools (LP-860)#18
djoseph-apphelix wants to merge 1 commit into
masterfrom
djoseph/LP-860

Conversation

@djoseph-apphelix

@djoseph-apphelix djoseph-apphelix commented Aug 5, 2026

Copy link
Copy Markdown
Member

Summary

Adds the support-tools UI for deactivating every enrollment in a set of courses. An operator uploads a single-column CSV of course ids, reviews a dry-run preview, supplies a reason, confirms, then watches progress to completion.

Jira ticket

LP-860

Requires the bulk_unenroll support API in edx-platform (edx/edx-platform#413). This PR is frontend only.

Backend PR

PR-413

Pages

Two routes, both global-staff only (the API enforces it), reachable from the Bulk Unenroll link in the header:

Route Purpose
/course_bulk_unenroll Upload → preview → confirm, then the progress view
/course_bulk_unenroll/batches Run history across all operators

/course_bulk_unenroll?batch_id=<id> opens straight into the progress view. The batch id lives in the URL so a run survives a reload and can be handed to a colleague — a batch can take hours, and the tab is not the only handle on it.

Flow

Upload rejects oversized files and reports row-numbered errors against the spreadsheet the operator is looking at. Preview lists each course with its active-enrollment count and flags ids the LMS does not recognise, filterable by valid / not-found. Nothing is mutated until Confirm, which requires a reason and states the course and learner counts in the modal.

Progress polls every 5s and stops once the batch is terminal — a finished batch will never change again, and a tab left open overnight must not keep hitting the API. Per-course rows are server-paginated and filterable by state. Cancel and Retry appear only when the batch state allows them; retry re-runs just the failed courses.

The batch list polls at 15s: it is background awareness, not progress being watched, and unlike a single batch it has no terminal state to stop on.

Stale-response handling

Both polling hooks issue overlapping requests by design — a poll tick, a filter change and a page change can be in flight together — and nothing guarantees replies arrive in order. Two guards in data/hooks.js:

  • Request sequence. Each fetch takes a ticket; a reply whose ticket is no longer current is dropped rather than written to state. It is still returned, so the poll loop schedules exactly as before.
  • Reset on batch id change, during render, and it retires the in-flight request at the same moment. Otherwise a reply for the previous batch could restore it under the new batch's URL — which matters because cancel and retry act on the URL's id, not on what is displayed.

data/hooks.test.jsx covers these with deferred promises resolved in reverse order, the case mockResolvedValue cannot produce since it always settles in call order.

Demo video

Screen.Recording.2026-08-05.at.2.59.03.PM.mov

Tests

npm test -- src/CourseBulkUnenroll74 passed, 5 suites. ESLint clean.

Covers upload and validation errors, the preview filters, confirm/cancel/retry gating per state, server-side pagination, polling start/stop, and the out-of-order response cases above.

Copilot AI lite review requested due to automatic review settings August 5, 2026 08:38

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new Course Bulk Unenroll feature area to the support tools frontend, providing a CSV-driven workflow (upload → preview → confirm → progress) plus a separate batch history page, wired to the new bulk_unenroll LMS support API.

Changes:

  • Introduces the CourseBulkUnenroll UI (upload/preview/confirm/progress) and a batch history page with polling.
  • Adds a dedicated API client + polling hooks with stale/out-of-order response guards and comprehensive test coverage.
  • Integrates the feature into global routing, header navigation, and global SCSS imports.

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/supportHeader/Header.jsx Adds “Bulk Unenroll” link in the support header.
src/index.scss Imports the new feature’s SCSS globally.
src/index.jsx Registers new routes for bulk unenroll and batch history pages.
src/data/constants/routes.js Adds route constants for bulk unenroll and its batches sub-route.
src/CourseBulkUnenroll/utils.js Shared helpers for state labels and timestamp formatting.
src/CourseBulkUnenroll/UploadPanel.jsx CSV file picker + upload submit UI for step 1.
src/CourseBulkUnenroll/TableActions.jsx Shared table control bar (filters + row count).
src/CourseBulkUnenroll/StateFilterDropdown.jsx Shared status dropdown used across tables.
src/CourseBulkUnenroll/ProgressPanel.jsx Progress view with polling indicators, actions, and server-paginated course table.
src/CourseBulkUnenroll/PreviewPanel.jsx Preview table with search + found/not-found filtering and rejected-row display.
src/CourseBulkUnenroll/messages.js i18n message descriptors for all new UI copy.
src/CourseBulkUnenroll/index.scss Feature styling aligned with CourseTeamManagement conventions.
src/CourseBulkUnenroll/data/hooks.test.jsx Unit tests for stale/out-of-order response handling in polling hooks.
src/CourseBulkUnenroll/data/hooks.js Polling hooks for batch status and batch list, including stale response guards.
src/CourseBulkUnenroll/data/api.test.js API client tests for upload/confirm/status/list/cancel/retry behaviors.
src/CourseBulkUnenroll/data/api.js API client for bulk unenroll endpoints with error normalization.
src/CourseBulkUnenroll/CourseBulkUnenrollPolling.test.jsx Fake-timer polling lifecycle tests (start/stop behavior).
src/CourseBulkUnenroll/CourseBulkUnenrollIndexPage.test.jsx End-to-end flow tests for upload/preview/confirm/progress and lookup.
src/CourseBulkUnenroll/CourseBulkUnenrollIndexPage.jsx Main page orchestrating the flow and URL-driven batch viewing.
src/CourseBulkUnenroll/CourseBulkUnenrollBatchList.test.jsx Navigation + batch history list behavior tests.
src/CourseBulkUnenroll/CourseBulkUnenrollBatchesPage.jsx Dedicated batch history route that links back/open-batch handoff.
src/CourseBulkUnenroll/constants.js Shared constants (states, variants, polling intervals, limits, etc.).
src/CourseBulkUnenroll/ConfirmModal.jsx Confirmation modal for irreversible action with StatefulButton states.
src/CourseBulkUnenroll/BatchMetadata.jsx Batch details block (reason/requester/file/timestamps/id).
src/CourseBulkUnenroll/BatchListPanel.jsx Batch history table with polling, filtering, and navigation into a batch.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/CourseBulkUnenroll/data/hooks.js
Comment thread src/CourseBulkUnenroll/UploadPanel.jsx
Comment thread src/CourseBulkUnenroll/UploadPanel.jsx Outdated
Comment thread src/index.jsx Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/CourseBulkUnenroll/data/api.js:45

  • toError maps every HTTP 404 to batchNotFoundError, which can produce a misleading “No batch found with that ID.” message for collection endpoints (upload/list) when the route is missing or misconfigured. Consider treating 404 as “batch not found” only for the batch-status endpoint, and otherwise falling back to the caller-provided message.
  if (status === 404) {
    return toAlert(intl, messages.batchNotFoundError);
  }

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/CourseBulkUnenroll/CourseBulkUnenrollBatchesPage.jsx:32

  • openBatch builds the query string by interpolating the batch id directly. If the id ever contains reserved URL characters (e.g., operator copy/paste with whitespace, &, ?), navigation can break or inject unintended query params. Encode the value before putting it in the URL.
  const openBatch = (batchId) => navigate(
    `${SUPPORT_TOOLS_TABS.SUB_DIRECTORY.COURSE_BULK_UNENROLL}?${BATCH_ID_PARAM}=${batchId}`,
  );

Copilot AI review requested due to automatic review settings August 10, 2026 08:15

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/CourseBulkUnenroll/data/hooks.js:123

  • Same unmount hazard as the batch list hook: if a status request is in-flight when the operator navigates away, it can resolve and call setBatch/setError/setIsLoading after unmount. Incrementing requestSeq in an unmount cleanup ensures late replies are treated as superseded and won’t touch state.
  const requestSeq = useRef(0);

src/CourseBulkUnenroll/data/hooks.js:40

  • fetchOnce can still call setIsLoading/setError/setBatches after the component unmounts (the polling effect’s cancelled flag only stops scheduling the next tick; it doesn’t prevent the in-flight request from resolving and updating state). This can produce React “setState on unmounted component” warnings when navigating away from the page. Consider retiring the request sequence on unmount (similar to the isMounted guard used in src/users/CourseReset.jsx:35-75).

This issue also appears on line 123 of the same file.

  const requestSeq = useRef(0);

src/CourseBulkUnenroll/data/api.js:57

  • The 400-handling path treats any validation error containing the substring "row" as a row-cap rejection, which can mislabel other upload validation errors (e.g., “invalid row 3”, “bad row format”) as “too many rows”. Narrowing the detection to the specific “too many rows” signal avoids hiding real server messages.
    const detail = JSON.stringify(data ?? '');
    if (/row/i.test(detail)) {
      return toAlert(intl, messages.tooManyRowsError, { maxRows: MAX_ROWS });
    }

@abhalsod-sonata

Copy link
Copy Markdown
Member

Notes: This PR is quite large (4,000+ lines changed), which makes it difficult to review thoroughly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants