Update error handling on backend

This commit is contained in:
James Allen 2018-07-17 11:12:09 +01:00
parent ca97698724
commit 2c25cbaf25
8 changed files with 30 additions and 16 deletions

View file

@ -13,6 +13,7 @@ UserSessionsManager = require("./UserSessionsManager")
UserUpdater = require("./UserUpdater") UserUpdater = require("./UserUpdater")
SudoModeHandler = require('../SudoMode/SudoModeHandler') SudoModeHandler = require('../SudoMode/SudoModeHandler')
settings = require "settings-sharelatex" settings = require "settings-sharelatex"
Errors = require "../Errors/Errors"
module.exports = UserController = module.exports = UserController =
@ -100,7 +101,7 @@ module.exports = UserController =
UserUpdater.changeEmailAddress user_id, newEmail, (err)-> UserUpdater.changeEmailAddress user_id, newEmail, (err)->
if err? if err?
logger.err err:err, user_id:user_id, newEmail:newEmail, "problem updaing users email address" logger.err err:err, user_id:user_id, newEmail:newEmail, "problem updaing users email address"
if err.message == "alread_exists" if err instanceof Errors.EmailExistsError
message = req.i18n.translate("email_already_registered") message = req.i18n.translate("email_already_registered")
else else
message = req.i18n.translate("problem_changing_email_address") message = req.i18n.translate("problem_changing_email_address")

View file

@ -26,7 +26,8 @@ module.exports = UserEmailsController =
role: req.body.role role: req.body.role
department: req.body.department department: req.body.department
UserUpdater.addEmailAddress userId, email, affiliationOptions, (error)-> UserUpdater.addEmailAddress userId, email, affiliationOptions, (error)->
return next(error) if error? if error?
return UserEmailsController._handleEmailError error, req, res, next
UserEmailsConfirmationHandler.sendConfirmationEmail userId, email, (err) -> UserEmailsConfirmationHandler.sendConfirmationEmail userId, email, (err) ->
return next(error) if error? return next(error) if error?
res.sendStatus 204 res.sendStatus 204
@ -49,15 +50,7 @@ module.exports = UserEmailsController =
UserUpdater.updateV1AndSetDefaultEmailAddress userId, email, (error)-> UserUpdater.updateV1AndSetDefaultEmailAddress userId, email, (error)->
if error? if error?
if error instanceof Errors.UnconfirmedEmailError return UserEmailsController._handleEmailError error, req, res, next
return res.sendStatus 409
else if error instanceof Errors.EmailExistsError
return res.status(409).json {
error:
message: "The email '#{email}' is already in use by another account"
}
else
return next(error)
else else
return res.sendStatus 200 return res.sendStatus 200
@ -105,3 +98,15 @@ module.exports = UserEmailsController =
next(error) next(error)
else else
res.sendStatus 200 res.sendStatus 200
_handleEmailError: (error, req, res, next) ->
if error instanceof Errors.UnconfirmedEmailError
return res.status(409).json {
message: 'email must be confirmed'
}
else if error instanceof Errors.EmailExistsError
return res.status(409).json {
message: req.i18n.translate("email_already_registered")
}
else
return next(error)

View file

@ -4,6 +4,7 @@ logger = require('logger-sharelatex')
db = mongojs.db db = mongojs.db
ObjectId = mongojs.ObjectId ObjectId = mongojs.ObjectId
{ getAffiliations } = require("./UserAffiliationsManager") { getAffiliations } = require("./UserAffiliationsManager")
Errors = require("../Errors/Errors")
module.exports = UserGetter = module.exports = UserGetter =
getUser: (query, projection, callback = (error, user) ->) -> getUser: (query, projection, callback = (error, user) ->) ->
@ -77,7 +78,7 @@ module.exports = UserGetter =
# check for duplicate email address. This is also enforced at the DB level # check for duplicate email address. This is also enforced at the DB level
ensureUniqueEmailAddress: (newEmail, callback) -> ensureUniqueEmailAddress: (newEmail, callback) ->
@getUserByAnyEmail newEmail, (error, user) -> @getUserByAnyEmail newEmail, (error, user) ->
return callback(message: 'alread_exists') if user? return callback(new Errors.EmailExistsError('alread_exists')) if user?
callback(error) callback(error)
decorateFullEmails = (defaultEmail, emailsData, affiliationsData) -> decorateFullEmails = (defaultEmail, emailsData, affiliationsData) ->

View file

@ -136,7 +136,9 @@ module.exports = UserUpdater =
user: Settings.apis.v1.user user: Settings.apis.v1.user
pass: Settings.apis.v1.pass pass: Settings.apis.v1.pass
sendImmediately: true sendImmediately: true
json: { email: newEmail }, json:
user:
email: newEmail
timeout: 5 * 1000 timeout: 5 * 1000
}, (error, response, body) -> }, (error, response, body) ->
if error? if error?

View file

@ -470,8 +470,8 @@ describe "UserEmails", ->
}, (error, response, body) => }, (error, response, body) =>
return done(error) if error? return done(error) if error?
expect(response.statusCode).to.equal 409 expect(response.statusCode).to.equal 409
expect(body.error).to.deep.equal { expect(body).to.deep.equal {
message: "The email 'exists-in-v1@example.com' is already in use by another account" message: "This email is already registered"
} }
cb() cb()
(cb) => (cb) =>

View file

@ -69,7 +69,7 @@ module.exports = MockV1Api =
res.json [] res.json []
app.put '/api/v1/sharelatex/users/:id/email', (req, res, next) => app.put '/api/v1/sharelatex/users/:id/email', (req, res, next) =>
{ email } = req.body { email } = req.body?.user
if email in @existingEmails if email in @existingEmails
return res.sendStatus 409 return res.sendStatus 409
else else

View file

@ -9,6 +9,7 @@ MockResponse = require "../helpers/MockResponse"
MockRequest = require "../helpers/MockRequest" MockRequest = require "../helpers/MockRequest"
ObjectId = require("mongojs").ObjectId ObjectId = require("mongojs").ObjectId
assert = require("assert") assert = require("assert")
Errors = require "../../../../app/js/Features/Errors/Errors"
describe "UserController", -> describe "UserController", ->
beforeEach -> beforeEach ->
@ -81,6 +82,7 @@ describe "UserController", ->
log:-> log:->
err:-> err:->
"metrics-sharelatex": inc:-> "metrics-sharelatex": inc:->
"../Errors/Errors": Errors
@res = @res =
send: sinon.stub() send: sinon.stub()

View file

@ -5,6 +5,7 @@ path = require('path')
sinon = require('sinon') sinon = require('sinon')
modulePath = path.join __dirname, "../../../../app/js/Features/User/UserGetter" modulePath = path.join __dirname, "../../../../app/js/Features/User/UserGetter"
expect = require("chai").expect expect = require("chai").expect
Errors = require "../../../../app/js/Features/Errors/Errors"
describe "UserGetter", -> describe "UserGetter", ->
@ -30,6 +31,7 @@ describe "UserGetter", ->
'settings-sharelatex': settings 'settings-sharelatex': settings
'./UserAffiliationsManager': './UserAffiliationsManager':
getAffiliations: @getAffiliations getAffiliations: @getAffiliations
"../Errors/Errors": Errors
describe "getUser", -> describe "getUser", ->
it "should get user", (done)-> it "should get user", (done)->
@ -150,6 +152,7 @@ describe "UserGetter", ->
@UserGetter.getUserByAnyEmail.callsArgWith(1, null, @fakeUser) @UserGetter.getUserByAnyEmail.callsArgWith(1, null, @fakeUser)
@UserGetter.ensureUniqueEmailAddress @newEmail, (err)=> @UserGetter.ensureUniqueEmailAddress @newEmail, (err)=>
should.exist(err) should.exist(err)
expect(err).to.be.an.instanceof(Errors.EmailExistsError)
err.message.should.equal 'alread_exists' err.message.should.equal 'alread_exists'
done() done()