From 6ea0b9cf468b1226c97306e1635e3ad452bf60d3 Mon Sep 17 00:00:00 2001 From: Chrystal Maria Griffiths Date: Tue, 13 Nov 2018 10:32:29 +0000 Subject: [PATCH] Merge pull request #1097 from sharelatex/hb-fix-institutions-callbacks Fix multiple callback and silent failures on institutions controller GitOrigin-RevId: 2883445a4d960e7aca2dfbab45b8e268437e4769 --- .../InstitutionsController.coffee | 25 +++++++++++-------- .../InstitutionsControllerTests.coffee | 14 +++++++++-- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/services/web/app/coffee/Features/Institutions/InstitutionsController.coffee b/services/web/app/coffee/Features/Institutions/InstitutionsController.coffee index 83ac256eac..7251effcc8 100644 --- a/services/web/app/coffee/Features/Institutions/InstitutionsController.coffee +++ b/services/web/app/coffee/Features/Institutions/InstitutionsController.coffee @@ -24,14 +24,17 @@ affiliateUsers = (hostname, callback = (error)->) -> if error? logger.err error: error, 'problem fetching users by hostname' return callback(error) - async.map users, ((user) -> - matchingEmails = user.emails.filter (email) -> email.reversedHostname == reversedHostname - for email in matchingEmails - addAffiliation user._id, email.email, {confirmedAt: email.confirmedAt}, (error) => - if error? - logger.err error: error, 'problem adding affiliation while confirming hostname' - return callback(error) - ), (error) -> - if error? - return callback(error) - callback() + + async.map users, ((user, innerCallback) -> + affiliateUserByReversedHostname user, reversedHostname, innerCallback + ), callback + +affiliateUserByReversedHostname = (user, reversedHostname, callback) -> + matchingEmails = user.emails.filter (email) -> email.reversedHostname == reversedHostname + async.map matchingEmails, ((email, innerCallback) -> + addAffiliation user._id, email.email, {confirmedAt: email.confirmedAt}, (error) => + if error? + logger.err error: error, 'problem adding affiliation while confirming hostname' + return innerCallback(error) + innerCallback() + ), callback diff --git a/services/web/test/unit/coffee/Institutions/InstitutionsControllerTests.coffee b/services/web/test/unit/coffee/Institutions/InstitutionsControllerTests.coffee index d8c080389c..6152b706ea 100644 --- a/services/web/test/unit/coffee/Institutions/InstitutionsControllerTests.coffee +++ b/services/web/test/unit/coffee/Institutions/InstitutionsControllerTests.coffee @@ -47,17 +47,27 @@ describe "InstitutionsController", -> json: sinon.stub() @next = sinon.stub() - describe 'confirmDomain', -> + describe 'affiliateUsers', -> it 'should add affiliations for matching users', (done)-> @res.sendStatus = (code) => + code.should.equal 200 @getUsersByHostname.calledOnce.should.equal true @addAffiliation.calledThrice.should.equal true @addAffiliation.calledWith(@stubbedUser1._id, @stubbedUser1.emails[0].email).should.equal true @addAffiliation.calledWith(@stubbedUser1._id, @stubbedUser1.emails[2].email).should.equal true + @addAffiliation.calledWith(@stubbedUser2._id, @stubbedUser2.emails[0].email).should.equal true done() @InstitutionsController.confirmDomain @req, @res, @next - describe 'create institution', -> + it 'should return errors if last affiliation cannot be added', (done)-> + @addAffiliation.onCall(2).callsArgWith(3, new Error("error")) + @next = (error) => + expect(error).to.exist + @getUsersByHostname.calledOnce.should.equal true + done() + @InstitutionsController.confirmDomain @req, @res, @next + + describe 'createInstitution', -> it 'should create new institution', (done)-> @req.body.institution_id = 123 expectedData = v1Id: 123