Allow editing/deleting of comments and threads

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

@@ -58,4 +58,25 @@ module.exports = ChatApiHandler =
 		ChatApiHandler._apiRequest {
 			url:    "#{settings.apis.chat.internal_url}/project/#{project_id}/thread/#{thread_id}/reopen"
 			method: "POST"
-		}, callback
+		}, callback
+	
+	deleteThread: (project_id, thread_id, callback = (error) ->) ->
+		ChatApiHandler._apiRequest {
+			url:    "#{settings.apis.chat.internal_url}/project/#{project_id}/thread/#{thread_id}"
+			method: "DELETE"
+		}, callback
+	
+	editMessage: (project_id, thread_id, message_id, content, callback = (error) ->) ->
+		ChatApiHandler._apiRequest {
+			url:    "#{settings.apis.chat.internal_url}/project/#{project_id}/thread/#{thread_id}/messages/#{message_id}/edit"
+			method: "POST"
+			json:
+				content: content
+		}, callback
+	
+	deleteMessage: (project_id, thread_id, message_id, callback = (error) ->) ->
+		ChatApiHandler._apiRequest {
+			url:    "#{settings.apis.chat.internal_url}/project/#{project_id}/thread/#{thread_id}/messages/#{message_id}"
+			method: "DELETE"
+		}, callback
+		

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

@@ -4,6 +4,7 @@ logger = require("logger-sharelatex")
 AuthenticationController = require('../Authentication/AuthenticationController')
 UserInfoManager = require('../User/UserInfoManager')
 UserInfoController = require('../User/UserInfoController')
+DocumentUpdaterHandler = require "../DocumentUpdater/DocumentUpdaterHandler"
 async = require "async"
 
 module.exports = CommentsController =
@@ -50,6 +51,33 @@ module.exports = CommentsController =
 			return next(err) if err?
 			EditorRealTimeController.emitToRoom project_id, "reopen-thread", thread_id, (err)->
 			res.send 204
+	
+	deleteThread: (req, res, next) ->
+		{project_id, doc_id, thread_id} = req.params
+		logger.log {project_id, doc_id, thread_id}, "deleting comment thread"
+		DocumentUpdaterHandler.deleteThread project_id, doc_id, thread_id, (err) ->
+			return next(err) if err?
+			ChatApiHandler.deleteThread project_id, thread_id, (err, threads) ->
+				return next(err) if err?
+				EditorRealTimeController.emitToRoom project_id, "delete-thread", thread_id, (err)->
+				res.send 204
+	
+	editMessage: (req, res, next) ->
+		{project_id, thread_id, message_id} = req.params
+		{content} = req.body
+		logger.log {project_id, thread_id, message_id}, "editing message thread"
+		ChatApiHandler.editMessage project_id, thread_id, message_id, content, (err) ->
+			return next(err) if err?
+			EditorRealTimeController.emitToRoom project_id, "edit-message", thread_id, message_id, content, (err)->
+			res.send 204
+	
+	deleteMessage: (req, res, next) ->
+		{project_id, thread_id, message_id} = req.params
+		logger.log {project_id, thread_id, message_id}, "deleting message"
+		ChatApiHandler.deleteMessage project_id, thread_id, message_id, (err, threads) ->
+			return next(err) if err?
+			EditorRealTimeController.emitToRoom project_id, "delete-message", thread_id, message_id, (err)->
+			res.send 204
 
 	_injectUserInfoIntoThreads: (threads, callback = (error, threads) ->) ->
 		userCache = {}

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

@@ -153,6 +153,22 @@ module.exports = DocumentUpdaterHandler =
 				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}")
 
+	deleteThread: (project_id, doc_id, thread_id, callback = (error) ->) ->
+		timer = new metrics.Timer("delete-thread")
+		url = "#{settings.apis.documentupdater.url}/project/#{project_id}/doc/#{doc_id}/comment/#{thread_id}"
+		logger.log {project_id, doc_id, thread_id}, "deleting comment range in document updater"
+		request.del url, (error, res, body)->
+			timer.done()
+			if error?
+				logger.error {err:error, project_id, doc_id, thread_id}, "error deleting comment range in doc updater"
+				return callback(error)
+			if res.statusCode >= 200 and res.statusCode < 300
+				logger.log {project_id, doc_id, thread_id}, "deleted comment rangee in document updater"
+				return callback(null)
+			else
+				logger.error {project_id, doc_id, thread_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"
 DOCIDSWITHPENDINGUPDATES = "DocsWithPendingUpdates"

services/web/app/coffee/router.coffee

@@ -238,6 +238,9 @@ module.exports = class Router
 		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.delete "/project/:project_id/doc/:doc_id/thread/:thread_id", AuthorizationMiddlewear.ensureUserCanWriteProjectContent, CommentsController.deleteThread
+		webRouter.post "/project/:project_id/thread/:thread_id/messages/:message_id/edit", AuthorizationMiddlewear.ensureUserCanWriteProjectContent, CommentsController.editMessage
+		webRouter.delete  "/project/:project_id/thread/:thread_id/messages/:message_id", AuthorizationMiddlewear.ensureUserCanWriteProjectContent, CommentsController.deleteMessage
 
 		webRouter.post "/project/:Project_id/references/index", AuthorizationMiddlewear.ensureUserCanReadProject, ReferencesController.index
 		webRouter.post "/project/:Project_id/references/indexAll", AuthorizationMiddlewear.ensureUserCanReadProject, ReferencesController.indexAll

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

@@ -7,7 +7,7 @@
 			docs="docs"
 			on-open="refreshResolvedCommentsDropdown();"
 			on-unresolve="unresolveComment(threadId);"
-			on-delete="deleteComment(entryId, threadId);"
+			on-delete="deleteThread(entryId, docId, threadId);"
 			is-loading="reviewPanel.dropdown.loading"
 			permissions="permissions"
 		)
@@ -51,6 +51,8 @@
 						on-resolve="resolveComment(entry, entry_id)"
 						on-reply="submitReply(entry, entry_id);"
 						on-indicator-click="toggleReviewPanel();"
+						on-save-edit="saveEdit(entry.thread_id, comment)"
+						on-delete="deleteComment(entry.thread_id, comment)"
 						permissions="permissions"
 						ng-if="!reviewPanel.loadingThreads"
 					)
@@ -94,6 +96,8 @@
 						entry="entry"
 						threads="reviewPanel.commentThreads"
 						on-reply="submitReply(entry, entry_id);"
+						on-save-edit="saveEdit(entry.thread_id, comment)"
+						on-delete="deleteComment(entry.thread_id, comment)"
 						on-indicator-click="toggleReviewPanel();"
 						ng-click="gotoEntry(doc.doc.id, entry)"
 						permissions="permissions"
@@ -175,18 +179,42 @@ script(type='text/ng-template', id='commentEntryTemplate')
 		.rp-entry.rp-entry-comment(
 			ng-class="{ 'rp-entry-focused': entry.focused, 'rp-entry-comment-resolving': state.animating }"
 		)
+
+			.rp-loading(ng-if="!threads[entry.thread_id] || threads[entry.thread_id].messages.length == 0")
+				| No comments
 			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")
+					p.rp-comment-content
+						span(ng-if="!comment.editing")
+							span.rp-entry-user(
+								style="color: hsl({{ comment.user.hue }}, 70%, 40%);",
+							) {{ comment.user.name }}: 
+							| {{ comment.content }}
+						textarea.rp-comment-input(
+							ng-if="comment.editing"
+							ng-model="comment.content"
+							ng-keypress="saveEditOnEnter($event, comment);"
+							ng-blur="saveEdit(comment)"
+							autofocus
+							stop-propagation="click"
+						)
+					.rp-entry-metadata(ng-if="!comment.editing")
+						span(ng-if="!comment.deleting") {{ comment.timestamp | date : 'MMM d, y h:mm a' }}
+						span.rp-comment-actions(ng-if="comment.user.isSelf && !comment.deleting")
+							|  • 
+							a(href, ng-click="startEditing(comment)") Edit
+							|  • 
+							a(href, ng-click="confirmDelete(comment)") Delete
+						span.rp-confim-delete(ng-if="comment.user.isSelf && comment.deleting")
+							| Are you sure?
+							| • 
+							a(href, ng-click="doDelete(comment)") Delete
+							|  • 
+							a(href, ng-click="cancelDelete(comment)") Cancel
+
+			.rp-loading(ng-if="threads[entry.thread_id].submitting")
 				i.fa.fa-spinner.fa-spin
 			.rp-comment-reply(ng-if="permissions.comment")
 				textarea.rp-comment-input(
@@ -249,11 +277,11 @@ script(type='text/ng-template', id='resolvedCommentEntryTemplate')
 				ng-click="onUnresolve({ 'threadId': thread.threadId });"
 			) 
 				|  Re-open
-			//- a.rp-entry-button(
-			//- 	href
-			//- 	ng-click="onDelete({ 'entryId': thread.entryId, 'threadId': thread.threadId });"
-			//- )
-			//- 	|  Delete
+			a.rp-entry-button(
+				href
+				ng-click="onDelete({ 'entryId': thread.entryId, 'docId': thread.docId, 'threadId': thread.threadId });"
+			)
+				|  Delete
 
 
 script(type='text/ng-template', id='addCommentEntryTemplate')
@@ -324,7 +352,7 @@ script(type='text/ng-template', id='resolvedCommentsDropdownTemplate')
 					ng-repeat="thread in resolvedComments | orderBy:'resolved_at':true"
 					thread="thread"
 					on-unresolve="handleUnresolve(threadId);"
-					on-delete="handleDelete(entryId, threadId);"
+					on-delete="handleDelete(entryId, docId, threadId);"
 					permissions="permissions"
 				)
 				.rp-loading(ng-if="!resolvedComments.length")

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

@@ -105,7 +105,6 @@ load = (EventEmitter) ->
 				throw new Error("unknown op type")
 			
 		addComment: (op, metadata) ->
-			# TODO: Don't allow overlapping comments?
 			@comments.push comment = {
 				id: op.t or @newId()
 				op: # Copy because we'll modify in place

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

@@ -65,6 +65,14 @@ define [
 			
 		ide.socket.on "reopen-thread", (thread_id) ->
 			_onCommentReopened(thread_id)
+		
+		ide.socket.on "edit-message", (thread_id, message_id, content) ->
+			_onCommentEdited(thread_id, message_id, content)
+			$scope.$apply () ->
+		
+		ide.socket.on "delete-message", (thread_id, message_id) ->
+			_onCommentDeleted(thread_id, message_id)
+			$scope.$apply () ->
 
 		rangesTrackers = {}
 
@@ -342,7 +350,8 @@ define [
 			event_tracking.sendMB "rp-comment-reopen"
 		
 		_onCommentResolved = (thread_id, user) ->
-			thread = $scope.reviewPanel.commentThreads[thread_id]
+			thread = getThread(thread_id)
+			return if !thread?
 			thread.resolved = true
 			thread.resolved_by_user = formatUser(user)
 			thread.resolved_at = new Date()
@@ -350,23 +359,63 @@ define [
 			$scope.$broadcast "comment:resolve_thread", thread_id
 		
 		_onCommentReopened = (thread_id) ->
-			thread = $scope.reviewPanel.commentThreads[thread_id]
+			thread = getThread(thread_id)
+			return if !thread?
 			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
 
-		_onCommentDeleted = (thread_id) ->
-			if $scope.reviewPanel.resolvedThreadIds[thread_id]?
-				delete $scope.reviewPanel.resolvedThreadIds[thread_id]
-				
+		_onThreadDeleted = (thread_id) ->
+			delete $scope.reviewPanel.resolvedThreadIds[thread_id]
 			delete $scope.reviewPanel.commentThreads[thread_id]
 		
-		$scope.deleteComment = (entry_id, thread_id) ->
-			_onCommentDeleted(thread_id)
+		_onCommentEdited = (thread_id, comment_id, content) ->
+			thread = getThread(thread_id)
+			return if !thread?
+			for message in thread.messages
+				if message.id == comment_id
+					message.content = content
+			updateEntries()
+		
+		_onCommentDeleted = (thread_id, comment_id) ->
+			thread = getThread(thread_id)
+			return if !thread?
+			thread.messages = thread.messages.filter (m) -> m.id != comment_id
+			updateEntries()
+		
+		$scope.deleteThread = (entry_id, doc_id, thread_id) ->
+			_onThreadDeleted(thread_id)
+			$http({
+				method: "DELETE"
+				url: "/project/#{$scope.project_id}/doc/#{doc_id}/thread/#{thread_id}",
+				headers: {
+					'X-CSRF-Token': window.csrfToken
+				}
+			})
 			$scope.$broadcast "comment:remove", entry_id
 			event_tracking.sendMB "rp-comment-delete"
+		
+		$scope.saveEdit = (thread_id, comment) ->
+			$http.post("/project/#{$scope.project_id}/thread/#{thread_id}/messages/#{comment.id}/edit", {
+				content: comment.content
+				_csrf: window.csrfToken
+			})
+			$timeout () ->
+				$scope.$broadcast "review-panel:layout"
+
+		$scope.deleteComment = (thread_id, comment) ->
+			_onCommentDeleted(thread_id, comment.id)
+			$http({
+				method: "DELETE"
+				url: "/project/#{$scope.project_id}/thread/#{thread_id}/messages/#{comment.id}",
+				headers: {
+					'X-CSRF-Token': window.csrfToken
+				}
+			})
+			$timeout () ->
+				$scope.$broadcast "review-panel:layout"
 
 		$scope.setSubView = (subView) -> 
 			$scope.reviewPanel.subView = subView

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

@@ -11,6 +11,8 @@ define [
 			onResolve: "&"
 			onReply: "&"
 			onIndicatorClick: "&"
+			onSaveEdit: "&"
+			onDelete: "&"
 		link: (scope, element, attrs) ->
 			scope.state =
 				animating: false
@@ -26,4 +28,33 @@ define [
 				scope.state.animating = true
 				element.find(".rp-entry").css("top", 0)
 				$timeout((() -> scope.onResolve()), 350)
-				return true
+				return true
+			
+			scope.startEditing = (comment) ->
+				comment.editing = true
+				setTimeout () ->
+					scope.$emit "review-panel:layout"
+			
+			scope.saveEdit = (comment) ->
+				comment.editing = false
+				scope.onSaveEdit({comment:comment})
+			
+			scope.confirmDelete = (comment) ->
+				comment.deleting = true
+				setTimeout () ->
+					scope.$emit "review-panel:layout"
+				
+			scope.cancelDelete = (comment) ->
+				comment.deleting = false
+				setTimeout () ->
+					scope.$emit "review-panel:layout"
+			
+			scope.doDelete = (comment) ->
+				comment.deleting = false
+				scope.onDelete({comment: comment})
+	
+			scope.saveEditOnEnter = (ev, comment) ->
+				if ev.keyCode == 13 and !ev.shiftKey and !ev.ctrlKey and !ev.metaKey
+					ev.preventDefault()
+					scope.saveEdit(comment)
+				

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

@@ -31,8 +31,9 @@ define [
 				scope.onUnresolve({ threadId })
 				scope.resolvedComments = scope.resolvedComments.filter (c) -> c.threadId != threadId
 
-			scope.handleDelete = (entryId, threadId) ->
-				scope.onDelete({ entryId, threadId })
+			scope.handleDelete = (entryId, docId, threadId) ->
+				scope.onDelete({ entryId, docId, threadId })
+				scope.resolvedComments = scope.resolvedComments.filter (c) -> c.threadId != threadId
 
 			getDocNameById = (docId) ->
 				doc = _.find(scope.docs, (doc) -> doc.doc.id == docId)

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

@@ -351,6 +351,9 @@
 					font-weight: @rp-semibold-weight;
 					font-style: normal;
 				}
+				.rp-comment-actions {
+					a { color: @rp-type-blue; }
+				}
 
 			.rp-content-highlight {
 				color: @rp-type-darkgrey;
@@ -414,12 +417,6 @@
 			margin: 0;
 			color: @rp-type-darkgrey;
 		}
-
-		.rp-comment-metadata {
-			color: @rp-type-blue;
-			font-size: @rp-small-font-size;
-			margin: 0;
-		}
 	
 	.rp-comment-resolver {
 		color: @rp-type-blue;
@@ -452,6 +449,7 @@
 		border: solid 1px @rp-border-grey;
 		resize: vertical;
 		color: @rp-type-darkgrey;
+		margin-top: 3px;
 	}
 
 .rp-icon-delete {

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

@@ -23,6 +23,7 @@ describe "CommentsController", ->
 			'../Authentication/AuthenticationController': @AuthenticationController
 			'../User/UserInfoManager': @UserInfoManager = {}
 			'../User/UserInfoController': @UserInfoController = {}
+			"../DocumentUpdater/DocumentUpdaterHandler": @DocumentUpdaterHandler = {}
 		@req = {}
 		@res =
 			json: sinon.stub()
@@ -134,6 +135,80 @@ describe "CommentsController", ->
 		it "should return a success code", ->
 			@res.send.calledWith(204).should.equal
 
+	describe "deleteThread", ->
+		beforeEach ->
+			@req.params =
+				project_id: @project_id = "mock-project-id"
+				doc_id: @doc_id = "mock-doc-id"
+				thread_id: @thread_id = "mock-thread-id"
+			@DocumentUpdaterHandler.deleteThread = sinon.stub().yields()
+			@ChatApiHandler.deleteThread = sinon.stub().yields()
+			@CommentsController.deleteThread @req, @res
+		
+		it "should ask the doc udpater to delete the thread", ->
+			@DocumentUpdaterHandler.deleteThread
+				.calledWith(@project_id, @doc_id, @thread_id)
+				.should.equal true
+
+		it "should ask the chat handler to delete the thread", ->
+			@ChatApiHandler.deleteThread
+				.calledWith(@project_id, @thread_id)
+				.should.equal true
+
+		it "should tell the client the thread was deleted", ->
+			@EditorRealTimeController.emitToRoom
+				.calledWith(@project_id, "delete-thread", @thread_id)
+				.should.equal true
+
+		it "should return a success code", ->
+			@res.send.calledWith(204).should.equal
+
+	describe "editMessage", ->
+		beforeEach ->
+			@req.params =
+				project_id: @project_id = "mock-project-id"
+				thread_id: @thread_id = "mock-thread-id"
+				message_id: @message_id = "mock-thread-id"
+			@req.body =
+				content: @content = "mock-content"
+			@ChatApiHandler.editMessage = sinon.stub().yields()
+			@CommentsController.editMessage @req, @res
+
+		it "should ask the chat handler to edit the comment", ->
+			@ChatApiHandler.editMessage
+				.calledWith(@project_id, @thread_id, @message_id, @content)
+				.should.equal true
+
+		it "should tell the client the comment was edited", ->
+			@EditorRealTimeController.emitToRoom
+				.calledWith(@project_id, "edit-message", @thread_id, @message_id, @content)
+				.should.equal true
+
+		it "should return a success code", ->
+			@res.send.calledWith(204).should.equal
+
+	describe "deleteMessage", ->
+		beforeEach ->
+			@req.params =
+				project_id: @project_id = "mock-project-id"
+				thread_id: @thread_id = "mock-thread-id"
+				message_id: @message_id = "mock-thread-id"
+			@ChatApiHandler.deleteMessage = sinon.stub().yields()
+			@CommentsController.deleteMessage @req, @res
+
+		it "should ask the chat handler to deleted the message", ->
+			@ChatApiHandler.deleteMessage
+				.calledWith(@project_id, @thread_id, @message_id)
+				.should.equal true
+
+		it "should tell the client the message was deleted", ->
+			@EditorRealTimeController.emitToRoom
+				.calledWith(@project_id, "delete-message", @thread_id, @message_id)
+				.should.equal true
+
+		it "should return a success code", ->
+			@res.send.calledWith(204).should.equal
+
 	describe "_injectUserInfoIntoThreads", ->
 		beforeEach ->
 			@users = {

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

@@ -330,3 +330,38 @@ describe 'DocumentUpdaterHandler', ->
 				@callback
 					.calledWith(new Error("doc updater returned failure status code: 500"))
 					.should.equal true
+
+	describe "deleteThread", ->
+		beforeEach ->
+			@thread_id = "mock-thread-id-1"
+			@callback = sinon.stub()
+
+		describe "successfully", ->
+			beforeEach ->
+				@request.del = sinon.stub().callsArgWith(1, null, {statusCode: 200}, @body)
+				@handler.deleteThread @project_id, @doc_id, @thread_id, @callback
+
+			it 'should delete the thread in the document updater', ->
+				url = "#{@settings.apis.documentupdater.url}/project/#{@project_id}/doc/#{@doc_id}/comment/#{@thread_id}"
+				@request.del.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.del = sinon.stub().callsArgWith(1, @error = new Error("something went wrong"), null, null)
+				@handler.deleteThread @project_id, @doc_id, @thread_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.del = sinon.stub().callsArgWith(1, null, { statusCode: 500 }, "")
+				@handler.deleteThread @project_id, @doc_id, @thread_id, @callback
+
+			it "should return the callback with an error", ->
+				@callback
+					.calledWith(new Error("doc updater returned failure status code: 500"))
+					.should.equal true