improve calls for invite notifications and auto mark as read

services/web/app/coffee/Features/Notifications/NotificationsHandler.coffee

@@ -9,7 +9,7 @@ module.exports =
 		opts = 
 			uri: "#{settings.apis.notifications.url}/user/#{user_id}"
 			json: true
-			timeout: 2000
+			timeout: oneSecond
 		request.get opts, (err, res, unreadNotifications)->
 			statusCode =  if res? then res.statusCode else 500
 			if err? or statusCode != 200
@@ -24,7 +24,7 @@ module.exports =
 	createNotification: (user_id, key, templateKey, messageOpts, callback)->
 		opts = 
 			uri: "#{settings.apis.notifications.url}/user/#{user_id}"
-			timeout: 1000
+			timeout: oneSecond
 			json: {
 				key:key
 				messageOpts:messageOpts
@@ -36,10 +36,11 @@ module.exports =
 	markAsReadWithKey: (user_id, key, callback)->
 		opts = 
 			uri: "#{settings.apis.notifications.url}/user/#{user_id}"
-			timeout: 1000
+			timeout: oneSecond
 			json: {
 				key:key
 			}
+		logger.log user_id:user_id, key:key, "sending mark notification as read with key to notifications api"
 		request.del opts, callback
 	
 
@@ -47,5 +48,5 @@ module.exports =
 		opts =
 			uri: "#{settings.apis.notifications.url}/user/#{user_id}/notification/#{notification_id}"
 			timeout:oneSecond
-		logger.log user_id:user_id, notification_id:notification_id, "send mark notification to notifications api"
+		logger.log user_id:user_id, notification_id:notification_id, "sending mark notification as read to notifications api"
 		request.del opts, callback

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

@@ -70,7 +70,9 @@ module.exports =
 		subscription_id = req.params.subscription_id
 		if !SubscriptionDomainHandler.findDomainLicenceBySubscriptionId(subscription_id)?
 			return ErrorsController.notFound(req, res)
-		SubscriptionGroupHandler.processGroupVerification req.session.user.email, subscription_id, req.query.token, (err)->
+		email = req?.session?.user?.email
+		logger.log subscription_id:subscription_id, user_id:req?.session?.user?._id, email:email, "starting the completion of joining group"
+		SubscriptionGroupHandler.processGroupVerification email, subscription_id, req.query?.token, (err)->
 			if err? and err == "token_not_found"
 				res.redirect "/user/subscription/#{subscription_id}/group/invited?expired=true"
 			else if err?

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

@@ -9,15 +9,17 @@ logger = require("logger-sharelatex")
 OneTimeTokenHandler = require("../Security/OneTimeTokenHandler")
 EmailHandler = require("../Email/EmailHandler")
 settings = require("settings-sharelatex")
+NotificationsBuilder = require("../Notifications/NotificationsBuilder")
 
 module.exports = SubscriptionGroupHandler =
 
-	addUserToGroup: (adminUser_id, newEmail, callback)->
+	addUserToGroup: (subscription, newEmail, callback)->
 		UserCreator.getUserOrCreateHoldingAccount newEmail, (err, user)->
-			LimitationsManager.hasGroupMembersLimitReached adminUser_id, (err, limitReached)->
+			LimitationsManager.hasGroupMembersLimitReached subscription.admin_id, (err, limitReached)->
 				if limitReached
 					return callback(limitReached:limitReached)
-				SubscriptionUpdater.addUserToGroup adminUser_id, user._id, (err)->
+				SubscriptionUpdater.addUserToGroup subscription.admin_id, user._id, (err)->
+					NotificationsBuilder.groupPlan(user, {subscription_id:subscription._id}).read()
 					userViewModel = buildUserViewModel(user)
 					callback(err, userViewModel)
 
@@ -66,7 +68,7 @@ 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.admin_id, userEmail, callback
+				SubscriptionGroupHandler.addUserToGroup subscription, userEmail, callback
 
 
 buildUserViewModel = (user)->

services/web/app/coffee/Features/User/UserHandler.coffee

@@ -1,6 +1,8 @@
 SubscriptionDomainHandler = require("../Subscription/SubscriptionDomainHandler")
 NotificationsBuilder = require("../Notifications/NotificationsBuilder")
 SubscriptionGroupHandler = require("../Subscription/SubscriptionGroupHandler")
+logger = require("logger-sharelatex")
+
 
 module.exports = UserHandler =
 
@@ -10,8 +12,11 @@ module.exports = UserHandler =
 			return callback()
 
 		SubscriptionGroupHandler.isUserPartOfGroup user._id, licence.subscription_id, (err, alreadyPartOfGroup)->
-			if err? or alreadyPartOfGroup
+			if err?
 				return callback(err)
+			else if alreadyPartOfGroup
+				logger.log user_id:user._id, "user already part of group, not creating notifcation for them"
+				return callback()
 			else
 				NotificationsBuilder.groupPlan(user, licence).create(callback)
 

services/web/test/UnitTests/coffee/Notifications/NotificationsHandlerTests.coffee

@@ -32,7 +32,7 @@ describe 'NotificationsHandler', ->
 				getOpts =
 					uri: "#{notificationUrl}/user/#{user_id}"
 					json:true
-					timeout:2000
+					timeout:1000
 				@request.get.calledWith(getOpts).should.equal true
 				done()
 

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

@@ -12,6 +12,11 @@ describe "SubscriptionGroupHandler", ->
 		@newEmail = "bob@smith.com"
 		@user_id = "3121321"
 		@user = {_id:@user_id, email:@newEmail}
+		@subscription_id = "31DSd1123D"
+
+		@subscription =
+			admin_id:@adminUser_id
+			_id:@subscription_id
 
 		@SubscriptionLocator = 
 			getUsersSubscription: sinon.stub()
@@ -41,6 +46,10 @@ describe "SubscriptionGroupHandler", ->
 		@settings = 
 			siteUrl:"http://www.sharelatex.com"
 
+		@readStub = sinon.stub()
+		@NotificationsBuilder =
+			groupPlan: sinon.stub().returns({read:@readStub})
+
 		@Handler = SandboxedModule.require modulePath, requires:
 			"logger-sharelatex": log:->
 			"../User/UserCreator": @UserCreator
@@ -51,6 +60,7 @@ describe "SubscriptionGroupHandler", ->
 			"../Security/OneTimeTokenHandler":@OneTimeTokenHandler
 			"../Email/EmailHandler":@EmailHandler
 			"settings-sharelatex":@settings
+			"../Notifications/NotificationsBuilder": @NotificationsBuilder
 			"logger-sharelatex": 
 				err:->
 				log:->
@@ -59,28 +69,36 @@ describe "SubscriptionGroupHandler", ->
 	describe "addUserToGroup", ->
 		it "should find or create the user", (done)->
 			@LimitationsManager.hasGroupMembersLimitReached.callsArgWith(1, null, false)
-			@Handler.addUserToGroup @adminUser_id, @newEmail, (err)=>
+			@Handler.addUserToGroup @subscription, @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 @adminUser_id, @newEmail, (err)=>
+			@Handler.addUserToGroup @subscription, @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 @adminUser_id, @newEmail, (err)=>
+			@Handler.addUserToGroup @subscription, @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 @adminUser_id, @newEmail, (err)=>
+			@Handler.addUserToGroup @subscription, @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)=>
+				@NotificationsBuilder.groupPlan.calledWith(@user, {subscription_id:@subscription._id}).should.equal true
+				@readStub.called.should.equal true
+				done()
+				
+
 	describe "removeUserFromGroup", ->
 
 		it "should call the subscription updater to remove the user", (done)->
@@ -157,15 +175,14 @@ describe "SubscriptionGroupHandler", ->
 	describe "processGroupVerification", ->
 		beforeEach ->
 			@token = "31dDAd2Da"
-			@subscription_id = "31DSd1123D"
 			@admin_id = "eDSda1ew"
-			@SubscriptionLocator.getSubscription.callsArgWith(1, null, {admin_id:@admin_id})
+			@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(@admin_id, @email).should.equal true
+				@Handler.addUserToGroup.calledWith(@Subscription, @email).should.equal true
 				done()
 
 		it "should return token_not_found error if it couldn't get the token", (done)->