diff --git a/services/web/app/coffee/Features/BetaProgram/BetaProgramController.coffee b/services/web/app/coffee/Features/BetaProgram/BetaProgramController.coffee index 4e96ce113c..1e0577cfc1 100644 --- a/services/web/app/coffee/Features/BetaProgram/BetaProgramController.coffee +++ b/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) diff --git a/services/web/app/coffee/Features/Collaborators/CollaboratorsInviteController.coffee b/services/web/app/coffee/Features/Collaborators/CollaboratorsInviteController.coffee index 898abe52bb..f74a144bac 100644 --- a/services/web/app/coffee/Features/Collaborators/CollaboratorsInviteController.coffee +++ b/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) diff --git a/services/web/app/coffee/Features/Collaborators/CollaboratorsInviteHandler.coffee b/services/web/app/coffee/Features/Collaborators/CollaboratorsInviteHandler.coffee index ecca8ab86f..b511f56e53 100644 --- a/services/web/app/coffee/Features/Collaborators/CollaboratorsInviteHandler.coffee +++ b/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) diff --git a/services/web/app/coffee/Features/PasswordReset/PasswordResetHandler.coffee b/services/web/app/coffee/Features/PasswordReset/PasswordResetHandler.coffee index 4e67e9f1f4..3947b63004 100644 --- a/services/web/app/coffee/Features/PasswordReset/PasswordResetHandler.coffee +++ b/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" diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee index 5b7c2e0740..d6ce0dde59 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee +++ b/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() diff --git a/services/web/app/coffee/Features/User/UserController.coffee b/services/web/app/coffee/Features/User/UserController.coffee index ccbd0a86f1..cf7e6be33f 100644 --- a/services/web/app/coffee/Features/User/UserController.coffee +++ b/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() diff --git a/services/web/app/coffee/Features/User/UserGetter.coffee b/services/web/app/coffee/Features/User/UserGetter.coffee index 306fbc7a10..201981625e 100644 --- a/services/web/app/coffee/Features/User/UserGetter.coffee +++ b/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) -> diff --git a/services/web/app/coffee/Features/User/UserLocator.coffee b/services/web/app/coffee/Features/User/UserLocator.coffee deleted file mode 100644 index 9be32c76b0..0000000000 --- a/services/web/app/coffee/Features/User/UserLocator.coffee +++ /dev/null @@ -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 diff --git a/services/web/app/coffee/Features/User/UserPagesController.coffee b/services/web/app/coffee/Features/User/UserPagesController.coffee index 25825c35e6..5e6ea7d62b 100644 --- a/services/web/app/coffee/Features/User/UserPagesController.coffee +++ b/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' diff --git a/services/web/app/coffee/Features/User/UserRegistrationHandler.coffee b/services/web/app/coffee/Features/User/UserRegistrationHandler.coffee index f5db2e54a1..fab438ffa6 100644 --- a/services/web/app/coffee/Features/User/UserRegistrationHandler.coffee +++ b/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 diff --git a/services/web/app/coffee/Features/User/UserUpdater.coffee b/services/web/app/coffee/Features/User/UserUpdater.coffee index 530d81063d..174d37bc38 100644 --- a/services/web/app/coffee/Features/User/UserUpdater.coffee +++ b/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(), { diff --git a/services/web/test/unit/coffee/BetaProgram/BetaProgramControllerTests.coffee b/services/web/test/unit/coffee/BetaProgram/BetaProgramControllerTests.coffee index ab1f1b0567..713179b056 100644 --- a/services/web/test/unit/coffee/BetaProgram/BetaProgramControllerTests.coffee +++ b/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 diff --git a/services/web/test/unit/coffee/Collaborators/CollaboratorsInviteControllerTests.coffee b/services/web/test/unit/coffee/Collaborators/CollaboratorsInviteControllerTests.coffee index cdfff78249..4b57ff3697 100644 --- a/services/web/test/unit/coffee/Collaborators/CollaboratorsInviteControllerTests.coffee +++ b/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) => diff --git a/services/web/test/unit/coffee/Collaborators/CollaboratorsInviteHandlerTests.coffee b/services/web/test/unit/coffee/Collaborators/CollaboratorsInviteHandlerTests.coffee index 177c42d4ba..58b373f61f 100644 --- a/services/web/test/unit/coffee/Collaborators/CollaboratorsInviteHandlerTests.coffee +++ b/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) -> diff --git a/services/web/test/unit/coffee/PasswordReset/PasswordResetHandlerTests.coffee b/services/web/test/unit/coffee/PasswordReset/PasswordResetHandlerTests.coffee index b29839246a..261f5582dd 100644 --- a/services/web/test/unit/coffee/PasswordReset/PasswordResetHandlerTests.coffee +++ b/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 diff --git a/services/web/test/unit/coffee/Subscription/SubscriptionGroupHandlerTests.coffee b/services/web/test/unit/coffee/Subscription/SubscriptionGroupHandlerTests.coffee index 88b4366a30..bca9ac7600 100644 --- a/services/web/test/unit/coffee/Subscription/SubscriptionGroupHandlerTests.coffee +++ b/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]} diff --git a/services/web/test/unit/coffee/User/UserControllerTests.coffee b/services/web/test/unit/coffee/User/UserControllerTests.coffee index c358f35b22..e815d8d701 100644 --- a/services/web/test/unit/coffee/User/UserControllerTests.coffee +++ b/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 diff --git a/services/web/test/unit/coffee/User/UserCreatorTests.coffee b/services/web/test/unit/coffee/User/UserCreatorTests.coffee index 01d13bba10..cc2b1ec150 100644 --- a/services/web/test/unit/coffee/User/UserCreatorTests.coffee +++ b/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: ()->} diff --git a/services/web/test/unit/coffee/User/UserGetterTests.coffee b/services/web/test/unit/coffee/User/UserGetterTests.coffee new file mode 100644 index 0000000000..2c32bd8250 --- /dev/null +++ b/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() diff --git a/services/web/test/unit/coffee/User/UserLocatorTests.coffee b/services/web/test/unit/coffee/User/UserLocatorTests.coffee deleted file mode 100644 index dc3fc84dfa..0000000000 --- a/services/web/test/unit/coffee/User/UserLocatorTests.coffee +++ /dev/null @@ -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() - - - diff --git a/services/web/test/unit/coffee/User/UserPagesControllerTests.coffee b/services/web/test/unit/coffee/User/UserPagesControllerTests.coffee index 529f5b1be6..a0f155846f 100644 --- a/services/web/test/unit/coffee/User/UserPagesControllerTests.coffee +++ b/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" diff --git a/services/web/test/unit/coffee/User/UserRegistrationHandlerTests.coffee b/services/web/test/unit/coffee/User/UserRegistrationHandlerTests.coffee index d0b96da2de..9411059022 100644 --- a/services/web/test/unit/coffee/User/UserRegistrationHandlerTests.coffee +++ b/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)=> diff --git a/services/web/test/unit/coffee/User/UserUpdaterTests.coffee b/services/web/test/unit/coffee/User/UserUpdaterTests.coffee index a6239e2e65..202a4f3f1a 100644 --- a/services/web/test/unit/coffee/User/UserUpdaterTests.coffee +++ b/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()