Refactor adding and removing collaborators to not go through EditorController

services/web/app/coffee/Features/Collaborators/CollaboratorsController.coffee

@@ -1,6 +1,11 @@
 ProjectGetter = require "../Project/ProjectGetter"
 CollaboratorsHandler = require "./CollaboratorsHandler"
-EditorController = require "../Editor/EditorController"
+CollaboratorsEmailHandler = require "./CollaboratorsEmailHandler"
+ProjectEditorHandler = require "../Project/ProjectEditorHandler"
+EditorRealTimeController = require "../Editor/EditorRealTimeController"
+UserGetter = require "../User/UserGetter"
+LimitationsManager = require "../Subscription/LimitationsManager"
+mimelib = require("mimelib")
 
 module.exports = CollaboratorsController =
 	getCollaborators: (req, res, next = (error) ->) ->
@@ -11,28 +16,49 @@ module.exports = CollaboratorsController =
 				CollaboratorsController._formatCollaborators project, (error, collaborators) ->
 					return next(error) if error?
 					res.send(JSON.stringify(collaborators))
-
-	removeSelfFromProject: (req, res, next = (error) ->) ->
-		user_id = req.session?.user?._id
-		if !user_id?
-			return next(new Error("User should be logged in"))
-		CollaboratorsHandler.removeUserFromProject req.params.project_id, user_id, (error) ->
-			return next(error) if error?
-			res.sendStatus 204
 			
 	addUserToProject: (req, res, next) ->
 		project_id = req.params.Project_id
-		{email, privileges} = req.body
-		EditorController.addUserToProject project_id, email, privileges, (error, user) ->
+		LimitationsManager.isCollaboratorLimitReached project_id, (error, limit_reached) =>
 			return next(error) if error?
-			res.json user: user
-			
+
+			if limit_reached
+				return res.json { user: false }
+			else
+				{email, privileges} = req.body
+				
+				email = mimelib.parseAddresses(email or "")[0]?.address?.toLowerCase()
+				if !email? or email == ""
+					return res.status(400).send("invalid email address")
+					
+				CollaboratorsHandler.addEmailToProject project_id, email, privileges, (error, user_id) =>
+					return next(error) if error?
+					UserGetter.getUser user_id, (error, raw_user) ->
+						return next(error) if error?
+						user = ProjectEditorHandler.buildUserModelView(raw_user, privileges)
+						CollaboratorsEmailHandler.notifyUserOfProjectShare project_id, user.email
+						EditorRealTimeController.emitToRoom(project_id, 'userAddedToProject', user, privileges)
+						return res.json { user: user }
+
 	removeUserFromProject: (req, res, next) ->
 		project_id = req.params.Project_id
 		user_id    = req.params.user_id
-		EditorController.removeUserFromProject project_id, user_id, (error)->
+		CollaboratorsController._removeUserIdFromProject project_id, user_id, (error) ->
 			return next(error) if error?
 			res.sendStatus 204
+	
+	removeSelfFromProject: (req, res, next = (error) ->) ->
+		project_id = req.params.Project_id
+		user_id    = req.session?.user?._id
+		CollaboratorsController._removeUserIdFromProject project_id, user_id, (error) ->
+			return next(error) if error?
+			res.sendStatus 204
+
+	_removeUserIdFromProject: (project_id, user_id, callback = (error) ->) ->
+		CollaboratorsHandler.removeUserFromProject project_id, user_id, (error)->
+			return callback(error) if error?
+			EditorRealTimeController.emitToRoom(project_id, 'userRemovedFromProject', user_id)
+			callback()
 
 	_formatCollaborators: (project, callback = (error, collaborators) ->) ->
 		collaborators = []

services/web/app/coffee/Features/Collaborators/CollaboratorsEmailHandler.coffee

@@ -0,0 +1,25 @@
+Project = require("../../models/Project").Project
+EmailHandler = require("../Email/EmailHandler")
+Settings = require "settings-sharelatex"
+
+module.exports =
+	notifyUserOfProjectShare: (project_id, email, callback)->
+		Project
+			.findOne(_id: project_id )
+			.select("name owner_ref")
+			.populate('owner_ref')
+			.exec (err, project)->
+				emailOptions =
+					to: email
+					replyTo: project.owner_ref.email
+					project:
+						name: project.name
+						url: "#{Settings.siteUrl}/project/#{project._id}?" + [
+								"project_name=#{encodeURIComponent(project.name)}"
+								"user_first_name=#{encodeURIComponent(project.owner_ref.first_name)}"
+								"new_email=#{encodeURIComponent(email)}"
+								"r=#{project.owner_ref.referal_id}" # Referal
+								"rs=ci" # referral source = collaborator invite
+							].join("&")
+					owner: project.owner_ref
+				EmailHandler.sendEmail "projectSharedWithYou", emailOptions, callback

services/web/app/coffee/Features/Collaborators/CollaboratorsHandler.coffee

@@ -1,78 +1,44 @@
-User = require('../../models/User').User
+UserCreator = require('../User/UserCreator')
 Project = require("../../models/Project").Project
-Settings = require('settings-sharelatex')
-EmailHandler = require("../Email/EmailHandler")
+ProjectEntityHandler = require("../Project/ProjectEntityHandler")
 mimelib = require("mimelib")
 logger = require('logger-sharelatex')
-async = require("async")
 
-
-module.exports =
-
-	removeUserFromProject: (project_id, user_id, callback = ->)->
+module.exports = CollaboratorsHandler =
+	removeUserFromProject: (project_id, user_id, callback = (error) ->)->
 		logger.log user_id: user_id, project_id: project_id, "removing user"
 		conditions = _id:project_id
 		update = $pull:{}
 		update["$pull"] = collaberator_refs:user_id, readOnly_refs:user_id
 		Project.update conditions, update, (err)->
 			if err?
-				logger.err err: err, "problem removing user from project collaberators"
+				logger.error err: err, "problem removing user from project collaberators"
 			callback(err)
-
-	addUserToProject: (project_id, email, privilegeLevel, callback)->
-		emails = mimelib.parseAddresses(email)
+	
+	addEmailToProject: (project_id, unparsed_email, privilegeLevel, callback = (error, user) ->) ->
+		emails = mimelib.parseAddresses(unparsed_email)
 		email = emails[0]?.address?.toLowerCase()
-		return callback(new Error("no valid email provided")) if !email?
-		self = @
-		User.findOne {'email':email}, (err, user)->
-			async.waterfall [
-				(cb)->
-					if user?
-						return cb(null, user)
-					else 
-						self._createHoldingAccount email, cb
-				(@user, cb)=>
-					self._updateProjectWithUserPrivileges project_id, user, privilegeLevel, cb
-				(cb)->
-					self._notifyUserViaEmail project_id, email, cb
-			], (err)=>
-				callback(err, @user)
+		if !email? or email == ""
+			return callback(new Error("no valid email provided: '#{unparsed_email}'"))
+		UserCreator.getUserOrCreateHoldingAccount email, (error, user) ->
+			return callback(error) if error?
+			CollaboratorsHandler.addUserToProject project_id, user._id, privilegeLevel, (error) ->
+				return callback(error) if error?
+				return callback null, user._id
 
-	_createHoldingAccount: (email, callback)->
-		user = new User 'email':email, holdingAccount:true
-		user.save (err)->
-			callback(err, user)
-
-	_updateProjectWithUserPrivileges: (project_id, user, privilegeLevel, callback)->
+	addUserToProject: (project_id, user_id, privilegeLevel, callback = (error) ->)->
 		if privilegeLevel == 'readAndWrite'
-			level = {"collaberator_refs":user}
-			logger.log privileges: "readAndWrite", user: user, project_id: project_id, "adding user"
+			level = {"collaberator_refs":user_id}
+			logger.log {privileges: "readAndWrite", user_id, project_id}, "adding user"
 		else if privilegeLevel == 'readOnly'
-			level = {"readOnly_refs":user}
-			logger.log privileges: "readOnly", user: user, project_id: project_id, "adding user"
-		Project.update {_id: project_id}, {$push:level}, (err)->
-			callback(err)
-
-
-	_notifyUserViaEmail: (project_id, email, callback)->
-		Project.findOne(_id: project_id )
-		.select("name owner_ref")
-		.populate('owner_ref')
-		.exec (err, project)->
-			emailOptions =
-				to : email
-				replyTo  : project.owner_ref.email
-				project:
-					name: project.name
-					url: "#{Settings.siteUrl}/project/#{project._id}?" + [
-							"project_name=#{encodeURIComponent(project.name)}"
-							"user_first_name=#{encodeURIComponent(project.owner_ref.first_name)}"
-							"new_email=#{encodeURIComponent(email)}"
-							"r=#{project.owner_ref.referal_id}" # Referal
-							"rs=ci" # referral source = collaborator invite
-						].join("&")
-				owner: project.owner_ref
-			EmailHandler.sendEmail "projectSharedWithYou", emailOptions, callback
-
-
-
+			level = {"readOnly_refs":user_id}
+			logger.log {privileges: "readOnly", user_id, project_id}, "adding user"
+		else
+			return callback(new Error("unknown privilegeLevel: #{privilegeLevel}"))
+		Project.update { _id: project_id }, { $push:level }, (error) ->
+			return callback(error) if error?
+			# Flush to TPDS in background to add files to collaborator's Dropbox
+			ProjectEntityHandler.flushProjectToThirdPartyDataStore project_id, (error) ->
+				if error?
+					logger.error {err: error, project_id, user_id}, "error flushing to TPDS after adding collaborator"
+			callback()

services/web/app/coffee/Features/Collaborators/CollaboratorsRouter.coffee

@@ -4,7 +4,7 @@ AuthenticationController = require('../Authentication/AuthenticationController')
 
 module.exports =
 	apply: (webRouter, apiRouter) ->
-		webRouter.post '/project/:project_id/leave', AuthenticationController.requireLogin(), CollaboratorsController.removeSelfFromProject
+		webRouter.post '/project/:Project_id/leave', AuthenticationController.requireLogin(), CollaboratorsController.removeSelfFromProject
 		apiRouter.get  '/project/:Project_id/collaborators', SecurityManager.requestCanAccessProject(allow_auth_token: true), CollaboratorsController.getCollaborators
 
 		webRouter.post   '/project/:Project_id/users', SecurityManager.requestIsOwner, CollaboratorsController.addUserToProject

services/web/app/coffee/Features/Editor/EditorController.coffee

@@ -1,48 +1,18 @@
 logger = require('logger-sharelatex')
 Metrics = require('../../infrastructure/Metrics')
 sanitize = require('sanitizer')
-ProjectEditorHandler = require('../Project/ProjectEditorHandler')
 ProjectEntityHandler = require('../Project/ProjectEntityHandler')
 ProjectOptionsHandler = require('../Project/ProjectOptionsHandler')
 ProjectDetailsHandler = require('../Project/ProjectDetailsHandler')
 ProjectDeleter = require("../Project/ProjectDeleter")
-CollaboratorsHandler = require("../Collaborators/CollaboratorsHandler")
 DocumentUpdaterHandler = require('../DocumentUpdater/DocumentUpdaterHandler')
-LimitationsManager = require("../Subscription/LimitationsManager")
 EditorRealTimeController = require("./EditorRealTimeController")
 TrackChangesManager = require("../TrackChanges/TrackChangesManager")
-Settings = require('settings-sharelatex')
 async = require('async')
 LockManager = require("../../infrastructure/LockManager")
 _ = require('underscore')
-redis = require("redis-sharelatex")
-rclientPub = redis.createClient(Settings.redis.web)
-rclientSub = redis.createClient(Settings.redis.web)
 
 module.exports = EditorController =
-	addUserToProject: (project_id, email, privileges, callback = (error, collaborator_added)->)->
-		email = email.toLowerCase()
-		LimitationsManager.isCollaboratorLimitReached project_id, (error, limit_reached) =>
-			if error?
-				logger.error err:error, "error adding user to to project when checking if collaborator limit has been reached"
-				return callback(new Error("Something went wrong"))
-
-			if limit_reached
-				callback null, false
-			else
-				CollaboratorsHandler.addUserToProject project_id, email, privileges, (err, user)=>
-					return callback(err) if error?
-					# Flush to TPDS to add files to collaborator's Dropbox
-					ProjectEntityHandler.flushProjectToThirdPartyDataStore project_id, ->
-					EditorRealTimeController.emitToRoom(project_id, 'userAddedToProject', user, privileges)
-					callback null, ProjectEditorHandler.buildUserModelView(user, privileges)
-
-	removeUserFromProject: (project_id, user_id, callback = (error) ->)->
-		CollaboratorsHandler.removeUserFromProject project_id, user_id, (error) =>
-			return callback(error) if error?
-			EditorRealTimeController.emitToRoom(project_id, 'userRemovedFromProject', user_id)
-			callback()
-
 	setDoc: (project_id, doc_id, docLines, source, callback = (err)->)->
 		DocumentUpdaterHandler.setDocument project_id, doc_id, docLines, source, (err)=>
 			logger.log project_id:project_id, doc_id:doc_id, "notifying users that the document has been updated"

services/web/test/UnitTests/coffee/Collaborators/CollaboratorsControllerTests.coffee

@@ -11,15 +11,18 @@ ObjectId = require("mongojs").ObjectId
 
 describe "CollaboratorsController", ->
 	beforeEach ->
-		@CollaboratorsHandler =
-			removeUserFromProject:sinon.stub()
 		@CollaboratorsController = SandboxedModule.require modulePath, requires:
 			"../Project/ProjectGetter": @ProjectGetter = {}
-			"./CollaboratorsHandler": @CollaboratorsHandler
-			"../Editor/EditorController": @EditorController = {}
+			"./CollaboratorsHandler": @CollaboratorsHandler = {}
+			"./CollaboratorsEmailHandler": @CollaboratorsEmailHandler = {}
+			"../User/UserGetter": @UserGetter = {}
+			"../Editor/EditorRealTimeController": @EditorRealTimeController = {}
+			'../Subscription/LimitationsManager' : @LimitationsManager = {}
+			'../Project/ProjectEditorHandler' : @ProjectEditorHandler = {}
 		@res = new MockResponse()
 		@req = new MockRequest()
 
+		@project_id = "project-id-123"
 		@callback = sinon.stub()
 
 	describe "getCollaborators", ->
@@ -51,40 +54,89 @@ describe "CollaboratorsController", ->
 		it "should return the formatted collaborators", ->
 			@res.body.should.equal JSON.stringify(@collaborators)
 
-	describe "removeSelfFromProject", ->
-		beforeEach ->
-			@req.session =
-				user: _id: @user_id = "user-id-123"
-				destroy:->
-			@req.params = project_id: @project_id
-			@CollaboratorsHandler.removeUserFromProject = sinon.stub().callsArg(2)
-
-			@CollaboratorsController.removeSelfFromProject(@req, @res)
-
-		it "should remove the logged in user from the project", ->
-			@CollaboratorsHandler.removeUserFromProject.calledWith(@project_id, @user_id)
-
-		it "should return a success code", ->
-			@res.statusCode.should.equal 204
-			
 	describe "addUserToProject", ->
 		beforeEach ->
 			@req.params =
-				Project_id: @project_id = "project-id-123"
+				Project_id: @project_id
 			@req.body =
-				email: @email = "joe@example.com"
-				privileges: @privileges = "readAndWrite"
+				email: @email = "Joe@example.com"
+				privileges: @privileges = "readOnly"
 			@res.json = sinon.stub()
-			@EditorController.addUserToProject = sinon.stub().callsArgWith(3, null, @user = {"mock": "user"})
-			@CollaboratorsController.addUserToProject @req, @res
+			@user_id = "mock-user-id"
+			@raw_user = {
+				_id: @user_id, email: "joe@example.com", first_name: "Joe", last_name: "Example", unused: "foo"
+			}
+			@user_view = {
+				id: @user_id, first_name: "Joe", last_name: "Example", email: "joe@example.com"
+			}
+			@LimitationsManager.isCollaboratorLimitReached = sinon.stub().callsArgWith(1, null, false)
+			@ProjectEditorHandler.buildUserModelView = sinon.stub().returns(@user_view)
+			@CollaboratorsHandler.addEmailToProject = sinon.stub().callsArgWith(3, null, @user_id)
+			@UserGetter.getUser = sinon.stub().callsArgWith(1, null, @user)
+			@CollaboratorsEmailHandler.notifyUserOfProjectShare = sinon.stub()
+			@EditorRealTimeController.emitToRoom = sinon.stub()
+			@callback = sinon.stub()
+
+		describe "when the project can accept more collaborators", ->
+			beforeEach ->
+				@CollaboratorsController.addUserToProject @req, @res, @next
+
+			it "should add the user to the project", ->
+				@CollaboratorsHandler.addEmailToProject
+					.calledWith(@project_id, @email.toLowerCase(), @privileges)
+					.should.equal true
+
+			it "should emit a userAddedToProject event", ->
+				@EditorRealTimeController.emitToRoom
+					.calledWith(@project_id, "userAddedToProject", @user_view, @privileges)
+					.should.equal true
 			
-		it "should add the user to the project", ->
-			@EditorController.addUserToProject
-				.calledWith(@project_id, @email, @privileges)
-				.should.equal true
-				
-		it "should send the back the added user", ->
-			@res.json.calledWith(user: @user).should.equal true
+			it "should send an email to the shared-with user", ->
+				@CollaboratorsEmailHandler.notifyUserOfProjectShare
+					.calledWith(@project_id, @email.toLowerCase())
+					.should.equal true
+
+			it "should send the user as the response body", ->
+				@res.json
+					.calledWith({
+						user: @user_view
+					})
+					.should.equal true
+
+		describe "when the project cannot accept more collaborators", ->
+			beforeEach ->
+				@LimitationsManager.isCollaboratorLimitReached = sinon.stub().callsArgWith(1, null, true)
+				@CollaboratorsController.addUserToProject @req, @res, @next
+		
+			it "should not add the user to the project", ->
+				@CollaboratorsHandler.addEmailToProject.called.should.equal false
+		
+			it "should not emit a userAddedToProject event", ->
+				@EditorRealTimeController.emitToRoom.called.should.equal false
+
+			it "should send user: false as the response body", ->
+				@res.json
+					.calledWith({
+						user: false
+					})
+					.should.equal true
+
+		describe "when the email is not valid", ->
+			beforeEach ->
+				@req.body.email = "not-valid"
+				@res.status = sinon.stub().returns @res
+				@res.send = sinon.stub()
+				@CollaboratorsController.addUserToProject @req, @res, @next
+		
+			it "should not add the user to the project", ->
+				@CollaboratorsHandler.addEmailToProject.called.should.equal false
+		
+			it "should not emit a userAddedToProject event", ->
+				@EditorRealTimeController.emitToRoom.called.should.equal false
+
+			it "should return a 400 response", ->
+				@res.status.calledWith(400).should.equal true
+				@res.send.calledWith("invalid email address").should.equal true
 			
 	describe "removeUserFromProject", ->
 		beforeEach ->
@@ -92,17 +144,45 @@ describe "CollaboratorsController", ->
 				Project_id: @project_id = "project-id-123"
 				user_id: @user_id = "user-id-123"
 			@res.sendStatus = sinon.stub()
-			@EditorController.removeUserFromProject = sinon.stub().callsArg(2)
+			@EditorRealTimeController.emitToRoom = sinon.stub()
+			@CollaboratorsHandler.removeUserFromProject = sinon.stub().callsArg(2)
 			@CollaboratorsController.removeUserFromProject @req, @res
 			
 		it "should from the user from the project", ->
-			@EditorController.removeUserFromProject
+			@CollaboratorsHandler.removeUserFromProject
 				.calledWith(@project_id, @user_id)
 				.should.equal true
+		
+		it "should emit a userRemovedFromProject event to the proejct", ->
+			@EditorRealTimeController.emitToRoom
+				.calledWith(@project_id, 'userRemovedFromProject', @user_id)
+				.should.equal true
 				
 		it "should send the back a success response", ->
 			@res.sendStatus.calledWith(204).should.equal true
 
+	describe "removeSelfFromProject", ->
+		beforeEach ->
+			@req.session =
+				user: _id: @user_id = "user-id-123"
+			@req.params = Project_id: @project_id
+			@res.sendStatus = sinon.stub()
+			@EditorRealTimeController.emitToRoom = sinon.stub()
+			@CollaboratorsHandler.removeUserFromProject = sinon.stub().callsArg(2)
+			@CollaboratorsController.removeSelfFromProject(@req, @res)
+
+		it "should remove the logged in user from the project", ->
+			@CollaboratorsHandler.removeUserFromProject
+				.calledWith(@project_id, @user_id)
+				.should.equal true
+	
+		it "should emit a userRemovedFromProject event to the proejct", ->
+			@EditorRealTimeController.emitToRoom
+				.calledWith(@project_id, 'userRemovedFromProject', @user_id)
+				.should.equal true
+
+		it "should return a success code", ->
+			@res.sendStatus.calledWith(204).should.equal true
 
 	describe "_formatCollaborators", ->
 		beforeEach ->

services/web/test/UnitTests/coffee/Collaborators/CollaboratorsHandlerTests.coffee

@@ -7,42 +7,110 @@ modulePath = path.join __dirname, "../../../../app/js/Features/Collaborators/Col
 expect = require("chai").expect
 
 describe "CollaboratorsHandler", ->
-
 	beforeEach ->
-
-		@user =
-			email:"bob@bob.com"
-		@UserModel = 
-			findById:sinon.stub().callsArgWith(1, null, @user)
-			update: sinon.stub()
-
-		@settings = {}
-		@ProjectModel = 
-			update: sinon.stub().callsArgWith(1)
 		@CollaboratorHandler = SandboxedModule.require modulePath, requires:
-			"settings-sharelatex":@settings
-			"logger-sharelatex": 
-				log:->
-				err:->
-			'../../models/User': User:@UserModel
-			"../../models/Project": Project:@ProjectModel
-			"../Email/EmailHandler": {}
+			"logger-sharelatex": @logger = { log: sinon.stub(), err: sinon.stub() }
+			'../User/UserCreator': @UserCreator = {}
+			"../../models/Project": Project: @Project = {}
+			"../Project/ProjectEntityHandler": @ProjectEntityHandler = {}
 
-		@project_id = "123l2j13lkj"
-		@user_id = "132kj1lk2j"
+		@project_id = "mock-project-id"
+		@user_id = "mock-user-id"
+		@callback = sinon.stub()
 
 	describe "removeUserFromProject", ->
-
 		beforeEach ->
-			@ProjectModel.update.callsArgWith(2)
+			@Project.update = sinon.stub().callsArg(2)
+			@CollaboratorHandler.removeUserFromProject @project_id, @user_id, @callback
 
-		it "should remove the user from mongo", (done)->
-
-			@CollaboratorHandler.removeUserFromProject @project_id, @user_id, =>
-				update = @ProjectModel.update.args[0][1]
-				assert.deepEqual update, "$pull":{collaberator_refs:@user_id, readOnly_refs:@user_id}
-				done()
+		it "should remove the user from mongo", ->
+			@Project.update
+				.calledWith({
+					_id: @project_id
+				}, {
+					"$pull":{collaberator_refs:@user_id, readOnly_refs:@user_id}
+				})
+				.should.equal true
+	
+	describe "addUserToProject", ->
+		beforeEach ->
+			@Project.update = sinon.stub().callsArg(2)
+			@ProjectEntityHandler.flushProjectToThirdPartyDataStore = sinon.stub().callsArg(1)
+			
+		describe "as readOnly", ->
+			beforeEach ->
+				@CollaboratorHandler.addUserToProject @project_id, @user_id, "readOnly", @callback
 
+			it "should add the user to the readOnly_refs", ->
+				@Project.update
+					.calledWith({
+						_id: @project_id
+					}, {
+						"$push":{ readOnly_refs: @user_id }
+					})
+					.should.equal true
+			
+			it "should flush the project to the TPDS", ->
+				@ProjectEntityHandler.flushProjectToThirdPartyDataStore
+					.calledWith(@project_id)
+					.should.equal true
+					
+		describe "as readAndWrite", ->
+			beforeEach ->
+				@CollaboratorHandler.addUserToProject @project_id, @user_id, "readAndWrite", @callback
+
+			it "should add the user to the collaberator_refs", ->
+				@Project.update
+					.calledWith({
+						_id: @project_id
+					}, {
+						"$push":{ collaberator_refs: @user_id }
+					})
+					.should.equal true
+			
+			it "should flush the project to the TPDS", ->
+				@ProjectEntityHandler.flushProjectToThirdPartyDataStore
+					.calledWith(@project_id)
+					.should.equal true
+		
+		describe "with invalid privilegeLevel", ->
+			beforeEach ->
+				@CollaboratorHandler.addUserToProject @project_id, @user_id, "notValid", @callback
+
+			it "should call the callback with an error", ->
+				@callback.calledWith(new Error()).should.equal true
+	
+	describe "addEmailToProject", ->
+		beforeEach ->
+			@UserCreator.getUserOrCreateHoldingAccount = sinon.stub().callsArgWith(1, null, @user = {_id: @user_id})
+			@CollaboratorHandler.addUserToProject = sinon.stub().callsArg(3)
+
+		describe "with a valid email", ->
+			beforeEach ->
+				@CollaboratorHandler.addEmailToProject @project_id, (@email = "Joe@example.com"), (@privilegeLevel = "readAndWrite"), @callback
+			
+			it "should get the user with the lowercased email", ->
+				@UserCreator.getUserOrCreateHoldingAccount
+					.calledWith(@email.toLowerCase())
+					.should.equal true
+		
+			it "should add the user to the project by id", ->
+				@CollaboratorHandler.addUserToProject
+					.calledWith(@project_id, @user_id, @privilegeLevel)
+					.should.equal true
+			
+			it "should return the callback with the user_id", ->
+				@callback.calledWith(null, @user_id).should.equal true
+		
+		describe "with an invalid email", ->
+			beforeEach ->
+				@CollaboratorHandler.addEmailToProject @project_id, "not-and-email", (@privilegeLevel = "readAndWrite"), @callback
+		
+			it "should call the callback with an error", ->
+				@callback.calledWith(new Error()).should.equal true
+			
+			it "should not add any users to the proejct", ->
+				@CollaboratorHandler.addUserToProject.called.should.equal false
 
 
 

services/web/test/UnitTests/coffee/Editor/EditorControllerTests.coffee

@@ -33,10 +33,8 @@ describe "EditorController", ->
 			setSpellCheckLanguage: sinon.spy()
 		@ProjectEntityHandler = 
 			flushProjectToThirdPartyDataStore:sinon.stub()
-		@ProjectEditorHandler = {}
 		@Project =
 			findPopulatedById: sinon.stub().callsArgWith(1, null, @project)
-		@LimitationsManager = {}
 		@client = new MockClient()
 
 		@settings = 
@@ -56,14 +54,12 @@ describe "EditorController", ->
 			releaseLock : sinon.stub()
 		@EditorController = SandboxedModule.require modulePath, requires:
 			"../../infrastructure/Server" : io : @io
-			'../Project/ProjectEditorHandler' : @ProjectEditorHandler
 			'../Project/ProjectEntityHandler' : @ProjectEntityHandler
 			'../Project/ProjectOptionsHandler' : @ProjectOptionsHandler
 			'../Project/ProjectDetailsHandler': @ProjectDetailsHandler
 			'../Project/ProjectDeleter' : @ProjectDeleter
 			'../Collaborators/CollaboratorsHandler': @CollaboratorsHandler
 			'../DocumentUpdater/DocumentUpdaterHandler' : @DocumentUpdaterHandler
-			'../Subscription/LimitationsManager' : @LimitationsManager
 			'../../models/Project' : Project: @Project
 			"settings-sharelatex":@settings
 			'../Dropbox/DropboxProjectLinker':@dropboxProjectLinker
@@ -76,67 +72,6 @@ describe "EditorController", ->
 				log: sinon.stub()
 				err: sinon.stub()
 
-	describe "addUserToProject", ->
-		beforeEach ->
-			@email = "Jane.Doe@example.com"
-			@priveleges = "readOnly"
-			@addedUser = { _id: "added-user" }
-			@ProjectEditorHandler.buildUserModelView = sinon.stub().returns(@addedUser)
-			@CollaboratorsHandler.addUserToProject = sinon.stub().callsArgWith(3, null, @addedUser)
-			@EditorRealTimeController.emitToRoom = sinon.stub()
-			@callback = sinon.stub()
-
-		describe "when the project can accept more collaborators", ->
-			beforeEach ->
-				@LimitationsManager.isCollaboratorLimitReached = sinon.stub().callsArgWith(1, null, false)
-
-			it "should add the user to the project", (done)->
-				@EditorController.addUserToProject @project_id, @email, @priveleges, =>
-					@CollaboratorsHandler.addUserToProject.calledWith(@project_id, @email.toLowerCase(), @priveleges).should.equal true
-					done()
-
-			it "should emit a userAddedToProject event", (done)->
-				@EditorController.addUserToProject @project_id, @email, @priveleges, =>
-					@EditorRealTimeController.emitToRoom.calledWith(@project_id, "userAddedToProject", @addedUser).should.equal true
-					done()
-
-			it "should return the user to the callback", (done)->
-				@EditorController.addUserToProject @project_id, @email, @priveleges, (err, result)=>
-					result.should.equal @addedUser
-					done()
-
-
-		describe "when the project cannot accept more collaborators", ->
-			beforeEach ->
-				@LimitationsManager.isCollaboratorLimitReached = sinon.stub().callsArgWith(1, null, true)
-				@EditorController.addUserToProject(@project_id, @email, @priveleges, @callback)
-
-			it "should not add the user to the project", ->
-				@CollaboratorsHandler.addUserToProject.called.should.equal false
-
-			it "should not emit a userAddedToProject event", ->
-				@EditorRealTimeController.emitToRoom.called.should.equal false
-
-			it "should return false to the callback", ->
-				@callback.calledWith(null, false).should.equal true
-
-
-	describe "removeUserFromProject", ->
-		beforeEach ->
-			@removed_user_id = "removed-user-id"
-			@CollaboratorsHandler.removeUserFromProject = sinon.stub().callsArgWith(2)
-			@EditorRealTimeController.emitToRoom = sinon.stub()
-
-			@EditorController.removeUserFromProject(@project_id, @removed_user_id)
-
-		it "remove the user from the project", ->
-			@CollaboratorsHandler.removeUserFromProject
-				.calledWith(@project_id, @removed_user_id)
-				.should.equal true
-
-		it "should emit a userRemovedFromProject event", ->
-			@EditorRealTimeController.emitToRoom.calledWith(@project_id, "userRemovedFromProject", @removed_user_id).should.equal true
-
 	describe "updating compiler used for project", ->
 		it "should send the new compiler and project id to the project options handler", (done)->
 			compiler = "latex"