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

Get user info via web, not chat

James Allen 9 лет назад
Родитель
Сommit
3a5d45fa32

+ 12 - 4
services/web/app/coffee/Features/Chat/ChatController.coffee

@@ -2,6 +2,9 @@ ChatApiHandler = require("./ChatApiHandler")
 EditorRealTimeController = require("../Editor/EditorRealTimeController")
 logger = require("logger-sharelatex")
 AuthenticationController = require('../Authentication/AuthenticationController')
+UserInfoManager = require('../User/UserInfoManager')
+UserInfoController = require('../User/UserInfoController')
+CommentsController = require('../Comments/CommentsController')
 
 module.exports =
 	sendMessage: (req, res, next)->
@@ -13,8 +16,11 @@ module.exports =
 			return next(err)
 		ChatApiHandler.sendGlobalMessage project_id, user_id, content, (err, message) ->
 			return next(err) if err?
-			EditorRealTimeController.emitToRoom project_id, "new-chat-message", message, (err)->
-			res.send(204)
+			UserInfoManager.getPersonalInfo message.user_id, (err, user) ->
+				return next(err) if err?
+				message.user = UserInfoController.formatPersonalInfo(user)
+				EditorRealTimeController.emitToRoom project_id, "new-chat-message", message, (err)->
+				res.send(204)
 
 	getMessages: (req, res, next)->
 		project_id = req.params.project_id
@@ -22,5 +28,7 @@ module.exports =
 		logger.log project_id:project_id, query:query, "getting messages"
 		ChatApiHandler.getGlobalMessages project_id, query.limit, query.before, (err, messages) ->
 			return next(err) if err?
-			logger.log length: messages?.length, "sending messages to client"
-			res.json messages
+			CommentsController._injectUserInfoIntoThreads [{ messages: messages }], (err) ->
+				return next(err) if err?
+				logger.log length: messages?.length, "sending messages to client"
+				res.json messages

+ 50 - 9
services/web/app/coffee/Features/Comments/CommentsController.coffee

@@ -2,6 +2,9 @@ ChatApiHandler = require("../Chat/ChatApiHandler")
 EditorRealTimeController = require("../Editor/EditorRealTimeController")
 logger = require("logger-sharelatex")
 AuthenticationController = require('../Authentication/AuthenticationController')
+UserInfoManager = require('../User/UserInfoManager')
+UserInfoController = require('../User/UserInfoController')
+async = require "async"
 
 module.exports = CommentsController =
 	sendComment: (req, res, next) ->
@@ -14,29 +17,67 @@ module.exports = CommentsController =
 		logger.log {project_id, thread_id, user_id, content}, "sending comment"
 		ChatApiHandler.sendComment project_id, thread_id, user_id, content, (err, comment) ->
 			return next(err) if err?
-			EditorRealTimeController.emitToRoom project_id, "new-comment", thread_id, comment, (err) ->
-			res.send 204
+			UserInfoManager.getPersonalInfo comment.user_id, (err, user) ->
+				return next(err) if err?
+				comment.user = UserInfoController.formatPersonalInfo(user)
+				EditorRealTimeController.emitToRoom project_id, "new-comment", thread_id, comment, (err) ->
+				res.send 204
 
 	getThreads: (req, res, next) ->
 		{project_id} = req.params
 		logger.log {project_id}, "getting comment threads for project"
 		ChatApiHandler.getThreads project_id, (err, threads) ->
 			return next(err) if err?
-			res.json threads
-	
+			CommentsController._injectUserInfoIntoThreads threads, (error, threads) ->
+				return next(err) if err?
+				res.json threads
+
 	resolveThread: (req, res, next) ->
 		{project_id, thread_id} = req.params
 		user_id = AuthenticationController.getLoggedInUserId(req)
 		logger.log {project_id, thread_id, user_id}, "resolving comment thread"
-		ChatApiHandler.resolveThread project_id, thread_id, user_id, (err, threads) ->
+		ChatApiHandler.resolveThread project_id, thread_id, user_id, (err) ->
 			return next(err) if err?
-			EditorRealTimeController.emitToRoom project_id, "resolve-thread", thread_id, user_id, (err)->
-			res.send 204
-	
+			UserInfoManager.getPersonalInfo user_id, (err, user) ->
+				return next(err) if err?
+				EditorRealTimeController.emitToRoom project_id, "resolve-thread", thread_id, UserInfoController.formatPersonalInfo(user), (err)->
+				res.send 204
+
 	reopenThread: (req, res, next) ->
 		{project_id, thread_id} = req.params
 		logger.log {project_id, thread_id}, "reopening comment thread"
 		ChatApiHandler.reopenThread project_id, thread_id, (err, threads) ->
 			return next(err) if err?
 			EditorRealTimeController.emitToRoom project_id, "reopen-thread", thread_id, (err)->
-			res.send 204
+			res.send 204
+
+	_injectUserInfoIntoThreads: (threads, callback = (error, threads) ->) ->
+		userCache = {}
+		getUserDetails = (user_id, callback = (error, user) ->) ->
+			return callback(null, userCache[user_id]) if userCache[user_id]?
+			UserInfoManager.getPersonalInfo user_id, (err, user) ->
+				return callback(error) if error?
+				user = UserInfoController.formatPersonalInfo user
+				userCache[user_id] = user
+				callback null, user
+		
+		jobs = []
+		for thread in threads
+			do (thread) ->
+				if thread.resolved
+					jobs.push (cb) ->
+						getUserDetails thread.resolved_by_user_id, (error, user) ->
+							cb(error) if error?
+							thread.resolved_by_user = user
+							cb()
+				for message in thread.messages
+					do (message) ->
+						jobs.push (cb) ->
+							getUserDetails message.user_id, (error, user) ->
+								cb(error) if error?
+								message.user = user
+								cb()
+		
+		async.series jobs, (error) ->
+			return callback(error) if error?
+			return callback null, threads

+ 9 - 0
services/web/app/coffee/Features/Ranges/RangesController.coffee

@@ -1,5 +1,6 @@
 RangesManager = require "./RangesManager"
 logger = require "logger-sharelatex"
+UserInfoController = require "../User/UserInfoController"
 
 module.exports = RangesController =
 	getAllRanges: (req, res, next) ->
@@ -9,3 +10,11 @@ module.exports = RangesController =
 			return next(error) if error?
 			docs = ({id: d._id, ranges: d.ranges} for d in docs)
 			res.json docs
+	
+	getAllRangesUsers: (req, res, next) ->
+		project_id = req.params.project_id
+		logger.log {project_id}, "request for project range users"
+		RangesManager.getAllRangesUsers project_id, (error, users) ->
+			return next(error) if error?
+			users = (UserInfoController.formatPersonalInfo(user) for user in users)
+			res.json users

+ 16 - 1
services/web/app/coffee/Features/Ranges/RangesManager.coffee

@@ -1,8 +1,23 @@
 DocumentUpdaterHandler = require "../DocumentUpdater/DocumentUpdaterHandler"
 DocstoreManager = require "../Docstore/DocstoreManager"
+UserInfoManager = require "../User/UserInfoManager"
+async = require "async"
 
 module.exports = RangesManager =
 	getAllRanges: (project_id, callback = (error, docs) ->) ->
 		DocumentUpdaterHandler.flushProjectToMongo project_id, (error) ->
 			return callback(error) if error?
-			DocstoreManager.getAllRanges project_id, callback
+			DocstoreManager.getAllRanges project_id, callback
+	
+	getAllRangesUsers: (project_id, callback = (error, users) ->) ->
+		user_ids = {}
+		RangesManager.getAllRanges project_id, (error, docs) ->
+			return callback(error) if error?
+			jobs = []
+			for doc in docs
+				for change in doc.ranges?.changes or []
+					user_ids[change.metadata.user_id] = true
+			
+			async.mapSeries Object.keys(user_ids), (user_id, cb) ->
+				UserInfoManager.getPersonalInfo user_id, cb
+			, callback

+ 8 - 13
services/web/app/coffee/Features/User/UserInfoController.coffee

@@ -26,17 +26,12 @@ module.exports = UserController =
 			UserController.sendFormattedPersonalInfo(user, res, next)
 
 	sendFormattedPersonalInfo: (user, res, next = (error) ->) ->
-		UserController._formatPersonalInfo user, (error, info) ->
-			return next(error) if error?
-			res.send JSON.stringify(info)
+		info = UserController.formatPersonalInfo(user)
+		res.send JSON.stringify(info)
 
-	_formatPersonalInfo: (user, callback = (error, info) ->) ->
-		callback null, {
-			id: user._id.toString()
-			first_name: user.first_name
-			last_name: user.last_name
-			email: user.email
-			signUpDate: user.signUpDate
-			role: user.role
-			institution: user.institution
-		}
+	formatPersonalInfo: (user, callback = (error, info) ->) ->
+		formatted_user = { id: user._id.toString() }
+		for key in ["first_name", "last_name", "email", "signUpDate", "role", "institution"]
+			if user[key]?
+				formatted_user[key] = user[key]
+		return formatted_user

+ 5 - 0
services/web/app/coffee/Features/User/UserInfoManager.coffee

@@ -0,0 +1,5 @@
+UserGetter = require "./UserGetter"
+
+module.exports = UserInfoManager =
+	getPersonalInfo: (user_id, callback = (error) ->) ->
+		UserGetter.getUser user_id, { _id: true, first_name: true, last_name: true, email: true }, callback

+ 1 - 0
services/web/app/coffee/router.coffee

@@ -178,6 +178,7 @@ module.exports = class Router
 		webRouter.post "/project/:Project_id/doc/:doc_id/version/:version_id/restore", AuthorizationMiddlewear.ensureUserCanReadProject, HistoryController.proxyToHistoryApi
 
 		webRouter.get "/project/:project_id/ranges", AuthorizationMiddlewear.ensureUserCanReadProject, RangesController.getAllRanges
+		webRouter.get "/project/:project_id/ranges/users", AuthorizationMiddlewear.ensureUserCanReadProject, RangesController.getAllRangesUsers
 
 		webRouter.get  '/Project/:Project_id/download/zip', AuthorizationMiddlewear.ensureUserCanReadProject, ProjectDownloadsController.downloadProject
 		webRouter.get  '/project/download/zip', AuthorizationMiddlewear.ensureUserCanReadMultipleProjects, ProjectDownloadsController.downloadMultipleProjects

+ 8 - 2
services/web/public/coffee/ide/chat/services/chatMessages.coffee

@@ -1,5 +1,6 @@
 define [
 	"base"
+	"libs/md5"
 ], (App) ->
 	App.factory "chatMessages", ($http, ide) ->
 		MESSAGES_URL = "/project/#{ide.project_id}/messages"
@@ -72,7 +73,7 @@ define [
 				firstMessage.contents.unshift message.content
 			else
 				chat.state.messages.unshift({
-					user: message.user
+					user: formatUser(message.user)
 					timestamp: message.timestamp
 					contents: [message.content]
 				})
@@ -93,9 +94,14 @@ define [
 				lastMessage.contents.push message.content
 			else
 				chat.state.messages.push({
-					user: message.user
+					user: formatUser(message.user)
 					timestamp: message.timestamp
 					contents: [message.content]
 				})
+				
+		formatUser = (user) ->
+			hash = CryptoJS.MD5(user.email.toLowerCase())
+			user.gravatar_url = "//www.gravatar.com/avatar/#{hash}"
+			return user
 
 		return chat

+ 41 - 21
services/web/public/coffee/ide/review-panel/controllers/ReviewPanelController.coffee

@@ -24,18 +24,10 @@ define [
 			adding: false
 			content: ""
 
+		$scope.users = {}
+
 		$scope.reviewPanelEventsBridge = new EventEmitter()
 		
-		$http.get "/project/#{$scope.project_id}/threads"
-			.success (threads) ->
-				for thread_id, thread of threads
-					for comment in thread.messages
-						formatComment(comment)
-					if thread.resolved_by_user?
-						$scope.$broadcast "comment:resolve_thread", thread_id
-						formatUser(thread.resolved_by_user)
-				$scope.reviewPanel.commentThreads = threads
-		
 		ide.socket.on "new-comment", (thread_id, comment) ->
 			$scope.reviewPanel.commentThreads[thread_id] ?= { messages: [] }
 			$scope.reviewPanel.commentThreads[thread_id].messages.push(formatComment(comment))
@@ -141,6 +133,9 @@ define [
 				for key, value of new_entry
 					entries[change.id][key] = value
 
+				if !$scope.users[change.metadata.user_id]?
+					refreshChangeUsers(change.metadata.user_id)
+
 			for comment in rangesTracker.comments
 				delete delete_changes[comment.id]
 				entries[comment.id] ?= {}
@@ -239,7 +234,7 @@ define [
 			entry.focused = false
 			thread = $scope.reviewPanel.commentThreads[entry.thread_id]
 			thread.resolved = true
-			thread.resolved_by_user = $scope.users[window.user_id]
+			thread.resolved_by_user = formatUser(ide.$scope.user)
 			thread.resolved_at = new Date()
 			$http.post "/project/#{$scope.project_id}/thread/#{entry.thread_id}/resolve", {_csrf: window.csrfToken}
 			$scope.$broadcast "comment:resolve_thread", entry.thread_id
@@ -271,12 +266,41 @@ define [
 		$scope.gotoEntry = (doc_id, entry) ->
 			ide.editorManager.openDocId(doc_id, { gotoOffset: entry.offset })
 
-		# TODO: Eventually we need to get this from the server, and update it 
-		# when we get an id we don't know. This'll do for client side testing
-		refreshUsers = () ->
-			$scope.users = {}
-			for member in $scope.project.members.concat($scope.project.owner)
-				$scope.users[member._id] = formatUser(member)
+		_refreshingRangeUsers = false
+		_refreshedForUserIds = {}
+		refreshChangeUsers = (refresh_for_user_id) ->
+			if refresh_for_user_id?
+				if _refreshedForUserIds[refresh_for_user_id]?
+					# We've already tried to refresh to get this user id, so stop it looping
+					return
+				_refreshedForUserIds[refresh_for_user_id] = true
+
+			# Only do one refresh at once
+			if _refreshingRangeUsers
+				return
+			_refreshingRangeUsers = true
+
+			$http.get "/project/#{$scope.project_id}/ranges/users"
+				.success (users) ->
+					_refreshingRangeUsers = false
+					$scope.users = {}
+					for user in users
+						$scope.users[user.id] = formatUser(user)
+				.error () ->
+					_refreshingRangeUsers = false
+
+		refreshThreads = () ->
+			$http.get "/project/#{$scope.project_id}/threads"
+				.success (threads) ->
+					for thread_id, thread of threads
+						for comment in thread.messages
+							formatComment(comment)
+						if thread.resolved_by_user?
+							$scope.$broadcast "comment:resolve_thread", thread_id
+							formatUser(thread.resolved_by_user)
+					$scope.reviewPanel.commentThreads = threads
+
+		refreshThreads()
 
 		formatComment = (comment) ->
 			comment.user = formatUser(user)
@@ -308,7 +332,3 @@ define [
 				hue: ColorManager.getHueForUserId(id)
 				avatar_text: [user.first_name, user.last_name].filter((n) -> n?).map((n) -> n[0]).join ""
 			}
-		
-		$scope.$watch "project.members", (members) ->
-			return if !members?
-			refreshUsers()

+ 18 - 1
services/web/test/UnitTests/coffee/Chat/ChatControllerTests.coffee

@@ -21,6 +21,9 @@ describe "ChatController", ->
 			"./ChatApiHandler": @ChatApiHandler
 			"../Editor/EditorRealTimeController": @EditorRealTimeController
 			'../Authentication/AuthenticationController': @AuthenticationController
+			'../User/UserInfoManager': @UserInfoManager = {}
+			'../User/UserInfoController': @UserInfoController = {}
+			'../Comments/CommentsController': @CommentsController = {}
 		@req =
 			params:
 				project_id: @project_id
@@ -32,9 +35,22 @@ describe "ChatController", ->
 		beforeEach ->
 			@req.body =
 				content: @content = "message-content"
-			@ChatApiHandler.sendGlobalMessage = sinon.stub().yields(null, @message = {"mock": "message"})
+			@UserInfoManager.getPersonalInfo = sinon.stub().yields(null, @user = {"unformatted": "user"})
+			@UserInfoController.formatPersonalInfo = sinon.stub().returns(@formatted_user = {"formatted": "user"})
+			@ChatApiHandler.sendGlobalMessage = sinon.stub().yields(null, @message = {"mock": "message", user_id: @user_id})
 			@ChatController.sendMessage @req, @res
 
+		it "should look up the user", ->
+			@UserInfoManager.getPersonalInfo
+				.calledWith(@user_id)
+				.should.equal true
+
+		it "should format and inject the user into the message", ->
+			@UserInfoController.formatPersonalInfo
+				.calledWith(@user)
+				.should.equal true
+			@message.user.should.deep.equal @formatted_user
+
 		it "should tell the chat handler about the message", ->
 			@ChatApiHandler.sendGlobalMessage
 				.calledWith(@project_id, @user_id, @content)
@@ -53,6 +69,7 @@ describe "ChatController", ->
 			@req.query =
 				limit: @limit = "30"
 				before: @before = "12345"
+			@CommentsController._injectUserInfoIntoThreads = sinon.stub().yields()
 			@ChatApiHandler.getGlobalMessages = sinon.stub().yields(null, @messages = ["mock", "messages"])
 			@ChatController.getMessages @req, @res
 

+ 149 - 5
services/web/test/UnitTests/coffee/Comments/CommentsControllerTests.coffee

@@ -21,6 +21,8 @@ describe "CommentsController", ->
 			"../Chat/ChatApiHandler": @ChatApiHandler
 			"../Editor/EditorRealTimeController": @EditorRealTimeController
 			'../Authentication/AuthenticationController': @AuthenticationController
+			'../User/UserInfoManager': @UserInfoManager = {}
+			'../User/UserInfoController': @UserInfoController = {}
 		@req = {}
 		@res =
 			json: sinon.stub()
@@ -29,12 +31,25 @@ describe "CommentsController", ->
 	describe "sendComment", ->
 		beforeEach ->
 			@req.params =
-				project_id: @project_id
-				thread_id: @thread_id
+				project_id: @project_id = "mock-project-id"
+				thread_id: @thread_id = "mock-thread-id"
 			@req.body =
 				content: @content = "message-content"
-			@ChatApiHandler.sendComment = sinon.stub().yields(null, @message = {"mock": "message"})
+			@UserInfoManager.getPersonalInfo = sinon.stub().yields(null, @user = {"unformatted": "user"})
+			@UserInfoController.formatPersonalInfo = sinon.stub().returns(@formatted_user = {"formatted": "user"})
+			@ChatApiHandler.sendComment = sinon.stub().yields(null, @message = {"mock": "message", user_id: @user_id})
 			@CommentsController.sendComment @req, @res
+		
+		it "should look up the user", ->
+			@UserInfoManager.getPersonalInfo
+				.calledWith(@user_id)
+				.should.equal true
+
+		it "should format and inject the user into the comment", ->
+			@UserInfoController.formatPersonalInfo
+				.calledWith(@user)
+				.should.equal true
+			@message.user.should.deep.equal @formatted_user
 
 		it "should tell the chat handler about the message", ->
 			@ChatApiHandler.sendComment
@@ -52,14 +67,143 @@ describe "CommentsController", ->
 	describe "getThreads", ->
 		beforeEach ->
 			@req.params =
-				project_id: @project_id
+				project_id: @project_id = "mock-project-id"
 			@ChatApiHandler.getThreads = sinon.stub().yields(null, @threads = {"mock", "threads"})
+			@CommentsController._injectUserInfoIntoThreads = sinon.stub().yields(null, @threads)
 			@CommentsController.getThreads @req, @res
 
 		it "should ask the chat handler about the request", ->
 			@ChatApiHandler.getThreads
 				.calledWith(@project_id)
 				.should.equal true
+			
+		it "should inject the user details into the threads", ->
+			@CommentsController._injectUserInfoIntoThreads
+				.calledWith(@threads)
+				.should.equal true
 
 		it "should return the messages", ->
-			@res.json.calledWith(@threads).should.equal true
+			@res.json.calledWith(@threads).should.equal true
+
+	describe "resolveThread", ->
+		beforeEach ->
+			@req.params =
+				project_id: @project_id = "mock-project-id"
+				thread_id: @thread_id = "mock-thread-id"
+			@ChatApiHandler.resolveThread = sinon.stub().yields()
+			@UserInfoManager.getPersonalInfo = sinon.stub().yields(null, @user = {"unformatted": "user"})
+			@UserInfoController.formatPersonalInfo = sinon.stub().returns(@formatted_user = {"formatted": "user"})
+			@CommentsController.resolveThread @req, @res
+
+		it "should ask the chat handler to resolve the thread", ->
+			@ChatApiHandler.resolveThread
+				.calledWith(@project_id, @thread_id)
+				.should.equal true
+			
+		it "should look up the user", ->
+			@UserInfoManager.getPersonalInfo
+				.calledWith(@user_id)
+				.should.equal true
+
+		it "should tell the client the comment was resolved", ->
+			@EditorRealTimeController.emitToRoom
+				.calledWith(@project_id, "resolve-thread", @thread_id, @formatted_user)
+				.should.equal true
+
+		it "should return a success code", ->
+			@res.send.calledWith(204).should.equal
+
+	describe "reopenThread", ->
+		beforeEach ->
+			@req.params =
+				project_id: @project_id = "mock-project-id"
+				thread_id: @thread_id = "mock-thread-id"
+			@ChatApiHandler.reopenThread = sinon.stub().yields()
+			@CommentsController.reopenThread @req, @res
+
+		it "should ask the chat handler to reopen the thread", ->
+			@ChatApiHandler.reopenThread
+				.calledWith(@project_id, @thread_id)
+				.should.equal true
+
+		it "should tell the client the comment was resolved", ->
+			@EditorRealTimeController.emitToRoom
+				.calledWith(@project_id, "reopen-thread", @thread_id)
+				.should.equal true
+
+		it "should return a success code", ->
+			@res.send.calledWith(204).should.equal
+
+	describe "_injectUserInfoIntoThreads", ->
+		beforeEach ->
+			@users = {
+				"user_id_1": {
+					"mock": "user_1"
+				}
+				"user_id_2": {
+					"mock": "user_2"
+				}
+			}
+			@UserInfoManager.getPersonalInfo = (user_id, callback) =>
+				return callback(null, @users[user_id])
+			sinon.spy @UserInfoManager, "getPersonalInfo"
+			@UserInfoController.formatPersonalInfo = (user) ->
+				return { "formatted": user["mock"] }
+			
+		it "should inject a user object into messaged and resolved data", (done) ->
+			@CommentsController._injectUserInfoIntoThreads [
+				{
+					resolved: true
+					resolved_by_user_id: "user_id_1"
+					messages: [{
+						user_id: "user_id_1"
+						content: "foo"
+					}, {
+						user_id: "user_id_2"
+						content: "bar"
+					}]
+				},
+				{
+					messages: [{
+						user_id: "user_id_1"
+						content: "baz"
+					}]
+				}
+			], (error, threads) ->
+				expect(threads).to.deep.equal [
+					{
+						resolved: true
+						resolved_by_user_id: "user_id_1"
+						resolved_by_user: { "formatted": "user_1" }
+						messages: [{
+							user_id: "user_id_1"
+							user: { "formatted": "user_1" }
+							content: "foo"
+						}, {
+							user_id: "user_id_2"
+							user: { "formatted": "user_2" }
+							content: "bar"
+						}]
+					},
+					{
+						messages: [{
+							user_id: "user_id_1"
+							user: { "formatted": "user_1" }
+							content: "baz"
+						}]
+					}
+				]
+				done()
+
+		it "should only need to look up each user once", (done) ->
+			@CommentsController._injectUserInfoIntoThreads [{
+				messages: [{
+					user_id: "user_id_1"
+					content: "foo"
+				}, {
+					user_id: "user_id_1"
+					content: "bar"
+				}]
+			}], (error, threads) =>
+				@UserInfoManager.getPersonalInfo.calledOnce.should.equal true
+				done()

+ 55 - 0
services/web/test/UnitTests/coffee/Ranges/RangesManagerTests.coffee

@@ -0,0 +1,55 @@
+should = require('chai').should()
+SandboxedModule = require('sandboxed-module')
+assert = require('assert')
+sinon = require('sinon')
+path = require "path"
+modulePath = path.join __dirname, "../../../../app/js/Features/Ranges/RangesManager"
+expect = require("chai").expect
+
+describe "RangesManager", ->
+	beforeEach ->
+		@RangesManager = SandboxedModule.require modulePath, requires:
+			"../DocumentUpdater/DocumentUpdaterHandler": @DocumentUpdaterHandler = {}
+			"../Docstore/DocstoreManager": @DocstoreManager = {}
+			"../User/UserInfoManager": @UserInfoManager = {}
+
+	describe "getAllRangesUsers", ->
+		beforeEach ->
+			@project_id = "mock-project-id"
+			@user_id1 = "mock-user-id-1"
+			@user_id1 = "mock-user-id-2"
+			@docs = [{
+				ranges:
+					changes: [{
+						op: { i: "foo", p: 42 }
+						metadata:
+							user_id: @user_id1
+					}, {
+						op: { i: "bar", p: 102 }
+						metadata:
+							user_id: @user_id2
+					}]
+			}, {
+				ranges:
+					changes: [{
+						op: { i: "baz", p: 3 }
+						metadata:
+							user_id: @user_id1
+					}]
+			}]
+			@users = {}
+			@users[@user_id1] = {"mock": "user-1"}
+			@users[@user_id2] = {"mock": "user-2"}
+			@UserInfoManager.getPersonalInfo = (user_id, callback) => callback null, @users[user_id]
+			sinon.spy @UserInfoManager, "getPersonalInfo"
+			@RangesManager.getAllRanges = sinon.stub().yields(null, @docs)
+
+		it "should return an array of unique users", (done) ->
+			@RangesManager.getAllRangesUsers @project_id, (error, users) =>
+				users.should.deep.equal [{"mock": "user-1"}, {"mock": "user-2"}]
+				done()
+
+		it "should only call getPersonalInfo once for each user", (done) ->
+			@RangesManager.getAllRangesUsers @project_id, (error, users) =>
+				@UserInfoManager.getPersonalInfo.calledTwice.should.equal true
+				done()

+ 12 - 13
services/web/test/UnitTests/coffee/User/UserInfoControllerTests.coffee

@@ -93,18 +93,18 @@ describe "UserInfoController", ->
 				first_name: @user.first_name
 				last_name: @user.last_name
 				email: @user.email
-			@UserInfoController._formatPersonalInfo = sinon.stub().callsArgWith(1, null, @formattedInfo)
+			@UserInfoController.formatPersonalInfo = sinon.stub().returns(@formattedInfo)
 			@UserInfoController.sendFormattedPersonalInfo @user, @res
 
 		it "should format the user details for the response", ->
-			@UserInfoController._formatPersonalInfo
+			@UserInfoController.formatPersonalInfo
 				.calledWith(@user)
 				.should.equal true
 
 		it "should send the formatted details back to the client", ->
 			@res.body.should.equal JSON.stringify(@formattedInfo)
 
-	describe "_formatPersonalInfo", ->
+	describe "formatPersonalInfo", ->
 		it "should return the correctly formatted data", ->
 			@user =
 				_id: ObjectId()
@@ -115,14 +115,13 @@ describe "UserInfoController", ->
 				signUpDate: new Date()
 				role:"student"
 				institution:"sheffield"
-			@UserInfoController._formatPersonalInfo @user, (error, info) =>
-				expect(info).to.deep.equal {
-					id: @user._id.toString()
-					first_name: @user.first_name
-					last_name: @user.last_name
-					email: @user.email
-					signUpDate: @user.signUpDate
-					role: @user.role
-					institution: @user.institution
-				}
+			expect(@UserInfoController.formatPersonalInfo(@user)).to.deep.equal {
+				id: @user._id.toString()
+				first_name: @user.first_name
+				last_name: @user.last_name
+				email: @user.email
+				signUpDate: @user.signUpDate
+				role: @user.role
+				institution: @user.institution
+			}