From ae1ac83fc543ae955266289c5b340909d4f0039f Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Wed, 31 Jul 2024 11:16:25 +0100 Subject: [PATCH] Fixed members import-with-tier alpha creating unexpected invoices (#20695) ref https://linear.app/tryghost/issue/ONC-199 The `updateSubscriptionItemPrice()` method in our Stripe library used by the importer when moving a subscription over to a Ghost product/price was setting `proration_behavior: 'always_invoice'`. This resulted in invoices being created when changing the subscription (even though no prices were changing as far as the customer is concerned) and in some cases where a customer previously had a one-off discount the customer was incorrectly charged the proration difference because the discount was no longer applied to the new invoice. - updated `updateSubscriptionItemPrice()` to accept an `options` param allowing the `proration_behavior` property passed to the Stripe API to be overridden on a per-call basis - updated the `forceStripeSubscriptionToProduct()` method used by the importer to pass an options object with `prorationBehavior: 'none'` when updating the subscription item price so that no invoice and no unexpected charges occur when importing --- .../lib/MembersCSVImporterStripeUtils.js | 6 ++++-- .../test/MembersCSVImporterStripeUtils.test.js | 6 ++++-- ghost/stripe/lib/StripeAPI.js | 9 ++++++--- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/ghost/members-importer/lib/MembersCSVImporterStripeUtils.js b/ghost/members-importer/lib/MembersCSVImporterStripeUtils.js index 82f19adb40..113d97295a 100644 --- a/ghost/members-importer/lib/MembersCSVImporterStripeUtils.js +++ b/ghost/members-importer/lib/MembersCSVImporterStripeUtils.js @@ -153,7 +153,8 @@ module.exports = class MembersCSVImporterStripeUtils { await this._stripeAPIService.updateSubscriptionItemPrice( stripeSubscription.id, stripeSubscriptionItem.id, - newStripePrice.id + newStripePrice.id, + {prorationBehavior: 'none'} ); stripePriceId = newStripePrice.id; @@ -167,7 +168,8 @@ module.exports = class MembersCSVImporterStripeUtils { await this._stripeAPIService.updateSubscriptionItemPrice( stripeSubscription.id, stripeSubscriptionItem.id, - stripePriceId + stripePriceId, + {prorationBehavior: 'none'} ); } } diff --git a/ghost/members-importer/test/MembersCSVImporterStripeUtils.test.js b/ghost/members-importer/test/MembersCSVImporterStripeUtils.test.js index 9b1176613d..f76159c72e 100644 --- a/ghost/members-importer/test/MembersCSVImporterStripeUtils.test.js +++ b/ghost/members-importer/test/MembersCSVImporterStripeUtils.test.js @@ -304,7 +304,8 @@ describe('MembersCSVImporterStripeUtils', function () { stripeAPIServiceStub.updateSubscriptionItemPrice.calledWithExactly( stripeCustomer.subscriptions.data[0].id, stripeCustomerSubscriptionItem.id, - GHOST_PRODUCT_STRIPE_PRICE_ID + GHOST_PRODUCT_STRIPE_PRICE_ID, + {prorationBehavior: 'none'} ).should.be.true(); }); @@ -346,7 +347,8 @@ describe('MembersCSVImporterStripeUtils', function () { stripeAPIServiceStub.updateSubscriptionItemPrice.calledWithExactly( stripeCustomer.subscriptions.data[0].id, stripeCustomerSubscriptionItem.id, - NEW_STRIPE_PRICE_ID + NEW_STRIPE_PRICE_ID, + {prorationBehavior: 'none'} ).should.be.true(); }); diff --git a/ghost/stripe/lib/StripeAPI.js b/ghost/stripe/lib/StripeAPI.js index 504b8b0c1f..999e5a0155 100644 --- a/ghost/stripe/lib/StripeAPI.js +++ b/ghost/stripe/lib/StripeAPI.js @@ -698,20 +698,23 @@ module.exports = class StripeAPI { * @param {string} subscriptionId - The ID of the Subscription to modify * @param {string} id - The ID of the SubscriptionItem * @param {string} price - The ID of the new Price + * @param {object} [options={}] - Additional data to set on the subscription object + * @param {('always_invoice'|'create_prorations'|'none')} [options.prorationBehavior='always_invoice'] - The proration behavior to use. See [Stripe docs](https://docs.stripe.com/api/subscriptions/update#update_subscription-proration_behavior) for more info + * @param {string} [options.cancellationReason=null] - The user defined cancellation reason * * @returns {Promise} */ - async updateSubscriptionItemPrice(subscriptionId, id, price) { + async updateSubscriptionItemPrice(subscriptionId, id, price, options = {}) { await this._rateLimitBucket.throttle(); const subscription = await this._stripe.subscriptions.update(subscriptionId, { - proration_behavior: 'always_invoice', + proration_behavior: options.prorationBehavior || 'always_invoice', items: [{ id, price }], cancel_at_period_end: false, metadata: { - cancellation_reason: null + cancellation_reason: options.cancellationReason ?? null } }); return subscription;