Просмотр исходного кода

Return user details like email and name in updates

James Allen 12 лет назад
Родитель
Сommit
b45db3aa2b

+ 1 - 1
services/track-changes/app/coffee/DiffManager.coffee

@@ -5,7 +5,7 @@ logger = require "logger-sharelatex"
 
 module.exports = DiffManager =
 	getLatestDocAndUpdates: (project_id, doc_id, fromVersion, toVersion, callback = (error, lines, version, updates) ->) ->
-		UpdatesManager.getUpdates doc_id, from: fromVersion, to: toVersion, (error, updates) ->
+		UpdatesManager.getUpdatesWithUserInfo doc_id, from: fromVersion, to: toVersion, (error, updates) ->
 			return callback(error) if error?
 			DocumentUpdaterManager.getDocument project_id, doc_id, (error, lines, version) ->
 				return callback(error) if error?

+ 1 - 3
services/track-changes/app/coffee/HttpController.coffee

@@ -37,7 +37,7 @@ module.exports = HttpController =
 		if req.query.limit?
 			limit = parseInt(req.query.limit, 10)
 
-		UpdatesManager.getUpdates doc_id, to: to, limit: limit, (error, updates) ->
+		UpdatesManager.getUpdatesWithUserInfo doc_id, to: to, limit: limit, (error, updates) ->
 			return next(error) if error?
 			formattedUpdates = for update in updates
 				{
@@ -45,5 +45,3 @@ module.exports = HttpController =
 					v: update.v
 				}
 			res.send JSON.stringify updates: formattedUpdates
-
-

+ 30 - 0
services/track-changes/app/coffee/UpdatesManager.coffee

@@ -2,7 +2,9 @@ MongoManager = require "./MongoManager"
 RedisManager = require "./RedisManager"
 UpdateCompressor = require "./UpdateCompressor"
 LockManager = require "./LockManager"
+WebApiManager = require "./WebApiManager"
 logger = require "logger-sharelatex"
+async = require "async"
 
 module.exports = UpdatesManager =
 	compressAndSaveRawUpdates: (doc_id, rawUpdates, callback = (error) ->) ->
@@ -70,3 +72,31 @@ module.exports = UpdatesManager =
 			return callback(error) if error?
 			MongoManager.getUpdates doc_id, options, callback
 
+	getUpdatesWithUserInfo: (doc_id, options = {}, callback = (error, updates) ->) ->
+		UpdatesManager.getUpdates doc_id, options, (error, updates) ->
+			return callback(error) if error?
+			UpdatesManager.fillUserInfo updates, (error, updates) ->
+				return callback(error) if error?
+				callback null, updates
+
+	fillUserInfo: (updates, callback = (error, updates) ->) ->
+		users = {}
+		for update in updates
+			users[update.meta.user_id] = true
+
+		jobs = []
+		for user_id, _ of users
+			do (user_id) ->
+				jobs.push (callback) ->
+					WebApiManager.getUserInfo user_id, (error, userInfo) ->
+						return callback(error) if error?
+						users[user_id] = userInfo
+						callback()
+
+		async.series jobs, (error) ->
+			return callback(error) if error?
+			for update in updates
+				user_id = update.meta.user_id
+				delete update.meta.user_id
+				update.meta.user = users[user_id]
+			callback null, updates

+ 32 - 0
services/track-changes/app/coffee/WebApiManager.coffee

@@ -0,0 +1,32 @@
+request = require "request"
+logger = require "logger-sharelatex"
+Settings = require "settings-sharelatex"
+
+module.exports = WebApiManager =
+	getUserInfo: (user_id, callback = (error, userInfo) ->) ->
+		url = "#{Settings.apis.web.url}/user/#{user_id}/personal_info"
+		logger.log user_id: user_id, "getting user info from web"
+		request.get {
+			url: url
+			auth:
+				user: Settings.apis.web.user
+				pass: Settings.apis.web.pass
+				sendImmediately: true
+		}, (error, res, body)->
+			if error?
+				return callback(error)
+			if res.statusCode >= 200 and res.statusCode < 300
+				try
+					user = JSON.parse(body)
+				catch error
+					return callback(error)
+				callback null, {
+					id: user.id
+					email: user.email
+					first_name: user.first_name
+					last_name: user.last_name
+				}
+			else
+				error = new Error("web returned a non-success status code: #{res.statusCode}")
+				logger.error err: error, user_id: user_id, url: url, "error accessing web"
+				callback error

+ 4 - 0
services/track-changes/config/settings.development.coffee

@@ -8,3 +8,7 @@ module.exports =
 	apis:
 		documentupdater:
 			url: "http://localhost:3003"
+		web:
+			url: "http://localhost:3000"
+			user: "sharelatex"
+			pass: "password"

+ 11 - 2
services/track-changes/test/acceptance/coffee/GettingADiffTests.coffee

@@ -9,6 +9,7 @@ Settings = require "settings-sharelatex"
 
 TrackChangesClient = require "./helpers/TrackChangesClient"
 MockDocUpdaterApi = require "./helpers/MockDocUpdaterApi"
+MockWebApi = require "./helpers/MockWebApi"
 
 describe "Getting a diff", ->
 	before (done) ->
@@ -21,6 +22,13 @@ describe "Getting a diff", ->
 		@doc_id = ObjectId().toString()
 		@project_id = ObjectId().toString()
 
+		MockWebApi.users[@user_id] = @user =
+			email: "user@sharelatex.com"
+			first_name: "Leo"
+			last_name: "Lion"
+			id: @user_id
+		sinon.spy MockWebApi, "getUser"
+
 		twoMinutes = 2 * 60 * 1000
 
 		@updates = [{
@@ -43,8 +51,8 @@ describe "Getting a diff", ->
 		@lines = ["one two three four"]
 		@expected_diff = [
 			{ u: "one " }
-			{ i: "two ", meta: { start_ts: @from + twoMinutes, end_ts: @from + twoMinutes, user_id: @user_id } }
-			{ i: "three ", meta: { start_ts: @to - twoMinutes, end_ts: @to - twoMinutes, user_id: @user_id } }
+			{ i: "two ", meta: { start_ts: @from + twoMinutes, end_ts: @from + twoMinutes, user: @user } }
+			{ i: "three ", meta: { start_ts: @to - twoMinutes, end_ts: @to - twoMinutes, user: @user } }
 		]
 
 		MockDocUpdaterApi.docs[@doc_id] =
@@ -60,6 +68,7 @@ describe "Getting a diff", ->
 
 	after () ->
 		MockDocUpdaterApi.getDoc.restore()
+		MockWebApi.getUser.restore()
 
 	it "should return the diff", ->
 		expect(@diff).to.deep.equal @expected_diff

+ 19 - 3
services/track-changes/test/acceptance/coffee/GettingUpdatesTests.coffee

@@ -8,6 +8,7 @@ ObjectId = mongojs.ObjectId
 Settings = require "settings-sharelatex"
 
 TrackChangesClient = require "./helpers/TrackChangesClient"
+MockWebApi = require "./helpers/MockWebApi"
 
 describe "Getting updates", ->
 	before (done) ->
@@ -19,6 +20,13 @@ describe "Getting updates", ->
 
 		@minutes = 60 * 1000
 
+		MockWebApi.users[@user_id] = @user =
+			email: "user@sharelatex.com"
+			first_name: "Leo"
+			last_name: "Lion"
+			id: @user_id
+		sinon.spy MockWebApi, "getUser"
+
 		@updates = [{
 			op: [{ i: "one ", p: 0 }]
 			meta: { ts: @to - 4 * @minutes, user_id: @user_id }
@@ -44,17 +52,25 @@ describe "Getting updates", ->
 				@updates = body.updates
 				done()
 
-	it "should return the diff", ->
+	after: () ->
+		MockWebApi.getUser.restore()
+
+	it "should fetch the user details from the web api", ->
+		MockWebApi.getUser
+			.calledWith(@user_id)
+			.should.equal true
+
+	it "should return the updates", ->
 		expect(@updates).to.deep.equal [{
 			meta:
 				start_ts: @to
 				end_ts: @to
-				user_id: @user_id
+				user: @user
 			v: 5
 		}, {
 			meta:
 				start_ts: @to - 2 * @minutes
 				end_ts: @to - 2 * @minutes
-				user_id: @user_id
+				user: @user
 			v: 4
 		}]

+ 24 - 0
services/track-changes/test/acceptance/coffee/helpers/MockWebApi.coffee

@@ -0,0 +1,24 @@
+express = require("express")
+app = express()
+
+module.exports = MockWebApi =
+	users: {}
+
+	getUser: (user_id, callback = (error) ->) ->
+		callback null, @users[user_id]
+
+	run: () ->
+		app.get "/user/:user_id/personal_info", (req, res, next) =>
+			@getUser req.params.user_id, (error, user) ->
+				if error?
+					res.send 500
+				if !user?
+					res.send 404
+				else
+					res.send JSON.stringify user
+
+		app.listen 3000, (error) ->
+			throw error if error?
+
+MockWebApi.run()
+

+ 2 - 2
services/track-changes/test/unit/coffee/DiffManager/DiffManagerTests.coffee

@@ -25,7 +25,7 @@ describe "DiffManager", ->
 			@updates = [ "mock-update-1", "mock-update-2" ]
 
 			@DocumentUpdaterManager.getDocument = sinon.stub().callsArgWith(2, null, @lines, @version)
-			@UpdatesManager.getUpdates = sinon.stub().callsArgWith(2, null, @updates)
+			@UpdatesManager.getUpdatesWithUserInfo = sinon.stub().callsArgWith(2, null, @updates)
 			@DiffManager.getLatestDocAndUpdates @project_id, @doc_id, @from, @to, @callback
 
 		it "should get the latest version of the doc", ->
@@ -34,7 +34,7 @@ describe "DiffManager", ->
 				.should.equal true
 
 		it "should get the latest updates", ->
-			@UpdatesManager.getUpdates
+			@UpdatesManager.getUpdatesWithUserInfo
 				.calledWith(@doc_id, from: @from, to: @to)
 				.should.equal true
 

+ 3 - 3
services/track-changes/test/unit/coffee/HttpController/HttpControllerTests.coffee

@@ -78,15 +78,15 @@ describe "HttpController", ->
 				meta:   @meta = "mock-meta"
 				doc_id: @doc_id
 			}]
-			@UpdatesManager.getUpdates = sinon.stub().callsArgWith(2, null, @rawUpdates)
+			@UpdatesManager.getUpdatesWithUserInfo = sinon.stub().callsArgWith(2, null, @rawUpdates)
 			@HttpController.getUpdates @req, @res, @next
 
 		it "should get the updates", ->
-			@UpdatesManager.getUpdates
+			@UpdatesManager.getUpdatesWithUserInfo
 				.calledWith(@doc_id, to: @to, limit: @limit)
 				.should.equal true
 
-		it "should return the updates", ->
+		it "should return the formatted updates", ->
 			updates = for update in @rawUpdates
 				{
 					meta: @meta

+ 85 - 0
services/track-changes/test/unit/coffee/UpdatesManager/UpdatesManagerTests.coffee

@@ -12,6 +12,7 @@ describe "UpdatesManager", ->
 			"./MongoManager" : @MongoManager = {}
 			"./RedisManager" : @RedisManager = {}
 			"./LockManager"  : @LockManager = {}
+			"./WebApiManager": @WebApiManager = {}
 			"logger-sharelatex": { log: sinon.stub(), error: sinon.stub() }
 		@doc_id = "doc-id-123"
 		@callback = sinon.stub()
@@ -219,3 +220,87 @@ describe "UpdatesManager", ->
 				.calledWith(null, @updates)
 				.should.equal true
 
+	describe "getUpdatesWithUserInfo", ->
+		beforeEach ->
+			@updates = ["mock-updates"]
+			@options = { to: "mock-to", limit: "mock-limit" }
+			@updatesWithUserInfo = ["updates-with-user-info"]
+			@UpdatesManager.getUpdates = sinon.stub().callsArgWith(2, null, @updates)
+			@UpdatesManager.fillUserInfo = sinon.stub().callsArgWith(1, null, @updatesWithUserInfo)
+			@UpdatesManager.getUpdatesWithUserInfo @doc_id, @options, @callback
+
+		it "should get the updates", ->
+			@UpdatesManager.getUpdates
+				.calledWith(@doc_id, @options)
+				.should.equal true
+
+		it "should file the updates with the user info", ->
+			@UpdatesManager.fillUserInfo
+				.calledWith(@updates)
+				.should.equal true
+
+		it "shoudl return the updates with the filled details", ->
+			@callback.calledWith(null, @updatesWithUserInfo).should.equal true
+
+	describe "fillUserInfo", ->
+		beforeEach (done) ->
+			@user_id_1 = "user-id-1"
+			@user_id_2 = "user-id-2"
+			@updates = [{
+				meta:
+					user_id: @user_id_1
+				op: "mock-op-1"
+			}, {
+				meta:
+					user_id: @user_id_1
+				op: "mock-op-2"
+			}, {
+				meta:
+					user_id: @user_id_2
+				op: "mock-op-3"
+			}]
+			@user_info =
+				"user-id-1": {
+					email: "user1@sharelatex.com"
+				}
+				"user-id-2": {
+					email: "user2@sharelatex.com"
+				}
+			@WebApiManager.getUserInfo = (user_id, callback = (error, userInfo) ->) =>
+				callback null, @user_info[user_id]
+			sinon.spy @WebApiManager, "getUserInfo"
+
+			@UpdatesManager.fillUserInfo @updates, (error, @results) =>
+				done()
+
+		it "should only call getUserInfo once for each user_id", ->
+			@WebApiManager.getUserInfo.calledTwice.should.equal true
+			@WebApiManager.getUserInfo
+				.calledWith(@user_id_1)
+				.should.equal true
+			@WebApiManager.getUserInfo
+				.calledWith(@user_id_2)
+				.should.equal true
+
+		it "should return the updates with the user info filled", ->
+			expect(@results).to.deep.equal [{
+				meta:
+					user:
+						email: "user1@sharelatex.com"
+				op: "mock-op-1"
+			}, {
+				meta:
+					user:
+						email: "user1@sharelatex.com"
+				op: "mock-op-2"
+			}, {
+				meta:
+					user:
+						email: "user2@sharelatex.com"
+				op: "mock-op-3"
+			}]
+
+
+
+
+

+ 71 - 0
services/track-changes/test/unit/coffee/WebApiManager/WebApiManagerTests.coffee

@@ -0,0 +1,71 @@
+sinon = require('sinon')
+chai = require('chai')
+should = chai.should()
+expect = chai.expect
+modulePath = "../../../../app/js/WebApiManager.js"
+SandboxedModule = require('sandboxed-module')
+
+describe "WebApiManager", ->
+	beforeEach ->
+		@WebApiManager = SandboxedModule.require modulePath, requires:
+			"request": @request = {}
+			"logger-sharelatex": @logger = { log: sinon.stub(), error: sinon.stub() }
+			'settings-sharelatex': @settings =
+				apis:
+					web:
+						url: "http://example.com"
+						user: "sharelatex"
+						pass: "password"
+		@callback = sinon.stub()
+		@user_id = "mock-user-id"
+		@user_info =
+			email: "leo@sharelatex.com"
+			id: @user_id
+			first_name: "Leo"
+			last_nane: "Lion"
+			extra_param: "blah"
+
+	describe "getUserInfo", ->
+		describe "successfully", ->
+			beforeEach ->
+				@body = JSON.stringify @user_info
+				@request.get = sinon.stub().callsArgWith(1, null, {statusCode: 200}, @body)
+				@WebApiManager.getUserInfo @user_id, @callback
+
+			it 'should get the user from the web api', ->
+				url = 
+				@request.get
+					.calledWith({
+						url: "#{@settings.apis.web.url}/user/#{@user_id}/personal_info"
+						auth:
+							user: @settings.apis.web.user
+							pass: @settings.apis.web.pass
+							sendImmediately: true
+					})
+					.should.equal true
+
+			it "should call the callback with only the email, id and names", ->
+				@callback.calledWith(null, {
+					id:         @user_id
+					email:      @user_info.email
+					first_name: @user_info.first_name
+					last_name:  @user_info.last_name
+				}).should.equal true
+
+		describe "when the web API returns an error", ->
+			beforeEach ->
+				@request.get = sinon.stub().callsArgWith(1, @error = new Error("something went wrong"), null, null)
+				@WebApiManager.getUserInfo @user_id, @callback
+
+			it "should return an error to the callback", ->
+				@callback.calledWith(@error).should.equal true
+
+		describe "when the web returns a failure error code", ->
+			beforeEach ->
+				@request.get = sinon.stub().callsArgWith(1, null, { statusCode: 500 }, "")
+				@WebApiManager.getUserInfo @user_id, @callback
+
+			it "should return the callback with an error", ->
+				@callback
+					.calledWith(new Error("web returned failure status code: 500"))
+					.should.equal true