[PM-39237] fix: Handle mismatched and orphaned migration cohort assignments in admin dropdown#7836
[PM-39237] fix: Handle mismatched and orphaned migration cohort assignments in admin dropdown#7836cyprain-okeke wants to merge 5 commits into
Conversation
…nments in admin dropdown The cohort dropdown only listed cohorts whose migration path matched the organization's current plan. When an assignment pointed to a cohort outside that filtered set, the selected value silently dropped, making the dropdown appear unassigned or showing the wrong selection. Now the assigned cohort is always included and labeled "(plan mismatch)" when its path no longer matches the plan, or "(assigned cohort no longer exists)" when the cohort is missing.
🤖 Bitwarden Claude Code ReviewOverall Assessment: APPROVE Reviewed the admin migration-cohort dropdown fix (PM-39237). The change ensures a currently-assigned cohort that falls outside the plan-filtered set is still rendered and labeled, covering both plan-mismatch and orphaned-cohort states. Verified the GET fallback is correctly mirrored by the POST resolver's Code Review DetailsNo blocking findings. Correctness verified:
Test coverage is thorough: regression tests assert no-delete on resubmit for both mismatch and orphan, correct delete-then-create ordering on a valid switch, lock-guard precedence, and the compatibility guard rejecting a mismatch-to-mismatch switch. |
…ps://140.82.121.4/bitwarden/server into billing/pm-39237/fix-mismatched-cohort-dropdown
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #7836 +/- ##
=======================================
Coverage 61.23% 61.23%
=======================================
Files 2209 2209
Lines 97743 97779 +36
Branches 8815 8823 +8
=======================================
+ Hits 59849 59873 +24
- Misses 35770 35781 +11
- Partials 2124 2125 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| if (currentAssignment?.CohortId is { } assignedId | ||
| && visibleCohorts.All(c => c.Id != assignedId)) | ||
| { | ||
| var assignedCohort = await _organizationPlanMigrationCohortRepository.GetByIdAsync(assignedId); |
There was a problem hiding this comment.
Do we need to go to the database for this, or can we use the migrationCohorts list above? I assume the list would have the one we're looking for
sven-bitwarden
left a comment
sven-bitwarden
left a comment
There was a problem hiding this comment.
Approving with one comment that might be a minor optimization, but could also be unnecessary.
2 participants
🎟️ Tracking
PM-39237
📔 Objective
The admin organization edit page builds the migration cohort dropdown by listing
only cohorts whose migration path matches the organization's current plan. When
an organization's existing assignment points to a cohort outside that filtered
set, the selected value was silently dropped — the dropdown rendered as
"(Not assigned)" or with the wrong selection, hiding the real assignment from
admins.
This PR ensures the currently-assigned cohort is always present in the dropdown
and clearly labeled:
the organization's plan (including a null/retired path id during a multi-stage
rollout), the option is appended and labeled
(plan mismatch). The POSTresolver already mirrors this guard, blocking a switch to a null-path cohort
while letting the current value round-trip unchanged.
repository, the dropdown shows
(assigned cohort no longer exists)and awarning is logged (org id and cohort id only, no sensitive data).
Covered by 52 unit tests in
OrganizationsControllerTests(all passing).📸 Screenshots