Add coupon support to invoice preview and subscription creation#6994
Add coupon support to invoice preview and subscription creation#6994cyprain-okeke wants to merge 8 commits intomainfrom
Conversation
|
New Issues (2)Checkmarx found the following issues in this Pull Request
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #6994 +/- ##
==========================================
- Coverage 56.28% 56.26% -0.02%
==========================================
Files 1986 1987 +1
Lines 87660 87718 +58
Branches 7814 7821 +7
==========================================
+ Hits 49339 49355 +16
- Misses 36490 36530 +40
- Partials 1831 1833 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
amorask-bitwarden
left a comment
There was a problem hiding this comment.
Good start - few changes we need to clean this up.
For testing, can you:
- Add tests for
SubscriptionDiscountService - Update the tests for
CreatePremiumCloudHostedSubscriptionCommandto validate coupon functionality instead of passingnullto all - Update the tests for
PreviewPremiumTaxCommandto validate coupon functionality instead of passingnullto all
If you want, optionally add more tests for OrganizationBillingService.Finalize to cover your cases.
| @@ -0,0 +1,34 @@ | |||
| #nullable enable | |||
| return discount.AudienceType switch | ||
| { | ||
| DiscountAudienceType.UserHasNoPreviousSubscriptions => | ||
| !user.Premium && string.IsNullOrEmpty(user.GatewaySubscriptionId), |
There was a problem hiding this comment.
❌ This checks if the user has a premium subscription; not if they've ever had a premium subscription. I think we need to check this and also pull their customer to check for any inactive premium subscriptions from the past.
| OrganizationSubscriptionPurchase purchase, | ||
| BillingAddress billingAddress); | ||
| BillingAddress billingAddress, | ||
| string? coupon); |
There was a problem hiding this comment.
❌ This should be part of the OrganizationSubscriptionPurchase.
| [FromBody] PremiumCloudHostedSubscriptionRequest request) | ||
| { | ||
| var (paymentMethod, billingAddress, additionalStorageGb) = request.ToDomain(); | ||
| var (paymentMethod, billingAddress, additionalStorageGb, coupon) = request.ToDomain(); |
There was a problem hiding this comment.
⛏️ At 4 values, I think this is becoming a bit too unwieldy. Can we create a new PremiumSubscriptionPurchase in src/Core/Billing/Premium/Models, similar to the OrganizationSubscriptionPurchase and pass that into the command instead?
| { | ||
| var (purchase, billingAddress) = request.ToDomain(); | ||
| var result = await previewOrganizationTaxCommand.Run(purchase, billingAddress); | ||
| var (purchase, billingAddress, coupon) = request.ToDomain(); |
There was a problem hiding this comment.
❌ coupon should be part of the purchase.
| var (organization, customerSetup, subscriptionSetup) = sale; | ||
| var (organization, customerSetup, subscriptionSetup, owner) = sale; | ||
|
|
||
| if (!string.IsNullOrWhiteSpace(customerSetup?.Coupon) && owner != null) |
There was a problem hiding this comment.
❌ If the owner is set to null in the OrganizationSignup model that constructed the sale, the validation you've added will be bypassed and the coupon will be applied regardless.
Additionally:
- This change would break SM Standalone coupon application on release, since that coupon isn't represented as a
SubscriptionDiscountnor does it have an audience. - This change applies to all organization sales regardless of tier even though the ticket specifies these coupons are only for users purchasing Premium or Families.
| [Range(0, 99)] | ||
| public short AdditionalStorageGb { get; set; } = 0; | ||
|
|
||
| public string? Coupon { get; set; } |
There was a problem hiding this comment.
⛏️ The DB only takes 50 characters so we should [MaxLength(50)] this.
|
|
||
| public (OrganizationSubscriptionPurchase, BillingAddress) ToDomain() => | ||
| (Purchase.ToDomain(), BillingAddress.ToDomain()); | ||
| public string? Coupon { get; set; } |
There was a problem hiding this comment.
⛏️ The DB only takes 50 characters so we should [MaxLength(50)] this.
| public required MinimalBillingAddressRequest BillingAddress { get; set; } | ||
|
|
||
| public (short, BillingAddress) ToDomain() => (AdditionalStorage, BillingAddress.ToDomain()); | ||
| public string? Coupon { get; set; } |
There was a problem hiding this comment.
⛏️ The DB only takes 50 characters so we should [MaxLength(50)] this.
| /// <item>The user meets the audience targeting criteria for the discount</item> | ||
| /// </list> | ||
| /// </remarks> | ||
| Task<bool> ValidateDiscountForUserAsync(User user, string stripeCouponId); |
There was a problem hiding this comment.
❌ The signature and description of this method are too vague in asserting what it actually does. It doesn't just validate a coupon ID for a user; it specifically checks to see if that coupon ID matches a specific discount audience. That needs to be explicit here. Your options are:
- Rename the method to emphasize the specific audience the discount is being validated against
- Take in an audience as a parameter to validate against
|





🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-30272
📔 Objective
This PR adds coupon/discount code support to the subscription purchase flow, enabling users to apply promotional discounts during both
tax preview and subscription creation for Premium and Organization subscriptions.
What's Changed
1. Tax Preview with Coupon Support
Added optional coupon parameter to tax preview endpoints, allowing users to see discounted pricing before purchase:
Both endpoints now accept an optional coupon field in the request body and return tax calculations with the discount applied.
2. Subscription Creation with Coupon Support
Updated subscription creation endpoints to accept and validate coupon codes:
Coupons are validated server-side before being applied to ensure user eligibility.
3. Server-Side Validation
Implemented ISubscriptionDiscountService to validate coupon eligibility:
4. Special Case Handling
Technical Implementation
API Layer:
Business Logic:
📸 Screenshots