From e7d8848a6a74e43036a0306a89c8f6012ff56d5f Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Mon, 4 Dec 2017 15:12:26 +0000 Subject: [PATCH 1/8] Include possible overleaf subscription in feature setting --- .../Subscription/SubscriptionUpdater.coffee | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee index 12faf0e234..e26f39de07 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee @@ -8,6 +8,7 @@ Settings = require("settings-sharelatex") logger = require("logger-sharelatex") ObjectId = require('mongoose').Types.ObjectId ReferalAllocator = require("../Referal/ReferalAllocator") +Modules = require '../../infrastructure/Modules' oneMonthInSeconds = 60 * 60 * 24 * 30 @@ -106,17 +107,26 @@ module.exports = SubscriptionUpdater = SubscriptionLocator.getUsersSubscription user_id, cb groupSubscription: (cb)-> SubscriptionLocator.getGroupSubscriptionMemberOf user_id, cb + overleafPlanCode: (cb) -> + Modules.hooks.fire 'getOverleafPlanCode', user_id, cb async.series jobs, (err, results)-> if err? logger.err err:err, user_id:user, "error getting subscription or group for _setUsersMinimumFeatures" return callback(err) - {subscription, groupSubscription} = results - if subscription? and subscription.planCode? and subscription.planCode != Settings.defaultPlanCode - logger.log user_id:user_id, "using users subscription plan code for features" - UserFeaturesUpdater.updateFeatures user_id, subscription.planCode, callback - else if groupSubscription? and groupSubscription.planCode? + {subscription, groupSubscription, overleafPlanCode} = results + # Group Subscription + if groupSubscription? and groupSubscription.planCode? logger.log user_id:user_id, "using group which user is memor of for features" UserFeaturesUpdater.updateFeatures user_id, groupSubscription.planCode, callback + # Personal Subscription + else if subscription? and subscription.planCode? and subscription.planCode != Settings.defaultPlanCode + logger.log user_id:user_id, "using users subscription plan code for features" + UserFeaturesUpdater.updateFeatures user_id, subscription.planCode, callback + # Overleaf Subscription + else if overleafPlanCode? + logger.log user_id: user_id, "using the overleaf plan for features" + UserFeaturesUpdater.updateFeatures user_id, overleafPlanCode, callback + # Default else logger.log user_id:user_id, "using default features for user with no subscription or group" UserFeaturesUpdater.updateFeatures user_id, Settings.defaultPlanCode, (err)-> From 44c289d8dd28f5b1d7756524968cf413c71ddd1b Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Mon, 4 Dec 2017 15:34:46 +0000 Subject: [PATCH 2/8] Fix unit tests for SubscriptionUpdater --- .../Subscription/SubscriptionUpdaterTests.coffee | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/services/web/test/unit/coffee/Subscription/SubscriptionUpdaterTests.coffee b/services/web/test/unit/coffee/Subscription/SubscriptionUpdaterTests.coffee index 87ff474a0a..3f25465ae0 100644 --- a/services/web/test/unit/coffee/Subscription/SubscriptionUpdaterTests.coffee +++ b/services/web/test/unit/coffee/Subscription/SubscriptionUpdaterTests.coffee @@ -57,6 +57,7 @@ describe "SubscriptionUpdater", -> @ReferalAllocator = assignBonus:sinon.stub().callsArgWith(1) @ReferalAllocator.cock = true + @Modules = {hooks: {fire: sinon.stub().callsArgWith(2, null, null)}} @SubscriptionUpdater = SandboxedModule.require modulePath, requires: '../../models/Subscription': Subscription:@SubscriptionModel './UserFeaturesUpdater': @UserFeaturesUpdater @@ -65,6 +66,7 @@ describe "SubscriptionUpdater", -> "logger-sharelatex": log:-> 'settings-sharelatex': @Settings "../Referal/ReferalAllocator" : @ReferalAllocator + '../../infrastructure/Modules': @Modules describe "syncSubscription", -> @@ -204,10 +206,22 @@ describe "SubscriptionUpdater", -> assert.equal args[1], @groupSubscription.planCode done() + it "should call updateFeatures with the overleaf subscription if set", (done)-> + @SubscriptionLocator.getUsersSubscription.callsArgWith(1, null) + @SubscriptionLocator.getGroupSubscriptionMemberOf.callsArgWith(1, null, null) + @Modules.hooks.fire = sinon.stub().callsArgWith(2, null, 'ol_pro') + + @SubscriptionUpdater._setUsersMinimumFeatures @adminUser._id, (err)=> + args = @UserFeaturesUpdater.updateFeatures.args[0] + assert.equal args[0], @adminUser._id + assert.equal args[1], 'ol_pro' + done() + it "should call not call updateFeatures with users subscription if the subscription plan code is the default one (downgraded)", (done)-> @subscription.planCode = @Settings.defaultPlanCode @SubscriptionLocator.getUsersSubscription.callsArgWith(1, null, @subscription) @SubscriptionLocator.getGroupSubscriptionMemberOf.callsArgWith(1, null, @groupSubscription) + @Modules.hooks.fire = sinon.stub().callsArgWith(2, null, null) @SubscriptionUpdater._setUsersMinimumFeatures @adminuser_id, (err)=> args = @UserFeaturesUpdater.updateFeatures.args[0] assert.equal args[0], @adminUser._id @@ -218,6 +232,7 @@ describe "SubscriptionUpdater", -> it "should call updateFeatures with default if there are no subscriptions for user", (done)-> @SubscriptionLocator.getUsersSubscription.callsArgWith(1, null) @SubscriptionLocator.getGroupSubscriptionMemberOf.callsArgWith(1, null) + @Modules.hooks.fire = sinon.stub().callsArgWith(2, null, null) @SubscriptionUpdater._setUsersMinimumFeatures @adminuser_id, (err)=> args = @UserFeaturesUpdater.updateFeatures.args[0] assert.equal args[0], @adminUser._id From 4e86a03b4632e70567ecd1f8765c5865a56e4c20 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Thu, 7 Dec 2017 15:06:32 +0000 Subject: [PATCH 3/8] Fix a problem with the order of module initialization --- .../app/coffee/Features/Subscription/SubscriptionUpdater.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee index e26f39de07..33ade7a5ea 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee @@ -8,7 +8,6 @@ Settings = require("settings-sharelatex") logger = require("logger-sharelatex") ObjectId = require('mongoose').Types.ObjectId ReferalAllocator = require("../Referal/ReferalAllocator") -Modules = require '../../infrastructure/Modules' oneMonthInSeconds = 60 * 60 * 24 * 30 @@ -108,6 +107,7 @@ module.exports = SubscriptionUpdater = groupSubscription: (cb)-> SubscriptionLocator.getGroupSubscriptionMemberOf user_id, cb overleafPlanCode: (cb) -> + Modules = require '../../infrastructure/Modules' Modules.hooks.fire 'getOverleafPlanCode', user_id, cb async.series jobs, (err, results)-> if err? From 3adbc52089fc4af1a5d547b9d61642e610794f24 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Thu, 7 Dec 2017 16:28:07 +0000 Subject: [PATCH 4/8] Add a method to explicitely refresh subscription feature --- .../coffee/Features/Subscription/SubscriptionUpdater.coffee | 3 +++ 1 file changed, 3 insertions(+) diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee index 33ade7a5ea..98d241fbd3 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee @@ -62,6 +62,9 @@ module.exports = SubscriptionUpdater = invited_emails: email }, callback + refreshSubscription: (user_id, callback=(err)->) -> + SubscriptionUpdater._setUsersMinimumFeatures user_id, callback + deleteSubscription: (subscription_id, callback = (error) ->) -> SubscriptionLocator.getSubscription subscription_id, (err, subscription) -> return callback(err) if err? From a675e774b8f5828c869db4a066515feae2e8bca9 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Fri, 8 Dec 2017 11:29:11 +0000 Subject: [PATCH 5/8] Fix how the `getOverleafPlanCode` integration is done --- .../coffee/Features/Subscription/SubscriptionUpdater.coffee | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee index 98d241fbd3..9b5fe83f88 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee @@ -111,7 +111,8 @@ module.exports = SubscriptionUpdater = SubscriptionLocator.getGroupSubscriptionMemberOf user_id, cb overleafPlanCode: (cb) -> Modules = require '../../infrastructure/Modules' - Modules.hooks.fire 'getOverleafPlanCode', user_id, cb + Modules.hooks.fire 'getOverleafPlanCode', user_id, (err, results) -> + cb(err, results[0]) async.series jobs, (err, results)-> if err? logger.err err:err, user_id:user, "error getting subscription or group for _setUsersMinimumFeatures" From eefe56d0f77c40a0b794ea6b9e8de1687c071ae8 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Fri, 8 Dec 2017 11:37:24 +0000 Subject: [PATCH 6/8] Fix up error handling in SubscriptionUpdater --- .../coffee/Features/Subscription/SubscriptionUpdater.coffee | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee index 9b5fe83f88..d3b1a0feb7 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee @@ -112,10 +112,11 @@ module.exports = SubscriptionUpdater = overleafPlanCode: (cb) -> Modules = require '../../infrastructure/Modules' Modules.hooks.fire 'getOverleafPlanCode', user_id, (err, results) -> - cb(err, results[0]) + cb(err, results?[0] || null) async.series jobs, (err, results)-> if err? - logger.err err:err, user_id:user, "error getting subscription or group for _setUsersMinimumFeatures" + logger.err err:err, user_id:user_id, + "error getting subscription or group for _setUsersMinimumFeatures" return callback(err) {subscription, groupSubscription, overleafPlanCode} = results # Group Subscription From 6fe937bcadcc9ff895381785e88f9cda8f39fcf4 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Wed, 13 Dec 2017 10:57:56 +0000 Subject: [PATCH 7/8] Finish unit tests for SubscriptionUpdater --- .../Subscription/SubscriptionUpdaterTests.coffee | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/services/web/test/unit/coffee/Subscription/SubscriptionUpdaterTests.coffee b/services/web/test/unit/coffee/Subscription/SubscriptionUpdaterTests.coffee index 3f25465ae0..93479a03a1 100644 --- a/services/web/test/unit/coffee/Subscription/SubscriptionUpdaterTests.coffee +++ b/services/web/test/unit/coffee/Subscription/SubscriptionUpdaterTests.coffee @@ -209,7 +209,7 @@ describe "SubscriptionUpdater", -> it "should call updateFeatures with the overleaf subscription if set", (done)-> @SubscriptionLocator.getUsersSubscription.callsArgWith(1, null) @SubscriptionLocator.getGroupSubscriptionMemberOf.callsArgWith(1, null, null) - @Modules.hooks.fire = sinon.stub().callsArgWith(2, null, 'ol_pro') + @Modules.hooks.fire = sinon.stub().callsArgWith(2, null, ['ol_pro']) @SubscriptionUpdater._setUsersMinimumFeatures @adminUser._id, (err)=> args = @UserFeaturesUpdater.updateFeatures.args[0] @@ -278,3 +278,13 @@ describe "SubscriptionUpdater", -> @SubscriptionUpdater._setUsersMinimumFeatures .calledWith(user_id) .should.equal true + + describe 'refreshSubscription', -> + beforeEach -> + @SubscriptionUpdater._setUsersMinimumFeatures = sinon.stub() + .callsArgWith(1, null) + + it 'should call to _setUsersMinimumFeatures', -> + @SubscriptionUpdater.refreshSubscription(@adminUser._id, ()->) + @SubscriptionUpdater._setUsersMinimumFeatures.callCount.should.equal 1 + @SubscriptionUpdater._setUsersMinimumFeatures.calledWith(@adminUser._id).should.equal true From 56ec81bb3efc4e67122cbb206b0137f9d3c51127 Mon Sep 17 00:00:00 2001 From: Shane Kilkelly Date: Mon, 18 Dec 2017 13:52:37 +0000 Subject: [PATCH 8/8] Change 'oveleafPlanCode' to 'v1PlanCode' --- .../Subscription/SubscriptionUpdater.coffee | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee index d3b1a0feb7..649551b5b2 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee @@ -109,16 +109,16 @@ module.exports = SubscriptionUpdater = SubscriptionLocator.getUsersSubscription user_id, cb groupSubscription: (cb)-> SubscriptionLocator.getGroupSubscriptionMemberOf user_id, cb - overleafPlanCode: (cb) -> + v1PlanCode: (cb) -> Modules = require '../../infrastructure/Modules' - Modules.hooks.fire 'getOverleafPlanCode', user_id, (err, results) -> + Modules.hooks.fire 'getV1PlanCode', user_id, (err, results) -> cb(err, results?[0] || null) async.series jobs, (err, results)-> if err? logger.err err:err, user_id:user_id, "error getting subscription or group for _setUsersMinimumFeatures" return callback(err) - {subscription, groupSubscription, overleafPlanCode} = results + {subscription, groupSubscription, v1PlanCode} = results # Group Subscription if groupSubscription? and groupSubscription.planCode? logger.log user_id:user_id, "using group which user is memor of for features" @@ -127,10 +127,10 @@ module.exports = SubscriptionUpdater = else if subscription? and subscription.planCode? and subscription.planCode != Settings.defaultPlanCode logger.log user_id:user_id, "using users subscription plan code for features" UserFeaturesUpdater.updateFeatures user_id, subscription.planCode, callback - # Overleaf Subscription - else if overleafPlanCode? - logger.log user_id: user_id, "using the overleaf plan for features" - UserFeaturesUpdater.updateFeatures user_id, overleafPlanCode, callback + # V1 Subscription + else if v1PlanCode? + logger.log user_id: user_id, "using the V1 plan for features" + UserFeaturesUpdater.updateFeatures user_id, v1PlanCode, callback # Default else logger.log user_id:user_id, "using default features for user with no subscription or group"