Merge pull request #742 from sharelatex/afc-use-subscription-id

Pass the subscription id in methods that modify a subscription

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

@@ -89,13 +89,13 @@ module.exports = LimitationsManager =
 
 		return currentTotal >= subscription.membersLimit
 
-	hasGroupMembersLimitReached: (user_id, callback = (err, limitReached, subscription)->)->
+	hasGroupMembersLimitReached: (subscriptionId, callback = (err, limitReached, subscription)->)->
-		SubscriptionLocator.getUsersSubscription user_id, (err, subscription)->
+		SubscriptionLocator.getSubscription subscriptionId, (err, subscription)->
 			if err?
-				logger.err err:err, user_id:user_id, "error getting users subscription"
+				logger.err err:err, subscriptionId: subscriptionId, "error getting subscription"
 				return callback(err)
 			if !subscription?
-				logger.err user_id:user_id, "no subscription found for user"
+				logger.err subscriptionId: subscriptionId, "no subscription found"
 				return callback("no subscription found")
 
 			limitReached = LimitationsManager.teamHasReachedMemberLimit(subscription)

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

@@ -9,11 +9,16 @@ async = require("async")
 
 module.exports =
 
-	addUserToGroup: (req, res)->
+	addUserToGroup: (req, res, next)->
 		adminUserId = AuthenticationController.getLoggedInUserId(req)
 		newEmail = req.body?.email?.toLowerCase()?.trim()
+
+		SubscriptionLocator.getManagedSubscription adminUserId, (error, subscription) ->
+			return next(error) if error?
+
 			logger.log adminUserId:adminUserId, newEmail:newEmail, "adding user to group subscription"
-		SubscriptionGroupHandler.addUserToGroup adminUserId, newEmail, (err, user)->
+
+			SubscriptionGroupHandler.addUserToGroup subscription._id, newEmail, (err, user)->
 				if err?
 					logger.err err:err, newEmail:newEmail, adminUserId:adminUserId, "error adding user from group"
 					return res.sendStatus 500
@@ -23,32 +28,37 @@ module.exports =
 					result.limitReached = true
 				res.json(result)
 
-	removeUserFromGroup: (req, res)->
+	removeUserFromGroup: (req, res, next)->
 		adminUserId = AuthenticationController.getLoggedInUserId(req)
 		userToRemove_id = req.params.user_id
+		SubscriptionLocator.getManagedSubscription adminUserId, (error, subscription) ->
+			return next(error) if error?
 			logger.log adminUserId:adminUserId, userToRemove_id:userToRemove_id, "removing user from group subscription"
-		SubscriptionGroupHandler.removeUserFromGroup adminUserId, userToRemove_id, (err)->
+			SubscriptionGroupHandler.removeUserFromGroup subscription._id, userToRemove_id, (err)->
 				if err?
 					logger.err err:err, adminUserId:adminUserId, userToRemove_id:userToRemove_id, "error removing user from group"
 					return res.sendStatus 500
 				res.send()
 
-	removeSelfFromGroup: (req, res)->
+	removeSelfFromGroup: (req, res, next)->
 		adminUserId = req.query.admin_user_id
 		userToRemove_id = AuthenticationController.getLoggedInUserId(req)
+		SubscriptionLocator.getManagedSubscription adminUserId, (error, subscription) ->
+			return next(error) if error?
 			logger.log adminUserId:adminUserId, userToRemove_id:userToRemove_id, "removing user from group subscription after self request"
-		SubscriptionGroupHandler.removeUserFromGroup adminUserId, userToRemove_id, (err)->
+			SubscriptionGroupHandler.removeUserFromGroup subscription._id, userToRemove_id, (err)->
 				if err?
 					logger.err err:err, userToRemove_id:userToRemove_id, adminUserId:adminUserId, "error removing self from group"
 					return res.sendStatus 500
 				res.send()
 
-	renderSubscriptionGroupAdminPage: (req, res)->
+	renderSubscriptionGroupAdminPage: (req, res, next)->
 		user_id = AuthenticationController.getLoggedInUserId(req)
-		SubscriptionLocator.getUsersSubscription user_id, (err, subscription)->
+		SubscriptionLocator.getManagedSubscription user_id, (error, subscription)->
+			return next(error) if error?
 			if !subscription?.groupPlan
 				return res.redirect("/user/subscription")
-			SubscriptionGroupHandler.getPopulatedListOfMembers user_id, (err, users)->
+			SubscriptionGroupHandler.getPopulatedListOfMembers subscription._id, (err, users)->
 				res.render "subscriptions/group_admin",
 					title: 'group_admin'
 					users: users
@@ -57,10 +67,11 @@ module.exports =
 	exportGroupCsv: (req, res)->
 		user_id = AuthenticationController.getLoggedInUserId(req)
 		logger.log user_id: user_id, "exporting group csv"
-		SubscriptionLocator.getUsersSubscription user_id, (err, subscription)->
+		SubscriptionLocator.getManagedSubscription user_id, (err, subscription)->
+			return next(error) if error?
 			if !subscription.groupPlan
 				return res.redirect("/")
-			SubscriptionGroupHandler.getPopulatedListOfMembers user_id, (err, users)->
+			SubscriptionGroupHandler.getPopulatedListOfMembers subscription._id, (err, users)->
 				groupCsv = ""
 				for user in users
 					groupCsv += user.email + "\n"

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

@@ -14,19 +14,19 @@ NotificationsBuilder = require("../Notifications/NotificationsBuilder")
 
 module.exports = SubscriptionGroupHandler =
 
-	addUserToGroup: (adminUserId, newEmail, callback)->
+	addUserToGroup: (subscriptionId, newEmail, callback)->
-		logger.log adminUserId:adminUserId, newEmail:newEmail, "adding user to group"
+		logger.log subscriptionId:subscriptionId, newEmail:newEmail, "adding user to group"
-		LimitationsManager.hasGroupMembersLimitReached adminUserId, (err, limitReached, subscription)->
+		LimitationsManager.hasGroupMembersLimitReached subscriptionId, (err, limitReached, subscription)->
 			if err?
-				logger.err err:err, adminUserId:adminUserId, newEmail:newEmail, "error checking if limit reached for group plan"
+				logger.err err:err, subscriptionId:subscriptionId, newEmail:newEmail, "error checking if limit reached for group plan"
 				return callback(err)
 			if limitReached
-				logger.err adminUserId:adminUserId, newEmail:newEmail, "group subscription limit reached not adding user to group"
+				logger.err subscriptionId:subscriptionId, newEmail:newEmail, "group subscription limit reached not adding user to group"
 				return callback(limitReached:limitReached)
 			UserGetter.getUserByAnyEmail newEmail, (err, user)->
 				return callback(err) if err?
 				if user?
-					SubscriptionUpdater.addUserToGroup adminUserId, user._id, (err)->
+					SubscriptionUpdater.addUserToGroup subscriptionId, user._id, (err)->
 						if err?
 							logger.err err:err, "error adding user to group"
 							return callback(err)
@@ -34,13 +34,13 @@ module.exports = SubscriptionGroupHandler =
 						userViewModel = buildUserViewModel(user)
 						callback(err, userViewModel)
 				else
-					TeamInvitesHandler.createInvite adminUserId, newEmail, (err) ->
+					TeamInvitesHandler.createInvite subscriptionId, newEmail, (err) ->
 						return callback(err) if err?
 						userViewModel = buildEmailInviteViewModel(newEmail)
 						callback(err, userViewModel)
 
-	removeUserFromGroup: (adminUser_id, userToRemove_id, callback)->
+	removeUserFromGroup: (subscriptionId, userToRemove_id, callback)->
-		SubscriptionUpdater.removeUserFromGroup adminUser_id, userToRemove_id, callback
+		SubscriptionUpdater.removeUserFromGroup subscriptionId, userToRemove_id, callback
 
 	replaceUserReferencesInGroups: (oldId, newId, callback) ->
 		Subscription.update {admin_id: oldId}, {admin_id: newId}, (error) ->
@@ -56,8 +56,8 @@ module.exports = SubscriptionGroupHandler =
 				return callback(error) if error?
 				Subscription.update query, removeOldUserUpdate, { multi: true }, callback
 
-	getPopulatedListOfMembers: (adminUser_id, callback)->
+	getPopulatedListOfMembers: (subscriptionId, callback)->
-		SubscriptionLocator.getUsersSubscription adminUser_id, (err, subscription)->
+		SubscriptionLocator.getSubscription subscriptionId, (err, subscription)->
 			users = []
 
 			for email in subscription.invited_emails or []

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

@@ -14,6 +14,16 @@ module.exports =
 			logger.log user_id:user_id, "got users subscription"
 			callback(err, subscription)
 
+	getManagedSubscription: (managerId, callback)->
+			logger.log managerId: managerId, "getting managed subscription"
+			Subscription.findOne admin_id: managerId, (err, subscription)->
+				if subscription?
+					logger.log managerId: managerId, "got managed subscription"
+				else
+					err ||= new Error("No subscription found managed by user #{managerId}")
+
+				callback(err, subscription)
+
 	getMemberSubscriptions: (user_or_id, callback) ->
 		if user_or_id? and user_or_id._id?
 			user_id = user_or_id._id

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

@@ -25,13 +25,13 @@ module.exports = SubscriptionUpdater =
 					return callback(err) if err?
 					SubscriptionUpdater._updateSubscriptionFromRecurly recurlySubscription, subscription, callback
 
-	addUserToGroup: (adminUserId, userId, callback)->
+	addUserToGroup: (subscriptionId, userId, callback)->
-		@addUsersToGroup(adminUserId, [userId], callback)
+		@addUsersToGroup(subscriptionId, [userId], callback)
 
-	addUsersToGroup: (adminUserId, memberIds, callback)->
+	addUsersToGroup: (subscriptionId, memberIds, callback)->
-		logger.log adminUserId: adminUserId, memberIds: memberIds, "adding members into mongo subscription"
+		logger.log subscriptionId: subscriptionId, memberIds: memberIds, "adding members into mongo subscription"
 		searchOps =
-			admin_id: adminUserId
+			_id: subscriptionId
 		insertOperation =
 			{ $push: { member_ids: { $each: memberIds } } }
 
@@ -46,9 +46,9 @@ module.exports = SubscriptionUpdater =
 				async.map userIds, FeaturesUpdater.refreshFeatures, callback
 
 
-	removeUserFromGroup: (adminUser_id, user_id, callback)->
+	removeUserFromGroup: (subscriptionId, user_id, callback)->
 		searchOps =
-			admin_id: adminUser_id
+			_id: subscriptionId
 		removeOperation =
 			"$pull": {member_ids:user_id}
 		Subscription.update searchOps, removeOperation, (err)->

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

@@ -23,6 +23,7 @@ describe "LimitationsManager", ->
 
 		@SubscriptionLocator =
 			getUsersSubscription: sinon.stub()
+			getSubscription: sinon.stub()
 
 		@LimitationsManager = SandboxedModule.require modulePath, requires:
 			'../Project/ProjectGetter': @ProjectGetter
@@ -301,7 +302,7 @@ describe "LimitationsManager", ->
 	describe "hasGroupMembersLimitReached", ->
 
 		beforeEach ->
-			@user_id = "12312"
+			@subscriptionId = "12312"
 			@subscription =
 				membersLimit: 3
 				member_ids: ["", ""]
@@ -310,22 +311,22 @@ describe "LimitationsManager", ->
 				]
 
 		it "should return true if the limit is hit (including members and invites)", (done)->
-			@SubscriptionLocator.getUsersSubscription.callsArgWith(1, null, @subscription)
+			@SubscriptionLocator.getSubscription.callsArgWith(1, null, @subscription)
-			@LimitationsManager.hasGroupMembersLimitReached @user_id, (err, limitReached)->
+			@LimitationsManager.hasGroupMembersLimitReached @subscriptionId, (err, limitReached)->
 				limitReached.should.equal true
 				done()
 
 		it "should return false if the limit is not hit (including members and invites)", (done)->
 			@subscription.membersLimit = 4
-			@SubscriptionLocator.getUsersSubscription.callsArgWith(1, null, @subscription)
+			@SubscriptionLocator.getSubscription.callsArgWith(1, null, @subscription)
-			@LimitationsManager.hasGroupMembersLimitReached @user_id, (err, limitReached)->
+			@LimitationsManager.hasGroupMembersLimitReached @subscriptionId, (err, limitReached)->
 				limitReached.should.equal false
 				done()
 
 		it "should return true if the limit has been exceded (including members and invites)", (done)->
 			@subscription.membersLimit = 2
-			@SubscriptionLocator.getUsersSubscription.callsArgWith(1, null, @subscription)
+			@SubscriptionLocator.getSubscription.callsArgWith(1, null, @subscription)
-			@LimitationsManager.hasGroupMembersLimitReached @user_id, (err, limitReached)->
+			@LimitationsManager.hasGroupMembersLimitReached @subscriptionId, (err, limitReached)->
 				limitReached.should.equal true
 				done()
 
@@ -379,4 +380,3 @@ describe "LimitationsManager", ->
 				@V1SubscriptionManager.getSubscriptionsFromV1.calledWith(@user_id).should.equal true
 				result.should.equal false
 				done()
-

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

@@ -10,7 +10,7 @@ describe "SubscriptionGroupController", ->
 	beforeEach ->
 		@user = {_id:"!@312431",email:"user@email.com"}
 		@adminUserId = "123jlkj"
-		@subscription_id = "123434325412"
+		@subscriptionId = "123434325412"
 		@user_email = "bob@gmail.com"
 		@req =
 			session:
@@ -18,15 +18,19 @@ describe "SubscriptionGroupController", ->
 					_id: @adminUserId
 					email:@user_email
 			params:
-				subscription_id:@subscription_id
+				subscriptionId:@subscriptionId
 			query:{}
-		@subscription = {}
+		@subscription = {
+			_id: @subscriptionId
+		}
 		@GroupHandler =
 			addUserToGroup: sinon.stub().callsArgWith(2, null, @user)
 			removeUserFromGroup: sinon.stub().callsArgWith(2)
 			isUserPartOfGroup: sinon.stub()
 			getPopulatedListOfMembers: sinon.stub().callsArgWith(1, null, [@user])
-		@SubscriptionLocator = getUsersSubscription: sinon.stub().callsArgWith(1, null, @subscription)
+		@SubscriptionLocator =
+			getManagedSubscription: sinon.stub().callsArgWith(1, null, @subscription)
+
 		@AuthenticationController =
 			getLoggedInUserId: (req) -> req.session.user._id
 			getSessionUser: (req) -> req.session.user
@@ -55,25 +59,25 @@ describe "SubscriptionGroupController", ->
 
 	describe "addUserToGroup", ->
 
-		it "should use the admin id for the logged in user and take the email address from the body", (done)->
+		it "should use the subscription id for the logged in user and take the email address from the body", (done)->
 			newEmail = " boB@gmaiL.com "
 			@req.body = email: newEmail
 			res =
 				json : (data)=>
-					@GroupHandler.addUserToGroup.calledWith(@adminUserId, "bob@gmail.com").should.equal true
+					@GroupHandler.addUserToGroup.calledWith(@subscriptionId, "bob@gmail.com").should.equal true
 					data.user.should.deep.equal @user
 					done()
 			@Controller.addUserToGroup @req, res
 
 
 	describe "removeUserFromGroup", ->
-		it "should use the admin id for the logged in user and take the user id from the params", (done)->
+		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
 
 			res =
 				send : =>
-					@GroupHandler.removeUserFromGroup.calledWith(@adminUserId, userIdToRemove).should.equal true
+					@GroupHandler.removeUserFromGroup.calledWith(@subscriptionId, userIdToRemove).should.equal true
 					done()
 			@Controller.removeUserFromGroup @req, res
 

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

@@ -156,13 +156,13 @@ describe "SubscriptionGroupHandler", ->
 	describe "getPopulatedListOfMembers", ->
 		beforeEach ->
 			@subscription = {}
-			@SubscriptionLocator.getUsersSubscription.callsArgWith(1, null, @subscription)
+			@SubscriptionLocator.getSubscription.callsArgWith(1, null, @subscription)
 			@UserGetter.getUser.callsArgWith(1, null, {_id:"31232"})
 
 		it "should locate the subscription", (done)->
 			@UserGetter.getUser.callsArgWith(1, null, {_id:"31232"})
-			@Handler.getPopulatedListOfMembers @adminUser_id, (err, users)=>
+			@Handler.getPopulatedListOfMembers @subscriptionId, (err, users)=>
-				@SubscriptionLocator.getUsersSubscription.calledWith(@adminUser_id).should.equal true
+				@SubscriptionLocator.getSubscription.calledWith(@subscriptionId).should.equal true
 				done()
 
 		it "should get the users by id", (done)->

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

@@ -18,6 +18,7 @@ describe "SubscriptionUpdater", ->
 		@otherUserId = "5208dd34438842e2db000005"
 		@allUserIds = ["13213", "dsadas", "djsaiud89"]
 		@subscription = subscription =
+			_id: "111111111111111111111111"
 			admin_id: @adminUser._id
 			member_ids: @allUserIds
 			save: sinon.stub().callsArgWith(0)
@@ -26,6 +27,7 @@ describe "SubscriptionUpdater", ->
 		@user_id = @adminuser_id
 
 		@groupSubscription =
+			_id: "222222222222222222222222"
 			admin_id: @adminUser._id
 			member_ids: @allUserIds
 			save: sinon.stub().callsArgWith(0)
@@ -158,9 +160,9 @@ describe "SubscriptionUpdater", ->
 			@SubscriptionUpdater.addUsersToGroup = sinon.stub().yields(null)
 
 		it "delegates to addUsersToGroup", (done)->
-			@SubscriptionUpdater.addUserToGroup @adminUser._id, @otherUserId, =>
+			@SubscriptionUpdater.addUserToGroup @subscription._id, @otherUserId, =>
 				@SubscriptionUpdater.addUsersToGroup
-					.calledWith(@adminUser._id, [@otherUserId]).should.equal true
+					.calledWith(@subscription._id, [@otherUserId]).should.equal true
 				done()
 
 	describe "addUsersToGroup", ->
@@ -168,16 +170,16 @@ describe "SubscriptionUpdater", ->
 			@FeaturesUpdater.refreshFeatures = sinon.stub().callsArgWith(1)
 
 		it "should add the user ids to the group as a set", (done)->
-			@SubscriptionUpdater.addUsersToGroup @adminUser._id, [@otherUserId], =>
+			@SubscriptionUpdater.addUsersToGroup @subscription._id, [@otherUserId], =>
 				searchOps =
-					admin_id: @adminUser._id
+					_id: @subscription._id
 				insertOperation =
 					{ $push: { member_ids: { $each: [@otherUserId] } } }
 				@findAndModifyStub.calledWith(searchOps, insertOperation).should.equal true
 				done()
 
 		it "should update the users features", (done)->
-			@SubscriptionUpdater.addUserToGroup @adminUser._id, @otherUserId, =>
+			@SubscriptionUpdater.addUserToGroup @subscription._id, @otherUserId, =>
 				@FeaturesUpdater.refreshFeatures.calledWith(@otherUserId).should.equal true
 				done()
 
@@ -186,16 +188,16 @@ describe "SubscriptionUpdater", ->
 			@FeaturesUpdater.refreshFeatures = sinon.stub().callsArgWith(1)
 
 		it "should pull the users id from the group", (done)->
-			@SubscriptionUpdater.removeUserFromGroup @adminUser._id, @otherUserId, =>
+			@SubscriptionUpdater.removeUserFromGroup @subscription._id, @otherUserId, =>
 				searchOps =
-					admin_id:@adminUser._id
+					_id: @subscription._id
 				removeOperation =
 					"$pull": {member_ids:@otherUserId}
 				@updateStub.calledWith(searchOps, removeOperation).should.equal true
 				done()
 
 		it "should update the users features", (done)->
-			@SubscriptionUpdater.removeUserFromGroup @adminUser._id, @otherUserId, =>
+			@SubscriptionUpdater.removeUserFromGroup @subscription._id, @otherUserId, =>
 				@FeaturesUpdater.refreshFeatures.calledWith(@otherUserId).should.equal true
 				done()