Merge pull request #1065 from sharelatex/ta-manage-group-members

Manage Group Members

GitOrigin-RevId: f0b120630ded1874dfc0352055633afff0015da9

services/web/app/coffee/Features/Subscription/SubscriptionGroupController.coffee

@@ -1,8 +1,6 @@
 SubscriptionGroupHandler = require("./SubscriptionGroupHandler")
 logger = require("logger-sharelatex")
 SubscriptionLocator = require("./SubscriptionLocator")
-ErrorsController = require("../Errors/ErrorController")
-SubscriptionDomainHandler = require("./SubscriptionDomainHandler")
 AuthenticationController = require('../Authentication/AuthenticationController')
 _ = require("underscore")
 async = require("async")
@@ -10,16 +8,14 @@ async = require("async")
 module.exports =
 
 	removeUserFromGroup: (req, res, next)->
-		adminUserId = AuthenticationController.getLoggedInUserId(req)
+		subscription = req.entity
 		userToRemove_id = req.params.user_id
-		getManagedSubscription adminUserId, (error, subscription) ->
+		logger.log subscriptionId: subscription._id, userToRemove_id:userToRemove_id, "removing user from group subscription"
-			return next(error) if error?
+		SubscriptionGroupHandler.removeUserFromGroup subscription._id, userToRemove_id, (err)->
-			logger.log adminUserId:adminUserId, userToRemove_id:userToRemove_id, "removing user from group subscription"
+			if err?
-			SubscriptionGroupHandler.removeUserFromGroup subscription._id, userToRemove_id, (err)->
+				logger.err err:err, adminUserId:adminUserId, userToRemove_id:userToRemove_id, "error removing user from group"
-				if err?
+				return next(err)
-					logger.err err:err, adminUserId:adminUserId, userToRemove_id:userToRemove_id, "error removing user from group"
+			res.send()
-					return res.sendStatus 500
-				res.send()
 
 	removeSelfFromGroup: (req, res, next)->
 		adminUserId = req.query.admin_user_id
@@ -33,24 +29,6 @@ module.exports =
 					return res.sendStatus 500
 				res.send()
 
-	exportGroupCsv: (req, res, next)->
-		user_id = AuthenticationController.getLoggedInUserId(req)
-		logger.log user_id: user_id, "exporting group csv"
-		getManagedSubscription user_id, (err, subscription)->
-			return next(error) if error?
-			if !subscription.groupPlan
-				return res.redirect("/")
-			SubscriptionGroupHandler.getPopulatedListOfMembers subscription._id, (err, users)->
-				groupCsv = ""
-				for user in users
-					groupCsv += user.email + "\n"
-				res.header(
-					"Content-Disposition",
-					"attachment; filename=Group.csv"
-				)
-				res.contentType('text/csv')
-				res.send(groupCsv)
-
 	# legacy route
 	redirectToSubscriptionGroupAdminPage: (req, res, next) ->
 		user_id = AuthenticationController.getLoggedInUserId(req)

services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee

@@ -7,7 +7,6 @@ Subscription = require("../../models/Subscription").Subscription
 LimitationsManager = require("./LimitationsManager")
 logger = require("logger-sharelatex")
 OneTimeTokenHandler = require("../Security/OneTimeTokenHandler")
-TeamInvitesHandler = require("./TeamInvitesHandler")
 EmailHandler = require("../Email/EmailHandler")
 settings = require("settings-sharelatex")
 NotificationsBuilder = require("../Notifications/NotificationsBuilder")

services/web/app/coffee/Features/Subscription/SubscriptionRouter.coffee

@@ -21,19 +21,13 @@ module.exports =
 
 
 		webRouter.get '/subscription/group', AuthenticationController.requireLogin(), SubscriptionGroupController.redirectToSubscriptionGroupAdminPage
-		webRouter.get '/subscription/group/export',  AuthenticationController.requireLogin(), SubscriptionGroupController.exportGroupCsv
-		webRouter.delete '/subscription/group/user/:user_id', AuthenticationController.requireLogin(), SubscriptionGroupController.removeUserFromGroup
 		webRouter.delete '/subscription/group/user', AuthenticationController.requireLogin(), SubscriptionGroupController.removeSelfFromGroup
 
 		# Team invites
-		webRouter.post '/subscription/invites',  AuthenticationController.requireLogin(),
-			TeamInvitesController.createInvite
 		webRouter.get '/subscription/invites/:token/',  AuthenticationController.requireLogin(),
 			TeamInvitesController.viewInvite
 		webRouter.put '/subscription/invites/:token/',  AuthenticationController.requireLogin(),
 			TeamInvitesController.acceptInvite
-		webRouter.delete '/subscription/invites/:email/',  AuthenticationController.requireLogin(),
-			TeamInvitesController.revokeInvite
 
 		# Routes to join a domain licence team
 		webRouter.get '/user/subscription/domain/join', AuthenticationController.requireLogin(), DomainLicenceController.join

services/web/app/coffee/Features/Subscription/TeamInvitesController.coffee

@@ -9,11 +9,12 @@ EmailHelper = require("../Helpers/EmailHelper")
 module.exports =
 	createInvite: (req, res, next) ->
 		teamManagerId = AuthenticationController.getLoggedInUserId(req)
+		subscription = req.entity
 		email = EmailHelper.parseEmail(req.body.email)
 		if !email?
 			return res.sendStatus(400)
 
-		TeamInvitesHandler.createInvite teamManagerId, email, (err, invite) ->
+		TeamInvitesHandler.createInvite teamManagerId, subscription, email, (err, invite) ->
 			return next(err) if err?
 			inviteView = { user:
 				{ email: invite.email, sentAt: invite.sentAt, invite: true }
@@ -48,11 +49,12 @@ module.exports =
 			res.sendStatus 204
 
 	revokeInvite: (req, res) ->
+		subscription = req.entity
 		email = EmailHelper.parseEmail(req.params.email)
 		teamManagerId = AuthenticationController.getLoggedInUserId(req)
 		if !email?
 			return res.sendStatus(400)
 
-		TeamInvitesHandler.revokeInvite teamManagerId, email, (err, results) ->
+		TeamInvitesHandler.revokeInvite teamManagerId, subscription, email, (err, results) ->
 			return next(err) if err?
 			res.sendStatus 204

services/web/app/coffee/Features/Subscription/TeamInvitesHandler.coffee

@@ -27,24 +27,21 @@ module.exports = TeamInvitesHandler =
 			invite = subscription.teamInvites.find (i) -> i.token == token
 			return callback(null, invite, subscription)
 
-	createInvite: (teamManagerId, email, callback) ->
+	createInvite: (teamManagerId, subscription, email, callback) ->
 		email = EmailHelper.parseEmail(email)
 		return callback(new Error('invalid email')) if !email?
 		logger.log {teamManagerId, email}, "Creating manager team invite"
 		UserGetter.getUser teamManagerId, (error, teamManager) ->
 			return callback(error) if error?
 
-			SubscriptionLocator.getUsersSubscription teamManagerId, (error, subscription) ->
+			if teamManager.first_name and teamManager.last_name
+				inviterName = "#{teamManager.first_name} #{teamManager.last_name} (#{teamManager.email})"
+			else
+				inviterName = teamManager.email
+
+			removeLegacyInvite subscription.id, email, (error) ->
 				return callback(error) if error?
-
+				createInvite(subscription, email, inviterName, callback)
-				if teamManager.first_name and teamManager.last_name
-					inviterName = "#{teamManager.first_name} #{teamManager.last_name} (#{teamManager.email})"
-				else
-					inviterName = teamManager.email
-
-				removeLegacyInvite subscription.id, email, (error) ->
-					return callback(error) if error?
-					createInvite(subscription, email, inviterName, callback)
 
 	createDomainInvite: (user, licence, callback) ->
 		email = EmailHelper.parseEmail(user.email)
@@ -83,14 +80,11 @@ module.exports = TeamInvitesHandler =
 
 				removeInviteFromTeam(subscription.id, invite.email, callback)
 
-	revokeInvite: (teamManagerId, email, callback) ->
+	revokeInvite: (teamManagerId, subscription, email, callback) ->
 		email = EmailHelper.parseEmail(email)
 		return callback(new Error('invalid email')) if !email?
 		logger.log {teamManagerId, email}, "Revoking invite"
-		SubscriptionLocator.getUsersSubscription teamManagerId, (err, teamSubscription) ->
+		removeInviteFromTeam(subscription.id, email, callback)
-			return callback(err) if err?
-
-			removeInviteFromTeam(teamSubscription.id, email, callback)
 
 	# Legacy method to allow a user to receive a confirmation email if their
 	# email is in Subscription.invited_emails when they join. We'll remove this
@@ -100,7 +94,7 @@ module.exports = TeamInvitesHandler =
 			return callback(err) if err?
 
 			async.map teams,
-				(team, cb) -> TeamInvitesHandler.createInvite(team.admin_id, email, cb)
+				(team, cb) -> TeamInvitesHandler.createInvite(team.admin_id, team, email, cb)
 			, callback
 
 createInvite = (subscription, email, inviterName, callback) ->

services/web/app/coffee/Features/UserMembership/UserMembershipController.coffee

@@ -38,3 +38,18 @@ module.exports =
 		UserMembershipHandler.removeUser entity, entityConfig, userId, (error, user)->
 			return next(error) if error?
 			res.send()
+
+	exportCsv: (req, res, next)->
+		{ entity, entityConfig } = req
+		logger.log subscriptionId: entity._id, "exporting csv"
+		UserMembershipHandler.getUsers entity, entityConfig, (error, users)->
+			return next(error) if error?
+			csvOutput = ""
+			for user in users
+				csvOutput += user.email + "\n"
+			res.header(
+				"Content-Disposition",
+				"attachment; filename=Group.csv"
+			)
+			res.contentType('text/csv')
+			res.send(csvOutput)

services/web/app/coffee/Features/UserMembership/UserMembershipEntityConfigs.coffee

@@ -13,11 +13,11 @@ module.exports =
 		translations:
 			title: 'group_account'
 			remove: 'remove_from_group'
-		pathsFor: () ->
+		pathsFor: (id) ->
-			addMember: '/subscription/invites'
+			addMember: "/manage/groups/#{id}/invites"
-			removeMember: '/subscription/group/user'
+			removeMember: "/manage/groups/#{id}/user"
-			removeInvite: '/subscription/invites'
+			removeInvite: "/manage/groups/#{id}/invites"
-			exportMembers: '/subscription/group/export'
+			exportMembers: "/manage/groups/#{id}/members/export"
 
 	team: # for metrics only
 		modelName: 'Subscription'

services/web/app/coffee/Features/UserMembership/UserMembershipRouter.coffee

@@ -1,11 +1,25 @@
 UserMembershipAuthorization = require './UserMembershipAuthorization'
 UserMembershipController = require './UserMembershipController'
+SubscriptionGroupController = require '../Subscription/SubscriptionGroupController'
+TeamInvitesController = require '../Subscription/TeamInvitesController'
 
 module.exports =
 	apply: (webRouter) ->
 		webRouter.get '/manage/groups/:id/members',
 			UserMembershipAuthorization.requireEntityAccess('group'),
 			UserMembershipController.index
+		webRouter.post '/manage/groups/:id/invites',
+			UserMembershipAuthorization.requireEntityAccess('group'),
+			TeamInvitesController.createInvite
+		webRouter.delete '/manage/groups/:id/user/:user_id',
+			UserMembershipAuthorization.requireEntityAccess('group'),
+			SubscriptionGroupController.removeUserFromGroup
+		webRouter.delete '/manage/groups/:id/invites/:email',
+			UserMembershipAuthorization.requireEntityAccess('group'),
+			TeamInvitesController.revokeInvite
+		webRouter.get '/manage/groups/:id/members/export',
+			UserMembershipAuthorization.requireEntityAccess('group'),
+			UserMembershipController.exportCsv
 
 
 		regularEntitites =

services/web/test/unit/coffee/Subscription/SubscriptionGroupControllerTests.coffee

@@ -20,13 +20,14 @@ describe "SubscriptionGroupController", ->
 			params:
 				subscriptionId:@subscriptionId
 			query:{}
+
 		@subscription = {
 			_id: @subscriptionId
 		}
+
 		@GroupHandler =
 			removeUserFromGroup: sinon.stub().callsArgWith(2)
-			isUserPartOfGroup: sinon.stub()
+
-			getPopulatedListOfMembers: sinon.stub().callsArgWith(1, null, [@user])
 		@SubscriptionLocator =
 			findManagedSubscription: sinon.stub().callsArgWith(1, null, @subscription)
 
@@ -34,62 +35,21 @@ describe "SubscriptionGroupController", ->
 			getLoggedInUserId: (req) -> req.session.user._id
 			getSessionUser: (req) -> req.session.user
 
-		@SubscriptionDomainHandler =
-			findDomainLicenceBySubscriptionId:sinon.stub()
-
-		@OneTimeTokenHandler =
-			getValueFromTokenAndExpire:sinon.stub()
-
-
-		@ErrorsController =
-			notFound:sinon.stub()
-
 		@Controller = SandboxedModule.require modulePath, requires:
 			"./SubscriptionGroupHandler":@GroupHandler
 			"logger-sharelatex": log:->
 			"./SubscriptionLocator": @SubscriptionLocator
-			"./SubscriptionDomainHandler":@SubscriptionDomainHandler
-			"../Errors/ErrorController":@ErrorsController
 			'../Authentication/AuthenticationController': @AuthenticationController
 
 
-		@token = "super-secret-token"
-
-
 	describe "removeUserFromGroup", ->
 		it "should use the subscription id for the logged in user and take the user id from the params", (done)->
 			userIdToRemove = "31231"
 			@req.params = user_id: userIdToRemove
+			@req.entity = @subscription
 
 			res =
 				send : =>
 					@GroupHandler.removeUserFromGroup.calledWith(@subscriptionId, userIdToRemove).should.equal true
 					done()
 			@Controller.removeUserFromGroup @req, res
-
-	describe "exportGroupCsv", ->
-
-		beforeEach ->
-			@subscription.groupPlan = true
-			@res = new MockResponse()
-			@res.contentType = sinon.stub()
-			@res.header = sinon.stub()
-			@res.send = sinon.stub()
-			@Controller.exportGroupCsv @req, @res
-
-		it "should set the correct content type on the request", ->
-			@res.contentType
-				.calledWith("text/csv")
-				.should.equal true
-
-		it "should name the exported csv file", ->
-			@res.header
-				.calledWith(
-					"Content-Disposition",
-					"attachment; filename=Group.csv")
-				.should.equal true
-
-		it "should export the correct csv", ->
-			@res.send
-				.calledWith("user@email.com\n")
-				.should.equal true

services/web/test/unit/coffee/Subscription/SubscriptionGroupHandlerTests.coffee

@@ -65,7 +65,6 @@ describe "SubscriptionGroupHandler", ->
 			"logger-sharelatex": log:->
 			"../User/UserCreator": @UserCreator
 			"./SubscriptionUpdater": @SubscriptionUpdater
-			"./TeamInvitesHandler": @TeamInvitesHandler
 			"./SubscriptionLocator": @SubscriptionLocator
 			"../../models/Subscription": Subscription: @Subscription
 			"../User/UserGetter": @UserGetter

services/web/test/unit/coffee/Subscription/TeamInvitesHandlerTests.coffee

@@ -104,7 +104,7 @@ describe "TeamInvitesHandler", ->
 
 	describe "createInvite", ->
 		it "adds the team invite to the subscription", (done) ->
-			@TeamInvitesHandler.createInvite @manager.id, "John.Snow@example.com", (err, invite) =>
+			@TeamInvitesHandler.createInvite @manager.id, @subscription, "John.Snow@example.com", (err, invite) =>
 				expect(err).to.eq(null)
 				expect(invite.token).to.eq(@newToken)
 				expect(invite.email).to.eq("john.snow@example.com")
@@ -113,7 +113,7 @@ describe "TeamInvitesHandler", ->
 				done()
 
 		it "sends an email", (done) ->
-			@TeamInvitesHandler.createInvite @manager.id, "John.Snow@example.com", (err, invite) =>
+			@TeamInvitesHandler.createInvite @manager.id, @subscription, "John.Snow@example.com", (err, invite) =>
 				@EmailHandler.sendEmail.calledWith("verifyEmailToJoinTeam",
 					sinon.match({
 						to: "john.snow@example.com",
@@ -126,7 +126,7 @@ describe "TeamInvitesHandler", ->
 		it "refreshes the existing invite if the email has already been invited", (done) ->
 			originalInvite = Object.assign({}, @teamInvite)
 
-			@TeamInvitesHandler.createInvite @manager.id, originalInvite.email, (err, invite) =>
+			@TeamInvitesHandler.createInvite @manager.id, @subscription, originalInvite.email, (err, invite) =>
 				expect(err).to.eq(null)
 				expect(invite).to.exist
 
@@ -140,7 +140,7 @@ describe "TeamInvitesHandler", ->
 				done()
 
 		it "removes any legacy invite from the subscription", (done) ->
-			@TeamInvitesHandler.createInvite @manager.id, "John.Snow@example.com", (err, invite) =>
+			@TeamInvitesHandler.createInvite @manager.id, @subscription, "John.Snow@example.com", (err, invite) =>
 				@Subscription.update.calledWith(
 					{ _id: new ObjectId("55153a8014829a865bbf700d") },
 					{ '$pull': { invited_emails: "john.snow@example.com" } }
@@ -245,7 +245,7 @@ describe "TeamInvitesHandler", ->
 
 	describe "revokeInvite", ->
 		it "removes the team invite from the subscription", (done) ->
-			@TeamInvitesHandler.revokeInvite @manager.id, "jorah@example.com", =>
+			@TeamInvitesHandler.revokeInvite @manager.id, @subscription, "jorah@example.com", =>
 				@Subscription.update.calledWith(
 					{ _id: new ObjectId("55153a8014829a865bbf700d") },
 					{ '$pull': { teamInvites: { email: "jorah@example.com" } } }
@@ -269,6 +269,7 @@ describe "TeamInvitesHandler", ->
 
 				@TeamInvitesHandler.createInvite.calledWith(
 					@subscription.admin_id,
+					@subscription,
 					"eddard@example.com"
 				).should.eq true
 
@@ -279,13 +280,13 @@ describe "TeamInvitesHandler", ->
 	describe "validation", ->
 		it "doesn't create an invite if the team limit has been reached", (done) ->
 			@LimitationsManager.teamHasReachedMemberLimit = sinon.stub().returns(true)
-			@TeamInvitesHandler.createInvite @manager.id, "John.Snow@example.com", (err, invite) =>
+			@TeamInvitesHandler.createInvite @manager.id, @subscription, "John.Snow@example.com", (err, invite) =>
 				expect(err).to.deep.equal(limitReached: true)
 				done()
 
 		it "doesn't create an invite if the subscription is not in a group plan", (done) ->
 			@subscription.groupPlan = false
-			@TeamInvitesHandler.createInvite @manager.id, "John.Snow@example.com", (err, invite) =>
+			@TeamInvitesHandler.createInvite @manager.id, @subscription, "John.Snow@example.com", (err, invite) =>
 				expect(err).to.deep.equal(wrongPlan: true)
 				done()
 
@@ -299,7 +300,7 @@ describe "TeamInvitesHandler", ->
 			@subscription.member_ids = [member.id]
 			@UserGetter.getUserByAnyEmail.withArgs(member.email).yields(null, member)
 
-			@TeamInvitesHandler.createInvite @manager.id, "tyrion@example.com", (err, invite) =>
+			@TeamInvitesHandler.createInvite @manager.id, @subscription, "tyrion@example.com", (err, invite) =>
 				expect(err).to.deep.equal(alreadyInTeam: true)
 				expect(invite).not.to.exist
 				done()

services/web/test/unit/coffee/UserMembership/UserMembershipControllerTests.coffee

@@ -20,7 +20,10 @@ describe "UserMembershipController", ->
 		@newUser = _id: 'mock-new-user-id', email: 'new-user-email@foo.bar'
 		@subscription = { _id: 'mock-subscription-id'}
 		@institution = _id: 'mock-institution-id', v1Id: 123
-		@users = [{ _id: 'mock-member-id-1' }, { _id: 'mock-member-id-2' }]
+		@users = [
+			{ _id: 'mock-member-id-1', email: 'mock-email-1@foo.com' }
+			{ _id: 'mock-member-id-2', email: 'mock-email-2@foo.com' }
+		]
 
 		@AuthenticationController =
 			getSessionUser: sinon.stub().returns(@user)
@@ -57,7 +60,7 @@ describe "UserMembershipController", ->
 				expect(viewParams.users).to.deep.equal @users
 				expect(viewParams.groupSize).to.equal @subscription.membersLimit
 				expect(viewParams.translations.title).to.equal 'group_account'
-				expect(viewParams.paths.addMember).to.equal '/subscription/invites'
+				expect(viewParams.paths.addMember).to.equal "/manage/groups/#{@subscription._id}/invites"
 				done()
 
 		it 'render group managers view', (done) ->
@@ -129,3 +132,34 @@ describe "UserMembershipController", ->
 				expect(error).to.extist
 				expect(error).to.be.an.instanceof(Errors.NotFoundError)
 				done()
+
+	describe "exportCsv", ->
+
+		beforeEach ->
+			@req.entity = @subscription
+			@req.entityConfig = EntityConfigs.groupManagers
+			@res = new MockResponse()
+			@res.contentType = sinon.stub()
+			@res.header = sinon.stub()
+			@res.send = sinon.stub()
+			@UserMembershipController.exportCsv @req, @res
+
+		it 'get users', ->
+			sinon.assert.calledWithMatch(
+				@UserMembershipHandler.getUsers,
+				@subscription,
+				modelName: 'Subscription',
+			)
+
+		it "should set the correct content type on the request", ->
+			assertCalledWith(@res.contentType, "text/csv")
+
+		it "should name the exported csv file", ->
+			assertCalledWith(
+				@res.header
+				"Content-Disposition",
+				"attachment; filename=Group.csv"
+			)
+
+		it "should export the correct csv", ->
+			assertCalledWith(@res.send, "mock-email-1@foo.com\nmock-email-2@foo.com\n")