[ds-mobile-app] Implement reject changes in document updater (#27483)

* [ds-mobile-app] Implement reject changes in document updater

* Return rejected change ids

GitOrigin-RevId: 7915170af9164852ce39d783776106e722aaa925

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

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,

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,

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

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}`,

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