From cd2fca8f2414a4dc305106c8211e9b4e4582431f Mon Sep 17 00:00:00 2001
From: Eric Mc Sween <5454374+emcsween@users.noreply.github.com>
Date: Thu, 6 Jun 2024 08:50:43 -0400
Subject: [PATCH] Merge pull request #18747 from
 overleaf/em-update-compressor-comment-ids

Propagate commentIds in UpdateCompressor

GitOrigin-RevId: 03315146c2dc816d02e69594df44d0d25f7952ca
---
 .../app/js/UpdateCompressor.js                | 35 +++++++++++-
 .../UpdateCompressor/UpdateCompressorTests.js | 56 ++++++++++++++++++-
 2 files changed, 88 insertions(+), 3 deletions(-)

diff --git a/services/project-history/app/js/UpdateCompressor.js b/services/project-history/app/js/UpdateCompressor.js
index 4c9b3108fa..53513dd1a4 100644
--- a/services/project-history/app/js/UpdateCompressor.js
+++ b/services/project-history/app/js/UpdateCompressor.js
@@ -300,7 +300,8 @@ function _concatTwoUpdates(firstUpdate, secondUpdate) {
     firstOp.i != null &&
     secondOp.i != null &&
     secondOpInsideFirstOp &&
-    combinedLengthUnderLimit
+    combinedLengthUnderLimit &&
+    insertOpsInsideSameComments(firstOp, secondOp)
   ) {
     return [
       mergeUpdatesWithOp(firstUpdate, secondUpdate, {
@@ -379,6 +380,10 @@ function _concatTwoUpdates(firstUpdate, secondUpdate) {
         if (firstOp.u && secondOp.u) {
           op.u = true
         }
+        if ('i' in op && secondOp.commentIds != null) {
+          // Make sure that commentIds metadata is propagated to inserts
+          op.commentIds = secondOp.commentIds
+        }
         return mergeUpdatesWithOp(firstUpdate, secondUpdate, op)
       }
     )
@@ -436,3 +441,31 @@ export function diffAsShareJsOps(before, after) {
   }
   return ops
 }
+
+/**
+ * Checks if two insert ops are inside the same comments
+ *
+ * @param {InsertOp} op1
+ * @param {InsertOp} op2
+ * @returns {boolean}
+ */
+function insertOpsInsideSameComments(op1, op2) {
+  const commentIds1 = op1.commentIds
+  const commentIds2 = op2.commentIds
+  if (commentIds1 == null && commentIds2 == null) {
+    // None are inside comments
+    return true
+  }
+
+  if (
+    commentIds1 != null &&
+    commentIds2 != null &&
+    commentIds1.every(id => commentIds2.includes(id)) &&
+    commentIds2.every(id => commentIds1.includes(id))
+  ) {
+    // Both are inside the same comments
+    return true
+  }
+
+  return false
+}
diff --git a/services/project-history/test/unit/js/UpdateCompressor/UpdateCompressorTests.js b/services/project-history/test/unit/js/UpdateCompressor/UpdateCompressorTests.js
index 95dbf3f676..8124e30e0b 100644
--- a/services/project-history/test/unit/js/UpdateCompressor/UpdateCompressorTests.js
+++ b/services/project-history/test/unit/js/UpdateCompressor/UpdateCompressorTests.js
@@ -817,6 +817,57 @@ describe('UpdateCompressor', function () {
           },
         ])
       })
+
+      it('should not merge inserts inside different comments', function () {
+        expect(
+          this.UpdateCompressor.compressUpdates([
+            {
+              op: { p: 3, i: 'foo', hpos: 13, commentIds: ['comment-id-1'] },
+              meta: { ts: this.ts1, user_id: this.user_id },
+              v: 42,
+            },
+            {
+              op: { p: 6, i: 'bar', hpos: 16 },
+              meta: { ts: this.ts2, user_id: this.user_id },
+              v: 43,
+            },
+          ])
+        ).to.deep.equal([
+          {
+            op: { p: 3, i: 'foo', hpos: 13, commentIds: ['comment-id-1'] },
+            meta: { ts: this.ts1, user_id: this.user_id },
+            v: 42,
+          },
+          {
+            op: { p: 6, i: 'bar', hpos: 16 },
+            meta: { ts: this.ts2, user_id: this.user_id },
+            v: 43,
+          },
+        ])
+      })
+
+      it('should propagate the commentIds property', function () {
+        expect(
+          this.UpdateCompressor.compressUpdates([
+            {
+              op: { p: 3, i: 'foo', hpos: 13, commentIds: ['comment-id-1'] },
+              meta: { ts: this.ts1, user_id: this.user_id },
+              v: 42,
+            },
+            {
+              op: { p: 6, i: 'bar', hpos: 16, commentIds: ['comment-id-1'] },
+              meta: { ts: this.ts2, user_id: this.user_id },
+              v: 43,
+            },
+          ])
+        ).to.deep.equal([
+          {
+            op: { p: 3, i: 'foobar', hpos: 13, commentIds: ['comment-id-1'] },
+            meta: { ts: this.ts1, user_id: this.user_id },
+            v: 43,
+          },
+        ])
+      })
     })
 
     describe('delete - delete', function () {
@@ -1174,7 +1225,7 @@ describe('UpdateCompressor', function () {
         ).to.deep.equal([])
       })
 
-      it('should preserver history metadata', function () {
+      it('should preserve history metadata', function () {
         expect(
           this.UpdateCompressor.compressUpdates([
             {
@@ -1191,6 +1242,7 @@ describe('UpdateCompressor', function () {
                 p: 3,
                 i: 'one 2 three four five six seven eight',
                 hpos: 13,
+                commentIds: ['comment-1'],
               },
               meta: { ts: this.ts2, user_id: this.user_id, doc_length: 100 },
               v: 43,
@@ -1203,7 +1255,7 @@ describe('UpdateCompressor', function () {
             v: 43,
           },
           {
-            op: { p: 7, i: '2', hpos: 17 },
+            op: { p: 7, i: '2', hpos: 17, commentIds: ['comment-1'] },
             meta: { ts: this.ts1, user_id: this.user_id, doc_length: 97 },
             v: 43,
           },