Skip to content

[PM-30101] add multiple coupon support to server preview/purchase#7229

Open
kdenney wants to merge 3 commits intomainfrom
billing/PM-30101/purchase-allows-multiple-discounts
Open

[PM-30101] add multiple coupon support to server preview/purchase#7229
kdenney wants to merge 3 commits intomainfrom
billing/PM-30101/purchase-allows-multiple-discounts

Conversation

@kdenney
Copy link
Contributor

@kdenney kdenney commented Mar 16, 2026

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-30101

📔 Objective

Added support for multiple coupons for the preview invoice and purchase flows for premium and family (orgs).

@kdenney kdenney added the ai-review Request a Claude code review label Mar 16, 2026
@claude
Copy link
Contributor

claude bot commented Mar 16, 2026

Claude finished @kdenney's task in 2m 59s —— View job


Code Review: PM-30101 Multiple Coupon Support

  • Gather context and read changed files
  • Analyze code changes
  • Post review findings

Overall Assessment: Approve

Reviewed the full diff across all 22 changed files. The PR cleanly refactors the billing discount system from single-coupon (string? Coupon) to multi-coupon (string[]? Coupons) support across preview and purchase flows for both premium and organization subscriptions.

Key observations:

  • API request models consistently updated across all four entry points
  • Core services (SubscriptionDiscountService, OrganizationBillingService, premium commands) properly handle the array with null/empty guards
  • Preview flows use graceful fallback (log warning + continue without discounts) while purchase flows use fail-fast validation — appropriate distinction
  • Previous review findings have been addressed (dead IsDiscountActive method removed)
  • Test coverage is thorough with new multi-coupon test cases across all affected commands

No new issues found. The implementation is consistent and well-tested.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 16, 2026

Logo
Checkmarx One – Scan Summary & Details6798acc0-4700-41b9-bff0-9bf5930e085c

Great job! No new security vulnerabilities introduced in this pull request

@codecov
Copy link

codecov bot commented Mar 16, 2026

Codecov Report

❌ Patch coverage is 90.58824% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.64%. Comparing base (a7259df) to head (df1658c).

Files with missing lines Patch % Lines
...st/Organizations/OrganizationCreateRequestModel.cs 0.00% 2 Missing ⚠️
...zations/OrganizationSubscriptionPurchaseRequest.cs 0.00% 2 Missing ⚠️
...s/Premium/PremiumCloudHostedSubscriptionRequest.cs 0.00% 2 Missing ⚠️
...ce/PreviewPremiumSubscriptionPurchaseTaxRequest.cs 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7229      +/-   ##
==========================================
+ Coverage   57.62%   57.64%   +0.01%     
==========================================
  Files        2032     2032              
  Lines       89517    89542      +25     
  Branches     7974     7973       -1     
==========================================
+ Hits        51587    51617      +30     
+ Misses      36071    36070       -1     
+ Partials     1859     1855       -4     

☔ View full report in Codecov by Sentry.
📢 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.

@kdenney kdenney marked this pull request as ready for review March 17, 2026 15:08
@kdenney kdenney requested review from a team as code owners March 17, 2026 15:09
@kdenney kdenney requested review from cyprain-okeke and r-tome March 17, 2026 15:09
Copy link
Contributor

@cyprain-okeke cyprain-okeke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks Good

r-tome
r-tome previously approved these changes Mar 18, 2026
Copy link
Contributor

@cyprain-okeke cyprain-okeke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, Please fix the conflict.

…scounts

# Conflicts:
#	test/Core.Test/Billing/Services/OrganizationBillingServiceTests.cs
@sonarqubecloud
Copy link

@kdenney kdenney requested a review from r-tome March 18, 2026 20:04
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants