From d49a8f83df9d44fa1003da22d02cab696239bbd4 Mon Sep 17 00:00:00 2001 From: Jimmy Domagala-Tang Date: Wed, 28 May 2025 10:38:36 -0400 Subject: [PATCH] Revert Recurly based subscription upgrades on failed payments (#25824) * feat: add ability to set restore point for subscriptions * feat: update recurly client with ability to get past due invoices and fail invoices * utility to retrieve last valid subscription * create revert requests and fail invoices, revert subscriptions to previous valid states on failed upgrade payments * add restore point and call to revert plans on failed payments * code style for PaymentProviderEntities * moving subs restore point check to SubscriptionController, and removing unecessary error * adding ability to stop sub restores without a deploy * ensure that subs restore point is set before changing plan * changing reverted flag on subscription to count, and only reverting automatic invoices * updating tests with restorepoint functions * rethrow error after voiding restore point, and ensure that recurly failed_payment always gets a 200 response * only void restore point if the changeRequest fails GitOrigin-RevId: cf3074c13db22d1cf680b59c4d57817c390db23e --- .../web/app/src/Features/Errors/Errors.js | 3 + .../Subscription/PaymentProviderEntities.js | 38 +++++++++ .../Features/Subscription/RecurlyClient.js | 36 ++++++++ .../Subscription/SubscriptionController.js | 32 +++++++- .../Subscription/SubscriptionHandler.js | 82 ++++++++++++++++++- .../Subscription/SubscriptionLocator.js | 28 ++++++- .../Subscription/SubscriptionUpdater.js | 54 ++++++++++++ services/web/app/src/models/Subscription.js | 7 ++ 8 files changed, 277 insertions(+), 3 deletions(-) diff --git a/services/web/app/src/Features/Errors/Errors.js b/services/web/app/src/Features/Errors/Errors.js index 4b1b7dd064..487b8cbd03 100644 --- a/services/web/app/src/Features/Errors/Errors.js +++ b/services/web/app/src/Features/Errors/Errors.js @@ -47,6 +47,8 @@ class DuplicateNameError extends OError {} class InvalidNameError extends BackwardCompatibleError {} +class IndeterminateInvoiceError extends OError {} + class UnsupportedFileTypeError extends BackwardCompatibleError {} class FileTooLargeError extends BackwardCompatibleError {} @@ -333,6 +335,7 @@ module.exports = { UnconfirmedEmailError, EmailExistsError, InvalidError, + IndeterminateInvoiceError, NotInV2Error, OutputFileFetchFailedError, SAMLAssertionAudienceMismatch, diff --git a/services/web/app/src/Features/Subscription/PaymentProviderEntities.js b/services/web/app/src/Features/Subscription/PaymentProviderEntities.js index f6a8af4aa5..6fe8638389 100644 --- a/services/web/app/src/Features/Subscription/PaymentProviderEntities.js +++ b/services/web/app/src/Features/Subscription/PaymentProviderEntities.js @@ -2,6 +2,7 @@ /** * @import { PaymentProvider } from '../../../../types/subscription/dashboard/subscription' + * @import { AddOn } from '../../../../types/subscription/plan' */ const OError = require('@overleaf/o-error') @@ -254,6 +255,43 @@ class PaymentProviderSubscription { }) } + /** + * Form a request to revert the plan to it's last saved backup state + * + * @param {string} previousPlanCode + * @param {Array | null} previousAddOns + * @return {PaymentProviderSubscriptionChangeRequest} + * + * @throws {OError} if the restore point plan doesnt exist + */ + getRequestForPlanRevert(previousPlanCode, previousAddOns) { + const lastSuccessfulPlan = + PlansLocator.findLocalPlanInSettings(previousPlanCode) + if (lastSuccessfulPlan == null) { + throw new OError('Unable to find plan in settings', { previousPlanCode }) + } + const changeRequest = new PaymentProviderSubscriptionChangeRequest({ + subscription: this, + timeframe: 'now', + planCode: previousPlanCode, + }) + + // defaulting to empty array is important, as that will wipe away any add-ons that were added in the failed payment + // but were not part of the last successful subscription + const addOns = [] + for (const previousAddon of previousAddOns || []) { + const addOnUpdate = new PaymentProviderSubscriptionAddOnUpdate({ + code: previousAddon.addOnCode, + quantity: previousAddon.quantity, + unitPrice: previousAddon.unitAmountInCents / 100, + }) + addOns.push(addOnUpdate) + } + changeRequest.addOnUpdates = addOns + + return changeRequest + } + /** * Upgrade group plan with the plan code provided * diff --git a/services/web/app/src/Features/Subscription/RecurlyClient.js b/services/web/app/src/Features/Subscription/RecurlyClient.js index fdb3b023e6..753d49ba0f 100644 --- a/services/web/app/src/Features/Subscription/RecurlyClient.js +++ b/services/web/app/src/Features/Subscription/RecurlyClient.js @@ -685,6 +685,38 @@ function subscriptionUpdateRequestToApi(updateRequest) { return requestBody } +/** + * Retrieves a list of failed invoices for a given Recurly subscription ID. + * + * @async + * @function + * @param {string} subscriptionId - The ID of the Recurly subscription to fetch failed invoices for. + * @returns {Promise>} A promise that resolves to an array of failed invoice objects. + */ +async function getPastDueInvoices(subscriptionId) { + const failed = [] + const invoices = client.listSubscriptionInvoices(`uuid-${subscriptionId}`, { + params: { state: 'past_due' }, + }) + + for await (const invoice of invoices.each()) { + failed.push(invoice) + } + return failed +} + +/** + * Marks an invoice as failed using the Recurly client. + * + * @async + * @function failInvoice + * @param {string} invoiceId - The ID of the invoice to be marked as failed. + * @returns {Promise} Resolves when the invoice has been successfully marked as failed. + */ +async function failInvoice(invoiceId) { + await client.markInvoiceFailed(invoiceId) +} + module.exports = { errors: recurly.errors, @@ -706,6 +738,8 @@ module.exports = { subscriptionIsCanceledOrExpired, pauseSubscriptionByUuid: callbackify(pauseSubscriptionByUuid), resumeSubscriptionByUuid: callbackify(resumeSubscriptionByUuid), + getPastDueInvoices: callbackify(getPastDueInvoices), + failInvoice: callbackify(failInvoice), promises: { getSubscription, @@ -726,5 +760,7 @@ module.exports = { getPaymentMethod, getAddOn, getPlan, + getPastDueInvoices, + failInvoice, }, } diff --git a/services/web/app/src/Features/Subscription/SubscriptionController.js b/services/web/app/src/Features/Subscription/SubscriptionController.js index db278b23c0..1cc2ad0094 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionController.js +++ b/services/web/app/src/Features/Subscription/SubscriptionController.js @@ -410,6 +410,8 @@ async function purchaseAddon(req, res, next) { logger.debug({ userId: user._id, addOnCode }, 'purchasing add-ons') try { + // set a restore point in the case of a failed payment for the upgrade (Recurly only) + await SubscriptionHandler.promises.setSubscriptionRestorePoint(user._id) await SubscriptionHandler.promises.purchaseAddon( user._id, addOnCode, @@ -574,7 +576,35 @@ function recurlyCallback(req, res, next) { ) ) - if ( + // this is a recurly only case which is required since Recurly does not have a reliable way to check credit info pre-upgrade purchase + if (event === 'failed_payment_notification') { + if (!Settings.planReverts?.enabled) { + return res.sendStatus(200) + } + + SubscriptionHandler.getSubscriptionRestorePoint( + eventData.transaction.subscription_id, + function (err, lastSubscription) { + if (err) { + return next(err) + } + // if theres no restore point it could be a failed renewal, or no restore set. Either way it will be handled through dunning automatically + if (!lastSubscription || !lastSubscription?.planCode) { + res.sendStatus(200) + } + SubscriptionHandler.revertPlanChange( + eventData.transaction.subscription_id, + lastSubscription, + function (err) { + if (err) { + return next(err) + } + res.sendStatus(200) + } + ) + } + ) + } else if ( [ 'new_subscription_notification', 'updated_subscription_notification', diff --git a/services/web/app/src/Features/Subscription/SubscriptionHandler.js b/services/web/app/src/Features/Subscription/SubscriptionHandler.js index 39a44f305f..1296a2a7de 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionHandler.js +++ b/services/web/app/src/Features/Subscription/SubscriptionHandler.js @@ -11,7 +11,7 @@ const LimitationsManager = require('./LimitationsManager') const EmailHandler = require('../Email/EmailHandler') const { callbackify } = require('@overleaf/promise-utils') const UserUpdater = require('../User/UserUpdater') -const { NotFoundError } = require('../Errors/Errors') +const { NotFoundError, IndeterminateInvoiceError } = require('../Errors/Errors') const Modules = require('../../infrastructure/Modules') /** @@ -387,6 +387,80 @@ async function resumeSubscription(user) { ) } +/** + * @param recurlySubscriptionId + */ +async function getSubscriptionRestorePoint(recurlySubscriptionId) { + const lastSubscription = + await SubscriptionLocator.promises.getLastSuccessfulSubscription( + recurlySubscriptionId + ) + return lastSubscription +} + +/** + * @param recurlySubscriptionId + * @param subscriptionRestorePoint + */ +async function revertPlanChange( + recurlySubscriptionId, + subscriptionRestorePoint +) { + const subscription = await RecurlyClient.promises.getSubscription( + recurlySubscriptionId + ) + + const changeRequest = subscription.getRequestForPlanRevert( + subscriptionRestorePoint.planCode, + subscriptionRestorePoint.addOns + ) + + const pastDue = await RecurlyClient.promises.getPastDueInvoices( + recurlySubscriptionId + ) + + // only process revert requests within the past 24 hours, as we dont want to restore plans at the end of their dunning cycle + const yesterday = new Date() + yesterday.setDate(yesterday.getDate() - 1) + if ( + pastDue.length !== 1 || + !pastDue[0].id || + !pastDue[0].dueAt || + pastDue[0].dueAt < yesterday || + pastDue[0].collectionMethod !== 'automatic' + ) { + throw new IndeterminateInvoiceError( + 'cant determine invoice to fail for plan revert', + { + info: { recurlySubscriptionId }, + } + ) + } + + await RecurlyClient.promises.failInvoice(pastDue[0].id) + await SubscriptionUpdater.promises.setSubscriptionWasReverted( + subscriptionRestorePoint._id + ) + await RecurlyClient.promises.applySubscriptionChangeRequest(changeRequest) + await syncSubscription({ uuid: recurlySubscriptionId }, {}) +} + +async function setSubscriptionRestorePoint(userId) { + const subscription = + await SubscriptionLocator.promises.getUsersSubscription(userId) + // if the subscription is not a recurly one, we can return early as we dont allow for failed payments on other payment providers + // we need to deal with it for recurly, because we cant verify payment in advance + if (!subscription?.recurlySubscription_id || !subscription.planCode) { + return + } + await SubscriptionUpdater.promises.setRestorePoint( + subscription.id, + subscription.planCode, + subscription.addOns, + false + ) +} + module.exports = { validateNoSubscriptionInRecurly: callbackify(validateNoSubscriptionInRecurly), createSubscription: callbackify(createSubscription), @@ -403,6 +477,9 @@ module.exports = { removeAddon: callbackify(removeAddon), pauseSubscription: callbackify(pauseSubscription), resumeSubscription: callbackify(resumeSubscription), + revertPlanChange: callbackify(revertPlanChange), + setSubscriptionRestorePoint: callbackify(setSubscriptionRestorePoint), + getSubscriptionRestorePoint: callbackify(getSubscriptionRestorePoint), promises: { validateNoSubscriptionInRecurly, createSubscription, @@ -419,5 +496,8 @@ module.exports = { removeAddon, pauseSubscription, resumeSubscription, + revertPlanChange, + setSubscriptionRestorePoint, + getSubscriptionRestorePoint, }, } diff --git a/services/web/app/src/Features/Subscription/SubscriptionLocator.js b/services/web/app/src/Features/Subscription/SubscriptionLocator.js index 8526ad0fb2..978f4d41b7 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionLocator.js +++ b/services/web/app/src/Features/Subscription/SubscriptionLocator.js @@ -1,3 +1,7 @@ +/** + * @import { AddOn } from '../../../../types/subscription/plan' + */ + const { callbackifyAll } = require('@overleaf/promise-utils') const { Subscription } = require('../../models/Subscription') const { DeletedSubscription } = require('../../models/DeletedSubscription') @@ -124,7 +128,8 @@ const SubscriptionLocator = { // todo: as opposed to recurlyEntities which use addon.code, subscription model uses addon.addOnCode // which we hope to align via https://github.com/overleaf/internal/issues/25494 return Boolean( - isStandaloneAiAddOnPlanCode(subscription?.planCode) || + (subscription?.planCode && + isStandaloneAiAddOnPlanCode(subscription?.planCode)) || subscription?.addOns?.some(addOn => addOn.addOnCode === AI_ADD_ON_CODE) ) }, @@ -136,6 +141,27 @@ const SubscriptionLocator = { return userOrId } }, + + /** + * Retrieves the last successful subscription for a given user. + * + * @async + * @function + * @param {string} recurlyId - The ID of the recurly subscription tied to the mongo subscription to check for a previous successful state. + * @returns {Promise<{_id: ObjectId, planCode: string, addOns: [AddOn]}|null>} A promise that resolves to the last successful planCode and addon state, + * or null if we havent stored a previous + */ + async getLastSuccessfulSubscription(recurlyId) { + const subscription = await Subscription.findOne({ + recurlySubscription_id: recurlyId, + }).exec() + return subscription && subscription.lastSuccesfulSubscription + ? { + ...subscription.lastSuccesfulSubscription, + _id: subscription._id, + } + : null + }, } module.exports = { diff --git a/services/web/app/src/Features/Subscription/SubscriptionUpdater.js b/services/web/app/src/Features/Subscription/SubscriptionUpdater.js index 15f61b6160..b0e24ce5ad 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionUpdater.js +++ b/services/web/app/src/Features/Subscription/SubscriptionUpdater.js @@ -19,6 +19,7 @@ const Modules = require('../../infrastructure/Modules') * @typedef {import('../../../../types/subscription/dashboard/subscription').Subscription} Subscription * @typedef {import('../../../../types/subscription/dashboard/subscription').PaymentProvider} PaymentProvider * @typedef {import('../../../../types/group-management/group-audit-log').GroupAuditLog} GroupAuditLog + * @import { AddOn } from '../../../../types/subscription/plan' */ /** @@ -486,6 +487,53 @@ async function _sendSubscriptionEventForAllMembers(subscriptionId, event) { } } +/** + * Sets the plan code and addon state to revert the plan to in case of failed upgrades, or clears the last restore point if it was used/ voided + * @param {ObjectId} subscriptionId the mongo ID of the subscription to set the restore point for + * @param {string} planCode the plan code to revert to + * @param {Array} addOns the addOns to revert to + * @param {Boolean} consumed whether the restore point was used to revert a subscription + */ +async function setRestorePoint(subscriptionId, planCode, addOns, consumed) { + const update = { + $set: { + 'lastSuccesfulSubscription.planCode': planCode, + 'lastSuccesfulSubscription.addOns': addOns, + }, + } + + if (consumed) { + update.$inc = { revertedDueToFailedPayment: 1 } + } + + await Subscription.updateOne({ _id: subscriptionId }, update).exec() +} + +/** + * Clears the restore point for a given subscription, and signals that the subscription was sucessfully reverted. + * + * @async + * @function setSubscriptionWasReverted + * @param {ObjectId} subscriptionId the mongo ID of the subscription to set the restore point for + * @returns {Promise} Resolves when the restore point has been cleared. + */ +async function setSubscriptionWasReverted(subscriptionId) { + // consume the backup and flag that the subscription was reverted due to failed payment + await setRestorePoint(subscriptionId, null, null, true) +} + +/** + * Clears the restore point for a given subscription, and signals that the subscription was not reverted. + * + * @async + * @function voidRestorePoint + * @param {string} subscriptionId - The unique identifier of the subscription. + * @returns {Promise} Resolves when the restore point has been cleared. + */ +async function voidRestorePoint(subscriptionId) { + await setRestorePoint(subscriptionId, null, null, false) +} + module.exports = { updateAdmin: callbackify(updateAdmin), syncSubscription: callbackify(syncSubscription), @@ -500,6 +548,9 @@ module.exports = { restoreSubscription: callbackify(restoreSubscription), updateSubscriptionFromRecurly: callbackify(updateSubscriptionFromRecurly), scheduleRefreshFeatures: callbackify(scheduleRefreshFeatures), + setSubscriptionRestorePoint: callbackify(setRestorePoint), + setSubscriptionWasReverted: callbackify(setSubscriptionWasReverted), + voidRestorePoint: callbackify(voidRestorePoint), promises: { updateAdmin, syncSubscription, @@ -514,5 +565,8 @@ module.exports = { restoreSubscription, updateSubscriptionFromRecurly, scheduleRefreshFeatures, + setRestorePoint, + setSubscriptionWasReverted, + voidRestorePoint, }, } diff --git a/services/web/app/src/models/Subscription.js b/services/web/app/src/models/Subscription.js index 92a7739515..4a5fed6f1f 100644 --- a/services/web/app/src/models/Subscription.js +++ b/services/web/app/src/models/Subscription.js @@ -25,6 +25,13 @@ const SubscriptionSchema = new Schema( invited_emails: [String], teamInvites: [TeamInviteSchema], recurlySubscription_id: String, + lastSuccesfulSubscription: { + planCode: { + type: String, + }, + addOns: Schema.Types.Mixed, + }, + timesRevertedDueToFailedPayment: { type: Number, default: 0 }, teamName: { type: String }, teamNotice: { type: String }, planCode: { type: String },