Skip to content

[PM-16456] Enhance PreviewOrganizationTaxCommand to support Teams Starter 2023 - #8000

Merged
JaredScar merged 4 commits into
mainfrom
ac/pm-16456-add-occupied-seat-count-chek-when-restoring-user
Jul 20, 2026
Merged

[PM-16456] Enhance PreviewOrganizationTaxCommand to support Teams Starter 2023#8000
JaredScar merged 4 commits into
mainfrom
ac/pm-16456-add-occupied-seat-count-chek-when-restoring-user

Conversation

@JaredScar

Copy link
Copy Markdown
Contributor

🎟️ Tracking

📔 Objective

Fixes issue where showing the upgrade plan modal to a Teams Starter 2023 plan errors out...

📸 Screenshots

BEFORE
image

NOW
image

…ogging for subscription discrepancies. Added a logger for better traceability and refined quantity determination logic for password manager plans.
@JaredScar
JaredScar requested a review from a team as a code owner July 16, 2026 20:53
@JaredScar
JaredScar requested a review from connerbw July 16, 2026 20:53
@JaredScar JaredScar added ai-review Request a Claude code review t:bugfix Change Type - Bugfix labels Jul 16, 2026
@github-actions

github-actions Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the bugfix to PreviewOrganizationTaxCommand that handles upgrading from a flat-rate, non-seat-based plan (e.g. Teams Starter 2023) to a seat-based plan. The change replaces an unconditional dictionary indexer — which threw KeyNotFoundException when no matching Password Manager line item existed — with a TryGetValue lookup that falls back to the organization's occupied seat count or returns a graceful BadRequest on subscription/plan mismatch. Both branches are covered by new regression tests.

Code Review Details

No blocking findings.

  • The new _logger field re-captures the primary constructor parameter, which is idiomatic here since BaseBillingCommand<T> does not expose its logger.
  • (long)organization.Seats! is unchanged pre-existing behavior, not introduced by this PR.
  • The TryGetValue + BadRequest fallback correctly resolves the KeyNotFoundException and logs the discrepancy without exposing sensitive data.
  • The prior review's concern about missing test coverage is fully addressed — both the Teams Starter fallback and the missing-line-item BadRequest paths now have regression tests.

@codecov

codecov Bot commented Jul 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.36%. Comparing base (eeb1c26) to head (7b340b0).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #8000   +/-   ##
=======================================
  Coverage   62.36%   62.36%           
=======================================
  Files        2286     2286           
  Lines       99673    99693   +20     
  Branches     9003     9003           
=======================================
+ Hits        62157    62177   +20     
  Misses      35344    35344           
  Partials     2172     2172           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

connerbw
connerbw previously approved these changes Jul 17, 2026
…tionTaxCommandTests

- Implemented tests to ensure correct handling of plan upgrades from non-seat-based to seat-based plans, utilizing the organization's seat count.
- Added a test to verify that a missing price line item for the current plan results in a BadRequest instead of an exception.
- Enhanced error handling for subscription discrepancies during plan changes.
@JaredScar
JaredScar merged commit a43d029 into main Jul 20, 2026
40 of 41 checks passed
@JaredScar
JaredScar deleted the ac/pm-16456-add-occupied-seat-count-chek-when-restoring-user branch July 20, 2026 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Request a Claude code review t:bugfix Change Type - Bugfix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants