Merge pull request #2139 from overleaf/ta-subscription-restore-misc

Restore Deleted Subscriptions Extra

GitOrigin-RevId: 6fceb1142af8ba510dc266ef31acc86ddca69f0e

services/web/app/src/Features/Subscription/SubscriptionUpdater.js

@@ -1,6 +1,5 @@
 const { db } = require('../../infrastructure/mongojs')
 const async = require('async')
-const _ = require('underscore')
 const { promisifyAll } = require('../../util/promises')
 const { Subscription } = require('../../models/Subscription')
 const SubscriptionLocator = require('./SubscriptionLocator')
@@ -184,40 +183,28 @@ const SubscriptionUpdater = {
     if (callback == null) {
       callback = function() {}
     }
-    SubscriptionLocator.getSubscription(subscription._id, function(
-      err,
-      subscription
-    ) {
-      if (err != null) {
-        return callback(err)
-      }
-      const affectedUserIds = [subscription.admin_id].concat(
-        subscription.member_ids || []
-      )
-      logger.log(
-        { subscriptionId: subscription._id, affectedUserIds },
-        'deleting subscription and downgrading users'
-      )
-      SubscriptionUpdater._createDeletedSubscription(
-        subscription,
-        deleterData,
-        error => {
-          if (error) {
-            return callback(error)
-          }
-          Subscription.remove({ _id: subscription._id }, function(err) {
-            if (err != null) {
-              return callback(err)
-            }
-            async.mapSeries(
-              affectedUserIds,
-              FeaturesUpdater.refreshFeatures,
-              callback
-            )
-          })
-        }
-      )
-    })
+    logger.log(
+      { subscriptionId: subscription._id },
+      'deleting subscription and downgrading users'
+    )
+    async.series(
+      [
+        cb =>
+          // 1. create deletedSubscription
+          SubscriptionUpdater._createDeletedSubscription(
+            subscription,
+            deleterData,
+            cb
+          ),
+        cb =>
+          // 2. remove subscription
+          Subscription.remove({ _id: subscription._id }, cb),
+        cb =>
+          // 3. refresh users features
+          SubscriptionUpdater._refreshUsersFeatures(subscription, cb)
+      ],
+      callback
+    )
   },
 
   restoreSubscription(subscriptionId, callback) {
@@ -240,21 +227,22 @@ const SubscriptionUpdater = {
               cb
             ),
           cb =>
-            // 2. remove deleted subscription
+            // 2. refresh users features. Do this before removing the
+            //    subscription so the restore can be retried if this fails
+            SubscriptionUpdater._refreshUsersFeatures(subscription, cb),
+          cb =>
+            // 3. remove deleted subscription
             DeletedSubscription.deleteOne(
               { 'subscription._id': subscription._id },
               callback
-            ),
-          cb =>
-            // 3. refresh users features
-            SubscriptionUpdater._refreshUsersFeature(subscription, cb)
+            )
         ],
         callback
       )
     })
   },
 
-  _refreshUsersFeature(subscription, callback) {
+  _refreshUsersFeatures(subscription, callback) {
     const userIds = [subscription.admin_id].concat(
       subscription.member_ids || []
     )
@@ -311,12 +299,11 @@ const SubscriptionUpdater = {
       subscription.groupPlan = true
       subscription.membersLimit = plan.membersLimit
     }
-    subscription.save(function() {
-      const allIds = _.union(subscription.member_ids, [subscription.admin_id])
-      const jobs = allIds.map(userId => cb =>
-        FeaturesUpdater.refreshFeatures(userId, cb)
-      )
-      async.series(jobs, callback)
+    subscription.save(function(error) {
+      if (error) {
+        return callback(error)
+      }
+      SubscriptionUpdater._refreshUsersFeatures(subscription, callback)
     })
   }
 }

services/web/test/acceptance/src/SubscriptionDeletionTests.js

@@ -22,11 +22,13 @@ describe('Subscriptions', function() {
               invitedEmails: ['foo@bar.com'],
               teamInvites: [{ email: 'foo@baz.com' }],
               groupPlan: true,
-              state: 'expired'
+              state: 'expired',
+              planCode: 'professional'
             })
             this.subscription = this.recurlySubscription.subscription
             this.recurlySubscription.ensureExists(cb)
-          }
+          },
+          cb => this.subscription.refreshUsersFeatures(cb)
         ],
         done
       )
@@ -45,6 +47,21 @@ describe('Subscriptions', function() {
       })
     })
 
+    it('refresh features', function(done) {
+      let url = '/user/subscription/callback'
+      let body = this.recurlySubscription.buildCallbackXml()
+
+      request.post({ url, body }, (error, { statusCode }) => {
+        if (error) {
+          return done(error)
+        }
+        this.memberUser.getFeatures((error, features) => {
+          expect(features.collaborators).to.equal(1)
+          done(error)
+        })
+      })
+    })
+
     it('allows deletion when deletedSubscription exists', function(done) {
       let url = '/user/subscription/callback'
       let body = this.recurlySubscription.buildCallbackXml()

services/web/test/acceptance/src/helpers/Subscription.js

@@ -1,5 +1,6 @@
 const { ObjectId } = require('../../../../app/src/infrastructure/mongojs')
 const { expect } = require('chai')
+const SubscriptionUpdater = require('../../../../app/src/Features/Subscription/SubscriptionUpdater')
 const SubscriptionModel = require('../../../../app/src/models/Subscription')
   .Subscription
 const DeletedSubscriptionModel = require(`../../../../app/src/models/DeletedSubscription`)
@@ -14,6 +15,7 @@ class Subscription {
     this.member_ids = options.memberIds || []
     this.invited_emails = options.invitedEmails || []
     this.teamInvites = options.teamInvites || []
+    this.planCode = options.planCode
   }
 
   ensureExists(callback) {
@@ -40,6 +42,10 @@ class Subscription {
     )
   }
 
+  refreshUsersFeatures(callback) {
+    SubscriptionUpdater._refreshUsersFeatures(this, callback)
+  }
+
   expectDeleted(deleterData, callback) {
     DeletedSubscriptionModel.find(
       { 'subscription._id': this._id },

services/web/test/acceptance/src/helpers/User.js

@@ -326,6 +326,15 @@ class User {
     )
   }
 
+  getFeatures(callback) {
+    const features = settings.defaultFeatures
+    return db.users.findOne(
+      { _id: ObjectId(this.id) },
+      { features: 1 },
+      (error, user) => callback(error, user && user.features)
+    )
+  }
+
   full_delete_user(email, callback) {
     if (callback == null) {
       callback = function(error) {}

services/web/test/unit/src/Subscription/SubscriptionUpdaterTests.js

@@ -465,12 +465,6 @@ describe('SubscriptionUpdater', function() {
       this.SubscriptionUpdater.deleteSubscription(this.subscription, {}, done)
     })
 
-    it('should look up the subscription', function() {
-      this.SubscriptionLocator.getSubscription
-        .calledWith(this.subscription._id)
-        .should.equal(true)
-    })
-
     it('should remove the subscription', function() {
       this.SubscriptionModel.remove
         .calledWith({ _id: this.subscription._id })