From 0830c473ad94da146a1549db656d1424ed224c86 Mon Sep 17 00:00:00 2001 From: James Allen Date: Thu, 17 May 2018 16:58:58 +0100 Subject: [PATCH] Update unit tests and refactor to make more testable --- .../Features/Referal/ReferalAllocator.coffee | 46 +--- .../Features/Referal/ReferalFeatures.coffee | 44 ++++ .../SubscriptionController.coffee | 4 +- .../Subscription/SubscriptionRouter.coffee | 2 +- .../Subscription/SubscriptionUpdater.coffee | 48 ++-- .../Subscription/UserFeaturesUpdater.coffee | 2 - .../coffee/SubscriptionTests.coffee | 2 +- .../Referal/ReferalAllocatorTests.coffee | 125 +--------- .../Referal/ReferalFeaturesTests.coffee | 65 +++++ .../SubscriptionControllerTests.coffee | 1 + .../SubscriptionUpdaterTests.coffee | 236 ++++++++++++------ .../UserFeaturesUpdaterTests.coffee | 17 +- .../V1SusbcriptionManagerTests.coffee | 128 ++++++++++ 13 files changed, 438 insertions(+), 282 deletions(-) create mode 100644 services/web/app/coffee/Features/Referal/ReferalFeatures.coffee create mode 100644 services/web/test/unit/coffee/Referal/ReferalFeaturesTests.coffee create mode 100644 services/web/test/unit/coffee/Subscription/V1SusbcriptionManagerTests.coffee diff --git a/services/web/app/coffee/Features/Referal/ReferalAllocator.coffee b/services/web/app/coffee/Features/Referal/ReferalAllocator.coffee index aef283d68d..3344747968 100644 --- a/services/web/app/coffee/Features/Referal/ReferalAllocator.coffee +++ b/services/web/app/coffee/Features/Referal/ReferalAllocator.coffee @@ -1,8 +1,8 @@ _ = require("underscore") logger = require('logger-sharelatex') User = require('../../models/User').User -SubscriptionLocator = require "../Subscription/SubscriptionLocator" Settings = require "settings-sharelatex" +SubscriptionUpdater = require "../Subscription/SubscriptionUpdater" module.exports = ReferalAllocator = allocate: (referal_id, new_user_id, referal_source, referal_medium, callback = ->)-> @@ -25,48 +25,6 @@ module.exports = ReferalAllocator = if err? logger.err err:err, referal_id:referal_id, new_user_id:new_user_id, "something went wrong allocating referal" return callback(err) - ReferalAllocator.assignBonus user._id, callback + SubscriptionUpdater.refreshFeatures user._id, callback else callback() - - - - getBonusFeatures: (user_id, callback = (error) ->) -> - query = _id: user_id - User.findOne query, (error, user) -> - return callback(error) if error - return callback(new Error("user not found #{user_id} for assignBonus")) if !user? - logger.log user_id: user_id, refered_user_count: user.refered_user_count, "assigning bonus" - if user.refered_user_count? and user.refered_user_count > 0 - newFeatures = ReferalAllocator._calculateFeatures(user) - callback null, newFeatures - else - callback() - - _calculateFeatures : (user)-> - bonusLevel = ReferalAllocator._getBonusLevel(user) - currentFeatures = _.clone(user.features) #need to clone because we exend with underscore later - betterBonusFeatures = {} - _.each Settings.bonus_features["#{bonusLevel}"], (bonusLevel, key)-> - currentLevel = user?.features?[key] - if _.isBoolean(currentLevel) and currentLevel == false - betterBonusFeatures[key] = bonusLevel - - if _.isNumber(currentLevel) - if currentLevel == -1 - return - bonusIsGreaterThanCurrent = currentLevel < bonusLevel - if bonusIsGreaterThanCurrent or bonusLevel == -1 - betterBonusFeatures[key] = bonusLevel - newFeatures = _.extend(currentFeatures, betterBonusFeatures) - return newFeatures - - - _getBonusLevel: (user)-> - highestBonusLevel = 0 - _.each _.keys(Settings.bonus_features), (level)-> - levelIsLessThanUser = level <= user.refered_user_count - levelIsMoreThanCurrentHighest = level >= highestBonusLevel - if levelIsLessThanUser and levelIsMoreThanCurrentHighest - highestBonusLevel = level - return highestBonusLevel diff --git a/services/web/app/coffee/Features/Referal/ReferalFeatures.coffee b/services/web/app/coffee/Features/Referal/ReferalFeatures.coffee new file mode 100644 index 0000000000..34651ef1f5 --- /dev/null +++ b/services/web/app/coffee/Features/Referal/ReferalFeatures.coffee @@ -0,0 +1,44 @@ +_ = require("underscore") +logger = require('logger-sharelatex') +User = require('../../models/User').User +Settings = require "settings-sharelatex" + +module.exports = ReferalFeatures = + getBonusFeatures: (user_id, callback = (error) ->) -> + query = _id: user_id + User.findOne query, (error, user) -> + return callback(error) if error + return callback(new Error("user not found #{user_id} for assignBonus")) if !user? + logger.log user_id: user_id, refered_user_count: user.refered_user_count, "assigning bonus" + if user.refered_user_count? and user.refered_user_count > 0 + newFeatures = ReferalFeatures._calculateFeatures(user) + callback null, newFeatures + else + callback null, {} + + _calculateFeatures : (user)-> + bonusLevel = ReferalFeatures._getBonusLevel(user) + currentFeatures = _.clone(user.features) #need to clone because we exend with underscore later + betterBonusFeatures = {} + _.each Settings.bonus_features["#{bonusLevel}"], (bonusLevel, key)-> + currentLevel = user?.features?[key] + if _.isBoolean(currentLevel) and currentLevel == false + betterBonusFeatures[key] = bonusLevel + + if _.isNumber(currentLevel) + if currentLevel == -1 + return + bonusIsGreaterThanCurrent = currentLevel < bonusLevel + if bonusIsGreaterThanCurrent or bonusLevel == -1 + betterBonusFeatures[key] = bonusLevel + newFeatures = _.extend(currentFeatures, betterBonusFeatures) + return newFeatures + + _getBonusLevel: (user)-> + highestBonusLevel = 0 + _.each _.keys(Settings.bonus_features), (level)-> + levelIsLessThanUser = level <= user.refered_user_count + levelIsMoreThanCurrentHighest = level >= highestBonusLevel + if levelIsLessThanUser and levelIsMoreThanCurrentHighest + highestBonusLevel = level + return highestBonusLevel diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionController.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionController.coffee index 1b7078d9cb..f127f7ed35 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionController.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionController.coffee @@ -239,8 +239,8 @@ module.exports = SubscriptionController = req.body = body next() - refreshUserSubscription: (req, res, next) -> + refreshUserFeatures: (req, res, next) -> {user_id} = req.params - SubscriptionUpdater.refreshSubscription user_id, (error) -> + SubscriptionUpdater.refreshFeatures user_id, (error) -> return next(error) if error? res.sendStatus 200 \ 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 692f6c92c8..8c5f631e12 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionRouter.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionRouter.coffee @@ -47,5 +47,5 @@ module.exports = webRouter.post "/user/subscription/upgrade-annual", AuthenticationController.requireLogin(), SubscriptionController.processUpgradeToAnnualPlan # Currently used in acceptance tests only, as a way to trigger the syncing logic - publicApiRouter.post "/user/:user_id/subscription/sync", AuthenticationController.httpAuth, SubscriptionController.refreshUserSubscription + publicApiRouter.post "/user/:user_id/features/sync", AuthenticationController.httpAuth, SubscriptionController.refreshUserFeatures diff --git a/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee b/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee index c76bffd453..8e583bd07b 100644 --- a/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee +++ b/services/web/app/coffee/Features/Subscription/SubscriptionUpdater.coffee @@ -7,7 +7,7 @@ PlansLocator = require("./PlansLocator") Settings = require("settings-sharelatex") logger = require("logger-sharelatex") ObjectId = require('mongoose').Types.ObjectId -ReferalAllocator = require("../Referal/ReferalAllocator") +ReferalFeatures = require("../Referal/ReferalFeatures") V1SubscriptionManager = require("./V1SubscriptionManager") oneMonthInSeconds = 60 * 60 * 24 * 30 @@ -54,7 +54,7 @@ module.exports = SubscriptionUpdater = if err? logger.err err:err, searchOps:searchOps, removeOperation:removeOperation, "error removing user from group" return callback(err) - SubscriptionUpdater._setUsersMinimumFeatures user_id, callback + SubscriptionUpdater.refreshFeatures user_id, callback removeEmailInviteFromGroup: (adminUser_id, email, callback)-> Subscription.update { @@ -63,9 +63,6 @@ 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? @@ -73,7 +70,7 @@ module.exports = SubscriptionUpdater = logger.log {subscription_id, affected_user_ids}, "deleting subscription and downgrading users" Subscription.remove {_id: ObjectId(subscription_id)}, (err) -> return callback(err) if err? - async.mapSeries affected_user_ids, SubscriptionUpdater._setUsersMinimumFeatures, callback + async.mapSeries affected_user_ids, SubscriptionUpdater.refreshFeatures, callback _createNewSubscription: (adminUser_id, callback)-> logger.log adminUser_id:adminUser_id, "creating new subscription" @@ -101,36 +98,41 @@ module.exports = SubscriptionUpdater = allIds = _.union subscription.member_ids, [subscription.admin_id] jobs = allIds.map (user_id)-> return (cb)-> - SubscriptionUpdater._setUsersMinimumFeatures user_id, cb + SubscriptionUpdater.refreshFeatures user_id, cb async.series jobs, callback - _setUsersMinimumFeatures: (user_id, callback)-> + refreshFeatures: (user_id, callback)-> jobs = - individualFeatures: (cb)-> - SubscriptionLocator.getUsersSubscription user_id, (err, sub)-> - cb err, SubscriptionUpdater._subscriptionToFeatures(sub) - groupFeatures: (cb) -> - SubscriptionLocator.getGroupSubscriptionsMemberOf user_id, (err, subs) -> - cb err, (subs or []).map SubscriptionUpdater._subscriptionToFeatures - v1Features: (cb) -> - V1SubscriptionManager.getPlanCodeFromV1 user_id, (err, planCode) -> - cb err, SubscriptionUpdater._planCodeToFeatures(planCode) - bonusFeatures: (cb) -> - ReferalAllocator.getBonusFeatures user_id, cb + individualFeatures: (cb) -> SubscriptionUpdater._getIndividualFeatures user_id, cb + groupFeatureSets: (cb) -> SubscriptionUpdater._getGroupFeatureSets user_id, cb + v1Features: (cb) -> SubscriptionUpdater._getV1Features user_id, cb + bonusFeatures: (cb) -> ReferalFeatures.getBonusFeatures user_id, cb async.series jobs, (err, results)-> if err? logger.err err:err, user_id:user_id, - "error getting subscription or group for _setUsersMinimumFeatures" + "error getting subscription or group for refreshFeatures" return callback(err) - {individualFeatures, groupFeatures, v1Features, bonusFeatures} = results - logger.log {user_id, individualFeatures, groupFeatures, v1Features, bonusFeatures}, 'merging user features' - featureSets = groupFeatures.concat [individualFeatures, v1Features, bonusFeatures] + {individualFeatures, groupFeatureSets, v1Features, bonusFeatures} = results + logger.log {user_id, individualFeatures, groupFeatureSets, v1Features, bonusFeatures}, 'merging user features' + featureSets = groupFeatureSets.concat [individualFeatures, v1Features, bonusFeatures] features = _.reduce(featureSets, SubscriptionUpdater._mergeFeatures, Settings.defaultFeatures) logger.log {user_id, features}, 'updating user features' UserFeaturesUpdater.updateFeatures user_id, features, callback + _getIndividualFeatures: (user_id, callback = (error, features = {}) ->) -> + SubscriptionLocator.getUsersSubscription user_id, (err, sub)-> + callback err, SubscriptionUpdater._subscriptionToFeatures(sub) + + _getGroupFeatureSets: (user_id, callback = (error, featureSets = []) ->) -> + SubscriptionLocator.getGroupSubscriptionsMemberOf user_id, (err, subs) -> + callback err, (subs or []).map SubscriptionUpdater._subscriptionToFeatures + + _getV1Features: (user_id, callback = (error, features = {}) ->) -> + V1SubscriptionManager.getPlanCodeFromV1 user_id, (err, planCode) -> + callback err, SubscriptionUpdater._planCodeToFeatures(planCode) + _mergeFeatures: (featuresA, featuresB) -> features = Object.assign({}, featuresA) for key, value of featuresB diff --git a/services/web/app/coffee/Features/Subscription/UserFeaturesUpdater.coffee b/services/web/app/coffee/Features/Subscription/UserFeaturesUpdater.coffee index 520139e9be..28a49b2126 100644 --- a/services/web/app/coffee/Features/Subscription/UserFeaturesUpdater.coffee +++ b/services/web/app/coffee/Features/Subscription/UserFeaturesUpdater.coffee @@ -1,9 +1,7 @@ logger = require("logger-sharelatex") User = require('../../models/User').User -PlansLocator = require("./PlansLocator") module.exports = - updateFeatures: (user_id, features, callback = (err, features)->)-> conditions = _id:user_id update = {} diff --git a/services/web/test/acceptance/coffee/SubscriptionTests.coffee b/services/web/test/acceptance/coffee/SubscriptionTests.coffee index 55f3a0efbc..ce762ea0ee 100644 --- a/services/web/test/acceptance/coffee/SubscriptionTests.coffee +++ b/services/web/test/acceptance/coffee/SubscriptionTests.coffee @@ -12,7 +12,7 @@ MockV1Api = require "./helpers/MockV1Api" syncUserAndGetFeatures = (user, callback = (error, features) ->) -> request { method: 'POST', - url: "/user/#{user._id}/subscription/sync", + url: "/user/#{user._id}/features/sync", auth: user: 'sharelatex' pass: 'password' diff --git a/services/web/test/unit/coffee/Referal/ReferalAllocatorTests.coffee b/services/web/test/unit/coffee/Referal/ReferalAllocatorTests.coffee index 3fe925f91c..ef0f0243c8 100644 --- a/services/web/test/unit/coffee/Referal/ReferalAllocatorTests.coffee +++ b/services/web/test/unit/coffee/Referal/ReferalAllocatorTests.coffee @@ -4,12 +4,12 @@ require('chai').should() sinon = require('sinon') modulePath = require('path').join __dirname, '../../../../app/js/Features/Referal/ReferalAllocator.js' -describe 'Referalallocator', -> +describe 'ReferalAllocator', -> beforeEach -> @ReferalAllocator = SandboxedModule.require modulePath, requires: '../../models/User': User: @User = {} - "../Subscription/SubscriptionLocator": @SubscriptionLocator = {} + "../Subscription/SubscriptionUpdater": @SubscriptionUpdater = {} "settings-sharelatex": @Settings = {} 'logger-sharelatex': log:-> @@ -26,7 +26,7 @@ describe 'Referalallocator', -> @referal_source = "bonus" @User.update = sinon.stub().callsArgWith 3, null @User.findOne = sinon.stub().callsArgWith 1, null, { _id: @user_id } - @ReferalAllocator.assignBonus = sinon.stub().callsArg 1 + @SubscriptionUpdater.refreshFeatures = sinon.stub().yields() @ReferalAllocator.allocate @referal_id, @new_user_id, @referal_source, @referal_medium, @callback it 'should update the referring user with the refered users id', -> @@ -44,8 +44,8 @@ describe 'Referalallocator', -> .calledWith( referal_id: @referal_id ) .should.equal true - it "shoudl assign the user their bonus", -> - @ReferalAllocator.assignBonus + it "should refresh the user's subscription", -> + @SubscriptionUpdater.refreshFeatures .calledWith(@user_id) .should.equal true @@ -57,7 +57,7 @@ describe 'Referalallocator', -> @referal_source = "public_share" @User.update = sinon.stub().callsArgWith 3, null @User.findOne = sinon.stub().callsArgWith 1, null, { _id: @user_id } - @ReferalAllocator.assignBonus = sinon.stub().callsArg 1 + @SubscriptionUpdater.refreshFeatures = sinon.stub().yields() @ReferalAllocator.allocate @referal_id, @new_user_id, @referal_source, @referal_medium, @callback it 'should not update the referring user with the refered users id', -> @@ -69,118 +69,7 @@ describe 'Referalallocator', -> .should.equal true it "should not assign the user a bonus", -> - @ReferalAllocator.assignBonus.called.should.equal false + @SubscriptionUpdater.refreshFeatures.called.should.equal false it "should call the callback", -> @callback.called.should.equal true - - describe "assignBonus", -> - beforeEach -> - @refered_user_count = 3 - @Settings.bonus_features = - "3": - collaborators: 3 - dropbox: false - versioning: false - stubbedUser = { - refered_user_count: @refered_user_count, - features:{collaborators:1, dropbox:false, versioning:false} - } - - @User.findOne = sinon.stub().callsArgWith 1, null, stubbedUser - @User.update = sinon.stub().callsArgWith 2, null - @ReferalAllocator.assignBonus @user_id, @callback - - it "should get the users number of refered user", -> - @User.findOne - .calledWith(_id: @user_id) - .should.equal true - - it "should update the user to bonus features", -> - @User.update - .calledWith({ - _id: @user_id - }, { - $set: - features: - @Settings.bonus_features[@refered_user_count.toString()] - }) - .should.equal true - - it "should call the callback", -> - @callback.called.should.equal true - - describe "when there is nothing to assign", -> - - beforeEach -> - @ReferalAllocator._calculateBonuses = sinon.stub().returns({}) - @stubbedUser = - refered_user_count:4 - features:{collaborators:3, versioning:true, dropbox:false} - @Settings.bonus_features = - "4": - collaborators:3 - versioning:true - dropbox:false - @User.findOne = sinon.stub().callsArgWith 1, null, @stubbedUser - @User.update = sinon.stub().callsArgWith 2, null - - it "should not call update if there are no bonuses to apply", (done)-> - @ReferalAllocator.assignBonus @user_id, (err)=> - @User.update.called.should.equal false - done() - - describe "when the user has better features already", -> - - beforeEach -> - @refered_user_count = 3 - @stubbedUser = - refered_user_count:4 - features: - collaborators:3 - dropbox:false - versioning:false - @Settings.bonus_features = - "4": - collaborators: 10 - dropbox: true - versioning: false - - @User.findOne = sinon.stub().callsArgWith 1, null, @stubbedUser - @User.update = sinon.stub().callsArgWith 2, null - - it "should not set in in mongo when the feature is better", (done)-> - @ReferalAllocator.assignBonus @user_id, => - @User.update.calledWith({_id: @user_id }, {$set: features:{dropbox:true, versioning:false, collaborators:10} }).should.equal true - done() - - it "should not overright if the user has -1 users", (done)-> - @stubbedUser.features.collaborators = -1 - @ReferalAllocator.assignBonus @user_id, => - @User.update.calledWith({_id: @user_id }, {$set: features:{dropbox:true, versioning:false, collaborators:-1} }).should.equal true - done() - - describe "when the user is not at a bonus level", -> - beforeEach -> - @refered_user_count = 0 - @Settings.bonus_features = - "1": - collaborators: 3 - dropbox: false - versioning: false - @User.findOne = sinon.stub().callsArgWith 1, null, { refered_user_count: @refered_user_count } - @User.update = sinon.stub().callsArgWith 2, null - @ReferalAllocator.assignBonus @user_id, @callback - - it "should get the users number of refered user", -> - @User.findOne - .calledWith(_id: @user_id) - .should.equal true - - it "should not update the user to bonus features", -> - @User.update.called.should.equal false - - it "should call the callback", -> - @callback.called.should.equal true - - diff --git a/services/web/test/unit/coffee/Referal/ReferalFeaturesTests.coffee b/services/web/test/unit/coffee/Referal/ReferalFeaturesTests.coffee new file mode 100644 index 0000000000..dfa70f8ebe --- /dev/null +++ b/services/web/test/unit/coffee/Referal/ReferalFeaturesTests.coffee @@ -0,0 +1,65 @@ +SandboxedModule = require('sandboxed-module') +assert = require('assert') +require('chai').should() +sinon = require('sinon') +modulePath = require('path').join __dirname, '../../../../app/js/Features/Referal/ReferalFeatures.js' + +describe 'ReferalFeatures', -> + + beforeEach -> + @ReferalFeatures = SandboxedModule.require modulePath, requires: + '../../models/User': User: @User = {} + "settings-sharelatex": @Settings = {} + 'logger-sharelatex': + log:-> + err:-> + @callback = sinon.stub() + @referal_id = "referal-id-123" + @referal_medium = "twitter" + @user_id = "user-id-123" + @new_user_id = "new-user-id-123" + + describe "getBonusFeatures", -> + beforeEach -> + @refered_user_count = 3 + @Settings.bonus_features = + "3": + collaborators: 3 + dropbox: false + versioning: false + stubbedUser = { + refered_user_count: @refered_user_count, + features:{collaborators:1, dropbox:false, versioning:false} + } + + @User.findOne = sinon.stub().callsArgWith 1, null, stubbedUser + @ReferalFeatures.getBonusFeatures @user_id, @callback + + it "should get the users number of refered user", -> + @User.findOne + .calledWith(_id: @user_id) + .should.equal true + + it "should call the callback with the features", -> + @callback.calledWith(null, @Settings.bonus_features[3]).should.equal true + + describe "when the user is not at a bonus level", -> + beforeEach -> + @refered_user_count = 0 + @Settings.bonus_features = + "1": + collaborators: 3 + dropbox: false + versioning: false + @User.findOne = sinon.stub().callsArgWith 1, null, { refered_user_count: @refered_user_count } + @ReferalFeatures.getBonusFeatures @user_id, @callback + + it "should get the users number of refered user", -> + @User.findOne + .calledWith(_id: @user_id) + .should.equal true + + it "should call the callback with no features", -> + @callback.calledWith(null, {}).should.equal true + + diff --git a/services/web/test/unit/coffee/Subscription/SubscriptionControllerTests.coffee b/services/web/test/unit/coffee/Subscription/SubscriptionControllerTests.coffee index 22f571ffa8..47301ecf7c 100644 --- a/services/web/test/unit/coffee/Subscription/SubscriptionControllerTests.coffee +++ b/services/web/test/unit/coffee/Subscription/SubscriptionControllerTests.coffee @@ -75,6 +75,7 @@ describe "SubscriptionController", -> "./SubscriptionDomainHandler":@SubscriptionDomainHandler "../User/UserGetter": @UserGetter "./RecurlyWrapper": @RecurlyWrapper = {} + "./SubscriptionUpdater": @SubscriptionUpdater = {} @res = new MockResponse() diff --git a/services/web/test/unit/coffee/Subscription/SubscriptionUpdaterTests.coffee b/services/web/test/unit/coffee/Subscription/SubscriptionUpdaterTests.coffee index 93479a03a1..71ae6346ce 100644 --- a/services/web/test/unit/coffee/Subscription/SubscriptionUpdaterTests.coffee +++ b/services/web/test/unit/coffee/Subscription/SubscriptionUpdaterTests.coffee @@ -1,5 +1,6 @@ SandboxedModule = require('sandboxed-module') should = require('chai').should() +expect = require('chai').expect sinon = require 'sinon' modulePath = "../../../../app/js/Features/Subscription/SubscriptionUpdater" assert = require("chai").assert @@ -22,6 +23,7 @@ describe "SubscriptionUpdater", -> save: sinon.stub().callsArgWith(0) freeTrial:{} planCode:"student_or_something" + @user_id = @adminuser_id @groupSubscription = admin_id: @adminUser._id @@ -48,15 +50,15 @@ describe "SubscriptionUpdater", -> @Settings = freeTrialPlanCode: "collaborator" defaultPlanCode: "personal" + defaultFeatures: { "default": "features" } @UserFeaturesUpdater = - updateFeatures : sinon.stub().callsArgWith(2) + updateFeatures : sinon.stub().yields() @PlansLocator = findLocalPlanInSettings: sinon.stub().returns({}) - @ReferalAllocator = assignBonus:sinon.stub().callsArgWith(1) - @ReferalAllocator.cock = true + @ReferalFeatures = getBonusFeatures: sinon.stub().callsArgWith(1) @Modules = {hooks: {fire: sinon.stub().callsArgWith(2, null, null)}} @SubscriptionUpdater = SandboxedModule.require modulePath, requires: '../../models/Subscription': Subscription:@SubscriptionModel @@ -65,8 +67,9 @@ describe "SubscriptionUpdater", -> './PlansLocator': @PlansLocator "logger-sharelatex": log:-> 'settings-sharelatex': @Settings - "../Referal/ReferalAllocator" : @ReferalAllocator + "../Referal/ReferalFeatures" : @ReferalFeatures '../../infrastructure/Modules': @Modules + "./V1SubscriptionManager": @V1SubscriptionManager = {} describe "syncSubscription", -> @@ -97,7 +100,7 @@ describe "SubscriptionUpdater", -> describe "_updateSubscriptionFromRecurly", -> beforeEach -> - @SubscriptionUpdater._setUsersMinimumFeatures = sinon.stub().callsArgWith(1) + @SubscriptionUpdater.refreshFeatures = sinon.stub().callsArgWith(1) it "should update the subscription with token etc when not expired", (done)-> @SubscriptionUpdater._updateSubscriptionFromRecurly @recurlySubscription, @subscription, (err)=> @@ -108,7 +111,7 @@ describe "SubscriptionUpdater", -> assert.equal(@subscription.freeTrial.expiresAt, undefined) assert.equal(@subscription.freeTrial.planCode, undefined) @subscription.save.called.should.equal true - @SubscriptionUpdater._setUsersMinimumFeatures.calledWith(@adminUser._id).should.equal true + @SubscriptionUpdater.refreshFeatures.calledWith(@adminUser._id).should.equal true done() it "should remove the recurlySubscription_id when expired", (done)-> @@ -117,15 +120,15 @@ describe "SubscriptionUpdater", -> @SubscriptionUpdater._updateSubscriptionFromRecurly @recurlySubscription, @subscription, (err)=> assert.equal(@subscription.recurlySubscription_id, undefined) @subscription.save.called.should.equal true - @SubscriptionUpdater._setUsersMinimumFeatures.calledWith(@adminUser._id).should.equal true + @SubscriptionUpdater.refreshFeatures.calledWith(@adminUser._id).should.equal true done() it "should update all the users features", (done)-> @SubscriptionUpdater._updateSubscriptionFromRecurly @recurlySubscription, @subscription, (err)=> - @SubscriptionUpdater._setUsersMinimumFeatures.calledWith(@adminUser._id).should.equal true - @SubscriptionUpdater._setUsersMinimumFeatures.calledWith(@allUserIds[0]).should.equal true - @SubscriptionUpdater._setUsersMinimumFeatures.calledWith(@allUserIds[1]).should.equal true - @SubscriptionUpdater._setUsersMinimumFeatures.calledWith(@allUserIds[2]).should.equal true + @SubscriptionUpdater.refreshFeatures.calledWith(@adminUser._id).should.equal true + @SubscriptionUpdater.refreshFeatures.calledWith(@allUserIds[0]).should.equal true + @SubscriptionUpdater.refreshFeatures.calledWith(@allUserIds[1]).should.equal true + @SubscriptionUpdater.refreshFeatures.calledWith(@allUserIds[2]).should.equal true done() it "should set group to true and save how many members can be added to group", (done)-> @@ -168,7 +171,7 @@ describe "SubscriptionUpdater", -> describe "removeUserFromGroup", -> beforeEach -> - @SubscriptionUpdater._setUsersMinimumFeatures = sinon.stub().callsArgWith(1) + @SubscriptionUpdater.refreshFeatures = sinon.stub().callsArgWith(1) it "should pull the users id from the group", (done)-> @SubscriptionUpdater.removeUserFromGroup @adminUser._id, @otherUserId, => @@ -181,70 +184,159 @@ describe "SubscriptionUpdater", -> it "should update the users features", (done)-> @SubscriptionUpdater.removeUserFromGroup @adminUser._id, @otherUserId, => - @SubscriptionUpdater._setUsersMinimumFeatures.calledWith(@otherUserId).should.equal true + @SubscriptionUpdater.refreshFeatures.calledWith(@otherUserId).should.equal true done() - describe "_setUsersMinimumFeatures", -> + describe "refreshFeatures", -> + beforeEach -> + @SubscriptionUpdater._getIndividualFeatures = sinon.stub().yields(null, { 'individual': 'features' }) + @SubscriptionUpdater._getGroupFeatureSets = sinon.stub().yields(null, [{ 'group': 'features' }, { 'group': 'features2' }]) + @SubscriptionUpdater._getV1Features = sinon.stub().yields(null, { 'v1': 'features' }) + @ReferalFeatures.getBonusFeatures = sinon.stub().yields(null, { 'bonus': 'features' }) + @SubscriptionUpdater._mergeFeatures = sinon.stub().returns({'merged': 'features'}) + @callback = sinon.stub() + @SubscriptionUpdater.refreshFeatures @user_id, @callback - it "should call updateFeatures with the subscription if set", (done)-> - @SubscriptionLocator.getUsersSubscription.callsArgWith(1, null, @subscription) - @SubscriptionLocator.getGroupSubscriptionMemberOf.callsArgWith(1, null) + it "should get the individual features", -> + @SubscriptionUpdater._getIndividualFeatures + .calledWith(@user_id) + .should.equal true - @SubscriptionUpdater._setUsersMinimumFeatures @adminUser._id, (err)=> - args = @UserFeaturesUpdater.updateFeatures.args[0] - assert.equal args[0], @adminUser._id - assert.equal args[1], @subscription.planCode - done() + it "should get the group features", -> + @SubscriptionUpdater._getGroupFeatureSets + .calledWith(@user_id) + .should.equal true - it "should call updateFeatures with the group subscription if set", (done)-> - @SubscriptionLocator.getUsersSubscription.callsArgWith(1, null) - @SubscriptionLocator.getGroupSubscriptionMemberOf.callsArgWith(1, null, @groupSubscription) + it "should get the v1 features", -> + @SubscriptionUpdater._getV1Features + .calledWith(@user_id) + .should.equal true - @SubscriptionUpdater._setUsersMinimumFeatures @adminUser._id, (err)=> - args = @UserFeaturesUpdater.updateFeatures.args[0] - assert.equal args[0], @adminUser._id - assert.equal args[1], @groupSubscription.planCode - done() + it "should get the bonus features", -> + @ReferalFeatures.getBonusFeatures + .calledWith(@user_id) + .should.equal true - 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']) + it "should merge from the default features", -> + @SubscriptionUpdater._mergeFeatures.calledWith(@Settings.defaultFeatures).should.equal true - @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 merge the individual features", -> + @SubscriptionUpdater._mergeFeatures.calledWith(sinon.match.any, { 'individual': 'features' }).should.equal true - 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 - assert.equal args[1], @groupSubscription.planCode - done() + it "should merge the group features", -> + @SubscriptionUpdater._mergeFeatures.calledWith(sinon.match.any, { 'group': 'features' }).should.equal true + @SubscriptionUpdater._mergeFeatures.calledWith(sinon.match.any, { 'group': 'features2' }).should.equal true + it "should merge the v1 features", -> + @SubscriptionUpdater._mergeFeatures.calledWith(sinon.match.any, { 'v1': 'features' }).should.equal true - 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 - assert.equal args[1], @Settings.defaultPlanCode - done() + it "should merge the bonus features", -> + @SubscriptionUpdater._mergeFeatures.calledWith(sinon.match.any, { 'bonus': 'features' }).should.equal true - it "should call assignBonus", (done)-> - @SubscriptionLocator.getUsersSubscription.callsArgWith(1, null) - @SubscriptionLocator.getGroupSubscriptionMemberOf.callsArgWith(1, null) - @SubscriptionUpdater._setUsersMinimumFeatures @adminuser_id, (err)=> - @ReferalAllocator.assignBonus.calledWith(@adminuser_id).should.equal true - done() + it "should update the user with the merged features", -> + @UserFeaturesUpdater.updateFeatures + .calledWith(@user_id, {'merged': 'features'}) + .should.equal true + + describe "_mergeFeatures", -> + it "should prefer priority over standard for compileGroup", -> + expect(@SubscriptionUpdater._mergeFeatures({ + compileGroup: 'priority' + }, { + compileGroup: 'standard' + })).to.deep.equal({ + compileGroup: 'priority' + }) + expect(@SubscriptionUpdater._mergeFeatures({ + compileGroup: 'standard' + }, { + compileGroup: 'priority' + })).to.deep.equal({ + compileGroup: 'priority' + }) + expect(@SubscriptionUpdater._mergeFeatures({ + compileGroup: 'priority' + }, { + compileGroup: 'priority' + })).to.deep.equal({ + compileGroup: 'priority' + }) + expect(@SubscriptionUpdater._mergeFeatures({ + compileGroup: 'standard' + }, { + compileGroup: 'standard' + })).to.deep.equal({ + compileGroup: 'standard' + }) + + it "should prefer -1 over any other for collaborators", -> + expect(@SubscriptionUpdater._mergeFeatures({ + collaborators: -1 + }, { + collaborators: 10 + })).to.deep.equal({ + collaborators: -1 + }) + expect(@SubscriptionUpdater._mergeFeatures({ + collaborators: 10 + }, { + collaborators: -1 + })).to.deep.equal({ + collaborators: -1 + }) + expect(@SubscriptionUpdater._mergeFeatures({ + collaborators: 4 + }, { + collaborators: 10 + })).to.deep.equal({ + collaborators: 10 + }) + + it "should prefer the higher of compileTimeout", -> + expect(@SubscriptionUpdater._mergeFeatures({ + compileTimeout: 20 + }, { + compileTimeout: 10 + })).to.deep.equal({ + compileTimeout: 20 + }) + expect(@SubscriptionUpdater._mergeFeatures({ + compileTimeout: 10 + }, { + compileTimeout: 20 + })).to.deep.equal({ + compileTimeout: 20 + }) + + it "should prefer the true over false for other keys", -> + expect(@SubscriptionUpdater._mergeFeatures({ + github: true + }, { + github: false + })).to.deep.equal({ + github: true + }) + expect(@SubscriptionUpdater._mergeFeatures({ + github: false + }, { + github: true + })).to.deep.equal({ + github: true + }) + expect(@SubscriptionUpdater._mergeFeatures({ + github: true + }, { + github: true + })).to.deep.equal({ + github: true + }) + expect(@SubscriptionUpdater._mergeFeatures({ + github: false + }, { + github: false + })).to.deep.equal({ + github: false + }) describe "deleteSubscription", -> beforeEach (done) -> @@ -255,7 +347,7 @@ describe "SubscriptionUpdater", -> member_ids: [ ObjectId(), ObjectId(), ObjectId() ] } @SubscriptionLocator.getSubscription = sinon.stub().yields(null, @subscription) - @SubscriptionUpdater._setUsersMinimumFeatures = sinon.stub().yields() + @SubscriptionUpdater.refreshFeatures = sinon.stub().yields() @SubscriptionUpdater.deleteSubscription @subscription_id, done it "should look up the subscription", -> @@ -269,22 +361,12 @@ describe "SubscriptionUpdater", -> .should.equal true it "should downgrade the admin_id", -> - @SubscriptionUpdater._setUsersMinimumFeatures + @SubscriptionUpdater.refreshFeatures .calledWith(@subscription.admin_id) .should.equal true it "should downgrade all of the members", -> for user_id in @subscription.member_ids - @SubscriptionUpdater._setUsersMinimumFeatures + @SubscriptionUpdater.refreshFeatures .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 diff --git a/services/web/test/unit/coffee/Subscription/UserFeaturesUpdaterTests.coffee b/services/web/test/unit/coffee/Subscription/UserFeaturesUpdaterTests.coffee index d388d67c3b..1ba4e0b32f 100644 --- a/services/web/test/unit/coffee/Subscription/UserFeaturesUpdaterTests.coffee +++ b/services/web/test/unit/coffee/Subscription/UserFeaturesUpdaterTests.coffee @@ -4,31 +4,20 @@ sinon = require 'sinon' modulePath = "../../../../app/js/Features/Subscription/UserFeaturesUpdater" assert = require("chai").assert - describe "UserFeaturesUpdater", -> - beforeEach -> - @User = update: sinon.stub().callsArgWith(2) - - @PlansLocator = - findLocalPlanInSettings : sinon.stub() - @UserFeaturesUpdater = SandboxedModule.require modulePath, requires: '../../models/User': User:@User "logger-sharelatex": log:-> - './PlansLocator': @PlansLocator describe "updateFeatures", -> - it "should send the users features", (done)-> user_id = "5208dd34438842e2db000005" - plan_code = "student" - @features = features:{versioning:true, collaborators:10} - @PlansLocator.findLocalPlanInSettings = sinon.stub().returns(@features) - @UserFeaturesUpdater.updateFeatures user_id, plan_code, (err, features)=> + @features = {versioning:true, collaborators:10} + @UserFeaturesUpdater.updateFeatures user_id, @features, (err, features)=> update = {"features.versioning":true, "features.collaborators":10} @User.update.calledWith({"_id":user_id}, update).should.equal true - features.should.deep.equal @features.features + features.should.deep.equal @features done() \ No newline at end of file diff --git a/services/web/test/unit/coffee/Subscription/V1SusbcriptionManagerTests.coffee b/services/web/test/unit/coffee/Subscription/V1SusbcriptionManagerTests.coffee new file mode 100644 index 0000000000..ae6237e627 --- /dev/null +++ b/services/web/test/unit/coffee/Subscription/V1SusbcriptionManagerTests.coffee @@ -0,0 +1,128 @@ +should = require('chai').should() +SandboxedModule = require('sandboxed-module') +assert = require('assert') +path = require('path') +modulePath = path.join __dirname, '../../../../app/js/Features/Subscription/V1SubscriptionManager' +sinon = require("sinon") +expect = require("chai").expect + + +describe 'V1SubscriptionManager', -> + beforeEach -> + @V1SubscriptionManager = SandboxedModule.require modulePath, requires: + "../User/UserGetter": @UserGetter = {} + "logger-sharelatex": + log: sinon.stub() + err: sinon.stub() + warn: sinon.stub() + "settings-sharelatex": + overleaf: + host: @host = "http://overleaf.example.com" + "request": @request = sinon.stub() + @V1SubscriptionManager._v1PlanRequest = sinon.stub() + @userId = 'abcd' + @v1UserId = 42 + @user = + _id: @userId + email: 'user@example.com' + overleaf: + id: @v1UserId + + describe 'getPlanCodeFromV1', -> + beforeEach -> + @responseBody = + id: 32, + plan_name: 'pro' + @UserGetter.getUser = sinon.stub() + .yields(null, @user) + @V1SubscriptionManager._v1PlanRequest = sinon.stub() + .yields(null, @responseBody) + @call = (cb) => + @V1SubscriptionManager.getPlanCodeFromV1 @userId, cb + + describe 'when all goes well', -> + + it 'should call getUser', (done) -> + @call (err, planCode) => + expect( + @UserGetter.getUser.callCount + ).to.equal 1 + expect( + @UserGetter.getUser.calledWith(@userId) + ).to.equal true + done() + + it 'should call _v1PlanRequest', (done) -> + @call (err, planCode) => + expect( + @V1SubscriptionManager._v1PlanRequest.callCount + ).to.equal 1 + expect( + @V1SubscriptionManager._v1PlanRequest.calledWith( + @v1UserId + ) + ).to.equal true + done() + + it 'should produce a plan-code without error', (done) -> + @call (err, planCode) => + expect(err).to.not.exist + expect(planCode).to.equal 'v1_pro' + done() + + describe 'when the plan_name from v1 is null', -> + beforeEach -> + @responseBody.plan_name = null + + it 'should produce a null plan-code without error', (done) -> + @call (err, planCode) => + expect(err).to.not.exist + expect(planCode).to.equal null + done() + + describe 'when getUser produces an error', -> + beforeEach -> + @UserGetter.getUser = sinon.stub() + .yields(new Error('woops')) + + it 'should not call _v1PlanRequest', (done) -> + @call (err, planCode) => + expect( + @V1SubscriptionManager._v1PlanRequest.callCount + ).to.equal 0 + done() + + it 'should produce an error', (done) -> + @call (err, planCode) => + expect(err).to.exist + expect(planCode).to.not.exist + done() + + describe 'when getUser does not find a user', -> + beforeEach -> + @UserGetter.getUser = sinon.stub() + .yields(null, null) + + it 'should not call _v1PlanRequest', (done) -> + @call (err, planCode) => + expect( + @V1SubscriptionManager._v1PlanRequest.callCount + ).to.equal 0 + done() + + it 'should produce a null plan-code, without error', (done) -> + @call (err, planCode) => + expect(err).to.not.exist + expect(planCode).to.not.exist + done() + + describe 'when the request to v1 fails', -> + beforeEach -> + @V1SubscriptionManager._v1PlanRequest = sinon.stub() + .yields(new Error('woops')) + + it 'should produce an error', (done) -> + @call (err, planCode) => + expect(err).to.exist + expect(planCode).to.not.exist + done()