diff --git a/services/web/app/coffee/Features/Subscription/FeaturesUpdater.coffee b/services/web/app/coffee/Features/Subscription/FeaturesUpdater.coffee index ef5a6c4bb2..b091ed9f30 100644 --- a/services/web/app/coffee/Features/Subscription/FeaturesUpdater.coffee +++ b/services/web/app/coffee/Features/Subscription/FeaturesUpdater.coffee @@ -52,8 +52,14 @@ module.exports = FeaturesUpdater = callback err, (subs or []).map FeaturesUpdater._subscriptionToFeatures _getV1Features: (user_id, callback = (error, features = {}) ->) -> - V1SubscriptionManager.getPlanCodeFromV1 user_id, (err, planCode) -> - callback err, FeaturesUpdater._planCodeToFeatures(planCode) + V1SubscriptionManager.getPlanCodeFromV1 user_id, (err, planCode, v1Id) -> + if err? + return callback(err) + + callback(err, FeaturesUpdater._mergeFeatures( + V1SubscriptionManager.getGrandfatheredFeaturesForV1User(v1Id) or {}, + FeaturesUpdater._planCodeToFeatures(planCode) + )) _mergeFeatures: (featuresA, featuresB) -> features = Object.assign({}, featuresA) diff --git a/services/web/app/coffee/Features/Subscription/V1SubscriptionManager.coffee b/services/web/app/coffee/Features/Subscription/V1SubscriptionManager.coffee index 857881df4d..2d25c8b3e5 100644 --- a/services/web/app/coffee/Features/Subscription/V1SubscriptionManager.coffee +++ b/services/web/app/coffee/Features/Subscription/V1SubscriptionManager.coffee @@ -11,12 +11,12 @@ module.exports = V1SubscriptionManager = # - 'v1_pro_plus' # - 'v1_student' # - 'v1_free' - getPlanCodeFromV1: (userId, callback=(err, planCode)->) -> + getPlanCodeFromV1: (userId, callback=(err, planCode, v1Id)->) -> 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) -> + }, (error, body, v1Id) -> return callback(error) if error? planName = body?.plan_name logger.log {userId, planName, body}, "[V1SubscriptionManager] fetched v1 plan for user" @@ -25,7 +25,7 @@ module.exports = V1SubscriptionManager = else # Throw away 'anonymous', etc as being equivalent to null planName = null - return callback(null, planName) + return callback(null, planName, v1Id) notifyV1OfFeaturesChange: (userId, callback = (error) ->) -> V1SubscriptionManager._v1Request userId, { @@ -33,21 +33,40 @@ module.exports = V1SubscriptionManager = url: (v1Id) -> "/api/v1/sharelatex/users/#{v1Id}/sync" }, callback - getSubscriptionsFromV1: (userId, callback=(err, subscriptions) ->) -> + getSubscriptionsFromV1: (userId, callback=(err, subscriptions, v1Id) ->) -> V1SubscriptionManager._v1Request userId, { method: 'GET', url: (v1Id) -> "/api/v1/sharelatex/users/#{v1Id}/subscriptions" }, callback - _v1Request: (userId, options, callback=(err, body)->) -> - if !settings?.apis?.v1 - return callback null, null + v1IdForUser: (userId, callback=(err, v1Id) ->) -> 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) + + callback(null, v1Id) + + # v1 accounts created before migration to v2 had github and mendeley for free + # but these are now paid-for features for new accounts (v1id > cutoff) + getGrandfatheredFeaturesForV1User: (v1Id) -> + cutoff = settings.v1GrandfatheredFeaturesUidCutoff + return {} if !cutoff? + return {} if !v1Id? + + if (v1Id < cutoff) + return settings.v1GrandfatheredFeatures or {} + else + return {} + + _v1Request: (userId, options, callback=(err, body, v1Id)->) -> + if !settings?.apis?.v1 + return callback null, null + + V1SubscriptionManager.v1IdForUser userId, (err, v1Id) -> + return callback(err) if err? + return callback(null, null, null) if !v1Id? request { baseUrl: settings.apis.v1.url url: options.url(v1Id) @@ -64,7 +83,6 @@ module.exports = V1SubscriptionManager = error = new V1ConnectionError('No V1 connection') if error.code == 'ECONNREFUSED' return callback(error) if 200 <= response.statusCode < 300 - return callback null, body + return callback null, body, v1Id else return callback new Error("non-success code from v1: #{response.statusCode}") - diff --git a/services/web/test/unit/coffee/Subscription/V1SusbcriptionManagerTests.coffee b/services/web/test/unit/coffee/Subscription/V1SusbcriptionManagerTests.coffee index 9d85d81f84..7666a418bb 100644 --- a/services/web/test/unit/coffee/Subscription/V1SusbcriptionManagerTests.coffee +++ b/services/web/test/unit/coffee/Subscription/V1SusbcriptionManagerTests.coffee @@ -15,10 +15,14 @@ describe 'V1SubscriptionManager', -> log: sinon.stub() err: sinon.stub() warn: sinon.stub() - "settings-sharelatex": + "settings-sharelatex": @Settings = apis: v1: host: @host = "http://overleaf.example.com" + v1GrandfatheredFeaturesUidCutoff: 10 + v1GrandfatheredFeatures: + github: true + mendeley: true "request": @request = sinon.stub() @userId = 'abcd' @v1UserId = 42 @@ -51,6 +55,11 @@ describe 'V1SubscriptionManager', -> ).to.equal true done() + it 'should return the v1 user id', (done) -> + @call (err, planCode, v1Id) -> + expect(v1Id).to.equal @v1UserId + done() + it 'should produce a plan-code without error', (done) -> @call (err, planCode) => expect(err).to.not.exist @@ -67,14 +76,27 @@ describe 'V1SubscriptionManager', -> expect(planCode).to.equal null done() + describe 'getGrandfatheredFeaturesForV1User', -> + describe 'when the user ID is greater than the cutoff', -> + it 'should return an empty feature set', (done) -> + expect(@V1SubscriptionManager.getGrandfatheredFeaturesForV1User 100).to.eql {} + done() + + describe 'when the user ID is less than the cutoff', -> + it 'should return a feature set with grandfathered properties for github and mendeley', (done) -> + expect(@V1SubscriptionManager.getGrandfatheredFeaturesForV1User 1).to.eql + github: true + mendeley: true + done() + describe '_v1Request', -> beforeEach -> @UserGetter.getUser = sinon.stub() .yields(null, @user) - describe 'when getUser produces an error', -> + describe 'when v1IdForUser produces an error', -> beforeEach -> - @UserGetter.getUser = sinon.stub() + @V1SubscriptionManager.v1IdForUser = sinon.stub() .yields(new Error('woops')) @call = (cb) => @V1SubscriptionManager._v1Request @user_id, { url: () -> '/foo' }, cb @@ -91,9 +113,9 @@ describe 'V1SubscriptionManager', -> expect(err).to.exist done() - describe 'when getUser does not find a user', -> + describe 'when v1IdForUser does not find a user', -> beforeEach -> - @UserGetter.getUser = sinon.stub() + @V1SubscriptionManager.v1IdForUser = sinon.stub() .yields(null, null) @call = (cb) => @V1SubscriptionManager._v1Request @user_id, { url: () -> '/foo' }, cb @@ -120,3 +142,83 @@ describe 'V1SubscriptionManager', -> @call (err) => expect(err).to.exist done() + + describe 'when the call succeeds', -> + beforeEach -> + @V1SubscriptionManager.v1IdForUser = sinon.stub() + .yields(null, @v1UserId) + @request.yields(null, { statusCode: 200 }, "{}") + @call = (cb) => + @V1SubscriptionManager._v1Request @user_id, { url: () -> '/foo' }, cb + + it 'should not produce an error', (done) -> + @call (err, body, v1Id) => + expect(err).not.to.exist + done() + + it 'should return the v1 user id', (done) -> + @call (err, body, v1Id) => + expect(v1Id).to.equal @v1UserId + done() + + it 'should return the http response body', (done) -> + @call (err, body, v1Id) => + expect(body).to.equal "{}" + done() + + describe 'when the call returns an http error status code', -> + beforeEach -> + @V1SubscriptionManager.v1IdForUser = sinon.stub() + .yields(null, @v1UserId) + @request.yields(null, { statusCode: 500 }, "{}") + @call = (cb) => + @V1SubscriptionManager._v1Request @user_id, { url: () -> '/foo' }, cb + + it 'should produce an error', (done) -> + @call (err, body, v1Id) => + expect(err).to.exist + done() + + describe 'v1IdForUser', -> + 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.v1IdForUser @user_id, cb + + it 'should produce an error', (done) -> + @call (err) => + expect(err).to.exist + done() + + describe 'when getUser does not find a user', -> + beforeEach -> + @UserGetter.getUser = sinon.stub() + .yields(null, null) + @call = (cb) => + @V1SubscriptionManager.v1IdForUser @user_id, cb + + it 'should not error', (done) -> + @call (err, user_id) => + expect(err).to.not.exist + done() + + describe 'when it works', -> + beforeEach -> + @call = (cb) => + @V1SubscriptionManager.v1IdForUser @user_id, cb + + it 'should not error', (done) -> + @call (err, user_id) => + expect(err).to.not.exist + done() + + it 'should return the v1 user id', (done) -> + @call (err, user_id) => + expect(user_id).to.eql 42 + done() \ No newline at end of file