Use an email based invite system for groups, not holding accounts

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

@@ -64,8 +64,9 @@ module.exports =
 			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"
+			currentTotal = (subscription.member_ids or []).length + (subscription.invited_emails or []).length
+			limitReached = currentTotal >= subscription.membersLimit
+			logger.log user_id:user_id, limitReached:limitReached, currentTotal: currentTotal, membersLimit: subscription.membersLimit, "checking if subscription members limit has been reached"
 			callback(err, limitReached, subscription)
 
 getOwnerIdOfProject = (project_id, callback)->

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

@@ -33,6 +33,14 @@ module.exports =
 				return res.sendStatus 500
 			res.send()
 
+	removeEmailInviteFromGroup: (req, res)->
+		adminUserId = AuthenticationController.getLoggedInUserId(req)
+		email = req.params.email
+		logger.log {adminUserId, email}, "removing email invite from group subscription"
+		SubscriptionGroupHandler.removeEmailInviteFromGroup adminUserId, email, (err)->
+			return next(error) if error?
+			res.send()
+
 	removeSelfFromGroup: (req, res)->
 		adminUserId = req.query.admin_user_id
 		userToRemove_id = AuthenticationController.getLoggedInUserId(req)

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

@@ -1,6 +1,5 @@
 async = require("async")
 _ = require("underscore")
-UserCreator = require("../User/UserCreator")
 SubscriptionUpdater = require("./SubscriptionUpdater")
 SubscriptionLocator = require("./SubscriptionLocator")
 UserLocator = require("../User/UserLocator")
@@ -15,36 +14,40 @@ module.exports = SubscriptionGroupHandler =
 
 	addUserToGroup: (adminUserId, newEmail, callback)->
 		logger.log adminUserId:adminUserId, newEmail:newEmail, "adding user to group"
-		UserCreator.getUserOrCreateHoldingAccount newEmail, (err, user)->
+		LimitationsManager.hasGroupMembersLimitReached adminUserId, (err, limitReached, subscription)->
 			if err?
-				logger.err err:err,  adminUserId:adminUserId, newEmail:newEmail, "error creating user for holding account"
+				logger.err err:err, adminUserId:adminUserId, newEmail:newEmail, "error checking if limit reached for group plan"
 				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?
-					logger.err err:err, adminUserId:adminUserId, 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"
-					return callback(limitReached:limitReached)
-				SubscriptionUpdater.addUserToGroup adminUserId, user._id, (err)->
-					if err?
-						logger.err err:err, "error adding user to group"
-						return callback(err)
-					NotificationsBuilder.groupPlan(user, {subscription_id:subscription._id}).read()
-					userViewModel = buildUserViewModel(user)
-					callback(err, userViewModel)
+			if limitReached
+				logger.err adminUserId:adminUserId, newEmail:newEmail, "group subscription limit reached not adding user to group"
+				return callback(limitReached:limitReached)
+			UserLocator.findByEmail newEmail, (err, user)->
+				return callback(err) if err?
+				if user?
+					SubscriptionUpdater.addUserToGroup adminUserId, user._id, (err)->
+						if err?
+							logger.err err:err, "error adding user to group"
+							return callback(err)
+						NotificationsBuilder.groupPlan(user, {subscription_id:subscription._id}).read()
+						userViewModel = buildUserViewModel(user)
+						callback(err, userViewModel)
+				else
+					SubscriptionUpdater.addEmailInviteToGroup adminUserId, newEmail, (err) ->
+						return callback(err) if err?
+						userViewModel = buildEmailInviteViewModel(newEmail)
+						callback(err, userViewModel)
 
 	removeUserFromGroup: (adminUser_id, userToRemove_id, callback)->
 		SubscriptionUpdater.removeUserFromGroup adminUser_id, userToRemove_id, callback
-
+	
+	removeEmailInviteFromGroup: (adminUser_id, email, callback) ->
+		SubscriptionUpdater.removeEmailInviteFromGroup adminUser_id, email, callback
 
 	getPopulatedListOfMembers: (adminUser_id, callback)->
 		SubscriptionLocator.getUsersSubscription adminUser_id, (err, subscription)-> 
 			users = []
+			for email in subscription.invited_emails or []
+				users.push buildEmailInviteViewModel(email)
 			jobs = _.map subscription.member_ids, (user_id)->
 				return (cb)->
 					UserLocator.findById user_id, (err, user)->
@@ -91,7 +94,21 @@ module.exports = SubscriptionGroupHandler =
 					return callback()
 				SubscriptionGroupHandler.addUserToGroup subscription?.admin_id, userEmail, callback
 
-
+	convertEmailInvitesToMemberships: (email, user_id, callback = (err) ->) ->
+		SubscriptionLocator.getGroupsWithEmailInvite email, (err, groups = []) ->
+			return callback(err) if err?
+			logger.log {email, user_id, groups}, "found groups to convert from email invite to member"
+			jobs = []
+			for group in groups
+				do (group) ->
+					jobs.push (cb) ->
+						SubscriptionUpdater.removeEmailInviteFromGroup group.admin_id, email, (err) ->
+							return cb(err) if err?
+							SubscriptionUpdater.addUserToGroup group.admin_id, user_id, (err) ->
+								return cb(err) if err?
+								logger.log {group_id: group._id, user_id, email}, "converted email invite to group membership"
+								return cb()
+			async.series jobs, callback
 
 buildUserViewModel = (user)->
 	u = 
@@ -101,3 +118,9 @@ buildUserViewModel = (user)->
 		holdingAccount: user.holdingAccount
 		_id: user._id
 	return u
+
+buildEmailInviteViewModel = (email) ->
+	return {
+		email: email
+		holdingAccount: true
+	}

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

@@ -30,3 +30,6 @@ module.exports =
 
 	getGroupSubscriptionMemberOf: (user_id, callback)->
 		Subscription.findOne {member_ids: user_id}, {_id:1, planCode:1}, callback
+
+	getGroupsWithEmailInvite: (email, callback) ->
+		Subscription.find { invited_emails: email }, callback

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

@@ -25,6 +25,7 @@ module.exports =
 		webRouter.post '/subscription/group/user', AuthenticationController.requireLogin(),  SubscriptionGroupController.addUserToGroup
 		webRouter.get '/subscription/group/export',  AuthenticationController.requireLogin(), SubscriptionGroupController.exportGroupCsv
 		webRouter.delete '/subscription/group/user/:user_id', AuthenticationController.requireLogin(), SubscriptionGroupController.removeUserFromGroup
+		webRouter.delete '/subscription/group/email/:email', AuthenticationController.requireLogin(), SubscriptionGroupController.removeEmailInviteFromGroup
 		webRouter.delete '/subscription/group/user', AuthenticationController.requireLogin(), SubscriptionGroupController.removeSelfFromGroup
 
 

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

@@ -35,6 +35,14 @@ module.exports = SubscriptionUpdater =
 				logger.err err:err, searchOps:searchOps, insertOperation:insertOperation, "error findy and modify add user to group"
 				return callback(err)
 			UserFeaturesUpdater.updateFeatures user_id, subscription.planCode, callback
+	
+	addEmailInviteToGroup: (adminUser_id, email, callback) ->
+		logger.log {adminUser_id, email}, "adding email into mongo subscription"
+		searchOps = 
+			admin_id: adminUser_id
+		insertOperation =
+			"$addToSet": {invited_emails: email}
+		Subscription.findAndModify searchOps, insertOperation, callback
 
 	removeUserFromGroup: (adminUser_id, user_id, callback)->
 		searchOps = 
@@ -47,6 +55,12 @@ module.exports = SubscriptionUpdater =
 				return callback(err)
 			SubscriptionUpdater._setUsersMinimumFeatures user_id, callback
 
+	removeEmailInviteFromGroup: (adminUser_id, email, callback)->
+		Subscription.update {
+			admin_id: adminUser_id
+		}, "$pull": {
+			invited_emails: email
+		}, callback
 
 	_createNewSubscription: (adminUser_id, callback)->
 		logger.log adminUser_id:adminUser_id, "creating new subscription"

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

@@ -7,19 +7,22 @@ logger = require("logger-sharelatex")
 module.exports = UserHandler =
 
 	populateGroupLicenceInvite: (user, callback = ->)->
-		logger.log user_id:user._id, "populating any potential group licence invites"
-		licence = SubscriptionDomainHandler.getLicenceUserCanJoin user
-		if !licence?
-			return callback()
+		SubscriptionGroupHandler.convertEmailInvitesToMemberships user.email, user._id, (err) ->
+			return callback(err) if err?
 
-		SubscriptionGroupHandler.isUserPartOfGroup user._id, licence.subscription_id, (err, 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"
+			logger.log user_id:user._id, "populating any potential group licence invites"
+			licence = SubscriptionDomainHandler.getLicenceUserCanJoin user
+			if !licence?
 				return callback()
-			else
-				NotificationsBuilder.groupPlan(user, licence).create(callback)
+
+			SubscriptionGroupHandler.isUserPartOfGroup user._id, licence.subscription_id, (err, 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)
 
 	setupLoginData: (user, callback = ->)->
 		@populateGroupLicenceInvite user, callback

services/web/app/coffee/models/Subscription.coffee

@@ -6,7 +6,8 @@ ObjectId = Schema.ObjectId
 
 SubscriptionSchema = new Schema
 	admin_id     :   {type:ObjectId, ref:'User', index: {unique: true, dropDups: true}}
-	member_ids 	 :   [ type:ObjectId, ref:'User' ]
+	member_ids   :   [ type:ObjectId, ref:'User' ]
+	invited_emails:  [ String ]
 	recurlySubscription_id : String
 	planCode 	 : {type: String}
 	groupPlan	 : {type: Boolean, default: false}

services/web/public/coffee/main/group-members.coffee

@@ -33,9 +33,13 @@ define [
 		$scope.removeMembers = () ->
 			for user in $scope.selectedUsers
 				do (user) ->
+					if user.holdingAccount
+						url = "/subscription/group/email/#{user.email}"
+					else
+						url = "/subscription/group/user/#{user._id}"
 					queuedHttp({
 						method: "DELETE",
-						url: "/subscription/group/user/#{user._id}"
+						url: url
 						headers:
 							"X-Csrf-Token": window.csrfToken
 					})

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

@@ -257,24 +257,25 @@ describe "LimitationsManager", ->
 		beforeEach ->
 			@user_id = "12312"
 			@subscription =
-				membersLimit: 2
+				membersLimit: 3
 				member_ids: ["", ""]
+				invited_emails: ["bob@example.com"]
 
-		it "should return true if the limit is hit", (done)->
+		it "should return true if the limit is hit (including members and invites)", (done)->
 			@SubscriptionLocator.getUsersSubscription.callsArgWith(1, null, @subscription)
 			@LimitationsManager.hasGroupMembersLimitReached @user_id, (err, limitReached)->
 				limitReached.should.equal true
 				done()
 
-		it "should return false if the limit is not hit", (done)->
-			@subscription.membersLimit = 3
+		it "should return false if the limit is not hit (including members and invites)", (done)->
+			@subscription.membersLimit = 4
 			@SubscriptionLocator.getUsersSubscription.callsArgWith(1, null, @subscription)
 			@LimitationsManager.hasGroupMembersLimitReached @user_id, (err, limitReached)->
 				limitReached.should.equal false
 				done()
 
-		it "should return true if the limit has been exceded", (done)->
-			@subscription.membersLimit = 0
+		it "should return true if the limit has been exceded (including members and invites)", (done)->
+			@subscription.membersLimit = 2
 			@SubscriptionLocator.getUsersSubscription.callsArgWith(1, null, @subscription)
 			@LimitationsManager.hasGroupMembersLimitReached @user_id, (err, limitReached)->
 				limitReached.should.equal true

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

@@ -24,6 +24,7 @@ describe "SubscriptionGroupController", ->
 		@GroupHandler =
 			addUserToGroup: sinon.stub().callsArgWith(2, null, @user)
 			removeUserFromGroup: sinon.stub().callsArgWith(2)
+			removeEmailInviteFromGroup: sinon.stub().callsArgWith(2)
 			isUserPartOfGroup: sinon.stub()
 			sendVerificationEmail:sinon.stub()
 			processGroupVerification:sinon.stub()
@@ -69,7 +70,7 @@ describe "SubscriptionGroupController", ->
 
 
 	describe "removeUserFromGroup", ->
-		it "should use the admin id for the logged in user and take the email address from the body", (done)->
+		it "should use the admin id for the logged in user and take the user id from the params", (done)->
 			userIdToRemove = "31231"
 			@req.params = user_id: userIdToRemove
 
@@ -79,6 +80,15 @@ describe "SubscriptionGroupController", ->
 					done()
 			@Controller.removeUserFromGroup @req, res
 
+	describe "removeEmailInviteFromGroup", ->
+		it "should use the admin id for the logged in user and take the email from the params", (done)->
+			email = "jo@example.com"
+			@req.params = email: email
+			res =
+				send : =>
+					@GroupHandler.removeEmailInviteFromGroup.calledWith(@adminUserId, email).should.equal true
+					done()
+			@Controller.removeEmailInviteFromGroup @req, res
 
 	describe "renderSubscriptionGroupAdminPage", ->
 		it "should redirect you if you don't have a group account", (done)->

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

@@ -11,6 +11,7 @@ describe "SubscriptionGroupHandler", ->
 		@adminUser_id = "12321"
 		@newEmail = "bob@smith.com"
 		@user_id = "3121321"
+		@email = "jim@example.com"
 		@user = {_id:@user_id, email:@newEmail}
 		@subscription_id = "31DSd1123D"
 
@@ -29,9 +30,12 @@ describe "SubscriptionGroupHandler", ->
 		@SubscriptionUpdater =
 			addUserToGroup: sinon.stub().callsArgWith(2)
 			removeUserFromGroup: sinon.stub().callsArgWith(2)
+			addEmailInviteToGroup: sinon.stub().callsArgWith(2)
+			removeEmailInviteFromGroup: sinon.stub().callsArgWith(2)
 
 		@UserLocator =
 			findById: sinon.stub()
+			findByEmail: sinon.stub()
 
 		@LimitationsManager =
 			hasGroupMembersLimitReached: sinon.stub()
@@ -68,14 +72,16 @@ describe "SubscriptionGroupHandler", ->
 
 
 	describe "addUserToGroup", ->
-		it "should find or create the user", (done)->
+		beforeEach ->
 			@LimitationsManager.hasGroupMembersLimitReached.callsArgWith(1, null, false, @subscription)
+			@UserLocator.findByEmail.callsArgWith(1, null, @user)
+			
+		it "should find the user", (done)->
 			@Handler.addUserToGroup @adminUser_id, @newEmail, (err)=>
-				@UserCreator.getUserOrCreateHoldingAccount.calledWith(@newEmail).should.equal true
+				@UserLocator.findByEmail.calledWith(@newEmail).should.equal true
 				done()
 
 		it "should add the user to the group", (done)->
-			@LimitationsManager.hasGroupMembersLimitReached.callsArgWith(1, null, false, @subscription)
 			@Handler.addUserToGroup @adminUser_id, @newEmail, (err)=>
 				@SubscriptionUpdater.addUserToGroup.calledWith(@adminUser_id, @user._id).should.equal true
 				done()
@@ -93,12 +99,16 @@ describe "SubscriptionGroupHandler", ->
 				done()
 
 		it "should mark any notification as read if it is part of a licence", (done)->
-			@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()
-				
+		
+		it "should add an email invite if no user is found", (done) ->
+			@UserLocator.findByEmail.callsArgWith(1, null, null)
+			@Handler.addUserToGroup @adminUser_id, @newEmail, (err)=>
+				@SubscriptionUpdater.addEmailInviteToGroup.calledWith(@adminUser_id, @newEmail).should.equal true
+				done()
 
 	describe "removeUserFromGroup", ->
 
@@ -138,6 +148,16 @@ describe "SubscriptionGroupHandler", ->
 				assert.deepEqual users[1], {_id:@subscription.member_ids[1]}
 				assert.deepEqual users[2], {_id:@subscription.member_ids[2]}
 				done()
+		
+		it "should return any invited users", (done) ->
+			@subscription.invited_emails = ["jo@example.com", "charlie@example.com"]
+			@Handler.getPopulatedListOfMembers @adminUser_id, (err, users)=>
+				users[0].email.should.equal "jo@example.com"
+				users[0].holdingAccount.should.equal true
+				users[1].email.should.equal "charlie@example.com"
+				users[1].holdingAccount.should.equal true
+				users.length.should.equal @subscription.invited_emails.length
+				done()
 
 	describe "isUserPartOfGroup", ->
 		beforeEach ->
@@ -191,7 +211,28 @@ describe "SubscriptionGroupHandler", ->
 				err.should.equal "token_not_found"
 				done()
 
+	describe "convertEmailInvitesToMemberships", ->
+		beforeEach ->
+			@SubscriptionLocator.getGroupsWithEmailInvite = sinon.stub().yields(null, @groups = [{ admin_id: "group-1" }, { admin_id: "group-2" }])
 
-
-
+		it "should get groups with the email address invited to", (done) ->
+			@Handler.convertEmailInvitesToMemberships @email, @user_id, (err) =>
+				@SubscriptionLocator.getGroupsWithEmailInvite.calledWith(@email).should.equal true
+				done()
+		
+		it "should remove the email from each group", (done) ->
+			@Handler.convertEmailInvitesToMemberships @email, @user_id, (err) =>
+				for group in @groups
+					@SubscriptionUpdater.removeEmailInviteFromGroup
+						.calledWith(group.admin_id, @email)
+						.should.equal true
+				done()
+		
+		it "should add the user to each group", (done) ->
+			@Handler.convertEmailInvitesToMemberships @email, @user_id, (err) =>
+				for group in @groups
+					@SubscriptionUpdater.addUserToGroup
+						.calledWith(group.admin_id, @user_id)
+						.should.equal true
+				done()
 

services/web/test/UnitTests/coffee/User/UserHandlerTests.coffee

@@ -19,6 +19,7 @@ describe "UserHandler", ->
 
 		@SubscriptionGroupHandler =
 			isUserPartOfGroup:sinon.stub()
+			convertEmailInvitesToMemberships: sinon.stub().callsArgWith(2)
 		@createStub = sinon.stub().callsArgWith(0)
 		@NotificationsBuilder = 
 			groupPlan:sinon.stub().returns({create:@createStub})
@@ -30,11 +31,15 @@ describe "UserHandler", ->
 			"../Subscription/SubscriptionGroupHandler":@SubscriptionGroupHandler
 
 	describe "populateGroupLicenceInvite", ->
-
 		describe "no licence", ->
 			beforeEach (done)->
 				@SubscriptionDomainHandler.getLicenceUserCanJoin.returns()
 				@UserHandler.populateGroupLicenceInvite @user, done
+			
+			it "should call convertEmailInvitesToMemberships", ->
+				@SubscriptionGroupHandler.convertEmailInvitesToMemberships
+					.calledWith(@user.email, @user._id)
+					.should.equal true
 
 			it "should not call NotificationsBuilder", (done)->
 				@NotificationsBuilder.groupPlan.called.should.equal false
@@ -45,7 +50,6 @@ describe "UserHandler", ->
 				done()
 
 		describe "with matching licence user is not in", ->
-
 			beforeEach (done)->
 				@SubscriptionDomainHandler.getLicenceUserCanJoin.returns(@licence)
 				@SubscriptionGroupHandler.isUserPartOfGroup.callsArgWith(2, null, false)
@@ -54,11 +58,13 @@ describe "UserHandler", ->
 			it "should create notifcation", (done)->
 				@NotificationsBuilder.groupPlan.calledWith(@user, @licence).should.equal true
 				done()
-
-
+			
+			it "should call convertEmailInvitesToMemberships", ->
+				@SubscriptionGroupHandler.convertEmailInvitesToMemberships
+					.calledWith(@user.email, @user._id)
+					.should.equal true
 
 		describe "with matching licence user is already in", ->
-
 			beforeEach (done)->
 				@SubscriptionDomainHandler.getLicenceUserCanJoin.returns(@licence)
 				@SubscriptionGroupHandler.isUserPartOfGroup.callsArgWith(2, null, true)
@@ -66,4 +72,9 @@ describe "UserHandler", ->
 
 			it "should create notifcation", (done)->
 				@NotificationsBuilder.groupPlan.called.should.equal false
-				done()
+				done()
+			
+			it "should call convertEmailInvitesToMemberships", ->
+				@SubscriptionGroupHandler.convertEmailInvitesToMemberships
+					.calledWith(@user.email, @user._id)
+					.should.equal true