Merge pull request #586 from sharelatex/ta-refactor-user-getter

Canonical Way to Get Users by Email

services/web/app/coffee/Features/BetaProgram/BetaProgramController.coffee

@@ -1,5 +1,5 @@
 BetaProgramHandler = require './BetaProgramHandler'
-UserLocator = require "../User/UserLocator"
+UserGetter = require "../User/UserGetter"
 Settings = require "settings-sharelatex"
 logger = require 'logger-sharelatex'
 AuthenticationController = require '../Authentication/AuthenticationController'
@@ -30,7 +30,7 @@ module.exports = BetaProgramController =
 	optInPage: (req, res, next)->
 		user_id = AuthenticationController.getLoggedInUserId(req)
 		logger.log {user_id}, "showing beta participation page for user"
-		UserLocator.findById user_id, (err, user)->
+		UserGetter.getUser user_id, (err, user)->
 			if err
 				logger.err {err, user_id}, "error fetching user"
 				return next(err)

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

@@ -27,7 +27,7 @@ module.exports = CollaboratorsInviteController =
 	_checkShouldInviteEmail: (email, callback=(err, shouldAllowInvite)->) ->
 		if Settings.restrictInvitesToExistingAccounts == true
 			logger.log {email}, "checking if user exists with this email"
-			UserGetter.getUser {email: email}, {_id: 1}, (err, user) ->
+			UserGetter.getUserByMainEmail email, {_id: 1}, (err, user) ->
 				return callback(err) if err?
 				userExists = user? and user?._id?
 				callback(null, userExists)

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

@@ -32,7 +32,7 @@ module.exports = CollaboratorsInviteHandler =
 
 	_trySendInviteNotification: (projectId, sendingUser, invite, callback=(err)->) ->
 		email = invite.email
-		UserGetter.getUser {email: email}, {_id: 1}, (err, existingUser) ->
+		UserGetter.getUserByMainEmail email, {_id: 1}, (err, existingUser) ->
 			if err?
 				logger.err {projectId, email}, "error checking if user exists"
 				return callback(err)

services/web/app/coffee/Features/PasswordReset/PasswordResetHandler.coffee

@@ -9,7 +9,7 @@ logger = require("logger-sharelatex")
 module.exports =
 
 	generateAndEmailResetToken:(email, callback = (error, exists) ->)->
-		UserGetter.getUser email:email, (err, user)->
+		UserGetter.getUserByMainEmail email, (err, user)->
 			if err then return callback(err)
 			if !user? or user.holdingAccount
 				logger.err email:email, "user could not be found for password reset"

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

@@ -2,7 +2,7 @@ async = require("async")
 _ = require("underscore")
 SubscriptionUpdater = require("./SubscriptionUpdater")
 SubscriptionLocator = require("./SubscriptionLocator")
-UserLocator = require("../User/UserLocator")
+UserGetter = require("../User/UserGetter")
 LimitationsManager = require("./LimitationsManager")
 logger = require("logger-sharelatex")
 OneTimeTokenHandler = require("../Security/OneTimeTokenHandler")
@@ -21,7 +21,7 @@ module.exports = SubscriptionGroupHandler =
 			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)->
+			UserGetter.getUserByMainEmail newEmail, (err, user)->
 				return callback(err) if err?
 				if user?
 					SubscriptionUpdater.addUserToGroup adminUserId, user._id, (err)->
@@ -50,7 +50,7 @@ module.exports = SubscriptionGroupHandler =
 				users.push buildEmailInviteViewModel(email)
 			jobs = _.map subscription.member_ids, (user_id)->
 				return (cb)->
-					UserLocator.findById user_id, (err, user)->
+					UserGetter.getUser user_id, (err, user)->
 						if err? or !user?
 							users.push _id:user_id
 							return cb()

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

@@ -1,6 +1,6 @@
 UserHandler = require("./UserHandler")
 UserDeleter = require("./UserDeleter")
-UserLocator = require("./UserLocator")
+UserGetter = require("./UserGetter")
 User = require("../../models/User").User
 newsLetterManager = require('../Newsletter/NewsletterManager')
 UserRegistrationHandler = require("./UserRegistrationHandler")
@@ -45,7 +45,7 @@ module.exports = UserController =
 
 	unsubscribe: (req, res)->
 		user_id = AuthenticationController.getLoggedInUserId(req)
-		UserLocator.findById user_id, (err, user)->
+		UserGetter.getUser user_id, (err, user)->
 			newsLetterManager.unsubscribe user, ->
 				res.send()
 

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

@@ -6,6 +6,8 @@ ObjectId = mongojs.ObjectId
 
 module.exports = UserGetter =
 	getUser: (query, projection, callback = (error, user) ->) ->
+		if query?.email?
+			return callback(new Error("Don't use getUser to find user by email"), null)
 		if arguments.length == 2
 			callback = projection
 			projection = {}
@@ -19,6 +21,13 @@ module.exports = UserGetter =
 
 		db.users.findOne query, projection, callback
 
+	getUserByMainEmail: (email, projection, callback = (error, user) ->) ->
+		email = email.trim()
+		if arguments.length == 2
+			callback = projection
+			projection = {}
+		db.users.findOne email: email, projection, callback
+
 	getUsers: (user_ids, projection, callback = (error, users) ->) ->
 		try
 			user_ids = user_ids.map (u) -> ObjectId(u.toString())
@@ -39,6 +48,7 @@ module.exports = UserGetter =
 
 [
 	'getUser',
+	'getUserByMainEmail',
 	'getUsers',
 	'getUserOrUserStubById'
 ].map (method) ->

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

@@ -1,21 +0,0 @@
-mongojs = require("../../infrastructure/mongojs")
-metrics = require("metrics-sharelatex")
-db = mongojs.db
-ObjectId = mongojs.ObjectId
-logger = require('logger-sharelatex')
-
-module.exports = UserLocator =
-
-	findByEmail: (email, callback)->
-		email = email.trim()
-		db.users.findOne email:email, (err, user)->
-			callback(err, user)
-
-	findById: (_id, callback)->
-		db.users.findOne _id:ObjectId(_id+""), callback
-
-[
-	'findById',
-	'findByEmail'
-].map (method) ->
-	metrics.timeAsyncMethod UserLocator, method, 'mongo.UserLocator', logger

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

@@ -1,4 +1,3 @@
-UserLocator = require("./UserLocator")
 UserGetter = require("./UserGetter")
 UserSessionsManager = require("./UserSessionsManager")
 ErrorController = require("../Errors/ErrorController")
@@ -61,7 +60,7 @@ module.exports =
 		user_id = AuthenticationController.getLoggedInUserId(req)
 		logger.log user: user_id, "loading settings page"
 		shouldAllowEditingDetails = !(Settings?.ldap?.updateUserDetailsOnLogin) and !(Settings?.saml?.updateUserDetailsOnLogin)
-		UserLocator.findById user_id, (err, user)->
+		UserGetter.getUser user_id, (err, user)->
 			return next(err) if err?
 			res.render 'user/settings',
 				title:'account_settings'

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

@@ -1,6 +1,7 @@
 sanitize = require('sanitizer')
 User = require("../../models/User").User
 UserCreator = require("./UserCreator")
+UserGetter = require("./UserGetter")
 AuthenticationManager = require("../Authentication/AuthenticationManager")
 NewsLetterManager = require("../Newsletter/NewsletterManager")
 async = require("async")
@@ -47,7 +48,7 @@ module.exports = UserRegistrationHandler =
 		if !requestIsValid
 			return callback(new Error("request is not valid"))
 		userDetails.email = userDetails.email?.trim()?.toLowerCase()
-		User.findOne email:userDetails.email, (err, user)->
+		UserGetter.getUserByMainEmail userDetails.email, (err, user) =>
 			if err?
 				return callback err
 			if user?.holdingAccount == false

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

@@ -3,7 +3,7 @@ mongojs = require("../../infrastructure/mongojs")
 metrics = require("metrics-sharelatex")
 db = mongojs.db
 ObjectId = mongojs.ObjectId
-UserLocator = require("./UserLocator")
+UserGetter = require("./UserGetter")
 
 module.exports = UserUpdater =
 	updateUser: (query, update, callback = (error) ->) ->
@@ -18,7 +18,7 @@ module.exports = UserUpdater =
 	changeEmailAddress: (user_id, newEmail, callback)->
 		self = @
 		logger.log user_id:user_id, newEmail:newEmail, "updaing email address of user"
-		UserLocator.findByEmail newEmail, (error, user) ->
+		UserGetter.getUserByMainEmail newEmail, (error, user) ->
 			if user?
 				return callback({message:"alread_exists"})
 			self.updateUser user_id.toString(), {

services/web/test/unit/coffee/BetaProgram/BetaProgramControllerTests.coffee

@@ -23,8 +23,8 @@ describe "BetaProgramController", ->
 				optIn: sinon.stub()
 				optOut: sinon.stub()
 			},
-			"../User/UserLocator": @UserLocator = {
-				findById: sinon.stub()
+			"../User/UserGetter": @UserGetter = {
+				getUser: sinon.stub()
 			},
 			"settings-sharelatex": @settings = {
 				languages: {}
@@ -119,7 +119,7 @@ describe "BetaProgramController", ->
 	describe "optInPage", ->
 
 		beforeEach ->
-			@UserLocator.findById.callsArgWith(1, null, @user)
+			@UserGetter.getUser.callsArgWith(1, null, @user)
 
 		it "should render the opt-in page", () ->
 			@BetaProgramController.optInPage @req, @res, @next
@@ -128,10 +128,10 @@ describe "BetaProgramController", ->
 			args[0].should.equal 'beta_program/opt_in'
 
 
-		describe "when UserLocator.findById produces an error", ->
+		describe "when UserGetter.getUser produces an error", ->
 
 			beforeEach ->
-				@UserLocator.findById.callsArgWith(1, new Error('woops'))
+				@UserGetter.getUser.callsArgWith(1, new Error('woops'))
 
 			it "should not render the opt-in page", () ->
 				@BetaProgramController.optInPage @req, @res, @next

services/web/test/unit/coffee/Collaborators/CollaboratorsInviteControllerTests.coffee

@@ -24,11 +24,14 @@ describe "CollaboratorsInviteController", ->
 			addCount: sinon.stub
 
 		@LimitationsManager = {}
+		@UserGetter =
+			getUserByMainEmail: sinon.stub()
+			getUser: sinon.stub()
 
 		@CollaboratorsInviteController = SandboxedModule.require modulePath, requires:
 			"../Project/ProjectGetter": @ProjectGetter = {}
 			'../Subscription/LimitationsManager' : @LimitationsManager
-			'../User/UserGetter': @UserGetter = {getUser: sinon.stub()}
+			'../User/UserGetter': @UserGetter
 			"./CollaboratorsHandler": @CollaboratorsHandler = {}
 			"./CollaboratorsInviteHandler": @CollaboratorsInviteHandler = {}
 			'logger-sharelatex': @logger = {err: sinon.stub(), error: sinon.stub(), log: sinon.stub()}
@@ -713,7 +716,7 @@ describe "CollaboratorsInviteController", ->
 
 				beforeEach ->
 					@user = {_id: ObjectId().toString()}
-					@UserGetter.getUser = sinon.stub().callsArgWith(2, null, @user)
+					@UserGetter.getUserByMainEmail = sinon.stub().callsArgWith(2, null, @user)
 
 				it 'should callback with `true`', (done) ->
 					@call (err, shouldAllow) =>
@@ -725,7 +728,7 @@ describe "CollaboratorsInviteController", ->
 
 				beforeEach ->
 					@user = null
-					@UserGetter.getUser = sinon.stub().callsArgWith(2, null, @user)
+					@UserGetter.getUserByMainEmail = sinon.stub().callsArgWith(2, null, @user)
 
 				it 'should callback with `false`', (done) ->
 					@call (err, shouldAllow) =>
@@ -735,15 +738,15 @@ describe "CollaboratorsInviteController", ->
 
 				it 'should have called getUser', (done) ->
 					@call (err, shouldAllow) =>
-						@UserGetter.getUser.callCount.should.equal 1
-						@UserGetter.getUser.calledWith({email: @email}, {_id: 1}).should.equal true
+						@UserGetter.getUserByMainEmail.callCount.should.equal 1
+						@UserGetter.getUserByMainEmail.calledWith(@email, {_id: 1}).should.equal true
 						done()
 
 			describe 'when getUser produces an error', ->
 
 				beforeEach ->
 					@user = null
-					@UserGetter.getUser = sinon.stub().callsArgWith(2, new Error('woops'))
+					@UserGetter.getUserByMainEmail = sinon.stub().callsArgWith(2, new Error('woops'))
 
 				it 'should callback with an error', (done) ->
 					@call (err, shouldAllow) =>

services/web/test/unit/coffee/Collaborators/CollaboratorsInviteHandlerTests.coffee

@@ -605,7 +605,7 @@ describe "CollaboratorsInviteHandler", ->
 				_id: ObjectId()
 				first_name: "jim"
 			@existingUser = {_id: ObjectId()}
-			@UserGetter.getUser = sinon.stub().callsArgWith(2, null, @existingUser)
+			@UserGetter.getUserByMainEmail = sinon.stub().callsArgWith(2, null, @existingUser)
 			@fakeProject =
 				_id: @project_id
 				name: "some project"
@@ -626,8 +626,8 @@ describe "CollaboratorsInviteHandler", ->
 
 			it 'should call getUser', (done) ->
 				@call (err) =>
-					@UserGetter.getUser.callCount.should.equal 1
-					@UserGetter.getUser.calledWith({email: @invite.email}).should.equal true
+					@UserGetter.getUserByMainEmail.callCount.should.equal 1
+					@UserGetter.getUserByMainEmail.calledWith(@invite.email).should.equal true
 					done()
 
 			it 'should call getProject', (done) ->
@@ -671,7 +671,7 @@ describe "CollaboratorsInviteHandler", ->
 		describe 'when the user does not exist', ->
 
 			beforeEach ->
-				@UserGetter.getUser = sinon.stub().callsArgWith(2, null, null)
+				@UserGetter.getUserByMainEmail = sinon.stub().callsArgWith(2, null, null)
 
 			it 'should not produce an error', (done) ->
 				@call (err) =>
@@ -680,8 +680,8 @@ describe "CollaboratorsInviteHandler", ->
 
 			it 'should call getUser', (done) ->
 				@call (err) =>
-					@UserGetter.getUser.callCount.should.equal 1
-					@UserGetter.getUser.calledWith({email: @invite.email}).should.equal true
+					@UserGetter.getUserByMainEmail.callCount.should.equal 1
+					@UserGetter.getUserByMainEmail.calledWith(@invite.email).should.equal true
 					done()
 
 			it 'should not call getProject', (done) ->
@@ -698,7 +698,7 @@ describe "CollaboratorsInviteHandler", ->
 		describe 'when the getUser produces an error', ->
 
 			beforeEach ->
-				@UserGetter.getUser = sinon.stub().callsArgWith(2, new Error('woops'))
+				@UserGetter.getUserByMainEmail = sinon.stub().callsArgWith(2, new Error('woops'))
 
 			it 'should produce an error', (done) ->
 				@call (err) =>
@@ -707,8 +707,8 @@ describe "CollaboratorsInviteHandler", ->
 
 			it 'should call getUser', (done) ->
 				@call (err) =>
-					@UserGetter.getUser.callCount.should.equal 1
-					@UserGetter.getUser.calledWith({email: @invite.email}).should.equal true
+					@UserGetter.getUserByMainEmail.callCount.should.equal 1
+					@UserGetter.getUserByMainEmail.calledWith(@invite.email).should.equal true
 					done()
 
 			it 'should not call getProject', (done) ->

services/web/test/unit/coffee/PasswordReset/PasswordResetHandlerTests.coffee

@@ -16,7 +16,7 @@ describe "PasswordResetHandler", ->
 			getNewToken:sinon.stub()
 			getValueFromTokenAndExpire:sinon.stub()
 		@UserGetter =
-			getUser:sinon.stub()
+			getUserByMainEmail:sinon.stub()
 		@EmailHandler = 
 			sendEmail:sinon.stub()
 		@AuthenticationManager =
@@ -40,7 +40,7 @@ describe "PasswordResetHandler", ->
 	describe "generateAndEmailResetToken", ->
 
 		it "should check the user exists", (done)->
-			@UserGetter.getUser.callsArgWith(1)
+			@UserGetter.getUserByMainEmail.callsArgWith(1)
 			@OneTimeTokenHandler.getNewToken.callsArgWith(1)
 			@PasswordResetHandler.generateAndEmailResetToken @user.email, (err, exists)=>
 				exists.should.equal false
@@ -49,7 +49,7 @@ describe "PasswordResetHandler", ->
 
 		it "should send the email with the token", (done)->
 
-			@UserGetter.getUser.callsArgWith(1, null, @user)
+			@UserGetter.getUserByMainEmail.callsArgWith(1, null, @user)
 			@OneTimeTokenHandler.getNewToken.callsArgWith(1, null, @token)
 			@EmailHandler.sendEmail.callsArgWith(2)
 			@PasswordResetHandler.generateAndEmailResetToken @user.email, (err, exists)=>
@@ -62,7 +62,7 @@ describe "PasswordResetHandler", ->
 
 		it "should return exists = false for a holdingAccount", (done) ->
 			@user.holdingAccount = true
-			@UserGetter.getUser.callsArgWith(1, null, @user)
+			@UserGetter.getUserByMainEmail.callsArgWith(1, null, @user)
 			@OneTimeTokenHandler.getNewToken.callsArgWith(1)
 			@PasswordResetHandler.generateAndEmailResetToken @user.email, (err, exists)=>
 				exists.should.equal false

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

@@ -30,9 +30,9 @@ describe "SubscriptionGroupHandler", ->
 			addEmailInviteToGroup: sinon.stub().callsArgWith(2)
 			removeEmailInviteFromGroup: sinon.stub().callsArgWith(2)
 
-		@UserLocator =
-			findById: sinon.stub()
-			findByEmail: sinon.stub()
+		@UserGetter =
+			getUser: sinon.stub()
+			getUserByMainEmail: sinon.stub()
 
 		@LimitationsManager =
 			hasGroupMembersLimitReached: sinon.stub()
@@ -56,7 +56,7 @@ describe "SubscriptionGroupHandler", ->
 			"../User/UserCreator": @UserCreator
 			"./SubscriptionUpdater": @SubscriptionUpdater
 			"./SubscriptionLocator": @SubscriptionLocator
-			"../User/UserLocator": @UserLocator
+			"../User/UserGetter": @UserGetter
 			"./LimitationsManager": @LimitationsManager
 			"../Security/OneTimeTokenHandler":@OneTimeTokenHandler
 			"../Email/EmailHandler":@EmailHandler
@@ -71,11 +71,11 @@ describe "SubscriptionGroupHandler", ->
 	describe "addUserToGroup", ->
 		beforeEach ->
 			@LimitationsManager.hasGroupMembersLimitReached.callsArgWith(1, null, false, @subscription)
-			@UserLocator.findByEmail.callsArgWith(1, null, @user)
+			@UserGetter.getUserByMainEmail.callsArgWith(1, null, @user)
 			
 		it "should find the user", (done)->
 			@Handler.addUserToGroup @adminUser_id, @newEmail, (err)=>
-				@UserLocator.findByEmail.calledWith(@newEmail).should.equal true
+				@UserGetter.getUserByMainEmail.calledWith(@newEmail).should.equal true
 				done()
 
 		it "should add the user to the group", (done)->
@@ -102,7 +102,7 @@ describe "SubscriptionGroupHandler", ->
 				done()
 		
 		it "should add an email invite if no user is found", (done) ->
-			@UserLocator.findByEmail.callsArgWith(1, null, null)
+			@UserGetter.getUserByMainEmail.callsArgWith(1, null, null)
 			@Handler.addUserToGroup @adminUser_id, @newEmail, (err)=>
 				@SubscriptionUpdater.addEmailInviteToGroup.calledWith(@adminUser_id, @newEmail).should.equal true
 				done()
@@ -119,26 +119,26 @@ describe "SubscriptionGroupHandler", ->
 		beforeEach ->
 			@subscription = {}
 			@SubscriptionLocator.getUsersSubscription.callsArgWith(1, null, @subscription)
-			@UserLocator.findById.callsArgWith(1, null, {_id:"31232"})
+			@UserGetter.getUser.callsArgWith(1, null, {_id:"31232"})
 
 		it "should locate the subscription", (done)->
-			@UserLocator.findById.callsArgWith(1, null, {_id:"31232"})
+			@UserGetter.getUser.callsArgWith(1, null, {_id:"31232"})
 			@Handler.getPopulatedListOfMembers @adminUser_id, (err, users)=>
 				@SubscriptionLocator.getUsersSubscription.calledWith(@adminUser_id).should.equal true
 				done()
 
 		it "should get the users by id", (done)->
-			@UserLocator.findById.callsArgWith(1, null, {_id:"31232"})
+			@UserGetter.getUser.callsArgWith(1, null, {_id:"31232"})
 			@subscription.member_ids = ["1234", "342432", "312312"]
 			@Handler.getPopulatedListOfMembers @adminUser_id, (err, users)=>
-				@UserLocator.findById.calledWith(@subscription.member_ids[0]).should.equal true
-				@UserLocator.findById.calledWith(@subscription.member_ids[1]).should.equal true
-				@UserLocator.findById.calledWith(@subscription.member_ids[2]).should.equal true
+				@UserGetter.getUser.calledWith(@subscription.member_ids[0]).should.equal true
+				@UserGetter.getUser.calledWith(@subscription.member_ids[1]).should.equal true
+				@UserGetter.getUser.calledWith(@subscription.member_ids[2]).should.equal true
 				users.length.should.equal @subscription.member_ids.length
 				done()
 
 		it "should just return the id if the user can not be found as they may have deleted their account", (done)->
-			@UserLocator.findById.callsArgWith(1)
+			@UserGetter.getUser.callsArgWith(1)
 			@subscription.member_ids = ["1234", "342432", "312312"]
 			@Handler.getPopulatedListOfMembers @adminUser_id, (err, users)=>
 				assert.deepEqual users[0], {_id:@subscription.member_ids[0]}

services/web/test/unit/coffee/User/UserControllerTests.coffee

@@ -30,8 +30,8 @@ describe "UserController", ->
 
 		@UserDeleter =
 			deleteUser: sinon.stub().callsArgWith(1)
-		@UserLocator =
-			findById: sinon.stub().callsArgWith(1, null, @user)
+		@UserGetter =
+			getUser: sinon.stub().callsArgWith(1, null, @user)
 		@User =
 			findById: sinon.stub().callsArgWith(1, null, @user)
 		@NewsLetterManager =
@@ -63,7 +63,7 @@ describe "UserController", ->
 		@SudoModeHandler =
 			clearSudoMode: sinon.stub()
 		@UserController = SandboxedModule.require modulePath, requires:
-			"./UserLocator": @UserLocator
+			"./UserGetter": @UserGetter
 			"./UserDeleter": @UserDeleter
 			"./UserUpdater":@UserUpdater
 			"../../models/User": User:@User

services/web/test/unit/coffee/User/UserCreatorTests.coffee

@@ -15,11 +15,11 @@ describe "UserCreator", ->
 			constructor: ->
 				return self.user
 
-		@UserLocator = 
-			findByEmail: sinon.stub()
+		@UserGetter =
+			getUserByMainEmail: sinon.stub()
 		@UserCreator = SandboxedModule.require modulePath, requires:
 			"../../models/User": User:@UserModel
-			"./UserLocator":@UserLocator
+			"./UserGetter":@UserGetter
 			"logger-sharelatex":{log:->}
 			'metrics-sharelatex': {timeAsyncMethod: ()->}
 

services/web/test/unit/coffee/User/UserGetterTests.coffee

@@ -0,0 +1,58 @@
+should = require('chai').should()
+SandboxedModule = require('sandboxed-module')
+assert = require('assert')
+path = require('path')
+sinon = require('sinon')
+modulePath = path.join __dirname, "../../../../app/js/Features/User/UserGetter"
+expect = require("chai").expect
+
+describe "UserGetter", ->
+
+	beforeEach ->
+		@fakeUser = {_id:"12390i"}
+		@findOne = sinon.stub().callsArgWith(2, null, @fakeUser)
+		@Mongo =
+			db: users: findOne: @findOne
+			ObjectId: (id) -> return id
+
+		@UserGetter = SandboxedModule.require modulePath, requires:
+			"logger-sharelatex": log:->
+			"../../infrastructure/mongojs": @Mongo
+			"metrics-sharelatex": timeAsyncMethod: sinon.stub()
+
+	describe "getUser", ->
+		it "should get user", (done)->
+			query = _id: 'foo'
+			projection = email: 1
+			@UserGetter.getUser query, projection, (error, user) =>
+				@findOne.called.should.equal true
+				@findOne.calledWith(query, projection).should.equal true
+				user.should.deep.equal @fakeUser
+				done()
+
+		it "should not allow email in query", (done)->
+			@UserGetter.getUser email: 'foo@bar.com', {}, (error, user) =>
+				error.should.exist
+				done()
+
+	describe "getUserbyMainEmail", ->
+		it "query user by main email", (done)->
+			email = 'hello@world.com'
+			projection = emails: 1
+			@UserGetter.getUserByMainEmail email, projection, (error, user) =>
+				@findOne.called.should.equal true
+				@findOne.calledWith(email: email, projection).should.equal true
+				done()
+
+		it "return user if found", (done)->
+			email = 'hello@world.com'
+			@UserGetter.getUserByMainEmail email, (error, user) =>
+				user.should.deep.equal @fakeUser
+				done()
+
+		it "trim email", (done)->
+			email = 'hello@world.com'
+			@UserGetter.getUserByMainEmail " #{email} ", (error, user) =>
+				@findOne.called.should.equal true
+				@findOne.calledWith(email: email).should.equal true
+				done()

services/web/test/unit/coffee/User/UserLocatorTests.coffee

@@ -1,39 +0,0 @@
-sinon = require('sinon')
-chai = require('chai')
-should = chai.should()
-modulePath = "../../../../app/js/Features/User/UserLocator.js"
-SandboxedModule = require('sandboxed-module')
-
-describe "UserLocator", ->
-
-	beforeEach ->
-		@user = {_id:"12390i"}
-		@UserLocator = SandboxedModule.require modulePath, requires:
-			"../../infrastructure/mongojs": db: @db =  { users: {} }
-			"metrics-sharelatex": timeAsyncMethod: sinon.stub()
-			'logger-sharelatex' : { log: sinon.stub() }
-		@db.users =
-			findOne : sinon.stub().callsArgWith(1, null, @user)
-
-		@email = "bob.oswald@gmail.com"
-
-
-	describe "findByEmail", ->
-
-		it "should try and find a user with that email address", (done)->
-			@UserLocator.findByEmail @email, (err, user)=>
-				@db.users.findOne.calledWith(email:@email).should.equal true
-				done()
-
-		it "should trim white space", (done)->
-			@UserLocator.findByEmail "#{@email}   ", (err, user)=>
-				@db.users.findOne.calledWith(email:@email).should.equal true
-				done()
-
-		it "should return the user if found", (done)->
-			@UserLocator.findByEmail @email, (err, user)=>
-				user.should.deep.equal @user
-				done()
-
-
-

services/web/test/unit/coffee/User/UserPagesControllerTests.coffee

@@ -16,10 +16,7 @@ describe "UserPagesController", ->
 			features:{}
 			email: "joe@example.com"
 
-		@UserLocator =
-			findById: sinon.stub().callsArgWith(1, null, @user)
-		@UserGetter =
-			getUser: sinon.stub().callsArgWith(2, null, @user)
+		@UserGetter = getUser: sinon.stub()
 		@UserSessionsManager =
 			getAllUserSessions: sinon.stub()
 		@dropboxStatus = {}
@@ -37,7 +34,6 @@ describe "UserPagesController", ->
 			"logger-sharelatex":
 				log:->
 				err:->
-			"./UserLocator": @UserLocator
 			"./UserGetter": @UserGetter
 			"./UserSessionsManager": @UserSessionsManager
 			"../Errors/ErrorController": @ErrorController
@@ -136,6 +132,8 @@ describe "UserPagesController", ->
 				@UserPagesController.sessionsPage @req, @res, @next
 
 	describe "settingsPage", ->
+		beforeEach ->
+			@UserGetter.getUser = sinon.stub().callsArgWith(1, null, @user)
 
 		it "should render user/settings", (done)->
 			@res.render = (page)->
@@ -185,6 +183,7 @@ describe "UserPagesController", ->
 
 	describe "activateAccountPage", ->
 		beforeEach ->
+			@UserGetter.getUser = sinon.stub().callsArgWith(2, null, @user)
 			@req.query.user_id = @user_id
 			@req.query.token = @token = "mock-token-123"
 

services/web/test/unit/coffee/User/UserRegistrationHandlerTests.coffee

@@ -12,8 +12,9 @@ describe "UserRegistrationHandler", ->
 		@user =
 			_id: @user_id = "31j2lk21kjl"
 		@User = 
-			findOne:sinon.stub()
 			update: sinon.stub().callsArgWith(2)
+		@UserGetter =
+			getUserByMainEmail: sinon.stub()
 		@UserCreator = 
 			createNewUser:sinon.stub().callsArgWith(1, null, @user)
 		@AuthenticationManager =
@@ -26,6 +27,7 @@ describe "UserRegistrationHandler", ->
 			getNewToken: sinon.stub()
 		@handler = SandboxedModule.require modulePath, requires:
 			"../../models/User": {User:@User}
+			"./UserGetter": @UserGetter
 			"./UserCreator": @UserCreator
 			"../Authentication/AuthenticationManager":@AuthenticationManager
 			"../Newsletter/NewsletterManager":@NewsLetterManager
@@ -70,7 +72,7 @@ describe "UserRegistrationHandler", ->
 			beforeEach ->
 				@user.holdingAccount = true
 				@handler._registrationRequestIsValid = sinon.stub().returns true
-				@User.findOne.callsArgWith(1, null, @user)
+				@UserGetter.getUserByMainEmail.callsArgWith(1, null, @user)
 
 			it "should not create a new user if there is a holding account there", (done)->
 				@handler.registerNewUser @passingRequest, (err)=>
@@ -94,7 +96,7 @@ describe "UserRegistrationHandler", ->
 					done()
 
 			it "should return email registered in the error if there is a non holdingAccount there", (done)->
-				@User.findOne.callsArgWith(1, null, @user = {holdingAccount:false})
+				@UserGetter.getUserByMainEmail.callsArgWith(1, null, @user = {holdingAccount:false})
 				@handler.registerNewUser @passingRequest, (err, user)=>
 					err.should.deep.equal new Error("EmailAlreadyRegistered")
 					user.should.deep.equal @user
@@ -103,7 +105,7 @@ describe "UserRegistrationHandler", ->
 		describe "validRequest", ->
 			beforeEach ->
 				@handler._registrationRequestIsValid = sinon.stub().returns true
-				@User.findOne.callsArgWith 1
+				@UserGetter.getUserByMainEmail.callsArgWith 1
 
 			it "should create a new user", (done)->
 				@handler.registerNewUser @passingRequest, (err)=>

services/web/test/unit/coffee/User/UserUpdaterTests.coffee

@@ -14,12 +14,12 @@ describe "UserUpdater", ->
 		@mongojs = 
 			db:{}
 			ObjectId:(id)-> return id
-		@UserLocator =
-			findByEmail:sinon.stub()
+		@UserGetter =
+			getUserByMainEmail: sinon.stub()
 		@UserUpdater = SandboxedModule.require modulePath, requires:
 			"settings-sharelatex":@settings
 			"logger-sharelatex": log:->
-			"./UserLocator":@UserLocator
+			"./UserGetter": @UserGetter
 			"../../infrastructure/mongojs":@mongojs
 			"metrics-sharelatex": timeAsyncMethod: sinon.stub()
 
@@ -34,7 +34,7 @@ describe "UserUpdater", ->
 			@UserUpdater.updateUser = sinon.stub().callsArgWith(2)
 
 		it "should check if the new email already has an account", (done)->
-			@UserLocator.findByEmail.callsArgWith(1, null, @stubbedUser)
+			@UserGetter.getUserByMainEmail.callsArgWith(1, null, @stubbedUser)
 			@UserUpdater.changeEmailAddress @user_id, @stubbedUser.email, (err)=>
 				@UserUpdater.updateUser.called.should.equal false
 				should.exist(err)
@@ -42,7 +42,7 @@ describe "UserUpdater", ->
 
 
 		it "should set the users password", (done)->
-			@UserLocator.findByEmail.callsArgWith(1, null)
+			@UserGetter.getUserByMainEmail.callsArgWith(1, null)
 			@UserUpdater.changeEmailAddress @user_id, @newEmail, (err)=>
 				@UserUpdater.updateUser.calledWith(@user_id, $set: { "email": @newEmail}).should.equal true
 				done()