[PM-36951] Add Billing Plan Migration Cohorts Admin UI#7732
Open
kdenney wants to merge 62 commits into
Open
Conversation
Contributor
Bitwarden Claude Code ReviewOverall Assessment: APPROVE Reviewed the Cohorts CRUD admin UI for the business-plan price migration loop, including the Razor views, controller, dual-ORM repository implementations (Dapper + EF Core), stored procedures with corresponding migration scripts, value objects, and the supporting unit/integration tests. The change is feature-flag gated ( Code Review DetailsNo findings. Notable strengths observed (for context only — not actionable):
Open reviewer threads from amorask-bitwarden are either acknowledged (the Stripe round-trip thread is resolved by author decision) or appear to be addressed by the recent server-rendered |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #7732 +/- ##
==========================================
+ Coverage 60.69% 65.14% +4.45%
==========================================
Files 2154 2166 +12
Lines 95523 95955 +432
Branches 8560 8633 +73
==========================================
+ Hits 57978 62511 +4533
+ Misses 35498 31289 -4209
- Partials 2047 2155 +108 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…-per-org-cohort-assignment-to-the-admin-portal
kdenney
changed the title
amorask-bitwarden
previously approved these changes
May 28, 2026
Contributor
amorask-bitwarden
left a comment
amorask-bitwarden
left a comment
There was a problem hiding this comment.
Killer work here - this looks great. Just a couple ⛏️ / 💡 if you want to address them, but looks good to me.
…ling Adds id="proactive-coupon-row" to the wrapping div in the shared cohort form partial. Scripts in Create.cshtml and Edit.cshtml will toggle the row's hidden property based on whether the cohort is churn-only.
…Create Replaces the cohort Create script with a single applyVisibility helper that toggles both the proactive coupon row and the churn discount (optional) label whenever the migration path is "None (Churn-only)". Runs on load to cover validation-error redisplay where the user previously picked churn-only.
…Edit Replaces the cohort Edit script with a single applyVisibility helper that toggles both the proactive coupon row and the churn discount (optional) label whenever the migration path is "None (Churn-only)". Falls back to a Razor-rendered constant when the migration-path select is absent (locked cohorts), so locked churn-only cohorts no longer render (optional) on the required churn discount field.
Exposes a typed answer to "is this form representing a churn-only cohort?" alongside the existing GetMigrationPathId() method. Consumers no longer need to re-compare MigrationPathSelection against the "none" sentinel themselves.
…fields Conditionally renders the hidden attribute on the proactive coupon row and the churn discount (optional) label based on Model.IsChurnOnly, so the browser never paints them visible before the page-load script can hide them. Eliminates the FOUC observed on locked churn-only Edit and on Create after churn-only validation redisplay.
The server now renders the hidden attribute when MigrationPathSelection is "none" (validation-error redisplay), so the page-load script no longer needs to re-set visibility on load. Keeps only the change listener for interactive toggling.
… Edit The server now renders the hidden attribute on initial load for every Edit scenario (locked, unlocked, churn-only, migration), so the page-load script no longer needs the Razor-rendered MigrationPathSelection constant or the initial applyVisibility call. Keeps only the change listener for interactive toggling on the unlocked path.
# Conflicts: # src/Core/Billing/Extensions/ServiceCollectionExtensions.cs
|
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| MigrationPathSelection = cohort.MigrationPathId switch | ||
| { | ||
| null => NoMigrationPath, | ||
| var id => ((byte)id).ToString(), |
| .GetCouponAsync(Arg.Any<string>(), Arg.Any<CouponGetOptions?>()) | ||
| .ThrowsAsync(new StripeException { StripeError = new StripeError { Code = "resource_missing" } }); | ||
|
|
||
| var result = await sutProvider.Sut.Create(model); |
Comment on lines
+96
to
+101
| catch (Exception ex) | ||
| { | ||
| logger.LogError(ex, "Error creating cohort. Name: {Name}", model.Name); | ||
| ModelState.AddModelError(string.Empty, "An error occurred while saving the cohort."); | ||
| return View(model); | ||
| } |
Comment on lines
+170
to
+175
| catch (Exception ex) | ||
| { | ||
| logger.LogError(ex, "Error updating cohort. Id: {Id}", id); | ||
| ModelState.AddModelError(string.Empty, "An error occurred while saving the cohort."); | ||
| return View(EditCohortViewModel.From(cohort, model, assignmentState)); | ||
| } |
Comment on lines
+205
to
+210
| catch (Exception ex) | ||
| { | ||
| logger.LogError(ex, "Error deleting cohort. Id: {Id}", id); | ||
| TempData["Error"] = "An error occurred while attempting to delete the cohort."; | ||
| return RedirectToAction(nameof(Edit), new { id }); | ||
| } |
| .CreateAsync(Arg.Is<OrganizationPlanMigrationCohort>(c => | ||
| c.Name == model.Name | ||
| && c.MigrationPathId == MigrationPathId.Enterprise2020AnnualToCurrent | ||
| && c.IsActive == false)); |
amorask-bitwarden
approved these changes
May 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-36951
📔 Objective
Adds the operator-facing Cohorts CRUD UI under the Tools menu of the Bitwarden Portal (
src/Admin), gated by thePM35215_BusinessPlanPriceMigrationfeature flag. This is the workflow operators use to define, search, edit, and disable the Migration and Churn-only cohorts that drive the business-plan price migration loop introduced in PM-35215.What this adds:
EditCohortViewModel, with a shared_CohortFormFieldsRazor partial driving Name and coupon inputs across Create and Edit. Active state lives on the form and is persisted through the same POST.GetCohortAssignmentStateQuery.OrganizationPlanMigrationCohort_SearchWithCounts,OrganizationPlanMigrationCohort_ReadByName,OrganizationPlanMigrationCohortAssignment_CountNonPendingByCohortId.Where this deviates from the mockups and why
📸 Screenshots