From 9b18e58b6840cc5a902f654706879a8e9e69349e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez=20Capel?= Date: Fri, 1 Jun 2018 11:28:06 +0100 Subject: [PATCH] Choose a better method name This method is now only notifying users about a potential domain licence --- .../Features/User/UserController.coffee | 4 +-- .../coffee/Features/User/UserHandler.coffee | 30 ++++++++----------- .../coffee/User/UserControllerTests.coffee | 6 ++-- .../unit/coffee/User/UserHandlerTests.coffee | 30 +++++-------------- 4 files changed, 25 insertions(+), 45 deletions(-) diff --git a/services/web/app/coffee/Features/User/UserController.coffee b/services/web/app/coffee/Features/User/UserController.coffee index ccbd0a86f1..be1874f0cd 100644 --- a/services/web/app/coffee/Features/User/UserController.coffee +++ b/services/web/app/coffee/Features/User/UserController.coffee @@ -110,9 +110,9 @@ module.exports = UserController = logger.err err:err, user_id:user_id, "error getting user for email update" return res.send 500 AuthenticationController.setInSessionUser(req, {email: user.email, first_name: user.first_name, last_name: user.last_name}) - UserHandler.populateGroupLicenceInvite user, (err)-> #need to refresh this in the background + UserHandler.notifyDomainLicence user, (err)-> #need to refresh this in the background if err? - logger.err err:err, "error populateGroupLicenceInvite" + logger.err err:err, "error notifyDomainLicence" res.sendStatus(200) logout : (req, res)-> diff --git a/services/web/app/coffee/Features/User/UserHandler.coffee b/services/web/app/coffee/Features/User/UserHandler.coffee index 53b2fd9c74..1d8fda7db9 100644 --- a/services/web/app/coffee/Features/User/UserHandler.coffee +++ b/services/web/app/coffee/Features/User/UserHandler.coffee @@ -6,24 +6,20 @@ logger = require("logger-sharelatex") module.exports = UserHandler = - populateGroupLicenceInvite: (user, callback = ->)-> - SubscriptionGroupHandler.convertEmailInvitesToMemberships user.email, user._id, (err) -> - return callback(err) if err? + notifyDomainLicence: (user, callback = ->)-> + logger.log user_id:user._id, "notiying user about a potential domain licence" + licence = SubscriptionDomainHandler.getLicenceUserCanJoin user + if !licence? + return callback() - logger.log user_id:user._id, "populating any potential group licence invites" - licence = SubscriptionDomainHandler.getLicenceUserCanJoin user - if !licence? + SubscriptionGroupHandler.isUserPartOfGroup user._id, licence.subscription_id, (err, alreadyPartOfGroup)-> + if err? + return callback(err) + else if alreadyPartOfGroup + logger.log user_id:user._id, "user already part of team, not creating notifcation for them" return callback() - - SubscriptionGroupHandler.isUserPartOfGroup user._id, licence.subscription_id, (err, alreadyPartOfGroup)-> - if err? - return callback(err) - else if alreadyPartOfGroup - logger.log user_id:user._id, "user already part of group, not creating notifcation for them" - return callback() - else - NotificationsBuilder.groupPlan(user, licence).create(callback) + else + NotificationsBuilder.groupPlan(user, licence).create(callback) setupLoginData: (user, callback = ->)-> - @populateGroupLicenceInvite user, callback - + @notifyDomainLicence user, callback diff --git a/services/web/test/unit/coffee/User/UserControllerTests.coffee b/services/web/test/unit/coffee/User/UserControllerTests.coffee index c358f35b22..a9ab47e6cd 100644 --- a/services/web/test/unit/coffee/User/UserControllerTests.coffee +++ b/services/web/test/unit/coffee/User/UserControllerTests.coffee @@ -55,7 +55,7 @@ describe "UserController", -> @settings = siteUrl: "sharelatex.example.com" @UserHandler = - populateGroupLicenceInvite:sinon.stub().callsArgWith(1) + notifyDomainLicence: sinon.stub().callsArgWith(1) @UserSessionsManager = trackSession: sinon.stub() untrackSession: sinon.stub() @@ -267,12 +267,12 @@ describe "UserController", -> done() @UserController.updateUserSettings @req, @res - it "should call populateGroupLicenceInvite", (done)-> + it "should call notifyDomainLicence", (done)-> @req.body.email = @newEmail.toUpperCase() @UserUpdater.changeEmailAddress.callsArgWith(2) @res.sendStatus = (code)=> code.should.equal 200 - @UserHandler.populateGroupLicenceInvite.calledWith(@user).should.equal true + @UserHandler.notifyDomainLicence.calledWith(@user).should.equal true done() @UserController.updateUserSettings @req, @res diff --git a/services/web/test/unit/coffee/User/UserHandlerTests.coffee b/services/web/test/unit/coffee/User/UserHandlerTests.coffee index d6e3cdf07e..51c9a75dcd 100644 --- a/services/web/test/unit/coffee/User/UserHandlerTests.coffee +++ b/services/web/test/unit/coffee/User/UserHandlerTests.coffee @@ -7,21 +7,20 @@ SandboxedModule = require('sandboxed-module') describe "UserHandler", -> beforeEach -> - @user = + @user = _id:"12390i" email: "bob@bob.com" remove: sinon.stub().callsArgWith(0) - @licence = + @licence = subscription_id: 12323434 @SubscriptionDomainHandler = getLicenceUserCanJoin: sinon.stub() @SubscriptionGroupHandler = isUserPartOfGroup:sinon.stub() - convertEmailInvitesToMemberships: sinon.stub().callsArgWith(2) @createStub = sinon.stub().callsArgWith(0) - @NotificationsBuilder = + @NotificationsBuilder = groupPlan:sinon.stub().returns({create:@createStub}) @UserHandler = SandboxedModule.require modulePath, requires: @@ -30,16 +29,11 @@ describe "UserHandler", -> "../Subscription/SubscriptionDomainHandler":@SubscriptionDomainHandler "../Subscription/SubscriptionGroupHandler":@SubscriptionGroupHandler - describe "populateGroupLicenceInvite", -> + describe "notifyDomainLicence", -> describe "no licence", -> beforeEach (done)-> @SubscriptionDomainHandler.getLicenceUserCanJoin.returns() - @UserHandler.populateGroupLicenceInvite @user, done - - it "should call convertEmailInvitesToMemberships", -> - @SubscriptionGroupHandler.convertEmailInvitesToMemberships - .calledWith(@user.email, @user._id) - .should.equal true + @UserHandler.notifyDomainLicence @user, done it "should not call NotificationsBuilder", (done)-> @NotificationsBuilder.groupPlan.called.should.equal false @@ -53,28 +47,18 @@ describe "UserHandler", -> beforeEach (done)-> @SubscriptionDomainHandler.getLicenceUserCanJoin.returns(@licence) @SubscriptionGroupHandler.isUserPartOfGroup.callsArgWith(2, null, false) - @UserHandler.populateGroupLicenceInvite @user, done + @UserHandler.notifyDomainLicence @user, done it "should create notifcation", (done)-> @NotificationsBuilder.groupPlan.calledWith(@user, @licence).should.equal true done() - - it "should call convertEmailInvitesToMemberships", -> - @SubscriptionGroupHandler.convertEmailInvitesToMemberships - .calledWith(@user.email, @user._id) - .should.equal true describe "with matching licence user is already in", -> beforeEach (done)-> @SubscriptionDomainHandler.getLicenceUserCanJoin.returns(@licence) @SubscriptionGroupHandler.isUserPartOfGroup.callsArgWith(2, null, true) - @UserHandler.populateGroupLicenceInvite @user, done + @UserHandler.notifyDomainLicence @user, done it "should create notifcation", (done)-> @NotificationsBuilder.groupPlan.called.should.equal false done() - - it "should call convertEmailInvitesToMemberships", -> - @SubscriptionGroupHandler.convertEmailInvitesToMemberships - .calledWith(@user.email, @user._id) - .should.equal true \ No newline at end of file