diff --git a/services/document-updater/app.js b/services/document-updater/app.js index 65c9895377..22a1421c02 100644 --- a/services/document-updater/app.js +++ b/services/document-updater/app.js @@ -176,6 +176,10 @@ app.post( '/project/:project_id/doc/:doc_id/change/accept', HttpController.acceptChanges ) +app.post( + '/project/:project_id/doc/:doc_id/change/reject', + HttpController.rejectChanges +) app.post( '/project/:project_id/doc/:doc_id/comment/:comment_id/resolve', HttpController.resolveComment diff --git a/services/document-updater/app/js/DocumentManager.js b/services/document-updater/app/js/DocumentManager.js index 3fb3d10a6e..442cf96164 100644 --- a/services/document-updater/app/js/DocumentManager.js +++ b/services/document-updater/app/js/DocumentManager.js @@ -384,6 +384,64 @@ const DocumentManager = { } }, + async rejectChanges(projectId, docId, changeIds, userId) { + const UpdateManager = require('./UpdateManager') + const HistoryOTUpdateManager = require('./HistoryOTUpdateManager') + + const { lines, version, ranges } = await DocumentManager.getDoc( + projectId, + docId + ) + if (lines == null || version == null) { + throw new Errors.NotFoundError(`document not found: ${docId}`) + } + + const changesToReject = ranges.changes + ? ranges.changes.filter(change => changeIds.includes(change.id)) + : [] + + // Apply inverted operations for rejected changes (based on reject-changes.ts logic) + // Sort changes in reverse order by position to avoid conflicts + changesToReject.sort((a, b) => b.op.p - a.op.p) + + const ops = [] + for (const change of changesToReject) { + if (change.op.i) { + const deleteOp = { + p: change.op.p, + d: change.op.i, + u: true, + } + ops.push(deleteOp) + } else if (change.op.d) { + const insertOp = { + p: change.op.p, + i: change.op.d, + u: true, + } + ops.push(insertOp) + } + } + + const update = { + doc: docId, + op: ops, + v: version, + meta: { + user_id: userId, + ts: new Date().toISOString(), + }, + } + + if (HistoryOTUpdateManager.isHistoryOTEditOperationUpdate(update)) { + await HistoryOTUpdateManager.applyUpdate(projectId, docId, update) + } else { + await UpdateManager.promises.applyUpdate(projectId, docId, update) + } + + return { rejectedChangeIds: changesToReject.map(c => c.id) } + }, + async updateCommentState(projectId, docId, commentId, userId, resolved) { const { lines, version, pathname, historyRangesSupport } = await DocumentManager.getDoc(projectId, docId) @@ -658,6 +716,17 @@ const DocumentManager = { ) }, + async rejectChangesWithLock(projectId, docId, changeIds, userId) { + const UpdateManager = require('./UpdateManager') + return await UpdateManager.promises.lockUpdatesAndDo( + DocumentManager.rejectChanges, + projectId, + docId, + changeIds, + userId + ) + }, + async updateCommentStateWithLock( projectId, docId, diff --git a/services/document-updater/app/js/HttpController.js b/services/document-updater/app/js/HttpController.js index 0a6ae3b2b4..04dfdaf543 100644 --- a/services/document-updater/app/js/HttpController.js +++ b/services/document-updater/app/js/HttpController.js @@ -363,6 +363,33 @@ function acceptChanges(req, res, next) { }) } +function rejectChanges(req, res, next) { + const { project_id: projectId, doc_id: docId } = req.params + const changeIds = req.body.change_ids + const userId = req.body.user_id + + logger.debug( + { projectId, docId }, + `rejecting ${changeIds.length} changes via http` + ) + DocumentManager.rejectChangesWithLock( + projectId, + docId, + changeIds, + userId, + (error, response) => { + if (error) { + return next(error) + } + logger.debug( + { projectId, docId, changeIds, response }, + `rejected ${changeIds.length} changes via http` + ) + res.json(response) + } + ) +} + function resolveComment(req, res, next) { const { project_id: projectId, @@ -559,6 +586,7 @@ module.exports = { deleteProject, deleteMultipleProjects, acceptChanges, + rejectChanges, resolveComment, reopenComment, deleteComment, diff --git a/services/document-updater/test/acceptance/js/RejectingChangesTests.js b/services/document-updater/test/acceptance/js/RejectingChangesTests.js new file mode 100644 index 0000000000..8b53e50ecf --- /dev/null +++ b/services/document-updater/test/acceptance/js/RejectingChangesTests.js @@ -0,0 +1,300 @@ +const sinon = require('sinon') +const { expect } = require('chai') + +const MockWebApi = require('./helpers/MockWebApi') +const DocUpdaterClient = require('./helpers/DocUpdaterClient') +const DocUpdaterApp = require('./helpers/DocUpdaterApp') + +const sandbox = sinon.createSandbox() + +describe('Rejecting Changes', function () { + before(function (done) { + DocUpdaterApp.ensureRunning(done) + }) + + describe('rejecting a single change', function () { + beforeEach(function (done) { + this.project_id = DocUpdaterClient.randomId() + this.user_id = DocUpdaterClient.randomId() + this.doc = { + id: DocUpdaterClient.randomId(), + lines: ['the brown fox jumps over the lazy dog'], + } + + MockWebApi.insertDoc(this.project_id, this.doc.id, { + lines: this.doc.lines, + version: 0, + historyRangesSupport: true, + }) + + this.id_seed = 'tc_reject_test' + this.update = { + doc: this.doc.id, + op: [{ i: 'quick ', p: 4 }], + v: 0, + meta: { + user_id: this.user_id, + tc: this.id_seed, + }, + } + + DocUpdaterClient.sendUpdate( + this.project_id, + this.doc.id, + this.update, + done + ) + }) + + afterEach(function () { + sandbox.restore() + }) + + it('should reject the change and restore the original text', function (done) { + DocUpdaterClient.getDoc( + this.project_id, + this.doc.id, + (error, res, data) => { + if (error != null) { + throw error + } + + expect(data.ranges.changes).to.have.length(1) + const change = data.ranges.changes[0] + expect(change.op).to.deep.equal({ i: 'quick ', p: 4 }) + expect(change.id).to.equal(this.id_seed + '000001') + + expect(data.lines).to.deep.equal([ + 'the quick brown fox jumps over the lazy dog', + ]) + + DocUpdaterClient.rejectChanges( + this.project_id, + this.doc.id, + [change.id], + this.user_id, + (error, res, body) => { + if (error != null) { + throw error + } + + expect(res.statusCode).to.equal(200) + expect(body.rejectedChangeIds).to.be.an('array') + expect(body.rejectedChangeIds).to.include(change.id) + + DocUpdaterClient.getDoc( + this.project_id, + this.doc.id, + (error, res, data) => { + if (error != null) { + throw error + } + + expect(data.ranges.changes || []).to.have.length(0) + expect(data.lines).to.deep.equal([ + 'the brown fox jumps over the lazy dog', + ]) + done() + } + ) + } + ) + } + ) + }) + + it('should return 200 status code with rejectedChangeIds on successful rejection', function (done) { + DocUpdaterClient.getDoc( + this.project_id, + this.doc.id, + (error, res, data) => { + if (error != null) { + throw error + } + + const changeId = data.ranges.changes[0].id + + DocUpdaterClient.rejectChanges( + this.project_id, + this.doc.id, + [changeId], + this.user_id, + (error, res, body) => { + if (error != null) { + throw error + } + + expect(res.statusCode).to.equal(200) + expect(body.rejectedChangeIds).to.be.an('array') + expect(body.rejectedChangeIds).to.include(changeId) + done() + } + ) + } + ) + }) + }) + + describe('rejecting multiple changes', function () { + beforeEach(function (done) { + this.project_id = DocUpdaterClient.randomId() + this.user_id = DocUpdaterClient.randomId() + this.doc = { + id: DocUpdaterClient.randomId(), + lines: ['the brown fox jumps over the lazy dog'], + } + + MockWebApi.insertDoc(this.project_id, this.doc.id, { + lines: this.doc.lines, + version: 0, + historyRangesSupport: true, + }) + + this.id_seed_1 = 'tc_reject_1' + this.id_seed_2 = 'tc_reject_2' + + this.updates = [ + { + doc: this.doc.id, + op: [{ i: 'quick ', p: 4 }], + v: 0, + meta: { + user_id: this.user_id, + tc: this.id_seed_1, + }, + }, + { + doc: this.doc.id, + op: [{ d: 'lazy ', p: 35 }], + v: 1, + meta: { + user_id: this.user_id, + tc: this.id_seed_2, + }, + }, + ] + + DocUpdaterClient.sendUpdates( + this.project_id, + this.doc.id, + this.updates, + done + ) + }) + + afterEach(function () { + sandbox.restore() + }) + + it('should reject multiple changes in order', function (done) { + DocUpdaterClient.getDoc( + this.project_id, + this.doc.id, + (error, res, data) => { + if (error != null) { + throw error + } + + expect(data.ranges.changes).to.have.length(2) + + expect(data.lines).to.deep.equal([ + 'the quick brown fox jumps over the dog', + ]) + + const changeIds = data.ranges.changes.map(change => change.id) + + DocUpdaterClient.rejectChanges( + this.project_id, + this.doc.id, + changeIds, + this.user_id, + (error, res, body) => { + if (error != null) { + throw error + } + + expect(res.statusCode).to.equal(200) + expect(body.rejectedChangeIds).to.be.an('array') + expect(body.rejectedChangeIds).to.have.length(2) + expect(body.rejectedChangeIds).to.include.members(changeIds) + + DocUpdaterClient.getDoc( + this.project_id, + this.doc.id, + (error, res, data) => { + if (error != null) { + throw error + } + + expect(data.ranges.changes || []).to.have.length(0) + expect(data.lines).to.deep.equal([ + 'the brown fox jumps over the lazy dog', + ]) + done() + } + ) + } + ) + } + ) + }) + }) + + describe('error cases', function () { + beforeEach(function (done) { + this.project_id = DocUpdaterClient.randomId() + this.user_id = DocUpdaterClient.randomId() + this.doc = { + id: DocUpdaterClient.randomId(), + lines: ['the brown fox jumps over the lazy dog'], + } + + MockWebApi.insertDoc(this.project_id, this.doc.id, { + lines: this.doc.lines, + version: 0, + historyRangesSupport: true, + }) + + DocUpdaterApp.ensureRunning(done) + }) + + it('should handle rejection of non-existent changes gracefully', function (done) { + const nonExistentChangeId = 'nonexistent_change_id' + + DocUpdaterClient.rejectChanges( + this.project_id, + this.doc.id, + [nonExistentChangeId], + this.user_id, + (error, res, body) => { + // Should still return 200 with empty rejectedChangeIds if no changes were found to reject + if (error != null) { + throw error + } + expect(res.statusCode).to.equal(200) + expect(body.rejectedChangeIds).to.be.an('array') + expect(body.rejectedChangeIds).to.have.length(0) + done() + } + ) + }) + + it('should handle empty change_ids array', function (done) { + DocUpdaterClient.rejectChanges( + this.project_id, + this.doc.id, + [], + this.user_id, + (error, res, body) => { + if (error != null) { + throw error + } + expect(res.statusCode).to.equal(200) + expect(body.rejectedChangeIds).to.be.an('array') + expect(body.rejectedChangeIds).to.have.length(0) + done() + } + ) + }) + }) +}) diff --git a/services/document-updater/test/acceptance/js/helpers/DocUpdaterClient.js b/services/document-updater/test/acceptance/js/helpers/DocUpdaterClient.js index 0a4ec8922e..a915aed95b 100644 --- a/services/document-updater/test/acceptance/js/helpers/DocUpdaterClient.js +++ b/services/document-updater/test/acceptance/js/helpers/DocUpdaterClient.js @@ -215,6 +215,16 @@ module.exports = DocUpdaterClient = { ) }, + rejectChanges(projectId, docId, changeIds, userId, callback) { + request.post( + { + url: `http://127.0.0.1:3003/project/${projectId}/doc/${docId}/change/reject`, + json: { change_ids: changeIds, user_id: userId }, + }, + callback + ) + }, + removeComment(projectId, docId, comment, callback) { request.del( `http://127.0.0.1:3003/project/${projectId}/doc/${docId}/comment/${comment}`, diff --git a/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterHandler.js b/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterHandler.js index 497686398c..c59e796c1e 100644 --- a/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterHandler.js +++ b/services/web/app/src/Features/DocumentUpdater/DocumentUpdaterHandler.js @@ -246,6 +246,25 @@ function acceptChanges(projectId, docId, changeIds, callback) { ) } +/** + * @param {string} projectId + * @param {string} docId + * @param {string[]} changeIds + * @param {Callback} callback + */ +function rejectChanges(projectId, docId, changeIds, userId, callback) { + _makeRequest( + { + path: `/project/${projectId}/doc/${docId}/change/reject`, + json: { change_ids: changeIds, user_id: userId }, + method: 'POST', + }, + projectId, + 'reject-changes', + callback + ) +} + function resolveThread(projectId, docId, threadId, userId, callback) { _makeRequest( { @@ -630,6 +649,7 @@ module.exports = { getProjectDocsIfMatch, clearProjectState, acceptChanges, + rejectChanges, resolveThread, reopenThread, deleteThread, @@ -655,6 +675,7 @@ module.exports = { getProjectLastUpdatedAt: promisify(getProjectLastUpdatedAt), clearProjectState: promisify(clearProjectState), acceptChanges: promisify(acceptChanges), + rejectChanges: promisify(rejectChanges), resolveThread: promisify(resolveThread), reopenThread: promisify(reopenThread), deleteThread: promisify(deleteThread),