Merge pull request #1833 from overleaf/em-edit-subscription-admin

Change admin button in subscription admin page

GitOrigin-RevId: d446f54299578c3806ef7146d2163ec24e831b6d

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

@@ -1,39 +1,43 @@
-/* eslint-disable
-    camelcase,
-    handle-callback-err,
-    max-len,
-    no-undef,
-    no-unused-vars,
-*/
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS102: Remove unnecessary code created because of implicit returns
- * DS207: Consider shorter variations of null checks
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
-let SubscriptionUpdater
 const async = require('async')
 const _ = require('underscore')
 const { Subscription } = require('../../models/Subscription')
 const SubscriptionLocator = require('./SubscriptionLocator')
 const UserGetter = require('../User/UserGetter')
 const PlansLocator = require('./PlansLocator')
-const Settings = require('settings-sharelatex')
 const logger = require('logger-sharelatex')
 const { ObjectId } = require('mongoose').Types
 const FeaturesUpdater = require('./FeaturesUpdater')
 
-const oneMonthInSeconds = 60 * 60 * 24 * 30
+const SubscriptionUpdater = {
+  /**
+   * Change the admin of the given subscription
+   *
+   * Validation checks are assumed to have been made:
+   *   * subscription exists
+   *   * user exists
+   *   * user does not have another subscription
+   *   * subscription is not a Recurly subscription
+   *
+   * If the subscription is Recurly, we silently do nothing.
+   */
+  updateAdmin(subscriptionId, adminId, callback) {
+    const query = {
+      _id: ObjectId(subscriptionId),
+      customAccount: true
+    }
+    const update = {
+      $set: { admin_id: ObjectId(adminId) },
+      $addToSet: { manager_ids: ObjectId(adminId) }
+    }
+    Subscription.update(query, update, callback)
+  },
 
-module.exports = SubscriptionUpdater = {
-  syncSubscription(recurlySubscription, adminUser_id, callback) {
+  syncSubscription(recurlySubscription, adminUserId, callback) {
     logger.log(
-      { adminUser_id, recurlySubscription },
+      { adminUserId, recurlySubscription },
       'syncSubscription, creating new if subscription does not exist'
     )
-    return SubscriptionLocator.getUsersSubscription(adminUser_id, function(
+    SubscriptionLocator.getUsersSubscription(adminUserId, function(
       err,
       subscription
     ) {
@@ -42,42 +46,42 @@ module.exports = SubscriptionUpdater = {
       }
       if (subscription != null) {
         logger.log(
-          { adminUser_id, recurlySubscription },
+          { adminUserId, recurlySubscription },
           'subscription does exist'
         )
-        return SubscriptionUpdater._updateSubscriptionFromRecurly(
+        SubscriptionUpdater._updateSubscriptionFromRecurly(
           recurlySubscription,
           subscription,
           callback
         )
       } else {
         logger.log(
-          { adminUser_id, recurlySubscription },
+          { adminUserId, recurlySubscription },
           'subscription does not exist, creating a new one'
         )
-        return SubscriptionUpdater._createNewSubscription(
-          adminUser_id,
-          function(err, subscription) {
-            if (err != null) {
-              return callback(err)
-            }
-            return SubscriptionUpdater._updateSubscriptionFromRecurly(
-              recurlySubscription,
-              subscription,
-              callback
-            )
+        SubscriptionUpdater._createNewSubscription(adminUserId, function(
+          err,
+          subscription
+        ) {
+          if (err != null) {
+            return callback(err)
           }
-        )
+          SubscriptionUpdater._updateSubscriptionFromRecurly(
+            recurlySubscription,
+            subscription,
+            callback
+          )
+        })
       }
     })
   },
 
   addUserToGroup(subscriptionId, userId, callback) {
-    return this.addUsersToGroup(subscriptionId, [userId], callback)
+    this.addUsersToGroup(subscriptionId, [userId], callback)
   },
 
   addUsersToGroup(subscriptionId, memberIds, callback) {
-    return this.addUsersToGroupWithoutFeaturesRefresh(
+    this.addUsersToGroupWithoutFeaturesRefresh(
       subscriptionId,
       memberIds,
       function(err) {
@@ -86,13 +90,13 @@ module.exports = SubscriptionUpdater = {
         }
 
         // Only apply features updates to users, not user stubs
-        return UserGetter.getUsers(memberIds, { _id: 1 }, function(err, users) {
+        UserGetter.getUsers(memberIds, { _id: 1 }, function(err, users) {
           if (err != null) {
             return callback(err)
           }
 
           const userIds = users.map(u => u._id.toString())
-          return async.map(userIds, FeaturesUpdater.refreshFeatures, callback)
+          async.map(userIds, FeaturesUpdater.refreshFeatures, callback)
         })
       }
     )
@@ -106,20 +110,20 @@ module.exports = SubscriptionUpdater = {
     const searchOps = { _id: subscriptionId }
     const insertOperation = { $addToSet: { member_ids: { $each: memberIds } } }
 
-    return Subscription.findAndModify(searchOps, insertOperation, callback)
+    Subscription.findAndModify(searchOps, insertOperation, callback)
   },
 
-  removeUserFromGroups(filter, user_id, callback) {
-    const removeOperation = { $pull: { member_ids: user_id } }
-    return Subscription.updateMany(filter, removeOperation, function(err) {
+  removeUserFromGroups(filter, userId, callback) {
+    const removeOperation = { $pull: { member_ids: userId } }
+    Subscription.updateMany(filter, removeOperation, function(err) {
       if (err != null) {
         logger.err(
-          { err, searchOps, removeOperation },
+          { err, filter, removeOperation },
           'error removing user from groups'
         )
         return callback(err)
       }
-      return UserGetter.getUserOrUserStubById(user_id, {}, function(
+      UserGetter.getUserOrUserStubById(userId, {}, function(
         error,
         user,
         isStub
@@ -130,21 +134,21 @@ module.exports = SubscriptionUpdater = {
         if (isStub) {
           return callback()
         }
-        return FeaturesUpdater.refreshFeatures(user_id, callback)
+        FeaturesUpdater.refreshFeatures(userId, callback)
       })
     })
   },
 
-  removeUserFromGroup(subscriptionId, user_id, callback) {
-    return SubscriptionUpdater.removeUserFromGroups(
+  removeUserFromGroup(subscriptionId, userId, callback) {
+    SubscriptionUpdater.removeUserFromGroups(
       { _id: subscriptionId },
-      user_id,
+      userId,
       callback
     )
   },
 
-  removeUserFromAllGroups(user_id, callback) {
-    return SubscriptionLocator.getMemberSubscriptions(user_id, function(
+  removeUserFromAllGroups(userId, callback) {
+    SubscriptionLocator.getMemberSubscriptions(userId, function(
       error,
       subscriptions
     ) {
@@ -155,44 +159,42 @@ module.exports = SubscriptionUpdater = {
         return callback()
       }
       const subscriptionIds = subscriptions.map(sub => sub._id)
-      return SubscriptionUpdater.removeUserFromGroups(
+      SubscriptionUpdater.removeUserFromGroups(
         { _id: subscriptionIds },
-        user_id,
+        userId,
         callback
       )
     })
   },
 
   deleteWithV1Id(v1TeamId, callback) {
-    return Subscription.deleteOne({ 'overleaf.id': v1TeamId }, callback)
+    Subscription.deleteOne({ 'overleaf.id': v1TeamId }, callback)
   },
 
-  deleteSubscription(subscription_id, callback) {
+  deleteSubscription(subscriptionId, callback) {
     if (callback == null) {
-      callback = function(error) {}
+      callback = function() {}
     }
-    return SubscriptionLocator.getSubscription(subscription_id, function(
+    SubscriptionLocator.getSubscription(subscriptionId, function(
       err,
       subscription
     ) {
       if (err != null) {
         return callback(err)
       }
-      const affected_user_ids = [subscription.admin_id].concat(
+      const affectedUserIds = [subscription.admin_id].concat(
         subscription.member_ids || []
       )
       logger.log(
-        { subscription_id, affected_user_ids },
+        { subscriptionId, affectedUserIds },
         'deleting subscription and downgrading users'
       )
-      return Subscription.remove({ _id: ObjectId(subscription_id) }, function(
-        err
-      ) {
+      Subscription.remove({ _id: ObjectId(subscriptionId) }, function(err) {
         if (err != null) {
           return callback(err)
         }
-        return async.mapSeries(
-          affected_user_ids,
+        async.mapSeries(
+          affectedUserIds,
           FeaturesUpdater.refreshFeatures,
           callback
         )
@@ -200,13 +202,13 @@ module.exports = SubscriptionUpdater = {
     })
   },
 
-  _createNewSubscription(adminUser_id, callback) {
-    logger.log({ adminUser_id }, 'creating new subscription')
+  _createNewSubscription(adminUserId, callback) {
+    logger.log({ adminUserId }, 'creating new subscription')
     const subscription = new Subscription({
-      admin_id: adminUser_id,
-      manager_ids: [adminUser_id]
+      admin_id: adminUserId,
+      manager_ids: [adminUserId]
     })
-    return subscription.save(err => callback(err, subscription))
+    subscription.save(err => callback(err, subscription))
   },
 
   _updateSubscriptionFromRecurly(recurlySubscription, subscription, callback) {
@@ -226,12 +228,14 @@ module.exports = SubscriptionUpdater = {
       subscription.groupPlan = true
       subscription.membersLimit = plan.membersLimit
     }
-    return subscription.save(function() {
+    subscription.save(function() {
       const allIds = _.union(subscription.member_ids, [subscription.admin_id])
-      const jobs = allIds.map(user_id => cb =>
-        FeaturesUpdater.refreshFeatures(user_id, cb)
+      const jobs = allIds.map(userId => cb =>
+        FeaturesUpdater.refreshFeatures(userId, cb)
       )
-      return async.series(jobs, callback)
+      async.series(jobs, callback)
     })
   }
 }
+
+module.exports = SubscriptionUpdater

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

@@ -1,31 +1,15 @@
-/* eslint-disable
-    camelcase,
-    handle-callback-err,
-    max-len,
-    no-return-assign,
-    no-unused-vars,
-*/
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS101: Remove unnecessary use of Array.from
- * DS102: Remove unnecessary code created because of implicit returns
- * DS206: Consider reworking classes to avoid initClass
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
 const SandboxedModule = require('sandboxed-module')
-const should = require('chai').should()
-const { expect } = require('chai')
+const chai = require('chai')
 const sinon = require('sinon')
 const modulePath =
   '../../../../app/src/Features/Subscription/SubscriptionUpdater'
 const { assert } = require('chai')
 const { ObjectId } = require('mongoose').Types
 
+chai.should()
+
 describe('SubscriptionUpdater', function() {
   beforeEach(function() {
-    let subscription
     this.recurlySubscription = {
       uuid: '1238uoijdasjhd',
       plan: {
@@ -39,7 +23,7 @@ describe('SubscriptionUpdater', function() {
       _id: 'mock-user-stub-id',
       email: 'mock-stub-email@baz.com'
     }
-    this.subscription = subscription = {
+    this.subscription = {
       _id: '111111111111111111111111',
       admin_id: this.adminUser._id,
       manager_ids: [this.adminUser._id],
@@ -63,23 +47,24 @@ describe('SubscriptionUpdater', function() {
     this.findAndModifyStub = sinon
       .stub()
       .callsArgWith(2, null, this.subscription)
-    this.SubscriptionModel = (function() {
-      const Cls = class {
-        static initClass() {
-          this.remove = sinon.stub().yields()
-        }
-        constructor(opts) {
-          subscription.admin_id = opts.admin_id
-          subscription.manager_ids = [opts.admin_id]
-          return subscription
-        }
+    this.findOneAndUpdateStub = sinon
+      .stub()
+      .callsArgWith(2, null, this.subscription)
+
+    let subscription = this.subscription
+    this.SubscriptionModel = class {
+      constructor(opts) {
+        // Always return our mock subscription when creating a new one
+        subscription.admin_id = opts.admin_id
+        subscription.manager_ids = [opts.admin_id]
+        return subscription
       }
-      Cls.initClass()
-      return Cls
-    })()
+    }
+    this.SubscriptionModel.remove = sinon.stub().yields()
     this.SubscriptionModel.update = this.updateStub
     this.SubscriptionModel.updateMany = this.updateManyStub
     this.SubscriptionModel.findAndModify = this.findAndModifyStub
+    this.SubscriptionModel.findOneAndUpdate = this.findOneAndUpdateStub
 
     this.SubscriptionLocator = {
       getUsersSubscription: sinon.stub(),
@@ -99,14 +84,17 @@ describe('SubscriptionUpdater', function() {
     this.UserGetter = {
       getUsers(memberIds, projection, callback) {
         const users = memberIds.map(id => ({ _id: id }))
-        return callback(null, users)
+        callback(null, users)
       },
       getUserOrUserStubById: sinon.stub()
     }
 
     this.ReferalFeatures = { getBonusFeatures: sinon.stub().callsArgWith(1) }
     this.Modules = { hooks: { fire: sinon.stub().callsArgWith(2, null, null) } }
-    return (this.SubscriptionUpdater = SandboxedModule.require(modulePath, {
+    this.FeaturesUpdater = {
+      refreshFeatures: sinon.stub().yields()
+    }
+    this.SubscriptionUpdater = SandboxedModule.require(modulePath, {
       requires: {
         '../../models/Subscription': {
           Subscription: this.SubscriptionModel
@@ -119,9 +107,37 @@ describe('SubscriptionUpdater', function() {
           log() {}
         },
         'settings-sharelatex': this.Settings,
-        './FeaturesUpdater': (this.FeaturesUpdater = {})
+        './FeaturesUpdater': this.FeaturesUpdater
       }
-    }))
+    })
+  })
+
+  describe('updateAdmin', function() {
+    it('should update the subscription admin', function(done) {
+      this.SubscriptionUpdater.updateAdmin(
+        this.subscription._id,
+        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) },
+            $addToSet: { 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() {
@@ -131,18 +147,21 @@ describe('SubscriptionUpdater', function() {
         null,
         this.subscription
       )
-      return (this.SubscriptionUpdater._updateSubscriptionFromRecurly = sinon
+      this.SubscriptionUpdater._updateSubscriptionFromRecurly = sinon
         .stub()
-        .callsArgWith(2))
+        .callsArgWith(2)
     })
 
     it('should update the subscription if the user already is admin of one', function(done) {
       this.SubscriptionUpdater._createNewSubscription = sinon.stub()
 
-      return this.SubscriptionUpdater.syncSubscription(
+      this.SubscriptionUpdater.syncSubscription(
         this.recurlySubscription,
         this.adminUser._id,
         err => {
+          if (err != null) {
+            return done(err)
+          }
           this.SubscriptionLocator.getUsersSubscription
             .calledWith(this.adminUser._id)
             .should.equal(true)
@@ -152,16 +171,19 @@ describe('SubscriptionUpdater', function() {
           this.SubscriptionUpdater._updateSubscriptionFromRecurly
             .calledWith(this.recurlySubscription, this.subscription)
             .should.equal(true)
-          return done()
+          done()
         }
       )
     })
 
-    return it('should not call updateFeatures with group subscription if recurly subscription is not expired', function(done) {
-      return this.SubscriptionUpdater.syncSubscription(
+    it('should not call updateFeatures with group subscription if recurly subscription is not expired', function(done) {
+      this.SubscriptionUpdater.syncSubscription(
         this.recurlySubscription,
         this.adminUser._id,
         err => {
+          if (err != null) {
+            return done(err)
+          }
           this.SubscriptionLocator.getUsersSubscription
             .calledWith(this.adminUser._id)
             .should.equal(true)
@@ -172,7 +194,7 @@ describe('SubscriptionUpdater', function() {
             .calledWith(this.recurlySubscription, this.subscription)
             .should.equal(true)
           this.UserFeaturesUpdater.updateFeatures.called.should.equal(false)
-          return done()
+          done()
         }
       )
     })
@@ -181,16 +203,17 @@ describe('SubscriptionUpdater', function() {
   describe('_updateSubscriptionFromRecurly', function() {
     beforeEach(function() {
       this.FeaturesUpdater.refreshFeatures = sinon.stub().callsArgWith(1)
-      return (this.SubscriptionUpdater.deleteSubscription = sinon
-        .stub()
-        .yields())
+      this.SubscriptionUpdater.deleteSubscription = sinon.stub().yields()
     })
 
     it('should update the subscription with token etc when not expired', function(done) {
-      return this.SubscriptionUpdater._updateSubscriptionFromRecurly(
+      this.SubscriptionUpdater._updateSubscriptionFromRecurly(
         this.recurlySubscription,
         this.subscription,
         err => {
+          if (err != null) {
+            return done(err)
+          }
           this.subscription.recurlySubscription_id.should.equal(
             this.recurlySubscription.uuid
           )
@@ -201,30 +224,36 @@ describe('SubscriptionUpdater', function() {
           this.FeaturesUpdater.refreshFeatures
             .calledWith(this.adminUser._id)
             .should.equal(true)
-          return done()
+          done()
         }
       )
     })
 
     it('should remove the subscription when expired', function(done) {
       this.recurlySubscription.state = 'expired'
-      return this.SubscriptionUpdater._updateSubscriptionFromRecurly(
+      this.SubscriptionUpdater._updateSubscriptionFromRecurly(
         this.recurlySubscription,
         this.subscription,
         err => {
+          if (err != null) {
+            return done(err)
+          }
           this.SubscriptionUpdater.deleteSubscription
             .calledWith(this.subscription._id)
             .should.equal(true)
-          return done()
+          done()
         }
       )
     })
 
     it('should update all the users features', function(done) {
-      return this.SubscriptionUpdater._updateSubscriptionFromRecurly(
+      this.SubscriptionUpdater._updateSubscriptionFromRecurly(
         this.recurlySubscription,
         this.subscription,
         err => {
+          if (err != null) {
+            return done(err)
+          }
           this.FeaturesUpdater.refreshFeatures
             .calledWith(this.adminUser._id)
             .should.equal(true)
@@ -237,7 +266,7 @@ describe('SubscriptionUpdater', function() {
           this.FeaturesUpdater.refreshFeatures
             .calledWith(this.allUserIds[2])
             .should.equal(true)
-          return done()
+          done()
         }
       )
     })
@@ -246,25 +275,31 @@ describe('SubscriptionUpdater', function() {
       this.PlansLocator.findLocalPlanInSettings
         .withArgs(this.recurlySubscription.plan.plan_code)
         .returns({ groupPlan: true, membersLimit: 5 })
-      return this.SubscriptionUpdater._updateSubscriptionFromRecurly(
+      this.SubscriptionUpdater._updateSubscriptionFromRecurly(
         this.recurlySubscription,
         this.subscription,
         err => {
+          if (err != null) {
+            return done(err)
+          }
           this.subscription.membersLimit.should.equal(5)
           this.subscription.groupPlan.should.equal(true)
-          return done()
+          done()
         }
       )
     })
 
-    return it('should not set group to true or set groupPlan', function(done) {
-      return this.SubscriptionUpdater._updateSubscriptionFromRecurly(
+    it('should not set group to true or set groupPlan', function(done) {
+      this.SubscriptionUpdater._updateSubscriptionFromRecurly(
         this.recurlySubscription,
         this.subscription,
         err => {
+          if (err != null) {
+            return done(err)
+          }
           assert.notEqual(this.subscription.membersLimit, 5)
           assert.notEqual(this.subscription.groupPlan, true)
-          return done()
+          done()
         }
       )
     })
@@ -272,33 +307,31 @@ describe('SubscriptionUpdater', function() {
 
   describe('_createNewSubscription', () =>
     it('should create a new subscription then update the subscription', function(done) {
-      return this.SubscriptionUpdater._createNewSubscription(
+      this.SubscriptionUpdater._createNewSubscription(
         this.adminUser._id,
         () => {
           this.subscription.admin_id.should.equal(this.adminUser._id)
           this.subscription.manager_ids.should.deep.equal([this.adminUser._id])
           this.subscription.save.called.should.equal(true)
-          return done()
+          done()
         }
       )
     }))
 
   describe('addUserToGroup', function() {
     beforeEach(function() {
-      return (this.SubscriptionUpdater.addUsersToGroup = sinon
-        .stub()
-        .yields(null))
+      this.SubscriptionUpdater.addUsersToGroup = sinon.stub().yields(null)
     })
 
-    return it('delegates to addUsersToGroup', function(done) {
-      return this.SubscriptionUpdater.addUserToGroup(
+    it('delegates to addUsersToGroup', function(done) {
+      this.SubscriptionUpdater.addUserToGroup(
         this.subscription._id,
         this.otherUserId,
         () => {
           this.SubscriptionUpdater.addUsersToGroup
             .calledWith(this.subscription._id, [this.otherUserId])
             .should.equal(true)
-          return done()
+          done()
         }
       )
     })
@@ -306,13 +339,11 @@ describe('SubscriptionUpdater', function() {
 
   describe('addUsersToGroup', function() {
     beforeEach(function() {
-      return (this.FeaturesUpdater.refreshFeatures = sinon
-        .stub()
-        .callsArgWith(1))
+      this.FeaturesUpdater.refreshFeatures = sinon.stub().callsArgWith(1)
     })
 
     it('should add the user ids to the group as a set', function(done) {
-      return this.SubscriptionUpdater.addUsersToGroup(
+      this.SubscriptionUpdater.addUsersToGroup(
         this.subscription._id,
         [this.otherUserId],
         () => {
@@ -323,20 +354,20 @@ describe('SubscriptionUpdater', function() {
           this.findAndModifyStub
             .calledWith(searchOps, insertOperation)
             .should.equal(true)
-          return done()
+          done()
         }
       )
     })
 
-    return it('should update the users features', function(done) {
-      return this.SubscriptionUpdater.addUserToGroup(
+    it('should update the users features', function(done) {
+      this.SubscriptionUpdater.addUserToGroup(
         this.subscription._id,
         this.otherUserId,
         () => {
           this.FeaturesUpdater.refreshFeatures
             .calledWith(this.otherUserId)
             .should.equal(true)
-          return done()
+          done()
         }
       )
     })
@@ -347,14 +378,14 @@ describe('SubscriptionUpdater', function() {
       this.FeaturesUpdater.refreshFeatures = sinon.stub().callsArgWith(1)
       this.UserGetter.getUserOrUserStubById.yields(null, {}, false)
       this.fakeSubscriptions = [{ _id: 'fake-id-1' }, { _id: 'fake-id-2' }]
-      return this.SubscriptionLocator.getMemberSubscriptions.yields(
+      this.SubscriptionLocator.getMemberSubscriptions.yields(
         null,
         this.fakeSubscriptions
       )
     })
 
     it('should pull the users id from the group', function(done) {
-      return this.SubscriptionUpdater.removeUserFromGroup(
+      this.SubscriptionUpdater.removeUserFromGroup(
         this.subscription._id,
         this.otherUserId,
         () => {
@@ -363,50 +394,47 @@ describe('SubscriptionUpdater', function() {
           this.updateManyStub
             .calledWith(searchOps, removeOperation)
             .should.equal(true)
-          return done()
+          done()
         }
       )
     })
 
     it('should pull the users id from all groups', function(done) {
-      return this.SubscriptionUpdater.removeUserFromAllGroups(
-        this.otherUserId,
-        () => {
-          const filter = { _id: ['fake-id-1', 'fake-id-2'] }
-          const removeOperation = { $pull: { member_ids: this.otherUserId } }
-          sinon.assert.calledWith(this.updateManyStub, filter, removeOperation)
-          return done()
-        }
-      )
+      this.SubscriptionUpdater.removeUserFromAllGroups(this.otherUserId, () => {
+        const filter = { _id: ['fake-id-1', 'fake-id-2'] }
+        const removeOperation = { $pull: { member_ids: this.otherUserId } }
+        sinon.assert.calledWith(this.updateManyStub, filter, removeOperation)
+        done()
+      })
     })
 
     it('should update the users features', function(done) {
-      return this.SubscriptionUpdater.removeUserFromGroup(
+      this.SubscriptionUpdater.removeUserFromGroup(
         this.subscription._id,
         this.otherUserId,
         () => {
           this.FeaturesUpdater.refreshFeatures
             .calledWith(this.otherUserId)
             .should.equal(true)
-          return done()
+          done()
         }
       )
     })
 
-    return it('should not update features for user stubs', function(done) {
+    it('should not update features for user stubs', function(done) {
       this.UserGetter.getUserOrUserStubById.yields(null, {}, true)
-      return this.SubscriptionUpdater.removeUserFromGroup(
+      this.SubscriptionUpdater.removeUserFromGroup(
         this.subscription._id,
         this.userStub._id,
         () => {
           this.FeaturesUpdater.refreshFeatures.called.should.equal(false)
-          return done()
+          done()
         }
       )
     })
   })
 
-  return describe('deleteSubscription', function() {
+  describe('deleteSubscription', function() {
     beforeEach(function(done) {
       this.subscription_id = ObjectId().toString()
       this.subscription = {
@@ -418,36 +446,33 @@ describe('SubscriptionUpdater', function() {
         .stub()
         .yields(null, this.subscription)
       this.FeaturesUpdater.refreshFeatures = sinon.stub().yields()
-      return this.SubscriptionUpdater.deleteSubscription(
-        this.subscription_id,
-        done
-      )
+      this.SubscriptionUpdater.deleteSubscription(this.subscription_id, done)
     })
 
     it('should look up the subscription', function() {
-      return this.SubscriptionLocator.getSubscription
+      this.SubscriptionLocator.getSubscription
         .calledWith(this.subscription_id)
         .should.equal(true)
     })
 
     it('should remove the subscription', function() {
-      return this.SubscriptionModel.remove
+      this.SubscriptionModel.remove
         .calledWith({ _id: ObjectId(this.subscription_id) })
         .should.equal(true)
     })
 
     it('should downgrade the admin_id', function() {
-      return this.FeaturesUpdater.refreshFeatures
+      this.FeaturesUpdater.refreshFeatures
         .calledWith(this.subscription.admin_id)
         .should.equal(true)
     })
 
-    return it('should downgrade all of the members', function() {
-      return Array.from(this.subscription.member_ids).map(user_id =>
+    it('should downgrade all of the members', function() {
+      for (const userId of this.subscription.member_ids) {
         this.FeaturesUpdater.refreshFeatures
-          .calledWith(user_id)
+          .calledWith(userId)
           .should.equal(true)
-      )
+      }
     })
   })
 })