diff --git a/services/web/app/coffee/Features/User/UserAffiliationsManager.coffee b/services/web/app/coffee/Features/User/UserAffiliationsManager.coffee new file mode 100644 index 0000000000..dc78466d97 --- /dev/null +++ b/services/web/app/coffee/Features/User/UserAffiliationsManager.coffee @@ -0,0 +1,64 @@ +logger = require("logger-sharelatex") +metrics = require("metrics-sharelatex") +settings = require "settings-sharelatex" +request = require "request" + +module.exports = UserAffiliationsManager = + getAffiliations: (userId, callback = (error, body) ->) -> + makeAffiliationRequest { + method: 'GET' + path: "/api/v2/users/#{userId.toString()}/affiliations" + defaultErrorMessage: "Couldn't get affiliations" + }, callback + + + addAffiliation: (userId, email, { university, department, role }, callback = (error) ->) -> + makeAffiliationRequest { + method: 'POST' + path: "/api/v2/users/#{userId.toString()}/affiliations" + body: { email, university, department, role } + defaultErrorMessage: "Couldn't create affiliation" + }, callback + + + removeAffiliation: (userId, email, callback = (error) ->) -> + email = encodeURIComponent(email) + makeAffiliationRequest { + method: 'DELETE' + path: "/api/v2/users/#{userId.toString()}/affiliations/#{email}" + extraSuccessStatusCodes: [404] # `Not Found` responses are considered successful + defaultErrorMessage: "Couldn't remove affiliation" + }, callback + + +makeAffiliationRequest = (requestOptions, callback = (error) ->) -> + return callback(null) unless settings?.apis?.v1?.url # service is not configured + requestOptions.extraSuccessStatusCodes ||= [] + request { + method: requestOptions.method + url: "#{settings.apis.v1.url}#{requestOptions.path}" + body: requestOptions.body + auth: { user: settings.apis.v1.user, pass: settings.apis.v1.pass } + json: true, + timeout: 20 * 1000 + }, (error, response, body) -> + return callback(error) if error? + isSuccess = 200 <= response.statusCode < 300 + isSuccess ||= response.statusCode in requestOptions.extraSuccessStatusCodes + unless isSuccess + if body?.errors + errorMessage = "#{response.statusCode}: #{body.errors}" + else + errorMessage = "#{requestOptions.defaultErrorMessage}: #{response.statusCode}" + return callback(new Error(errorMessage)) + + callback(null, body) + +[ + 'getAffiliations', + 'addAffiliation', + 'removeAffiliation', +].map (method) -> + metrics.timeAsyncMethod( + UserAffiliationsManager, method, 'mongo.UserAffiliationsManager', logger + ) diff --git a/services/web/app/coffee/Features/User/UserGetter.coffee b/services/web/app/coffee/Features/User/UserGetter.coffee index 21969656a0..bdcdd24482 100644 --- a/services/web/app/coffee/Features/User/UserGetter.coffee +++ b/services/web/app/coffee/Features/User/UserGetter.coffee @@ -3,8 +3,7 @@ metrics = require('metrics-sharelatex') logger = require('logger-sharelatex') db = mongojs.db ObjectId = mongojs.ObjectId -settings = require "settings-sharelatex" -request = require "request" +{ getAffiliations } = require("./UserAffiliationsManager") module.exports = UserGetter = getUser: (query, projection, callback = (error, user) ->) -> @@ -94,22 +93,6 @@ decorateFullEmails = (defaultEmail, emailsData, affiliationsData) -> emailData -getAffiliations = (userId, callback = (error) ->) -> - return callback(null, []) unless settings?.apis?.v1?.url # service is not configured - request { - method: 'GET' - url: "#{settings.apis.v1.url}/api/v2/users/#{userId.toString()}/affiliations" - auth: { user: settings.apis.v1.user, pass: settings.apis.v1.pass } - json: true, - timeout: 20 * 1000 - }, (error, response, body) -> - return callback(error) if error? - unless 200 <= response.statusCode < 300 - errorMessage = "Couldn't get affiliations: #{response.statusCode}" - return callback(new Error(errorMessage)) - - callback(null, body) - [ 'getUser', 'getUserEmail', diff --git a/services/web/app/coffee/Features/User/UserUpdater.coffee b/services/web/app/coffee/Features/User/UserUpdater.coffee index 8bb6015597..d268f12d73 100644 --- a/services/web/app/coffee/Features/User/UserUpdater.coffee +++ b/services/web/app/coffee/Features/User/UserUpdater.coffee @@ -5,10 +5,9 @@ db = mongojs.db async = require("async") ObjectId = mongojs.ObjectId UserGetter = require("./UserGetter") +{ addAffiliation, removeAffiliation } = require("./UserAffiliationsManager") EmailHelper = require "../Helpers/EmailHelper" Errors = require "../Errors/Errors" -settings = require "settings-sharelatex" -request = require "request" module.exports = UserUpdater = updateUser: (query, update, callback = (error) ->) -> @@ -66,6 +65,7 @@ module.exports = UserUpdater = return callback(error) callback() + # remove one of the user's email addresses. The email cannot be the user's # default email address removeEmailAddress: (userId, email, callback) -> @@ -115,46 +115,6 @@ module.exports = UserUpdater = return callback(new Errors.NotFoundError('user id and email do no match')) callback() -addAffiliation = (userId, email, { university, department, role }, callback = (error) ->) -> - makeAffiliationRequest { - method: 'POST' - path: "/api/v2/users/#{userId.toString()}/affiliations" - body: { email, university, department, role } - defaultErrorMessage: "Couldn't create affiliation" - }, callback - -removeAffiliation = (userId, email, callback = (error) ->) -> - email = encodeURIComponent(email) - makeAffiliationRequest { - method: 'DELETE' - path: "/api/v2/users/#{userId.toString()}/affiliations/#{email}" - extraSuccessStatusCodes: [404] # `Not Found` responses are considered successful - defaultErrorMessage: "Couldn't remove affiliation" - }, callback - -makeAffiliationRequest = (requestOptions, callback = (error) ->) -> - return callback(null) unless settings?.apis?.v1?.url # service is not configured - requestOptions.extraSuccessStatusCodes ||= [] - request { - method: requestOptions.method - url: "#{settings.apis.v1.url}#{requestOptions.path}" - body: requestOptions.body - auth: { user: settings.apis.v1.user, pass: settings.apis.v1.pass } - json: true, - timeout: 20 * 1000 - }, (error, response, body) -> - return callback(error) if error? - isSuccess = 200 <= response.statusCode < 300 - isSuccess ||= response.statusCode in requestOptions.extraSuccessStatusCodes - unless isSuccess - if body?.errors - errorMessage = "#{response.statusCode}: #{body.errors}" - else - errorMessage = "#{requestOptions.defaultErrorMessage}: #{response.statusCode}" - return callback(new Error(errorMessage)) - - callback(null) - [ 'updateUser' 'changeEmailAddress' diff --git a/services/web/test/unit/coffee/User/UserAffiliationsManagerTests.coffee b/services/web/test/unit/coffee/User/UserAffiliationsManagerTests.coffee new file mode 100644 index 0000000000..4c69e4d0b0 --- /dev/null +++ b/services/web/test/unit/coffee/User/UserAffiliationsManagerTests.coffee @@ -0,0 +1,105 @@ +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/UserAffiliationsManager" +expect = require("chai").expect + +describe "UserAffiliationsManager", -> + + beforeEach -> + @logger = err: sinon.stub(), log: -> + settings = apis: { v1: { url: 'v1.url', user: '', pass: '' } } + @request = sinon.stub() + @UserAffiliationsManager = SandboxedModule.require modulePath, requires: + "logger-sharelatex": @logger + "metrics-sharelatex": timeAsyncMethod: sinon.stub() + 'settings-sharelatex': settings + 'request': @request + + @stubbedUser = + _id: "3131231" + name:"bob" + email:"hello@world.com" + @newEmail = "bob@bob.com" + + describe 'getAffiliations', -> + it 'get affiliations', (done)-> + responseBody = [{ foo: 'bar' }] + @request.callsArgWith(1, null, { statusCode: 201 }, responseBody) + @UserAffiliationsManager.getAffiliations @stubbedUser._id, (err, body) => + should.not.exist(err) + @request.calledOnce.should.equal true + requestOptions = @request.lastCall.args[0] + expectedUrl = "v1.url/api/v2/users/#{@stubbedUser._id}/affiliations" + requestOptions.url.should.equal expectedUrl + requestOptions.method.should.equal 'GET' + should.not.exist(requestOptions.body) + body.should.equal responseBody + done() + + it 'handle error', (done)-> + body = errors: 'affiliation error message' + @request.callsArgWith(1, null, { statusCode: 503 }, body) + @UserAffiliationsManager.getAffiliations @stubbedUser._id, (err) => + should.exist(err) + err.message.should.have.string 503 + err.message.should.have.string body.errors + done() + + describe 'addAffiliation', -> + beforeEach -> + @request.callsArgWith(1, null, { statusCode: 201 }) + + it 'add affiliation', (done)-> + affiliationOptions = + university: { id: 1 } + role: 'Prof' + department: 'Math' + @UserAffiliationsManager.addAffiliation @stubbedUser._id, @newEmail, affiliationOptions, (err)=> + should.not.exist(err) + @request.calledOnce.should.equal true + requestOptions = @request.lastCall.args[0] + expectedUrl = "v1.url/api/v2/users/#{@stubbedUser._id}/affiliations" + requestOptions.url.should.equal expectedUrl + requestOptions.method.should.equal 'POST' + + body = requestOptions.body + Object.keys(body).length.should.equal 4 + body.email.should.equal @newEmail + body.university.should.equal affiliationOptions.university + body.department.should.equal affiliationOptions.department + body.role.should.equal affiliationOptions.role + done() + + it 'handle error', (done)-> + body = errors: 'affiliation error message' + @request.callsArgWith(1, null, { statusCode: 422 }, body) + @UserAffiliationsManager.addAffiliation @stubbedUser._id, @newEmail, {}, (err)=> + should.exist(err) + err.message.should.have.string 422 + err.message.should.have.string body.errors + done() + + describe 'removeAffiliation', -> + beforeEach -> + @request.callsArgWith(1, null, { statusCode: 404 }) + + it 'remove affiliation', (done)-> + @UserAffiliationsManager.removeAffiliation @stubbedUser._id, @newEmail, (err)=> + should.not.exist(err) + @request.calledOnce.should.equal true + requestOptions = @request.lastCall.args[0] + expectedUrl = "v1.url/api/v2/users/#{@stubbedUser._id}/affiliations/" + expectedUrl += encodeURIComponent(@newEmail) + requestOptions.url.should.equal expectedUrl + requestOptions.method.should.equal 'DELETE' + done() + + it 'handle error', (done)-> + @request.callsArgWith(1, null, { statusCode: 500 }) + @UserAffiliationsManager.removeAffiliation @stubbedUser._id, @newEmail, (err)=> + should.exist(err) + err.message.should.exist + done() diff --git a/services/web/test/unit/coffee/User/UserGetterTests.coffee b/services/web/test/unit/coffee/User/UserGetterTests.coffee index 391fcf9136..50ab2261f1 100644 --- a/services/web/test/unit/coffee/User/UserGetterTests.coffee +++ b/services/web/test/unit/coffee/User/UserGetterTests.coffee @@ -21,14 +21,15 @@ describe "UserGetter", -> db: users: findOne: @findOne ObjectId: (id) -> return id settings = apis: { v1: { url: 'v1.url', user: '', pass: '' } } - @request = sinon.stub() + @getAffiliations = sinon.stub().callsArgWith(1, null, []) @UserGetter = SandboxedModule.require modulePath, requires: "logger-sharelatex": log:-> "../../infrastructure/mongojs": @Mongo "metrics-sharelatex": timeAsyncMethod: sinon.stub() 'settings-sharelatex': settings - 'request': @request + './UserAffiliationsManager': + getAffiliations: @getAffiliations describe "getUser", -> it "should get user", (done)-> @@ -46,9 +47,6 @@ describe "UserGetter", -> done() describe "getUserFullEmails", -> - beforeEach -> - @request.callsArgWith(1, null, { statusCode: 200 }, []) - it "should get user", (done)-> @UserGetter.getUser = sinon.stub().callsArgWith(2, null, @fakeUser) projection = email: 1, emails: 1 @@ -77,7 +75,7 @@ describe "UserGetter", -> institution: { name: 'University Name', isUniversity: true } } ] - @request.callsArgWith(1, null, { statusCode: 200 }, affiliationsData) + @getAffiliations.callsArgWith(1, null, affiliationsData) @UserGetter.getUserFullEmails @fakeUser._id, (error, fullEmails) => assert.deepEqual fullEmails, [ { diff --git a/services/web/test/unit/coffee/User/UserUpdaterTests.coffee b/services/web/test/unit/coffee/User/UserUpdaterTests.coffee index a2348b54e2..fc9a4d8558 100644 --- a/services/web/test/unit/coffee/User/UserUpdaterTests.coffee +++ b/services/web/test/unit/coffee/User/UserUpdaterTests.coffee @@ -19,15 +19,16 @@ describe "UserUpdater", -> getUserByAnyEmail: sinon.stub() ensureUniqueEmailAddress: sinon.stub() @logger = err: sinon.stub(), log: -> - settings = apis: { v1: { url: 'v1.url', user: '', pass: '' } } - @request = sinon.stub() + @addAffiliation = sinon.stub().callsArgWith(3, null) + @removeAffiliation = sinon.stub().callsArgWith(2, null) @UserUpdater = SandboxedModule.require modulePath, requires: "logger-sharelatex": @logger "./UserGetter": @UserGetter + './UserAffiliationsManager': + addAffiliation: @addAffiliation + removeAffiliation: @removeAffiliation "../../infrastructure/mongojs":@mongojs "metrics-sharelatex": timeAsyncMethod: sinon.stub() - 'settings-sharelatex': settings - 'request': @request @stubbedUser = _id: "3131231" @@ -69,7 +70,6 @@ describe "UserUpdater", -> beforeEach -> @UserGetter.ensureUniqueEmailAddress = sinon.stub().callsArgWith(1) @UserUpdater.updateUser = sinon.stub().callsArgWith(2, null) - @request.callsArgWith(1, null, { statusCode: 201 }) it 'add email', (done)-> @UserUpdater.addEmailAddress @stubbedUser._id, @newEmail, (err)=> @@ -88,18 +88,11 @@ describe "UserUpdater", -> department: 'Math' @UserUpdater.addEmailAddress @stubbedUser._id, @newEmail, affiliationOptions, (err)=> should.not.exist(err) - @request.calledOnce.should.equal true - requestOptions = @request.lastCall.args[0] - expectedUrl = "v1.url/api/v2/users/#{@stubbedUser._id}/affiliations" - requestOptions.url.should.equal expectedUrl - requestOptions.method.should.equal 'POST' - - body = requestOptions.body - Object.keys(body).length.should.equal 4 - body.email.should.equal @newEmail - body.university.should.equal affiliationOptions.university - body.department.should.equal affiliationOptions.department - body.role.should.equal affiliationOptions.role + @addAffiliation.calledOnce.should.equal true + args = @addAffiliation.lastCall.args + args[0].should.equal @stubbedUser._id + args[1].should.equal @newEmail + args[2].should.equal affiliationOptions done() it 'handle error', (done)-> @@ -112,17 +105,15 @@ describe "UserUpdater", -> it 'handle affiliation error', (done)-> body = errors: 'affiliation error message' - @request.callsArgWith(1, null, { statusCode: 422 }, body) + @addAffiliation.callsArgWith(3, new Error('nope')) @UserUpdater.addEmailAddress @stubbedUser._id, @newEmail, (err)=> - err.message.should.have.string 422 - err.message.should.have.string body.errors + should.exist(err) @UserUpdater.updateUser.called.should.equal false done() describe 'removeEmailAddress', -> beforeEach -> @UserUpdater.updateUser = sinon.stub().callsArgWith(2, null, nMatched: 1) - @request.callsArgWith(1, null, { statusCode: 404 }) it 'remove email', (done)-> @UserUpdater.removeEmailAddress @stubbedUser._id, @newEmail, (err)=> @@ -136,12 +127,10 @@ describe "UserUpdater", -> it 'remove affiliation', (done)-> @UserUpdater.removeEmailAddress @stubbedUser._id, @newEmail, (err)=> should.not.exist(err) - @request.calledOnce.should.equal true - requestOptions = @request.lastCall.args[0] - expectedUrl = "v1.url/api/v2/users/#{@stubbedUser._id}/affiliations/" - expectedUrl += encodeURIComponent(@newEmail) - requestOptions.url.should.equal expectedUrl - requestOptions.method.should.equal 'DELETE' + @removeAffiliation.calledOnce.should.equal true + args = @removeAffiliation.lastCall.args + args[0].should.equal @stubbedUser._id + args[1].should.equal @newEmail done() it 'handle error', (done)-> @@ -159,9 +148,9 @@ describe "UserUpdater", -> done() it 'handle affiliation error', (done)-> - @request.callsArgWith(1, null, { statusCode: 500 }) + @removeAffiliation.callsArgWith(2, new Error('nope')) @UserUpdater.removeEmailAddress @stubbedUser._id, @newEmail, (err)=> - err.message.should.exist + should.exist(err) @UserUpdater.updateUser.called.should.equal false done() @@ -216,6 +205,3 @@ describe "UserUpdater", -> @UserUpdater.confirmEmail @stubbedUser._id, @newEmail, (err)=> should.exist(err) done() - - -