From 114e9bc9c8aba6930625a399b757860b038b91b3 Mon Sep 17 00:00:00 2001 From: Alexandre Bourdin Date: Wed, 9 Aug 2023 11:07:10 +0200 Subject: [PATCH] Merge pull request #14130 from overleaf/ab-cancel-reactivate-sub-sync-status [web] Update subscription from Recurly when canceling/reactivating GitOrigin-RevId: 7ba9a3d8ee41efa3435ef6d8b29c7b71f008c069 --- .../Subscription/SubscriptionHandler.js | 136 +++++++++--------- .../Subscription/SubscriptionHandlerTests.js | 63 ++++---- 2 files changed, 103 insertions(+), 96 deletions(-) diff --git a/services/web/app/src/Features/Subscription/SubscriptionHandler.js b/services/web/app/src/Features/Subscription/SubscriptionHandler.js index 6b04c96f46..9c598accb1 100644 --- a/services/web/app/src/Features/Subscription/SubscriptionHandler.js +++ b/services/web/app/src/Features/Subscription/SubscriptionHandler.js @@ -9,6 +9,7 @@ const LimitationsManager = require('./LimitationsManager') const EmailHandler = require('../Email/EmailHandler') const PlansLocator = require('./PlansLocator') const SubscriptionHelper = require('./SubscriptionHelper') +const { callbackify } = require('../../util/promises') function validateNoSubscriptionInRecurly(userId, callback) { RecurlyWrapper.listAccountActiveSubscriptions( @@ -177,80 +178,62 @@ function cancelPendingSubscriptionChange(user, callback) { ) } -function cancelSubscription(user, callback) { - LimitationsManager.userHasV2Subscription( - user, - function (err, hasSubscription, subscription) { - if (err) { - logger.warn( - { err, userId: user._id, hasSubscription }, - 'there was an error checking user v2 subscription' - ) - } - if (hasSubscription) { - RecurlyClient.cancelSubscriptionByUuid( - subscription.recurlySubscription_id, - function (error) { - if (error) { - return callback(error) - } - const emailOpts = { - to: user.email, - first_name: user.first_name, - } - const ONE_HOUR_IN_MS = 1000 * 60 * 60 - EmailHandler.sendDeferredEmail( - 'canceledSubscription', - emailOpts, - ONE_HOUR_IN_MS - ) - callback() - } - ) - } else { - callback() +async function cancelSubscription(user) { + try { + const { hasSubscription, subscription } = + await LimitationsManager.promises.userHasV2Subscription(user) + if (hasSubscription) { + await RecurlyClient.promises.cancelSubscriptionByUuid( + subscription.recurlySubscription_id + ) + await _updateSubscriptionFromRecurly(subscription) + const emailOpts = { + to: user.email, + first_name: user.first_name, } + const ONE_HOUR_IN_MS = 1000 * 60 * 60 + EmailHandler.sendDeferredEmail( + 'canceledSubscription', + emailOpts, + ONE_HOUR_IN_MS + ) } - ) + } catch (err) { + logger.warn( + { err, userId: user._id }, + 'there was an error checking user v2 subscription' + ) + } } -function reactivateSubscription(user, callback) { - LimitationsManager.userHasV2Subscription( - user, - function (err, hasSubscription, subscription) { - if (err) { - logger.warn( - { err, userId: user._id, hasSubscription }, - 'there was an error checking user v2 subscription' - ) - } - if (hasSubscription) { - RecurlyClient.reactivateSubscriptionByUuid( - subscription.recurlySubscription_id, - function (error) { - if (error) { - return callback(error) - } - EmailHandler.sendEmail( - 'reactivatedSubscription', - { to: user.email }, - err => { - if (err) { - logger.warn( - { err }, - 'failed to send reactivation confirmation email' - ) - } - } +async function reactivateSubscription(user) { + try { + const { hasSubscription, subscription } = + await LimitationsManager.promises.userHasV2Subscription(user) + if (hasSubscription) { + await RecurlyClient.promises.reactivateSubscriptionByUuid( + subscription.recurlySubscription_id + ) + await _updateSubscriptionFromRecurly(subscription) + EmailHandler.sendEmail( + 'reactivatedSubscription', + { to: user.email }, + err => { + if (err) { + logger.warn( + { err }, + 'failed to send reactivation confirmation email' ) - callback() } - ) - } else { - callback() - } + } + ) } - ) + } catch (err) { + logger.warn( + { err, userId: user._id }, + 'there was an error checking user v2 subscription' + ) + } } function syncSubscription(recurlySubscription, requesterData, callback) { @@ -323,13 +306,24 @@ function extendTrial(subscription, daysToExend, callback) { ) } +async function _updateSubscriptionFromRecurly(subscription) { + const recurlySubscription = await RecurlyWrapper.promises.getSubscription( + subscription.recurlySubscription_id, + {} + ) + await SubscriptionUpdater.promises.updateSubscriptionFromRecurly( + recurlySubscription, + subscription + ) +} + module.exports = { validateNoSubscriptionInRecurly, createSubscription, updateSubscription, cancelPendingSubscriptionChange, - cancelSubscription, - reactivateSubscription, + cancelSubscription: callbackify(cancelSubscription), + reactivateSubscription: callbackify(reactivateSubscription), syncSubscription, attemptPaypalInvoiceCollection, extendTrial, @@ -338,8 +332,8 @@ module.exports = { createSubscription: promisify(createSubscription), updateSubscription: promisify(updateSubscription), cancelPendingSubscriptionChange: promisify(cancelPendingSubscriptionChange), - cancelSubscription: promisify(cancelSubscription), - reactivateSubscription: promisify(reactivateSubscription), + cancelSubscription, + reactivateSubscription, syncSubscription: promisify(syncSubscription), attemptPaypalInvoiceCollection: promisify(attemptPaypalInvoiceCollection), extendTrial: promisify(extendTrial), diff --git a/services/web/test/unit/src/Subscription/SubscriptionHandlerTests.js b/services/web/test/unit/src/Subscription/SubscriptionHandlerTests.js index a8b1512429..7510d1c644 100644 --- a/services/web/test/unit/src/Subscription/SubscriptionHandlerTests.js +++ b/services/web/test/unit/src/Subscription/SubscriptionHandlerTests.js @@ -89,6 +89,9 @@ describe('SubscriptionHandler', function () { getAccountPastDueInvoices: sinon.stub().yields(), attemptInvoiceCollection: sinon.stub().yields(), listAccountActiveSubscriptions: sinon.stub().yields(null, []), + promises: { + getSubscription: sinon.stub().resolves(this.activeRecurlySubscription), + }, } this.RecurlyClient = { changeSubscriptionByUuid: sinon @@ -101,14 +104,28 @@ describe('SubscriptionHandler', function () { .stub() .yields(null, this.activeRecurlyClientSubscription), cancelSubscriptionByUuid: sinon.stub().yields(), + promises: { + reactivateSubscriptionByUuid: sinon + .stub() + .resolves(this.activeRecurlyClientSubscription), + cancelSubscriptionByUuid: sinon.stub().resolves(), + }, } this.SubscriptionUpdater = { syncSubscription: sinon.stub().yields(), startFreeTrial: sinon.stub().callsArgWith(1), + promises: { + updateSubscriptionFromRecurly: sinon.stub().resolves(), + }, } - this.LimitationsManager = { userHasV2Subscription: sinon.stub() } + this.LimitationsManager = { + userHasV2Subscription: sinon.stub(), + promises: { + userHasV2Subscription: sinon.stub().resolves(), + }, + } this.EmailHandler = { sendEmail: sinon.stub(), @@ -377,7 +394,7 @@ describe('SubscriptionHandler', function () { describe('cancelSubscription', function () { describe('with a user without a subscription', function () { beforeEach(function (done) { - this.LimitationsManager.userHasV2Subscription.callsArgWith( + this.LimitationsManager.promises.userHasV2Subscription.callsArgWith( 1, null, false, @@ -393,18 +410,18 @@ describe('SubscriptionHandler', function () { describe('with a user with a subscription', function () { beforeEach(function (done) { - this.LimitationsManager.userHasV2Subscription.callsArgWith( - 1, - null, - true, - this.subscription - ) + this.LimitationsManager.promises.userHasV2Subscription.resolves({ + hasSubscription: true, + subscription: this.subscription, + }) this.SubscriptionHandler.cancelSubscription(this.user, done) }) it('should cancel the subscription', function () { - this.RecurlyClient.cancelSubscriptionByUuid.called.should.equal(true) - this.RecurlyClient.cancelSubscriptionByUuid + this.RecurlyClient.promises.cancelSubscriptionByUuid.called.should.equal( + true + ) + this.RecurlyClient.promises.cancelSubscriptionByUuid .calledWith(this.subscription.recurlySubscription_id) .should.equal(true) }) @@ -420,15 +437,13 @@ describe('SubscriptionHandler', function () { }) }) - describe('reactiveRecurlySubscription', function () { + describe('reactivateSubscription', function () { describe('with a user without a subscription', function () { beforeEach(function (done) { - this.LimitationsManager.userHasV2Subscription.callsArgWith( - 1, - null, - false, - this.subscription - ) + this.LimitationsManager.promises.userHasV2Subscription.resolves({ + hasSubscription: false, + subscription: this.subscription, + }) this.SubscriptionHandler.reactivateSubscription(this.user, done) }) @@ -445,20 +460,18 @@ describe('SubscriptionHandler', function () { describe('with a user with a subscription', function () { beforeEach(function (done) { - this.LimitationsManager.userHasV2Subscription.callsArgWith( - 1, - null, - true, - this.subscription - ) + this.LimitationsManager.promises.userHasV2Subscription.resolves({ + hasSubscription: true, + subscription: this.subscription, + }) this.SubscriptionHandler.reactivateSubscription(this.user, done) }) it('should reactivate the subscription', function () { - this.RecurlyClient.reactivateSubscriptionByUuid.called.should.equal( + this.RecurlyClient.promises.reactivateSubscriptionByUuid.called.should.equal( true ) - this.RecurlyClient.reactivateSubscriptionByUuid + this.RecurlyClient.promises.reactivateSubscriptionByUuid .calledWith(this.subscription.recurlySubscription_id) .should.equal(true) })