feat(payments-next): Cancel free trial on upgrade#20328
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the upgrade flow so that upgrading from a trialing subscription immediately ends the free trial (preventing users from carrying a free trial onto an upgraded plan) and informs users they’ll be billed today.
Changes:
- Ends Stripe trials on subscription upgrade by setting
trial_endto “now” for trialing subscriptions. - Adjusts upcoming-invoice previews for upgrades to account for ending trials immediately.
- Adds an upgrade-page warning message when upgrading from an active free trial.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| libs/payments/customer/src/lib/invoice.manager.ts | Adds optional trialEnd handling when previewing upcoming invoices for upgrades. |
| libs/payments/cart/src/lib/checkout.service.ts | Cancels trials on upgrade via Stripe subscription update; passes trialEnd for invoice previews. |
| libs/payments/cart/src/lib/checkout.service.spec.ts | Adds tests asserting trial_end: 'now' is passed on upgrades from trialing subscriptions. |
| libs/payments/cart/src/lib/cart.types.ts | Extends cart DTO with isUpgradeFromTrial flag. |
| libs/payments/cart/src/lib/cart.service.ts | Computes isUpgradeFromTrial and passes trialEnd into upgrade invoice previews; returns flag to client. |
| libs/payments/cart/src/lib/cart.service.spec.ts | Adds/updates tests to validate isUpgradeFromTrial behavior. |
| apps/payments/next/app/[locale]/[offeringId]/[interval]/upgrade/[cartId]/(startLayout)/start/page.tsx | Displays different acknowledgment text when upgrading from a trial. |
| apps/payments/next/app/[locale]/[offeringId]/[interval]/upgrade/[cartId]/(startLayout)/start/en.ftl | Adds localized string for the “upgrade from trial” acknowledgment. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -147,6 +149,7 @@ export class InvoiceManager { | |||
| ], | |||
| proration_behavior: 'always_invoice', | |||
| proration_date: Math.floor(Date.now() / 1000), | |||
| ...(trialEnd && { trial_end: trialEnd }), | |||
| }, | |||
There was a problem hiding this comment.
trialEnd is newly supported here, but there’s no unit test that verifies trial_end is actually forwarded to Stripe when provided. Adding a spec that calls previewUpcomingUpgrade({ trialEnd: <number> }) and asserts stripeClient.invoicesRetrieveUpcoming received subscription_details.trial_end would prevent regressions in this billing-critical behavior.
| const fromSubscription = | ||
| await this.subscriptionManager.retrieveForCustomerAndPrice( | ||
| customer.id, | ||
| eligibility.fromPrice.id | ||
| ); | ||
| assert( | ||
| fromSubscription, | ||
| new DetermineCheckoutAmountSubscriptionRequiredError( | ||
| customer.id, | ||
| eligibility.fromPrice.id | ||
| ) | ||
| ); | ||
| const isTrialing = fromSubscription.status === 'trialing'; | ||
| const fromSubscriptionItem = retrieveSubscriptionItem(fromSubscription); | ||
| const upcomingInvoice = | ||
| await this.invoiceManager.previewUpcomingForUpgrade({ | ||
| priceId, | ||
| customer, | ||
| fromSubscriptionItem, | ||
| ...(isTrialing && { | ||
| trialEnd: Math.floor(Date.now() / 1000), | ||
| }), | ||
| }); |
There was a problem hiding this comment.
determineCheckoutAmount now conditionally passes trialEnd when the existing subscription is trialing, but the test suite doesn’t cover this branch (it only asserts the upgrade path calls previewUpcomingForUpgrade). Add a test where retrieveForCustomerAndPrice returns a trialing subscription and assert invoiceManager.previewUpcomingForUpgrade is called with a trialEnd number (e.g. expect.any(Number)).
| @@ -928,12 +929,16 @@ export class CartService { | |||
| eligibility.fromPrice.id | |||
| ); | |||
| assert(fromSubscription, new GetCartSubscriptionMissingError(cartId)); | |||
| isUpgradeFromTrial = fromSubscription.status === 'trialing'; | |||
| const fromSubscriptionItem = retrieveSubscriptionItem(fromSubscription); | |||
| upcomingInvoicePreview = | |||
| await this.invoiceManager.previewUpcomingForUpgrade({ | |||
| priceId: price.id, | |||
| customer, | |||
| fromSubscriptionItem, | |||
| ...(isUpgradeFromTrial && { | |||
| trialEnd: Math.floor(Date.now() / 1000), | |||
| }), | |||
| }); | |||
There was a problem hiding this comment.
getCart now passes trialEnd into previewUpcomingForUpgrade when upgrading from a trialing subscription, but the new spec only asserts isUpgradeFromTrial: true and doesn’t verify the invoice-preview call includes trialEnd. Consider asserting invoiceManager.previewUpcomingForUpgrade was called with trialEnd: expect.any(Number) in the trialing-subscription test to ensure the preview matches the post-upgrade billing behavior.
|
I like the Copilot suggestions to add a couple tests, especially the one in cart.service.spec. Otherwise WFM and LGTM! r+ thanks Davey! |
|
On However, if the customer has a free trial and tries to upgrade, the page crashes. |
xlisachan
left a comment
There was a problem hiding this comment.
If the customer does not have a default payment method on their account, the Upgrade page crashes. Expected behavior should be the same as on main, in which the customer would be able to select a new payment method.
xlisachan
left a comment
There was a problem hiding this comment.
r+wc! Thanks Davey! 🎉
I know you're aiming to get this PR onto stage today, could you add the missing tests in a polish PR?
Because: * When a user upgrades a subscription, we update their subscription in Stripe. By default, a user's free trial remains attached to the subscription after its update. * A user can use this to obtain a free trial on an upgraded subscription, regardless of that subscription's free trial eligibility This commit: * Cancels a free trial when the user upgrades their subscription by setting the trial's end time to "now" * Adds in warning messaging to the upgrade page alerting the user to the fact that they will be billed the full amount Closes #PAY-3602


Because:
This commit:
Closes #PAY-3602
Checklist
Put an
xin the boxes that applyHow to review (Optional)
Screenshots (Optional)
Please attach the screenshots of the changes made in case of change in user interface.



Other information (Optional)
Any other information that is important to this pull request.