diff --git a/services/web/app/coffee/Features/Subscription/LimitationsManager.coffee b/services/web/app/coffee/Features/Subscription/LimitationsManager.coffee index f0513fb590..d323c19a9d 100644 --- a/services/web/app/coffee/Features/Subscription/LimitationsManager.coffee +++ b/services/web/app/coffee/Features/Subscription/LimitationsManager.coffee @@ -52,11 +52,17 @@ module.exports = return callback(err) if err? callback err, subscriptions.length > 0, subscriptions - hasGroupMembersLimitReached: (user_id, callback)-> + hasGroupMembersLimitReached: (user_id, callback = (err, limitReached, subscription)->)-> SubscriptionLocator.getUsersSubscription user_id, (err, subscription)-> + if err? + logger.err err:err, user_id:user_id, "error getting users subscription" + return callback(err) + if !subscription? + logger.err user_id:user_id, "no subscription found for user" + return callback("no subscription found") limitReached = subscription.member_ids.length >= subscription.membersLimit logger.log user_id:user_id, limitReached:limitReached, currentTotal: subscription.member_ids.length, membersLimit: subscription.membersLimit, "checking if subscription members limit has been reached" - callback(err, limitReached) + callback(err, limitReached, subscription) getOwnerOfProject = (project_id, callback)-> Project.findById project_id, 'owner_ref', (error, project) -> diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee index 74d7a845cc..4905203282 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee @@ -13,14 +13,21 @@ NotificationsBuilder = require("../Notifications/NotificationsBuilder") module.exports = SubscriptionGroupHandler = - addUserToGroup: (subscription, newEmail, callback)-> + addUserToGroup: (adminUserId, newEmail, callback)-> UserCreator.getUserOrCreateHoldingAccount newEmail, (err, user)-> - LimitationsManager.hasGroupMembersLimitReached subscription.admin_id, (err, limitReached)-> + if err? + logger.err err:err, "error creating user for holding account" + return callback(err) + if !user? + msg = "no user returned whenc reating holidng account or getting user" + logger.err adminUserId:adminUserId, newEmail:newEmail, msg + return callback(msg) + LimitationsManager.hasGroupMembersLimitReached adminUserId, (err, limitReached, subscription)-> if err? return callback(err) if limitReached return callback(limitReached:limitReached) - SubscriptionUpdater.addUserToGroup subscription.admin_id, user._id, (err)-> + SubscriptionUpdater.addUserToGroup adminUserId, user._id, (err)-> if err? logger.err err:err, "error adding user to group" return callback(err) @@ -73,7 +80,14 @@ module.exports = SubscriptionGroupHandler = logger.err userEmail:userEmail, token:token, "token value not found for processing group verification" return callback("token_not_found") SubscriptionLocator.getSubscription subscription_id, (err, subscription)-> - SubscriptionGroupHandler.addUserToGroup subscription, userEmail, callback + if err? + logger.err err:err, subscription:subscription, userEmail:userEmail, subscription_id:subscription_id, "error getting subscription" + return callback(err) + if !subscription? + logger.warn subscription_id:subscription_id, "no subscription found" + return callback() + SubscriptionGroupHandler.addUserToGroup subscription?.admin_id, userEmail, callback + buildUserViewModel = (user)-> diff --git a/services/web/test/UnitTests/coffee/Subscription/SubscriptionGroupHandlerTests.coffee b/services/web/test/UnitTests/coffee/Subscription/SubscriptionGroupHandlerTests.coffee index 44bde72ff9..d21679408e 100644 --- a/services/web/test/UnitTests/coffee/Subscription/SubscriptionGroupHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/Subscription/SubscriptionGroupHandlerTests.coffee @@ -64,36 +64,37 @@ describe "SubscriptionGroupHandler", -> "logger-sharelatex": err:-> log:-> + warn:-> describe "addUserToGroup", -> it "should find or create the user", (done)-> - @LimitationsManager.hasGroupMembersLimitReached.callsArgWith(1, null, false) - @Handler.addUserToGroup @subscription, @newEmail, (err)=> + @LimitationsManager.hasGroupMembersLimitReached.callsArgWith(1, null, false, @subscription) + @Handler.addUserToGroup @adminUser_id, @newEmail, (err)=> @UserCreator.getUserOrCreateHoldingAccount.calledWith(@newEmail).should.equal true done() it "should add the user to the group", (done)-> - @LimitationsManager.hasGroupMembersLimitReached.callsArgWith(1, null, false) - @Handler.addUserToGroup @subscription, @newEmail, (err)=> + @LimitationsManager.hasGroupMembersLimitReached.callsArgWith(1, null, false, @subscription) + @Handler.addUserToGroup @adminUser_id, @newEmail, (err)=> @SubscriptionUpdater.addUserToGroup.calledWith(@adminUser_id, @user._id).should.equal true done() it "should not add the user to the group if the limit has been reached", (done)-> - @LimitationsManager.hasGroupMembersLimitReached.callsArgWith(1, null, true) - @Handler.addUserToGroup @subscription, @newEmail, (err)=> + @LimitationsManager.hasGroupMembersLimitReached.callsArgWith(1, null, true, @subscription) + @Handler.addUserToGroup @adminUser_id, @newEmail, (err)=> @SubscriptionUpdater.addUserToGroup.called.should.equal false done() it "should return error that limit has been reached", (done)-> - @LimitationsManager.hasGroupMembersLimitReached.callsArgWith(1, null, true) - @Handler.addUserToGroup @subscription, @newEmail, (err)=> + @LimitationsManager.hasGroupMembersLimitReached.callsArgWith(1, null, true, @subscription) + @Handler.addUserToGroup @adminUser_id, @newEmail, (err)=> err.limitReached.should.equal true done() it "should mark any notification as read if it is part of a licence", (done)-> - @LimitationsManager.hasGroupMembersLimitReached.callsArgWith(1, null, false) - @Handler.addUserToGroup @subscription, @newEmail, (err)=> + @LimitationsManager.hasGroupMembersLimitReached.callsArgWith(1, null, false, @subscription) + @Handler.addUserToGroup @adminUser_id, @newEmail, (err)=> @NotificationsBuilder.groupPlan.calledWith(@user, {subscription_id:@subscription._id}).should.equal true @readStub.called.should.equal true done() @@ -175,14 +176,13 @@ describe "SubscriptionGroupHandler", -> describe "processGroupVerification", -> beforeEach -> @token = "31dDAd2Da" - @admin_id = "eDSda1ew" - @SubscriptionLocator.getSubscription.callsArgWith(1, null, @Subscription) + @SubscriptionLocator.getSubscription.callsArgWith(1, null, @subscription) @Handler.addUserToGroup = sinon.stub().callsArgWith(2) it "should addUserToGroup", (done)-> @OneTimeTokenHandler.getValueFromTokenAndExpire.callsArgWith(1, null, @subscription_id) @Handler.processGroupVerification @email, @subscription_id, @token, (err)=> - @Handler.addUserToGroup.calledWith(@Subscription, @email).should.equal true + @Handler.addUserToGroup.calledWith(@adminUser_id, @email).should.equal true done() it "should return token_not_found error if it couldn't get the token", (done)->