Trigger reject tracked changes notification from document-updater

GitOrigin-RevId: 9ac47490d052b3058931ca250f4090e6576f56b2

services/document-updater/app/js/RangesManager.js

@@ -29,7 +29,7 @@ const RangesManager = {
    * @param {string[]} newDocLines - the document lines after the updates were applied
    * @param {object} opts
    * @param {boolean} [opts.historyRangesSupport] - whether history ranges support is enabled
-   * @returns {{ newRanges: Ranges, rangesWereCollapsed: boolean, historyUpdates: HistoryUpdate[] }}
+   * @returns {{ newRanges: Ranges, rangesWereCollapsed: boolean, historyUpdates: HistoryUpdate[], removedChangeIds: string[] }}
    */
   applyUpdate(projectId, docId, ranges, updates, newDocLines, opts = {}) {
     if (ranges == null) {
@@ -114,6 +114,9 @@ const RangesManager = {
       )
     }
     const newRanges = RangesManager._getRanges(rangesTracker)
+    const removedChangeIds = Object.keys(
+      rangesTracker.getDirtyState().change.removed
+    )
     logger.debug(
       {
         projectId,
@@ -124,7 +127,7 @@ const RangesManager = {
       },
       'applied updates to ranges'
     )
-    return { newRanges, rangesWereCollapsed, historyUpdates }
+    return { newRanges, rangesWereCollapsed, historyUpdates, removedChangeIds }
   },
 
   acceptChanges(projectId, docId, changeIds, ranges, lines) {

services/document-updater/app/js/UpdateManager.js

@@ -13,6 +13,7 @@ const Errors = require('./Errors')
 const DocumentManager = require('./DocumentManager')
 const RangesManager = require('./RangesManager')
 const SnapshotManager = require('./SnapshotManager')
+const WebApiManager = require('./WebApiManager')
 const Profiler = require('./Profiler')
 const { isInsert, isDelete, getDocLength, computeDocHash } = require('./Utils')
 const HistoryOTUpdateManager = require('./HistoryOTUpdateManager')
@@ -143,15 +144,19 @@ const UpdateManager = {
         sync: incomingUpdateVersion === previousVersion,
       })
 
-      const { newRanges, rangesWereCollapsed, historyUpdates } =
+      const {
-        RangesManager.applyUpdate(
+        newRanges,
-          projectId,
+        rangesWereCollapsed,
-          docId,
+        historyUpdates,
-          ranges,
+        removedChangeIds,
-          appliedOps,
+      } = RangesManager.applyUpdate(
-          updatedDocLines,
+        projectId,
-          { historyRangesSupport }
+        docId,
-        )
+        ranges,
+        appliedOps,
+        updatedDocLines,
+        { historyRangesSupport }
+      )
       profile.log('RangesManager.applyUpdate', { sync: true })
 
       await RedisManager.promises.updateDocument(
@@ -197,6 +202,27 @@ const UpdateManager = {
         }
       }
 
+      // applyUpdate is not triggered by accept change operations, so any
+      // tracked change removed by the ops we just applied was rejected.
+      if (removedChangeIds.length > 0) {
+        // Fire-and-forget without awaiting because
+        // we hold the doc lock here, and the result of the
+        // notification doesn't affect the update
+        WebApiManager.promises
+          .notifyTrackChangesRejected(
+            projectId,
+            docId,
+            removedChangeIds,
+            update.meta?.user_id
+          )
+          .catch(err => {
+            logger.warn(
+              { err, projectId, docId, removedChangeIds },
+              'failed to notify web of rejected track changes'
+            )
+          })
+      }
+
       if (rangesWereCollapsed) {
         Metrics.inc('doc-snapshot')
         logger.debug(

services/document-updater/app/js/WebApiManager.js

@@ -0,0 +1,39 @@
+// @ts-check
+const Settings = require('@overleaf/settings')
+const { fetchNothing } = require('@overleaf/fetch-utils')
+
+const MAX_HTTP_REQUEST_LENGTH = 5000
+
+/**
+ * @param {string} projectId
+ * @param {string} docId
+ * @param {string[]} rejectedChangeIds
+ * @param {string | undefined} userId
+ */
+async function notifyTrackChangesRejected(
+  projectId,
+  docId,
+  rejectedChangeIds,
+  userId
+) {
+  const url = new URL(
+    `/project/${projectId}/doc/${docId}/changes/reject`,
+    Settings.apis.web.url
+  )
+
+  await fetchNothing(url, {
+    method: 'POST',
+    json: { rejectedChangeIds, userId },
+    basicAuth: {
+      user: Settings.apis.web.user,
+      password: Settings.apis.web.pass,
+    },
+    signal: AbortSignal.timeout(MAX_HTTP_REQUEST_LENGTH),
+  })
+}
+
+module.exports = {
+  promises: {
+    notifyTrackChangesRejected,
+  },
+}

services/document-updater/test/unit/js/RangesManager/RangesManagerTests.js

@@ -258,6 +258,39 @@ describe('RangesManager', function () {
       })
     })
 
+    describe('removedChangeIds', function () {
+      it('should be empty when no tracked changes are removed', function () {
+        const result = this.RangesManager.applyUpdate(
+          this.project_id,
+          this.doc_id,
+          this.ranges,
+          this.updates,
+          this.newDocLines
+        )
+        expect(result.removedChangeIds).to.deep.equal([])
+      })
+
+      it('should report the ids of tracked changes that were removed by the update', function () {
+        // original text is "one [two ]three four five"
+        // [] denotes tracked deletes; rejecting it by re-inserting the
+        // matching text drops the tracked change.
+        const ranges = {
+          changes: makeRanges([{ d: 'two ', p: 4 }]),
+        }
+        const updates = makeUpdates([{ i: 'two ', p: 4, u: true }])
+        const newDocLines = ['one two three four five']
+        const result = this.RangesManager.applyUpdate(
+          this.project_id,
+          this.doc_id,
+          ranges,
+          updates,
+          newDocLines,
+          { historyRangesSupport: true }
+        )
+        expect(result.removedChangeIds).to.deep.equal(['1'])
+      })
+    })
+
     describe('with history ranges support', function () {
       describe('inserts among tracked deletes', function () {
         beforeEach(function () {

services/document-updater/test/unit/js/UpdateManager/UpdateManagerTests.js

@@ -74,6 +74,12 @@ describe('UpdateManager', function () {
       },
     }
 
+    this.WebApiManager = {
+      promises: {
+        notifyTrackChangesRejected: sinon.stub().resolves(),
+      },
+    }
+
     this.ProjectHistoryRedisManager = {
       promises: {
         queueOps: sinon
@@ -94,6 +100,7 @@ describe('UpdateManager', function () {
         './DocumentManager': this.DocumentManager,
         './RangesManager': this.RangesManager,
         './SnapshotManager': this.SnapshotManager,
+        './WebApiManager': this.WebApiManager,
         './Profiler': this.Profiler,
         './ProjectHistoryRedisManager': this.ProjectHistoryRedisManager,
       },
@@ -337,6 +344,7 @@ describe('UpdateManager', function () {
         newRanges: this.updated_ranges,
         rangesWereCollapsed: false,
         historyUpdates: this.historyUpdates,
+        removedChangeIds: [],
       })
       this.ShareJsUpdateManager.promises.applyUpdate = sinon.stub().resolves({
         updatedDocLines: this.updatedDocLines,
@@ -468,6 +476,7 @@ describe('UpdateManager', function () {
           newRanges: this.updated_ranges,
           rangesWereCollapsed: true,
           historyUpdates: this.historyUpdates,
+          removedChangeIds: [],
         })
         await this.UpdateManager.promises.applyUpdate(
           this.project_id,
@@ -524,6 +533,48 @@ describe('UpdateManager', function () {
         )
       })
     })
+
+    describe('when tracked changes are rejected', function () {
+      beforeEach(async function () {
+        this.removedChangeIds = ['change-1', 'change-2']
+        this.RangesManager.applyUpdate.returns({
+          newRanges: this.updated_ranges,
+          rangesWereCollapsed: false,
+          historyUpdates: this.historyUpdates,
+          removedChangeIds: this.removedChangeIds,
+        })
+        await this.UpdateManager.promises.applyUpdate(
+          this.project_id,
+          this.doc_id,
+          this.update
+        )
+      })
+
+      it('should notify web of the rejected tracked changes', function () {
+        this.WebApiManager.promises.notifyTrackChangesRejected.should.have.been.calledWith(
+          this.project_id,
+          this.doc_id,
+          this.removedChangeIds,
+          this.updateMeta.user_id
+        )
+      })
+    })
+
+    describe('when no tracked changes are rejected', function () {
+      beforeEach(async function () {
+        await this.UpdateManager.promises.applyUpdate(
+          this.project_id,
+          this.doc_id,
+          this.update
+        )
+      })
+
+      it('should not notify web', function () {
+        this.WebApiManager.promises.notifyTrackChangesRejected.called.should.equal(
+          false
+        )
+      })
+    })
   })
 
   describe('_adjustHistoryUpdatesMetadata', function () {

services/web/app/src/Features/Documents/DocumentController.mjs

@@ -105,7 +105,21 @@ async function setDocument(req, res) {
   res.json(result)
 }
 
+async function trackChangesRejected(req, res) {
+  const { Project_id: projectId, doc_id: docId } = req.params
+  const { rejectedChangeIds, userId } = req.body
+  await Modules.promises.hooks.fire(
+    'trackChangesRejected',
+    projectId,
+    docId,
+    rejectedChangeIds,
+    userId
+  )
+  res.sendStatus(204)
+}
+
 export default {
   getDocument: expressify(getDocument),
   setDocument: expressify(setDocument),
+  trackChangesRejected: expressify(trackChangesRejected),
 }

services/web/app/src/router.mjs

@@ -941,6 +941,11 @@ async function initialize(webRouter, privateApiRouter, publicApiRouter) {
     AuthenticationController.requirePrivateApiAuth(),
     DocumentController.setDocument
   )
+  privateApiRouter.post(
+    '/project/:Project_id/doc/:doc_id/changes/reject',
+    AuthenticationController.requirePrivateApiAuth(),
+    DocumentController.trackChangesRejected
+  )
 
   privateApiRouter.post(
     '/user/:user_id/project/new',