diff --git a/services/web/app/coffee/Features/Subscription/LimitationsManager.coffee b/services/web/app/coffee/Features/Subscription/LimitationsManager.coffee index ec29b9257a..4d85ccec47 100644 --- a/services/web/app/coffee/Features/Subscription/LimitationsManager.coffee +++ b/services/web/app/coffee/Features/Subscription/LimitationsManager.coffee @@ -64,8 +64,9 @@ module.exports = 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" + currentTotal = (subscription.member_ids or []).length + (subscription.invited_emails or []).length + limitReached = currentTotal >= subscription.membersLimit + logger.log user_id:user_id, limitReached:limitReached, currentTotal: currentTotal, membersLimit: subscription.membersLimit, "checking if subscription members limit has been reached" callback(err, limitReached, subscription) getOwnerIdOfProject = (project_id, callback)-> diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionGroupController.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionGroupController.coffee index 3e49cd20bd..05737061fe 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionGroupController.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionGroupController.coffee @@ -33,6 +33,14 @@ module.exports = return res.sendStatus 500 res.send() + removeEmailInviteFromGroup: (req, res)-> + adminUserId = AuthenticationController.getLoggedInUserId(req) + email = req.params.email + logger.log {adminUserId, email}, "removing email invite from group subscription" + SubscriptionGroupHandler.removeEmailInviteFromGroup adminUserId, email, (err)-> + return next(error) if error? + res.send() + removeSelfFromGroup: (req, res)-> adminUserId = req.query.admin_user_id userToRemove_id = AuthenticationController.getLoggedInUserId(req) diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee index 1f078f6674..5b7c2e0740 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee @@ -1,6 +1,5 @@ async = require("async") _ = require("underscore") -UserCreator = require("../User/UserCreator") SubscriptionUpdater = require("./SubscriptionUpdater") SubscriptionLocator = require("./SubscriptionLocator") UserLocator = require("../User/UserLocator") @@ -15,36 +14,40 @@ module.exports = SubscriptionGroupHandler = addUserToGroup: (adminUserId, newEmail, callback)-> logger.log adminUserId:adminUserId, newEmail:newEmail, "adding user to group" - UserCreator.getUserOrCreateHoldingAccount newEmail, (err, user)-> + LimitationsManager.hasGroupMembersLimitReached adminUserId, (err, limitReached, subscription)-> if err? - logger.err err:err, adminUserId:adminUserId, newEmail:newEmail, "error creating user for holding account" + logger.err err:err, adminUserId:adminUserId, newEmail:newEmail, "error checking if limit reached for group plan" 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? - logger.err err:err, adminUserId:adminUserId, newEmail:newEmail, "error checking if limit reached for group plan" - return callback(err) - if limitReached - logger.err adminUserId:adminUserId, newEmail:newEmail, "group subscription limit reached not adding user to group" - return callback(limitReached:limitReached) - SubscriptionUpdater.addUserToGroup adminUserId, user._id, (err)-> - if err? - logger.err err:err, "error adding user to group" - return callback(err) - NotificationsBuilder.groupPlan(user, {subscription_id:subscription._id}).read() - userViewModel = buildUserViewModel(user) - callback(err, userViewModel) + if limitReached + logger.err adminUserId:adminUserId, newEmail:newEmail, "group subscription limit reached not adding user to group" + return callback(limitReached:limitReached) + UserLocator.findByEmail newEmail, (err, user)-> + return callback(err) if err? + if user? + SubscriptionUpdater.addUserToGroup adminUserId, user._id, (err)-> + if err? + logger.err err:err, "error adding user to group" + return callback(err) + NotificationsBuilder.groupPlan(user, {subscription_id:subscription._id}).read() + userViewModel = buildUserViewModel(user) + callback(err, userViewModel) + else + SubscriptionUpdater.addEmailInviteToGroup adminUserId, newEmail, (err) -> + return callback(err) if err? + userViewModel = buildEmailInviteViewModel(newEmail) + callback(err, userViewModel) removeUserFromGroup: (adminUser_id, userToRemove_id, callback)-> SubscriptionUpdater.removeUserFromGroup adminUser_id, userToRemove_id, callback - + + removeEmailInviteFromGroup: (adminUser_id, email, callback) -> + SubscriptionUpdater.removeEmailInviteFromGroup adminUser_id, email, callback getPopulatedListOfMembers: (adminUser_id, callback)-> SubscriptionLocator.getUsersSubscription adminUser_id, (err, subscription)-> users = [] + for email in subscription.invited_emails or [] + users.push buildEmailInviteViewModel(email) jobs = _.map subscription.member_ids, (user_id)-> return (cb)-> UserLocator.findById user_id, (err, user)-> @@ -91,7 +94,21 @@ module.exports = SubscriptionGroupHandler = return callback() SubscriptionGroupHandler.addUserToGroup subscription?.admin_id, userEmail, callback - + convertEmailInvitesToMemberships: (email, user_id, callback = (err) ->) -> + SubscriptionLocator.getGroupsWithEmailInvite email, (err, groups = []) -> + return callback(err) if err? + logger.log {email, user_id, groups}, "found groups to convert from email invite to member" + jobs = [] + for group in groups + do (group) -> + jobs.push (cb) -> + SubscriptionUpdater.removeEmailInviteFromGroup group.admin_id, email, (err) -> + return cb(err) if err? + SubscriptionUpdater.addUserToGroup group.admin_id, user_id, (err) -> + return cb(err) if err? + logger.log {group_id: group._id, user_id, email}, "converted email invite to group membership" + return cb() + async.series jobs, callback buildUserViewModel = (user)-> u = @@ -101,3 +118,9 @@ buildUserViewModel = (user)-> holdingAccount: user.holdingAccount _id: user._id return u + +buildEmailInviteViewModel = (email) -> + return { + email: email + holdingAccount: true + } \ No newline at end of file diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionLocator.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionLocator.coffee index beee4a3158..1452f26d99 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionLocator.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionLocator.coffee @@ -30,3 +30,6 @@ module.exports = getGroupSubscriptionMemberOf: (user_id, callback)-> Subscription.findOne {member_ids: user_id}, {_id:1, planCode:1}, callback + + getGroupsWithEmailInvite: (email, callback) -> + Subscription.find { invited_emails: email }, callback \ No newline at end of file diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionRouter.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionRouter.coffee index 62d4d306ab..c5d54a9c49 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionRouter.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionRouter.coffee @@ -25,6 +25,7 @@ module.exports = webRouter.post '/subscription/group/user', AuthenticationController.requireLogin(), SubscriptionGroupController.addUserToGroup webRouter.get '/subscription/group/export', AuthenticationController.requireLogin(), SubscriptionGroupController.exportGroupCsv webRouter.delete '/subscription/group/user/:user_id', AuthenticationController.requireLogin(), SubscriptionGroupController.removeUserFromGroup + webRouter.delete '/subscription/group/email/:email', AuthenticationController.requireLogin(), SubscriptionGroupController.removeEmailInviteFromGroup webRouter.delete '/subscription/group/user', AuthenticationController.requireLogin(), SubscriptionGroupController.removeSelfFromGroup diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee index a4a0864d4f..7b857c6460 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee @@ -35,6 +35,14 @@ module.exports = SubscriptionUpdater = logger.err err:err, searchOps:searchOps, insertOperation:insertOperation, "error findy and modify add user to group" return callback(err) UserFeaturesUpdater.updateFeatures user_id, subscription.planCode, callback + + addEmailInviteToGroup: (adminUser_id, email, callback) -> + logger.log {adminUser_id, email}, "adding email into mongo subscription" + searchOps = + admin_id: adminUser_id + insertOperation = + "$addToSet": {invited_emails: email} + Subscription.findAndModify searchOps, insertOperation, callback removeUserFromGroup: (adminUser_id, user_id, callback)-> searchOps = @@ -47,6 +55,12 @@ module.exports = SubscriptionUpdater = return callback(err) SubscriptionUpdater._setUsersMinimumFeatures user_id, callback + removeEmailInviteFromGroup: (adminUser_id, email, callback)-> + Subscription.update { + admin_id: adminUser_id + }, "$pull": { + invited_emails: email + }, callback _createNewSubscription: (adminUser_id, callback)-> logger.log adminUser_id:adminUser_id, "creating new subscription" diff --git a/services/web/app/coffee/Features/User/UserHandler.coffee b/services/web/app/coffee/Features/User/UserHandler.coffee index 602858eac3..53b2fd9c74 100644 --- a/services/web/app/coffee/Features/User/UserHandler.coffee +++ b/services/web/app/coffee/Features/User/UserHandler.coffee @@ -7,19 +7,22 @@ logger = require("logger-sharelatex") module.exports = UserHandler = populateGroupLicenceInvite: (user, callback = ->)-> - logger.log user_id:user._id, "populating any potential group licence invites" - licence = SubscriptionDomainHandler.getLicenceUserCanJoin user - if !licence? - return callback() + SubscriptionGroupHandler.convertEmailInvitesToMemberships user.email, user._id, (err) -> + return callback(err) if err? - 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" + logger.log user_id:user._id, "populating any potential group licence invites" + licence = SubscriptionDomainHandler.getLicenceUserCanJoin user + if !licence? return callback() - else - NotificationsBuilder.groupPlan(user, licence).create(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) setupLoginData: (user, callback = ->)-> @populateGroupLicenceInvite user, callback diff --git a/services/web/app/coffee/models/Subscription.coffee b/services/web/app/coffee/models/Subscription.coffee index cd036fb6f1..13d8473bcb 100644 --- a/services/web/app/coffee/models/Subscription.coffee +++ b/services/web/app/coffee/models/Subscription.coffee @@ -6,7 +6,8 @@ ObjectId = Schema.ObjectId SubscriptionSchema = new Schema admin_id : {type:ObjectId, ref:'User', index: {unique: true, dropDups: true}} - member_ids : [ type:ObjectId, ref:'User' ] + member_ids : [ type:ObjectId, ref:'User' ] + invited_emails: [ String ] recurlySubscription_id : String planCode : {type: String} groupPlan : {type: Boolean, default: false} diff --git a/services/web/public/coffee/main/group-members.coffee b/services/web/public/coffee/main/group-members.coffee index e28dfc67ce..d1ecd31f53 100644 --- a/services/web/public/coffee/main/group-members.coffee +++ b/services/web/public/coffee/main/group-members.coffee @@ -33,9 +33,13 @@ define [ $scope.removeMembers = () -> for user in $scope.selectedUsers do (user) -> + if user.holdingAccount + url = "/subscription/group/email/#{user.email}" + else + url = "/subscription/group/user/#{user._id}" queuedHttp({ method: "DELETE", - url: "/subscription/group/user/#{user._id}" + url: url headers: "X-Csrf-Token": window.csrfToken }) diff --git a/services/web/test/UnitTests/coffee/Subscription/LimitationsManagerTests.coffee b/services/web/test/UnitTests/coffee/Subscription/LimitationsManagerTests.coffee index f81433e156..a26b2c7a75 100644 --- a/services/web/test/UnitTests/coffee/Subscription/LimitationsManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Subscription/LimitationsManagerTests.coffee @@ -257,24 +257,25 @@ describe "LimitationsManager", -> beforeEach -> @user_id = "12312" @subscription = - membersLimit: 2 + membersLimit: 3 member_ids: ["", ""] + invited_emails: ["bob@example.com"] - it "should return true if the limit is hit", (done)-> + it "should return true if the limit is hit (including members and invites)", (done)-> @SubscriptionLocator.getUsersSubscription.callsArgWith(1, null, @subscription) @LimitationsManager.hasGroupMembersLimitReached @user_id, (err, limitReached)-> limitReached.should.equal true done() - it "should return false if the limit is not hit", (done)-> - @subscription.membersLimit = 3 + it "should return false if the limit is not hit (including members and invites)", (done)-> + @subscription.membersLimit = 4 @SubscriptionLocator.getUsersSubscription.callsArgWith(1, null, @subscription) @LimitationsManager.hasGroupMembersLimitReached @user_id, (err, limitReached)-> limitReached.should.equal false done() - it "should return true if the limit has been exceded", (done)-> - @subscription.membersLimit = 0 + it "should return true if the limit has been exceded (including members and invites)", (done)-> + @subscription.membersLimit = 2 @SubscriptionLocator.getUsersSubscription.callsArgWith(1, null, @subscription) @LimitationsManager.hasGroupMembersLimitReached @user_id, (err, limitReached)-> limitReached.should.equal true diff --git a/services/web/test/UnitTests/coffee/Subscription/SubscriptionGroupControllerTests.coffee b/services/web/test/UnitTests/coffee/Subscription/SubscriptionGroupControllerTests.coffee index 5c835e5cc5..d493eacd8d 100644 --- a/services/web/test/UnitTests/coffee/Subscription/SubscriptionGroupControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Subscription/SubscriptionGroupControllerTests.coffee @@ -24,6 +24,7 @@ describe "SubscriptionGroupController", -> @GroupHandler = addUserToGroup: sinon.stub().callsArgWith(2, null, @user) removeUserFromGroup: sinon.stub().callsArgWith(2) + removeEmailInviteFromGroup: sinon.stub().callsArgWith(2) isUserPartOfGroup: sinon.stub() sendVerificationEmail:sinon.stub() processGroupVerification:sinon.stub() @@ -69,7 +70,7 @@ describe "SubscriptionGroupController", -> describe "removeUserFromGroup", -> - it "should use the admin id for the logged in user and take the email address from the body", (done)-> + it "should use the admin id for the logged in user and take the user id from the params", (done)-> userIdToRemove = "31231" @req.params = user_id: userIdToRemove @@ -79,6 +80,15 @@ describe "SubscriptionGroupController", -> done() @Controller.removeUserFromGroup @req, res + describe "removeEmailInviteFromGroup", -> + it "should use the admin id for the logged in user and take the email from the params", (done)-> + email = "jo@example.com" + @req.params = email: email + res = + send : => + @GroupHandler.removeEmailInviteFromGroup.calledWith(@adminUserId, email).should.equal true + done() + @Controller.removeEmailInviteFromGroup @req, res describe "renderSubscriptionGroupAdminPage", -> it "should redirect you if you don't have a group account", (done)-> diff --git a/services/web/test/UnitTests/coffee/Subscription/SubscriptionGroupHandlerTests.coffee b/services/web/test/UnitTests/coffee/Subscription/SubscriptionGroupHandlerTests.coffee index d21679408e..1daf43bbd7 100644 --- a/services/web/test/UnitTests/coffee/Subscription/SubscriptionGroupHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/Subscription/SubscriptionGroupHandlerTests.coffee @@ -11,6 +11,7 @@ describe "SubscriptionGroupHandler", -> @adminUser_id = "12321" @newEmail = "bob@smith.com" @user_id = "3121321" + @email = "jim@example.com" @user = {_id:@user_id, email:@newEmail} @subscription_id = "31DSd1123D" @@ -29,9 +30,12 @@ describe "SubscriptionGroupHandler", -> @SubscriptionUpdater = addUserToGroup: sinon.stub().callsArgWith(2) removeUserFromGroup: sinon.stub().callsArgWith(2) + addEmailInviteToGroup: sinon.stub().callsArgWith(2) + removeEmailInviteFromGroup: sinon.stub().callsArgWith(2) @UserLocator = findById: sinon.stub() + findByEmail: sinon.stub() @LimitationsManager = hasGroupMembersLimitReached: sinon.stub() @@ -68,14 +72,16 @@ describe "SubscriptionGroupHandler", -> describe "addUserToGroup", -> - it "should find or create the user", (done)-> + beforeEach -> @LimitationsManager.hasGroupMembersLimitReached.callsArgWith(1, null, false, @subscription) + @UserLocator.findByEmail.callsArgWith(1, null, @user) + + it "should find the user", (done)-> @Handler.addUserToGroup @adminUser_id, @newEmail, (err)=> - @UserCreator.getUserOrCreateHoldingAccount.calledWith(@newEmail).should.equal true + @UserLocator.findByEmail.calledWith(@newEmail).should.equal true done() it "should add the user to the group", (done)-> - @LimitationsManager.hasGroupMembersLimitReached.callsArgWith(1, null, false, @subscription) @Handler.addUserToGroup @adminUser_id, @newEmail, (err)=> @SubscriptionUpdater.addUserToGroup.calledWith(@adminUser_id, @user._id).should.equal true done() @@ -93,12 +99,16 @@ describe "SubscriptionGroupHandler", -> done() it "should mark any notification as read if it is part of a licence", (done)-> - @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() - + + it "should add an email invite if no user is found", (done) -> + @UserLocator.findByEmail.callsArgWith(1, null, null) + @Handler.addUserToGroup @adminUser_id, @newEmail, (err)=> + @SubscriptionUpdater.addEmailInviteToGroup.calledWith(@adminUser_id, @newEmail).should.equal true + done() describe "removeUserFromGroup", -> @@ -138,6 +148,16 @@ describe "SubscriptionGroupHandler", -> assert.deepEqual users[1], {_id:@subscription.member_ids[1]} assert.deepEqual users[2], {_id:@subscription.member_ids[2]} done() + + it "should return any invited users", (done) -> + @subscription.invited_emails = ["jo@example.com", "charlie@example.com"] + @Handler.getPopulatedListOfMembers @adminUser_id, (err, users)=> + users[0].email.should.equal "jo@example.com" + users[0].holdingAccount.should.equal true + users[1].email.should.equal "charlie@example.com" + users[1].holdingAccount.should.equal true + users.length.should.equal @subscription.invited_emails.length + done() describe "isUserPartOfGroup", -> beforeEach -> @@ -191,7 +211,28 @@ describe "SubscriptionGroupHandler", -> err.should.equal "token_not_found" done() + describe "convertEmailInvitesToMemberships", -> + beforeEach -> + @SubscriptionLocator.getGroupsWithEmailInvite = sinon.stub().yields(null, @groups = [{ admin_id: "group-1" }, { admin_id: "group-2" }]) - - + it "should get groups with the email address invited to", (done) -> + @Handler.convertEmailInvitesToMemberships @email, @user_id, (err) => + @SubscriptionLocator.getGroupsWithEmailInvite.calledWith(@email).should.equal true + done() + + it "should remove the email from each group", (done) -> + @Handler.convertEmailInvitesToMemberships @email, @user_id, (err) => + for group in @groups + @SubscriptionUpdater.removeEmailInviteFromGroup + .calledWith(group.admin_id, @email) + .should.equal true + done() + + it "should add the user to each group", (done) -> + @Handler.convertEmailInvitesToMemberships @email, @user_id, (err) => + for group in @groups + @SubscriptionUpdater.addUserToGroup + .calledWith(group.admin_id, @user_id) + .should.equal true + done() diff --git a/services/web/test/UnitTests/coffee/User/UserHandlerTests.coffee b/services/web/test/UnitTests/coffee/User/UserHandlerTests.coffee index 8bccd1b12c..d6e3cdf07e 100644 --- a/services/web/test/UnitTests/coffee/User/UserHandlerTests.coffee +++ b/services/web/test/UnitTests/coffee/User/UserHandlerTests.coffee @@ -19,6 +19,7 @@ describe "UserHandler", -> @SubscriptionGroupHandler = isUserPartOfGroup:sinon.stub() + convertEmailInvitesToMemberships: sinon.stub().callsArgWith(2) @createStub = sinon.stub().callsArgWith(0) @NotificationsBuilder = groupPlan:sinon.stub().returns({create:@createStub}) @@ -30,11 +31,15 @@ describe "UserHandler", -> "../Subscription/SubscriptionGroupHandler":@SubscriptionGroupHandler describe "populateGroupLicenceInvite", -> - 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 it "should not call NotificationsBuilder", (done)-> @NotificationsBuilder.groupPlan.called.should.equal false @@ -45,7 +50,6 @@ describe "UserHandler", -> done() describe "with matching licence user is not in", -> - beforeEach (done)-> @SubscriptionDomainHandler.getLicenceUserCanJoin.returns(@licence) @SubscriptionGroupHandler.isUserPartOfGroup.callsArgWith(2, null, false) @@ -54,11 +58,13 @@ describe "UserHandler", -> 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) @@ -66,4 +72,9 @@ describe "UserHandler", -> it "should create notifcation", (done)-> @NotificationsBuilder.groupPlan.called.should.equal false - done() \ No newline at end of file + done() + + it "should call convertEmailInvitesToMemberships", -> + @SubscriptionGroupHandler.convertEmailInvitesToMemberships + .calledWith(@user.email, @user._id) + .should.equal true \ No newline at end of file