From 259d690f7cf6916ffcf9aab4b2aadb530660e709 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez=20Capel?= Date: Fri, 25 May 2018 15:45:33 +0100 Subject: [PATCH 1/2] Add method to update user references --- .../SubscriptionGroupHandler.coffee | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee index 5b7c2e0740..6cfe6b1e79 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee @@ -39,12 +39,27 @@ module.exports = SubscriptionGroupHandler = removeUserFromGroup: (adminUser_id, userToRemove_id, callback)-> SubscriptionUpdater.removeUserFromGroup adminUser_id, userToRemove_id, callback - + removeEmailInviteFromGroup: (adminUser_id, email, callback) -> SubscriptionUpdater.removeEmailInviteFromGroup adminUser_id, email, callback + + replaceUserReferencesInGroups: (oldId, newId, callback) -> + Subscription.update {admin_id: userStubId}, {admin_id: slUserId}, (error) -> + callback(error) if error? + + # Mongo won't let us pull and addToSet in the same query, so do it in + # two. Note we need to add first, since the query is based on the old user. + query = {member_ids: userStubId} + addNewUserUpdate = $addToSet: {member_ids: slUserId} + removeOldUserUpdate = $pull: {member_ids: userStubId} + + Subscription.update query, addNewUserUpdate, { multi: true }, (error) -> + return callback(error) if error? + Subscription.update query, removeOldUserUpdate, { multi: true }, callback + getPopulatedListOfMembers: (adminUser_id, callback)-> - SubscriptionLocator.getUsersSubscription adminUser_id, (err, subscription)-> + SubscriptionLocator.getUsersSubscription adminUser_id, (err, subscription)-> users = [] for email in subscription.invited_emails or [] users.push buildEmailInviteViewModel(email) @@ -111,7 +126,7 @@ module.exports = SubscriptionGroupHandler = async.series jobs, callback buildUserViewModel = (user)-> - u = + u = email: user.email first_name: user.first_name last_name: user.last_name @@ -123,4 +138,4 @@ buildEmailInviteViewModel = (email) -> return { email: email holdingAccount: true - } \ No newline at end of file + } From b52fbdbfa4539bf56b5593c3772d078ca3ad6dba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberto=20Fern=C3=A1ndez=20Capel?= Date: Tue, 29 May 2018 15:35:46 +0100 Subject: [PATCH 2/2] Unit test SubscriptionGroupHandler.replaceUserReferencesInGroups --- .../SubscriptionGroupHandler.coffee | 9 ++-- .../SubscriptionGroupHandlerTests.coffee | 54 +++++++++++++++---- 2 files changed, 48 insertions(+), 15 deletions(-) diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee index 6cfe6b1e79..fe1e78e5e3 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionGroupHandler.coffee @@ -2,6 +2,7 @@ async = require("async") _ = require("underscore") SubscriptionUpdater = require("./SubscriptionUpdater") SubscriptionLocator = require("./SubscriptionLocator") +Subscription = require("../../models/Subscription").Subscription UserLocator = require("../User/UserLocator") LimitationsManager = require("./LimitationsManager") logger = require("logger-sharelatex") @@ -45,14 +46,14 @@ module.exports = SubscriptionGroupHandler = replaceUserReferencesInGroups: (oldId, newId, callback) -> - Subscription.update {admin_id: userStubId}, {admin_id: slUserId}, (error) -> + Subscription.update {admin_id: oldId}, {admin_id: newId}, (error) -> callback(error) if error? # Mongo won't let us pull and addToSet in the same query, so do it in # two. Note we need to add first, since the query is based on the old user. - query = {member_ids: userStubId} - addNewUserUpdate = $addToSet: {member_ids: slUserId} - removeOldUserUpdate = $pull: {member_ids: userStubId} + query = { member_ids: oldId } + addNewUserUpdate = $addToSet: { member_ids: newId } + removeOldUserUpdate = $pull: { member_ids: oldId } Subscription.update query, addNewUserUpdate, { multi: true }, (error) -> return callback(error) if error? diff --git a/services/web/test/unit/coffee/Subscription/SubscriptionGroupHandlerTests.coffee b/services/web/test/unit/coffee/Subscription/SubscriptionGroupHandlerTests.coffee index 1daf43bbd7..acb5bdea16 100644 --- a/services/web/test/unit/coffee/Subscription/SubscriptionGroupHandlerTests.coffee +++ b/services/web/test/unit/coffee/Subscription/SubscriptionGroupHandlerTests.coffee @@ -19,12 +19,12 @@ describe "SubscriptionGroupHandler", -> admin_id:@adminUser_id _id:@subscription_id - @SubscriptionLocator = + @SubscriptionLocator = getUsersSubscription: sinon.stub() getSubscriptionByMemberIdAndId: sinon.stub() getSubscription: sinon.stub() - @UserCreator = + @UserCreator = getUserOrCreateHoldingAccount: sinon.stub().callsArgWith(1, null, @user) @SubscriptionUpdater = @@ -47,7 +47,10 @@ describe "SubscriptionGroupHandler", -> @EmailHandler = sendEmail:sinon.stub() - @settings = + @Subscription = + update: sinon.stub().yields() + + @settings = siteUrl:"http://www.sharelatex.com" @readStub = sinon.stub() @@ -59,13 +62,14 @@ describe "SubscriptionGroupHandler", -> "../User/UserCreator": @UserCreator "./SubscriptionUpdater": @SubscriptionUpdater "./SubscriptionLocator": @SubscriptionLocator + "../../models/Subscription": Subscription: @Subscription "../User/UserLocator": @UserLocator "./LimitationsManager": @LimitationsManager "../Security/OneTimeTokenHandler":@OneTimeTokenHandler "../Email/EmailHandler":@EmailHandler "settings-sharelatex":@settings "../Notifications/NotificationsBuilder": @NotificationsBuilder - "logger-sharelatex": + "logger-sharelatex": err:-> log:-> warn:-> @@ -75,7 +79,7 @@ describe "SubscriptionGroupHandler", -> 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)=> @UserLocator.findByEmail.calledWith(@newEmail).should.equal true @@ -85,7 +89,7 @@ describe "SubscriptionGroupHandler", -> @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, @subscription) @Handler.addUserToGroup @adminUser_id, @newEmail, (err)=> @@ -103,7 +107,7 @@ describe "SubscriptionGroupHandler", -> @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)=> @@ -117,6 +121,35 @@ describe "SubscriptionGroupHandler", -> @SubscriptionUpdater.removeUserFromGroup.calledWith(@adminUser_id, @user._id).should.equal true done() + describe "replaceUserReferencesInGroups", -> + beforeEach -> + @oldId = "ba5eba11" + @newId = "5ca1ab1e" + + it "replaces the admin_id", (done) -> + @Handler.replaceUserReferencesInGroups @oldId, @newId, (err) => + + @Subscription.update.calledWith( + { admin_id: @oldId }, + { admin_id: @newId } + ).should.equal true + + done() + + it "replaces the member ids", (done) -> + @Handler.replaceUserReferencesInGroups @oldId, @newId, (err) => + + @Subscription.update.calledWith( + { member_ids: @oldId }, + { $addToSet: { member_ids: @newId } } + ).should.equal true + + @Subscription.update.calledWith( + { member_ids: @oldId }, + { $pull: { member_ids: @oldId } } + ).should.equal true + + done() describe "getPopulatedListOfMembers", -> beforeEach -> @@ -148,7 +181,7 @@ 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)=> @@ -219,7 +252,7 @@ describe "SubscriptionGroupHandler", -> @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 @@ -227,7 +260,7 @@ describe "SubscriptionGroupHandler", -> .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 @@ -235,4 +268,3 @@ describe "SubscriptionGroupHandler", -> .calledWith(group.admin_id, @user_id) .should.equal true done() -