fixed calls to addUserToGroup and added more logging

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

@@ -52,11 +52,17 @@ module.exports =
 			return callback(err) if err?
 			callback err, subscriptions.length > 0, subscriptions
 
-	hasGroupMembersLimitReached: (user_id, callback)->
+	hasGroupMembersLimitReached: (user_id, callback = (err, limitReached, subscription)->)->
 		SubscriptionLocator.getUsersSubscription user_id, (err, subscription)->
+			if err?
+				logger.err err:err, user_id:user_id, "error getting users subscription"
+				return callback(err)
+			if !subscription?
+				logger.err user_id:user_id, "no subscription found for user"
+				return callback("no subscription found")
 			limitReached = subscription.member_ids.length >= subscription.membersLimit
 			logger.log user_id:user_id, limitReached:limitReached, currentTotal: subscription.member_ids.length, membersLimit: subscription.membersLimit, "checking if subscription members limit has been reached"
-			callback(err, limitReached)
+			callback(err, limitReached, subscription)
 
 getOwnerOfProject = (project_id, callback)->
 	Project.findById project_id, 'owner_ref', (error, project) ->

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

@@ -13,14 +13,21 @@ NotificationsBuilder = require("../Notifications/NotificationsBuilder")
 
 module.exports = SubscriptionGroupHandler =
 
-	addUserToGroup: (subscription, newEmail, callback)->
+	addUserToGroup: (adminUserId, newEmail, callback)->
 		UserCreator.getUserOrCreateHoldingAccount newEmail, (err, user)->
-			LimitationsManager.hasGroupMembersLimitReached subscription.admin_id, (err, limitReached)->
+			if err?
+				logger.err err:err, "error creating user for holding account"
+				return callback(err)
+			if !user?
+				msg = "no user returned whenc reating holidng account or getting user"
+				logger.err adminUserId:adminUserId, newEmail:newEmail, msg
+				return callback(msg)
+			LimitationsManager.hasGroupMembersLimitReached adminUserId, (err, limitReached, subscription)->
 				if err?
 					return callback(err)
 				if limitReached
 					return callback(limitReached:limitReached)
-				SubscriptionUpdater.addUserToGroup subscription.admin_id, user._id, (err)->
+				SubscriptionUpdater.addUserToGroup adminUserId, user._id, (err)->
 					if err?
 						logger.err err:err, "error adding user to group"
 						return callback(err)
@@ -73,7 +80,14 @@ module.exports = SubscriptionGroupHandler =
 				logger.err userEmail:userEmail, token:token, "token value not found for processing group verification"
 				return callback("token_not_found")
 			SubscriptionLocator.getSubscription subscription_id, (err, subscription)->
-				SubscriptionGroupHandler.addUserToGroup subscription, userEmail, callback
+				if err?
+					logger.err err:err, subscription:subscription, userEmail:userEmail, subscription_id:subscription_id, "error getting subscription"
+					return callback(err)
+				if !subscription?
+					logger.warn subscription_id:subscription_id, "no subscription found"
+					return callback()
+				SubscriptionGroupHandler.addUserToGroup subscription?.admin_id, userEmail, callback
+
 
 
 buildUserViewModel = (user)->

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

@@ -64,36 +64,37 @@ describe "SubscriptionGroupHandler", ->
 			"logger-sharelatex": 
 				err:->
 				log:->
+				warn:->
 
 
 	describe "addUserToGroup", ->
 		it "should find or create the user", (done)->
-			@LimitationsManager.hasGroupMembersLimitReached.callsArgWith(1, null, false)
-			@Handler.addUserToGroup @subscription, @newEmail, (err)=>
+			@LimitationsManager.hasGroupMembersLimitReached.callsArgWith(1, null, false, @subscription)
+			@Handler.addUserToGroup @adminUser_id, @newEmail, (err)=>
 				@UserCreator.getUserOrCreateHoldingAccount.calledWith(@newEmail).should.equal true
 				done()
 
 		it "should add the user to the group", (done)->
-			@LimitationsManager.hasGroupMembersLimitReached.callsArgWith(1, null, false)
-			@Handler.addUserToGroup @subscription, @newEmail, (err)=>
+			@LimitationsManager.hasGroupMembersLimitReached.callsArgWith(1, null, false, @subscription)
+			@Handler.addUserToGroup @adminUser_id, @newEmail, (err)=>
 				@SubscriptionUpdater.addUserToGroup.calledWith(@adminUser_id, @user._id).should.equal true
 				done()
 				
 		it "should not add the user to the group if the limit has been reached", (done)->
-			@LimitationsManager.hasGroupMembersLimitReached.callsArgWith(1, null, true)
-			@Handler.addUserToGroup @subscription, @newEmail, (err)=>
+			@LimitationsManager.hasGroupMembersLimitReached.callsArgWith(1, null, true, @subscription)
+			@Handler.addUserToGroup @adminUser_id, @newEmail, (err)=>
 				@SubscriptionUpdater.addUserToGroup.called.should.equal false
 				done()
 
 		it "should return error that limit has been reached", (done)->
-			@LimitationsManager.hasGroupMembersLimitReached.callsArgWith(1, null, true)
-			@Handler.addUserToGroup @subscription, @newEmail, (err)=>
+			@LimitationsManager.hasGroupMembersLimitReached.callsArgWith(1, null, true, @subscription)
+			@Handler.addUserToGroup @adminUser_id, @newEmail, (err)=>
 				err.limitReached.should.equal true
 				done()
 
 		it "should mark any notification as read if it is part of a licence", (done)->
-			@LimitationsManager.hasGroupMembersLimitReached.callsArgWith(1, null, false)
-			@Handler.addUserToGroup @subscription, @newEmail, (err)=>
+			@LimitationsManager.hasGroupMembersLimitReached.callsArgWith(1, null, false, @subscription)
+			@Handler.addUserToGroup @adminUser_id, @newEmail, (err)=>
 				@NotificationsBuilder.groupPlan.calledWith(@user, {subscription_id:@subscription._id}).should.equal true
 				@readStub.called.should.equal true
 				done()
@@ -175,14 +176,13 @@ describe "SubscriptionGroupHandler", ->
 	describe "processGroupVerification", ->
 		beforeEach ->
 			@token = "31dDAd2Da"
-			@admin_id = "eDSda1ew"
-			@SubscriptionLocator.getSubscription.callsArgWith(1, null, @Subscription)
+			@SubscriptionLocator.getSubscription.callsArgWith(1, null, @subscription)
 			@Handler.addUserToGroup = sinon.stub().callsArgWith(2)
 
 		it "should addUserToGroup", (done)->
 			@OneTimeTokenHandler.getValueFromTokenAndExpire.callsArgWith(1, null, @subscription_id)
 			@Handler.processGroupVerification @email, @subscription_id, @token, (err)=>
-				@Handler.addUserToGroup.calledWith(@Subscription, @email).should.equal true
+				@Handler.addUserToGroup.calledWith(@adminUser_id, @email).should.equal true
 				done()
 
 		it "should return token_not_found error if it couldn't get the token", (done)->