Merge pull request #1027 from sharelatex/revert-1022-spd-no-github-for-new-users

Revert "Don't add old v1 features for new accounts"

services/web/app/coffee/Features/Subscription/FeaturesUpdater.coffee

@@ -52,14 +52,8 @@ module.exports = FeaturesUpdater =
 			callback err, (subs or []).map FeaturesUpdater._subscriptionToFeatures
 
 	_getV1Features: (user_id, callback = (error, features = {}) ->) ->
-		V1SubscriptionManager.getPlanCodeFromV1 user_id, (err, planCode, v1Id) ->
+		V1SubscriptionManager.getPlanCodeFromV1 user_id, (err, planCode) ->
-			if err?
+			callback err, FeaturesUpdater._planCodeToFeatures(planCode)
-				return callback(err)
-
-			callback(err, FeaturesUpdater._mergeFeatures(
-				V1SubscriptionManager.getGrandfatheredFeaturesForV1User(v1Id) or {},
-				FeaturesUpdater._planCodeToFeatures(planCode)
-			))
 
 	_mergeFeatures: (featuresA, featuresB) ->
 		features = Object.assign({}, featuresA)

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, v1Id)->) ->
+	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, v1Id) ->
+		}, (error, body) ->
 			return callback(error) if error?
 			planName = body?.plan_name
 			logger.log {userId, planName, body}, "[V1SubscriptionManager] fetched v1 plan for user"
@@ -33,40 +33,21 @@ module.exports = V1SubscriptionManager =
 			url: (v1Id) -> "/api/v1/sharelatex/users/#{v1Id}/sync"
 		}, callback
 
-	getSubscriptionsFromV1: (userId, callback=(err, subscriptions, v1Id) ->) ->
+	getSubscriptionsFromV1: (userId, callback=(err, subscriptions) ->) ->
 		V1SubscriptionManager._v1Request userId, {
 			method: 'GET',
 			url: (v1Id) -> "/api/v1/sharelatex/users/#{v1Id}/subscriptions"
 		}, callback
 
-	v1IdForUser: (userId, callback=(err, v1Id) ->) ->
+	_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)
-			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)
@@ -83,6 +64,7 @@ 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, v1Id
+					return callback null, body
 				else
 					return callback new Error("non-success code from v1: #{response.statusCode}")
+

services/web/test/unit/coffee/Subscription/V1SusbcriptionManagerTests.coffee

@@ -15,14 +15,10 @@ describe 'V1SubscriptionManager', ->
 				log: sinon.stub()
 				err: sinon.stub()
 				warn: sinon.stub()
-			"settings-sharelatex": @Settings =
+			"settings-sharelatex":
 				apis:
 					v1:
 						host: @host = "http://overleaf.example.com"
-				v1GrandfatheredFeaturesUidCutoff: 10
-				v1GrandfatheredFeatures:
-					github: true
-					mendeley: true
 			"request": @request = sinon.stub()
 		@userId = 'abcd'
 		@v1UserId = 42
@@ -71,27 +67,14 @@ 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 v1IdForUser produces an error', ->
+		describe 'when getUser produces an error', ->
 			beforeEach ->
-				@V1SubscriptionManager.v1IdForUser = sinon.stub()
+				@UserGetter.getUser = sinon.stub()
 					.yields(new Error('woops'))
 				@call = (cb) =>
 					@V1SubscriptionManager._v1Request @user_id, { url: () -> '/foo' }, cb
@@ -108,9 +91,9 @@ describe 'V1SubscriptionManager', ->
 					expect(err).to.exist
 					done()
 
-		describe 'when v1IdForUser does not find a user', ->
+		describe 'when getUser does not find a user', ->
 			beforeEach ->
-				@V1SubscriptionManager.v1IdForUser = sinon.stub()
+				@UserGetter.getUser = sinon.stub()
 					.yields(null, null)
 				@call = (cb) =>
 					@V1SubscriptionManager._v1Request @user_id, { url: () -> '/foo' }, cb
@@ -137,47 +120,3 @@ 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()