Merge pull request #393 from sharelatex/ja-track-changes

Ja track changes

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

@@ -0,0 +1,61 @@
+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
+	
+	resolveThread: (project_id, thread_id, user_id, callback = (error) ->) ->
+		ChatApiHandler._apiRequest {
+			url:    "#{settings.apis.chat.internal_url}/project/#{project_id}/thread/#{thread_id}/resolve"
+			method: "POST"
+			json:   {user_id}
+		}, callback
+	
+	reopenThread: (project_id, thread_id, callback = (error) ->) ->
+		ChatApiHandler._apiRequest {
+			url:    "#{settings.apis.chat.internal_url}/project/#{project_id}/thread/#{thread_id}/reopen"
+			method: "POST"
+		}, callback

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

@@ -1,33 +1,34 @@
-ChatHandler = require("./ChatHandler")
+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)->
-		project_id = req.params.Project_id
-		messageContent = req.body.content
+		project_id = req.params.project_id
+		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)->
-			if err?
-				logger.err err:err, project_id:project_id, user_id:user_id, messageContent:messageContent, "problem sending message to chat api"
-				return res.sendStatus(500)
-			EditorRealTimeController.emitToRoom project_id,  "new-chat-message", builtMessge, (err)->
-			res.send()
+		ChatApiHandler.sendGlobalMessage project_id, user_id, content, (err, message) ->
+			return next(err) if err?
+			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)->
-		project_id = req.params.Project_id
+	getMessages: (req, res, next)->
+		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)->
-			if err?
-				logger.err err:err, query:query, "problem getting messages from chat api"
-				return res.sendStatus 500
-			logger.log length:messages?.length, "sending messages to client"
-			res.set 'Content-Type', 'application/json'
-			res.send messages
+		ChatApiHandler.getGlobalMessages project_id, query.limit, query.before, (err, messages) ->
+			return next(err) if err?
+			CommentsController._injectUserInfoIntoThreads {global: { messages: messages }}, (err) ->
+				return next(err) if err?
+				logger.log length: messages?.length, "sending messages to client"
+				res.json 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,83 @@
+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) ->
+		{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?
+			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?
+			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) ->
+			return next(err) if err?
+			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
+
+	_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_id, thread of 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

services/web/app/coffee/Features/Docstore/DocstoreManager.coffee

@@ -29,6 +29,21 @@ module.exports = DocstoreManager =
 				error = new Error("docstore api responded with non-success code: #{res.statusCode}")
 				logger.error err: error, project_id: project_id, "error getting all docs from docstore"
 				callback(error)
+	
+	getAllRanges: (project_id, callback = (error) ->) ->
+		logger.log { project_id }, "getting all doc ranges for project in docstore api"
+		url = "#{settings.apis.docstore.url}/project/#{project_id}/ranges"
+		request.get {
+			url: url
+			json: true
+		}, (error, res, docs) ->
+			return callback(error) if error?
+			if 200 <= res.statusCode < 300
+				callback(null, docs)
+			else
+				error = new Error("docstore api responded with non-success code: #{res.statusCode}")
+				logger.error err: error, project_id: project_id, "error getting all doc ranges from docstore"
+				callback(error)
 
 	getDoc: (project_id, doc_id, options = {}, callback = (error, lines, rev, version) ->) ->
 		if typeof(options) == "function"
@@ -45,13 +60,13 @@ module.exports = DocstoreManager =
 			return callback(error) if error?
 			if 200 <= res.statusCode < 300
 				logger.log doc_id: doc_id, project_id: project_id, version: doc.version, rev: doc.rev, "got doc from docstore api"
-				callback(null, doc.lines, doc.rev, doc.version)
+				callback(null, doc.lines, doc.rev, doc.version, doc.ranges)
 			else
 				error = new Error("docstore api responded with non-success code: #{res.statusCode}")
 				logger.error err: error, project_id: project_id, doc_id: doc_id, "error getting doc from docstore"
 				callback(error)
 
-	updateDoc: (project_id, doc_id, lines, version, callback = (error, modified, rev) ->) ->
+	updateDoc: (project_id, doc_id, lines, version, ranges, callback = (error, modified, rev) ->) ->
 		logger.log project_id: project_id, doc_id: doc_id, "updating doc in docstore api"
 		url = "#{settings.apis.docstore.url}/project/#{project_id}/doc/#{doc_id}"
 		request.post {
@@ -59,6 +74,7 @@ module.exports = DocstoreManager =
 			json:
 				lines: lines
 				version: version
+				ranges: ranges
 		}, (error, res, result) ->
 			return callback(error) if error?
 			if 200 <= res.statusCode < 300

services/web/app/coffee/Features/DocumentUpdater/DocumentUpdaterHandler.coffee

@@ -95,7 +95,7 @@ module.exports = DocumentUpdaterHandler =
 				logger.error err: error, project_id: project_id, doc_id: doc_id, "document updater returned failure status code: #{res.statusCode}"
 				return callback(error)
 
-	getDocument: (project_id, doc_id, fromVersion, callback = (error, exists, doclines, version) ->) ->
+	getDocument: (project_id, doc_id, fromVersion, callback = (error, doclines, version, ranges, ops) ->) ->
 		timer = new metrics.Timer("get-document")
 		url = "#{settings.apis.documentupdater.url}/project/#{project_id}/doc/#{doc_id}?fromVersion=#{fromVersion}"
 		logger.log project_id:project_id, doc_id: doc_id, "getting doc from document updater"
@@ -110,7 +110,7 @@ module.exports = DocumentUpdaterHandler =
 					body = JSON.parse(body)
 				catch error
 					return callback(error)
-				callback null, body.lines, body.version, body.ops
+				callback null, body.lines, body.version, body.ranges, body.ops
 			else
 				logger.error project_id:project_id, doc_id:doc_id, url: url, "doc updater returned a non-success status code: #{res.statusCode}"
 				callback new Error("doc updater returned a non-success status code: #{res.statusCode}")
@@ -137,15 +137,21 @@ module.exports = DocumentUpdaterHandler =
 				logger.error project_id:project_id, doc_id:doc_id, url: url, "doc updater returned a non-success status code: #{res.statusCode}"
 				callback new Error("doc updater returned a non-success status code: #{res.statusCode}")
 
-	getNumberOfDocsInMemory : (callback)->
-		request.get "#{settings.apis.documentupdater.url}/total", (err, req, body)->
-			try
-				body = JSON.parse body
-			catch err
-				logger.err err:err, "error parsing response from doc updater about the total number of docs"
-			callback(err, body?.total)
-
-
+	acceptChange: (project_id, doc_id, change_id, callback = (error) ->) ->
+		timer = new metrics.Timer("accept-change")
+		url = "#{settings.apis.documentupdater.url}/project/#{project_id}/doc/#{doc_id}/change/#{change_id}/accept"
+		logger.log {project_id, doc_id, change_id}, "accepting change in document updater"
+		request.post url, (error, res, body)->
+			timer.done()
+			if error?
+				logger.error {err:error, project_id, doc_id, change_id}, "error accepting change in doc updater"
+				return callback(error)
+			if res.statusCode >= 200 and res.statusCode < 300
+				logger.log {project_id, doc_id, change_id}, "accepted change in document updater"
+				return callback(null)
+			else
+				logger.error {project_id, doc_id, change_id}, "doc updater returned a non-success status code: #{res.statusCode}"
+				callback new Error("doc updater returned a non-success status code: #{res.statusCode}")
 
 PENDINGUPDATESKEY = "PendingUpdates"
 DOCLINESKEY = "doclines"

services/web/app/coffee/Features/Documents/DocumentController.coffee

@@ -7,7 +7,7 @@ module.exports =
 		doc_id = req.params.doc_id
 		plain = req?.query?.plain == 'true'
 		logger.log doc_id:doc_id, project_id:project_id, "receiving get document request from api (docupdater)"
-		ProjectEntityHandler.getDoc project_id, doc_id, (error, lines, rev, version) ->
+		ProjectEntityHandler.getDoc project_id, doc_id, (error, lines, rev, version, ranges) ->
 			if error?
 				logger.err err:error, doc_id:doc_id, project_id:project_id, "error finding element for getDocument"
 				return next(error)
@@ -19,14 +19,15 @@ module.exports =
 				res.send JSON.stringify {
 					lines: lines
 					version: version
+					ranges: ranges
 				}
 
 	setDocument: (req, res, next = (error) ->) ->
 		project_id = req.params.Project_id
 		doc_id = req.params.doc_id
-		{lines, version} = req.body
+		{lines, version, ranges} = req.body
 		logger.log doc_id:doc_id, project_id:project_id, "receiving set document request from api (docupdater)"
-		ProjectEntityHandler.updateDocLines project_id, doc_id, lines, version, (error) ->
+		ProjectEntityHandler.updateDocLines project_id, doc_id, lines, version, ranges, (error) ->
 			if error?
 				logger.err err:error, doc_id:doc_id, project_id:project_id, "error finding element for getDocument"
 				return next(error)

services/web/app/coffee/Features/Editor/EditorController.coffee

@@ -7,7 +7,6 @@ ProjectDetailsHandler = require('../Project/ProjectDetailsHandler')
 ProjectDeleter = require("../Project/ProjectDeleter")
 DocumentUpdaterHandler = require('../DocumentUpdater/DocumentUpdaterHandler')
 EditorRealTimeController = require("./EditorRealTimeController")
-TrackChangesManager = require("../TrackChanges/TrackChangesManager")
 async = require('async')
 LockManager = require("../../infrastructure/LockManager")
 _ = require('underscore')

services/web/app/coffee/Features/History/HistoryController.coffee

@@ -0,0 +1,20 @@
+logger = require "logger-sharelatex"
+request = require "request"
+settings = require "settings-sharelatex"
+AuthenticationController = require "../Authentication/AuthenticationController"
+
+module.exports = HistoryController =
+	proxyToHistoryApi: (req, res, next = (error) ->) ->
+		user_id = AuthenticationController.getLoggedInUserId req
+		url = settings.apis.trackchanges.url + req.url
+		logger.log url: url, "proxying to track-changes api"
+		getReq = request(
+			url: url
+			method: req.method
+			headers:
+				"X-User-Id": user_id
+		)
+		getReq.pipe(res)
+		getReq.on "error", (error) ->
+			logger.error err: error, "track-changes API error"
+			next(error)

services/web/app/coffee/Features/History/HistoryManager.coffee

@@ -0,0 +1,28 @@
+settings = require "settings-sharelatex"
+request = require "request"
+logger = require "logger-sharelatex"
+
+module.exports = HistoryManager =
+	flushProject: (project_id, callback = (error) ->) ->
+		logger.log project_id: project_id, "flushing project in track-changes api"
+		url = "#{settings.apis.trackchanges.url}/project/#{project_id}/flush"
+		request.post url, (error, res, body) ->
+			return callback(error) if error?
+			if 200 <= res.statusCode < 300
+				callback(null)
+			else
+				error = new Error("track-changes api responded with non-success code: #{res.statusCode} #{url}")
+				logger.error err: error, project_id: project_id, "error flushing project in track-changes api"
+				callback(error)
+
+	archiveProject: (project_id, callback = ()->)->
+		logger.log project_id: project_id, "archving project in track-changes api"
+		url = "#{settings.apis.trackchanges.url}/project/#{project_id}/archive"
+		request.post url, (error, res, body) ->
+			return callback(error) if error?
+			if 200 <= res.statusCode < 300
+				callback(null)
+			else
+				error = new Error("track-changes api responded with non-success code: #{res.statusCode} #{url}")
+				logger.error err: error, project_id: project_id, "error archving project in track-changes api"
+				callback(error)

services/web/app/coffee/Features/InactiveData/InactiveProjectManager.coffee

@@ -5,8 +5,6 @@ DocstoreManager = require("../Docstore/DocstoreManager")
 ProjectGetter = require("../Project/ProjectGetter")
 ProjectUpdateHandler = require("../Project/ProjectUpdateHandler")
 Project = require("../../models/Project").Project
-TrackChangesManager = require("../TrackChanges/TrackChangesManager")
-
 
 MILISECONDS_IN_DAY = 86400000
 module.exports = InactiveProjectManager =
@@ -52,7 +50,6 @@ module.exports = InactiveProjectManager =
 		logger.log project_id:project_id, "deactivating inactive project"
 		jobs = [
 			(cb)-> DocstoreManager.archiveProject project_id, cb
-			# (cb)-> TrackChangesManager.archiveProject project_id, cb
 			(cb)-> ProjectUpdateHandler.markAsInactive project_id, cb
 		]
 		async.series jobs, (err)->

services/web/app/coffee/Features/Project/ProjectController.coffee

@@ -201,7 +201,7 @@ module.exports = ProjectController =
 
 		async.parallel {
 			project: (cb)->
-				ProjectGetter.getProject project_id, { name: 1, lastUpdated: 1}, cb
+				ProjectGetter.getProject project_id, { name: 1, lastUpdated: 1, track_changes: 1 }, cb
 			user: (cb)->
 				if !user_id?
 					cb null, defaultSettingsForAnonymousUser(user_id)
@@ -267,6 +267,7 @@ module.exports = ProjectController =
 						pdfViewer : user.ace.pdfViewer
 						syntaxValidation: user.ace.syntaxValidation
 					}
+					trackChangesEnabled: !!project.track_changes
 					privilegeLevel: privilegeLevel
 					chatUrl: Settings.apis.chat.url
 					anonymous: anonymous

services/web/app/coffee/Features/Project/ProjectEditorHandler.coffee

@@ -19,6 +19,11 @@ module.exports = ProjectEditorHandler =
 
 		if !result.invites?
 			result.invites = []
+			
+		hasTrackChanges = false
+		for member in members
+			if member.privilegeLevel == "owner" and member.user?.featureSwitches?.track_changes
+				hasTrackChanges = true
 
 		{owner, ownerFeatures, members} = @buildOwnerAndMembersViews(members)
 		result.owner = owner
@@ -32,6 +37,7 @@ module.exports = ProjectEditorHandler =
 			compileGroup:"standard"
 			templates: false
 			references: false
+			trackChanges: hasTrackChanges
 		})
 
 		return result

services/web/app/coffee/Features/Project/ProjectEntityHandler.coffee

@@ -126,7 +126,7 @@ module.exports = ProjectEntityHandler =
 			doc = new Doc name: docName
 			# Put doc in docstore first, so that if it errors, we don't have a doc_id in the project
 			# which hasn't been created in docstore.
-			DocstoreManager.updateDoc project_id.toString(), doc._id.toString(), docLines, 0, (err, modified, rev) ->
+			DocstoreManager.updateDoc project_id.toString(), doc._id.toString(), docLines, 0, {}, (err, modified, rev) ->
 				return callback(err) if err? 
 
 				ProjectEntityHandler._putElement project, folder_id, doc, "doc", (err, result)=>
@@ -292,7 +292,7 @@ module.exports = ProjectEntityHandler =
 					return callback(err)
 				callback(err, folder, parentFolder_id)
 
-	updateDocLines : (project_id, doc_id, lines, version, callback = (error) ->)->
+	updateDocLines : (project_id, doc_id, lines, version, ranges, callback = (error) ->)->
 		ProjectGetter.getProjectWithoutDocLines project_id, (err, project)->
 			return callback(err) if err?
 			return callback(new Errors.NotFoundError("project not found")) if !project?
@@ -307,7 +307,7 @@ module.exports = ProjectEntityHandler =
 					return callback(error)
 
 				logger.log project_id: project_id, doc_id: doc_id, "telling docstore manager to update doc"
-				DocstoreManager.updateDoc project_id, doc_id, lines, version, (err, modified, rev) ->
+				DocstoreManager.updateDoc project_id, doc_id, lines, version, ranges, (err, modified, rev) ->
 					if err?
 						logger.error err: err, doc_id: doc_id, project_id:project_id, lines: lines, "error sending doc to docstore"
 						return callback(err)

services/web/app/coffee/Features/TrackChanges/RangesManager.coffee

@@ -0,0 +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
+	
+	getAllChangesUsers: (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

services/web/app/coffee/Features/TrackChanges/TrackChangesController.coffee

@@ -1,20 +1,42 @@
+RangesManager = require "./RangesManager"
 logger = require "logger-sharelatex"
-request = require "request"
-settings = require "settings-sharelatex"
-AuthenticationController = require "../Authentication/AuthenticationController"
+UserInfoController = require "../User/UserInfoController"
+DocumentUpdaterHandler = require "../DocumentUpdater/DocumentUpdaterHandler"
+EditorRealTimeController = require("../Editor/EditorRealTimeController")
+TrackChangesManager = require "./TrackChangesManager"
 
 module.exports = TrackChangesController =
-	proxyToTrackChangesApi: (req, res, next = (error) ->) ->
-		user_id = AuthenticationController.getLoggedInUserId req
-		url = settings.apis.trackchanges.url + req.url
-		logger.log url: url, "proxying to track-changes api"
-		getReq = request(
-			url: url
-			method: req.method
-			headers:
-				"X-User-Id": user_id
-		)
-		getReq.pipe(res)
-		getReq.on "error", (error) ->
-			logger.error err: error, "track-changes API error"
-			next(error)
+	getAllRanges: (req, res, next) ->
+		project_id = req.params.project_id
+		logger.log {project_id}, "request for project ranges"
+		RangesManager.getAllRanges project_id, (error, docs = []) ->
+			return next(error) if error?
+			docs = ({id: d._id, ranges: d.ranges} for d in docs)
+			res.json docs
+	
+	getAllChangesUsers: (req, res, next) ->
+		project_id = req.params.project_id
+		logger.log {project_id}, "request for project range users"
+		RangesManager.getAllChangesUsers project_id, (error, users) ->
+			return next(error) if error?
+			users = (UserInfoController.formatPersonalInfo(user) for user in users)
+			# Get rid of any anonymous/deleted user objects
+			users = users.filter (u) -> u?.id?
+			res.json users
+	
+	acceptChange: (req, res, next) ->
+		{project_id, doc_id, change_id} = req.params
+		logger.log {project_id, doc_id, change_id}, "request to accept change"
+		DocumentUpdaterHandler.acceptChange project_id, doc_id, change_id, (error) ->
+			return next(error) if error?
+			EditorRealTimeController.emitToRoom project_id, "accept-change", doc_id, change_id, (err)->
+			res.send 204
+
+	toggleTrackChanges: (req, res, next) ->
+		{project_id} = req.params
+		track_changes_on = !!req.body.on
+		logger.log {project_id, track_changes_on}, "request to toggle track changes"
+		TrackChangesManager.toggleTrackChanges project_id, track_changes_on, (error) ->
+			return next(error) if error?
+			EditorRealTimeController.emitToRoom project_id, "toggle-track-changes", track_changes_on, (err)->
+			res.send 204

services/web/app/coffee/Features/TrackChanges/TrackChangesManager.coffee

@@ -1,28 +1,5 @@
-settings = require "settings-sharelatex"
-request = require "request"
-logger = require "logger-sharelatex"
+Project = require("../../models/Project").Project
 
 module.exports = TrackChangesManager =
-	flushProject: (project_id, callback = (error) ->) ->
-		logger.log project_id: project_id, "flushing project in track-changes api"
-		url = "#{settings.apis.trackchanges.url}/project/#{project_id}/flush"
-		request.post url, (error, res, body) ->
-			return callback(error) if error?
-			if 200 <= res.statusCode < 300
-				callback(null)
-			else
-				error = new Error("track-changes api responded with non-success code: #{res.statusCode} #{url}")
-				logger.error err: error, project_id: project_id, "error flushing project in track-changes api"
-				callback(error)
-
-	archiveProject: (project_id, callback = ()->)->
-		logger.log project_id: project_id, "archving project in track-changes api"
-		url = "#{settings.apis.trackchanges.url}/project/#{project_id}/archive"
-		request.post url, (error, res, body) ->
-			return callback(error) if error?
-			if 200 <= res.statusCode < 300
-				callback(null)
-			else
-				error = new Error("track-changes api responded with non-success code: #{res.statusCode} #{url}")
-				logger.error err: error, project_id: project_id, "error archving project in track-changes api"
-				callback(error)
+	toggleTrackChanges: (project_id, track_changes_on, callback = (error) ->) ->
+		Project.update {_id: project_id}, {track_changes: track_changes_on}, callback

services/web/app/coffee/Features/User/UserInfoController.coffee

@@ -26,17 +26,14 @@ 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) ->) ->
+		if !user?
+			return {}
+		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

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

services/web/app/coffee/models/Project.coffee

@@ -32,6 +32,7 @@ ProjectSchema = new Schema
 	archived          : { type: Boolean }
 	deletedDocs       : [DeletedDocSchema]
 	imageName         : { type: String }
+	track_changes     : { type: Boolean }
 
 ProjectSchema.statics.getProject = (project_or_id, fields, callback)->
 	if project_or_id._id?

services/web/app/coffee/models/User.coffee

@@ -39,7 +39,7 @@ UserSchema = new Schema
 							references:    { type:Boolean, default: Settings.defaultFeatures.references }
 						}
 	featureSwitches	  : {
-		pdfng: { type: Boolean }
+		track_changes: { type: Boolean }
 	}
 	referal_id : {type:String, default:() -> uuid.v4().split("-")[0]}
 	refered_users: [ type:ObjectId, ref:'User' ]

services/web/app/coffee/router.coffee

@@ -25,7 +25,7 @@ ClsiCookieManager = require("./Features/Compile/ClsiCookieManager")
 HealthCheckController = require("./Features/HealthCheck/HealthCheckController")
 ProjectDownloadsController = require "./Features/Downloads/ProjectDownloadsController"
 FileStoreController = require("./Features/FileStore/FileStoreController")
-TrackChangesController = require("./Features/TrackChanges/TrackChangesController")
+HistoryController = require("./Features/History/HistoryController")
 PasswordResetRouter = require("./Features/PasswordReset/PasswordResetRouter")
 StaticPagesRouter = require("./Features/StaticPages/StaticPagesRouter")
 ChatController = require("./Features/Chat/ChatController")
@@ -40,6 +40,8 @@ AuthorizationMiddlewear = require('./Features/Authorization/AuthorizationMiddlew
 BetaProgramController = require('./Features/BetaProgram/BetaProgramController')
 AnalyticsRouter = require('./Features/Analytics/AnalyticsRouter')
 AnnouncementsController = require("./Features/Announcements/AnnouncementsController")
+TrackChangesController = require("./Features/TrackChanges/TrackChangesController")
+CommentsController = require "./Features/Comments/CommentsController"
 
 logger = require("logger-sharelatex")
 _ = require("underscore")
@@ -171,9 +173,14 @@ module.exports = class Router
 
 		webRouter.post '/project/:Project_id/rename', AuthorizationMiddlewear.ensureUserCanAdminProject, ProjectController.renameProject
 
-		webRouter.get  "/project/:Project_id/updates", AuthorizationMiddlewear.ensureUserCanReadProject, TrackChangesController.proxyToTrackChangesApi
-		webRouter.get  "/project/:Project_id/doc/:doc_id/diff", AuthorizationMiddlewear.ensureUserCanReadProject, TrackChangesController.proxyToTrackChangesApi
-		webRouter.post "/project/:Project_id/doc/:doc_id/version/:version_id/restore", AuthorizationMiddlewear.ensureUserCanReadProject, TrackChangesController.proxyToTrackChangesApi
+		webRouter.get  "/project/:Project_id/updates", AuthorizationMiddlewear.ensureUserCanReadProject, HistoryController.proxyToHistoryApi
+		webRouter.get  "/project/:Project_id/doc/:doc_id/diff", AuthorizationMiddlewear.ensureUserCanReadProject, HistoryController.proxyToHistoryApi
+		webRouter.post "/project/:Project_id/doc/:doc_id/version/:version_id/restore", AuthorizationMiddlewear.ensureUserCanReadProject, HistoryController.proxyToHistoryApi
+
+		webRouter.get "/project/:project_id/ranges", AuthorizationMiddlewear.ensureUserCanReadProject, TrackChangesController.getAllRanges
+		webRouter.get "/project/:project_id/changes/users", AuthorizationMiddlewear.ensureUserCanReadProject, TrackChangesController.getAllChangesUsers
+		webRouter.post "/project/:project_id/doc/:doc_id/changes/:change_id/accept", AuthorizationMiddlewear.ensureUserCanWriteProjectContent, TrackChangesController.acceptChange
+		webRouter.post "/project/:project_id/track_changes", AuthorizationMiddlewear.ensureUserCanWriteProjectContent, TrackChangesController.toggleTrackChanges
 
 		webRouter.get  '/Project/:Project_id/download/zip', AuthorizationMiddlewear.ensureUserCanReadProject, ProjectDownloadsController.downloadProject
 		webRouter.get  '/project/download/zip', AuthorizationMiddlewear.ensureUserCanReadMultipleProjects, ProjectDownloadsController.downloadMultipleProjects
@@ -223,8 +230,14 @@ 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.post "/project/:Project_id/messages", AuthorizationMiddlewear.ensureUserCanReadProject, ChatController.sendMessage
+		webRouter.get  "/project/:project_id/messages", AuthorizationMiddlewear.ensureUserCanReadProject, ChatController.getMessages
+		webRouter.post "/project/:project_id/messages", AuthorizationMiddlewear.ensureUserCanReadProject, ChatController.sendMessage
+		
+		# Note: Read only users can still comment
+		webRouter.post "/project/:project_id/thread/:thread_id/messages", AuthorizationMiddlewear.ensureUserCanReadProject, CommentsController.sendComment
+		webRouter.get  "/project/:project_id/threads", AuthorizationMiddlewear.ensureUserCanReadProject, CommentsController.getThreads
+		webRouter.post "/project/:project_id/thread/:thread_id/resolve", AuthorizationMiddlewear.ensureUserCanWriteProjectContent, CommentsController.resolveThread
+		webRouter.post "/project/:project_id/thread/:thread_id/reopen", AuthorizationMiddlewear.ensureUserCanWriteProjectContent, CommentsController.reopenThread
 
 		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.jade

@@ -107,6 +107,7 @@ block requirejs
 		window.csrfToken = "!{csrfToken}";
 		window.anonymous = #{anonymous};
 		window.maxDocLength = #{maxDocLength};
+		window.trackChangesEnabled = #{trackChangesEnabled};
 		window.wikiEnabled = #{!!(settings.apis.wiki && settings.apis.wiki.url)};
 		window.requirejs = {
 			"paths" : {

services/web/app/views/project/editor/editor.jade

@@ -17,7 +17,9 @@ div.full-size(
 			'rp-state-current-file-mini': (reviewPanel.subView === SubViews.CUR_FILE && !ui.reviewPanelOpen),\
 			'rp-state-overview': (reviewPanel.subView === SubViews.OVERVIEW),\
 			'rp-size-mini': (!ui.reviewPanelOpen && reviewPanel.hasEntries),\
-			'rp-size-expanded': ui.reviewPanelOpen\
+			'rp-size-expanded': ui.reviewPanelOpen,\
+			'rp-layout-left': reviewPanel.layoutToLeft,\
+			'rp-loading-threads': reviewPanel.loadingThreads\
 		}"
 	)
 		.loading-panel(ng-show="!editor.sharejs_doc || editor.opening")
@@ -51,11 +53,20 @@ div.full-size(
 			syntax-validation="settings.syntaxValidation",
 			review-panel="reviewPanel",
 			events-bridge="reviewPanelEventsBridge"
-			track-changes-enabled="trackChangesFeatureFlag",
-			track-new-changes= "reviewPanel.trackNewChanges",
-			changes-tracker="reviewPanel.changesTracker",
+			track-changes-enabled="project.features.trackChanges",
+			track-changes= "editor.trackChanges",
 			doc-id="editor.open_doc_id"
+			renderer-data="reviewPanel.rendererData"
 		)
+
+		a.rp-track-changes-indicator(
+			href
+			ng-if="editor.wantTrackChanges"
+			ng-click="toggleReviewPanel();"
+			ng-class="{ 'rp-track-changes-indicator-on-dark' : darkTheme }"
+		) Track changes is 
+			strong on
+
 		
 		include ./review-panel
 

services/web/app/views/project/editor/header.jade

@@ -87,7 +87,7 @@ header.toolbar.toolbar-header.toolbar-with-labels(
 
 		a.btn.btn-full-height(
 			href,
-			ng-if="trackChangesFeatureFlag",
+			ng-if="project.features.trackChanges",
 			ng-class="{ active: ui.reviewPanelOpen }"
 			ng-click="toggleReviewPanel()"
 		)

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

@@ -1,10 +1,29 @@
 #review-panel
 	.review-panel-toolbar
-		span.review-panel-toolbar-label(ng-click="reviewPanel.trackNewChanges = true;", ng-if="reviewPanel.trackNewChanges === false") Track Changes is 
-			strong off
-		span.review-panel-toolbar-label(ng-click="reviewPanel.trackNewChanges = false;", ng-if="reviewPanel.trackNewChanges === true") Track Changes is 
-			strong on
-		review-panel-toggle(ng-model="reviewPanel.trackNewChanges")
+		resolved-comments-dropdown(
+			entries="reviewPanel.resolvedComments"
+			threads="reviewPanel.commentThreads"
+			resolved-ids="reviewPanel.resolvedThreadIds"
+			docs="docs"
+			on-open="refreshResolvedCommentsDropdown();"
+			on-unresolve="unresolveComment(threadId);"
+			on-delete="deleteComment(entryId, threadId);"
+			is-loading="reviewPanel.dropdown.loading"
+			permissions="permissions"
+		)
+		span.review-panel-toolbar-label(ng-if="permissions.write")
+			span(ng-click="toggleTrackChanges(true)", ng-if="editor.wantTrackChanges === false") Track Changes is 
+				strong off
+			span(ng-click="toggleTrackChanges(false)", ng-if="editor.wantTrackChanges === true") Track Changes is 
+				strong on
+			review-panel-toggle(ng-if="editor.wantTrackChanges == editor.trackChanges", ng-model="editor.wantTrackChanges", on-toggle="toggleTrackChanges")
+		span.review-panel-toolbar-label.review-panel-toolbar-label-disabled(ng-if="!permissions.write")
+			span(ng-if="editor.wantTrackChanges === false") Track Changes is 
+				strong off
+			span(ng-if="editor.wantTrackChanges === true") Track Changes is 
+				strong on
+		span.review-panel-toolbar-spinner(ng-if="editor.wantTrackChanges != editor.trackChanges")
+			i.fa.fa-spin.fa-spinner
 
 	.rp-entry-list(
 		review-panel-sorted
@@ -13,6 +32,7 @@
 		.rp-entry-list-inner
 			.rp-entry-wrapper(
 				ng-repeat="(entry_id, entry) in reviewPanel.entries[editor.open_doc_id]"
+				ng-if="!(entry.type === 'comment' && reviewPanel.commentThreads[entry.thread_id].resolved === true)"
 			)
 				div(ng-if="entry.type === 'insert' || entry.type === 'delete'")
 					change-entry(
@@ -21,22 +41,21 @@
 						on-reject="rejectChange(entry_id);"
 						on-accept="acceptChange(entry_id);"
 						on-indicator-click="toggleReviewPanel();"
+						permissions="permissions"
 					)
 
 				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)"
-						on-hide-thread="hideThread(entry)"
-						on-delete="deleteComment(entry_id)"
 						on-reply="submitReply(entry, entry_id);"
 						on-indicator-click="toggleReviewPanel();"
+						permissions="permissions"
+						ng-if="!reviewPanel.loadingThreads"
 					)
 				
-				div(ng-if="entry.type === 'add-comment'")
+				div(ng-if="entry.type === 'add-comment' && permissions.comment")
 					add-comment-entry(
 						on-start-new="startNewComment();"
 						on-submit="submitNewComment(content);"
@@ -47,34 +66,37 @@
 	.rp-entry-list(
 		ng-if="reviewPanel.subView === SubViews.OVERVIEW"
 	)
+		.rp-loading(ng-if="reviewPanel.overview.loading")
+			i.fa.fa-spinner.fa-spin
 		.rp-overview-file(
-			ng-repeat="(doc_id, entries) in reviewPanel.entries"
+			ng-repeat="doc in docs"
+			ng-if="!reviewPanel.overview.loading"
 		)
-			.rp-overview-file-header
-				| {{ getFileName(doc_id) }}
+			.rp-overview-file-header(
+				ng-if="reviewPanel.entries[doc.doc.id] | notEmpty"
+			)
+				| {{ doc.path }}
 			.rp-entry-wrapper(
-				ng-repeat="(entry_id, entry) in entries | orderOverviewEntries"
+				ng-repeat="(entry_id, entry) in reviewPanel.entries[doc.doc.id] | orderOverviewEntries"
+				ng-if="!(entry.type === 'comment' && reviewPanel.commentThreads[entry.thread_id].resolved === true)"
 			)
 				div(ng-if="entry.type === 'insert' || entry.type === 'delete'")
 					change-entry(
 						entry="entry"
 						user="users[entry.metadata.user_id]"
-						on-reject="rejectChange(entry.id);"
-						on-accept="acceptChange(entry.id);"
 						on-indicator-click="toggleReviewPanel();"
-						ng-click="gotoEntry(doc_id, entry)"
+						ng-click="gotoEntry(doc.doc.id, entry)"
+						permissions="permissions"
 					)
 
 				div(ng-if="entry.type === 'comment'")
 					comment-entry(
 						entry="entry"
-						users="users"
-						on-resolve="resolveComment(entry, entry.id)"
-						on-unresolve="unresolveComment(entry.id)"
-						on-delete="deleteComment(entry.id)"
+						threads="reviewPanel.commentThreads"
 						on-reply="submitReply(entry, entry_id);"
 						on-indicator-click="toggleReviewPanel();"
-						ng-click="gotoEntry(doc_id, entry)"
+						ng-click="gotoEntry(doc.doc.id, entry)"
+						permissions="permissions"
 					)
 
 	.rp-nav
@@ -109,20 +131,30 @@ script(type='text/ng-template', id='changeEntryTemplate')
 		.rp-entry(
 			ng-class="[ 'rp-entry-' + entry.type, (entry.focused ? 'rp-entry-focused' : '')]"
 		)
-			.rp-entry-header
+			.rp-entry-body
 				.rp-entry-action-icon(ng-switch="entry.type")
 					i.fa.fa-pencil(ng-switch-when="insert")
 					i.rp-icon-delete(ng-switch-when="delete")
-				.rp-entry-metadata
-					p.rp-entry-metadata-line(style="color: hsl({{ user.hue }}, 70%, 40%);") {{ user.name }} 
-					p.rp-entry-metadata-line {{ entry.metadata.ts | date : 'MMM d, y h:mm a' }}
-				.rp-avatar(style="background-color: hsl({{ user.hue }}, 70%, 50%);") {{ user.avatar_text | limitTo : 1 }}
-			.rp-entry-body(ng-switch="entry.type")
-					span(ng-switch-when="insert") Added 
-						ins.rp-content-highlight {{ entry.content }} 
-					span(ng-switch-when="delete") Deleted 
-						del.rp-content-highlight {{ entry.content }} 
-			.rp-entry-actions
+				.rp-entry-details
+					.rp-entry-description(ng-switch="entry.type")
+						span(ng-switch-when="insert") Added 
+							ins.rp-content-highlight {{ entry.content | limitTo:(isCollapsed ? contentLimit : entry.content.length) }}
+							a.rp-collapse-toggle(
+								href
+								ng-if="needsCollapsing"
+								ng-click="toggleCollapse();"
+							) {{ isCollapsed ? '... (show all)' : ' (show less)' }}
+						span(ng-switch-when="delete") Deleted 
+							del.rp-content-highlight {{ entry.content | limitTo:(isCollapsed ? contentLimit : entry.content.length) }}
+							a.rp-collapse-toggle(
+								href
+								ng-if="needsCollapsing"
+								ng-click="toggleCollapse();"
+							) {{ isCollapsed ? '... (show all)' : ' (show less)' }}
+					.rp-entry-metadata
+						| {{ entry.metadata.ts | date : 'MMM d, y h:mm a' }} • 
+						span.rp-entry-user(style="color: hsl({{ user.hue }}, 70%, 40%);") {{ user.name }} 
+			.rp-entry-actions(ng-if="permissions.write")
 				a.rp-entry-button(href, ng-click="onReject();") 
 					i.fa.fa-times
 					|  Reject
@@ -131,56 +163,98 @@ script(type='text/ng-template', id='changeEntryTemplate')
 					|  Accept
 
 script(type='text/ng-template', id='commentEntryTemplate')
-	div
-		.rp-entry-callout.rp-entry-callout-comment(ng-if="!entry.resolved")
+	.rp-comment-wrapper(
+		ng-class="{ 'rp-comment-wrapper-resolving': state.animating }"
+	)
+		.rp-entry-callout.rp-entry-callout-comment
 		.rp-entry-indicator(
 			ng-class="{ 'rp-entry-indicator-focused': entry.focused }"
 			ng-click="onIndicatorClick();"
 		)
 			i.fa.fa-comment
 		.rp-entry.rp-entry-comment(
-			ng-class="{ 'rp-entry-focused': entry.focused, 'rp-entry-comment-resolved': entry.resolved}"
+			ng-class="{ 'rp-entry-focused': entry.focused, 'rp-entry-comment-resolving': state.animating }"
 		)
-			.rp-comment(
-				ng-if="!entry.resolved || entry.showWhenResolved"
-				ng-repeat="comment in entry.thread"
-				ng-class="users[comment.user_id].isSelf ? 'rp-comment-self' : '';"
-			)
-				.rp-avatar(
-					ng-if="!users[comment.user_id].isSelf;"
-					style="background-color: hsl({{ users[comment.user_id].hue }}, 70%, 50%);"
-				) {{ users[comment.user_id].avatar_text | limitTo : 1 }}
-				.rp-comment-body(style="color: hsl({{ users[comment.user_id].hue }}, 70%, 90%);")
-					p.rp-comment-content {{ comment.content }}
-					p.rp-comment-metadata
-						| {{ comment.ts | date : 'MMM d, y h:mm a' }}
-						|  • 
-						span(style="color: hsl({{ users[comment.user_id].hue }}, 70%, 40%);") {{ users[comment.user_id].name }}
-			.rp-comment-reply(ng-if="!entry.resolved || entry.showWhenResolved")
+			div
+				.rp-comment(
+					ng-repeat="comment in threads[entry.thread_id].messages track by comment.id"
+				)
+					p.rp-comment-content 
+						span.rp-entry-user(
+							style="color: hsl({{ comment.user.hue }}, 70%, 40%);"
+						) {{ comment.user.name }}: 
+						| {{ comment.content }}
+					.rp-entry-metadata
+						| {{ comment.timestamp | date : 'MMM d, y h:mm a' }}
+			.rp-loading(ng-if="!threads[entry.thread_id] || threads[entry.thread_id].submitting")
+				i.fa.fa-spinner.fa-spin
+			.rp-comment-reply(ng-if="permissions.comment")
 				textarea.rp-comment-input(
 					ng-model="entry.replyContent"
 					ng-keypress="handleCommentReplyKeyPress($event);"
 					stop-propagation="click"
 					placeholder="{{ 'Hit \"Enter\" to reply' + (entry.resolved ? ' and re-open' : '') }}"
 				)
-			.rp-comment-resolved-description(ng-if="entry.resolved && !entry.showWhenResolved")
-				div
-					| Comment resolved by 
-					span(style="color: hsl({{ users[entry.resolved_data.user_id].hue }}, 70%, 40%);") {{ users[entry.resolved_data.user_id].name }}
-				div {{ entry.resolved_data.ts | date : 'MMM d, y h:mm a' }}
 			.rp-entry-actions
-				a.rp-entry-button(href, ng-click="onResolve();", ng-if="!entry.resolved") 
-					i.fa.fa-check
-					|  Mark as resolved
-				a.rp-entry-button(href, ng-click="onShowThread();", ng-if="entry.resolved && !entry.showWhenResolved")
-					|  Show
-				a.rp-entry-button(href, ng-click="onHideThread();", ng-if="entry.resolved && entry.showWhenResolved")
-					|  Hide
-				a.rp-entry-button(href, ng-click="onUnresolve();", ng-if="entry.resolved") 
-					|  Re-open
-				a.rp-entry-button(href, ng-click="onDelete();", ng-if="entry.resolved") 
-					|  Delete
+				button.rp-entry-button(
+					ng-click="animateAndCallOnResolve();"
+					ng-if="permissions.comment && permissions.write"
+				) 
+					i.fa.fa-inbox
+					|  Resolve
+				button.rp-entry-button(
+					ng-click="onReply();"
+					ng-if="permissions.comment"
+					ng-disabled="!entry.replyContent.length"
+				)
+					i.fa.fa-reply
+					|  Reply
+	
+script(type='text/ng-template', id='resolvedCommentEntryTemplate')
+	.rp-resolved-comment
+		div
+			.rp-resolved-comment-context
+				| Quoted text on 
+				span.rp-resolved-comment-context-file {{ thread.docName }}
+				p.rp-resolved-comment-context-quote
+					span {{ thread.content | limitTo:(isCollapsed ? contentLimit : thread.content.length)}}
+					a.rp-collapse-toggle(
+						href
+						ng-if="needsCollapsing"
+						ng-click="toggleCollapse();"
+					)  {{ isCollapsed ? '... (show all)' : ' (show less)' }}
+			.rp-comment(
+				ng-repeat="comment in thread.messages track by comment.id"
+			)
+				p.rp-comment-content 
+					span.rp-entry-user(
+						style="color: hsl({{ comment.user.hue }}, 70%, 40%);"
+						ng-if="$first || comment.user.id !== thread.messages[$index - 1].user.id"
+					) {{ comment.user.name }}: 
+					| {{ comment.content }}
+				.rp-entry-metadata
+					| {{ comment.timestamp | date : 'MMM d, y h:mm a' }}
+			.rp-comment.rp-comment-resolver
+				p.rp-comment-resolver-content 
+					span.rp-entry-user(
+						style="color: hsl({{ thread.resolved_by_user.hue }}, 70%, 40%);"
+					) {{ thread.resolved_by_user.name }}: 
+					| Marked as resolved.
+				.rp-entry-metadata
+					| {{ thread.resolved_at | date : 'MMM d, y h:mm a' }}
 				
+		.rp-entry-actions(ng-if="permissions.comment && permissions.write")
+			a.rp-entry-button(
+				href
+				ng-click="onUnresolve({ 'threadId': thread.threadId });"
+			) 
+				|  Re-open
+			//- a.rp-entry-button(
+			//- 	href
+			//- 	ng-click="onDelete({ 'entryId': thread.entryId, 'threadId': thread.threadId });"
+			//- )
+			//- 	|  Delete
+
 
 script(type='text/ng-template', id='addCommentEntryTemplate')
 	div
@@ -209,11 +283,50 @@ script(type='text/ng-template', id='addCommentEntryTemplate')
 						ng-model="state.content"
 						ng-keypress="handleCommentKeyPress($event);"
 						placeholder="Add your comment here"
+						focus-on="comment:new:open"
 					)
 				.rp-entry-actions
-					a.rp-entry-button(href, ng-click="cancelNewComment();") 
+					button.rp-entry-button(
+						ng-click="cancelNewComment();"
+					) 
 						i.fa.fa-times
 						|  Cancel
-					a.rp-entry-button(href, ng-click="submitNewComment()") 
+					button.rp-entry-button(
+						ng-click="submitNewComment()"
+						ng-disabled="!state.content.length"
+					) 
 						i.fa.fa-comment
-						|  Comment
+						|  Comment
+
+script(type='text/ng-template', id='resolvedCommentsDropdownTemplate')
+	.resolved-comments
+		.resolved-comments-backdrop(
+			ng-class="{ 'resolved-comments-backdrop-visible' : state.isOpen }"
+			ng-click="state.isOpen = false"
+		)
+		a.resolved-comments-toggle(
+			href
+			ng-click="toggleOpenState();"
+			tooltip="Resolved Comments"
+			tooltip-placement="bottom"
+			tooltip-append-to-body="true"
+		)
+			i.fa.fa-inbox
+		.resolved-comments-dropdown(
+			ng-class="{ 'resolved-comments-dropdown-open' : state.isOpen }"
+		)
+			.rp-loading(ng-if="isLoading")
+				i.fa.fa-spinner.fa-spin
+			.resolved-comments-scroller(
+				ng-if="!isLoading"
+			)
+				resolved-comment-entry(
+					ng-repeat="thread in resolvedComments | orderBy:'resolved_at':true"
+					thread="thread"
+					on-unresolve="handleUnresolve(threadId);"
+					on-delete="handleDelete(entryId, threadId);"
+					permissions="permissions"
+				)
+				.rp-loading(ng-if="!resolvedComments.length")
+					| No resolved threads.
+			

services/web/public/coffee/ide.coffee

@@ -57,9 +57,6 @@ define [
 			else
 				this.$originalApply(fn);
 
-		if window.location.search.match /tcon=true/ # track changes on
-			$scope.trackChangesFeatureFlag = true
-	
 		$scope.state = {
 			loading: true
 			load_progress: 40
@@ -70,7 +67,7 @@ define [
 			view: "editor"
 			chatOpen: false
 			pdfLayout: 'sideBySide'
-			reviewPanelOpen: localStorage("ui.reviewPanelOpen.#{window.project_id}") and $scope.trackChangesFeatureFlag
+			reviewPanelOpen: localStorage("ui.reviewPanelOpen.#{window.project_id}")
 			showCodeCheckerOnboarding: !window.userSettings.syntaxValidation?
 		}
 		$scope.user = window.user
@@ -86,6 +83,7 @@ define [
 
 		ide.toggleReviewPanel = $scope.toggleReviewPanel = () ->
 			$scope.ui.reviewPanelOpen = !$scope.ui.reviewPanelOpen
+			event_tracking.sendMB "rp-toggle-panel", { value : $scope.ui.reviewPanelOpen }
 
 		$scope.$watch "ui.reviewPanelOpen", (value) ->
 			if value?

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

services/web/public/coffee/ide/directives/layout.coffee

@@ -117,5 +117,10 @@ define [
 								element.layout().hide("east")
 							else
 								element.layout().show("east")
+
+				post: (scope, element, attrs) ->
+					name = attrs.layout
+					state = element.layout().readState()
+					scope.$broadcast "layout:#{name}:linked", state
 		}
 	]

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

@@ -1,7 +1,8 @@
 define [
 	"utils/EventEmitter"
 	"ide/editor/ShareJsDoc"
-], (EventEmitter, ShareJsDoc) ->
+	"ide/review-panel/RangesTracker"
+], (EventEmitter, ShareJsDoc, RangesTracker) ->
 	class Document extends EventEmitter
 		@getDocument: (ide, doc_id) ->
 			@openDocs ||= {}
@@ -40,6 +41,8 @@ define [
 			editorDoc = @ace?.getSession().getDocument()
 			editorDoc?.off "change", @_checkConsistency
 			@ide.$scope.$emit 'document:closed', @doc
+		
+		submitOp: (args...) -> @doc?.submitOp(args...)
 
 		_checkConsistency: () ->
 			# We've been seeing a lot of errors when I think there shouldn't be
@@ -77,6 +80,12 @@ define [
 
 		hasBufferedOps: () ->
 			@doc?.hasBufferedOps()
+		
+		setTrackingChanges: (track_changes) ->
+			@doc.track_changes = track_changes
+		
+		setTrackChangesIdSeeds: (id_seeds) ->
+			@doc.track_changes_id_seeds = id_seeds
 
 		_bindToSocketEvents: () ->
 			@_onUpdateAppliedHandler = (update) => @_onUpdateApplied(update)
@@ -239,16 +248,18 @@ define [
 
 		_joinDoc: (callback = (error) ->) ->
 			if @doc?
-				@ide.socket.emit 'joinDoc', @doc_id, @doc.getVersion(), (error, docLines, version, updates) =>
+				@ide.socket.emit 'joinDoc', @doc_id, @doc.getVersion(), (error, docLines, version, updates, ranges) =>
 					return callback(error) if error?
 					@joined = true
 					@doc.catchUp( updates )
+					@_catchUpRanges( ranges?.changes, ranges?.comments )
 					callback()
 			else
-				@ide.socket.emit 'joinDoc', @doc_id, (error, docLines, version) =>
+				@ide.socket.emit 'joinDoc', @doc_id, (error, docLines, version, updates, ranges) =>
 					return callback(error) if error?
 					@joined = true
 					@doc = new ShareJsDoc @doc_id, docLines, version, @ide.socket
+					@ranges = new RangesTracker(ranges?.changes, ranges?.comments)
 					@_bindToShareJsDocEvents()
 					callback()
 
@@ -307,6 +318,10 @@ define [
 					inflightOp: inflightOp,
 					pendingOp: pendingOp
 					v: version
+			@doc.on "change", (ops, oldSnapshot, msg) =>
+				@_applyOpsToRanges(ops, oldSnapshot, msg)
+			@doc.on "flipped_pending_to_inflight", () =>
+				@trigger "flipped_pending_to_inflight"
 
 		_onError: (error, meta = {}) ->
 			meta.doc_id = @doc_id
@@ -319,3 +334,34 @@ define [
 			# the disconnect event, which means we try to leaveDoc when the connection comes back.
 			# This could intefere with the new connection of a new instance of this document.
 			@_cleanUp()
+
+		_applyOpsToRanges: (ops = [], oldSnapshot, msg) ->
+			track_changes_as = null
+			remote_op = msg?
+			if msg?.meta?.tc?
+				old_id_seed = @ranges.getIdSeed()
+				@ranges.setIdSeed(msg.meta.tc)
+			if remote_op and msg.meta?.tc
+				track_changes_as = msg.meta.user_id
+			else if !remote_op and @track_changes_as?
+				track_changes_as = @track_changes_as
+			@ranges.track_changes = track_changes_as?
+			for op in ops
+				@ranges.applyOp op, { user_id: track_changes_as }
+			if old_id_seed?
+				@ranges.setIdSeed(old_id_seed)
+		
+		_catchUpRanges: (changes = [], comments = []) ->
+			# We've just been given the current server's ranges, but need to apply any local ops we have.
+			# Reset to the server state then apply our local ops again.
+			@ranges.emit "clear"
+			@ranges.changes = changes
+			@ranges.comments = comments
+			@ranges.track_changes = @doc.track_changes
+			for op in @doc.getInflightOp() or []
+				@ranges.setIdSeed(@doc.track_changes_id_seeds.inflight)
+				@ranges.applyOp(op, { user_id: @track_changes_as })
+			for op in @doc.getPendingOp() or []
+				@ranges.setIdSeed(@doc.track_changes_id_seeds.pending)
+				@ranges.applyOp(op, { user_id: @track_changes_as })
+			@ranges.emit "redraw"

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

@@ -10,6 +10,8 @@ define [
 				open_doc_id: null
 				open_doc_name: null
 				opening: true
+				trackChanges: false
+				wantTrackChanges: window.trackChangesEnabled
 			}
 
 			@$scope.$on "entity:selected", (event, entity) =>
@@ -31,6 +33,10 @@ define [
 
 			@$scope.$on "flush-changes", () =>
 				Document.flushAll()
+			
+			@$scope.$watch "editor.wantTrackChanges", (value) =>
+				return if !value?
+				@_syncTrackChangesState(@$scope.editor.sharejs_doc)
 
 		autoOpenDoc: () ->
 			open_doc_id = 
@@ -83,6 +89,8 @@ define [
 						"Sorry, something went wrong opening this document. Please try again."
 					)
 					return
+				
+				@_syncTrackChangesState(sharejs_doc)
 
 				@$scope.$broadcast "doc:opened"
 
@@ -144,3 +152,25 @@ define [
 			
 		stopIgnoringExternalUpdates: () ->
 			@_ignoreExternalUpdates = false
+		
+		_syncTimeout: null
+		_syncTrackChangesState: (doc) ->
+			return if !doc?
+
+			if @_syncTimeout?
+				clearTimeout @_syncTimeout
+				@_syncTimeout = null
+
+			want = @$scope.editor.wantTrackChanges
+			have = @$scope.editor.trackChanges
+			if want == have
+				return
+
+			do tryToggle = () =>
+				saved = !doc.getInflightOp()? and !doc.getPendingOp()?
+				if saved
+					doc.setTrackingChanges(want)
+					@$scope.$apply () =>
+						@$scope.editor.trackChanges = want
+				else
+					@_syncTimeout = setTimeout tryToggle, 100

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

@@ -9,21 +9,9 @@ define [
 			# Dencode any binary bits of data
 			# See http://ecmanaut.blogspot.co.uk/2006/07/encoding-decoding-utf8-in-javascript.html
 			@type = "text"
-			docLines = for line in docLines
-				if line.text?
-					@type = "json"
-					line.text = decodeURIComponent(escape(line.text))
-				else
-					@type = "text"
-					line = decodeURIComponent(escape(line))
-				line
-
-			if @type == "text"
-				snapshot = docLines.join("\n")
-			else if @type == "json"
-				snapshot = { lines: docLines }
-			else
-				throw new Error("Unknown type: #{@type}")
+			docLines = (decodeURIComponent(escape(line)) for line in docLines)
+			snapshot = docLines.join("\n")
+			@track_changes = false
 
 			@connection = {
 				send: (update) =>
@@ -34,6 +22,9 @@ define [
 					if window.dropUpdates? and Math.random() < window.dropUpdates
 						sl_console.log "Simulating a lost update", update
 						return
+					if @track_changes
+						update.meta ?= {}
+						update.meta.tc = @track_changes_id_seeds.inflight
 					@socket.emit "applyOtUpdate", @doc_id, update, (error) =>
 						return @_handleError(error) if error?
 				state: "ok"
@@ -43,8 +34,8 @@ define [
 			@_doc = new ShareJs.Doc @connection, @doc_id,
 				type: @type
 			@_doc.setFlushDelay(SINGLE_USER_FLUSH_DELAY)
-			@_doc.on "change", () =>
-				@trigger "change"
+			@_doc.on "change", (args...) =>
+				@trigger "change", args...
 			@_doc.on "acknowledge", () =>
 				@lastAcked = new Date() # note time of last ack from server for an op we sent
 				@trigger "acknowledge"
@@ -53,6 +44,8 @@ define [
 				# ops as quickly as possible for low latency.
 				@_doc.setFlushDelay(0)
 				@trigger "remoteop", args...
+			@_doc.on "flipped_pending_to_inflight", () =>
+				@trigger "flipped_pending_to_inflight"
 			@_doc.on "error", (e) =>
 				@_handleError(e)
 
@@ -70,6 +63,7 @@ define [
 				@_doc._onMessage message
 			catch error
 				# Version mismatches are thrown as errors
+				console.log error
 				@_handleError(error)
 
 			if message?.meta?.type == "external"
@@ -125,7 +119,7 @@ define [
 
 		attachToAce: (ace) -> @_doc.attach_ace(ace, false, window.maxDocLength)
 		detachFromAce: () -> @_doc.detach_ace?()
-	
+
 		INFLIGHT_OP_TIMEOUT: 5000 # Retry sending ops after 5 seconds without an ack
 		WAIT_FOR_CONNECTION_TIMEOUT: 500 # If we're waiting for the project to join, try again in 0.5 seconds
 		_startInflightOpTimeout: (update) ->

services/web/public/coffee/ide/editor/directives/aceEditor.coffee

@@ -54,10 +54,10 @@ define [
 				syntaxValidation: "="
 				reviewPanel: "="
 				eventsBridge: "="
-				trackNewChanges: "="
+				trackChanges: "="
 				trackChangesEnabled: "="
-				changesTracker: "="
 				docId: "="
+				rendererData: "="
 			}
 			link: (scope, element, attrs) ->
 				# Don't freak out if we're already in an apply callback
@@ -318,6 +318,14 @@ define [
 					
 					doc = session.getDocument()
 					doc.off "change", onChange
+				
+				editor.renderer.on "changeCharacterSize", () ->
+					scope.$apply () ->
+						scope.rendererData.lineHeight = editor.renderer.lineHeight
+				
+				scope.$watch "rendererData", (rendererData) ->
+					if rendererData?
+						rendererData.lineHeight = editor.renderer.lineHeight
 
 			template: """
 				<div class="ace-editor-wrapper">

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

@@ -10,18 +10,18 @@ define [
 		constructor: (@$scope, @editor, @element) ->
 			window.trackChangesManager ?= @
 
-			@$scope.$watch "changesTracker", (changesTracker) =>
-				return if !changesTracker?
-				@disconnectFromChangesTracker()
-				@changesTracker = changesTracker
-				@connectToChangesTracker()
-
-			@$scope.$watch "trackNewChanges", (track_new_changes) =>
-				return if !track_new_changes?
-				@changesTracker?.track_changes = track_new_changes
+			@$scope.$watch "trackChanges", (track_changes) =>
+				return if !track_changes?
+				@setTrackChanges(track_changes)
+			
+			@$scope.$watch "sharejsDoc", (doc) =>
+				return if !doc?
+				@disconnectFromRangesTracker()
+				@rangesTracker = doc.ranges
+				@connectToRangesTracker()
 			
-			@$scope.$on "comment:add", (e, comment) =>
-				@addCommentToSelection(comment)
+			@$scope.$on "comment:add", (e, thread_id) =>
+				@addCommentToSelection(thread_id)
 
 			@$scope.$on "comment:select_line", (e) =>
 				@selectLineIfNoSelection()
@@ -35,11 +35,11 @@ define [
 			@$scope.$on "comment:remove", (e, comment_id) =>
 				@removeCommentId(comment_id)
 			
-			@$scope.$on "comment:resolve", (e, comment_id, user_id) =>
-				@resolveCommentId(comment_id, user_id)
+			@$scope.$on "comment:resolve_thread", (e, thread_id) =>
+				@resolveCommentByThreadId(thread_id)
 			
-			@$scope.$on "comment:unresolve", (e, comment_id) =>
-				@unresolveCommentId(comment_id)
+			@$scope.$on "comment:unresolve_thread", (e, thread_id) =>
+				@unresolveCommentByThreadId(thread_id)
 			
 			@$scope.$on "review-panel:recalculate-screen-positions", () =>
 				@recalculateReviewEntriesScreenPositions()
@@ -58,48 +58,16 @@ define [
 			onResize = () =>
 				@recalculateReviewEntriesScreenPositions()
 
-			onChange = (e) =>
-				if !@editor.initing
-					# This change is trigger by a sharejs 'change' event, which is before the
-					# sharejs 'remoteop' event. So wait until the next event loop when the 'remoteop'
-					# will have fired, before we decide if it was a remote op.
-					setTimeout () =>
-						if @nextUpdateMetaData?
-							user_id = @nextUpdateMetaData.user_id
-							# The remote op may have contained multiple atomic ops, each of which is an Ace
-							# 'change' event (i.e. bulk commenting out of lines is a single remote op
-							# but gives us one event for each % inserted). These all come in a single event loop
-							# though, so wait until the next one before clearing the metadata.
-							setTimeout () =>
-								@nextUpdateMetaData = null
-						else
-							user_id = window.user.id
-						
-						was_tracking = @changesTracker.track_changes
-						if @dont_track_next_update
-							@changesTracker.track_changes = false
-							@dont_track_next_update = false
-						@applyChange(e, { user_id })
-						@changesTracker.track_changes = was_tracking
-						
-						# TODO: Just for debugging, remove before going live.
-						setTimeout () =>
-							@checkMapping()
-						, 100
-
 			onChangeSession = (e) =>
-				e.oldSession?.getDocument().off "change", onChange
-				e.session.getDocument().on "change", onChange
+				@clearAnnotations()
 				@redrawAnnotations()
 
 			bindToAce = () =>
-				@editor.getSession().getDocument().on "change", onChange
 				@editor.on "changeSelection", onChangeSelection
 				@editor.on "changeSession", onChangeSession
 				@editor.renderer.on "resize", onResize
 
 			unbindFromAce = () =>
-				@editor.getSession().getDocument().off "change", onChange
 				@editor.off "changeSelection", onChangeSelection
 				@editor.off "changeSession", onChangeSession
 				@editor.renderer.off "resize", onResize
@@ -111,94 +79,99 @@ define [
 				else
 					unbindFromAce()
 		
-		disconnectFromChangesTracker: () ->
+		disconnectFromRangesTracker: () ->
 			@changeIdToMarkerIdMap = {}
 
-			if @changesTracker?
-				@changesTracker.off "insert:added"
-				@changesTracker.off "insert:removed"
-				@changesTracker.off "delete:added"
-				@changesTracker.off "delete:removed"
-				@changesTracker.off "changes:moved"
-				@changesTracker.off "comment:added"
-				@changesTracker.off "comment:moved"
-				@changesTracker.off "comment:removed"
-				@changesTracker.off "comment:resolved"
-				@changesTracker.off "comment:unresolved"
+			if @rangesTracker?
+				@rangesTracker.off "insert:added"
+				@rangesTracker.off "insert:removed"
+				@rangesTracker.off "delete:added"
+				@rangesTracker.off "delete:removed"
+				@rangesTracker.off "changes:moved"
+				@rangesTracker.off "comment:added"
+				@rangesTracker.off "comment:moved"
+				@rangesTracker.off "comment:removed"
+
+		setTrackChanges: (value) ->
+			if value
+				@$scope.sharejsDoc?.track_changes_as = window.user.id or "anonymous"
+			else
+				@$scope.sharejsDoc?.track_changes_as = null
 		
-		connectToChangesTracker: () ->
-			@changesTracker.track_changes = @$scope.trackNewChanges
+		connectToRangesTracker: () ->
+			@setTrackChanges(@$scope.trackChanges)
 			
-			@changesTracker.on "insert:added", (change) =>
+			# Add a timeout because on remote ops, we get these notifications before
+			# ace has updated
+			@rangesTracker.on "insert:added", (change) =>
 				sl_console.log "[insert:added]", change
-				@_onInsertAdded(change)
-			@changesTracker.on "insert:removed", (change) =>
+				setTimeout () => @_onInsertAdded(change)
+			@rangesTracker.on "insert:removed", (change) =>
 				sl_console.log "[insert:removed]", change
-				@_onInsertRemoved(change)
-			@changesTracker.on "delete:added", (change) =>
+				setTimeout () => @_onInsertRemoved(change)
+			@rangesTracker.on "delete:added", (change) =>
 				sl_console.log "[delete:added]", change
-				@_onDeleteAdded(change)
-			@changesTracker.on "delete:removed", (change) =>
+				setTimeout () => @_onDeleteAdded(change)
+			@rangesTracker.on "delete:removed", (change) =>
 				sl_console.log "[delete:removed]", change
-				@_onDeleteRemoved(change)
-			@changesTracker.on "changes:moved", (changes) =>
+				setTimeout () => @_onDeleteRemoved(change)
+			@rangesTracker.on "changes:moved", (changes) =>
 				sl_console.log "[changes:moved]", changes
-				@_onChangesMoved(changes)
+				setTimeout () => @_onChangesMoved(changes)
 
-			@changesTracker.on "comment:added", (comment) =>
+			@rangesTracker.on "comment:added", (comment) =>
 				sl_console.log "[comment:added]", comment
-				@_onCommentAdded(comment)
-			@changesTracker.on "comment:moved", (comment) =>
+				setTimeout () => @_onCommentAdded(comment)
+			@rangesTracker.on "comment:moved", (comment) =>
 				sl_console.log "[comment:moved]", comment
-				@_onCommentMoved(comment)
-			@changesTracker.on "comment:removed", (comment) =>
+				setTimeout () => @_onCommentMoved(comment)
+			@rangesTracker.on "comment:removed", (comment) =>
 				sl_console.log "[comment:removed]", comment
-				@_onCommentRemoved(comment)
-			@changesTracker.on "comment:resolved", (comment) =>
-				sl_console.log "[comment:resolved]", comment
-				@_onCommentRemoved(comment)
-			@changesTracker.on "comment:unresolved", (comment) =>
-				sl_console.log "[comment:unresolved]", comment
-				@_onCommentAdded(comment)
+				setTimeout () => @_onCommentRemoved(comment)
 			
+			@rangesTracker.on "clear", () =>
+				@clearAnnotations()
+			@rangesTracker.on "redraw", () =>
+				@redrawAnnotations()
+		
+		clearAnnotations: () ->
+			session = @editor.getSession()
+			for change_id, markers of @changeIdToMarkerIdMap
+				for marker_name, marker_id of markers
+					session.removeMarker marker_id
+			@changeIdToMarkerIdMap = {}
+
 		redrawAnnotations: () ->
-			for change in @changesTracker.changes
+			for change in @rangesTracker.changes
 				if change.op.i?
 					@_onInsertAdded(change)
 				else if change.op.d?
 					@_onDeleteAdded(change)
 
-			for comment in @changesTracker.comments
+			for comment in @rangesTracker.comments
 				@_onCommentAdded(comment)
 
-		addComment: (offset, length, content) ->
-			@changesTracker.addComment offset, length, {
-				thread: [{
-					content: content
-					user_id: window.user_id
-					ts: new Date()
-				}]
-			}
+		addComment: (offset, content, thread_id) ->
+			op = { c: content, p: offset, t: thread_id }
+			# @rangesTracker.applyOp op # Will apply via sharejs
+			@$scope.sharejsDoc.submitOp op
 		
-		addCommentToSelection: (content) ->
+		addCommentToSelection: (thread_id) ->
 			range = @editor.getSelectionRange()
+			content = @editor.getSelectedText()
 			offset = @_aceRangeToShareJs(range.start)
-			end = @_aceRangeToShareJs(range.end)
-			length = end - offset
-			@addComment(offset, length, content)
+			@addComment(offset, content, thread_id)
 		
 		selectLineIfNoSelection: () ->
 			if @editor.selection.isEmpty()
 				@editor.selection.selectLine()
 		
 		acceptChangeId: (change_id) ->
-			@changesTracker.removeChangeId(change_id)
+			@rangesTracker.removeChangeId(change_id)
 		
 		rejectChangeId: (change_id) ->
-			change = @changesTracker.getChange(change_id)
+			change = @rangesTracker.getChange(change_id)
 			return if !change?
-			@changesTracker.removeChangeId(change_id)
-			@dont_track_next_update = true
 			session = @editor.getSession()
 			if change.op.d?
 				content = change.op.d
@@ -215,17 +188,20 @@ define [
 				throw new Error("unknown change: #{JSON.stringify(change)}")
 
 		removeCommentId: (comment_id) ->
-			@changesTracker.removeCommentId(comment_id)
+			@rangesTracker.removeCommentId(comment_id)
 
-		resolveCommentId: (comment_id, user_id) ->
-			@changesTracker.resolveCommentId(comment_id, {
-				user_id, ts: new Date()
-			})
+		resolveCommentByThreadId: (thread_id) ->
+			for comment in @rangesTracker?.comments or []
+				if comment.op.t == thread_id
+					@_onCommentRemoved(comment)
 			
-		unresolveCommentId: (comment_id) ->
-			@changesTracker.unresolveCommentId(comment_id)
+		unresolveCommentByThreadId: (thread_id) ->
+			for comment in @rangesTracker?.comments or []
+				if comment.op.t == thread_id
+					@_onCommentAdded(comment)
 
 		checkMapping: () ->
+			# TODO: reintroduce this check
 			session = @editor.getSession()
 
 			# Make a copy of session.getMarkers() so we can modify it
@@ -234,7 +210,7 @@ define [
 				markers[marker_id] = marker
 
 			expected_markers = []
-			for change in @changesTracker.changes
+			for change in @rangesTracker.changes
 				if @changeIdToMarkerIdMap[change.id]?
 					op = change.op
 					{background_marker_id, callout_marker_id} = @changeIdToMarkerIdMap[change.id]
@@ -246,11 +222,11 @@ define [
 					expected_markers.push { marker_id: background_marker_id, start, end }
 					expected_markers.push { marker_id: callout_marker_id, start, end: start }
 			
-			for comment in @changesTracker.comments
+			for comment in @rangesTracker.comments
 				if @changeIdToMarkerIdMap[comment.id]?
 					{background_marker_id, callout_marker_id} = @changeIdToMarkerIdMap[comment.id]
-					start = @_shareJsOffsetToAcePosition(comment.offset)
-					end = @_shareJsOffsetToAcePosition(comment.offset + comment.length)
+					start = @_shareJsOffsetToAcePosition(comment.op.p)
+					end = @_shareJsOffsetToAcePosition(comment.op.p + comment.op.c.length)
 					expected_markers.push { marker_id: background_marker_id, start, end }
 					expected_markers.push { marker_id: callout_marker_id, start, end: start }
 			
@@ -267,16 +243,13 @@ define [
 				if marker.clazz.match("track-changes")
 					console.error "Orphaned ace marker", marker
 		
-		applyChange: (delta, metadata) ->
-			op = @_aceChangeToShareJs(delta)
-			@changesTracker.applyOp(op, metadata)
-		
 		updateFocus: () ->
 			selection = @editor.getSelectionRange()
-			cursor_offset = @_aceRangeToShareJs(selection.start)
+			selection_start = @_aceRangeToShareJs(selection.start)
+			selection_end = @_aceRangeToShareJs(selection.end)
 			entries = @_getCurrentDocEntries()
-			selection = !(selection.start.column == selection.end.column and selection.start.row == selection.end.row)
-			@$scope.$emit "editor:focus:changed", cursor_offset, selection
+			is_selection = (selection_start != selection_end)
+			@$scope.$emit "editor:focus:changed", selection_start, selection_end, is_selection
 		
 		broadcastChange: () ->
 			@$scope.$emit "editor:track-changes:changed", @$scope.docId
@@ -364,10 +337,13 @@ define [
 			@broadcastChange()
 		
 		_onCommentAdded: (comment) ->
+			if @rangesTracker.resolvedThreadIds[comment.op.t]
+				# Comment is resolved so shouldn't be displayed.
+				return
 			if !@changeIdToMarkerIdMap[comment.id]?
 				# Only create new markers if they don't already exist
-				start = @_shareJsOffsetToAcePosition(comment.offset)
-				end = @_shareJsOffsetToAcePosition(comment.offset + comment.length)
+				start = @_shareJsOffsetToAcePosition(comment.op.p)
+				end = @_shareJsOffsetToAcePosition(comment.op.p + comment.op.c.length)
 				session = @editor.getSession()
 				doc = session.getDocument()
 				background_range = new Range(start.row, start.column, end.row, end.column)
@@ -412,8 +388,8 @@ define [
 			@broadcastChange()
 		
 		_onCommentMoved: (comment) ->
-			start = @_shareJsOffsetToAcePosition(comment.offset)
-			end = @_shareJsOffsetToAcePosition(comment.offset + comment.length)
+			start = @_shareJsOffsetToAcePosition(comment.op.p)
+			end = @_shareJsOffsetToAcePosition(comment.op.p + comment.op.c.length)
 			@_updateMarker(comment.id, start, end)
 			@editor.renderer.updateBackMarkers()
 			@broadcastChange()
@@ -423,11 +399,11 @@ define [
 			session = @editor.getSession()
 			markers = session.getMarkers()
 			{background_marker_id, callout_marker_id} = @changeIdToMarkerIdMap[change_id]
-			if background_marker_id?
+			if background_marker_id? and markers[background_marker_id]?
 				background_marker = markers[background_marker_id]
 				background_marker.range.start = start
 				background_marker.range.end = end
-			if callout_marker_id?
+			if callout_marker_id? and markers[callout_marker_id]?
 				callout_marker = markers[callout_marker_id]
 				callout_marker.range.start = start
 				callout_marker.range.end = start

services/web/public/coffee/ide/editor/sharejs/vendor/client/doc.coffee

@@ -71,7 +71,7 @@ class Doc
     # Its important that these event handlers are called with oldSnapshot.
     # The reason is that the OT type APIs might need to access the snapshots to
     # determine information about the received op.
-    @emit 'change', docOp, oldSnapshot
+    @emit 'change', docOp, oldSnapshot, msg
     @emit 'remoteop', docOp, oldSnapshot, msg if isRemote
   
   _connectionStateChanged: (state, data) ->
@@ -266,6 +266,8 @@ class Doc
     @pendingOp = null
     @pendingCallbacks = []
 
+    @emit "flipped_pending_to_inflight"
+
     #console.log "SENDING OP TO SERVER", @inflightOp, @version
     @connection.send {doc:@name, op:@inflightOp, v:@version}
 
@@ -274,6 +276,7 @@ class Doc
   submitOp: (op, callback) ->
     op = @type.normalize(op) if @type.normalize?
 
+    oldSnapshot = @snapshot
     # If this throws an exception, no changes should have been made to the doc
     @snapshot = @type.apply @snapshot, op
 
@@ -284,7 +287,7 @@ class Doc
 
     @pendingCallbacks.push callback if callback
 
-    @emit 'change', op
+    @emit 'change', op, oldSnapshot
 
     @delayedFlush()
 

services/web/public/coffee/ide/editor/sharejs/vendor/types/text-api.coffee

@@ -28,5 +28,5 @@ text.api =
       for component in op
         if component.i != undefined
           @emit 'insert', component.p, component.i
-        else
+        else if component.d != undefined
           @emit 'delete', component.p, component.d

services/web/public/coffee/ide/editor/sharejs/vendor/types/text.coffee

@@ -31,7 +31,8 @@ checkValidComponent = (c) ->
 
   i_type = typeof c.i
   d_type = typeof c.d
-  throw new Error 'component needs an i or d field' unless (i_type == 'string') ^ (d_type == 'string')
+  c_type = typeof c.c
+  throw new Error 'component needs an i, d or c field' unless (i_type == 'string') ^ (d_type == 'string') ^ (c_type == 'string')
 
   throw new Error 'position cannot be negative' unless c.p >= 0
 
@@ -44,11 +45,15 @@ text.apply = (snapshot, op) ->
   for component in op
     if component.i?
       snapshot = strInject snapshot, component.p, component.i
-    else
+    else if component.d?
       deleted = snapshot[component.p...(component.p + component.d.length)]
       throw new Error "Delete component '#{component.d}' does not match deleted text '#{deleted}'" unless component.d == deleted
       snapshot = snapshot[...component.p] + snapshot[(component.p + component.d.length)..]
-  
+    else if component.c?
+      comment = snapshot[component.p...(component.p + component.c.length)]
+      throw new Error "Comment component '#{component.c}' does not match commented text '#{comment}'" unless component.c == comment
+    else
+      throw new Error "Unknown op type"
   snapshot
 
 
@@ -112,7 +117,7 @@ transformPosition = (pos, c, insertAfter) ->
       pos + c.i.length
     else
       pos
-  else
+  else if c.d?
     # I think this could also be written as: Math.min(c.p, Math.min(c.p - otherC.p, otherC.d.length))
     # but I think its harder to read that way, and it compiles using ternary operators anyway
     # so its no slower written like this.
@@ -122,6 +127,10 @@ transformPosition = (pos, c, insertAfter) ->
       c.p
     else
       pos - c.d.length
+  else if c.c?
+    pos
+  else
+    throw new Error("unknown op type")
 
 # Helper method to transform a cursor position as a result of an op.
 #
@@ -143,7 +152,7 @@ text._tc = transformComponent = (dest, c, otherC, side) ->
   if c.i?
     append dest, {i:c.i, p:transformPosition(c.p, otherC, side == 'right')}
 
-  else # Delete
+  else if c.d? # Delete
     if otherC.i? # delete vs insert
       s = c.d
       if c.p < otherC.p
@@ -152,7 +161,7 @@ text._tc = transformComponent = (dest, c, otherC, side) ->
       if s != ''
         append dest, {d:s, p:c.p + otherC.i.length}
 
-    else # Delete vs delete
+    else if otherC.d? # Delete vs delete
       if c.p >= otherC.p + otherC.d.length
         append dest, {d:c.d, p:c.p - otherC.d.length}
       else if c.p + c.d.length <= otherC.p
@@ -177,6 +186,51 @@ text._tc = transformComponent = (dest, c, otherC, side) ->
           # This could be rewritten similarly to insert v delete, above.
           newC.p = transformPosition newC.p, otherC
           append dest, newC
+    
+    else if otherC.c?
+      append dest, c
+    
+    else
+      throw new Error("unknown op type")
+
+  else if c.c? # Comment
+    if otherC.i?
+      if c.p < otherC.p < c.p + c.c.length
+        offset = otherC.p - c.p
+        new_c = (c.c[0..(offset-1)] + otherC.i + c.c[offset...])
+        append dest, {c:new_c, p:c.p, t: c.t}
+      else
+        append dest, {c:c.c, p:transformPosition(c.p, otherC, true), t: c.t}
+    
+    else if otherC.d?
+      if c.p >= otherC.p + otherC.d.length
+        append dest, {c:c.c, p:c.p - otherC.d.length, t: c.t}
+      else if c.p + c.c.length <= otherC.p
+        append dest, c
+      else # Delete overlaps comment
+        # They overlap somewhere.
+        newC = {c:'', p:c.p, t: c.t}
+        if c.p < otherC.p
+          newC.c = c.c[...(otherC.p - c.p)]
+        if c.p + c.c.length > otherC.p + otherC.d.length
+          newC.c += c.c[(otherC.p + otherC.d.length - c.p)..]
+
+        # This is entirely optional - just for a check that the deleted
+        # text in the two ops matches
+        intersectStart = Math.max c.p, otherC.p
+        intersectEnd = Math.min c.p + c.c.length, otherC.p + otherC.d.length
+        cIntersect = c.c[intersectStart - c.p...intersectEnd - c.p]
+        otherIntersect = otherC.d[intersectStart - otherC.p...intersectEnd - otherC.p]
+        throw new Error 'Delete ops delete different text in the same region of the document' unless cIntersect == otherIntersect
+
+        newC.p = transformPosition newC.p, otherC
+        append dest, newC
+    
+    else if otherC.c?
+      append dest, c
+    
+    else
+      throw new Error("unknown op type")
   
   dest
 

services/web/public/coffee/ide/file-tree/FileTreeManager.coffee

@@ -275,6 +275,16 @@ define [
 						doc:  entity
 						path: path
 					}
+			# Keep list ordered by folders, then name
+			@$scope.docs.sort (a,b) ->
+				aDepth = (a.path.match(/\//g) || []).length
+				bDepth = (b.path.match(/\//g) || []).length
+				if aDepth - bDepth != 0
+					return -(aDepth - bDepth) # Deeper path == folder first
+				else if a.path < b.path
+					return -1
+				else
+					return 1
 			
 		getEntityPath: (entity) ->
 			@_getEntityPathInFolder @$scope.rootFolder, entity

services/web/public/coffee/ide/permissions/PermissionsManager.coffee

@@ -5,15 +5,22 @@ define [], () ->
 				read:  false
 				write: false
 				admin: false
+				comment: false
 			@$scope.$watch "permissionsLevel", (permissionsLevel) =>
 
 				if permissionsLevel?
 					if permissionsLevel == "readOnly"
 						@$scope.permissions.read = true
+						@$scope.permissions.comment = true
 					else if permissionsLevel == "readAndWrite"
 						@$scope.permissions.read = true
 						@$scope.permissions.write = true
+						@$scope.permissions.comment = true
 					else if permissionsLevel == "owner"
 						@$scope.permissions.read = true
 						@$scope.permissions.write = true
 						@$scope.permissions.admin = true
+						@$scope.permissions.comment = true
+
+				if @$scope.anonymous
+					@$scope.permissions.comment = false

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

@@ -1,7 +1,5 @@
-define [
-	"utils/EventEmitter"
-], (EventEmitter) ->
-	class ChangesTracker extends EventEmitter
+load = (EventEmitter) ->
+	class RangesTracker extends EventEmitter
 		# The purpose of this class is to track a set of inserts and deletes to a document, like
 		# track changes in Word. We store these as a set of ShareJs style ranges:
 		#   {i: "foo", p: 42} # Insert 'foo' at offset 42
@@ -36,30 +34,34 @@ define [
 		#   * Deletes by another user will consume deletes by the first user
 		#   * 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: () ->
-			# Change objects have the following structure:
-			# {
-			#   id: ... # Uniquely generated by us
-			#   op: { # ShareJs style op tracking the offset (p) and content inserted (i) or deleted (d)
-			#     i: "..."
-			#     p: 42
-			#   }
-			# }
-			#
-			# Ids are used to uniquely identify a change, e.g. for updating it in the database, or keeping in
-			# sync with Ace ranges.
-			@changes = []
-			@comments = []
-			@id = 0
+		constructor: (@changes = [], @comments = []) ->
+			@setIdSeed(RangesTracker.generateIdSeed())
+
+		getIdSeed: () ->
+			return @id_seed
+
+		setIdSeed: (seed) ->
+			@id_seed = seed
+			@id_increment = 0
 		
-		addComment: (offset, length, metadata) ->
-			# TODO: Don't allow overlapping comments?
-			@comments.push comment = {
-				id: @_newId()
-				offset, length, metadata
-			}
-			@emit "comment:added", comment
-			return comment
+		@generateIdSeed: () ->
+			# Generate a the first 18 characters of Mongo ObjectId, leaving 6 for the increment part
+			# Reference: https://github.com/dreampulse/ObjectId.js/blob/master/src/main/javascript/Objectid.js
+			pid = Math.floor(Math.random() * (32767)).toString(16)
+			machine = Math.floor(Math.random() * (16777216)).toString(16)
+			timestamp = Math.floor(new Date().valueOf() / 1000).toString(16)
+			return '00000000'.substr(0, 8 - timestamp.length) + timestamp +
+				'000000'.substr(0, 6 - machine.length) + machine +
+				'0000'.substr(0, 4 - pid.length) + pid
+		
+		@generateId: () ->
+			@generateIdSeed() + "000001"
+
+		newId: () ->
+			@id_increment++
+			increment = @id_increment.toString(16)
+			id = @id_seed + '000000'.substr(0, 6 - increment.length) + increment;
+			return id
 		
 		getComment: (comment_id) ->
 			comment = null
@@ -69,19 +71,6 @@ define [
 					break
 			return comment
 		
-		resolveCommentId: (comment_id, resolved_data) ->
-			comment = @getComment(comment_id)
-			return if !comment?
-			comment.metadata.resolved = true
-			comment.metadata.resolved_data = resolved_data
-			@emit "comment:resolved", comment
-		
-		unresolveCommentId: (comment_id) ->
-			comment = @getComment(comment_id)
-			return if !comment?
-			comment.metadata.resolved = false
-			@emit "comment:unresolved", comment
-		
 		removeCommentId: (comment_id) ->
 			comment = @getComment(comment_id)
 			return if !comment?
@@ -101,7 +90,7 @@ define [
 			return if !change?
 			@_removeChange(change)
 
-		applyOp: (op, metadata) ->
+		applyOp: (op, metadata = {}) ->
 			metadata.ts ?= new Date()
 			# Apply an op that has been applied to the document to our changes to keep them up to date
 			if op.i?
@@ -110,14 +99,32 @@ define [
 			else if op.d?
 				@applyDeleteToChanges(op, metadata)
 				@applyDeleteToComments(op)
+			else if op.c?
+				@addComment(op, metadata)
+			else
+				throw new Error("unknown op type")
+			
+		addComment: (op, metadata) ->
+			# TODO: Don't allow overlapping comments?
+			@comments.push comment = {
+				id: @newId()
+				op: # Copy because we'll modify in place
+					c: op.c
+					p: op.p
+					t: op.t
+				metadata
+			}
+			@emit "comment:added", comment
+			return comment
 		
 		applyInsertToComments: (op) ->
 			for comment in @comments
-				if op.p <= comment.offset
-					comment.offset += op.i.length
+				if op.p <= comment.op.p
+					comment.op.p += op.i.length
 					@emit "comment:moved", comment
-				else if op.p < comment.offset + comment.length
-					comment.length += op.i.length
+				else if op.p < comment.op.p + comment.op.c.length
+					offset = op.p - comment.op.p
+					comment.op.c = comment.op.c[0..(offset-1)] + op.i + comment.op.c[offset...]
 					@emit "comment:moved", comment
 
 		applyDeleteToComments: (op) ->
@@ -125,20 +132,35 @@ define [
 			op_length = op.d.length
 			op_end = op.p + op_length
 			for comment in @comments
-				comment_end = comment.offset + comment.length
-				if op_end <= comment.offset
+				comment_start = comment.op.p
+				comment_end = comment.op.p + comment.op.c.length
+				comment_length = comment_end - comment_start
+				if op_end <= comment_start
 					# delete is fully before comment
-					comment.offset -= op_length
+					comment.op.p -= op_length
 					@emit "comment:moved", comment
 				else if op_start >= comment_end
 					# delete is fully after comment, nothing to do
 				else
 					# delete and comment overlap
-					delete_length_before = Math.max(0, comment.offset - op_start)
-					delete_length_after = Math.max(0, op_end - comment_end)
-					delete_length_overlapping = op_length - delete_length_before - delete_length_after
-					comment.offset = Math.min(comment.offset, op_start)
-					comment.length -= delete_length_overlapping
+					if op_start <= comment_start
+						remaining_before = ""
+					else
+						remaining_before = comment.op.c.slice(0, op_start - comment_start)
+					if op_end >= comment_end
+						remaining_after = ""
+					else
+						remaining_after = comment.op.c.slice(op_end - comment_start)
+					
+					# Check deleted content matches delete op
+					deleted_comment = comment.op.c.slice(remaining_before.length, comment_length - remaining_after.length)
+					offset = Math.max(0, comment_start - op_start)
+					deleted_op_content = op.d.slice(offset).slice(0, deleted_comment.length)
+					if deleted_comment != deleted_op_content
+						throw new Error("deleted content does not match comment content")
+					
+					comment.op.p = Math.min(comment_start, op_start)
+					comment.op.c = remaining_before + remaining_after
 					@emit "comment:moved", comment
 
 		applyInsertToChanges: (op, metadata) ->
@@ -374,12 +396,9 @@ define [
 			if moved_changes.length > 0
 				@emit "changes:moved", moved_changes
 
-		_newId: () ->
-			(@id++).toString()
-
 		_addOp: (op, metadata) ->
 			change = {
-				id: @_newId()
+				id: @newId()
 				op: op
 				metadata: metadata
 			}
@@ -453,3 +472,9 @@ define [
 				else # Only update to the current change if we haven't removed it.
 					previous_change = change
 			return { moved_changes, remove_changes }
+
+if define?
+	define ["utils/EventEmitter"], load
+else
+	EventEmitter = require("events").EventEmitter
+	module.exports = load(EventEmitter)

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

@@ -5,5 +5,8 @@ define [
 	"ide/review-panel/directives/changeEntry"
 	"ide/review-panel/directives/commentEntry"
 	"ide/review-panel/directives/addCommentEntry"
+	"ide/review-panel/directives/resolvedCommentEntry"
+	"ide/review-panel/directives/resolvedCommentsDropdown"
+	"ide/review-panel/filters/notEmpty"
 	"ide/review-panel/filters/orderOverviewEntries"
 ], () ->

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

@@ -2,9 +2,9 @@ define [
 	"base",
 	"utils/EventEmitter"
 	"ide/colors/ColorManager"
-	"ide/review-panel/ChangesTracker"
-], (App, EventEmitter, ColorManager, ChangesTracker) ->
-	App.controller "ReviewPanelController", ($scope, $element, ide, $timeout) ->
+	"ide/review-panel/RangesTracker"
+], (App, EventEmitter, ColorManager, RangesTracker) ->
+	App.controller "ReviewPanelController", ($scope, $element, ide, $timeout, $http, event_tracking) ->
 		$reviewPanelEl = $element.find "#review-panel"
 
 		$scope.SubViews =
@@ -13,131 +13,90 @@ define [
 
 		$scope.reviewPanel =
 			entries: {}
-			trackNewChanges: false
+			resolvedComments: {}
 			hasEntries: false
 			subView: $scope.SubViews.CUR_FILE
 			openSubView: $scope.SubViews.CUR_FILE
+			overview:
+				loading: false
+			dropdown:
+				loading: false
+			commentThreads: {}
+			resolvedThreadIds: {}
+			layoutToLeft: false
+			rendererData: {}
+			loadingThreads: false
+
+		$scope.$on "layout:pdf:linked", (event, state) ->
+			$scope.reviewPanel.layoutToLeft = (state.east?.size < 220 || state.east?.initClosed)
+
+		$scope.$on "layout:pdf:resize", (event, state) ->
+			$scope.reviewPanel.layoutToLeft = (state.east?.size < 220 || state.east?.initClosed)
+
+		$scope.$watch "ui.pdfLayout", (layout) ->
+			$scope.reviewPanel.layoutToLeft = (layout == "flat")
 
 		$scope.commentState =
 			adding: false
 			content: ""
 
+		$scope.users = {}
+
 		$scope.reviewPanelEventsBridge = new EventEmitter()
+		
+		ide.socket.on "new-comment", (thread_id, comment) ->
+			thread = getThread(thread_id)
+			delete thread.submitting
+			thread.messages.push(formatComment(comment))
+			$scope.$apply()
+			$timeout () ->
+				$scope.$broadcast "review-panel:layout"
+		
+		ide.socket.on "accept-change", (doc_id, change_id) ->
+			if doc_id != $scope.editor.open_doc_id
+				getChangeTracker(doc_id).removeChangeId(change_id)
+			else
+				$scope.$broadcast "change:accept", change_id
+			updateEntries(doc_id)
+			$scope.$apply () ->
+		
+		ide.socket.on "resolve-thread", (thread_id, user) ->
+			_onCommentResolved(thread_id, user)
+			
+		ide.socket.on "reopen-thread", (thread_id) ->
+			_onCommentReopened(thread_id)
 
-		changesTrackers = {}
+		rangesTrackers = {}
 
 		getDocEntries = (doc_id) ->
 			$scope.reviewPanel.entries[doc_id] ?= {}
 			return $scope.reviewPanel.entries[doc_id]
 
+		getDocResolvedComments = (doc_id) ->
+			$scope.reviewPanel.resolvedComments[doc_id] ?= {}
+			return $scope.reviewPanel.resolvedComments[doc_id]
+		
+		getThread = (thread_id) ->
+			$scope.reviewPanel.commentThreads[thread_id] ?= { messages: [] }
+			return $scope.reviewPanel.commentThreads[thread_id]
+
 		getChangeTracker = (doc_id) ->
-			changesTrackers[doc_id] ?= new ChangesTracker()
-			return changesTrackers[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]?
-						changesTracker = getChangeTracker(entity.id)
-						for change in mock_changes[entity.name].changes
-							changesTracker._addOp change.op, change.metadata 
-						for comment in mock_changes[entity.name].comments
-							changesTracker.addComment comment.offset, comment.length, comment.metadata 
-						for doc_id, changesTracker of changesTrackers
-							updateEntries(doc_id)
+			if !rangesTrackers[doc_id]?
+				rangesTrackers[doc_id] = new RangesTracker()
+				rangesTrackers[doc_id].resolvedThreadIds = $scope.reviewPanel.resolvedThreadIds
+			return rangesTrackers[doc_id]
 
 		scrollbar = {}
 		$scope.reviewPanelEventsBridge.on "aceScrollbarVisibilityChanged", (isVisible, scrollbarWidth) ->
 			scrollbar = {isVisible, scrollbarWidth}
 			updateScrollbar()
-		
+
 		updateScrollbar = () ->
 			if scrollbar.isVisible and $scope.reviewPanel.subView == $scope.SubViews.CUR_FILE
 				$reviewPanelEl.css "right", "#{ scrollbar.scrollbarWidth }px"
 			else
 				$reviewPanelEl.css "right", "0"
-		
-		$scope.$watch "reviewPanel.subView", (subView) ->
-			return if !subView?
-			updateScrollbar()
-		
+
 		$scope.$watch "ui.reviewPanelOpen", (open) ->
 			return if !open?
 			if !open
@@ -147,33 +106,88 @@ define [
 			else
 				# Reset back to what we had when previously open
 				$scope.reviewPanel.subView = $scope.reviewPanel.openSubView
+		
+		$scope.$watch "reviewPanel.subView", (view) ->
+			return if !view?
+			updateScrollbar()
+			if view == $scope.SubViews.OVERVIEW
+				refreshOverviewPanel()
 
-		$scope.$watch "editor.open_doc_id", (open_doc_id) ->
-			return if !open_doc_id?
-			changesTrackers[open_doc_id] ?= new ChangesTracker()
-			$scope.reviewPanel.changesTracker = changesTrackers[open_doc_id]
+		$scope.$watch "editor.sharejs_doc", (doc, old_doc) ->
+			return if !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
+			rangesTrackers[doc.doc_id].resolvedThreadIds = $scope.reviewPanel.resolvedThreadIds
+			$scope.reviewPanel.rangesTracker = rangesTrackers[doc.doc_id]
+			if old_doc?
+				old_doc.off "flipped_pending_to_inflight"
+			doc.on "flipped_pending_to_inflight", () ->
+				regenerateTrackChangesId(doc)
+			regenerateTrackChangesId(doc)
 
 		$scope.$watch (() ->
 			entries = $scope.reviewPanel.entries[$scope.editor.open_doc_id] or {}
 			Object.keys(entries).length
 		), (nEntries) ->
-			$scope.reviewPanel.hasEntries = nEntries > 0 and $scope.trackChangesFeatureFlag
+			$scope.reviewPanel.hasEntries = nEntries > 0 and $scope.project.features.trackChanges
 
 		$scope.$watch "ui.reviewPanelOpen", (reviewPanelOpen) ->
 			return if !reviewPanelOpen?
 			$timeout () ->
 				$scope.$broadcast "review-panel:toggle"
 				$scope.$broadcast "review-panel:layout"
+
+		regenerateTrackChangesId = (doc) ->
+			old_id = getChangeTracker(doc.doc_id).getIdSeed()
+			new_id = RangesTracker.generateIdSeed()
+			getChangeTracker(doc.doc_id).setIdSeed(new_id)
+			doc.setTrackChangesIdSeeds({pending: new_id, inflight: old_id})
 		
+		refreshRanges = () ->
+			$http.get "/project/#{$scope.project_id}/ranges"
+				.success (docs) ->
+					for doc in docs
+						if doc.id != $scope.editor.open_doc_id # this is kept up to date in real-time, don't overwrite
+							rangesTracker = getChangeTracker(doc.id)
+							rangesTracker.comments = doc.ranges?.comments or []
+							rangesTracker.changes = doc.ranges?.changes or []
+						updateEntries(doc.id)
+
+		refreshOverviewPanel = () ->
+			$scope.reviewPanel.overview.loading = true
+			refreshRanges()
+				.then () ->
+					$scope.reviewPanel.overview.loading = false
+				.catch () ->
+					$scope.reviewPanel.overview.loading = false
+
+		$scope.refreshResolvedCommentsDropdown = () ->
+			$scope.reviewPanel.dropdown.loading = true
+			q = refreshRanges()
+			q.then () ->
+				$scope.reviewPanel.dropdown.loading = false
+			q.catch () ->
+				$scope.reviewPanel.dropdown.loading = false
+			return q
+
 		updateEntries = (doc_id) ->
-			changesTracker = getChangeTracker(doc_id)
+			rangesTracker = getChangeTracker(doc_id)
 			entries = getDocEntries(doc_id)
+			resolvedComments = getDocResolvedComments(doc_id)
 			
+			changed = false
+
 			# Assume we'll delete everything until we see it, then we'll remove it from this object
 			delete_changes = {}
-			delete_changes[change_id] = true for change_id, change of entries
+			for change_id, change of entries
+				if change_id != "add-comment"
+					delete_changes[change_id] = true 
+			for change_id, change of resolvedComments
+				delete_changes[change_id] = true 
 
-			for change in changesTracker.changes
+			for change in rangesTracker.changes
+				changed = true
 				delete delete_changes[change.id]
 				entries[change.id] ?= {}
 					
@@ -189,22 +203,35 @@ define [
 				for key, value of new_entry
 					entries[change.id][key] = value
 
-			for comment in changesTracker.comments
+				if !$scope.users[change.metadata.user_id]?
+					refreshChangeUsers(change.metadata.user_id)
+
+			if rangesTracker.comments.length > 0
+				ensureThreadsAreLoaded()
+
+			for comment in rangesTracker.comments
+				changed = true
 				delete delete_changes[comment.id]
-				entries[comment.id] ?= {}
+				if $scope.reviewPanel.resolvedThreadIds[comment.op.t]
+					new_comment = resolvedComments[comment.id] ?= {}
+				else
+					new_comment = entries[comment.id] ?= {}
 				new_entry = {
 					type: "comment"
-					thread: comment.metadata.thread
-					resolved: comment.metadata.resolved
-					resolved_data: comment.metadata.resolved_data
-					offset: comment.offset
-					length: comment.length
+					thread_id: comment.op.t
+					content: comment.op.c
+					offset: comment.op.p
 				}
 				for key, value of new_entry
-					entries[comment.id][key] = value
+					new_comment[key] = value
 
 			for change_id, _ of delete_changes
+				changed = true
 				delete entries[change_id]
+				delete resolvedComments[change_id]
+			
+			if changed
+				$scope.$broadcast "entries:changed"
 
 		$scope.$on "editor:track-changes:changed", () ->
 			doc_id = $scope.editor.open_doc_id
@@ -212,53 +239,64 @@ define [
 			$scope.$broadcast "review-panel:recalculate-screen-positions"
 			$scope.$broadcast "review-panel:layout"
 		
-		$scope.$on "editor:focus:changed", (e, cursor_offset, selection) ->
+		$scope.$on "editor:focus:changed", (e, selection_offset_start, selection_offset_end, selection) ->
 			doc_id = $scope.editor.open_doc_id
 			entries = getDocEntries(doc_id)
 
-			if !selection
-				delete entries["add-comment"]
-			else
-				entries["add-comment"] = {
-					type: "add-comment"
-					offset: cursor_offset
-				}
+			delete entries["add-comment"]
+			if selection
+				# Only show add comment if we're not already overlapping one
+				overlapping_comment = false
+				for id, entry of entries
+					if entry.type == "comment" and not $scope.reviewPanel.resolvedThreadIds[entry.thread_id]
+						unless entry.offset >= selection_offset_end or entry.offset + entry.content.length <= selection_offset_start
+							overlapping_comment = true
+				if !overlapping_comment
+					entries["add-comment"] = {
+						type: "add-comment"
+						offset: selection_offset_start
+					}
 			
 			for id, entry of entries
-				if entry.type == "comment" and not entry.resolved
-					entry.focused = (entry.offset <= cursor_offset <= entry.offset + entry.length)
+				if entry.type == "comment" and not $scope.reviewPanel.resolvedThreadIds[entry.thread_id]
+					entry.focused = (entry.offset <= selection_offset_start <= entry.offset + entry.content.length)
 				else if entry.type == "insert"
-					entry.focused = (entry.offset <= cursor_offset <= entry.offset + entry.content.length)
+					entry.focused = (entry.offset <= selection_offset_start <= entry.offset + entry.content.length)
 				else if entry.type == "delete"
-					entry.focused = (entry.offset == cursor_offset)
+					entry.focused = (entry.offset == selection_offset_start)
 				else if entry.type == "add-comment" and selection
 					entry.focused = true
 			
 			$scope.$broadcast "review-panel:recalculate-screen-positions"
 			$scope.$broadcast "review-panel:layout"
-		
+
 		$scope.acceptChange = (entry_id) ->
+			$http.post "/project/#{$scope.project_id}/doc/#{$scope.editor.open_doc_id}/changes/#{entry_id}/accept", {_csrf: window.csrfToken}
 			$scope.$broadcast "change:accept", entry_id
+			event_tracking.sendMB "rp-change-accepted", { view: if $scope.ui.reviewPanelOpen then $scope.reviewPanel.subView else 'mini' }
 		
 		$scope.rejectChange = (entry_id) ->
 			$scope.$broadcast "change:reject", entry_id
+			event_tracking.sendMB "rp-change-rejected", { view: if $scope.ui.reviewPanelOpen then $scope.reviewPanel.subView else 'mini' }
 		
 		$scope.startNewComment = () ->
-			# $scope.commentState.adding = true
 			$scope.$broadcast "comment:select_line"
 			$timeout () ->
 				$scope.$broadcast "review-panel:layout"
 		
 		$scope.submitNewComment = (content) ->
-			# $scope.commentState.adding = false
-			$scope.$broadcast "comment:add", content
-			# $scope.commentState.content = ""
+			thread_id = RangesTracker.generateId()
+			thread = getThread(thread_id)
+			thread.submitting = true
+			$scope.$broadcast "comment:add", thread_id
+			$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"
-		
+			event_tracking.sendMB "rp-new-comment", { size: content.length }
+
 		$scope.cancelNewComment = (entry) ->
-			# $scope.commentState.adding = false
-			# $scope.commentState.content = ""
 			$timeout () ->
 				$scope.$broadcast "review-panel:layout"
 		
@@ -267,117 +305,165 @@ 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 {
-				content: entry.replyContent
-				ts: new Date()
-				user_id: window.user_id
-			}
-			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
-			}
+		$scope.submitReply = (entry, entry_id) ->			
+			thread_id = entry.thread_id
+			content   = entry.replyContent
+			$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")
+			
+			trackingMetadata =
+				view: if $scope.ui.reviewPanelOpen then $scope.reviewPanel.subView else 'mini'
+				size: entry.replyContent.length
+				thread: thread_id
+
+			thread = getThread(thread_id)
+			thread.submitting = true
 			entry.replyContent = ""
 			entry.replying = false
 			$timeout () ->
 				$scope.$broadcast "review-panel:layout"
-		
+			event_tracking.sendMB "rp-comment-reply", trackingMetadata
+
 		$scope.cancelReply = (entry) ->
 			entry.replying = false
 			entry.replyContent = ""
 			$scope.$broadcast "review-panel:layout"
 
 		$scope.resolveComment = (entry, entry_id) ->
-			entry.showWhenResolved = false
 			entry.focused = false
-			$scope.$broadcast "comment:resolve", entry_id, window.user_id
+			$http.post "/project/#{$scope.project_id}/thread/#{entry.thread_id}/resolve", {_csrf: window.csrfToken}
+			_onCommentResolved(entry.thread_id, ide.$scope.user)
+			event_tracking.sendMB "rp-comment-resolve", { view: if $scope.ui.reviewPanelOpen then $scope.reviewPanel.subView else 'mini' }
+
+		$scope.unresolveComment = (thread_id) ->
+			_onCommentReopened(thread_id)
+			$http.post "/project/#{$scope.project_id}/thread/#{thread_id}/reopen", {_csrf: window.csrfToken}
+			event_tracking.sendMB "rp-comment-reopen"
 		
-		$scope.unresolveComment = (entry_id) ->
-			$scope.$broadcast "comment:unresolve", entry_id
+		_onCommentResolved = (thread_id, user) ->
+			thread = $scope.reviewPanel.commentThreads[thread_id]
+			thread.resolved = true
+			thread.resolved_by_user = formatUser(user)
+			thread.resolved_at = new Date()
+			$scope.reviewPanel.resolvedThreadIds[thread_id] = true
+			$scope.$broadcast "comment:resolve_thread", thread_id
 		
-		$scope.deleteComment = (entry_id) ->
-			$scope.$broadcast "comment:remove", entry_id
-
-		$scope.showThread = (entry) ->
-			entry.showWhenResolved = true
-			$timeout () ->
-				$scope.$broadcast "review-panel:layout"
+		_onCommentReopened = (thread_id) ->
+			thread = $scope.reviewPanel.commentThreads[thread_id]
+			delete thread.resolved
+			delete thread.resolved_by_user
+			delete thread.resolved_at
+			delete $scope.reviewPanel.resolvedThreadIds[thread_id]
+			$scope.$broadcast "comment:unresolve_thread", thread_id
 
-		$scope.hideThread = (entry) ->
-			entry.showWhenResolved = false
-			$timeout () ->
-				$scope.$broadcast "review-panel:layout"
+		_onCommentDeleted = (thread_id) ->
+			if $scope.reviewPanel.resolvedThreadIds[thread_id]?
+				delete $scope.reviewPanel.resolvedThreadIds[thread_id]
+				
+			delete $scope.reviewPanel.commentThreads[thread_id]
+		
+		$scope.deleteComment = (entry_id, thread_id) ->
+			_onCommentDeleted(thread_id)
+			$scope.$broadcast "comment:remove", entry_id
+			event_tracking.sendMB "rp-comment-delete"
 
 		$scope.setSubView = (subView) -> 
 			$scope.reviewPanel.subView = subView
+			event_tracking.sendMB "rp-subview-change", { subView }
 						
 		$scope.gotoEntry = (doc_id, entry) ->
 			ide.editorManager.openDocId(doc_id, { gotoOffset: entry.offset })
+		
+		$scope.toggleTrackChanges = (value) ->
+			$scope.editor.wantTrackChanges = value
+			$http.post "/project/#{$scope.project_id}/track_changes", {_csrf: window.csrfToken, on: value}
+			event_tracking.sendMB "rp-trackchanges-toggle", { value }
+		
+		ide.socket.on "toggle-track-changes", (value) ->
+			$scope.$apply () ->
+				$scope.editor.wantTrackChanges = value
 
-		DOC_ID_NAMES = {} 
-		$scope.getFileName = (doc_id) ->
-			# This is called a lot and is relatively expensive, so cache the result
-			if !DOC_ID_NAMES[doc_id]?
-				entity = ide.fileTreeManager.findEntityById(doc_id)
-				return if !entity?
-				DOC_ID_NAMES[doc_id] = ide.fileTreeManager.getEntityPath(entity)
-			return DOC_ID_NAMES[doc_id]
-
-		# 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 = {}
-			# 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"
-			}
+		_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
 
-			for member in $scope.project.members.concat($scope.project.owner)
-				if member._id == window.user_id
-					name = "You"
-					isSelf = true
-				else
-					name = "#{member.first_name} #{member.last_name}"
-					isSelf = false
-
-				$scope.users[member._id] = {
-					email: member.email
-					name: name
-					isSelf: isSelf
-					hue: ColorManager.getHueForUserId(member._id)
-					avatar_text: [member.first_name, member.last_name].filter((n) -> n?).map((n) -> n[0]).join ""
+			# Only do one refresh at once
+			if _refreshingRangeUsers
+				return
+			_refreshingRangeUsers = true
+
+			$http.get "/project/#{$scope.project_id}/changes/users"
+				.success (users) ->
+					_refreshingRangeUsers = false
+					$scope.users = {}
+					# Always include ourself, since if we submit an op, we might need to display info
+					# about it locally before it has been flushed through the server
+					if ide.$scope.user?.id?
+						$scope.users[ide.$scope.user.id] = formatUser(ide.$scope.user)
+					for user in users
+						if user.id?
+							$scope.users[user.id] = formatUser(user)
+				.error () ->
+					_refreshingRangeUsers = false
+
+		_threadsLoaded = false
+		ensureThreadsAreLoaded = () ->
+			if _threadsLoaded
+				# We get any updates in real time so only need to load them once.
+				return
+			_threadsLoaded = true
+			$scope.reviewPanel.loadingThreads = true
+			$http.get "/project/#{$scope.project_id}/threads"
+				.success (threads) ->
+					$scope.reviewPanel.loadingThreads = false
+					for thread_id, _ of $scope.reviewPanel.resolvedThreadIds
+						delete $scope.reviewPanel.resolvedThreadIds[thread_id]
+					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
+							thread.resolved_by_user = formatUser(thread.resolved_by_user)
+							$scope.reviewPanel.resolvedThreadIds[thread_id] = true
+					$scope.reviewPanel.commentThreads = threads
+					$timeout () ->
+						$scope.$broadcast "review-panel:layout"
+
+		formatComment = (comment) ->
+			comment.user = formatUser(comment.user)
+			comment.timestamp = new Date(comment.timestamp)
+			return comment
+
+		formatUser = (user) ->
+			id = user?._id or user?.id
+
+			if !id?
+				return {
+					email: null
+					name: "Anonymous"
+					isSelf: false
+					hue: ColorManager.ANONYMOUS_HUE
+					avatar_text: "A"
 				}
-		
-		$scope.$watch "project.members", (members) ->
-			return if !members?
-			refreshUsers()
+			if id == window.user_id
+				name = "You"
+				isSelf = true
+			else
+				name = [user.first_name, user.last_name].filter((n) -> n? and n != "").join(" ")
+				if name == ""
+					name = user.email?.split("@")[0] or "Unknown"
+				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 ""
+			}

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

@@ -17,6 +17,8 @@ define [
 			scope.startNewComment = () ->
 				scope.state.isAdding = true
 				scope.onStartNew()
+				setTimeout () ->
+					scope.$broadcast "comment:new:open"
 
 			scope.cancelNewComment = () ->
 				scope.state.isAdding = false
@@ -25,11 +27,11 @@ define [
 			scope.handleCommentKeyPress = (ev) ->
 				if ev.keyCode == 13 and !ev.shiftKey and !ev.ctrlKey and !ev.metaKey
 					ev.preventDefault()
-					ev.target.blur()
-					scope.submitNewComment()
+					if scope.state.content.length > 0 
+						ev.target.blur()
+						scope.submitNewComment()
 
 			scope.submitNewComment = () ->
-				console.log scope.state.content
 				scope.onSubmit { content: scope.state.content }
 				scope.state.isAdding = false
 				scope.state.content = ""

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

@@ -1,13 +1,25 @@
 define [
 	"base"
 ], (App) ->
-	App.directive "changeEntry", () ->
+	App.directive "changeEntry", ($timeout) ->
 		restrict: "E"
 		templateUrl: "changeEntryTemplate"
 		scope: 
 			entry: "="
 			user: "="
+			permissions: "="
 			onAccept: "&"
 			onReject: "&"
 			onIndicatorClick: "&"
-		
+		link: (scope, element, attrs) ->
+			scope.contentLimit = 40
+			scope.isCollapsed = true
+			scope.needsCollapsing = false
+
+			scope.toggleCollapse = () ->
+				scope.isCollapsed = !scope.isCollapsed
+				$timeout () ->
+					scope.$emit "review-panel:layout"
+
+			scope.$watch "entry.content.length", (contentLength) ->
+				scope.needsCollapsing = contentLength > scope.contentLimit

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

@@ -1,23 +1,29 @@
 define [
 	"base"
 ], (App) ->
-	App.directive "commentEntry", () ->
+	App.directive "commentEntry", ($timeout) ->
 		restrict: "E"
 		templateUrl: "commentEntryTemplate"
 		scope: 
 			entry: "="
-			users: "="
+			threads: "="
+			permissions: "="
 			onResolve: "&"
 			onReply: "&"
 			onIndicatorClick: "&"
-			onDelete: "&"
-			onUnresolve: "&"
-			onShowThread: "&"
-			onHideThread: "&"
 		link: (scope, element, attrs) ->
+			scope.state =
+				animating: false
+
 			scope.handleCommentReplyKeyPress = (ev) ->
 				if ev.keyCode == 13 and !ev.shiftKey and !ev.ctrlKey and !ev.metaKey
 					ev.preventDefault()
-					ev.target.blur()
-					scope.onReply()
-		
+					if scope.entry.replyContent.length > 0 
+						ev.target.blur()
+						scope.onReply()
+			
+			scope.animateAndCallOnResolve = () ->
+				scope.state.animating = true
+				element.find(".rp-entry").css("top", 0)
+				$timeout((() -> scope.onResolve()), 350)
+				return true

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

@@ -0,0 +1,21 @@
+define [
+	"base"
+], (App) ->
+	App.directive "resolvedCommentEntry", () ->
+		restrict: "E"
+		templateUrl: "resolvedCommentEntryTemplate"
+		scope:
+			thread: "="
+			permissions: "="
+			onUnresolve: "&"
+			onDelete: "&"
+		link: (scope, element, attrs) ->
+			scope.contentLimit = 40
+			scope.needsCollapsing = false
+			scope.isCollapsed = true
+
+			scope.toggleCollapse = () ->
+				scope.isCollapsed = !scope.isCollapsed
+
+			scope.$watch "thread.content.length", (contentLength) ->
+				scope.needsCollapsing = contentLength > scope.contentLimit

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

@@ -0,0 +1,58 @@
+define [
+	"base"
+], (App) ->
+	App.directive "resolvedCommentsDropdown", (_) ->
+		restrict: "E"
+		templateUrl: "resolvedCommentsDropdownTemplate"
+		scope: 
+			entries 	: "="
+			threads 	: "="
+			resolvedIds	: "="
+			docs		: "="
+			permissions: "="
+			onOpen		: "&"
+			onUnresolve	: "&"
+			onDelete	: "&"
+			isLoading	: "="
+
+		link: (scope, element, attrs) ->
+			scope.state = 
+				isOpen: false
+
+			scope.toggleOpenState = () ->
+				scope.state.isOpen = !scope.state.isOpen
+				if (scope.state.isOpen)
+					scope.onOpen()
+						.then () -> filterResolvedComments()
+
+			scope.resolvedComments = []
+
+			scope.handleUnresolve = (threadId) ->
+				scope.onUnresolve({ threadId })
+				scope.resolvedComments = scope.resolvedComments.filter (c) -> c.threadId != threadId
+
+			scope.handleDelete = (entryId, threadId) ->
+				scope.onDelete({ entryId, threadId })
+
+			getDocNameById = (docId) ->
+				doc = _.find(scope.docs, (doc) -> doc.doc.id == docId)
+				if doc?
+					return doc.path
+				else 
+					return null
+
+			filterResolvedComments = () ->
+				scope.resolvedComments = []
+
+				for docId, docEntries of scope.entries
+					for entryId, entry of docEntries
+						if entry.type == "comment" and scope.threads[entry.thread_id]?.resolved?
+							resolvedComment = angular.copy scope.threads[entry.thread_id]
+
+							resolvedComment.content = entry.content
+							resolvedComment.threadId = entry.thread_id
+							resolvedComment.entryId = entryId
+							resolvedComment.docId = docId
+							resolvedComment.docName = getDocNameById(docId)
+
+							scope.resolvedComments.push(resolvedComment)

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

@@ -32,6 +32,8 @@ define [
 					
 					return if entries.length == 0
 					
+					line_height = scope.reviewPanel.rendererData.lineHeight
+
 					focused_entry_index = Math.min(previous_focused_entry_index, entries.length - 1)
 					for entry, i in entries
 						if entry.scope.entry.focused
@@ -43,15 +45,34 @@ define [
 					previous_focused_entry_index = focused_entry_index
 					
 					sl_console.log "focused_entry_index", focused_entry_index
-					
-					line_height = 15
-					
-					# Put the focused entry exactly where it wants to be
-					focused_entry_top = Math.max(TOOLBAR_HEIGHT, focused_entry.scope.entry.screenPos.y)
+
+					# As we go backwards, we run the risk of pushing things off the top of the editor.
+					# If we go through the entries before and assume they are as pushed together as they
+					# could be, we can work out the 'ceiling' that each one can't go through. I.e. the first
+					# on can't go beyond the toolbar height, the next one can't go beyond the bottom of the first
+					# one at this minimum height, etc.
+					heights = (entry.$layout_el.height() for entry in entries_before)
+					previousMinTop = TOOLBAR_HEIGHT
+					min_tops = []
+					for height in heights
+						min_tops.push previousMinTop
+						previousMinTop += PADDING + height
+					min_tops.reverse()
+
+					positionLayoutEl = ($callout_el, original_top, top) ->
+						if original_top <= top
+							$callout_el.removeClass("rp-entry-callout-inverted")
+							$callout_el.css(top: original_top + line_height - 1, height: top - original_top)
+						else
+							$callout_el.addClass("rp-entry-callout-inverted")
+							$callout_el.css(top: top + line_height, height: original_top - top)
+
+					# Put the focused entry as close to where it wants to be as possible
+					focused_entry_top = Math.max(previousMinTop, focused_entry.scope.entry.screenPos.y)
 					focused_entry.$box_el.css(top: focused_entry_top)
 					focused_entry.$indicator_el.css(top: focused_entry_top)
-					focused_entry.$callout_el.css(top: focused_entry_top + line_height, height: 0)
-					
+					positionLayoutEl(focused_entry.$callout_el, focused_entry.scope.entry.screenPos.y, focused_entry_top)
+
 					previousBottom = focused_entry_top + focused_entry.$layout_el.height()
 					for entry in entries_after
 						original_top = entry.scope.entry.screenPos.y
@@ -60,23 +81,21 @@ define [
 						previousBottom = top + height
 						entry.$box_el.css(top: top)
 						entry.$indicator_el.css(top: top)
-						entry.$callout_el.removeClass("rp-entry-callout-inverted")
-						entry.$callout_el.css(top: original_top + line_height, height: top - original_top)
+						positionLayoutEl(entry.$callout_el, original_top, top)
 						sl_console.log "ENTRY", {entry: entry.scope.entry, top}
-					
+
 					previousTop = focused_entry_top
 					entries_before.reverse() # Work through backwards, starting with the one just above
-					for entry in entries_before
+					for entry, i in entries_before
 						original_top = entry.scope.entry.screenPos.y
 						height = entry.$layout_el.height()
 						original_bottom = original_top + height
 						bottom = Math.min(original_bottom, previousTop - PADDING)
-						top = bottom - height
+						top = Math.max(bottom - height, min_tops[i])
 						previousTop = top
 						entry.$box_el.css(top: top)
 						entry.$indicator_el.css(top: top)
-						entry.$callout_el.addClass("rp-entry-callout-inverted")
-						entry.$callout_el.css(top: top + line_height + 1, height: original_top - top)
+						positionLayoutEl(entry.$callout_el, original_top, top)
 						sl_console.log "ENTRY", {entry: entry.scope.entry, top}
 				
 				scope.$applyAsync () ->
@@ -85,6 +104,9 @@ define [
 				scope.$on "review-panel:layout", () ->
 					scope.$applyAsync () ->
 						layout()
+				
+				scope.$watch "reviewPanel.rendererData.lineHeight", () ->
+					layout()
 
 				## Scroll lock with Ace
 				scroller = element

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

@@ -4,10 +4,18 @@ define [
 	App.directive "reviewPanelToggle", () ->
 		restrict: "E"
 		scope: 
-			innerModel: '=ngModel'
+			onToggle: '='
+			ngModel: '='
+		link: (scope) ->
+			scope.onChange = (args...) ->
+				scope.onToggle(scope.localModel)
+			scope.localModel = scope.ngModel
+			scope.$watch "ngModel", (value) ->
+				scope.localModel = value
+
 		template: """
 <div class="rp-toggle">
-	<input id="rp-toggle-{{$id}}" type="checkbox" class="rp-toggle-hidden-input" ng-model="innerModel" />
+	<input id="rp-toggle-{{$id}}" type="checkbox" class="rp-toggle-hidden-input" ng-model="localModel" ng-change="onChange()" />
 	<label for="rp-toggle-{{$id}}" class="rp-toggle-btn"></label>
 </div>
 """

services/web/public/coffee/ide/review-panel/filters/notEmpty.coffee

@@ -0,0 +1,5 @@
+define [
+	"base"
+], (App) ->
+	app.filter 'notEmpty', () ->
+		(object) -> !angular.equals({}, object)

services/web/public/stylesheets/app/editor/review-panel.less

@@ -1,42 +1,60 @@
-@rp-base-font-size		: 12px;
-@rp-small-font-size		: 10px;
-@rp-icon-large-size		: 22px;
+@rp-base-font-size      : 12px;
+@rp-small-font-size     : 10px;
+@rp-icon-large-size     : 18px;
 
-@rp-bg-blue				: #dadfed;
-@rp-bg-dim-blue			: #fafafa;
-@rp-highlight-blue		: #8a96b5;
+@rp-bg-blue             : #dadfed;
+@rp-bg-dim-blue         : #fafafa;
+@rp-highlight-blue      : #8a96b5;
 
-@rp-border-grey			: #d9d9d9;
+@rp-border-grey         : #d9d9d9;
 
-@rp-green				: #2c8e30;
-@rp-dim-green			: #cae3cb;
-@rp-red					: #c5060b;
-@rp-dim-red				: #f3cdce;
-@rp-yellow				: #f3b111;
-@rp-dim-yellow			: #ffe9b2;
-@rp-grey				: #aaaaaa;
+@rp-green               : #2c8e30;
+@rp-dim-green           : #cae3cb;
+@rp-green-on-dark		: rgba(37, 107, 41, 0.5);
+@rp-red                 : #c5060b;
+@rp-dim-red             : #f3cdce;
+@rp-yellow              : #f3b111;
+@rp-yellow-on-dark		: rgba(194, 93, 11, 0.5);
+@rp-dim-yellow          : #ffe9b2;
+@rp-grey                : #aaaaaa;
 
-@rp-type-blue			: #6b7797;
-@rp-type-darkgrey		: #3f3f3f;
+@rp-type-blue           : #6b7797;
+@rp-type-darkgrey       : #3f3f3f;
+
+@rp-entry-ribbon-width  : 4px;
+@rp-entry-arrow-width   : 6px;
+@rp-semibold-weight     : 600;
+@review-panel-width     : 230px;
+@review-off-width       : 22px;
+
+@rp-toolbar-height      : 32px;
 
-@rp-entry-ribbon-width	: 4px;
-@rp-entry-arrow-width	: 6px;
-@rp-semibold-weight		: 600;
-@review-panel-width		: 230px;
-@review-off-width		: 22px;
 
-@rp-toolbar-height: 32px;
 
 .rp-button() {
 	background-color: @rp-highlight-blue;
 	color: #FFF;
 	text-align: center;
+	line-height: 1.3;
+	user-select: none;
+	border: 0;
+
 	&:hover,
 	&:focus {
+		outline: 0;
 		background-color: darken(@rp-highlight-blue, 5%);
 		text-decoration: none;
 		color: #FFF;
 	}
+
+	&[disabled] {
+		opacity: 0.5;
+
+		&:hover,
+		&:focus {
+			background-color: @rp-highlight-blue;
+		}
+	}
 }
 
 .triangle(@_, @width, @height, @color) {
@@ -82,6 +100,7 @@
 	.rp-size-mini & {
 		display: block;
 		width: @review-off-width;
+		z-index: 6;
 	}
 	
 	position: absolute;
@@ -92,6 +111,7 @@
 	border-left: solid 1px @rp-border-grey;
 	font-size: @rp-base-font-size;
 	color: @rp-type-blue;
+	z-index: 6;
 }
 
 .review-panel-toolbar {
@@ -99,26 +119,33 @@
 	.rp-size-expanded & {
 		display: flex;
 		align-items: center;
-		justify-content: space-between;
 		padding: 0 5px;
 	}
-	.rp-state-current-file & {
-		position: absolute;
-		top: 0;
-		left: 0;
-		right: 0;
-	}
+	// .rp-state-current-file & {
+	// 	position: absolute;
+	// 	top: 0;
+	// 	left: 0;
+	// 	right: 0;
+	// }
 	height: @rp-toolbar-height;
 	border-bottom: 1px solid @rp-border-grey;
 	background-color: @rp-bg-dim-blue;
 	text-align: center;
-	z-index: 2;
+	z-index: 3;
 	flex-basis: 32px;
 	flex-shrink: 0;
 }
 	.review-panel-toolbar-label {
 		cursor: pointer;
-		margin-right: 5px;
+		text-align: right;
+		flex-grow: 1;
+	}
+		.review-panel-toolbar-label-disabled {
+			cursor: auto;
+			margin-right: 5px;
+		}
+	.review-panel-toolbar-spinner {
+		margin-left: 5px;
 	}
 
 .rp-entry-list {
@@ -135,7 +162,6 @@
 		bottom: 0;
 	}
 	
-	
 	.rp-state-overview & {
 		flex-grow: 2;
 		overflow-y: auto;
@@ -150,7 +176,6 @@
 	display: none;
 	.rp-size-mini & {
 		display: block;
-		z-index: 12;
 	}
 	position: absolute;
 	left: 2px;
@@ -186,21 +211,39 @@
 		display: none;
 		left: @review-off-width + @rp-entry-arrow-width;
 		box-shadow: 0 0 10px 5px rgba(0, 0, 0, .2);
-		z-index: 11;
+		z-index: 1;
 
 		&::before {
-			.triangle(left, @rp-entry-arrow-width, @rp-entry-arrow-width * 1.5, inherit);
-			top: (@review-off-width / 2) - @rp-entry-arrow-width;
-			left: -(@rp-entry-ribbon-width  + @rp-entry-arrow-width);
-			content: '';
-		}
-		&::after {
 			content: '';
 			position: absolute;
 			top:  -(@review-off-width + @rp-entry-arrow-width);
 			right: -(@review-off-width + @rp-entry-arrow-width);
 			bottom: -(@review-off-width + @rp-entry-arrow-width);
+			left: -(2 * @rp-entry-arrow-width + 2);
+			z-index: -1;
+		}
+		&::after {
+			.triangle(left, @rp-entry-arrow-width, @rp-entry-arrow-width * 1.5, inherit);
+			top: (@review-off-width / 2) - @rp-entry-arrow-width;
+			left: -(@rp-entry-ribbon-width  + @rp-entry-arrow-width);
+			content: '';
+		}
+	}
+	.rp-state-current-file-mini.rp-layout-left & {
+		left: auto;
+		right: @review-off-width + @rp-entry-arrow-width;
+		border-left-width: 0;
+		border-right-width: @rp-entry-ribbon-width;
+		border-right-style: solid;
+
+		&::before {
 			left: -(@review-off-width + @rp-entry-arrow-width);
+			right: -(2 * @rp-entry-arrow-width + 2);
+		}
+		&::after {
+			.triangle(right, @rp-entry-arrow-width, @rp-entry-arrow-width * 1.5, inherit);
+			right: -(@rp-entry-ribbon-width  + @rp-entry-arrow-width);
+			left: auto;
 		}
 	}
 	.rp-state-current-file-expanded & {
@@ -224,10 +267,13 @@
 	}
 	.rp-state-overview & {
 		border-radius: 0;
-		padding: 2px 5px;
 		border-bottom: solid 1px @rp-border-grey;
 		cursor: pointer;
 	}
+	.resolved-comments-dropdown & {
+		position: static;
+		margin-bottom: 5px;
+	}
 
 	border-left: solid @rp-entry-ribbon-width transparent;
 	border-radius: 3px;
@@ -246,6 +292,16 @@
 		border-color: @rp-yellow;
 	}
 	
+	&-comment-resolving {
+		top: 4px;
+		left: 6px;
+		opacity: 0;
+		z-index: 3;
+		transform: scale(.1);
+		transform-origin: 0 0;
+		transition: top .35s ease-out, left .35s ease-out, transform .35s ease-out, opacity .35s ease-out .2s;
+	}
+	
 	&-comment-resolved {
 		border-color: @rp-grey;
 		background-color: #efefef;
@@ -264,69 +320,52 @@
 		}
 	}
 }
-
-	.rp-entry-header {
+	.rp-entry-body {
 		display: flex;
 		align-items: center;
-		padding: 5px;
-
-		.rp-state-overview & {
-			padding: 0px;
-		}
+		padding: 4px 5px;
 	}
 		.rp-entry-action-icon {
 			font-size: @rp-icon-large-size;
-			padding: 0 5px;
+			padding: 0 3px;
 			line-height: 0;
 
 			.rp-state-overview & {
-				font-size: @rp-base-font-size;
-				padding: 0px;
-				margin-right: 5px;
+				display: none;
 			}
 		}
 
-		.rp-entry-metadata {
-			flex-grow: 1;
-			padding: 0 5px;
-			line-height: 1.2;
+		.rp-entry-details {
+			line-height: 1.4;
+			margin-left: 5px;
 
 			.rp-state-overview & {
-				display: flex;
-				line-height: inherit;
-				padding: 0;
+				margin-left: 0;
 			}
 		}
-			.rp-entry-metadata-line {
-				margin: 0;
 
-				.rp-state-overview &:last-of-type {
-					flex-grow: 1;
-					text-align: right;
-				}
+			.rp-entry-metadata {
+				font-size: @rp-small-font-size;
 			}
+				.rp-entry-user {
+					font-weight: @rp-semibold-weight;
+					font-style: normal;
+				}
 
-	.rp-entry-body {
-		padding: 5px;
-
-		.rp-state-overview & {
-			padding: 0;
-		}
-	}
-		.rp-content-highlight {
-			color: @rp-type-darkgrey;
-			font-weight: @rp-semibold-weight;
-			text-decoration: none;
+			.rp-content-highlight {
+				color: @rp-type-darkgrey;
+				font-weight: @rp-semibold-weight;
+				text-decoration: none;
 
-			.rp-entry-delete & {
-				text-decoration: line-through;
+				.rp-entry-delete & {
+					text-decoration: line-through;
+				}
 			}
-		}
 
 	.rp-entry-actions {
 		display: flex;
 
-		.rp-state-overview & {
+		.rp-state-overview .rp-entry-list & {
 			display: none;
 		}
 	}
@@ -340,71 +379,59 @@
 				border-bottom-right-radius: 3px;
 				border-right-width: 0;
 			}
+
+			.rp-layout-left & {
+				&:first-child {
+					border-bottom-left-radius: 3px;
+				}
+				&:last-child {
+					border-bottom-right-radius: 0;
+				}
+			}
 		}
 
 	.rp-comment {
-		display: flex;
-		align-items: flex-start;
-		padding: 5px;
+		margin: 2px 5px;
+		padding-bottom: 3px;
+		line-height: 1.4;
+		border-bottom: solid 1px @rp-border-grey;
 
-		.rp-state-overview & {
-			padding: 3px 0;
-			line-height: 1.2;
+		&:last-child {
+			margin-bottom: 2px;
+			border-bottom-width: 0;
 		}
-	}
-		.rp-comment-body {
-			position: relative;
-			background-color: currentColor;
-			flex-grow: 1;
-			padding: 2px 5px;
-			margin-left: @rp-entry-arrow-width;
-			border-radius: 3px;
-
-			.rp-comment-self & {
-				margin-left: 0;
-				margin-right: @rp-entry-arrow-width;
-			}
-
-			&::after {
-				.triangle(left, @rp-entry-arrow-width, @rp-entry-arrow-width * 1.5, inherit);
-				top: (@review-off-width / 2) - @rp-entry-arrow-width;
-				left: -@rp-entry-arrow-width;
-				content: '';
 
-				.rp-comment-self & {
-					.triangle(right, @rp-entry-arrow-width, @rp-entry-arrow-width * 1.5, inherit);
-					right: -@rp-entry-arrow-width;
-					left: auto;
-				}
+		.rp-state-overview .rp-entry-list & {
+			margin: 4px 5px;
 
+			&:first-child {
+				margin-top: 0;
+				padding-top: 4px;
 			}
 		}
-			.rp-comment-content {
-				margin: 0;
-				color: @rp-type-darkgrey;
-			}
-
-			.rp-comment-metadata {
-				color: @rp-type-blue;
-				font-size: @rp-small-font-size;
-				margin: 0;
-			}
-
-		.rp-comment-reply {
-			padding: 0 5px;
-
-			.rp-state-overview & {
-				padding: 3px 0 0;
-			}
+	}
+		.rp-comment-content {
+			margin: 0;
+			color: @rp-type-darkgrey;
 		}
 
-		.rp-comment-resolved-description {
-			padding: 5px;
-
-			.rp-state-overview & {
-				padding: 0px;
-			}
+		.rp-comment-metadata {
+			color: @rp-type-blue;
+			font-size: @rp-small-font-size;
+			margin: 0;
 		}
+	
+	.rp-comment-resolver {
+		color: @rp-type-blue;
+	}
+		.rp-comment-resolver-content {
+			font-style: italic;
+			margin: 0;
+		}
+
+	.rp-comment-reply {
+		padding: 0 5px;
+	}
 
 	.rp-add-comment-btn {
 		.rp-button();
@@ -424,26 +451,9 @@
 		border-radius: 3px;
 		border: solid 1px @rp-border-grey;
 		resize: vertical;
+		color: @rp-type-darkgrey;
 	}
 
-.rp-avatar {
-	border-radius: 3px;
-	font-weight: @rp-semibold-weight;
-	font-size: @rp-icon-large-size;
-	line-height: 1.2;
-	text-transform: uppercase;
-	color: #FFF;
-	width: 1.3em;
-	height: 1.3em;
-	text-align: center;
-	flex-grow: 0;
-	flex-shrink: 0;
-
-	.rp-state-overview & {
-		display: none;
-	}
-}
-
 .rp-icon-delete {
 	display: inline-block;
 	line-height: 1;
@@ -456,6 +466,26 @@
 	}
 }
 
+.rp-resolved-comment {
+	border-left: solid @rp-entry-ribbon-width @rp-yellow;
+	border-radius: 3px;
+	background-color: #FFF;
+	margin-bottom: 5px;
+}
+	.rp-resolved-comment-context {
+		background-color: lighten(@rp-yellow, 35%);
+		padding: 4px 5px;
+	}
+		.rp-resolved-comment-context-file {
+			font-weight: @rp-semibold-weight;
+		}
+
+		.rp-resolved-comment-context-quote {
+			color: #000;
+			font-family: @font-family-monospace;
+			margin: 0;
+		}
+
 .rp-entry-callout {
 	.rp-state-current-file & {
 		position: absolute;
@@ -524,10 +554,24 @@
 	padding: 2px 5px;
 	border-top: solid 1px @rp-border-grey;
 	border-bottom: solid 1px @rp-border-grey;
-	background-color: #FFF;
+	background-color: @rp-bg-dim-blue;
 	margin-top: 10px;
 	font-weight: @rp-semibold-weight;
-	border-left: solid @rp-entry-ribbon-width currentColor;
+	text-align: center;
+}
+
+.rp-comment-wrapper {
+	transition: .35s opacity ease-out .2s;
+
+	&-resolving {
+		opacity: 0;
+	}
+}
+
+.rp-loading,
+.rp-empty {
+	text-align: center;
+	padding: 5px;
 }
 
 .rp-nav {
@@ -599,6 +643,7 @@
 .rp-toggle {
 	display: inline-block;
 	vertical-align: middle;
+	margin-left: 5px;
 }
 	.rp-toggle-hidden-input {
 		display: none;
@@ -647,7 +692,7 @@
 	.track-changes-marker-callout {
 		border-radius: 0;
 		position: absolute;
-		.rp-state-overview & {
+		.rp-state-overview &, .rp-loading-threads & {
 			display: none;
 		}
 	}
@@ -664,6 +709,9 @@
 	.track-changes-marker {
 		border-radius: 0;
 		position: absolute;
+		.rp-loading-threads & {
+			display: none;
+		}
 	}
 
 	.track-changes-comment-marker {
@@ -676,13 +724,25 @@
 		border-left: 2px dotted @rp-red;
 		margin-left: -1px;
 	}
+
+	.ace_dark {
+		.track-changes-comment-marker {
+			background-color: @rp-yellow-on-dark
+		}
+		.track-changes-added-marker {
+			background-color: @rp-green-on-dark;
+		}	
+	}
 }
 
 .review-icon {
-	position: absolute;
+	display: inline-block;
 	background: url('/img/review-icon-sprite.png') top/30px no-repeat;
 	width: 30px;
-	height: 30px;
+
+	&::before {
+		content: '\00a0'; // Non-breakable space. A non-breakable character here makes this icon work like font-awesome.
+	}
 
 	.toolbar .btn-full-height:hover & {
 		background-position-y: -30px;
@@ -692,9 +752,116 @@
 	.toolbar .btn-full-height:active & {
 		background-position-y: -60px;
 	}
+}
+
+.resolved-comments-toggle {
+	font-size: 14px;
+	color: lighten(@rp-type-blue, 25%);
+	border: solid 1px @rp-border-grey;
+	border-radius: 3px;
+	padding: 0 4px;
+	display: block;
+	height: 22px;
+	width: 22px;
+	line-height: 1.4;
+
+	&:hover,
+	&:focus {
+		text-decoration: none;
+		color: @rp-type-blue;
+	}
+}
+
+.resolved-comments-backdrop {
+	display: none;
+	position: fixed;
+	top: 0;
+	right: 0;
+	bottom: 0;
+	left: 0;
+
+	&-visible {
+		display: block;
+	}
+}
+
+.resolved-comments-dropdown {
+	display: none;
+	position: absolute;
+	width: 300px;
+	left: -150px;
+	max-height: 90%;
+	margin-top: @rp-entry-arrow-width * 1.5;
+	margin-left: 1em;
+	background-color: @rp-bg-blue;
+	text-align: left;
+	align-items: stretch;
+	justify-content: center;
+	border-radius: 3px;
+	box-shadow: 0 0 20px 10px rgba(0, 0, 0, .3);
+
+	&::before {
+		content: '';
+		.triangle(top, @rp-entry-arrow-width * 3, @rp-entry-arrow-width * 1.5, @rp-bg-blue);
+		top: -@rp-entry-ribbon-width * 2;
+		left: 50%;
+		margin-left: -@rp-entry-arrow-width * .75;
+	}
+
+	&-open {
+		display: flex;
+	}
+}
+	.resolved-comments-scroller {
+		flex: 0 0 100%;
+		padding: 5px;
+		overflow-y: auto;
+	}
+
+.rp-collapse-toggle {
+	color: @rp-type-blue;
+	font-weight: @rp-semibold-weight;
+
+	&:hover,
+	&:focus {
+		color: darken(@rp-type-blue, 5%);
+		text-decoration: none;
+	}
+}
+
+.rp-track-changes-indicator {
+	display: none;
+	position: absolute;
+	top: 0;
+	right: @review-off-width;
+	padding: 5px 10px;
+	background-color: rgba(240, 240, 240, 0.9);
+	color: @rp-type-blue;
+	text-align: center;
+	border-bottom-left-radius: 3px;
+	font-size: 10px;
+	z-index: 2;
 
-	& + .toolbar-label {
-		margin-left: 34px;
+	&.rp-track-changes-indicator-on-dark {
+		background-color: rgba(88, 88, 88, .8);
+		color: #FFF;
+		
+		&:hover,
+		&:focus {
+			background-color: rgba(88, 88, 88, 1);
+			color: #FFF;
+		}
+	}
+
+	&:hover,
+	&:focus {
+		outline: 0;
+		text-decoration: none;
+		background-color: rgba(240, 240, 240, 1);
+		color: @rp-type-blue;
 	}
 
+	.rp-size-mini & {
+		display: block;
+	}
 }

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

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,209 @@
+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
+			'../User/UserInfoManager': @UserInfoManager = {}
+			'../User/UserInfoController': @UserInfoController = {}
+		@req = {}
+		@res =
+			json: sinon.stub()
+			send: sinon.stub()
+
+	describe "sendComment", ->
+		beforeEach ->
+			@req.params =
+				project_id: @project_id = "mock-project-id"
+				thread_id: @thread_id = "mock-thread-id"
+			@req.body =
+				content: @content = "message-content"
+			@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
+				.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 = "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
+
+	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 {
+				thread1: {
+					resolved: true
+					resolved_by_user_id: "user_id_1"
+					messages: [{
+						user_id: "user_id_1"
+						content: "foo"
+					}, {
+						user_id: "user_id_2"
+						content: "bar"
+					}]
+				},
+				thread2: {
+					messages: [{
+						user_id: "user_id_1"
+						content: "baz"
+					}]
+				}
+			}, (error, threads) ->
+				expect(threads).to.deep.equal {
+					thread1: {
+						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"
+						}]
+					},
+					thread2: {
+						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()

services/web/test/UnitTests/coffee/Docstore/DocstoreManagerTests.coffee

@@ -57,12 +57,13 @@ describe "DocstoreManager", ->
 			@lines = ["mock", "doc", "lines"]
 			@rev = 5
 			@version = 42
+			@ranges = { "mock": "ranges" }
 			@modified = true
 
 		describe "with a successful response code", ->
 			beforeEach ->
 				@request.post = sinon.stub().callsArgWith(1, null, statusCode: 204, { modified: @modified, rev: @rev })
-				@DocstoreManager.updateDoc @project_id, @doc_id, @lines, @version, @callback
+				@DocstoreManager.updateDoc @project_id, @doc_id, @lines, @version, @ranges, @callback
 
 			it "should update the doc in the docstore api", ->
 				@request.post
@@ -71,6 +72,7 @@ describe "DocstoreManager", ->
 						json:
 							lines: @lines
 							version: @version
+							ranges: @ranges
 					})
 					.should.equal true
 
@@ -80,7 +82,7 @@ describe "DocstoreManager", ->
 		describe "with a failed response code", ->
 			beforeEach ->
 				@request.post = sinon.stub().callsArgWith(1, null, statusCode: 500, "")
-				@DocstoreManager.updateDoc @project_id, @doc_id, @lines, @version, @callback
+				@DocstoreManager.updateDoc @project_id, @doc_id, @lines, @version, @ranges, @callback
 
 			it "should call the callback with an error", ->
 				@callback.calledWith(new Error("docstore api responded with non-success code: 500")).should.equal true
@@ -100,6 +102,7 @@ describe "DocstoreManager", ->
 				lines:   @lines = ["mock", "doc", "lines"]
 				rev:     @rev = 5
 				version: @version = 42
+				ranges:  @ranges = { "mock": "ranges" }
 
 		describe "with a successful response code", ->
 			beforeEach ->
@@ -115,7 +118,7 @@ describe "DocstoreManager", ->
 					.should.equal true
 
 			it "should call the callback with the lines, version and rev", ->
-				@callback.calledWith(null, @lines, @rev, @version).should.equal true
+				@callback.calledWith(null, @lines, @rev, @version, @ranges).should.equal true
 
 		describe "with a failed response code", ->
 			beforeEach ->
@@ -148,7 +151,7 @@ describe "DocstoreManager", ->
 					.should.equal true
 
 			it "should call the callback with the lines, version and rev", ->
-				@callback.calledWith(null, @lines, @rev, @version).should.equal true
+				@callback.calledWith(null, @lines, @rev, @version, @ranges).should.equal true
 
 	describe "getAllDocs", ->
 		describe "with a successful response code", ->
@@ -183,6 +186,38 @@ describe "DocstoreManager", ->
 					}, "error getting all docs from docstore")
 					.should.equal true
 
+	describe "getAllRanges", ->
+		describe "with a successful response code", ->
+			beforeEach ->
+				@request.get = sinon.stub().callsArgWith(1, null, statusCode: 204, @docs = [{ _id: "mock-doc-id", ranges: "mock-ranges" }])
+				@DocstoreManager.getAllRanges @project_id, @callback
+
+			it "should get all the project doc ranges in the docstore api", ->
+				@request.get
+					.calledWith({
+						url: "#{@settings.apis.docstore.url}/project/#{@project_id}/ranges"
+						json: true
+					})
+					.should.equal true
+
+			it "should call the callback with the docs", ->
+				@callback.calledWith(null, @docs).should.equal true
+
+		describe "with a failed response code", ->
+			beforeEach ->
+				@request.get = sinon.stub().callsArgWith(1, null, statusCode: 500, "")
+				@DocstoreManager.getAllRanges @project_id, @callback
+
+			it "should call the callback with an error", ->
+				@callback.calledWith(new Error("docstore api responded with non-success code: 500")).should.equal true
+
+			it "should log the error", ->
+				@logger.error
+					.calledWith({
+						err: new Error("docstore api responded with a non-success code: 500")
+						project_id: @project_id
+					}, "error getting all doc ranges from docstore")
+					.should.equal true
 
 	describe "archiveProject", ->
 		describe "with a successful response code", ->

services/web/test/UnitTests/coffee/DocumentUpdater/DocumentUpdaterHandlerTests.coffee

@@ -8,8 +8,7 @@ path = require 'path'
 _ = require 'underscore'
 modulePath = path.join __dirname, '../../../../app/js/Features/DocumentUpdater/DocumentUpdaterHandler'
 
-describe 'DocumentUpdaterHandler - Flushing documents :', ->
-
+describe 'DocumentUpdaterHandler', ->
 	beforeEach ->
 		@project_id = "project-id-923"
 		@doc_id = "doc-id-394"
@@ -267,6 +266,7 @@ describe 'DocumentUpdaterHandler - Flushing documents :', ->
 					lines: @lines
 					version: @version
 					ops: @ops = ["mock-op-1", "mock-op-2"]
+					ranges: @ranges = {"mock":"ranges"}
 				@fromVersion = 2
 				@request.get = sinon.stub().callsArgWith(1, null, {statusCode: 200}, @body)
 				@handler.getDocument @project_id, @doc_id, @fromVersion, @callback
@@ -276,7 +276,7 @@ describe 'DocumentUpdaterHandler - Flushing documents :', ->
 				@request.get.calledWith(url).should.equal true
 
 			it "should call the callback with the lines and version", ->
-				@callback.calledWith(null, @lines, @version, @ops).should.equal true
+				@callback.calledWith(null, @lines, @version, @ranges, @ops).should.equal true
 
 		describe "when the document updater API returns an error", ->
 			beforeEach ->
@@ -295,3 +295,38 @@ describe 'DocumentUpdaterHandler - Flushing documents :', ->
 				@callback
 					.calledWith(new Error("doc updater returned failure status code: 500"))
 					.should.equal true
+
+	describe "acceptChange", ->
+		beforeEach ->
+			@change_id = "mock-change-id-1"
+			@callback = sinon.stub()
+
+		describe "successfully", ->
+			beforeEach ->
+				@request.post = sinon.stub().callsArgWith(1, null, {statusCode: 200}, @body)
+				@handler.acceptChange @project_id, @doc_id, @change_id, @callback
+
+			it 'should accept the change in the document updater', ->
+				url = "#{@settings.apis.documentupdater.url}/project/#{@project_id}/doc/#{@doc_id}/change/#{@change_id}/accept"
+				@request.post.calledWith(url).should.equal true
+
+			it "should call the callback", ->
+				@callback.calledWith(null).should.equal true
+
+		describe "when the document updater API returns an error", ->
+			beforeEach ->
+				@request.post = sinon.stub().callsArgWith(1, @error = new Error("something went wrong"), null, null)
+				@handler.acceptChange @project_id, @doc_id, @change_id, @callback
+
+			it "should return an error to the callback", ->
+				@callback.calledWith(@error).should.equal true
+
+		describe "when the document updater returns a failure error code", ->
+			beforeEach ->
+				@request.post = sinon.stub().callsArgWith(1, null, { statusCode: 500 }, "")
+				@handler.acceptChange @project_id, @doc_id, @change_id, @callback
+
+			it "should return the callback with an error", ->
+				@callback
+					.calledWith(new Error("doc updater returned failure status code: 500"))
+					.should.equal true

services/web/test/UnitTests/coffee/DocumentUpdater/GetNumberOfDocsInMemoryTests.coffee

@@ -1,43 +0,0 @@
-path = require("path")
-sinon = require("sinon")
-SandboxedModule = require('sandboxed-module')
-
-modulePath = path.join __dirname, '../../../../app/js/Features/DocumentUpdater/DocumentUpdaterHandler'
-
-describe "getNumberOfDocsInMemory", ->
-	beforeEach ->
-		@host = "doc.updater"
-		@noOfDocs = 42
-		@callback = sinon.stub()
-		@DocumentUpdateHandler = SandboxedModule.require modulePath, requires:
-			"redis-sharelatex" : 
-				createClient: () ->
-					auth:->
-			"soa-req-id": null
-			"logger-sharelatex": @logger =
-				log: sinon.stub()
-				error: sinon.stub()
-			"../../infrastructure/Metrics" : @metrics
-			"../../Features/Project/ProjectLocator": @ProjectLocator = {}
-			"../../models/Project":Project:{}
-			"request" : defaults: () => @request = {}
-			"settings-sharelatex": 
-				apis: documentupdater: url: @host
-				redis: web:{}
-
-
-		@request.get = sinon.stub().callsArgWith(1, null, {statusCode: 200}, JSON.stringify(total: @noOfDocs))
-		@DocumentUpdateHandler.getNumberOfDocsInMemory @callback
-
-	it "should call the doc updater", ->
-		@request.get
-			.calledWith("#{@host}/total")
-			.should.equal true
-
-	it "should return the number of docs", ->
-		@callback
-			.calledWith(null, @noOfDocs)
-			.should.equal true
-
-	
-

services/web/test/UnitTests/coffee/Documents/DocumentControllerTests.coffee

@@ -23,6 +23,7 @@ describe "DocumentController", ->
 		@doc_id = "doc-id-123"
 		@doc_lines = ["one", "two", "three"]
 		@version = 42
+		@ranges = {"mock": "ranges"}
 		@rev = 5
 
 	describe "getDocument", ->
@@ -33,7 +34,7 @@ describe "DocumentController", ->
 
 		describe "when the document exists", ->
 			beforeEach ->
-				@ProjectEntityHandler.getDoc = sinon.stub().callsArgWith(2, null, @doc_lines, @rev, @version)
+				@ProjectEntityHandler.getDoc = sinon.stub().callsArgWith(2, null, @doc_lines, @rev, @version, @ranges)
 				@DocumentController.getDocument(@req, @res, @next)
 
 			it "should get the document from Mongo", ->
@@ -46,6 +47,7 @@ describe "DocumentController", ->
 				@res.body.should.equal JSON.stringify
 					lines: @doc_lines
 					version: @version
+					ranges: @ranges
 
 		describe "when the document doesn't exist", ->
 			beforeEach ->
@@ -68,11 +70,12 @@ describe "DocumentController", ->
 				@req.body =
 					lines: @doc_lines
 					version: @version
+					ranges: @ranges
 				@DocumentController.setDocument(@req, @res, @next)
 
 			it "should update the document in Mongo", ->
 				@ProjectEntityHandler.updateDocLines
-					.calledWith(@project_id, @doc_id, @doc_lines, @version)
+					.calledWith(@project_id, @doc_id, @doc_lines, @version, @ranges)
 					.should.equal true
 
 			it "should return a successful response", ->

services/web/test/UnitTests/coffee/TrackChanges/TrackChangesControllerTests.coffee → services/web/test/UnitTests/coffee/History/HistoryControllerTests.coffee

@@ -1,21 +1,21 @@
 chai = require('chai')
 chai.should()
 sinon = require("sinon")
-modulePath = "../../../../app/js/Features/TrackChanges/TrackChangesController"
+modulePath = "../../../../app/js/Features/History/HistoryController"
 SandboxedModule = require('sandboxed-module')
 
-describe "TrackChangesController", ->
+describe "HistoryController", ->
 	beforeEach ->
 		@user_id = "user-id-123"
 		@AuthenticationController =
 			getLoggedInUserId: sinon.stub().returns(@user_id)
-		@TrackChangesController = SandboxedModule.require modulePath, requires:
+		@HistoryController = SandboxedModule.require modulePath, requires:
 			"request" : @request = sinon.stub()
 			"settings-sharelatex": @settings = {}
 			"logger-sharelatex": @logger = {log: sinon.stub(), error: sinon.stub()}
 			"../Authentication/AuthenticationController": @AuthenticationController
 
-	describe "proxyToTrackChangesApi", ->
+	describe "proxyToHistoryApi", ->
 		beforeEach ->
 			@req = { url: "/mock/url", method: "POST" }
 			@res = "mock-res"
@@ -28,7 +28,7 @@ describe "TrackChangesController", ->
 				pipe: sinon.stub()
 				on: (event, handler) -> @events[event] = handler
 			@request.returns @proxy
-			@TrackChangesController.proxyToTrackChangesApi @req, @res, @next
+			@HistoryController.proxyToHistoryApi @req, @res, @next
 
 		describe "successfully", ->
 			it "should get the user id", ->

services/web/test/UnitTests/coffee/TrackChanges/TrackChangesManagerTests.coffee → services/web/test/UnitTests/coffee/History/HistoryManagerTests.coffee

@@ -2,12 +2,12 @@ chai = require('chai')
 expect = chai.expect
 chai.should()
 sinon = require("sinon")
-modulePath = "../../../../app/js/Features/TrackChanges/TrackChangesManager"
+modulePath = "../../../../app/js/Features/History/HistoryManager"
 SandboxedModule = require('sandboxed-module')
 
-describe "TrackChangesManager", ->
+describe "HistoryManager", ->
 	beforeEach ->
-		@TrackChangesManager = SandboxedModule.require modulePath, requires:
+		@HistoryManager = SandboxedModule.require modulePath, requires:
 			"request" : @request = sinon.stub()
 			"settings-sharelatex": @settings =
 				apis:
@@ -22,7 +22,7 @@ describe "TrackChangesManager", ->
 		describe "with a successful response code", ->
 			beforeEach ->
 				@request.post = sinon.stub().callsArgWith(1, null, statusCode: 204, "")
-				@TrackChangesManager.flushProject @project_id, @callback
+				@HistoryManager.flushProject @project_id, @callback
 
 			it "should flush the project in the track changes api", ->
 				@request.post
@@ -35,7 +35,7 @@ describe "TrackChangesManager", ->
 		describe "with a failed response code", ->
 			beforeEach ->
 				@request.post = sinon.stub().callsArgWith(1, null, statusCode: 500, "")
-				@TrackChangesManager.flushProject @project_id, @callback
+				@HistoryManager.flushProject @project_id, @callback
 
 			it "should call the callback with an error", ->
 				@callback.calledWith(new Error("track-changes api responded with a non-success code: 500")).should.equal true
@@ -52,12 +52,12 @@ describe "TrackChangesManager", ->
 
 		it "should call the post endpoint", (done)->
 			@request.post.callsArgWith(1, null, {})
-			@TrackChangesManager.archiveProject @project_id, (err)=>
+			@HistoryManager.archiveProject @project_id, (err)=>
 				@request.post.calledWith("#{@settings.apis.trackchanges.url}/project/#{@project_id}/archive")
 				done()
 
 		it "should return an error on a non success", (done)->
 			@request.post.callsArgWith(1, null, {statusCode:500})
-			@TrackChangesManager.archiveProject @project_id, (err)=>
+			@HistoryManager.archiveProject @project_id, (err)=>
 				expect(err).to.exist
 				done()

services/web/test/UnitTests/coffee/Project/ProjectEntityHandlerTests.coffee

@@ -382,7 +382,9 @@ describe 'ProjectEntityHandler', ->
 		beforeEach ->
 			@lines = ["mock", "doc", "lines"]
 			@rev = 5
-			@DocstoreManager.getDoc = sinon.stub().callsArgWith(3, null, @lines, @rev)
+			@version = 42
+			@ranges = {"mock": "ranges"}
+			@DocstoreManager.getDoc = sinon.stub().callsArgWith(3, null, @lines, @rev, @version, @ranges)
 			@ProjectEntityHandler.getDoc project_id, doc_id, @callback
 
 		it "should call the docstore", ->
@@ -391,7 +393,7 @@ describe 'ProjectEntityHandler', ->
 				.should.equal true
 
 		it "should call the callback with the lines, version and rev", ->
-			@callback.calledWith(null, @lines, @rev).should.equal true
+			@callback.calledWith(null, @lines, @rev, @version, @ranges).should.equal true
 
 	describe 'addDoc', ->
 		beforeEach ->
@@ -590,6 +592,7 @@ describe 'ProjectEntityHandler', ->
 				_id: doc_id
 			}
 			@version = 42
+			@ranges = {"mock":"ranges"}
 			@ProjectGetter.getProjectWithoutDocLines = sinon.stub().callsArgWith(1, null, @project)
 			@projectLocator.findElement = sinon.stub().callsArgWith(1, null, @doc, {fileSystem: @path})
 			@tpdsUpdateSender.addDoc = sinon.stub().callsArg(1)
@@ -599,7 +602,7 @@ describe 'ProjectEntityHandler', ->
 		describe "when the doc has been modified", ->
 			beforeEach ->
 				@DocstoreManager.updateDoc = sinon.stub().yields(null, true, @rev = 5)
-				@ProjectEntityHandler.updateDocLines project_id, doc_id, @lines, @version, @callback
+				@ProjectEntityHandler.updateDocLines project_id, doc_id, @lines, @version, @ranges, @callback
 
 			it "should get the project without doc lines", ->
 				@ProjectGetter.getProjectWithoutDocLines
@@ -617,7 +620,7 @@ describe 'ProjectEntityHandler', ->
 
 			it "should update the doc in the docstore", ->
 				@DocstoreManager.updateDoc
-					.calledWith(project_id, doc_id, @lines, @version)
+					.calledWith(project_id, doc_id, @lines, @version, @ranges)
 					.should.equal true
 
 			it "should mark the project as updated", ->
@@ -642,7 +645,7 @@ describe 'ProjectEntityHandler', ->
 		describe "when the doc has not been modified", ->
 			beforeEach ->
 				@DocstoreManager.updateDoc = sinon.stub().yields(null, false, @rev = 5)
-				@ProjectEntityHandler.updateDocLines project_id, doc_id, @lines, @version, @callback
+				@ProjectEntityHandler.updateDocLines project_id, doc_id, @lines, @version, @ranges, @callback
 
 			it "should not mark the project as updated", ->
 				@projectUpdater.markAsUpdated.called.should.equal false
@@ -656,7 +659,7 @@ describe 'ProjectEntityHandler', ->
 		describe "when the project is not found", ->
 			beforeEach ->
 				@ProjectGetter.getProjectWithoutDocLines = sinon.stub().callsArgWith(1, null, null)
-				@ProjectEntityHandler.updateDocLines project_id, doc_id, @lines, @version, @callback
+				@ProjectEntityHandler.updateDocLines project_id, doc_id, @lines, @ranges, @version, @callback
 
 			it "should return a not found error", ->
 				@callback.calledWith(new Errors.NotFoundError()).should.equal true
@@ -664,7 +667,7 @@ describe 'ProjectEntityHandler', ->
 		describe "when the doc is not found", ->
 			beforeEach ->
 				@projectLocator.findElement = sinon.stub().callsArgWith(1, null, null, null)
-				@ProjectEntityHandler.updateDocLines project_id, doc_id, @lines, @version, @callback
+				@ProjectEntityHandler.updateDocLines project_id, doc_id, @lines, @ranges, @version, @callback
 
 			it "should log out the error", ->
 				@logger.error

services/web/test/UnitTests/coffee/TrackChanges/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/TrackChanges/RangesManager"
+expect = require("chai").expect
+
+describe "RangesManager", ->
+	beforeEach ->
+		@RangesManager = SandboxedModule.require modulePath, requires:
+			"../DocumentUpdater/DocumentUpdaterHandler": @DocumentUpdaterHandler = {}
+			"../Docstore/DocstoreManager": @DocstoreManager = {}
+			"../User/UserInfoManager": @UserInfoManager = {}
+
+	describe "getAllChangesUsers", ->
+		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.getAllChangesUsers @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.getAllChangesUsers @project_id, (error, users) =>
+				@UserInfoManager.getPersonalInfo.calledTwice.should.equal true
+				done()

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
+			}