From 4c5cdecffaac55189927b0c6f80faaf2f2a96fbe Mon Sep 17 00:00:00 2001 From: Kristina <7614497+khjrtbrg@users.noreply.github.com> Date: Mon, 2 Feb 2026 11:42:47 +0100 Subject: [PATCH] [web] fix bug where pending downgrades are removed when subscriptions change (#30420) * preserve pending changes when generating change requests * re-apply pending term_end changes after immediate updates * block changes when Stripe subscription has multiple phases * handle MultiplePendingChangesError & rm PendingChangeError GitOrigin-RevId: 0af11044766ff48e683d684ad6d62b732d17290c --- .../app/src/Features/Subscription/Errors.mjs | 4 +- .../Subscription/PaymentProviderEntities.mjs | 150 +++++-- .../Features/Subscription/RecurlyClient.mjs | 132 +++++++ .../Subscription/SubscriptionController.mjs | 68 +++- .../SubscriptionGroupController.mjs | 18 +- .../Subscription/SubscriptionGroupHandler.mjs | 16 - .../Subscription/SubscriptionHandler.mjs | 24 +- .../UserMembershipController.mjs | 5 +- .../dashboard/states/active/active.tsx | 2 +- .../change-plan/individual-plans-table.tsx | 18 +- .../active/change-plan/change-plan.test.tsx | 12 + .../subscription/fixtures/subscriptions.ts | 57 +++ .../PaymentProviderEntities.test.mjs | 365 +++++++++++++++++- .../src/Subscription/RecurlyClient.test.mjs | 237 ++++++++++++ .../SubscriptionController.test.mjs | 33 ++ .../SubscriptionGroupController.test.mjs | 69 +++- .../SubscriptionGroupHandler.test.mjs | 16 - .../Subscription/SubscriptionHandler.test.mjs | 78 +++- .../UserMembershipController.test.mjs | 4 +- 19 files changed, 1179 insertions(+), 129 deletions(-) diff --git a/services/web/app/src/Features/Subscription/Errors.mjs b/services/web/app/src/Features/Subscription/Errors.mjs index 546f5822e5..aac4b94fa7 100644 --- a/services/web/app/src/Features/Subscription/Errors.mjs +++ b/services/web/app/src/Features/Subscription/Errors.mjs @@ -21,7 +21,7 @@ export class MissingBillingInfoError extends OError {} export class ManuallyCollectedError extends OError {} -export class PendingChangeError extends OError {} +export class MultiplePendingChangesError extends OError {} export class InactiveError extends OError {} @@ -76,7 +76,7 @@ export default { PaymentFailedError, MissingBillingInfoError, ManuallyCollectedError, - PendingChangeError, + MultiplePendingChangesError, InactiveError, SubtotalLimitExceededError, HasPastDueInvoiceError, diff --git a/services/web/app/src/Features/Subscription/PaymentProviderEntities.mjs b/services/web/app/src/Features/Subscription/PaymentProviderEntities.mjs index 2858f593fc..da7057ad39 100644 --- a/services/web/app/src/Features/Subscription/PaymentProviderEntities.mjs +++ b/services/web/app/src/Features/Subscription/PaymentProviderEntities.mjs @@ -129,6 +129,26 @@ export class PaymentProviderSubscription { } } + /** + * Returns the add-ons that should be used as the base for a change request. + * + * For immediate changes (timeframe = 'now'), we use the current add-ons because + * the user is still paying for them until term_end. Any pending term_end changes + * will need to be re-applied after the immediate change by the payment provider client. + * + * For term_end changes, if there's already a pending change, we use its nextAddOns + * as the base to merge scheduled changes together. + * + * @param {"now" | "term_end"} timeframe - when the change will be applied + * @return {PaymentProviderSubscriptionAddOn[]} + */ + #getBaseAddOnsForChangeRequest(timeframe) { + if (timeframe === 'term_end' && this.pendingChange != null) { + return this.pendingChange.nextAddOns + } + return this.addOns + } + /** * Change this subscription's plan * @param {string} planCode - the new plan code @@ -137,41 +157,47 @@ export class PaymentProviderSubscription { * @return {PaymentProviderSubscriptionChangeRequest} */ getRequestForPlanChange(planCode, quantity, shouldChangeAtTermEnd) { - const changeRequest = new PaymentProviderSubscriptionChangeRequest({ - subscription: this, - timeframe: shouldChangeAtTermEnd ? 'term_end' : 'now', - planCode, - }) + const timeframe = shouldChangeAtTermEnd ? 'term_end' : 'now' + const baseAddOns = this.#getBaseAddOnsForChangeRequest(timeframe) + + // Start with all existing add-ons, but filter out additional-license + // since we'll handle it specially for group plans below + const addOnUpdates = baseAddOns + .filter(addOn => addOn.code !== MEMBERS_LIMIT_ADD_ON_CODE) + .map(addOn => addOn.toAddOnUpdate()) if (quantity !== 1) { // Only group plans in Stripe can have larger than 1 quantity - // This is because in Stripe, the group plans are configued with per-seat pricing + // This is because in Stripe, the group plans are configured with per-seat pricing // and the quantity is the number of seats // Setting the members limit add-on quantity accordingly - // so it is compitible with Recurly's group plan model (1 base plan + add-on for each member) - changeRequest.addOnUpdates = [ + // so it is compatible with Recurly's group plan model (1 base plan + add-on for each member) + addOnUpdates.push( new PaymentProviderSubscriptionAddOnUpdate({ code: MEMBERS_LIMIT_ADD_ON_CODE, quantity, - }), - ] + }) + ) } - // Carry the AI add-on to the new plan if applicable - if ( - this.isStandaloneAiAddOn() || - (!shouldChangeAtTermEnd && this.hasAddOn(AI_ADD_ON_CODE)) || - (shouldChangeAtTermEnd && this.hasAddOnNextPeriod(AI_ADD_ON_CODE)) - ) { - const addOnUpdate = new PaymentProviderSubscriptionAddOnUpdate({ - code: AI_ADD_ON_CODE, - quantity: 1, - }) - changeRequest.addOnUpdates = changeRequest.addOnUpdates - ? [...changeRequest.addOnUpdates, addOnUpdate] - : [addOnUpdate] + // When upgrading from standalone AI plan, add the AI add-on since + // the standalone plan doesn't have it as an add-on + if (this.isStandaloneAiAddOn() && !isStandaloneAiAddOnPlanCode(planCode)) { + addOnUpdates.push( + new PaymentProviderSubscriptionAddOnUpdate({ + code: AI_ADD_ON_CODE, + quantity: 1, + }) + ) } + const changeRequest = new PaymentProviderSubscriptionChangeRequest({ + subscription: this, + timeframe, + planCode, + addOnUpdates, + }) + return changeRequest } @@ -194,7 +220,8 @@ export class PaymentProviderSubscription { }) } - const addOnUpdates = this.addOns.map(addOn => addOn.toAddOnUpdate()) + const baseAddOns = this.#getBaseAddOnsForChangeRequest('now') + const addOnUpdates = baseAddOns.map(addOn => addOn.toAddOnUpdate()) addOnUpdates.push( new PaymentProviderSubscriptionAddOnUpdate({ code, quantity, unitPrice }) ) @@ -226,7 +253,8 @@ export class PaymentProviderSubscription { ) } - const addOnUpdates = this.addOns.map(addOn => { + const baseAddOns = this.#getBaseAddOnsForChangeRequest('now') + const addOnUpdates = baseAddOns.map(addOn => { const update = addOn.toAddOnUpdate() if (update.code === code) { @@ -261,13 +289,22 @@ export class PaymentProviderSubscription { } ) } - const addOnUpdates = this.addOns + const isInTrial = SubscriptionHelper.isInTrial(this.trialPeriodEnd) + const timeframe = isInTrial ? 'now' : 'term_end' + + const baseAddOns = this.#getBaseAddOnsForChangeRequest(timeframe) + const addOnUpdates = baseAddOns .filter(addOn => addOn.code !== code) .map(addOn => addOn.toAddOnUpdate()) - const isInTrial = SubscriptionHelper.isInTrial(this.trialPeriodEnd) + + // preserve pending plan change when scheduling add-on removal at term_end + const planCode = + timeframe === 'term_end' ? this.pendingChange?.nextPlanCode : undefined + return new PaymentProviderSubscriptionChangeRequest({ subscription: this, - timeframe: isInTrial ? 'now' : 'term_end', + timeframe, + planCode, addOnUpdates, }) } @@ -295,9 +332,62 @@ export class PaymentProviderSubscription { .map(addOn => addOn.toAddOnUpdate()) addOnUpdates.push(reactivatedAddOn.toAddOnUpdate()) + // preserve pending plan change when reactivating add-on + const planCode = pendingChange.nextPlanCode + return new PaymentProviderSubscriptionChangeRequest({ subscription: this, timeframe: 'term_end', + planCode, + addOnUpdates, + }) + } + + /** + * Cancel only the pending plan change while preserving any pending add-on changes + * + * This is used when a user wants to "keep their current plan" but has also + * scheduled add-on changes (additions or removals) that should still happen. + * + * @return {PaymentProviderSubscriptionChangeRequest | null} + */ + getRequestForPlanChangeCancellation() { + const pendingChange = this.pendingChange + if (pendingChange == null) { + return null + } + + const currentAddOnCodes = this.addOns.map(a => a.code) + const pendingAddOnCodes = pendingChange.nextAddOns.map(a => a.code) + + const hasAddOnChanges = + currentAddOnCodes.length !== pendingAddOnCodes.length || + !currentAddOnCodes.every(code => pendingAddOnCodes.includes(code)) + + if (!hasAddOnChanges) { + return null + } + + const addOnUpdates = pendingChange.nextAddOns + .filter(addOn => { + if ( + addOn.code === MEMBERS_LIMIT_ADD_ON_CODE && + !this.isGroupSubscription() && + PlansLocator.isGroupPlanCode(pendingChange.nextPlanCode) + ) { + // if there was a pending group plan upgrade, removing the plan change + // means we also need to remove the members limit add-on + return false + } + + return true + }) + .map(addOn => addOn.toAddOnUpdate()) + + return new PaymentProviderSubscriptionChangeRequest({ + subscription: this, + timeframe: 'term_end', + planCode: this.planCode, addOnUpdates, }) } @@ -346,8 +436,8 @@ export class PaymentProviderSubscription { * @return {PaymentProviderSubscriptionChangeRequest} */ getRequestForGroupPlanUpgrade(newPlanCode) { - // Ensure all the existing add-ons are added to the new plan - const addOns = this.addOns.map( + const baseAddOns = this.#getBaseAddOnsForChangeRequest('now') + const addOns = baseAddOns.map( addOn => new PaymentProviderSubscriptionAddOnUpdate({ code: addOn.code, diff --git a/services/web/app/src/Features/Subscription/RecurlyClient.mjs b/services/web/app/src/Features/Subscription/RecurlyClient.mjs index cfd4c9443a..5b50be27a4 100644 --- a/services/web/app/src/Features/Subscription/RecurlyClient.mjs +++ b/services/web/app/src/Features/Subscription/RecurlyClient.mjs @@ -25,6 +25,7 @@ import { } from './Errors.mjs' import RecurlyMetrics from './RecurlyMetrics.mjs' import { isStandaloneAiAddOnPlanCode, AI_ADD_ON_CODE } from './AiHelper.mjs' +import _ from 'lodash' /** * @import { PaymentProviderSubscriptionChangeRequest } from './PaymentProviderEntities.mjs' @@ -240,6 +241,24 @@ async function updateSubscriptionDetails(updateRequest) { async function applySubscriptionChangeRequest(changeRequest) { const body = subscriptionChangeRequestToApi(changeRequest) + // capture pending change before immediate update, as it will be removed by Recurly + const pendingChange = + changeRequest.timeframe === 'now' + ? changeRequest.subscription.pendingChange + : null + + if (pendingChange != null) { + logger.warn( + { + subscriptionId: changeRequest.subscription.id, + timeframe: changeRequest.timeframe, + pendingPlanCode: pendingChange.nextPlanCode, + pendingAddOnCodes: pendingChange.nextAddOns.map(a => a.code), + }, + 'Applying immediate change to subscription with pending term_end change - will attempt to re-apply' + ) + } + try { const change = await client.createSubscriptionChange( `uuid-${changeRequest.subscription.id}`, @@ -249,6 +268,13 @@ async function applySubscriptionChangeRequest(changeRequest) { { subscriptionId: changeRequest.subscription.id, changeId: change.id }, 'created subscription change' ) + + if (pendingChange != null) { + await _reapplyPendingChangeAfterImmediateUpdate( + changeRequest.subscription.id, + pendingChange + ) + } } catch (err) { if (err instanceof recurly.errors.ValidationError) { /** @@ -272,6 +298,112 @@ async function applySubscriptionChangeRequest(changeRequest) { } } +/** + * Re-apply a pending term_end change after an immediate update. + * + * When an immediate change is applied to a subscription that has a pending + * term_end change (e.g., scheduled add-on removal or plan downgrade), we need + * to re-create the pending change with the correct items based on what the + * pending change was trying to achieve and what happened in the immediate change. + * + * @param {string} subscriptionId + * @param {PaymentProviderSubscriptionChange} pendingChange + */ +async function _reapplyPendingChangeAfterImmediateUpdate( + subscriptionId, + pendingChange +) { + const updatedSubscription = await getSubscription(subscriptionId) + + const planCodeDiffers = + pendingChange.nextPlanCode !== updatedSubscription.planCode + + const immediateChangeIncludedPlanChange = + pendingChange.subscription.planCode !== updatedSubscription.planCode + + // Build add-on objects for comparison: { code: { quantity, unitPrice } } + const preUpdateAddOns = pendingChange.subscription.addOns.reduce( + (acc, addOn) => ({ + ...acc, + [addOn.code]: { quantity: addOn.quantity, unitPrice: addOn.unitPrice }, + }), + {} + ) + const preUpdatePendingAddOns = pendingChange.nextAddOns.reduce( + (acc, addOn) => ({ + ...acc, + [addOn.code]: { quantity: addOn.quantity, unitPrice: addOn.unitPrice }, + }), + {} + ) + const postUpdateAddOns = updatedSubscription.addOns.reduce( + (acc, addOn) => ({ + ...acc, + [addOn.code]: { quantity: addOn.quantity, unitPrice: addOn.unitPrice }, + }), + {} + ) + + // Merge: start with pending add-ons, add any new add-ons from immediate update + const mergedAddOns = { ...preUpdatePendingAddOns } + for (const [code, details] of Object.entries(postUpdateAddOns)) { + // include any add-ons that were added via immediate update just now + if (!(code in preUpdateAddOns) && !(code in mergedAddOns)) { + mergedAddOns[code] = details + } + } + + const addOnsDiffer = !_.isEqual(mergedAddOns, postUpdateAddOns) + + if (!planCodeDiffers && !addOnsDiffer) { + logger.debug( + { subscriptionId }, + 'No pending changes to re-apply after immediate update' + ) + return + } + + // only re-apply the pending plan change if: + // 1. immediate change didn't include a plan change (was add-on only) + // 2. pending plan actually differs from the current plan + // If the immediate change was a plan change, the user's new plan choice supersedes the old pending plan. + const shouldReapplyPlanCode = + !immediateChangeIncludedPlanChange && planCodeDiffers + + logger.debug( + { + subscriptionId, + shouldReapplyPlanCode, + shouldReapplyAddOns: addOnsDiffer, + }, + 're-applying pending term_end change after immediate update' + ) + + /** @type {recurly.SubscriptionChangeCreate} */ + const requestBody = { + timeframe: 'term_end', + addOns: Object.entries(mergedAddOns).map(([code, details]) => ({ + code, + quantity: details.quantity, + unitAmount: details.unitPrice, + })), + } + + if (shouldReapplyPlanCode) { + requestBody.planCode = pendingChange.nextPlanCode + } + + const change = await client.createSubscriptionChange( + `uuid-${subscriptionId}`, + requestBody + ) + + logger.debug( + { subscriptionId, changeId: change.id }, + 're-applied pending term_end change' + ) +} + /** * Preview a subscription change * diff --git a/services/web/app/src/Features/Subscription/SubscriptionController.mjs b/services/web/app/src/Features/Subscription/SubscriptionController.mjs index cd1951eb12..2f4fa3a9ec 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionController.mjs +++ b/services/web/app/src/Features/Subscription/SubscriptionController.mjs @@ -35,6 +35,7 @@ import { sanitizeSessionUserForFrontEnd } from '../../infrastructure/FrontEndUse import { z, parseReq } from '../../infrastructure/Validation.mjs' import { IndeterminateInvoiceError } from '../Errors/Errors.js' import SubscriptionLocator from './SubscriptionLocator.mjs' +import { PaymentProviderSubscriptionChange } from './PaymentProviderEntities.mjs' const { DuplicateAddOnError, @@ -42,6 +43,7 @@ const { PaymentActionRequiredError, PaymentFailedError, MissingBillingInfoError, + MultiplePendingChangesError, } = Errors const SUBSCRIPTION_PAUSED_REDIRECT_PATH = @@ -157,7 +159,6 @@ async function checkSubscriptionPauseStatus(user) { /** * @import { SubscriptionChangeDescription } from '../../../../types/subscription/subscription-change-preview' * @import { SubscriptionChangePreview } from '../../../../types/subscription/subscription-change-preview' - * @import { PaymentProviderSubscriptionChange } from './PaymentProviderEntities.mjs' * @import { PaymentMethod } from './types' */ @@ -655,6 +656,16 @@ async function purchaseAddon(req, res, next) { reason: err.info.reason, adviceCode: err.info.adviceCode, }) + } else if (err instanceof MultiplePendingChangesError) { + logger.warn( + { userId: user._id, err, addOnCode }, + 'Cannot purchase add-on: multiple pending changes' + ) + return res.status(422).json({ + code: 'multiple_pending_changes', + message: + 'Cannot complete purchase while there are multiple pending subscription changes. Please contact support.', + }) } else { if (err instanceof Error) { OError.tag(err, 'something went wrong purchasing add-ons', { @@ -703,6 +714,16 @@ async function removeAddon(req, res, next) { 'Your subscription does not contain the requested add-on', { addon: addOnCode } ) + } else if (err instanceof MultiplePendingChangesError) { + logger.warn( + { userId: user._id, err, addOnCode }, + 'Cannot remove add-on: multiple pending changes' + ) + return res.status(422).json({ + code: 'multiple_pending_changes', + message: + 'Cannot remove add-on while there are multiple pending subscription changes. Please contact support.', + }) } else { if (err instanceof Error) { OError.tag(err, 'something went wrong removing add-ons', { @@ -1104,9 +1125,38 @@ function makeChangePreview( paymentMethod ) { const subscription = subscriptionChange.subscription + + // For the future invoice display, if there's a pending change scheduled, + // we should show what will happen at renewal (the pending change state) + // merged with any new changes from this immediate update + const pendingChange = subscription.pendingChange + + let futureInvoiceChange + if (pendingChange) { + const pendingAddOnCodes = new Set(pendingChange.nextAddOns.map(a => a.code)) + const mergedAddOns = [...pendingChange.nextAddOns] + + for (const addOn of subscriptionChange.nextAddOns) { + if (!pendingAddOnCodes.has(addOn.code)) { + mergedAddOns.push(addOn) + } + } + + futureInvoiceChange = new PaymentProviderSubscriptionChange({ + subscription, + nextPlanCode: pendingChange.nextPlanCode, + nextPlanName: pendingChange.nextPlanName, + nextPlanPrice: pendingChange.nextPlanPrice, + nextAddOns: mergedAddOns, + }) + } else { + futureInvoiceChange = subscriptionChange + } + const nextPlan = PlansLocator.findLocalPlanInSettings( - subscriptionChange.nextPlanCode + futureInvoiceChange.nextPlanCode ) + return { change: subscriptionChangeDescription, currency: subscription.currency, @@ -1120,24 +1170,24 @@ function makeChangePreview( date: subscription.periodEnd.toISOString(), plan: { name: getPlanNameForDisplay( - subscriptionChange.nextPlanName, - subscriptionChange.nextPlanCode + futureInvoiceChange.nextPlanName, + futureInvoiceChange.nextPlanCode ), - amount: subscriptionChange.nextPlanPrice, + amount: futureInvoiceChange.nextPlanPrice, }, - addOns: subscriptionChange.nextAddOns.map(addOn => ({ + addOns: futureInvoiceChange.nextAddOns.map(addOn => ({ code: addOn.code, name: addOn.name, quantity: addOn.quantity, unitAmount: addOn.unitPrice, amount: addOn.preTaxTotal, })), - subtotal: subscriptionChange.subtotal, + subtotal: futureInvoiceChange.subtotal, tax: { rate: subscription.taxRate, - amount: subscriptionChange.tax, + amount: futureInvoiceChange.tax, }, - total: subscriptionChange.total, + total: futureInvoiceChange.total, }, } } diff --git a/services/web/app/src/Features/Subscription/SubscriptionGroupController.mjs b/services/web/app/src/Features/Subscription/SubscriptionGroupController.mjs index d52a8df6ec..7fcd3bbd6f 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionGroupController.mjs +++ b/services/web/app/src/Features/Subscription/SubscriptionGroupController.mjs @@ -14,12 +14,12 @@ import { isProfessionalGroupPlan } from './PlansHelper.mjs' import { MissingBillingInfoError, ManuallyCollectedError, - PendingChangeError, InactiveError, SubtotalLimitExceededError, HasPastDueInvoiceError, HasNoAdditionalLicenseWhenManuallyCollectedError, PaymentActionRequiredError, + MultiplePendingChangesError, } from './Errors.mjs' const MAX_NUMBER_OF_USERS = 20 @@ -146,9 +146,6 @@ async function addSeatsToGroupSubscription(req, res) { userId ) await SubscriptionGroupHandler.promises.ensureFlexibleLicensingEnabled(plan) - await SubscriptionGroupHandler.promises.ensureSubscriptionHasNoPendingChanges( - paymentProviderSubscription - ) await SubscriptionGroupHandler.promises.ensureSubscriptionIsActive( subscription ) @@ -186,7 +183,6 @@ async function addSeatsToGroupSubscription(req, res) { } if ( - error instanceof PendingChangeError || error instanceof InactiveError || error instanceof HasPastDueInvoiceError ) { @@ -228,7 +224,6 @@ async function previewAddSeatsSubscriptionChange(req, res) { } catch (error) { if ( error instanceof MissingBillingInfoError || - error instanceof PendingChangeError || error instanceof InactiveError || error instanceof HasPastDueInvoiceError || error instanceof HasNoAdditionalLicenseWhenManuallyCollectedError @@ -271,7 +266,7 @@ async function createAddSeatsSubscriptionChange(req, res) { } catch (error) { if ( error instanceof MissingBillingInfoError || - error instanceof PendingChangeError || + error instanceof MultiplePendingChangesError || error instanceof InactiveError || error instanceof HasPastDueInvoiceError || error instanceof HasNoAdditionalLicenseWhenManuallyCollectedError @@ -383,7 +378,7 @@ async function subscriptionUpgradePage(req, res) { return res.redirect('/user/subscription/group/subtotal-limit-exceeded') } - if (error instanceof PendingChangeError || error instanceof InactiveError) { + if (error instanceof InactiveError) { return res.redirect('/user/subscription') } @@ -406,6 +401,13 @@ async function upgradeSubscription(req, res) { publicKey: error.info.publicKey, }) } + if (error instanceof MultiplePendingChangesError) { + return res.status(422).json({ + code: 'multiple_pending_changes', + message: + 'Cannot upgrade subscription while there are multiple pending subscription changes. Please contact support.', + }) + } logger.err({ error }, 'error trying to upgrade subscription') return res.sendStatus(500) } diff --git a/services/web/app/src/Features/Subscription/SubscriptionGroupHandler.mjs b/services/web/app/src/Features/Subscription/SubscriptionGroupHandler.mjs index ff584fee1c..e2299c1ce8 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionGroupHandler.mjs +++ b/services/web/app/src/Features/Subscription/SubscriptionGroupHandler.mjs @@ -14,7 +14,6 @@ import Modules from '../../infrastructure/Modules.mjs' import PaymentProviderEntities from './PaymentProviderEntities.mjs' import { ManuallyCollectedError, - PendingChangeError, InactiveError, HasPastDueInvoiceError, HasNoAdditionalLicenseWhenManuallyCollectedError, @@ -100,16 +99,6 @@ async function ensureSubscriptionCollectionMethodIsNotManual( } } -async function ensureSubscriptionHasNoPendingChanges( - paymentProviderSubscription -) { - if (paymentProviderSubscription.pendingChange) { - throw new PendingChangeError('This subscription has a pending change', { - subscription_id: paymentProviderSubscription.id, - }) - } -} - async function ensureSubscriptionHasNoPastDueInvoice(subscription) { const [paymentRecord] = await Modules.promises.hooks.fire( 'getPaymentFromRecord', @@ -184,7 +173,6 @@ async function _addSeatsSubscriptionChange(userId, adding) { await getUsersGroupSubscriptionDetails(userId) await ensureFlexibleLicensingEnabled(plan) await ensureSubscriptionIsActive(subscription) - await ensureSubscriptionHasNoPendingChanges(paymentProviderSubscription) await checkBillingInfoExistence(paymentProviderSubscription, userId) await ensureSubscriptionHasNoPastDueInvoice(subscription) @@ -481,9 +469,6 @@ export default { ensureSubscriptionCollectionMethodIsNotManual: callbackify( ensureSubscriptionCollectionMethodIsNotManual ), - ensureSubscriptionHasNoPendingChanges: callbackify( - ensureSubscriptionHasNoPendingChanges - ), ensureSubscriptionHasNoPastDueInvoice: callbackify( ensureSubscriptionHasNoPastDueInvoice ), @@ -503,7 +488,6 @@ export default { ensureFlexibleLicensingEnabled, ensureSubscriptionIsActive, ensureSubscriptionCollectionMethodIsNotManual, - ensureSubscriptionHasNoPendingChanges, ensureSubscriptionHasNoPastDueInvoice, ensureSubscriptionHasAdditionalLicenseAddOnWhenCollectionMethodIsManual, getTotalConfirmedUsersInGroup, diff --git a/services/web/app/src/Features/Subscription/SubscriptionHandler.mjs b/services/web/app/src/Features/Subscription/SubscriptionHandler.mjs index 19f9f658c4..1e5fcde5c7 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionHandler.mjs +++ b/services/web/app/src/Features/Subscription/SubscriptionHandler.mjs @@ -126,10 +126,30 @@ async function cancelPendingSubscriptionChange(user) { await LimitationsManager.promises.userHasSubscription(user) if (hasSubscription && subscription != null) { - await Modules.promises.hooks.fire( - 'cancelPendingPaidSubscriptionChange', + const [paymentRecord] = await Modules.promises.hooks.fire( + 'getPaymentFromRecord', subscription ) + + if (paymentRecord != null) { + const changeRequest = + paymentRecord.subscription.getRequestForPlanChangeCancellation() + + if (changeRequest) { + // There are pending add-on changes to preserve, apply the change request + await Modules.promises.hooks.fire( + 'applySubscriptionChangeRequestAndSync', + changeRequest, + user._id.toString() + ) + } else if (paymentRecord.subscription.pendingChange != null) { + // No add-on changes to preserve, just remove the pending change + await Modules.promises.hooks.fire( + 'cancelPendingPaidSubscriptionChange', + subscription + ) + } + } } } diff --git a/services/web/app/src/Features/UserMembership/UserMembershipController.mjs b/services/web/app/src/Features/UserMembership/UserMembershipController.mjs index c204fee93b..01b93f16e3 100644 --- a/services/web/app/src/Features/UserMembership/UserMembershipController.mjs +++ b/services/web/app/src/Features/UserMembership/UserMembershipController.mjs @@ -58,10 +58,7 @@ async function manageGroupMembers(req, res, next) { } const canUseAddSeatsFeature = Boolean( - plan?.canUseFlexibleLicensing && - isAdmin && - recurlySubscription && - !recurlySubscription.pendingChange + plan?.canUseFlexibleLicensing && isAdmin && recurlySubscription ) res.render('user_membership/group-members-react', { diff --git a/services/web/frontend/js/features/subscription/components/dashboard/states/active/active.tsx b/services/web/frontend/js/features/subscription/components/dashboard/states/active/active.tsx index c4b5acb5ff..6cc3c7f935 100644 --- a/services/web/frontend/js/features/subscription/components/dashboard/states/active/active.tsx +++ b/services/web/frontend/js/features/subscription/components/dashboard/states/active/active.tsx @@ -368,7 +368,7 @@ function FlexibleGroupLicensingActions({ }) { const { t } = useTranslation() - if (subscription.pendingPlan || subscription.payment.hasPastDueInvoice) { + if (subscription.payment.hasPastDueInvoice) { return null } diff --git a/services/web/frontend/js/features/subscription/components/dashboard/states/active/change-plan/individual-plans-table.tsx b/services/web/frontend/js/features/subscription/components/dashboard/states/active/change-plan/individual-plans-table.tsx index 330b68dc82..2be2f9c431 100644 --- a/services/web/frontend/js/features/subscription/components/dashboard/states/active/change-plan/individual-plans-table.tsx +++ b/services/web/frontend/js/features/subscription/components/dashboard/states/active/change-plan/individual-plans-table.tsx @@ -38,22 +38,24 @@ function KeepCurrentPlanButton() { function ChangePlanButton({ plan }: { plan: Plan }) { const { t } = useTranslation() const { personalSubscription } = useSubscriptionDashboardContext() + const currentPlanCode = personalSubscription?.planCode?.split('_')[0] + const pendingPlanCode = + personalSubscription?.pendingPlan?.planCode?.split('_')[0] const isCurrentPlanForUser = - personalSubscription?.planCode && - plan.planCode === personalSubscription.planCode.split('_')[0] + currentPlanCode && plan.planCode === currentPlanCode + + if (isCurrentPlanForUser) { + if (pendingPlanCode && pendingPlanCode !== currentPlanCode) { + return + } - if (isCurrentPlanForUser && personalSubscription.pendingPlan) { - return - } else if (isCurrentPlanForUser && !personalSubscription.pendingPlan) { return (  {t('your_plan')} ) - } else if ( - personalSubscription?.pendingPlan?.planCode?.split('_')[0] === plan.planCode - ) { + } else if (pendingPlanCode === plan.planCode) { return ( diff --git a/services/web/test/frontend/features/subscription/components/dashboard/states/active/change-plan/change-plan.test.tsx b/services/web/test/frontend/features/subscription/components/dashboard/states/active/change-plan/change-plan.test.tsx index bbcb955d2d..4f029a3ae8 100644 --- a/services/web/test/frontend/features/subscription/components/dashboard/states/active/change-plan/change-plan.test.tsx +++ b/services/web/test/frontend/features/subscription/components/dashboard/states/active/change-plan/change-plan.test.tsx @@ -6,6 +6,7 @@ import { annualActiveSubscription, annualActiveSubscriptionEuro, annualActiveSubscriptionPro, + pendingAddOnChange, pendingSubscriptionChange, } from '../../../../../fixtures/subscriptions' import { ActiveSubscription } from '../../../../../../../../../frontend/js/features/subscription/components/dashboard/states/active/active' @@ -69,6 +70,17 @@ describe('', function () { screen.getByRole('button', { name: 'Keep my current plan' }) }) + it('renders "Your plan" when there is a pending add-on change but no plan change', async function () { + renderActiveSubscription(pendingAddOnChange) + + const button = screen.getByRole('button', { name: 'Change plan' }) + fireEvent.click(button) + + await screen.findByText('Your plan') + expect(screen.queryByRole('button', { name: 'Keep my current plan' })).to.be + .null + }) + it('does not render when Recurly did not load', function () { const { container } = renderWithSubscriptionDashContext( , diff --git a/services/web/test/frontend/features/subscription/fixtures/subscriptions.ts b/services/web/test/frontend/features/subscription/fixtures/subscriptions.ts index 56c37172dc..9f6795ee71 100644 --- a/services/web/test/frontend/features/subscription/fixtures/subscriptions.ts +++ b/services/web/test/frontend/features/subscription/fixtures/subscriptions.ts @@ -409,6 +409,63 @@ export const canceledSubscription: PaidSubscription = { }, } +export const pendingAddOnChange: PaidSubscription = { + manager_ids: ['abc123'], + member_ids: [], + invited_emails: [], + groupPlan: false, + membersLimit: 0, + _id: 'add-on-change-123', + admin_id: 'abc123', + teamInvites: [], + planCode: 'collaborator-annual', + plan: { + planCode: 'collaborator-annual', + name: 'Standard (Collaborator) Annual', + price_in_cents: 21900, + annual: true, + featureDescription: [], + canUseFlexibleLicensing: false, + }, + payment: { + taxRate: 0, + billingDetailsLink: '/user/subscription/payment/billing-details', + accountManagementLink: '/user/subscription/payment/account-management', + additionalLicenses: 0, + totalLicenses: 0, + nextPaymentDueAt, + nextPaymentDueDate, + currency: 'USD', + state: 'active', + trialEndsAtFormatted: null, + trialEndsAt: null, + activeCoupons: [], + accountEmail: 'fake@example.com', + hasPastDueInvoice: false, + displayPrice: '$199.00', + planOnlyDisplayPrice: '$199.00', + addOns: [ + { + code: 'AI', + quantity: 1, + unitPrice: 1000, + name: 'AI Add-on', + }, + ], + addOnDisplayPricesWithoutAdditionalLicense: {}, + isEligibleForGroupPlan: true, + isEligibleForPause: false, + isEligibleForDowngradeUpsell: false, + }, + pendingPlan: { + planCode: 'collaborator-annual', + name: 'Standard (Collaborator) Annual', + price_in_cents: 21900, + annual: true, + featureDescription: [], + }, +} + export const pendingSubscriptionChange: PaidSubscription = { manager_ids: ['abc123'], member_ids: [], diff --git a/services/web/test/unit/src/Subscription/PaymentProviderEntities.test.mjs b/services/web/test/unit/src/Subscription/PaymentProviderEntities.test.mjs index 404426e764..ef13aab412 100644 --- a/services/web/test/unit/src/Subscription/PaymentProviderEntities.test.mjs +++ b/services/web/test/unit/src/Subscription/PaymentProviderEntities.test.mjs @@ -104,6 +104,13 @@ describe('PaymentProviderEntities', function () { subscription: ctx.subscription, timeframe: 'now', planCode: 'premium-plan', + addOnUpdates: [ + new PaymentProviderSubscriptionAddOnUpdate({ + code: ctx.addOn.code, + quantity: ctx.addOn.quantity, + unitPrice: ctx.addOn.unitPrice, + }), + ], }) ) }) @@ -121,6 +128,13 @@ describe('PaymentProviderEntities', function () { subscription: ctx.subscription, timeframe: 'term_end', planCode: 'cheap-plan', + addOnUpdates: [ + new PaymentProviderSubscriptionAddOnUpdate({ + code: ctx.addOn.code, + quantity: ctx.addOn.quantity, + unitPrice: ctx.addOn.unitPrice, + }), + ], }) ) }) @@ -141,6 +155,13 @@ describe('PaymentProviderEntities', function () { subscription: ctx.subscription, timeframe: 'now', planCode: 'cheap-plan', + addOnUpdates: [ + new PaymentProviderSubscriptionAddOnUpdate({ + code: ctx.addOn.code, + quantity: ctx.addOn.quantity, + unitPrice: ctx.addOn.unitPrice, + }), + ], }) ) }) @@ -163,6 +184,7 @@ describe('PaymentProviderEntities', function () { new PaymentProviderSubscriptionAddOnUpdate({ code: AI_ADD_ON_CODE, quantity: 1, + unitPrice: ctx.addOn.unitPrice, }), ], }) @@ -187,6 +209,7 @@ describe('PaymentProviderEntities', function () { new PaymentProviderSubscriptionAddOnUpdate({ code: AI_ADD_ON_CODE, quantity: 1, + unitPrice: ctx.addOn.unitPrice, }), ], }) @@ -235,6 +258,11 @@ describe('PaymentProviderEntities', function () { timeframe: 'now', planCode: 'group_collaborator', addOnUpdates: [ + new PaymentProviderSubscriptionAddOnUpdate({ + code: ctx.addOn.code, + quantity: ctx.addOn.quantity, + unitPrice: ctx.addOn.unitPrice, + }), new PaymentProviderSubscriptionAddOnUpdate({ code: 'additional-license', quantity: 10, @@ -262,18 +290,84 @@ describe('PaymentProviderEntities', function () { timeframe: 'now', planCode: 'group_collaborator', addOnUpdates: [ - new PaymentProviderSubscriptionAddOnUpdate({ - code: 'additional-license', - quantity: 10, - }), new PaymentProviderSubscriptionAddOnUpdate({ code: AI_ADD_ON_CODE, quantity: 1, + unitPrice: ctx.addOn.unitPrice, + }), + new PaymentProviderSubscriptionAddOnUpdate({ + code: 'additional-license', + quantity: 10, }), ], }) ) }) + + describe('with pending add-on removal', function () { + beforeEach(function (ctx) { + // Reset to a subscription with an add-on and pending removal + const { + PaymentProviderSubscription, + PaymentProviderSubscriptionAddOn, + } = ctx.PaymentProviderEntities + ctx.addOn = new PaymentProviderSubscriptionAddOn({ + code: 'add-on-code', + name: 'My Add-On', + quantity: 1, + unitPrice: 2, + }) + ctx.subscription = new PaymentProviderSubscription({ + id: 'subscription-id', + userId: 'user-id', + planCode: 'regular-plan', + planName: 'My Plan', + planPrice: 10, + addOns: [ctx.addOn], + subtotal: 10.99, + taxRate: 0.2, + taxAmount: 2.4, + total: 14.4, + currency: 'USD', + }) + // Set up a pending change that removes the add-on at term_end + ctx.subscription.pendingChange = + new PaymentProviderSubscriptionChange({ + subscription: ctx.subscription, + nextPlanCode: ctx.subscription.planCode, + nextPlanName: ctx.subscription.planName, + nextPlanPrice: ctx.subscription.planPrice, + nextAddOns: [], // Add-on is scheduled to be removed + }) + }) + + it('preserves current add-ons for immediate upgrade', function (ctx) { + const changeRequest = ctx.subscription.getRequestForPlanChange( + 'premium-plan', + 1, + false // immediate change + ) + expect(changeRequest.timeframe).to.equal('now') + expect(changeRequest.addOnUpdates).to.deep.equal([ + new PaymentProviderSubscriptionAddOnUpdate({ + code: ctx.addOn.code, + quantity: ctx.addOn.quantity, + unitPrice: ctx.addOn.unitPrice, + }), + ]) + }) + + it('stacks with pending add-on removal for term_end downgrade', function (ctx) { + const changeRequest = ctx.subscription.getRequestForPlanChange( + 'cheap-plan', + 1, + true // term_end change + ) + expect(changeRequest.timeframe).to.equal('term_end') + // Should have NO add-ons because the pending change already removed them + expect(changeRequest.addOnUpdates).to.deep.equal([]) + }) + }) }) describe('getRequestForAddOnPurchase()', function () { @@ -340,6 +434,51 @@ describe('PaymentProviderEntities', function () { ctx.subscription.getRequestForAddOnPurchase(ctx.addOn.code) ).to.throw(Errors.DuplicateAddOnError) }) + + describe('with pending plan downgrade', function () { + beforeEach(function (ctx) { + // Scenario: downgrade is scheduled, and then they want to buy an add-on immediately + const { + PaymentProviderSubscription, + PaymentProviderSubscriptionChange, + } = ctx.PaymentProviderEntities + ctx.subscription = new PaymentProviderSubscription({ + id: 'subscription-id', + userId: 'user-id', + planCode: 'premium-plan', + planName: 'Premium Plan', + planPrice: 20, + addOns: [], + subtotal: 20, + taxRate: 0.2, + taxAmount: 4, + total: 24, + currency: 'USD', + }) + // Pending downgrade to cheaper plan at term_end + ctx.subscription.pendingChange = + new PaymentProviderSubscriptionChange({ + subscription: ctx.subscription, + nextPlanCode: 'cheap-plan', + nextPlanName: 'Cheap Plan', + nextPlanPrice: 5, + nextAddOns: [], + }) + }) + + it('uses current add-ons for immediate add-on purchase', function (ctx) { + const changeRequest = + ctx.subscription.getRequestForAddOnPurchase('assistant') + expect(changeRequest.timeframe).to.equal('now') + // Should only have the new add-on, based on current state, client will reapply the pending change + expect(changeRequest.addOnUpdates).to.deep.equal([ + new PaymentProviderSubscriptionAddOnUpdate({ + code: 'assistant', + quantity: 1, + }), + ]) + }) + }) }) describe('getRequestForAddOnUpdate()', function () { @@ -410,6 +549,57 @@ describe('PaymentProviderEntities', function () { ctx.subscription.getRequestForAddOnRemoval('another-add-on') ).to.throw(Errors.AddOnNotPresentError) }) + + describe('with pending changes', function () { + beforeEach(function (ctx) { + // Scenario: a subscription has two add-ons, one already scheduled for removal + const { + PaymentProviderSubscription, + PaymentProviderSubscriptionAddOn, + } = ctx.PaymentProviderEntities + ctx.addOn1 = new PaymentProviderSubscriptionAddOn({ + code: 'addon-1', + name: 'Add-On 1', + quantity: 1, + unitPrice: 2, + }) + ctx.addOn2 = new PaymentProviderSubscriptionAddOn({ + code: 'addon-2', + name: 'Add-On 2', + quantity: 1, + unitPrice: 3, + }) + ctx.subscription = new PaymentProviderSubscription({ + id: 'subscription-id', + userId: 'user-id', + planCode: 'regular-plan', + planName: 'My Plan', + planPrice: 10, + addOns: [ctx.addOn1, ctx.addOn2], + subtotal: 10.99, + taxRate: 0.2, + taxAmount: 2.4, + total: 14.4, + currency: 'USD', + }) + // Set up a pending change that removes addon-1 at term_end + ctx.subscription.pendingChange = + new PaymentProviderSubscriptionChange({ + subscription: ctx.subscription, + nextPlanCode: ctx.subscription.planCode, + nextPlanName: ctx.subscription.planName, + nextPlanPrice: ctx.subscription.planPrice, + nextAddOns: [ctx.addOn2], // Only addon-2 remains + }) + }) + + it('stacks multiple add-on removals at term_end', function (ctx) { + const changeRequest = + ctx.subscription.getRequestForAddOnRemoval('addon-2') + expect(changeRequest.timeframe).to.equal('term_end') + expect(changeRequest.addOnUpdates).to.deep.equal([]) // empty because both are scheduled for removal + }) + }) }) describe('getRequestForAddOnReactivation()', function () { @@ -471,6 +661,172 @@ describe('PaymentProviderEntities', function () { }) }) + describe('getRequestForPlanChangeCancellation()', function () { + it('returns null when there is no pending change', function (ctx) { + ctx.subscription.pendingChange = null + const result = ctx.subscription.getRequestForPlanChangeCancellation() + expect(result).to.be.null + }) + + it('returns null when pending change has no add-on changes', function (ctx) { + // Same add-ons in pending change as current subscription + ctx.subscription.pendingChange = + new PaymentProviderSubscriptionChange({ + subscription: ctx.subscription, + nextPlanCode: 'cheap-plan', // only plan change, no add-on change + nextPlanName: 'Cheap Plan', + nextPlanPrice: 5, + nextAddOns: [ctx.addOn], // same add-on as current subscription + }) + const result = ctx.subscription.getRequestForPlanChangeCancellation() + expect(result).to.be.null + }) + + it('returns a change request preserving add-on removal when canceling plan change', function (ctx) { + // Pending change has both plan downgrade AND add-on removal + ctx.subscription.pendingChange = + new PaymentProviderSubscriptionChange({ + subscription: ctx.subscription, + nextPlanCode: 'cheap-plan', + nextPlanName: 'Cheap Plan', + nextPlanPrice: 5, + nextAddOns: [], // add-on is being removed + }) + + const result = ctx.subscription.getRequestForPlanChangeCancellation() + expect(result).to.not.be.null + expect(result.timeframe).to.equal('term_end') + expect(result.planCode).to.equal(ctx.subscription.planCode) // keep current plan + expect(result.addOnUpdates).to.deep.equal([]) // preserve add-on removal + }) + + it('returns a change request preserving add-on addition when canceling plan change', function (ctx) { + const { + PaymentProviderSubscriptionAddOn, + PaymentProviderSubscriptionAddOnUpdate, + } = ctx.PaymentProviderEntities + + // Current subscription has one add-on + // Pending change has plan downgrade AND a new add-on + const newAddOn = new PaymentProviderSubscriptionAddOn({ + code: 'new-addon', + name: 'New Add-On', + quantity: 1, + unitPrice: 3, + }) + ctx.subscription.pendingChange = + new PaymentProviderSubscriptionChange({ + subscription: ctx.subscription, + nextPlanCode: 'cheap-plan', + nextPlanName: 'Cheap Plan', + nextPlanPrice: 5, + nextAddOns: [ctx.addOn, newAddOn], // current add-on plus new one + }) + + const result = ctx.subscription.getRequestForPlanChangeCancellation() + expect(result).to.not.be.null + expect(result.timeframe).to.equal('term_end') + expect(result.planCode).to.equal(ctx.subscription.planCode) // keep current plan + expect(result.addOnUpdates).to.deep.equal([ + new PaymentProviderSubscriptionAddOnUpdate({ + code: ctx.addOn.code, + quantity: ctx.addOn.quantity, + unitPrice: ctx.addOn.unitPrice, + }), + new PaymentProviderSubscriptionAddOnUpdate({ + code: newAddOn.code, + quantity: newAddOn.quantity, + unitPrice: newAddOn.unitPrice, + }), + ]) + }) + + it('removes additional-license add-on when canceling pending group plan upgrade', function (ctx) { + const { MEMBERS_LIMIT_ADD_ON_CODE } = ctx.PaymentProviderEntities + + // Current: professional-annual (no add-ons, not a group plan) + ctx.subscription.planCode = 'professional-annual' + ctx.subscription.planName = 'Professional Annual' + ctx.subscription.addOns = [] + + // Pending: group_professional_10_educational + 5 additional-license + const additionalLicenseAddOn = + new ctx.PaymentProviderEntities.PaymentProviderSubscriptionAddOn({ + code: MEMBERS_LIMIT_ADD_ON_CODE, + name: 'Additional License', + quantity: 5, + unitPrice: 10, + }) + + ctx.subscription.pendingChange = + new PaymentProviderSubscriptionChange({ + subscription: ctx.subscription, + nextPlanCode: 'group_professional_10_educational', + nextPlanName: 'Group Professional Educational', + nextPlanPrice: 100, + nextAddOns: [additionalLicenseAddOn], + }) + + const result = ctx.subscription.getRequestForPlanChangeCancellation() + + // Should return a change request that keeps the current plan + // but removes the additional-license add-on + expect(result).to.not.be.null + expect(result.timeframe).to.equal('term_end') + expect(result.planCode).to.equal('professional-annual') + expect(result.addOnUpdates).to.deep.equal([]) + }) + + it('preserves non-members-limit add-ons when canceling pending group plan upgrade', function (ctx) { + const { MEMBERS_LIMIT_ADD_ON_CODE } = ctx.PaymentProviderEntities + + // Current: professional-annual with AI add-on (not a group plan) + ctx.subscription.planCode = 'professional-annual' + ctx.subscription.planName = 'Professional Annual' + const aiAddOn = + new ctx.PaymentProviderEntities.PaymentProviderSubscriptionAddOn({ + code: AI_ADD_ON_CODE, + name: 'AI Add-On', + quantity: 1, + unitPrice: 20, + }) + ctx.subscription.addOns = [aiAddOn] + + // Pending: group_professional_10_educational + AI add-on + 5 additional-license + const additionalLicenseAddOn = + new ctx.PaymentProviderEntities.PaymentProviderSubscriptionAddOn({ + code: MEMBERS_LIMIT_ADD_ON_CODE, + name: 'Additional License', + quantity: 5, + unitPrice: 10, + }) + + ctx.subscription.pendingChange = + new PaymentProviderSubscriptionChange({ + subscription: ctx.subscription, + nextPlanCode: 'group_professional_10_educational', + nextPlanName: 'Group Professional Educational', + nextPlanPrice: 100, + nextAddOns: [aiAddOn, additionalLicenseAddOn], + }) + + const result = ctx.subscription.getRequestForPlanChangeCancellation() + + // Should return a change request that keeps the current plan and AI add-on + // but removes the additional-license add-on + expect(result).to.not.be.null + expect(result.timeframe).to.equal('term_end') + expect(result.planCode).to.equal('professional-annual') + expect(result.addOnUpdates).to.deep.equal([ + new PaymentProviderSubscriptionAddOnUpdate({ + code: AI_ADD_ON_CODE, + quantity: 1, + unitPrice: 20, + }), + ]) + }) + }) + describe('with an add-on pending cancellation', function () { beforeEach(function (ctx) { ctx.subscription.pendingChange = @@ -491,6 +847,7 @@ describe('PaymentProviderEntities', function () { new PaymentProviderSubscriptionChangeRequest({ subscription: ctx.subscription, timeframe: 'term_end', + planCode: ctx.subscription.pendingChange.nextPlanCode, addOnUpdates: [ctx.addOn.toAddOnUpdate()], }) ) diff --git a/services/web/test/unit/src/Subscription/RecurlyClient.test.mjs b/services/web/test/unit/src/Subscription/RecurlyClient.test.mjs index ec538a8c0f..121404a918 100644 --- a/services/web/test/unit/src/Subscription/RecurlyClient.test.mjs +++ b/services/web/test/unit/src/Subscription/RecurlyClient.test.mjs @@ -5,8 +5,10 @@ import PaymentProviderEntities from '../../../../app/src/Features/Subscription/P const { PaymentProviderSubscription, + PaymentProviderSubscriptionChange, PaymentProviderSubscriptionChangeRequest, PaymentProviderSubscriptionUpdateRequest, + PaymentProviderSubscriptionAddOn, PaymentProviderSubscriptionAddOnUpdate, PaymentProviderAccount, PaymentProviderCoupon, @@ -459,6 +461,241 @@ describe('RecurlyClient', function () { }) ).to.be.rejectedWith(Error) }) + + describe('when subscription has a pending add-on removal', function () { + beforeEach(function (ctx) { + // Create subscription with an add-on that has a pending removal scheduled + ctx.subscriptionWithAddOn = new PaymentProviderSubscription({ + id: 'subscription-id', + userId: 'user-id', + currency: 'EUR', + planCode: 'collaborator', + planName: 'Collaborator Plan', + planPrice: 13, + addOns: [ + new PaymentProviderSubscriptionAddOn({ + code: 'assistant', + name: 'AI Assistant', + quantity: 1, + unitPrice: 5, + }), + ], + subtotal: 18, + taxRate: 0.1, + taxAmount: 1.8, + total: 19.8, + periodStart: new Date(), + periodEnd: new Date(), + collectionMethod: 'automatic', + service: 'recurly', + state: 'active', + }) + ctx.subscriptionWithAddOn.pendingChange = + new PaymentProviderSubscriptionChange({ + subscription: ctx.subscriptionWithAddOn, + nextPlanCode: 'collaborator', + nextPlanName: 'Collaborator Plan', + nextPlanPrice: 13, + nextAddOns: [], // add-on is being removed + }) + + ctx.client.getSubscription = sinon.stub().resolves({ + uuid: 'subscription-id', + account: { code: 'user-id' }, + plan: { code: 'professional', name: 'Professional Plan' }, + addOns: [ + { + addOn: { code: 'assistant', name: 'AI Assistant' }, + quantity: 1, + unitAmount: 5, + }, + ], + unitAmount: 20, + subtotal: 25, + taxInfo: { rate: 0.1 }, + tax: 2.5, + total: 27.5, + currency: 'EUR', + currentPeriodStartedAt: new Date(), + currentPeriodEndsAt: new Date(), + collectionMethod: 'automatic', + netTerms: 0, + poNumber: '', + termsAndConditions: '', + }) + }) + + it('re-applies pending add-on removal after immediate plan upgrade', async function (ctx) { + await ctx.RecurlyClient.promises.applySubscriptionChangeRequest( + new PaymentProviderSubscriptionChangeRequest({ + subscription: ctx.subscriptionWithAddOn, + timeframe: 'now', + planCode: 'professional', + }) + ) + + // immediate change + expect(ctx.client.createSubscriptionChange).to.have.been.calledWith( + 'uuid-subscription-id', + { timeframe: 'now', planCode: 'professional' } + ) + + // re-apply pending change: only add-on removal, NOT the plan downgrade + // because the immediate change was a plan change, the user's new plan choice supersedes + expect(ctx.client.createSubscriptionChange).to.have.been.calledWith( + 'uuid-subscription-id', + { + timeframe: 'term_end', + addOns: [], + } + ) + + expect(ctx.client.createSubscriptionChange.callCount).to.equal(2) + }) + + it('does not re-apply pending change if plan and add-ons are the same', async function (ctx) { + ctx.subscriptionWithAddOn.pendingChange = + new PaymentProviderSubscriptionChange({ + subscription: ctx.subscriptionWithAddOn, + nextPlanCode: 'professional', // pending change to professional + nextPlanName: 'Professional Plan', + nextPlanPrice: 20, + nextAddOns: [ + new PaymentProviderSubscriptionAddOn({ + code: 'assistant', + name: 'AI Assistant', + quantity: 1, + unitPrice: 5, + }), + ], + }) + + await ctx.RecurlyClient.promises.applySubscriptionChangeRequest( + new PaymentProviderSubscriptionChangeRequest({ + subscription: ctx.subscriptionWithAddOn, + timeframe: 'now', + planCode: 'professional', + }) + ) + + expect(ctx.client.createSubscriptionChange.callCount).to.equal(1) + }) + + it('does not re-apply anything if no pending change existed', async function (ctx) { + ctx.subscriptionWithAddOn.pendingChange = null + + await ctx.RecurlyClient.promises.applySubscriptionChangeRequest( + new PaymentProviderSubscriptionChangeRequest({ + subscription: ctx.subscriptionWithAddOn, + timeframe: 'now', + planCode: 'professional', + }) + ) + + expect(ctx.client.createSubscriptionChange.callCount).to.equal(1) + }) + + it('does not re-apply when timeframe is term_end', async function (ctx) { + await ctx.RecurlyClient.promises.applySubscriptionChangeRequest( + new PaymentProviderSubscriptionChangeRequest({ + subscription: ctx.subscriptionWithAddOn, + timeframe: 'term_end', + planCode: 'professional', + }) + ) + + expect(ctx.client.createSubscriptionChange.callCount).to.equal(1) + }) + + it('re-applies pending plan downgrade while preserving newly purchased add-on', async function (ctx) { + const subscriptionWithPendingDowngrade = + new PaymentProviderSubscription({ + id: 'subscription-id', + userId: 'user-id', + currency: 'EUR', + planCode: 'professional', + planName: 'Professional Plan', + planPrice: 20, + addOns: [], // NO add-ons originally + subtotal: 20, + taxRate: 0.1, + taxAmount: 2, + total: 22, + periodStart: new Date(), + periodEnd: new Date(), + collectionMethod: 'automatic', + service: 'recurly', + state: 'active', + }) + subscriptionWithPendingDowngrade.pendingChange = + new PaymentProviderSubscriptionChange({ + subscription: subscriptionWithPendingDowngrade, + nextPlanCode: 'collaborator', + nextPlanName: 'Collaborator Plan', + nextPlanPrice: 13, + nextAddOns: [], // NO add-ons in pending change + }) + + // mock the subscription fetch after immediate update - now has add-on + ctx.client.getSubscription = sinon.stub().resolves({ + uuid: 'subscription-id', + account: { code: 'user-id' }, + plan: { code: 'professional', name: 'Professional Plan' }, + addOns: [ + { + addOn: { code: 'assistant', name: 'AI Assistant' }, + quantity: 1, + unitAmount: 5, + }, + ], + unitAmount: 20, + subtotal: 25, + taxInfo: { rate: 0.1 }, + tax: 2.5, + total: 27.5, + currency: 'EUR', + currentPeriodStartedAt: new Date(), + currentPeriodEndsAt: new Date(), + collectionMethod: 'automatic', + netTerms: 0, + poNumber: '', + termsAndConditions: '', + }) + + await ctx.RecurlyClient.promises.applySubscriptionChangeRequest( + new PaymentProviderSubscriptionChangeRequest({ + subscription: subscriptionWithPendingDowngrade, + timeframe: 'now', + addOnUpdates: [ + new PaymentProviderSubscriptionAddOnUpdate({ + code: 'assistant', + quantity: 1, + unitPrice: 5, + }), + ], + }) + ) + + expect(ctx.client.createSubscriptionChange).to.have.been.calledWith( + 'uuid-subscription-id', + { + timeframe: 'now', + addOns: [{ code: 'assistant', quantity: 1, unitAmount: 5 }], + } + ) + + expect(ctx.client.createSubscriptionChange).to.have.been.calledWith( + 'uuid-subscription-id', + { + timeframe: 'term_end', + planCode: 'collaborator', + addOns: [{ code: 'assistant', quantity: 1, unitAmount: 5 }], // add-on preserved! + } + ) + + expect(ctx.client.createSubscriptionChange.callCount).to.equal(2) + }) + }) }) describe('updateSubscriptionDetails', function () { diff --git a/services/web/test/unit/src/Subscription/SubscriptionController.test.mjs b/services/web/test/unit/src/Subscription/SubscriptionController.test.mjs index f515c068fc..ecd56ff630 100644 --- a/services/web/test/unit/src/Subscription/SubscriptionController.test.mjs +++ b/services/web/test/unit/src/Subscription/SubscriptionController.test.mjs @@ -1122,6 +1122,24 @@ describe('SubscriptionController', function () { ) ).to.be.true }) + + it('should handle MultiplePendingChangesError and return 422 with JSON response', async function (ctx) { + ctx.req.params.addOnCode = AI_ADD_ON_CODE + ctx.SubscriptionHandler.promises.purchaseAddon.rejects( + new SubscriptionErrors.MultiplePendingChangesError() + ) + + await ctx.SubscriptionController.purchaseAddon(ctx.req, ctx.res, ctx.next) + + expect(ctx.res.status).toHaveBeenCalledWith(422) + expect(ctx.res.json).toHaveBeenCalledWith({ + code: 'multiple_pending_changes', + message: + 'Cannot complete purchase while there are multiple pending subscription changes. Please contact support.', + }) + expect(ctx.FeaturesUpdater.promises.refreshFeatures).to.not.have.been + .called + }) }) describe('removeAddon', function () { @@ -1173,6 +1191,21 @@ describe('SubscriptionController', function () { { addon: AI_ADD_ON_CODE } ) }) + + it('should handle MultiplePendingChangesError and return 422 with JSON response', async function (ctx) { + ctx.SubscriptionHandler.promises.removeAddon.rejects( + new SubscriptionErrors.MultiplePendingChangesError() + ) + + await ctx.SubscriptionController.removeAddon(ctx.req, ctx.res, ctx.next) + + expect(ctx.res.status).toHaveBeenCalledWith(422) + expect(ctx.res.json).toHaveBeenCalledWith({ + code: 'multiple_pending_changes', + message: + 'Cannot remove add-on while there are multiple pending subscription changes. Please contact support.', + }) + }) }) describe('checkSubscriptionPauseStatus', function () { diff --git a/services/web/test/unit/src/Subscription/SubscriptionGroupController.test.mjs b/services/web/test/unit/src/Subscription/SubscriptionGroupController.test.mjs index 406195ab69..26d2a12e98 100644 --- a/services/web/test/unit/src/Subscription/SubscriptionGroupController.test.mjs +++ b/services/web/test/unit/src/Subscription/SubscriptionGroupController.test.mjs @@ -66,7 +66,6 @@ describe('SubscriptionGroupController', function () { ensureFlexibleLicensingEnabled: sinon.stub().resolves(), ensureSubscriptionIsActive: sinon.stub().resolves(), ensureSubscriptionCollectionMethodIsNotManual: sinon.stub().resolves(), - ensureSubscriptionHasNoPendingChanges: sinon.stub().resolves(), ensureSubscriptionHasNoPastDueInvoice: sinon.stub().resolves(), getGroupPlanUpgradePreview: sinon .stub() @@ -132,7 +131,6 @@ describe('SubscriptionGroupController', function () { ctx.Errors = { MissingBillingInfoError: class extends Error {}, ManuallyCollectedError: class extends Error {}, - PendingChangeError: class extends Error {}, InactiveError: class extends Error {}, SubtotalLimitExceededError: class extends Error {}, HasPastDueInvoiceError: class extends Error {}, @@ -143,6 +141,7 @@ describe('SubscriptionGroupController', function () { this.info = info } }, + MultiplePendingChangesError: class extends Error {}, } vi.doMock( @@ -440,9 +439,6 @@ describe('SubscriptionGroupController', function () { ctx.SubscriptionGroupHandler.promises.ensureFlexibleLicensingEnabled .calledWith(ctx.plan) .should.equal(true) - ctx.SubscriptionGroupHandler.promises.ensureSubscriptionHasNoPendingChanges - .calledWith(ctx.recurlySubscription) - .should.equal(true) ctx.SubscriptionGroupHandler.promises.ensureSubscriptionIsActive .calledWith(ctx.subscription) .should.equal(true) @@ -542,22 +538,6 @@ describe('SubscriptionGroupController', function () { }) }) - it('should redirect to subscription page when there is a pending change', async function (ctx) { - await new Promise(resolve => { - ctx.SubscriptionGroupHandler.promises.ensureSubscriptionHasNoPendingChanges = - sinon.stub().throws(new ctx.Errors.PendingChangeError()) - - const res = { - redirect: url => { - url.should.equal('/user/subscription') - resolve() - }, - } - - ctx.Controller.addSeatsToGroupSubscription(ctx.req, res) - }) - }) - it('should redirect to subscription page when subscription is not active', async function (ctx) { await new Promise(resolve => { ctx.SubscriptionGroupHandler.promises.ensureSubscriptionIsActive = sinon @@ -756,6 +736,28 @@ describe('SubscriptionGroupController', function () { ctx.Controller.createAddSeatsSubscriptionChange(ctx.req, res) }) }) + + it('should return 422 with MultiplePendingChangesError', async function (ctx) { + await new Promise(resolve => { + ctx.req.body = { adding: 2 } + ctx.SubscriptionGroupHandler.promises.createAddSeatsSubscriptionChange = + sinon.stub().throws(new ctx.Errors.MultiplePendingChangesError()) + + const res = { + status: statusCode => { + statusCode.should.equal(422) + + return { + end: () => { + resolve() + }, + } + }, + } + + ctx.Controller.createAddSeatsSubscriptionChange(ctx.req, res) + }) + }) }) describe('submitForm', function () { @@ -963,5 +965,30 @@ describe('SubscriptionGroupController', function () { ctx.Controller.upgradeSubscription(ctx.req, res) }) }) + + it('should send 422 response with MultiplePendingChangesError', async function (ctx) { + await new Promise(resolve => { + ctx.SubscriptionGroupHandler.promises.upgradeGroupPlan = sinon + .stub() + .rejects(new ctx.Errors.MultiplePendingChangesError()) + const res = { + status: code => { + code.should.equal(422) + return { + json: data => { + data.should.deep.equal({ + code: 'multiple_pending_changes', + message: + 'Cannot upgrade subscription while there are multiple pending subscription changes. Please contact support.', + }) + resolve() + }, + } + }, + } + + ctx.Controller.upgradeSubscription(ctx.req, res) + }) + }) }) }) diff --git a/services/web/test/unit/src/Subscription/SubscriptionGroupHandler.test.mjs b/services/web/test/unit/src/Subscription/SubscriptionGroupHandler.test.mjs index 7ea2548db6..ce068e2da3 100644 --- a/services/web/test/unit/src/Subscription/SubscriptionGroupHandler.test.mjs +++ b/services/web/test/unit/src/Subscription/SubscriptionGroupHandler.test.mjs @@ -819,22 +819,6 @@ describe('SubscriptionGroupHandler', function () { }) }) - describe('ensureSubscriptionHasNoPendingChanges', function () { - it('should throw if the subscription has pending change', async function (ctx) { - await expect( - ctx.Handler.promises.ensureSubscriptionHasNoPendingChanges({ - pendingChange: {}, - }) - ).to.be.rejectedWith('This subscription has a pending change') - }) - - it('should not throw if the subscription has no pending change', async function (ctx) { - await expect( - ctx.Handler.promises.ensureSubscriptionHasNoPendingChanges({}) - ).to.not.be.rejected - }) - }) - describe('ensureSubscriptionHasNoPastDueInvoice', function () { it('should throw if the subscription has past due invoice', async function (ctx) { ctx.Modules.promises.hooks.fire diff --git a/services/web/test/unit/src/Subscription/SubscriptionHandler.test.mjs b/services/web/test/unit/src/Subscription/SubscriptionHandler.test.mjs index 1ec71d8d4c..82d8e5c660 100644 --- a/services/web/test/unit/src/Subscription/SubscriptionHandler.test.mjs +++ b/services/web/test/unit/src/Subscription/SubscriptionHandler.test.mjs @@ -395,7 +395,7 @@ describe('SubscriptionHandler', function () { }) }) - it('should not fire cancelPendingPaidSubscriptionChange hook if user has no subscription', async function (ctx) { + it('should not fire hooks if user has no subscription', async function (ctx) { ctx.LimitationsManager.promises.userHasSubscription.resolves({ hasSubscription: false, subscription: null, @@ -404,25 +404,91 @@ describe('SubscriptionHandler', function () { ctx.user, ctx.plan_code ) - expect(ctx.Modules.promises.hooks.fire).to.not.have.been.calledWith( - 'cancelPendingPaidSubscriptionChange', - sinon.match.any - ) + expect(ctx.Modules.promises.hooks.fire).to.not.have.been.called }) - it('should fire cancelPendingPaidSubscriptionChange to update a valid subscription', async function (ctx) { + it('should get payment record and apply change request', async function (ctx) { + const changeRequest = { subscription: { id: 'sub_123' } } + const paymentProviderSubscription = { + id: 'sub_123', + service: 'stripe', + getRequestForPlanChangeCancellation: sinon + .stub() + .returns(changeRequest), + } + const paymentRecord = { subscription: paymentProviderSubscription } + ctx.LimitationsManager.promises.userHasSubscription.resolves({ hasSubscription: true, subscription: ctx.subscription, }) + ctx.Modules.promises.hooks.fire + .withArgs('getPaymentFromRecord', ctx.subscription) + .resolves([paymentRecord]) + ctx.Modules.promises.hooks.fire + .withArgs( + 'applySubscriptionChangeRequestAndSync', + changeRequest, + ctx.user._id.toString() + ) + .resolves([Promise.resolve()]) + await ctx.SubscriptionHandler.promises.cancelPendingSubscriptionChange( ctx.user, ctx.plan_code ) + + expect(ctx.Modules.promises.hooks.fire).to.have.been.calledWith( + 'getPaymentFromRecord', + ctx.subscription + ) + expect(paymentProviderSubscription.getRequestForPlanChangeCancellation).to + .have.been.called + expect(ctx.Modules.promises.hooks.fire).to.have.been.calledWith( + 'applySubscriptionChangeRequestAndSync', + changeRequest, + ctx.user._id.toString() + ) + }) + + it('should remove pending change when there are no add-on changes to preserve', async function (ctx) { + const paymentProviderSubscription = { + id: 'sub_123', + service: 'stripe', + pendingChange: { nextPlanCode: 'student' }, + getRequestForPlanChangeCancellation: sinon.stub().returns(null), + } + const paymentRecord = { subscription: paymentProviderSubscription } + + ctx.LimitationsManager.promises.userHasSubscription.resolves({ + hasSubscription: true, + subscription: ctx.subscription, + }) + ctx.Modules.promises.hooks.fire + .withArgs('getPaymentFromRecord', ctx.subscription) + .resolves([paymentRecord]) + ctx.Modules.promises.hooks.fire + .withArgs('cancelPendingPaidSubscriptionChange', ctx.subscription) + .resolves([Promise.resolve()]) + + await ctx.SubscriptionHandler.promises.cancelPendingSubscriptionChange( + ctx.user, + ctx.plan_code + ) + + expect(ctx.Modules.promises.hooks.fire).to.have.been.calledWith( + 'getPaymentFromRecord', + ctx.subscription + ) + expect(paymentProviderSubscription.getRequestForPlanChangeCancellation).to + .have.been.called expect(ctx.Modules.promises.hooks.fire).to.have.been.calledWith( 'cancelPendingPaidSubscriptionChange', ctx.subscription ) + expect(ctx.Modules.promises.hooks.fire).to.not.have.been.calledWith( + 'applySubscriptionChangeRequestAndSync' + ) }) }) diff --git a/services/web/test/unit/src/UserMembership/UserMembershipController.test.mjs b/services/web/test/unit/src/UserMembership/UserMembershipController.test.mjs index 146267dc14..d9ce158cbc 100644 --- a/services/web/test/unit/src/UserMembership/UserMembershipController.test.mjs +++ b/services/web/test/unit/src/UserMembership/UserMembershipController.test.mjs @@ -378,7 +378,7 @@ describe('UserMembershipController', () => { }) }) - it('should be false when recurly subscription has pending changes', async ({ + it('should be true when recurly subscription has pending changes', async ({ UserMembershipController, req, RecurlyClient, @@ -390,7 +390,7 @@ describe('UserMembershipController', () => { }) await UserMembershipController.manageGroupMembers(req, { render: (viewPath, viewParams) => { - expect(viewParams.canUseAddSeatsFeature).to.equal(false) + expect(viewParams.canUseAddSeatsFeature).to.equal(true) }, }) })