[PM-16456] Enhance PreviewOrganizationTaxCommand to support Teams Starter 2023#8000
[PM-16456] Enhance PreviewOrganizationTaxCommand to support Teams Starter 2023#8000JaredScar wants to merge 1 commit into
Conversation
…ogging for subscription discrepancies. Added a logger for better traceability and refined quantity determination logic for password manager plans.
JaredScar
added
ai-review
🤖 Bitwarden Claude Code ReviewOverall Assessment: APPROVE Reviewed the change to Code Review Details
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| if (currentPlan.HasNonSeatBasedPasswordManagerPlan() && !newPlan.HasNonSeatBasedPasswordManagerPlan()) | ||
| { | ||
| // The current plan doesn't have a per-seat subscription item to read a quantity from | ||
| // (e.g. upgrading from a flat-rate plan like Teams Starter), so fall back to the | ||
| // organization's occupied seat count instead of looking it up on the subscription. | ||
| quantity = (long)organization.Seats!; | ||
| } | ||
| else | ||
| { | ||
| var passwordManagerPriceId = currentPlan.HasNonSeatBasedPasswordManagerPlan() | ||
| ? currentPlan.PasswordManager.StripePlanId | ||
| : currentPlan.PasswordManager.StripeSeatPlanId]; | ||
| : currentPlan.PasswordManager.StripeSeatPlanId; | ||
|
|
||
| var quantity = currentPlan.HasNonSeatBasedPasswordManagerPlan() && | ||
| !newPlan.HasNonSeatBasedPasswordManagerPlan() | ||
| ? (long)organization.Seats! | ||
| : passwordManagerSeats.Quantity; | ||
| if (!subscriptionItemsByPriceId.TryGetValue(passwordManagerPriceId, out var passwordManagerSeats)) | ||
| { | ||
| _logger.LogError( | ||
| "Organization {OrganizationId}'s subscription {SubscriptionId} does not contain a " + | ||
| "Password Manager line item matching its current plan's price {PriceId}", | ||
| organization.Id, organization.GatewaySubscriptionId, passwordManagerPriceId); | ||
|
|
||
| return new BadRequest( | ||
| "Your organization's subscription does not match its current plan. Please contact support for assistance."); | ||
| } | ||
|
|
||
| quantity = passwordManagerSeats.Quantity; | ||
| } |
There was a problem hiding this comment.
❓ QUESTION: This bugfix (and the new BadRequest path) has no accompanying unit test.
Details
The existing Subscription Plan Change tests in PreviewOrganizationTaxCommandTests.cs only exercise current plans of Free, Families, Teams, and Enterprise — none of which are non-seat-based (HasNonSeatBasedPasswordManagerPlan()). The Teams Starter 2023 → seat-based upgrade path introduced here, and the new missing-line-item BadRequest return, are both untested.
Per the repo convention (.claude/CLAUDE.md: "ALWAYS add unit tests"), consider adding:
- A regression test upgrading from a non-seat-based plan (e.g.
PlanType.TeamsStarter2023) that assertsquantity == organization.Seats. - A test where the current plan's price is absent from
subscription.Items, asserting theBadRequestresult.
This locks in the fix and prevents regression of the original crash.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8000 +/- ##
==========================================
- Coverage 62.23% 62.23% -0.01%
==========================================
Files 2287 2287
Lines 99663 99683 +20
Branches 9004 9004
==========================================
+ Hits 62023 62035 +12
- Misses 35469 35477 +8
Partials 2171 2171 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
1 participant
🎟️ Tracking
📔 Objective
Fixes issue where showing the upgrade plan modal to a Teams Starter 2023 plan errors out...
📸 Screenshots
BEFORE
NOW
