Merge pull request #18654 from overleaf/mj-web-duplicate-threads

[web] Ensure single doc pointing to comment when reverting

GitOrigin-RevId: e86e566e1b21eed18bb08b285befcab0e740ec45

services/web/app/src/Features/Chat/ChatApiHandler.js

@@ -97,6 +97,18 @@ async function getResolvedThreadIds(projectId) {
   return body.resolvedThreadIds
 }
 
+async function duplicateCommentThreads(projectId, threads) {
+  return await fetchJson(
+    chatApiUrl(`/project/${projectId}/duplicate-comment-threads`),
+    {
+      method: 'POST',
+      json: {
+        threads,
+      },
+    }
+  )
+}
+
 function chatApiUrl(path) {
   return new URL(path, settings.apis.chat.internal_url)
 }
@@ -113,6 +125,7 @@ module.exports = {
   editMessage: callbackify(editMessage),
   deleteMessage: callbackify(deleteMessage),
   getResolvedThreadIds: callbackify(getResolvedThreadIds),
+  duplicateCommentThreads: callbackify(duplicateCommentThreads),
   promises: {
     getThreads,
     destroyProject,
@@ -125,5 +138,6 @@ module.exports = {
     editMessage,
     deleteMessage,
     getResolvedThreadIds,
+    duplicateCommentThreads,
   },
 }

services/web/app/src/Features/History/RestoreManager.js

@@ -9,6 +9,9 @@ const { callbackifyAll } = require('@overleaf/promise-utils')
 const { fetchJson } = require('@overleaf/fetch-utils')
 const ProjectLocator = require('../Project/ProjectLocator')
 const DocumentUpdaterHandler = require('../DocumentUpdater/DocumentUpdaterHandler')
+const ChatApiHandler = require('../Chat/ChatApiHandler')
+const DocstoreManager = require('../Docstore/DocstoreManager')
+const logger = require('@overleaf/logger')
 
 const RestoreManager = {
   async restoreFileFromV2(userId, projectId, version, pathname) {
@@ -107,12 +110,60 @@ const RestoreManager = {
       pathname
     )
 
+    const documentCommentIds = new Set(
+      ranges.comments?.map(({ op: { t } }) => t)
+    )
+
+    await DocumentUpdaterHandler.promises.flushProjectToMongo(projectId)
+
+    const docsWithRanges =
+      await DocstoreManager.promises.getAllRanges(projectId)
+
+    const nonOrphanedThreadIds = new Set()
+    for (const { ranges } of docsWithRanges) {
+      for (const comment of ranges.comments ?? []) {
+        nonOrphanedThreadIds.add(comment.op.t)
+      }
+    }
+
+    const commentIdsToDuplicate = Array.from(documentCommentIds).filter(id =>
+      nonOrphanedThreadIds.has(id)
+    )
+
+    const newRanges = { changes: ranges.changes, comments: [] }
+
+    if (commentIdsToDuplicate.length > 0) {
+      const { newThreads: newCommentIds } =
+        await ChatApiHandler.promises.duplicateCommentThreads(
+          projectId,
+          commentIdsToDuplicate
+        )
+
+      logger.debug({ mapping: newCommentIds }, 'replacing comment threads')
+
+      for (const comment of ranges.comments ?? []) {
+        if (Object.prototype.hasOwnProperty.call(newCommentIds, comment.op.t)) {
+          const result = newCommentIds[comment.op.t]
+          if (result.error) {
+            // We couldn't duplicate the thread, so we need to delete it from
+            // the resulting ranges.
+            continue
+          }
+          // We have a new id for this comment thread
+          comment.op.t = result.duplicateId
+          newRanges.comments.push(comment)
+        }
+      }
+    } else {
+      newRanges.comments = ranges.comments
+    }
+
     return await EditorController.promises.addDocWithRanges(
       projectId,
       parentFolderId,
       basename,
       importInfo.lines,
-      ranges,
+      newRanges,
       'revert',
       userId
     )

services/web/test/unit/src/History/RestoreManagerTests.js

@@ -24,7 +24,13 @@ describe('RestoreManager', function () {
         }),
         '../Project/ProjectLocator': (this.ProjectLocator = { promises: {} }),
         '../DocumentUpdater/DocumentUpdaterHandler':
-          (this.DocumentUpdaterHandler = { promises: {} }),
+          (this.DocumentUpdaterHandler = {
+            promises: { flushProjectToMongo: sinon.stub().resolves() },
+          }),
+        '../Docstore/DocstoreManager': (this.DocstoreManager = {
+          promises: {},
+        }),
+        '../Chat/ChatApiHandler': (this.ChatApiHandler = { promises: {} }),
       },
     })
     this.user_id = 'mock-user-id'
@@ -297,7 +303,27 @@ describe('RestoreManager', function () {
     describe("when reverting a file that doesn't current exist", function () {
       beforeEach(async function () {
         this.pathname = 'foo.tex'
+        this.comments = [
+          (this.comment = { op: { t: 'comment-1', p: 0, c: 'foo' } }),
+        ]
+        this.remappedComments = [{ op: { t: 'comment-2', p: 0, c: 'foo' } }]
         this.ProjectLocator.promises.findElementByPath = sinon.stub().rejects()
+        this.DocstoreManager.promises.getAllRanges = sinon.stub().resolves([
+          {
+            ranges: {
+              comments: [this.comment],
+            },
+          },
+        ])
+        this.ChatApiHandler.promises.duplicateCommentThreads = sinon
+          .stub()
+          .resolves({
+            newThreads: {
+              'comment-1': {
+                duplicateId: 'comment-2',
+              },
+            },
+          })
         this.tracked_changes = [
           {
             op: { pos: 4, i: 'bar' },
@@ -308,13 +334,12 @@ describe('RestoreManager', function () {
             metadata: { ts: '2024-01-01T00:00:00.000Z', user_id: 'user-2' },
           },
         ]
-        this.comments = [{ op: { t: 'comment-1', p: 0, c: 'foo' } }]
         this.FileSystemImportManager.promises.importFile = sinon
           .stub()
           .resolves({ type: 'doc', lines: ['foo', 'bar', 'baz'] })
         this.RestoreManager.promises._getRangesFromHistory = sinon
           .stub()
-          .resolves({ changes: this.tracked_changes, comment: this.comments })
+          .resolves({ changes: this.tracked_changes, comments: this.comments })
         this.EditorController.promises.addDocWithRanges = sinon
           .stub()
           .resolves(
@@ -336,7 +361,7 @@ describe('RestoreManager', function () {
           this.folder_id,
           'foo.tex',
           ['foo', 'bar', 'baz'],
-          { changes: this.tracked_changes, comment: this.comments }
+          { changes: this.tracked_changes, comments: this.remappedComments }
         )
       })