diff --git a/services/web/app/coffee/Features/Subscription/FeaturesUpdater.coffee b/services/web/app/coffee/Features/Subscription/FeaturesUpdater.coffee index 5c176c611f..5627072c93 100644 --- a/services/web/app/coffee/Features/Subscription/FeaturesUpdater.coffee +++ b/services/web/app/coffee/Features/Subscription/FeaturesUpdater.coffee @@ -11,7 +11,16 @@ V1SubscriptionManager = require("./V1SubscriptionManager") oneMonthInSeconds = 60 * 60 * 24 * 30 module.exports = FeaturesUpdater = - refreshFeatures: (user_id, callback)-> + refreshFeatures: (user_id, notifyV1 = true, callback = () ->)-> + if typeof notifyV1 == 'function' + callback = notifyV1 + notifyV1 = true + + if notifyV1 + V1SubscriptionManager.notifyV1OfFeaturesChange user_id, (error) -> + if error? + logger.err {err: error, user_id}, "error notifying v1 about updated features" + jobs = individualFeatures: (cb) -> FeaturesUpdater._getIndividualFeatures user_id, cb groupFeatureSets: (cb) -> FeaturesUpdater._getGroupFeatureSets user_id, cb @@ -80,4 +89,4 @@ module.exports = FeaturesUpdater = if !plan? return {} else - return plan.features \ No newline at end of file + return plan.features diff --git a/services/web/app/coffee/Features/Subscription/V1SubscriptionManager.coffee b/services/web/app/coffee/Features/Subscription/V1SubscriptionManager.coffee index 05dc140be2..e920b94f6c 100644 --- a/services/web/app/coffee/Features/Subscription/V1SubscriptionManager.coffee +++ b/services/web/app/coffee/Features/Subscription/V1SubscriptionManager.coffee @@ -12,39 +12,49 @@ module.exports = V1SubscriptionManager = # - 'v1_free' getPlanCodeFromV1: (userId, callback=(err, planCode)->) -> logger.log {userId}, "[V1SubscriptionManager] fetching v1 plan for user" + V1SubscriptionManager._v1Request userId, { + method: 'GET', + url: (v1Id) -> "/api/v1/sharelatex/users/#{v1Id}/plan_code" + }, (error, body) -> + return callback(error) if error? + planName = body?.plan_name + logger.log {userId, planName, body}, "[V1SubscriptionManager] fetched v1 plan for user" + if planName in ['pro', 'pro_plus', 'student', 'free'] + planName = "v1_#{planName}" + else + # Throw away 'anonymous', etc as being equivalent to null + planName = null + return callback(null, planName) + + notifyV1OfFeaturesChange: (userId, callback = (error) ->) -> + V1SubscriptionManager._v1Request userId, { + method: 'POST', + url: (v1Id) -> "/api/v1/sharelatex/users/#{v1Id}/sync" + }, callback + + _v1Request: (userId, options, callback=(err, body)->) -> + if !settings?.apis?.v1 + return callback null, null UserGetter.getUser userId, {'overleaf.id': 1}, (err, user) -> return callback(err) if err? v1Id = user?.overleaf?.id if !v1Id? logger.log {userId}, "[V1SubscriptionManager] no v1 id found for user" return callback(null, null) - V1SubscriptionManager._v1PlanRequest v1Id, (err, body) -> - return callback(err) if err? - planName = body?.plan_name - logger.log {userId, planName, body}, "[V1SubscriptionManager] fetched v1 plan for user" - if planName in ['pro', 'pro_plus', 'student', 'free'] - planName = "v1_#{planName}" + request { + baseUrl: settings.apis.v1.url + url: options.url(v1Id) + method: options.method + auth: + user: settings.apis.v1.user + pass: settings.apis.v1.pass + sendImmediately: true + json: true, + timeout: 5 * 1000 + }, (error, response, body) -> + return callback(error) if error? + if 200 <= response.statusCode < 300 + return callback null, body else - # Throw away 'anonymous', etc as being equivalent to null - planName = null - return callback(null, planName) + return callback new Error("non-success code from v1: #{response.statusCode}") - _v1PlanRequest: (v1Id, callback=(err, body)->) -> - if !settings?.apis?.v1 - return callback null, null - request { - method: 'GET', - url: settings.apis.v1.url + - "/api/v1/sharelatex/users/#{v1Id}/plan_code" - auth: - user: settings.apis.v1.user - pass: settings.apis.v1.pass - sendImmediately: true - json: true, - timeout: 5 * 1000 - }, (error, response, body) -> - return callback(error) if error? - if 200 <= response.statusCode < 300 - return callback null, body - else - return callback new Error("non-success code from v1: #{response.statusCode}") \ No newline at end of file diff --git a/services/web/test/acceptance/coffee/SubscriptionTests.coffee b/services/web/test/acceptance/coffee/FeatureUpdaterTests.coffee similarity index 85% rename from services/web/test/acceptance/coffee/SubscriptionTests.coffee rename to services/web/test/acceptance/coffee/FeatureUpdaterTests.coffee index ce762ea0ee..5bc3026fff 100644 --- a/services/web/test/acceptance/coffee/SubscriptionTests.coffee +++ b/services/web/test/acceptance/coffee/FeatureUpdaterTests.coffee @@ -6,27 +6,22 @@ settings = require "settings-sharelatex" {ObjectId} = require("../../../app/js/infrastructure/mongojs") Subscription = require("../../../app/js/models/Subscription").Subscription User = require("../../../app/js/models/User").User +FeaturesUpdater = require("../../../app/js/Features/Subscription/FeaturesUpdater") MockV1Api = require "./helpers/MockV1Api" +logger = require "logger-sharelatex" +logger.logger.level("error") syncUserAndGetFeatures = (user, callback = (error, features) ->) -> - request { - method: 'POST', - url: "/user/#{user._id}/features/sync", - auth: - user: 'sharelatex' - pass: 'password' - sendImmediately: true - }, (error, response, body) -> - throw error if error? - expect(response.statusCode).to.equal 200 + FeaturesUpdater.refreshFeatures user._id, false, (error) -> + return callback(error) if error? User.findById user._id, (error, user) -> return callback(error) if error? features = user.toObject().features delete features.$init # mongoose internals return callback null, features -describe "Subscriptions", -> +describe "FeatureUpdater.refreshFeatures", -> beforeEach (done) -> @user = new UserClient() @user.ensureUserExists (error) -> @@ -148,4 +143,22 @@ describe "Subscriptions", -> throw error if error? plan = settings.plans.find (plan) -> plan.planCode == 'professional' expect(features).to.deep.equal(plan.features) - done() \ No newline at end of file + done() + + describe "when the notifyV1Flag is passed", -> + beforeEach -> + User.update { + _id: @user._id + }, { + overleaf: + id: 42 + } # returns a promise + + it "should ping the v1 API end point to sync", (done) -> + FeaturesUpdater.refreshFeatures @user._id, true, (error) => + setTimeout () => + expect( + MockV1Api.syncUserFeatures.calledWith('42') + ).to.equal true + done() + , 500 diff --git a/services/web/test/acceptance/coffee/helpers/MockV1Api.coffee b/services/web/test/acceptance/coffee/helpers/MockV1Api.coffee index 5c2cf47ad9..389ecd1762 100644 --- a/services/web/test/acceptance/coffee/helpers/MockV1Api.coffee +++ b/services/web/test/acceptance/coffee/helpers/MockV1Api.coffee @@ -1,6 +1,7 @@ express = require("express") app = express() bodyParser = require('body-parser') +sinon = require 'sinon' app.use(bodyParser.json()) @@ -23,19 +24,25 @@ module.exports = MockV1Api = clearExportParams: () -> @exportParams = null + syncUserFeatures: sinon.stub() + run: () -> - app.get "/api/v1/sharelatex/users/:ol_user_id/plan_code", (req, res, next) => - user = @users[req.params.ol_user_id] + app.get "/api/v1/sharelatex/users/:v1_user_id/plan_code", (req, res, next) => + user = @users[req.params.v1_user_id] if user res.json user else res.sendStatus 404 + app.post "/api/v1/sharelatex/users/:v1_user_id/sync", (req, res, next) => + @syncUserFeatures(req.params.v1_user_id) + res.sendStatus 200 + app.post "/api/v1/sharelatex/exports", (req, res, next) => - #{project, version, pathname} @exportParams = Object.assign({}, req.body) res.json exportId: @exportId + app.listen 5000, (error) -> throw error if error? .on "error", (error) -> diff --git a/services/web/test/unit/coffee/Subscription/FeaturesUpdaterTests.coffee b/services/web/test/unit/coffee/Subscription/FeaturesUpdaterTests.coffee index c8f77bfd40..489c5676ff 100644 --- a/services/web/test/unit/coffee/Subscription/FeaturesUpdaterTests.coffee +++ b/services/web/test/unit/coffee/Subscription/FeaturesUpdaterTests.coffee @@ -22,6 +22,7 @@ describe "FeaturesUpdater", -> describe "refreshFeatures", -> beforeEach -> + @V1SubscriptionManager.notifyV1OfFeaturesChange = sinon.stub().yields() @UserFeaturesUpdater.updateFeatures = sinon.stub().yields() @FeaturesUpdater._getIndividualFeatures = sinon.stub().yields(null, { 'individual': 'features' }) @FeaturesUpdater._getGroupFeatureSets = sinon.stub().yields(null, [{ 'group': 'features' }, { 'group': 'features2' }]) @@ -29,48 +30,63 @@ describe "FeaturesUpdater", -> @ReferalFeatures.getBonusFeatures = sinon.stub().yields(null, { 'bonus': 'features' }) @FeaturesUpdater._mergeFeatures = sinon.stub().returns({'merged': 'features'}) @callback = sinon.stub() - @FeaturesUpdater.refreshFeatures @user_id, @callback - it "should get the individual features", -> - @FeaturesUpdater._getIndividualFeatures - .calledWith(@user_id) - .should.equal true + describe "normally", -> + beforeEach -> + @FeaturesUpdater.refreshFeatures @user_id, @callback - it "should get the group features", -> - @FeaturesUpdater._getGroupFeatureSets - .calledWith(@user_id) - .should.equal true + it "should get the individual features", -> + @FeaturesUpdater._getIndividualFeatures + .calledWith(@user_id) + .should.equal true - it "should get the v1 features", -> - @FeaturesUpdater._getV1Features - .calledWith(@user_id) - .should.equal true + it "should get the group features", -> + @FeaturesUpdater._getGroupFeatureSets + .calledWith(@user_id) + .should.equal true - it "should get the bonus features", -> - @ReferalFeatures.getBonusFeatures - .calledWith(@user_id) - .should.equal true + it "should get the v1 features", -> + @FeaturesUpdater._getV1Features + .calledWith(@user_id) + .should.equal true - it "should merge from the default features", -> - @FeaturesUpdater._mergeFeatures.calledWith(@Settings.defaultFeatures).should.equal true + it "should get the bonus features", -> + @ReferalFeatures.getBonusFeatures + .calledWith(@user_id) + .should.equal true - it "should merge the individual features", -> - @FeaturesUpdater._mergeFeatures.calledWith(sinon.match.any, { 'individual': 'features' }).should.equal true + it "should merge from the default features", -> + @FeaturesUpdater._mergeFeatures.calledWith(@Settings.defaultFeatures).should.equal true - it "should merge the group features", -> - @FeaturesUpdater._mergeFeatures.calledWith(sinon.match.any, { 'group': 'features' }).should.equal true - @FeaturesUpdater._mergeFeatures.calledWith(sinon.match.any, { 'group': 'features2' }).should.equal true + it "should merge the individual features", -> + @FeaturesUpdater._mergeFeatures.calledWith(sinon.match.any, { 'individual': 'features' }).should.equal true - it "should merge the v1 features", -> - @FeaturesUpdater._mergeFeatures.calledWith(sinon.match.any, { 'v1': 'features' }).should.equal true + it "should merge the group features", -> + @FeaturesUpdater._mergeFeatures.calledWith(sinon.match.any, { 'group': 'features' }).should.equal true + @FeaturesUpdater._mergeFeatures.calledWith(sinon.match.any, { 'group': 'features2' }).should.equal true - it "should merge the bonus features", -> - @FeaturesUpdater._mergeFeatures.calledWith(sinon.match.any, { 'bonus': 'features' }).should.equal true + it "should merge the v1 features", -> + @FeaturesUpdater._mergeFeatures.calledWith(sinon.match.any, { 'v1': 'features' }).should.equal true - it "should update the user with the merged features", -> - @UserFeaturesUpdater.updateFeatures - .calledWith(@user_id, {'merged': 'features'}) - .should.equal true + it "should merge the bonus features", -> + @FeaturesUpdater._mergeFeatures.calledWith(sinon.match.any, { 'bonus': 'features' }).should.equal true + + it "should update the user with the merged features", -> + @UserFeaturesUpdater.updateFeatures + .calledWith(@user_id, {'merged': 'features'}) + .should.equal true + + it "should notify v1", -> + @V1SubscriptionManager.notifyV1OfFeaturesChange + .called.should.equal true + + describe "with notifyV1 == false", -> + beforeEach -> + @FeaturesUpdater.refreshFeatures @user_id, false, @callback + + it "should not notify v1", -> + @V1SubscriptionManager.notifyV1OfFeaturesChange + .called.should.equal false describe "_mergeFeatures", -> it "should prefer priority over standard for compileGroup", -> diff --git a/services/web/test/unit/coffee/Subscription/V1SusbcriptionManagerTests.coffee b/services/web/test/unit/coffee/Subscription/V1SusbcriptionManagerTests.coffee index ae6237e627..9d85d81f84 100644 --- a/services/web/test/unit/coffee/Subscription/V1SusbcriptionManagerTests.coffee +++ b/services/web/test/unit/coffee/Subscription/V1SusbcriptionManagerTests.coffee @@ -16,10 +16,10 @@ describe 'V1SubscriptionManager', -> err: sinon.stub() warn: sinon.stub() "settings-sharelatex": - overleaf: - host: @host = "http://overleaf.example.com" + apis: + v1: + host: @host = "http://overleaf.example.com" "request": @request = sinon.stub() - @V1SubscriptionManager._v1PlanRequest = sinon.stub() @userId = 'abcd' @v1UserId = 42 @user = @@ -33,33 +33,20 @@ describe 'V1SubscriptionManager', -> @responseBody = id: 32, plan_name: 'pro' - @UserGetter.getUser = sinon.stub() - .yields(null, @user) - @V1SubscriptionManager._v1PlanRequest = sinon.stub() + @V1SubscriptionManager._v1Request = sinon.stub() .yields(null, @responseBody) @call = (cb) => @V1SubscriptionManager.getPlanCodeFromV1 @userId, cb describe 'when all goes well', -> - - it 'should call getUser', (done) -> + it 'should call _v1Request', (done) -> @call (err, planCode) => expect( - @UserGetter.getUser.callCount + @V1SubscriptionManager._v1Request.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 + @V1SubscriptionManager._v1Request.calledWith( + @userId ) ).to.equal true done() @@ -80,49 +67,56 @@ describe 'V1SubscriptionManager', -> expect(planCode).to.equal null done() + describe '_v1Request', -> + beforeEach -> + @UserGetter.getUser = sinon.stub() + .yields(null, @user) + describe 'when getUser produces an error', -> beforeEach -> @UserGetter.getUser = sinon.stub() .yields(new Error('woops')) + @call = (cb) => + @V1SubscriptionManager._v1Request @user_id, { url: () -> '/foo' }, cb - it 'should not call _v1PlanRequest', (done) -> + it 'should not call request', (done) -> @call (err, planCode) => expect( - @V1SubscriptionManager._v1PlanRequest.callCount + @request.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) + @call = (cb) => + @V1SubscriptionManager._v1Request @user_id, { url: () -> '/foo' }, cb - it 'should not call _v1PlanRequest', (done) -> + it 'should not call request', (done) -> @call (err, planCode) => expect( - @V1SubscriptionManager._v1PlanRequest.callCount + @request.callCount ).to.equal 0 done() - it 'should produce a null plan-code, without error', (done) -> - @call (err, planCode) => + it 'should not error', (done) -> + @call (err) => 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')) + @request.yields(new Error('woops')) + @call = (cb) => + @V1SubscriptionManager._v1Request @user_id, { url: () -> '/foo' }, cb it 'should produce an error', (done) -> - @call (err, planCode) => + @call (err) => expect(err).to.exist - expect(planCode).to.not.exist done()