From 7dffcbf6450d6edcc4d34c099523895825e7ef31 Mon Sep 17 00:00:00 2001
From: Eric Mc Sween <5454374+emcsween@users.noreply.github.com>
Date: Mon, 25 Nov 2024 07:49:28 -0500
Subject: [PATCH] Merge pull request #22081 from
 overleaf/em-revert-file-dangling-comments

Fix comment restoration when restoring a doc

GitOrigin-RevId: 2288ab991ab4ddbe38320bf4ff42cde80ce40e52
---
 .../src/Features/History/RestoreManager.js    |  9 ++++++-
 .../unit/src/History/RestoreManagerTests.js   | 26 ++++++++++++++-----
 2 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/services/web/app/src/Features/History/RestoreManager.js b/services/web/app/src/Features/History/RestoreManager.js
index 5b006c237e..f0bacd811e 100644
--- a/services/web/app/src/Features/History/RestoreManager.js
+++ b/services/web/app/src/Features/History/RestoreManager.js
@@ -187,8 +187,8 @@ const RestoreManager = {
           }
           // We have a new id for this comment thread
           comment.op.t = result.duplicateId
-          newRanges.comments.push(comment)
         }
+        newRanges.comments.push(comment)
       }
     } else {
       newRanges.comments = ranges.comments
@@ -200,6 +200,13 @@ const RestoreManager = {
         newRanges.comments.map(({ op: { t } }) => t)
       )
     await ChatManager.promises.injectUserInfoIntoThreads(newCommentThreadData)
+
+    // Only keep restored comment ranges that point to a valid thread.
+    // The chat service won't have generated thread data for deleted threads.
+    newRanges.comments = newRanges.comments.filter(
+      comment => newCommentThreadData[comment.op.t] != null
+    )
+
     logger.debug({ newCommentThreadData }, 'emitting new comment threads')
     EditorRealTimeController.emitToRoom(
       projectId,
diff --git a/services/web/test/unit/src/History/RestoreManagerTests.js b/services/web/test/unit/src/History/RestoreManagerTests.js
index a142f1409e..acc0a00202 100644
--- a/services/web/test/unit/src/History/RestoreManagerTests.js
+++ b/services/web/test/unit/src/History/RestoreManagerTests.js
@@ -259,14 +259,19 @@ describe('RestoreManager', function () {
       beforeEach(function () {
         this.pathname = 'foo.tex'
         this.comments = [
-          (this.comment = { op: { t: 'comment-1', p: 0, c: 'foo' } }),
+          { op: { t: 'comment-in-other-doc', p: 0, c: 'foo' } },
+          { op: { t: 'single-comment', p: 10, c: 'bar' } },
+          { op: { t: 'deleted-comment', p: 20, c: 'baz' } },
+        ]
+        this.remappedComments = [
+          { op: { t: 'duplicate-comment', p: 0, c: 'foo' } },
+          { op: { t: 'single-comment', p: 10, c: 'bar' } },
         ]
-        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],
+              comments: this.comments.slice(0, 1),
             },
           },
         ])
@@ -274,14 +279,14 @@ describe('RestoreManager', function () {
           .stub()
           .resolves({
             newThreads: {
-              'comment-1': {
-                duplicateId: 'comment-2',
+              'comment-in-other-doc': {
+                duplicateId: 'duplicate-comment',
               },
             },
           })
         this.ChatApiHandler.promises.generateThreadData = sinon.stub().resolves(
           (this.threadData = {
-            'comment-2': {
+            'single-comment': {
               messages: [
                 {
                   content: 'message-content',
@@ -290,6 +295,15 @@ describe('RestoreManager', function () {
                 },
               ],
             },
+            'duplicate-comment': {
+              messages: [
+                {
+                  content: 'another message',
+                  timestamp: '2024-01-01T00:00:00.000Z',
+                  user_id: 'user-1',
+                },
+              ],
+            },
           })
         )
         this.ChatManager.promises.injectUserInfoIntoThreads = sinon