Merge pull request #14130 from overleaf/ab-cancel-reactivate-sub-sync-status

[web] Update subscription from Recurly when canceling/reactivating

GitOrigin-RevId: 7ba9a3d8ee41efa3435ef6d8b29c7b71f008c069

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,23 +178,15 @@ 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'
-        )
-      }
+async function cancelSubscription(user) {
+  try {
+    const { hasSubscription, subscription } =
+      await LimitationsManager.promises.userHasV2Subscription(user)
     if (hasSubscription) {
-        RecurlyClient.cancelSubscriptionByUuid(
-          subscription.recurlySubscription_id,
-          function (error) {
-            if (error) {
-              return callback(error)
-            }
+      await RecurlyClient.promises.cancelSubscriptionByUuid(
+        subscription.recurlySubscription_id
+      )
+      await _updateSubscriptionFromRecurly(subscription)
       const emailOpts = {
         to: user.email,
         first_name: user.first_name,
@@ -204,33 +197,24 @@ function cancelSubscription(user, callback) {
         emailOpts,
         ONE_HOUR_IN_MS
       )
-            callback()
     }
-        )
-      } else {
-        callback()
-      }
-    }
-  )
-}
-
-function reactivateSubscription(user, callback) {
-  LimitationsManager.userHasV2Subscription(
-    user,
-    function (err, hasSubscription, subscription) {
-      if (err) {
+  } catch (err) {
     logger.warn(
-          { err, userId: user._id, hasSubscription },
+      { err, userId: user._id },
       'there was an error checking user v2 subscription'
     )
   }
+}
+
+async function reactivateSubscription(user) {
+  try {
+    const { hasSubscription, subscription } =
+      await LimitationsManager.promises.userHasV2Subscription(user)
     if (hasSubscription) {
-        RecurlyClient.reactivateSubscriptionByUuid(
-          subscription.recurlySubscription_id,
-          function (error) {
-            if (error) {
-              return callback(error)
-            }
+      await RecurlyClient.promises.reactivateSubscriptionByUuid(
+        subscription.recurlySubscription_id
+      )
+      await _updateSubscriptionFromRecurly(subscription)
       EmailHandler.sendEmail(
         'reactivatedSubscription',
         { to: user.email },
@@ -243,14 +227,13 @@ function reactivateSubscription(user, callback) {
           }
         }
       )
-            callback()
     }
+  } catch (err) {
+    logger.warn(
+      { err, userId: user._id },
+      'there was an error checking user v2 subscription'
     )
-      } else {
-        callback()
   }
-    }
-  )
 }
 
 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),

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)
       })