diff --git a/services/document-updater/app/js/DocumentManager.js b/services/document-updater/app/js/DocumentManager.js index 8b6ca6e565..8a27c18c69 100644 --- a/services/document-updater/app/js/DocumentManager.js +++ b/services/document-updater/app/js/DocumentManager.js @@ -422,7 +422,7 @@ const DocumentManager = { ) }, - deleteComment(projectId, docId, commentId, userId, _callback) { + deleteComment(projectId, docId, commentId, _callback) { const timer = new Metrics.Timer('docManager.deleteComment') const callback = (...args) => { timer.done() @@ -432,17 +432,7 @@ const DocumentManager = { DocumentManager.getDoc( projectId, docId, - ( - error, - lines, - version, - ranges, - pathname, - projectHistoryId, - unflushedTime, - alreadyLoaded, - historyRangesSupport - ) => { + (error, lines, version, ranges) => { if (error) { return callback(error) } @@ -471,27 +461,7 @@ const DocumentManager = { if (error) { return callback(error) } - if (historyRangesSupport) { - ProjectHistoryRedisManager.queueOps( - projectId, - JSON.stringify({ - pathname, - deleteComment: commentId, - meta: { - ts: new Date(), - user_id: userId, - }, - }), - error => { - if (error) { - return callback(error) - } - callback() - } - ) - } else { - callback() - } + callback() } ) } @@ -689,14 +659,13 @@ const DocumentManager = { ) }, - deleteCommentWithLock(projectId, docId, threadId, userId, callback) { + deleteCommentWithLock(projectId, docId, threadId, callback) { const UpdateManager = require('./UpdateManager') UpdateManager.lockUpdatesAndDo( DocumentManager.deleteComment, projectId, docId, threadId, - userId, callback ) }, diff --git a/services/document-updater/app/js/HttpController.js b/services/document-updater/app/js/HttpController.js index bc4fed1287..b23302121d 100644 --- a/services/document-updater/app/js/HttpController.js +++ b/services/document-updater/app/js/HttpController.js @@ -307,23 +307,16 @@ function deleteComment(req, res, next) { doc_id: docId, comment_id: commentId, } = req.params - const userId = req.body.user_id logger.debug({ projectId, docId, commentId }, 'deleting comment via http') const timer = new Metrics.Timer('http.deleteComment') - DocumentManager.deleteCommentWithLock( - projectId, - docId, - commentId, - userId, - error => { - timer.done() - if (error) { - return next(error) - } - logger.debug({ projectId, docId, commentId }, 'deleted comment via http') - res.sendStatus(204) // No Content + DocumentManager.deleteCommentWithLock(projectId, docId, commentId, error => { + timer.done() + if (error) { + return next(error) } - ) + logger.debug({ projectId, docId, commentId }, 'deleted comment via http') + res.sendStatus(204) // No Content + }) } function updateProject(req, res, next) { diff --git a/services/document-updater/test/unit/js/DocumentManager/DocumentManagerTests.js b/services/document-updater/test/unit/js/DocumentManager/DocumentManagerTests.js index 93d204d38e..2e0e071ac9 100644 --- a/services/document-updater/test/unit/js/DocumentManager/DocumentManagerTests.js +++ b/services/document-updater/test/unit/js/DocumentManager/DocumentManagerTests.js @@ -833,25 +833,13 @@ describe('DocumentManager', function () { this.lines = ['original', 'lines'] this.ranges = { comments: ['one', 'two', 'three'] } this.updated_ranges = { comments: ['one', 'three'] } - this.historyRangesSupport = true this.DocumentManager.getDoc = sinon .stub() - .yields( - null, - this.lines, - this.version, - this.ranges, - this.pathname, - this.projectHistoryId, - Date.now() - 1e9, - true, - this.historyRangesSupport - ) + .yields(null, this.lines, this.version, this.ranges) this.RangesManager.deleteComment = sinon .stub() .returns(this.updated_ranges) this.RedisManager.updateDocument = sinon.stub().yields() - this.ProjectHistoryRedisManager.queueOps = sinon.stub().yields() }) describe('successfully', function () { @@ -860,7 +848,6 @@ describe('DocumentManager', function () { this.project_id, this.doc_id, this.comment_id, - this.user_id, this.callback ) }) @@ -891,23 +878,6 @@ describe('DocumentManager', function () { .should.equal(true) }) - it('should queue the delete comment operation', function () { - this.ProjectHistoryRedisManager.queueOps - .calledWith( - this.project_id, - JSON.stringify({ - pathname: this.pathname, - deleteComment: this.comment_id, - meta: { - ts: new Date(), - user_id: this.user_id, - }, - }), - sinon.match.func - ) - .should.equal(true) - }) - it('should call the callback', function () { this.callback.called.should.equal(true) }) diff --git a/services/document-updater/test/unit/js/HttpController/HttpControllerTests.js b/services/document-updater/test/unit/js/HttpController/HttpControllerTests.js index f98c08a45e..adfe461ea7 100644 --- a/services/document-updater/test/unit/js/HttpController/HttpControllerTests.js +++ b/services/document-updater/test/unit/js/HttpController/HttpControllerTests.js @@ -11,7 +11,6 @@ describe('HttpController', function () { './HistoryManager': (this.HistoryManager = { flushProjectChangesAsync: sinon.stub(), }), - './ProjectHistoryRedisManager': (this.ProjectHistoryRedisManager = {}), './ProjectManager': (this.ProjectManager = {}), './ProjectFlusher': { flushAllProjects() {} }, './DeleteQueueManager': (this.DeleteQueueManager = {}), @@ -652,7 +651,6 @@ describe('HttpController', function () { describe('deleteComment', function () { beforeEach(function () { - this.user_id = 'user-id-123' this.req = { params: { project_id: this.project_id, @@ -660,9 +658,7 @@ describe('HttpController', function () { comment_id: (this.comment_id = 'mock-comment-id'), }, query: {}, - body: { - user_id: this.user_id, - }, + body: {}, } }) @@ -670,20 +666,13 @@ describe('HttpController', function () { beforeEach(function () { this.DocumentManager.deleteCommentWithLock = sinon .stub() - .callsArgWith(4) - - this.ProjectHistoryRedisManager.queueOps = sinon.stub() + .callsArgWith(3) this.HttpController.deleteComment(this.req, this.res, this.next) }) it('should accept the change', function () { this.DocumentManager.deleteCommentWithLock - .calledWith( - this.project_id, - this.doc_id, - this.comment_id, - this.user_id - ) + .calledWith(this.project_id, this.doc_id, this.comment_id) .should.equal(true) }) @@ -713,7 +702,7 @@ describe('HttpController', function () { beforeEach(function () { this.DocumentManager.deleteCommentWithLock = sinon .stub() - .callsArgWith(4, new Error('oops')) + .callsArgWith(3, new Error('oops')) this.HttpController.deleteComment(this.req, this.res, this.next) }) diff --git a/services/project-history/app/js/UpdateTranslator.js b/services/project-history/app/js/UpdateTranslator.js index f17d42801a..5f46cd723f 100644 --- a/services/project-history/app/js/UpdateTranslator.js +++ b/services/project-history/app/js/UpdateTranslator.js @@ -14,7 +14,6 @@ import * as OperationsCompressor from './OperationsCompressor.js' * @typedef {import('./types.ts').Op} Op * @typedef {import('./types.ts').RenameUpdate} RenameUpdate * @typedef {import('./types.ts').TextUpdate} TextUpdate - * @typedef {import('./types.ts').DeleteCommentUpdate} DeleteCommentUpdate * @typedef {import('./types.ts').Update} Update * @typedef {import('./types.ts').UpdateWithBlob} UpdateWithBlob */ @@ -78,13 +77,6 @@ function _convertToChange(projectId, updateWithBlob) { if (update.v != null) { v2DocVersions[update.doc] = { pathname, v: update.v } } - } else if (isDeleteCommentUpdate(update)) { - operations = [ - { - pathname: _convertPathname(update.pathname), - deleteComment: update.deleteComment, - }, - ] } else { const error = new Errors.UpdateWithUnknownFormatError( 'update with unknown format', @@ -188,14 +180,6 @@ export function isAddUpdate(update) { return _isAddDocUpdate(update) || _isAddFileUpdate(update) } -/** - * @param {Update} update - * @returns {update is DeleteCommentUpdate} - */ -export function isDeleteCommentUpdate(update) { - return 'deleteComment' in update -} - export function _convertPathname(pathname) { // Strip leading / pathname = pathname.replace(/^\//, '') diff --git a/services/project-history/app/js/types.ts b/services/project-history/app/js/types.ts index b1af671335..e508f3cd35 100644 --- a/services/project-history/app/js/types.ts +++ b/services/project-history/app/js/types.ts @@ -1,4 +1,4 @@ -export type Update = TextUpdate | AddDocUpdate | AddFileUpdate | RenameUpdate | DeleteCommentUpdate +export type Update = TextUpdate | AddDocUpdate | AddFileUpdate | RenameUpdate export type UpdateMeta = { user_id: string @@ -18,12 +18,6 @@ export type TextUpdate = { } } -export type DeleteCommentUpdate = { - pathname: string - deleteComment: string - meta: UpdateMeta -} - type ProjectUpdateBase = { version: string projectHistoryId: string diff --git a/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterHandler.js b/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterHandler.js index c331665c1a..f077c28313 100644 --- a/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterHandler.js +++ b/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterHandler.js @@ -212,14 +212,11 @@ function acceptChanges(projectId, docId, changeIds, callback) { ) } -function deleteThread(projectId, docId, threadId, userId, callback) { +function deleteThread(projectId, docId, threadId, callback) { _makeRequest( { path: `/project/${projectId}/doc/${docId}/comment/${threadId}`, method: 'DELETE', - json: { - user_id: userId, - }, }, projectId, 'delete-thread', diff --git a/services/web/test/unit/src/DocumentUpdater/DocumentUpdaterHandlerTests.js b/services/web/test/unit/src/DocumentUpdater/DocumentUpdaterHandlerTests.js index 23ec1e6875..820642d124 100644 --- a/services/web/test/unit/src/DocumentUpdater/DocumentUpdaterHandlerTests.js +++ b/services/web/test/unit/src/DocumentUpdater/DocumentUpdaterHandlerTests.js @@ -766,7 +766,6 @@ describe('DocumentUpdaterHandler', function () { this.project_id, this.doc_id, this.thread_id, - this.user_id, this.callback ) }) @@ -797,7 +796,6 @@ describe('DocumentUpdaterHandler', function () { this.project_id, this.doc_id, this.thread_id, - this.user_id, this.callback ) }) @@ -816,7 +814,6 @@ describe('DocumentUpdaterHandler', function () { this.project_id, this.doc_id, this.thread_id, - this.user_id, this.callback ) })