Don't add old v1 features for new accounts

Introduces the notion of v1 'grandfathered features', specifically Gihub and Mendeley integration. This allows us to create new v1 accounts for new users without them automatically getting the new features.

Requires a settings change in `settings.web.sl.coffee` to disable these features by default for v1 accounts.

bug: overleaf/sharelatex#1014
This commit is contained in:
Simon Detheridge 2018-10-11 12:16:59 +01:00
parent 1256d29af9
commit 48995d2d44
3 changed files with 101 additions and 16 deletions

View file

@ -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)

View file

@ -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"
@ -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) 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}")

View file

@ -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
@ -67,14 +71,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 +108,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 +137,47 @@ describe 'V1SubscriptionManager', ->
@call (err) =>
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()