Преглед на файлове

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
Simon Detheridge преди 8 години
родител
ревизия
6fc6c44605

+ 8 - 2
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)

+ 28 - 10
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) 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}")
-

+ 66 - 5
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
@@ -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()