remove UserLocator

Use UserGetter instead
This commit is contained in:
Tim Alby 2018-05-24 15:55:12 +02:00
parent bbaca91e57
commit 5fbe5c5537
10 changed files with 26 additions and 78 deletions

View file

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

View file

@ -2,7 +2,6 @@ async = require("async")
_ = require("underscore") _ = require("underscore")
SubscriptionUpdater = require("./SubscriptionUpdater") SubscriptionUpdater = require("./SubscriptionUpdater")
SubscriptionLocator = require("./SubscriptionLocator") SubscriptionLocator = require("./SubscriptionLocator")
UserLocator = require("../User/UserLocator")
UserGetter = require("../User/UserGetter") UserGetter = require("../User/UserGetter")
LimitationsManager = require("./LimitationsManager") LimitationsManager = require("./LimitationsManager")
logger = require("logger-sharelatex") logger = require("logger-sharelatex")
@ -51,7 +50,7 @@ module.exports = SubscriptionGroupHandler =
users.push buildEmailInviteViewModel(email) users.push buildEmailInviteViewModel(email)
jobs = _.map subscription.member_ids, (user_id)-> jobs = _.map subscription.member_ids, (user_id)->
return (cb)-> return (cb)->
UserLocator.findById user_id, (err, user)-> UserGetter.getUser user_id, (err, user)->
if err? or !user? if err? or !user?
users.push _id:user_id users.push _id:user_id
return cb() return cb()

View file

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

View file

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

View file

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

View file

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

View file

@ -30,10 +30,8 @@ describe "SubscriptionGroupHandler", ->
addEmailInviteToGroup: sinon.stub().callsArgWith(2) addEmailInviteToGroup: sinon.stub().callsArgWith(2)
removeEmailInviteFromGroup: sinon.stub().callsArgWith(2) removeEmailInviteFromGroup: sinon.stub().callsArgWith(2)
@UserLocator =
findById: sinon.stub()
@UserGetter = @UserGetter =
getUser: sinon.stub()
getUserByMainEmail: sinon.stub() getUserByMainEmail: sinon.stub()
@LimitationsManager = @LimitationsManager =
@ -58,7 +56,6 @@ describe "SubscriptionGroupHandler", ->
"../User/UserCreator": @UserCreator "../User/UserCreator": @UserCreator
"./SubscriptionUpdater": @SubscriptionUpdater "./SubscriptionUpdater": @SubscriptionUpdater
"./SubscriptionLocator": @SubscriptionLocator "./SubscriptionLocator": @SubscriptionLocator
"../User/UserLocator": @UserLocator
"../User/UserGetter": @UserGetter "../User/UserGetter": @UserGetter
"./LimitationsManager": @LimitationsManager "./LimitationsManager": @LimitationsManager
"../Security/OneTimeTokenHandler":@OneTimeTokenHandler "../Security/OneTimeTokenHandler":@OneTimeTokenHandler
@ -122,26 +119,26 @@ describe "SubscriptionGroupHandler", ->
beforeEach -> beforeEach ->
@subscription = {} @subscription = {}
@SubscriptionLocator.getUsersSubscription.callsArgWith(1, null, @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)-> 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)=> @Handler.getPopulatedListOfMembers @adminUser_id, (err, users)=>
@SubscriptionLocator.getUsersSubscription.calledWith(@adminUser_id).should.equal true @SubscriptionLocator.getUsersSubscription.calledWith(@adminUser_id).should.equal true
done() done()
it "should get the users by id", (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"] @subscription.member_ids = ["1234", "342432", "312312"]
@Handler.getPopulatedListOfMembers @adminUser_id, (err, users)=> @Handler.getPopulatedListOfMembers @adminUser_id, (err, users)=>
@UserLocator.findById.calledWith(@subscription.member_ids[0]).should.equal true @UserGetter.getUser.calledWith(@subscription.member_ids[0]).should.equal true
@UserLocator.findById.calledWith(@subscription.member_ids[1]).should.equal true @UserGetter.getUser.calledWith(@subscription.member_ids[1]).should.equal true
@UserLocator.findById.calledWith(@subscription.member_ids[2]).should.equal true @UserGetter.getUser.calledWith(@subscription.member_ids[2]).should.equal true
users.length.should.equal @subscription.member_ids.length users.length.should.equal @subscription.member_ids.length
done() done()
it "should just return the id if the user can not be found as they may have deleted their account", (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"] @subscription.member_ids = ["1234", "342432", "312312"]
@Handler.getPopulatedListOfMembers @adminUser_id, (err, users)=> @Handler.getPopulatedListOfMembers @adminUser_id, (err, users)=>
assert.deepEqual users[0], {_id:@subscription.member_ids[0]} assert.deepEqual users[0], {_id:@subscription.member_ids[0]}

View file

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

View file

@ -1,31 +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 ->
@fakeUser = {_id:"12390i"}
@findOne = sinon.stub().callsArgWith(1, null, @fakeUser)
@Mongo =
db: users: findOne: @findOne
ObjectId: (id) -> return id
@UserLocator = SandboxedModule.require modulePath, requires:
"../../infrastructure/mongojs": @Mongo
"metrics-sharelatex": timeAsyncMethod: sinon.stub()
'logger-sharelatex' : { log: sinon.stub() }
describe "findById", ->
it "should try and find a user with that id", (done)->
_id = '123e'
@UserLocator.findById _id, (err, user)=>
@findOne.calledWith(_id: _id).should.equal true
done()
it "should return the user if found", (done)->
@UserLocator.findById '123e', (err, user)=>
user.should.deep.equal @fakeUser
done()

View file

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