Revert "Send operations to project-history when deleting comments (#17198)" (#17476)

This reverts commit 34ab7e4ea6dd1b5fd749f9accaf4a6e16ae1d840.

GitOrigin-RevId: 005de52bcacbacf62d22cd8617d7a765f56bd72e
This commit is contained in:
Domagoj Kriskovic 2024-03-08 16:17:16 +01:00 committed by Copybot
parent 4a3a0752a4
commit f5b894bc2e
8 changed files with 18 additions and 125 deletions

View file

@ -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
)
},

View file

@ -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) {

View file

@ -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)
})

View file

@ -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)
})

View file

@ -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(/^\//, '')

View file

@ -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

View file

@ -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',

View file

@ -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
)
})