Skip to content

feat(invitations): complete external ID bulk invite flow and redesign result dialog#8406

Open
LWS49 wants to merge 4 commits into
masterfrom
lws49/feat-ext-id-invite-flow
Open

feat(invitations): complete external ID bulk invite flow and redesign result dialog#8406
LWS49 wants to merge 4 commits into
masterfrom
lws49/feat-ext-id-invite-flow

Conversation

@LWS49
Copy link
Copy Markdown
Collaborator

@LWS49 LWS49 commented May 26, 2026

Part 1: External ID in the invite flow

Summary

Completes the external ID bulk invite flow started in the previous PR. The CSV upload now acts as a full upsert for external IDs on existing records: a nil ext_id is filled in, a non-nil ext_id that differs from the CSV value is overwritten (not rejected), and a blank CSV cell preserves the existing value. The external_id_conflict error condition is removed - the only remaining hard rejection for ext_id is external_id_taken (the CSV value is already held by a different course member). Updated rows fold into the Existing Invitations and Existing Course Users sections with a bold ext_id badge and old-value tooltip; the separate "External IDs Updated" section is removed. Error messages in the controller are now per-error-type rather than a single sentence, enabling the result dialog to attribute each failure reason precisely. The external_id field is carried through to CourseUser on invitation acceptance. The score summary CSV download gains an opt-in ext_id column, shown only when at least one student has a non-blank ext_id.

Design decisions

  • Incoming available non-null external ID differing from user's current non-null ID treated as upsert, not error - CSV uploads are submitted exclusively by course staff (trusted actors). The real safety guard is external_id_taken (ID collision across different people), which is still a hard error. Flagging a staff member correcting a previous ext_id entry as a conflict imposes workflow cost with no safety gain. Nil-to-value and non-nil-to-different-value are now treated symmetrically.
  • Updated rows merged into existing sections - folding updated rows into Existing Invitations/Course Users with a bold ext_id badge and "Previously: X" tooltip is sufficient signal. A dedicated "External IDs Updated" section created a confusing fourth category; updated users are conceptually still existing users.
  • Failed section at top - rows in the Failed section require the admin to take action before re-uploading. Placing them below success sections risks the admin closing the dialog without noticing.
  • Save order: updated records before new records - within the transaction, updated invitations and course users are saved before new ones. This is required for freed-ID batch claims: if Alice changes from freed-id to new-id and Bob claims freed-id in the same batch, Alice's update must persist first so Bob's insert doesn't hit the unique constraint.
  • external_id coerced via .presence when building CourseUser from invitation - the partial unique index on course_users(course_id, external_id) only covers non-NULL values (WHERE external_id IS NOT NULL). Storing an empty string "" would subject it to the uniqueness constraint, causing duplicate-key errors when multiple invitees have no external ID set. .presence converts ""nil, keeping those rows outside the partial index scope.
  • 6-column CSV uploaded to a non-timeline course is rejected with an error toast - when show_personalized_timeline_features? is false, the parser reads row[4] as external_id. A 6-column file (the timeline template) would silently store the timeline string (e.g. "otot") as the external ID. Column count detection (row.length > 5) is used as the signal: value-matching against known algorithm names was rejected because those strings are valid external IDs. The upload is aborted before any row is processed and a descriptive toast tells staff to use the 5-column template.

Service behavior: master vs this PR

Ext_id is checked at four points in the service pipeline. @taken_external_ids is pre-loaded from both CourseUser and unconfirmed Course::UserInvitation ext_ids, so a conflict with either type anchors the same rejection.

Processing step Master This PR
New user (no account) - new invitation No ext_id check - invitation always created Checks @taken_external_ids; conflict - external_id_taken failure
Existing user (not yet in course) - new enrollment No ext_id check - always enrolled Same conflict check via enroll_new_user
Re-invite: existing pending invitation ext_id field absent Updates ext_id if free; conflict - external_id_taken failure
Re-invite: existing course member ext_id field absent Updates ext_id if free; conflict - external_id_taken failure
CSV email matches user with unconfirmed account Treated as new user - invitation sent Unchanged - unconfirmed account holders receive an invitation they can accept after confirming their email, not direct enrollment

Cross-type freed-ID ordering limitation

When an existing course user frees an ext_id and a brand-new user (no Coursemology account) claims it in the same batch, the new user is still rejected with external_id_taken. invite_new_users runs before add_existing_users in the pipeline, so the new invitation is evaluated before the existing course user releases the ID. The same-type freed-ID case (both are existing invitations, or both are course users) works correctly. The spec documents this behavior explicitly rather than asserting it should work - a two-pass fix would add complexity for a narrow edge case.

Regression prevention

Service specs cover: nil-to-value upsert for invitations and course users; non-nil-to-different-free-value upsert with previous_external_id capture; external_id_taken on new invitations, new enrollments (existing user not yet in course), existing invitation updates, and existing course user updates; conflict anchored by course user ext_id vs invitation ext_id; freed-ID batch claim within the same record type; cross-type freed-ID ordering limitation; unconfirmed-email path sends invitation not direct enrollment; isRetryable: false propagated through jbuilder; 6-column CSV uploaded to a non-timeline course raises CSV::MalformedCSVError (guards the silent-corruption path where timeline strings become external IDs).

Model specs cover: blank external_id on invitation ("") produces nil on the built CourseUser — guards the .presence call in User#build_course_user_from_invitation against silent removal (empty string would otherwise pass the partial unique index's IS NOT NULL guard and cause duplicate-key errors).

Frontend specs cover: InvitationResultExistingTable updated-row rendering (bold + badge + tooltip); InvitationResultDialog - failed invitations in Failed section not Existing Invitations, failedRowsSubtitle shown only for failed_to_send rows with correct count, subtitle absent when only CSV-issue duplicates present, mixed-failure count correctly split between header and caption; InvitationResultFailedTable reason text for all four failure reasons.

Manual testing: happy path new batch; nil-to-value upsert with badge; non-nil-to-different upsert with tooltip; external_id_taken in Failed section; freed-ID same-type batch; existing user no-change; failed-to-send rows in red with caption; CSV-issue rows without caption; mixed failure section showing correct split counts.

Backward compatibility: existing invite flows unaffected. The only behavioral change for existing data is that re-inviting a user whose ext_id differs from the CSV value now updates the record instead of rejecting it.


Part 2: Independent improvements

Summary

Five fixes made to the invite flow independent of the external ID feature.

First, invitations with is_retryable: false (permanent email delivery failures) were previously indistinguishable from normal pending invitations in the Existing Invitations section; they are now routed to a dedicated Failed section, highlighted in red, with a caption identifying exactly which rows could not be sent.

Second, UserInvitationsTable had two related fixes: it was importing getManageCourseUserPermissions from course/users/selectors (reads state.users) instead of the invitations store (state.invitations), causing the Personalized Timeline column to flicker on SPA navigation; and the column gate was changed from canManagePersonalTimes to showPersonalizedTimelineFeatures for correctness (see design decisions below).

Third, a conditional "Personalized Timeline" column is added to all three invitation result tables (new invitations/enrollments, existing, and failures). The column appears only when showPersonalizedTimelineFeatures is enabled. timeline_algorithm is now serialized on all seven record sections in the result data; the flag is added to the invitation index response, stored in Redux alongside defaultTimelineAlgorithm (and added to the initial state of all three bundles that share ManageCourseUsersSharedData: invitations, users, and enrol-requests), read by InvitationResultDialog, and threaded to each table component.

Fourth, build_course_user in the invitation service was ignoring the CSV timeline_algorithm column for directly-enrolled existing users and always assigning the course default instead. The fix passes user[:timeline_algorithm] through, consistent with build_invitation and build_course_user_from_invitation. build_course_user_from_invitation in User also had .presence added to external_id for consistency with user_registration_service.

Fifth, stale naming is cleaned up throughout the course bundle (service, controller, jbuilder, types, components, specs; instance bundle unchanged): InvitationResultUsersTableInvitationResultFailedTable, InvitationResultSkippedTableInvitationResultExistingTable (with its exported row type SkippedRowExistingRow), DuplicateUserDataFailedInvitationRowData, and duplicate_users / duplicateUsersfailed_users / failedUsers.

Design decisions

  • is_retryable: false invitations excluded from the "already invited" summary count - permanently failed invitations are not in the course and will never arrive; counting them as "already invited" misleads the admin into thinking those users are handled.
  • Timeline column gate: canManagePersonalTimes preserved for editable inputs, showPersonalizedTimelineFeatures used for read-only display - these two flags are related but distinct. canManagePersonalTimes is the compound show_personalized_timeline_features? && can?(:manage_personal_times) — it requires both the course feature to be on and the user to hold the manage permission. The invite forms and CSV upload (unchanged from master) correctly gate on canManagePersonalTimes: showing an editable timeline field only makes sense when the user can actually set it. The live invitations table (UserInvitationsTable) and the result tables are read-only display — the column should be visible to anyone who can see the page whenever the course feature is on, regardless of whether they hold the manage permission. UserInvitationsTable previously used canManagePersonalTimes (same as the forms); this PR corrects it to showPersonalizedTimelineFeatures to match the result tables. The result tables are new and use showPersonalizedTimelineFeatures from the start.

Regression prevention

Service specs cover:

  • build_course_user assigns the CSV timeline_algorithm to the enrolled CourseUser, not the course default (regression guard for the ignored-column bug).

Frontend specs cover:

  • InvitationsIndex - Personalized Timeline column appears when showPersonalizedTimelineFeatures is true in the API response (seeded into state.invitations) and is absent when false, with state.users unpopulated in both cases (regression guard for the wrong-store import and the gate change from canManagePersonalTimes to showPersonalizedTimelineFeatures).
  • column shown with correct algorithm label when showPersonalizedTimelineFeatures is true, column absent when false, dash rendered when timelineAlgorithm is undefined - for each of InvitationResultPrimaryTable, InvitationResultExistingTable, and InvitationResultFailedTable. InvitationResultDialog integration tests verify the flag is read from the Redux store and threads to tables (column visible with showPersonalizedTimelineFeatures: true in store, hidden with default store).

Manual testing: is_retryable: false invitations appear in Failed section highlighted in red; Personalized Timeline column visible on both SPA navigation and hard refresh when personalized timelines enabled, absent in both when disabled.

Backward compatibility: invite forms and CSV upload gate unchanged (canManagePersonalTimes). Result dialog unchanged for courses with Personalized Timelines disabled.

Current Result Display

Happy Path

image

All Failure Reasons Shown

image

Timelines Shown In Table If Enabled

image

Error Flagged if CSV with >5 Columns Used with Timelines Off

image

@LWS49 LWS49 changed the base branch from master to lws49/feat-add-ext-id May 26, 2026 07:36
@LWS49 LWS49 changed the title feat(invitations): complete external ID bulk invite flow feat(invitations): complete external ID bulk invite flow and redesign result dialog May 26, 2026
@LWS49 LWS49 force-pushed the lws49/feat-ext-id-invite-flow branch 4 times, most recently from c0dcc05 to e892fd9 Compare May 26, 2026 15:12
@LWS49 LWS49 force-pushed the lws49/feat-add-ext-id branch from d6e1808 to b75a804 Compare May 28, 2026 00:47
@LWS49 LWS49 force-pushed the lws49/feat-ext-id-invite-flow branch 7 times, most recently from a037a06 to 023720d Compare May 28, 2026 09:41
@LWS49 LWS49 force-pushed the lws49/feat-add-ext-id branch from b75a804 to 4347219 Compare May 29, 2026 05:32
…itations

  Allows institutions to link Coursemology enrolments to their own student
  records or LMS identifiers. The field is stored on CourseUser and
  Course::UserInvitation and validated unique per course across both tables
  via a cross-table concern and partial DB index - a pending invitation holds
  its ext_id until confirmed, preventing collisions with direct enrolments.

  Surfaces:
  - Individual invite form: ext_id input field
  - Bulk CSV invite: ext_id column in both template variants (with and without
    Timeline column); set on new records only - existing pending invitations
    and enrolled users are read-only in this flow
  - Manage users table: inline editable ext_id column (always visible)
  - Score summary export: ext_id column included when any student has one
  - Statistics > Students table: ext_id column, sortable and searchable,
    shown only when at least one student has a non-null ext_id

  View-only tables suppress the ext_id column when no course members have
  one set, consistent with how group manager, gamification, and video columns
  are conditionally shown. Edit tables always show it.

  Also changes error responses from the invitations controller from a single
  concatenated string to an array of per-record strings, enabling the frontend
  to render overflow counts without truncating meaningful error detail.
@LWS49 LWS49 force-pushed the lws49/feat-add-ext-id branch from 9c8e986 to 007edda Compare May 29, 2026 06:45
@LWS49 LWS49 force-pushed the lws49/feat-ext-id-invite-flow branch 3 times, most recently from 0ef7442 to 62635b4 Compare May 29, 2026 08:47
@LWS49 LWS49 marked this pull request as ready for review May 29, 2026 08:56
@LWS49 LWS49 force-pushed the lws49/feat-ext-id-invite-flow branch from 62635b4 to ba302dd Compare June 1, 2026 10:17
@LWS49 LWS49 force-pushed the lws49/feat-add-ext-id branch 2 times, most recently from cbe8d6f to 6226857 Compare June 2, 2026 00:59
@LWS49 LWS49 force-pushed the lws49/feat-add-ext-id branch from 6226857 to 9c27231 Compare June 2, 2026 00:59
@LWS49 LWS49 force-pushed the lws49/feat-ext-id-invite-flow branch from ba302dd to 3442e03 Compare June 2, 2026 04:48
…itations

  Allows institutions to link Coursemology enrolments to their own student
  records or LMS identifiers. The field is stored on CourseUser and
  Course::UserInvitation and validated unique per course across both tables
  via a cross-table concern and partial DB index - a pending invitation holds
  its ext_id until confirmed, preventing collisions with direct enrolments.

  Surfaces:
  - Individual invite form: ext_id input field
  - Bulk CSV invite: ext_id column in both template variants (with and without
    Timeline column); set on new records only - existing pending invitations
    and enrolled users are read-only in this flow
  - Manage users table: inline editable ext_id column (always visible)
  - Score summary export: ext_id column included when any student has one
  - Statistics > Students table: ext_id column, sortable and searchable,
    shown only when at least one student has a non-null ext_id

  View-only tables suppress the ext_id column when no course members have
  one set, consistent with how group manager, gamification, and video columns
  are conditionally shown. Edit tables always show it.

  Also changes error responses from the invitations controller from a single
  concatenated string to an array of per-record strings, enabling the frontend
  to render overflow counts without truncating meaningful error detail.
@LWS49 LWS49 force-pushed the lws49/feat-ext-id-invite-flow branch 2 times, most recently from 54e9ffd to 29ab1cf Compare June 2, 2026 04:54
@LWS49 LWS49 force-pushed the lws49/feat-ext-id-invite-flow branch 6 times, most recently from 952cf4a to 06af4fa Compare June 2, 2026 09:10
… dialog

- add ext_id to CourseUser and UserInvitation with unique-per-course index
- accept ext_id in bulk CSV and individual invite form; upsert for existing
  records (enrolled users and pending invitations)
- conflicts (duplicate email or ext_id) surface in Failed with reasons
- redesign result dialog: replace updated chip with row tint and tooltip
@LWS49 LWS49 force-pushed the lws49/feat-ext-id-invite-flow branch from 06af4fa to ae12c5e Compare June 2, 2026 09:28
@LWS49 LWS49 force-pushed the lws49/feat-add-ext-id branch 5 times, most recently from 3498902 to 9c390c4 Compare June 2, 2026 16:48
Base automatically changed from lws49/feat-add-ext-id to master June 2, 2026 17:28
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.

1 participant