Merge pull request #3064 from overleaf/ta-manage-multiple-groups

Allow Users to Manage Multiple Groups

GitOrigin-RevId: c918da0114cdd1d679223f69d81159b4c2608874

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

@@ -43,9 +43,12 @@ module.exports = {
   },
 
   removeSelfFromGroup(req, res, next) {
-    const adminUserId = req.query.admin_user_id
+    const subscriptionId = req.query.subscriptionId
     const userToRemove_id = AuthenticationController.getLoggedInUserId(req)
-    return getManagedSubscription(adminUserId, function(error, subscription) {
+    return SubscriptionLocator.getSubscription(subscriptionId, function(
+      error,
+      subscription
+    ) {
       if (error != null) {
         return next(error)
       }
@@ -56,7 +59,7 @@ module.exports = {
         function(err) {
           if (err != null) {
             logger.err(
-              { err, userToRemove_id, adminUserId },
+              { err, userToRemove_id, subscriptionId },
               'error removing self from group'
             )
             return res.sendStatus(500)
@@ -65,35 +68,5 @@ module.exports = {
         }
       )
     })
-  },
-
-  // legacy route
-  redirectToSubscriptionGroupAdminPage(req, res, next) {
-    const user_id = AuthenticationController.getLoggedInUserId(req)
-    return getManagedSubscription(user_id, function(error, subscription) {
-      if (error != null) {
-        return next(error)
-      }
-      if (!(subscription != null ? subscription.groupPlan : undefined)) {
-        return res.redirect('/user/subscription')
-      }
-      return res.redirect(`/manage/groups/${subscription._id}/members`)
-    })
   }
 }
-
-var getManagedSubscription = (managerId, callback) =>
-  SubscriptionLocator.findManagedSubscription(managerId, function(
-    err,
-    subscription
-  ) {
-    if (err) {
-      return callback(err)
-    } else if (!subscription) {
-      return callback(
-        new Error(`No subscription found managed by user ${managerId}`)
-      )
-    }
-
-    return callback(null, subscription)
-  })

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

@@ -42,10 +42,6 @@ const SubscriptionLocator = {
     )
   },
 
-  findManagedSubscription(managerId, callback) {
-    return Subscription.findOne({ manager_ids: managerId }, callback)
-  },
-
   getManagedGroupSubscriptions(user_or_id, callback) {
     if (callback == null) {
       callback = function(error, managedSubscriptions) {}
@@ -121,9 +117,6 @@ SubscriptionLocator.promises = {
   getUserIndividualSubscription: promisify(
     SubscriptionLocator.getUserIndividualSubscription
   ),
-  findManagedSubscription: promisify(
-    SubscriptionLocator.findManagedSubscription
-  ),
   getManagedGroupSubscriptions: promisify(
     SubscriptionLocator.getManagedGroupSubscriptions
   ),

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

@@ -47,11 +47,6 @@ module.exports = {
       SubscriptionController.canceledSubscription
     )
 
-    webRouter.get(
-      '/subscription/group',
-      AuthenticationController.requireLogin(),
-      SubscriptionGroupController.redirectToSubscriptionGroupAdminPage
-    )
     webRouter.delete(
       '/subscription/group/user',
       AuthenticationController.requireLogin(),

services/web/app/views/subscriptions/dashboard/_group_memberships.pug

@@ -11,7 +11,7 @@ div(ng-controller="GroupMembershipController")
 						//- Team notice is sanitized in SubscriptionViewModelBuilder
 						em(ng-non-bindable) !{groupSubscription.teamNotice}
 				span 
-					button.btn.btn-danger.text-capitalise(ng-click="removeSelfFromGroup('"+groupSubscription.admin_id._id+"')") #{translate("leave_group")}
+					button.btn.btn-danger.text-capitalise(ng-click="removeSelfFromGroup('"+groupSubscription._id+"')") #{translate("leave_group")}
 			hr
 
 script(type='text/ng-template', id='LeaveGroupModalTemplate')

services/web/app/views/subscriptions/successful_subscription.pug

@@ -17,7 +17,7 @@ block content
 								a(href="/user/subscription")  #{translate("manage_subscription")}.
 						p
 							- if (personalSubscription.groupPlan == true)
-								a.btn.btn-success.btn-large(href="/subscription/group") #{translate("add_your_first_group_member_now")}
+								a.btn.btn-success.btn-large(href=`/manage/groups/${personalSubscription._id}/members`) #{translate("add_your_first_group_member_now")}
 						p.letter-from-founders
 							p #{translate("thanks_for_subscribing_you_help_sl", {planName:personalSubscription.plan.name})}
 							p #{translate("need_anything_contact_us_at")} 

services/web/frontend/js/main/subscription-dashboard.js

@@ -148,7 +148,10 @@ App.controller('LeaveGroupModalController', function(
     return $http({
       url: '/subscription/group/user',
       method: 'DELETE',
-      params: { admin_user_id: $scope.admin_id, _csrf: window.csrfToken }
+      params: {
+        subscriptionId: $scope.subscriptionId,
+        _csrf: window.csrfToken
+      }
     })
       .then(() => location.reload())
       .catch(() => console.log('something went wrong changing plan'))
@@ -158,8 +161,8 @@ App.controller('LeaveGroupModalController', function(
 })
 
 App.controller('GroupMembershipController', function($scope, $modal) {
-  $scope.removeSelfFromGroup = function(admin_id) {
-    $scope.admin_id = admin_id
+  $scope.removeSelfFromGroup = function(subscriptionId) {
+    $scope.subscriptionId = subscriptionId
     return $modal.open({
       templateUrl: 'LeaveGroupModalTemplate',
       controller: 'LeaveGroupModalController',

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

@@ -23,11 +23,10 @@ describe('mongoose', function() {
   })
 
   describe('Subsription', function() {
-    let user, otherUser
+    let user
 
     beforeEach(async function() {
       user = await User.create({ email: 'wombat@potato.net' })
-      otherUser = await User.create({ email: 'giraffe@turnip.org' })
     })
 
     it('allows the creation of a subscription', async function() {
@@ -41,23 +40,5 @@ describe('mongoose', function() {
     it('does not allow the creation of a subscription without a manager', async function() {
       await expect(Subscription.create({ admin_id: user._id })).to.be.rejected
     })
-
-    it('does not allow a user to manage more than one group', async function() {
-      await expect(
-        Subscription.create({ admin_id: user._id, manager_ids: [user._id] })
-      ).to.be.fulfilled
-      await expect(
-        Subscription.create({
-          admin_id: otherUser._id,
-          manager_ids: [otherUser._id]
-        })
-      ).to.be.fulfilled
-      await expect(
-        Subscription.update(
-          { admin_id: user._id },
-          { $push: { manager_ids: otherUser._id } }
-        )
-      ).to.be.rejected
-    })
   })
 })

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

@@ -44,9 +44,7 @@ describe('SubscriptionGroupController', function() {
     this.GroupHandler = { removeUserFromGroup: sinon.stub().callsArgWith(2) }
 
     this.SubscriptionLocator = {
-      findManagedSubscription: sinon
-        .stub()
-        .callsArgWith(1, null, this.subscription)
+      getSubscription: sinon.stub().callsArgWith(1, null, this.subscription)
     }
 
     this.AuthenticationController = {
@@ -91,4 +89,29 @@ describe('SubscriptionGroupController', function() {
       return this.Controller.removeUserFromGroup(this.req, res)
     })
   })
+
+  describe('removeSelfFromGroup', function() {
+    it('gets subscription and remove user', function(done) {
+      const userIdToRemove = '31231'
+      this.req.query = { subscriptionId: this.subscriptionId }
+      const memberUserIdToremove = 123456789
+      this.req.session.user._id = memberUserIdToremove
+
+      const res = {
+        sendStatus: () => {
+          sinon.assert.calledWith(
+            this.SubscriptionLocator.getSubscription,
+            this.subscriptionId
+          )
+          sinon.assert.calledWith(
+            this.GroupHandler.removeUserFromGroup,
+            this.subscriptionId,
+            memberUserIdToremove
+          )
+          return done()
+        }
+      }
+      return this.Controller.removeSelfFromGroup(this.req, res)
+    })
+  })
 })

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

@@ -88,21 +88,5 @@ describe('Subscription Locator Tests', function() {
         }
       )
     })
-
-    describe('finding managed subscription', function() {
-      it('should query the database', function(done) {
-        this.Subscription.findOne.callsArgWith(1, null, this.subscription)
-        return this.SubscriptionLocator.findManagedSubscription(
-          this.user._id,
-          (err, subscription) => {
-            this.Subscription.findOne
-              .calledWith({ manager_ids: this.user._id })
-              .should.equal(true)
-            subscription.should.equal(this.subscription)
-            return done()
-          }
-        )
-      })
-    })
   })
 })