Send and get comments via the chat api

services/web/app/coffee/Features/Chat/ChatApiHandler.coffee

@@ -0,0 +1,48 @@
+request = require("request")
+settings = require("settings-sharelatex")
+logger = require("logger-sharelatex")
+
+module.exports = ChatApiHandler =
+	_apiRequest: (opts, callback = (error, data) ->) ->
+		request opts, (error, response, data) ->
+			return callback(error) if error?
+			if 200 <= response.statusCode < 300
+				return callback null, data
+			else
+				error = new Error("chat api returned non-success code: #{response.statusCode}")
+				error.statusCode = response.statusCode
+				logger.error {err: error, opts}, "error sending request to chat api"
+				return callback error
+
+	sendGlobalMessage: (project_id, user_id, content, callback)->
+		ChatApiHandler._apiRequest {
+			url:    "#{settings.apis.chat.internal_url}/project/#{project_id}/messages"
+			method: "POST"
+			json:   {user_id, content}
+		}, callback
+
+	getGlobalMessages: (project_id, limit, before, callback)->
+		qs = {}
+		qs.limit = limit if limit?
+		qs.before = before if before?
+		
+		ChatApiHandler._apiRequest {
+			url:    "#{settings.apis.chat.internal_url}/project/#{project_id}/messages"
+			method: "GET"
+			qs:     qs
+			json:   true
+		}, callback
+	
+	sendComment: (project_id, thread_id, user_id, content, callback = (error) ->) ->
+		ChatApiHandler._apiRequest {
+			url:    "#{settings.apis.chat.internal_url}/project/#{project_id}/thread/#{thread_id}/messages"
+			method: "POST"
+			json:   {user_id, content}
+		}, callback
+	
+	getThreads: (project_id, callback = (error) ->) ->
+		ChatApiHandler._apiRequest {
+			url:    "#{settings.apis.chat.internal_url}/project/#{project_id}/threads"
+			method: "GET"
+			json:   true
+		}, callback

services/web/app/coffee/Features/Chat/ChatController.coffee

@@ -1,33 +1,26 @@
-ChatHandler = require("./ChatHandler")
+ChatApiHandler = require("./ChatApiHandler")
 EditorRealTimeController = require("../Editor/EditorRealTimeController")
 logger = require("logger-sharelatex")
 AuthenticationController = require('../Authentication/AuthenticationController')
 
 module.exports =
-
-
 	sendMessage: (req, res, next)->
-		project_id = req.params.Project_id
+		project_id = req.params.project_id
-		messageContent = req.body.content
+		content = req.body.content
 		user_id = AuthenticationController.getLoggedInUserId(req)
 		if !user_id?
 			err = new Error('no logged-in user')
 			return next(err)
-		ChatHandler.sendMessage project_id, user_id, messageContent, (err, builtMessge)->
+		ChatApiHandler.sendGlobalMessage project_id, user_id, content, (err, message) ->
-			if err?
+			return next(err) if err?
-				logger.err err:err, project_id:project_id, user_id:user_id, messageContent:messageContent, "problem sending message to chat api"
+			EditorRealTimeController.emitToRoom project_id, "new-chat-message", message, (err)->
-				return res.sendStatus(500)
+			res.send(204)
-			EditorRealTimeController.emitToRoom project_id,  "new-chat-message", builtMessge, (err)->
-			res.send()
 
-	getMessages: (req, res)->
+	getMessages: (req, res, next)->
-		project_id = req.params.Project_id
+		project_id = req.params.project_id
 		query = req.query
 		logger.log project_id:project_id, query:query, "getting messages"
-		ChatHandler.getMessages project_id, query, (err, messages)->
+		ChatApiHandler.getGlobalMessages project_id, query.limit, query.before, (err, messages) ->
-			if err?
+			return next(err) if err?
-				logger.err err:err, query:query, "problem getting messages from chat api"
+			logger.log length: messages?.length, "sending messages to client"
-				return res.sendStatus 500
+			res.json messages
-			logger.log length:messages?.length, "sending messages to client"
-			res.set 'Content-Type', 'application/json'
-			res.send messages

services/web/app/coffee/Features/Chat/ChatHandler.coffee

@@ -1,32 +0,0 @@
-request = require("request")
-settings = require("settings-sharelatex")
-logger = require("logger-sharelatex")
-
-module.exports = 
-
-	sendMessage: (project_id, user_id, messageContent, callback)->
-		opts =
-			method:"post"
-			json:
-				content:messageContent
-				user_id:user_id
-			uri:"#{settings.apis.chat.internal_url}/room/#{project_id}/messages"
-		request opts, (err, response, body)->
-			if err?
-				logger.err err:err, "problem sending new message to chat"
-			callback(err, body)
-
-
-
-	getMessages: (project_id, query, callback)->
-		qs = {}
-		qs.limit = query.limit if query?.limit?
-		qs.before = query.before if query?.before?
-
-		opts =
-			uri:"#{settings.apis.chat.internal_url}/room/#{project_id}/messages"
-			method:"get"
-			qs: qs
-
-		request opts, (err, response, body)->
-			callback(err, body)

services/web/app/coffee/Features/Comments/CommentsController.coffee

@@ -0,0 +1,25 @@
+ChatApiHandler = require("../Chat/ChatApiHandler")
+EditorRealTimeController = require("../Editor/EditorRealTimeController")
+logger = require("logger-sharelatex")
+AuthenticationController = require('../Authentication/AuthenticationController')
+
+module.exports = CommentsController =
+	sendComment: (req, res, next) ->
+		{project_id, thread_id} = req.params
+		content = req.body.content
+		user_id = AuthenticationController.getLoggedInUserId(req)
+		if !user_id?
+			err = new Error('no logged-in user')
+			return next(err)
+		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
+
+	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

services/web/app/coffee/router.coffee

@@ -41,6 +41,7 @@ BetaProgramController = require('./Features/BetaProgram/BetaProgramController')
 AnalyticsRouter = require('./Features/Analytics/AnalyticsRouter')
 AnnouncementsController = require("./Features/Announcements/AnnouncementsController")
 RangesController = require("./Features/Ranges/RangesController")
+CommentsController = require "./Features/Comments/CommentsController"
 
 logger = require("logger-sharelatex")
 _ = require("underscore")
@@ -226,8 +227,11 @@ module.exports = class Router
 		webRouter.post "/spelling/check", AuthenticationController.requireLogin(), SpellingController.proxyRequestToSpellingApi
 		webRouter.post "/spelling/learn", AuthenticationController.requireLogin(), SpellingController.proxyRequestToSpellingApi
 
-		webRouter.get  "/project/:Project_id/messages", AuthorizationMiddlewear.ensureUserCanReadProject, ChatController.getMessages
+		webRouter.get  "/project/:project_id/messages", AuthorizationMiddlewear.ensureUserCanReadProject, ChatController.getMessages
-		webRouter.post "/project/:Project_id/messages", AuthorizationMiddlewear.ensureUserCanReadProject, ChatController.sendMessage
+		webRouter.post "/project/:project_id/messages", AuthorizationMiddlewear.ensureUserCanReadProject, ChatController.sendMessage
+		
+		webRouter.post "/project/:project_id/thread/:thread_id/messages", AuthorizationMiddlewear.ensureUserCanWriteProjectContent, CommentsController.sendComment
+		webRouter.get  "/project/:project_id/threads", AuthorizationMiddlewear.ensureUserCanReadProject, CommentsController.getThreads
 
 		webRouter.post "/project/:Project_id/references/index", AuthorizationMiddlewear.ensureUserCanReadProject, ReferencesController.index
 		webRouter.post "/project/:Project_id/references/indexAll", AuthorizationMiddlewear.ensureUserCanReadProject, ReferencesController.indexAll

services/web/app/views/project/editor/review-panel.jade

@@ -28,7 +28,7 @@
 				div(ng-if="entry.type === 'comment'")
 					comment-entry(
 						entry="entry"
-						users="users"
+						threads="reviewPanel.commentThreads"
 						on-resolve="resolveComment(entry, entry_id)"
 						on-unresolve="unresolveComment(entry_id)"
 						on-show-thread="showThread(entry)"
@@ -150,19 +150,19 @@ script(type='text/ng-template', id='commentEntryTemplate')
 		)
 			.rp-comment(
 				ng-if="!entry.resolved || entry.showWhenResolved"
-				ng-repeat="comment in entry.thread"
+				ng-repeat="comment in threads[entry.thread_id]"
-				ng-class="users[comment.user_id].isSelf ? 'rp-comment-self' : '';"
+				ng-class="comment.user.isSelf ? 'rp-comment-self' : '';"
 			)
 				.rp-avatar(
-					ng-if="!users[comment.user_id].isSelf;"
+					ng-if="!comment.user.isSelf;"
-					style="background-color: hsl({{ users[comment.user_id].hue }}, 70%, 50%);"
+					style="background-color: hsl({{ comment.user.hue }}, 70%, 50%);"
-				) {{ users[comment.user_id].avatar_text | limitTo : 1 }}
+				) {{ comment.user.avatar_text | limitTo : 1 }}
-				.rp-comment-body(style="color: hsl({{ users[comment.user_id].hue }}, 70%, 90%);")
+				.rp-comment-body(style="color: hsl({{ comment.user.hue }}, 70%, 90%);")
 					p.rp-comment-content {{ comment.content }}
 					p.rp-comment-metadata
-						| {{ comment.ts | date : 'MMM d, y h:mm a' }}
+						| {{ comment.timestamp | date : 'MMM d, y h:mm a' }}
 						|  • 
-						span(style="color: hsl({{ users[comment.user_id].hue }}, 70%, 40%);") {{ users[comment.user_id].name }}
+						span(style="color: hsl({{ comment.user.hue }}, 70%, 40%);") {{ comment.user.name }}
 			.rp-comment-reply(ng-if="!entry.resolved || entry.showWhenResolved")
 				textarea.rp-comment-input(
 					ng-model="entry.replyContent"

services/web/public/coffee/ide/editor/Document.coffee

@@ -339,8 +339,6 @@ define [
 				track_changes_as = msg.meta.user_id
 			else if !remote_op and @track_changes_as?
 				track_changes_as = @track_changes_as
-			console.log "CHANGED", oldSnapshot, ops, track_changes_as
 			@ranges.track_changes = track_changes_as?
 			for op in ops
-				console.log "APPLYING OP", op, @ranges.track_changes 
 				@ranges.applyOp op, { user_id: track_changes_as }

services/web/public/coffee/ide/editor/directives/aceEditor/track-changes/TrackChangesManager.coffee

@@ -20,8 +20,8 @@ define [
 				@rangesTracker = doc.ranges
 				@connectToRangesTracker()
 			
-			@$scope.$on "comment:add", (e) =>
+			@$scope.$on "comment:add", (e, thread_id) =>
-				@addCommentToSelection()
+				@addCommentToSelection(thread_id)
 
 			@$scope.$on "comment:select_line", (e) =>
 				@selectLineIfNoSelection()
@@ -146,16 +146,16 @@ define [
 			for comment in @rangesTracker.comments
 				@_onCommentAdded(comment)
 
-		addComment: (offset, content) ->
+		addComment: (offset, content, thread_id) ->
-			op = { c: content, p: offset }
+			op = { c: content, p: offset, t: thread_id }
 			# @rangesTracker.applyOp op # Will apply via sharejs
 			@$scope.sharejsDoc.submitOp op
 		
-		addCommentToSelection: () ->
+		addCommentToSelection: (thread_id) ->
 			range = @editor.getSelectionRange()
 			content = @editor.getSelectedText()
 			offset = @_aceRangeToShareJs(range.start)
-			@addComment(offset, content)
+			@addComment(offset, content, thread_id)
 		
 		selectLineIfNoSelection: () ->
 			if @editor.selection.isEmpty()

services/web/public/coffee/ide/review-panel/RangesTracker.coffee

@@ -35,18 +35,25 @@ load = (EventEmitter) ->
 		#   * Inserts by another user will not combine with inserts by the first user. If they are in the
 		#     middle of a previous insert by the first user, the original insert will be split into two.
 		constructor: (@changes = [], @comments = []) ->
-			# Change objects have the following structure:
+
-			# {
+		@_increment: 0
-			#   id: ... # Uniquely generated by us
+		@newId: () ->
-			#   op: { # ShareJs style op tracking the offset (p) and content inserted (i) or deleted (d)
+			# Generate a Mongo ObjectId
-			#     i: "..."
+			# Reference: https://github.com/dreampulse/ObjectId.js/blob/master/src/main/javascript/Objectid.js
-			#     p: 42
+			@_pid ?= Math.floor(Math.random() * (32767))
-			#   }
+			@_machine ?= Math.floor(Math.random() * (16777216))
-			# }
+			timestamp = Math.floor(new Date().valueOf() / 1000)
-			#
+			@_increment++
-			# Ids are used to uniquely identify a change, e.g. for updating it in the database, or keeping in
+			
-			# sync with Ace ranges.
+			timestamp = timestamp.toString(16)
-			@id = 0
+			machine = @_machine.toString(16)
+			pid = @_pid.toString(16)
+			increment = @_increment.toString(16)
+			
+			return '00000000'.substr(0, 8 - timestamp.length) + timestamp +
+				'000000'.substr(0, 6 - machine.length) + machine +
+				'0000'.substr(0, 4 - pid.length) + pid +
+				'000000'.substr(0, 6 - increment.length) + increment;
 		
 		getComment: (comment_id) ->
 			comment = null
@@ -105,7 +112,7 @@ load = (EventEmitter) ->
 		addComment: (op, metadata) ->
 			# TODO: Don't allow overlapping comments?
 			@comments.push comment = {
-				id: @_newId()
+				id: RangesTracker.newId()
 				op: # Copy because we'll modify in place
 					c: op.c
 					p: op.p
@@ -394,28 +401,9 @@ load = (EventEmitter) ->
 			if moved_changes.length > 0
 				@emit "changes:moved", moved_changes
 
-		_newId: () ->
-			# Generate a Mongo ObjectId
-			# Reference: https://github.com/dreampulse/ObjectId.js/blob/master/src/main/javascript/Objectid.js
-			@_pid ?= Math.floor(Math.random() * (32767))
-			@_machine ?= Math.floor(Math.random() * (16777216))
-			timestamp = Math.floor(new Date().valueOf() / 1000)
-			@_increment ?= 0
-			@_increment++
-			
-			timestamp = timestamp.toString(16)
-			machine = @_machine.toString(16)
-			pid = @_pid.toString(16)
-			increment = @_increment.toString(16)
-			
-			return '00000000'.substr(0, 8 - timestamp.length) + timestamp +
-				'000000'.substr(0, 6 - machine.length) + machine +
-				'0000'.substr(0, 4 - pid.length) + pid +
-				'000000'.substr(0, 6 - increment.length) + increment;
-
 		_addOp: (op, metadata) ->
 			change = {
-				id: @_newId()
+				id: RangesTracker.newId()
 				op: op
 				metadata: metadata
 			}

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

@@ -18,12 +18,27 @@ define [
 			openSubView: $scope.SubViews.CUR_FILE
 			overview:
 				loading: false
+			commentThreads: {}
 
 		$scope.commentState =
 			adding: false
 			content: ""
 
 		$scope.reviewPanelEventsBridge = new EventEmitter()
+		
+		$http.get "/project/#{$scope.project_id}/threads"
+			.success (threads) ->
+				for thread_id, comments of threads
+					for comment in comments
+						formatComment(comment)
+				$scope.reviewPanel.commentThreads = threads
+		
+		ide.socket.on "new-comment", (thread_id, comment) ->
+			$scope.reviewPanel.commentThreads[thread_id] ?= []
+			$scope.reviewPanel.commentThreads[thread_id].push(formatComment(comment))
+			$scope.$apply()
+			$timeout () ->
+				$scope.$broadcast "review-panel:layout"
 
 		rangesTrackers = {}
 
@@ -35,95 +50,6 @@ define [
 			rangesTrackers[doc_id] ?= new RangesTracker()
 			return rangesTrackers[doc_id]
 
-		# TODO Just for prototyping purposes; remove afterwards.
-		mockedUserId = 'mock_user_id_1'
-		mockedUserId2 = 'mock_user_id_2'
-
-		if window.location.search.match /mocktc=true/
-			mock_changes = {
-				"main.tex":
-					changes: [{
-						op: { i: "Habitat loss and conflicts with humans are the greatest causes of concern.", p: 925 - 38 }
-						metadata: { user_id: mockedUserId, ts: new Date(Date.now() - 30 * 60 * 1000) }
-					}, {
-						op: { d: "The lion is now a vulnerable species. ", p: 778 }
-						metadata: { user_id: mockedUserId, ts: new Date(Date.now() - 31 * 60 * 1000) }
-					}]
-					comments: [{
-						offset: 1375 - 38
-						length: 79
-						metadata:
-							thread: [{
-								content: "Do we have a source for this?"
-								user_id: mockedUserId
-								ts: new Date(Date.now() - 45 * 60 * 1000) 
-							}]
-					}]
-				"chapter_1.tex":
-					changes: [{
-						"op":{"p":740,"d":", to take down large animals"},
-						"metadata":{"user_id":mockedUserId, ts: new Date(Date.now() - 15 * 60 * 1000)}
-					}, {
-						"op":{"i":", to keep hold of the prey","p":920},
-						"metadata":{"user_id":mockedUserId, ts: new Date(Date.now() - 130 * 60 * 1000)}
-					}, {
-						"op":{"i":" being","p":1057},
-						"metadata":{"user_id":mockedUserId2, ts: new Date(Date.now() - 72 * 60 * 1000)}
-					}]
-					comments:[{
-						"offset":111,"length":5,
-						"metadata":{
-							"thread": [
-								{"content":"Have we used 'pride' too much here?","user_id":mockedUserId, ts: new Date(Date.now() - 12 * 60 * 1000)},
-								{"content":"No, I think this is OK","user_id":mockedUserId2, ts: new Date(Date.now() - 9 * 60 * 1000)}
-							]
-						}
-					},{
-						"offset":452,"length":21,
-						"metadata":{
-							"thread":[
-								{"content":"TODO: Don't use as many parentheses!","user_id":mockedUserId2, ts: new Date(Date.now() - 99 * 60 * 1000)}
-							]
-						}
-					}]
-				"chapter_2.tex":
-					changes: [{
-						"op":{"p":458,"d":"other"},
-						"metadata":{"user_id":mockedUserId, ts: new Date(Date.now() - 133 * 60 * 1000)}
-					},{
-						"op":{"i":"usually 2-3, ","p":928},
-						"metadata":{"user_id":mockedUserId, ts: new Date(Date.now() - 27 * 60 * 1000)}
-					},{
-						"op":{"i":"If the parents are a male lion and a female tiger, it is called a liger. A tigon comes from a male tiger and a female lion.","p":1126},
-						"metadata":{"user_id":mockedUserId, ts: new Date(Date.now() - 152 * 60 * 1000)}
-					}]
-					comments: [{
-						"offset":299,"length":10,
-						"metadata":{
-							"thread":[{
-								"content":"Should we use a different word here if 'den' needs clarifying?","user_id":mockedUserId,"ts": new Date(Date.now() - 430 * 60 * 1000)
-							}]
-						}
-					},{
-						"offset":843,"length":66,
-						"metadata":{
-							"thread":[{
-								"content":"This sentence is a little ambiguous","user_id":mockedUserId,"ts": new Date(Date.now() - 430 * 60 * 1000)
-							}]
-						}
-					}]
-			}
-			ide.$scope.$on "file-tree:initialized", () ->
-				ide.fileTreeManager.forEachEntity (entity) ->
-					if mock_changes[entity.name]?
-						rangesTracker = getChangeTracker(entity.id)
-						for change in mock_changes[entity.name].changes
-							rangesTracker._addOp change.op, change.metadata 
-						for comment in mock_changes[entity.name].comments
-							rangesTracker.addComment comment.offset, comment.length, comment.metadata 
-						for doc_id, rangesTracker of rangesTrackers
-							updateEntries(doc_id)
-
 		scrollbar = {}
 		$scope.reviewPanelEventsBridge.on "aceScrollbarVisibilityChanged", (isVisible, scrollbarWidth) ->
 			scrollbar = {isVisible, scrollbarWidth}
@@ -156,7 +82,6 @@ define [
 
 		$scope.$watch "editor.sharejs_doc", (doc) ->
 			return if !doc?
-			console.log "DOC changed", doc
 			# The open doc range tracker is kept up to date in real-time so
 			# replace any outdated info with this
 			rangesTrackers[doc.doc_id] = doc.ranges
@@ -218,7 +143,7 @@ define [
 				entries[comment.id] ?= {}
 				new_entry = {
 					type: "comment"
-					thread: comment.metadata.thread or []
+					thread_id: comment.op.t
 					resolved: comment.metadata?.resolved
 					resolved_data: comment.metadata?.resolved_data
 					content: comment.op.c
@@ -250,7 +175,7 @@ define [
 			
 			for id, entry of entries
 				if entry.type == "comment" and not entry.resolved
-					entry.focused = (entry.offset <= cursor_offset <= entry.offset + entry.length)
+					entry.focused = (entry.offset <= cursor_offset <= entry.offset + entry.content.length)
 				else if entry.type == "insert"
 					entry.focused = (entry.offset <= cursor_offset <= entry.offset + entry.content.length)
 				else if entry.type == "delete"
@@ -268,21 +193,20 @@ define [
 			$scope.$broadcast "change:reject", entry_id
 		
 		$scope.startNewComment = () ->
-			# $scope.commentState.adding = true
 			$scope.$broadcast "comment:select_line"
 			$timeout () ->
 				$scope.$broadcast "review-panel:layout"
 		
 		$scope.submitNewComment = (content) ->
-			# $scope.commentState.adding = false
+			thread_id = RangesTracker.newId()
-			$scope.$broadcast "comment:add", content
+			$scope.$broadcast "comment:add", thread_id
-			# $scope.commentState.content = ""
+			$http.post("/project/#{$scope.project_id}/thread/#{thread_id}/messages", {content, _csrf: window.csrfToken})
+				.error (error) ->
+					ide.showGenericMessageModal("Error submitting comment", "Sorry, there was a problem submitting your comment")
 			$timeout () ->
 				$scope.$broadcast "review-panel:layout"
 		
 		$scope.cancelNewComment = (entry) ->
-			# $scope.commentState.adding = false
-			# $scope.commentState.content = ""
 			$timeout () ->
 				$scope.$broadcast "review-panel:layout"
 		
@@ -291,40 +215,19 @@ define [
 			$timeout () ->
 				$scope.$broadcast "review-panel:layout"
 
-		# $scope.handleCommentReplyKeyPress = (ev, entry) ->
-		# 	if ev.keyCode == 13 and !ev.shiftKey and !ev.ctrlKey and !ev.metaKey
-		# 		ev.preventDefault()
-		# 		ev.target.blur()
-		# 		$scope.submitReply(entry)
-
 		$scope.submitReply = (entry, entry_id) ->
 			$scope.unresolveComment(entry_id)
-			entry.thread.push {
+			thread_id = entry.thread_id
-				content: entry.replyContent
+			content   = entry.replyContent
-				ts: new Date()
+			$http.post("/project/#{$scope.project_id}/thread/#{thread_id}/messages", {content, _csrf: window.csrfToken})
-				user_id: window.user_id
+				.error (error) ->
-			}
+					ide.showGenericMessageModal("Error submitting comment", "Sorry, there was a problem submitting your comment")
-			entry.replyContent = ""
-			entry.replying = false
-			$timeout () ->
-				$scope.$broadcast "review-panel:layout"
-			# TODO Just for prototyping purposes; remove afterwards
-			window.setTimeout((() -> 
-				$scope.$applyAsync(() -> submitMockedReply(entry))
-			), 1000 * 2)
 
-		# TODO Just for prototyping purposes; remove afterwards.
-		submitMockedReply = (entry) ->
-			entry.thread.push {
-				content: 'Sounds good!'
-				ts: new Date()
-				user_id: mockedUserId
-			}
 			entry.replyContent = ""
 			entry.replying = false
 			$timeout () ->
 				$scope.$broadcast "review-panel:layout"
-		
+
 		$scope.cancelReply = (entry) ->
 			entry.replying = false
 			entry.replyContent = ""
@@ -361,37 +264,39 @@ define [
 		# when we get an id we don't know. This'll do for client side testing
 		refreshUsers = () ->
 			$scope.users = {}
-			# TODO Just for prototyping purposes; remove afterwards.
-			$scope.users[mockedUserId] = {
-				email: "paulo@sharelatex.com"
-				name: "Paulo Reis"
-				isSelf: false
-				hue: 70
-				avatar_text: "PR"
-			}
-			$scope.users[mockedUserId2] = {
-				email: "james@sharelatex.com"
-				name: "James Allen"
-				isSelf: false
-				hue: 320
-				avatar_text: "JA"
-			}
-
 			for member in $scope.project.members.concat($scope.project.owner)
-				if member._id == window.user_id
+				$scope.users[member._id] = formatUser(member)
-					name = "You"
-					isSelf = true
-				else
-					name = "#{member.first_name} #{member.last_name}"
-					isSelf = false
 
-				$scope.users[member._id] = {
+		formatComment = (comment) ->
-					email: member.email
+			comment.user = formatUser(user)
-					name: name
+			comment.timestamp = new Date(comment.timestamp)
-					isSelf: isSelf
+			return comment
-					hue: ColorManager.getHueForUserId(member._id)
+
-					avatar_text: [member.first_name, member.last_name].filter((n) -> n?).map((n) -> n[0]).join ""
+		formatUser = (user) ->
+			if !user?
+				return {
+					email: null
+					name: "Anonymous"
+					isSelf: false
+					hue: ColorManager.ANONYMOUS_HUE
+					avatar_text: "A"
 				}
+
+			id = user._id or user.id
+			if id == window.user_id
+				name = "You"
+				isSelf = true
+			else
+				name = "#{user.first_name} #{user.last_name}"
+				isSelf = false
+			return {
+				id: id
+				email: user.email
+				name: name
+				isSelf: isSelf
+				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?

services/web/public/coffee/ide/review-panel/directives/commentEntry.coffee

@@ -6,7 +6,7 @@ define [
 		templateUrl: "commentEntryTemplate"
 		scope: 
 			entry: "="
-			users: "="
+			threads: "="
 			onResolve: "&"
 			onReply: "&"
 			onIndicatorClick: "&"

services/web/test/UnitTests/coffee/Chat/ChatApiHandlerTests.coffee

@@ -0,0 +1,92 @@
+should = require('chai').should()
+SandboxedModule = require('sandboxed-module')
+assert = require('assert')
+path = require('path')
+sinon = require('sinon')
+modulePath = path.join __dirname, "../../../../app/js/Features/Chat/ChatApiHandler"
+expect = require("chai").expect
+
+describe "ChatApiHandler", ->
+	beforeEach ->
+		@settings = 
+			apis:
+				chat:
+					internal_url:"chat.sharelatex.env"
+		@request = sinon.stub()
+		@ChatApiHandler = SandboxedModule.require modulePath, requires:
+			"settings-sharelatex": @settings
+			"logger-sharelatex": { log: sinon.stub(), error: sinon.stub() }
+			"request": @request
+		@project_id = "3213213kl12j"
+		@user_id = "2k3jlkjs9"
+		@content = "my message here"
+		@callback = sinon.stub()
+
+	describe "sendGlobalMessage", ->
+		describe "successfully", ->
+			beforeEach ->
+				@message = { "mock": "message" }
+				@request.callsArgWith(1, null, {statusCode: 200}, @message)
+				@ChatApiHandler.sendGlobalMessage @project_id, @user_id, @content, @callback
+				
+			it "should post the data to the chat api", ->
+				@request.calledWith({
+					url: "#{@settings.apis.chat.internal_url}/project/#{@project_id}/messages"
+					method: "POST"
+					json:
+						content: @content
+						user_id: @user_id
+				}).should.equal true
+
+			it "should return the message from the post", ->
+				@callback.calledWith(null, @message).should.equal true
+	
+		describe "with a non-success status code", ->
+			beforeEach ->
+				@request.callsArgWith(1, null, {statusCode: 500})
+				@ChatApiHandler.sendGlobalMessage @project_id, @user_id, @content, @callback
+			
+			it "should return an error", ->
+				error = new Error()
+				error.statusCode = 500
+				@callback.calledWith(error).should.equal true
+
+	describe "getGlobalMessages", ->
+		beforeEach ->
+			@messages = [{ "mock": "message" }]
+			@limit = 30
+			@before = "1234"
+
+		describe "successfully", ->
+			beforeEach ->
+				@request.callsArgWith(1, null, {statusCode: 200}, @messages)
+				@ChatApiHandler.getGlobalMessages @project_id, @limit, @before, @callback
+	
+			it "should make get request for room to chat api", ->
+				@request.calledWith({
+					method: "GET"
+					url: "#{@settings.apis.chat.internal_url}/project/#{@project_id}/messages"
+					qs:
+						limit: @limit
+						before: @before
+					json: true
+				}).should.equal true
+	
+			it "should return the messages from the request", ->
+				@callback.calledWith(null, @messages).should.equal true
+
+		describe "with failure error code", ->
+			beforeEach ->
+				@request.callsArgWith(1, null, {statusCode: 500}, null)
+				@ChatApiHandler.getGlobalMessages @project_id, @limit, @before, @callback
+			
+			it "should return an error", ->
+				error = new Error()
+				error.statusCode = 500
+				@callback.calledWith(error).should.equal true
+
+
+
+
+
+

services/web/test/UnitTests/coffee/Chat/ChatControllerTests.coffee

@@ -7,75 +7,59 @@ modulePath = path.join __dirname, "../../../../app/js/Features/Chat/ChatControll
 expect = require("chai").expect
 
 describe "ChatController", ->
-
 	beforeEach ->
-
+		@user_id = 'mock-user-id'
-		@user_id = 'ier_'
 		@settings = {}
-		@ChatHandler =
+		@ChatApiHandler = {}
-			sendMessage:sinon.stub()
-			getMessages:sinon.stub()
-
 		@EditorRealTimeController =
 			emitToRoom:sinon.stub().callsArgWith(3)
-
 		@AuthenticationController =
 			getLoggedInUserId: sinon.stub().returns(@user_id)
 		@ChatController = SandboxedModule.require modulePath, requires:
-			"settings-sharelatex":@settings
+			"settings-sharelatex": @settings
-			"logger-sharelatex": log:->
+			"logger-sharelatex": log: ->
-			"./ChatHandler":@ChatHandler
+			"./ChatApiHandler": @ChatApiHandler
-			"../Editor/EditorRealTimeController":@EditorRealTimeController
+			"../Editor/EditorRealTimeController": @EditorRealTimeController
 			'../Authentication/AuthenticationController': @AuthenticationController
-		@query =
-			before:"some time"
-
 		@req =
 			params:
-				Project_id:@project_id
+				project_id: @project_id
-			session:
-				user:
-					_id:@user_id
-			body:
-				content:@messageContent
 		@res =
-			set:sinon.stub()
+			json: sinon.stub()
+			send: sinon.stub()
 
 	describe "sendMessage", ->
-
+		beforeEach ->
-		it "should tell the chat handler about the message", (done)->
+			@req.body =
-			@ChatHandler.sendMessage.callsArgWith(3)
+				content: @content = "message-content"
-			@res.send = =>
+			@ChatApiHandler.sendGlobalMessage = sinon.stub().yields(null, @message = {"mock": "message"})
-				@ChatHandler.sendMessage.calledWith(@project_id, @user_id, @messageContent).should.equal true
-				done()
 			@ChatController.sendMessage @req, @res
 
-		it "should tell the editor real time controller about the update with the data from the chat handler", (done)->
+		it "should tell the chat handler about the message", ->
-			@chatMessage =
+			@ChatApiHandler.sendGlobalMessage
-				content:"hello world"
+				.calledWith(@project_id, @user_id, @content)
-			@ChatHandler.sendMessage.callsArgWith(3, null, @chatMessage)
+				.should.equal true
-			@res.send = =>
+
-				@EditorRealTimeController.emitToRoom.calledWith(@project_id, "new-chat-message", @chatMessage).should.equal true
+		it "should tell the editor real time controller about the update with the data from the chat handler", ->
-				done()
+			@EditorRealTimeController.emitToRoom
-			@ChatController.sendMessage @req, @res
+				.calledWith(@project_id, "new-chat-message", @message)
+				.should.equal true
+				
+		it "should return a 204 status code", ->
+			@res.send.calledWith(204).should.equal true
 
 	describe "getMessages", ->
 		beforeEach ->
-			@req.query = @query
+			@req.query =
-
+				limit: @limit = "30"
-		it "should ask the chat handler about the request", (done)->
+				before: @before = "12345"
-
+			@ChatApiHandler.getGlobalMessages = sinon.stub().yields(null, @messages = ["mock", "messages"])
-			@ChatHandler.getMessages.callsArgWith(2)
-			@res.send = =>
-				@ChatHandler.getMessages.calledWith(@project_id, @query).should.equal true
-				done()
 			@ChatController.getMessages @req, @res
 
-		it "should return the messages", (done)->
+		it "should ask the chat handler about the request", ->
-			messages = [{content:"hello"}]
+			@ChatApiHandler.getGlobalMessages
-			@ChatHandler.getMessages.callsArgWith(2, null, messages)
+				.calledWith(@project_id, @limit, @before)
-			@res.send = (sentMessages)=>
+				.should.equal true
-				@res.set.calledWith('Content-Type', 'application/json').should.equal true
+
-				sentMessages.should.deep.equal messages
+		it "should return the messages", ->
-				done()
+			@res.json.calledWith(@messages).should.equal true
-			@ChatController.getMessages @req, @res

services/web/test/UnitTests/coffee/Chat/ChatHandlerTests.coffee

@@ -1,89 +0,0 @@
-should = require('chai').should()
-SandboxedModule = require('sandboxed-module')
-assert = require('assert')
-path = require('path')
-sinon = require('sinon')
-modulePath = path.join __dirname, "../../../../app/js/Features/Chat/ChatHandler"
-expect = require("chai").expect
-
-describe "ChatHandler", ->
-
-	beforeEach ->
-
-		@settings = 
-			apis:
-				chat:
-					internal_url:"chat.sharelatex.env"
-		@request = sinon.stub()
-		@ChatHandler = SandboxedModule.require modulePath, requires:
-			"settings-sharelatex":@settings
-			"logger-sharelatex": log:->
-			"request": @request
-		@project_id = "3213213kl12j"
-		@user_id = "2k3jlkjs9"
-		@messageContent = "my message here"
-
-	describe "sending message", ->
-
-		beforeEach ->
-			@messageResponse = 
-				message:"Details"
-			@request.callsArgWith(1, null, null, @messageResponse)
-
-		it "should post the data to the chat api", (done)->
-
-			@ChatHandler.sendMessage @project_id, @user_id, @messageContent, (err)=>
-				@opts =
-					method:"post"
-					json:
-						content:@messageContent
-						user_id:@user_id
-					uri:"#{@settings.apis.chat.internal_url}/room/#{@project_id}/messages"
-				@request.calledWith(@opts).should.equal true
-				done()
-
-		it "should return the message from the post", (done)->			
-			@ChatHandler.sendMessage @project_id, @user_id, @messageContent, (err, returnedMessage)=>
-				returnedMessage.should.equal @messageResponse
-				done()
-
-	describe "get messages", ->
-
-		beforeEach ->
-			@returnedMessages = [{content:"hello world"}]
-			@request.callsArgWith(1, null, null, @returnedMessages)
-			@query = {}
-
-		it "should make get request for room to chat api", (done)->
-
-			@ChatHandler.getMessages @project_id, @query, (err)=>
-				@opts =
-					method:"get"
-					uri:"#{@settings.apis.chat.internal_url}/room/#{@project_id}/messages"
-					qs:{}
-				@request.calledWith(@opts).should.equal true
-				done()
-
-		it "should make get request for room to chat api with query string", (done)->
-			@query = {limit:5, before:12345, ignore:"this"}
-
-			@ChatHandler.getMessages @project_id, @query, (err)=>
-				@opts =
-					method:"get"
-					uri:"#{@settings.apis.chat.internal_url}/room/#{@project_id}/messages"
-					qs:
-						limit:5
-						before:12345
-				@request.calledWith(@opts).should.equal true
-				done()
-
-		it "should return the messages from the request", (done)->
-			@ChatHandler.getMessages @project_id, @query, (err, returnedMessages)=>
-				returnedMessages.should.equal @returnedMessages
-				done()
-
-
-
-
-
-

services/web/test/UnitTests/coffee/Comments/CommentsControllerTests.coffee

@@ -0,0 +1,65 @@
+should = require('chai').should()
+SandboxedModule = require('sandboxed-module')
+assert = require('assert')
+path = require('path')
+sinon = require('sinon')
+modulePath = path.join __dirname, "../../../../app/js/Features/Comments/CommentsController"
+expect = require("chai").expect
+
+describe "CommentsController", ->
+	beforeEach ->
+		@user_id = 'mock-user-id'
+		@settings = {}
+		@ChatApiHandler = {}
+		@EditorRealTimeController =
+			emitToRoom:sinon.stub()
+		@AuthenticationController =
+			getLoggedInUserId: sinon.stub().returns(@user_id)
+		@CommentsController = SandboxedModule.require modulePath, requires:
+			"settings-sharelatex": @settings
+			"logger-sharelatex": log: ->
+			"../Chat/ChatApiHandler": @ChatApiHandler
+			"../Editor/EditorRealTimeController": @EditorRealTimeController
+			'../Authentication/AuthenticationController': @AuthenticationController
+		@req = {}
+		@res =
+			json: sinon.stub()
+			send: sinon.stub()
+
+	describe "sendComment", ->
+		beforeEach ->
+			@req.params =
+				project_id: @project_id
+				thread_id: @thread_id
+			@req.body =
+				content: @content = "message-content"
+			@ChatApiHandler.sendComment = sinon.stub().yields(null, @message = {"mock": "message"})
+			@CommentsController.sendComment @req, @res
+
+		it "should tell the chat handler about the message", ->
+			@ChatApiHandler.sendComment
+				.calledWith(@project_id, @thread_id, @user_id, @content)
+				.should.equal true
+
+		it "should tell the editor real time controller about the update with the data from the chat handler", ->
+			@EditorRealTimeController.emitToRoom
+				.calledWith(@project_id, "new-comment", @thread_id, @message)
+				.should.equal true
+				
+		it "should return a 204 status code", ->
+			@res.send.calledWith(204).should.equal true
+
+	describe "getThreads", ->
+		beforeEach ->
+			@req.params =
+				project_id: @project_id
+			@ChatApiHandler.getThreads = sinon.stub().yields(null, @threads = {"mock", "threads"})
+			@CommentsController.getThreads @req, @res
+
+		it "should ask the chat handler about the request", ->
+			@ChatApiHandler.getThreads
+				.calledWith(@project_id)
+				.should.equal true
+
+		it "should return the messages", ->
+			@res.json.calledWith(@threads).should.equal true