Merge pull request #2638 from overleaf/ta-subscription-change-admin-fix

Change Manager When Changing Admin of Individual Subscriptions

GitOrigin-RevId: 3da53789f4331c4eb8d0a458650ea41f8b2b4f97

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

@@ -12,7 +12,10 @@ const { DeletedSubscription } = require('../../models/DeletedSubscription')
 
 const SubscriptionUpdater = {
   /**
-   * Change the admin of the given subscription
+   * Change the admin of the given subscription.
+   *
+   * If the subscription is a group, add the new admin as manager while keeping
+   * the old admin. Otherwise, replace the manager.
    *
    * Validation checks are assumed to have been made:
    *   * subscription exists
@@ -22,14 +25,18 @@ const SubscriptionUpdater = {
    *
    * If the subscription is Recurly, we silently do nothing.
    */
-  updateAdmin(subscriptionId, adminId, callback) {
+  updateAdmin(subscription, adminId, callback) {
     const query = {
-      _id: ObjectId(subscriptionId),
+      _id: ObjectId(subscription._id),
       customAccount: true
     }
     const update = {
-      $set: { admin_id: ObjectId(adminId) },
-      $addToSet: { manager_ids: ObjectId(adminId) }
+      $set: { admin_id: ObjectId(adminId) }
+    }
+    if (subscription.groupPlan) {
+      update.$addToSet = { manager_ids: ObjectId(adminId) }
+    } else {
+      update.$set.manager_ids = [ObjectId(adminId)]
     }
     Subscription.update(query, update, callback)
   },

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

@@ -120,8 +120,9 @@ describe('SubscriptionUpdater', function() {
 
   describe('updateAdmin', function() {
     it('should update the subscription admin', function(done) {
+      this.subscription.groupPlan = true
       this.SubscriptionUpdater.updateAdmin(
-        this.subscription._id,
+        this.subscription,
         this.otherUserId,
         err => {
           if (err != null) {
@@ -144,6 +145,34 @@ describe('SubscriptionUpdater', function() {
         }
       )
     })
+
+    it('should remove the manager for non-group subscriptions', function(done) {
+      this.SubscriptionUpdater.updateAdmin(
+        this.subscription,
+        this.otherUserId,
+        err => {
+          if (err != null) {
+            return done(err)
+          }
+          const query = {
+            _id: ObjectId(this.subscription._id),
+            customAccount: true
+          }
+          const update = {
+            $set: {
+              admin_id: ObjectId(this.otherUserId),
+              manager_ids: [ObjectId(this.otherUserId)]
+            }
+          }
+          this.SubscriptionModel.update.should.have.been.calledOnce
+          this.SubscriptionModel.update.should.have.been.calledWith(
+            query,
+            update
+          )
+          done()
+        }
+      )
+    })
   })
 
   describe('syncSubscription', function() {