Merge pull request #610 from sharelatex/ja-sync-from-v2

Tell v1 when refreshing features in v2

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
+			return plan.features

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

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()
+				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

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

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", ->

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