From c85f4ab5e5f1be0ab6ebcaebd3c6617b7a7a4e5f Mon Sep 17 00:00:00 2001
From: Eric Mc Sween <5454374+emcsween@users.noreply.github.com>
Date: Thu, 30 May 2024 07:54:17 -0400
Subject: [PATCH] Merge pull request #18491 from
 overleaf/em-filter-summarized-updates

Filter out comment ops from summarized updates

GitOrigin-RevId: 97a44821b6dc001cd1ea84115cf69c8d712e9946
---
 .../app/js/SummarizedUpdatesManager.js        | 31 +++++++++++++++----
 .../SummarizedUpdatesManagerTests.js          | 29 +++++++++++++++++
 2 files changed, 54 insertions(+), 6 deletions(-)

diff --git a/services/project-history/app/js/SummarizedUpdatesManager.js b/services/project-history/app/js/SummarizedUpdatesManager.js
index a539e4aac6..7bc0840eba 100644
--- a/services/project-history/app/js/SummarizedUpdatesManager.js
+++ b/services/project-history/app/js/SummarizedUpdatesManager.js
@@ -71,6 +71,7 @@ export function getSummarizedProjectUpdates(projectId, options, callback) {
         if (error) return callback(error)
         let chunksRequested = 0
         let summarizedUpdates = []
+        let toV = null
 
         const shouldRequestMoreUpdates = cb => {
           return cb(
@@ -93,11 +94,12 @@ export function getSummarizedProjectUpdates(projectId, options, callback) {
               // Updates are returned in time order, but we want to go back in time
               updateSet.reverse()
               updateSet = discardUnwantedUpdates(updateSet)
-              summarizedUpdates = _summarizeUpdates(
+              ;({ summarizedUpdates, toV } = _summarizeUpdates(
                 updateSet,
                 labelsByVersion,
-                summarizedUpdates
-              )
+                summarizedUpdates,
+                toV
+              ))
               nextVersionToRequest = startVersion
               chunksRequested += 1
               cb()
@@ -176,12 +178,24 @@ function _getProjectUpdates(projectId, historyId, version, callback) {
   })
 }
 
-function _summarizeUpdates(updates, labels, existingSummarizedUpdates) {
+function _summarizeUpdates(updates, labels, existingSummarizedUpdates, toV) {
   if (existingSummarizedUpdates == null) {
     existingSummarizedUpdates = []
   }
   const summarizedUpdates = existingSummarizedUpdates.slice()
   for (const update of updates) {
+    if (toV == null) {
+      // This is the first update we've seen. Initialize toV.
+      toV = update.v + 1
+    }
+
+    // Skip empty updates (only record their version). Empty updates are
+    // updates that only contain comment operations. We don't have a UI for
+    // these yet.
+    if (isUpdateEmpty(update)) {
+      continue
+    }
+
     // The client needs to know the exact version that a delete happened, in order
     // to be able to restore. So even when summarizing, retain the version that each
     // projectOp happened at.
@@ -199,7 +213,7 @@ function _summarizeUpdates(updates, labels, existingSummarizedUpdates) {
     } else {
       const newUpdate = {
         fromV: update.v,
-        toV: update.v + 1,
+        toV,
         meta: {
           users: update.meta.users,
           start_ts: update.meta.start_ts,
@@ -215,9 +229,10 @@ function _summarizeUpdates(updates, labels, existingSummarizedUpdates) {
 
       summarizedUpdates.push(newUpdate)
     }
+    toV = update.v
   }
 
-  return summarizedUpdates
+  return { summarizedUpdates, toV }
 }
 
 /**
@@ -318,3 +333,7 @@ function _mergeUpdate(update, summarizedUpdate) {
     summarizedUpdate.pathnames.add(pathname)
   }
 }
+
+function isUpdateEmpty(update) {
+  return update.project_ops.length === 0 && update.pathnames.length === 0
+}
diff --git a/services/project-history/test/unit/js/SummarizedUpdatesManager/SummarizedUpdatesManagerTests.js b/services/project-history/test/unit/js/SummarizedUpdatesManager/SummarizedUpdatesManagerTests.js
index c2c3d3e892..3cd08426b0 100644
--- a/services/project-history/test/unit/js/SummarizedUpdatesManager/SummarizedUpdatesManagerTests.js
+++ b/services/project-history/test/unit/js/SummarizedUpdatesManager/SummarizedUpdatesManagerTests.js
@@ -655,6 +655,35 @@ describe('SummarizedUpdatesManager', function () {
         )
       })
 
+      describe('empty updates', function () {
+        setupChunks([
+          [
+            makeUpdate({ startTs: 0, v: 1, pathnames: ['main.tex'] }),
+            makeUpdate({ startTs: 10, v: 2, pathnames: [] }),
+            makeUpdate({ startTs: 20, v: 3, pathnames: ['main.tex'] }),
+            makeUpdate({ startTs: 30, v: 4, pathnames: [] }),
+            makeUpdate({ startTs: 40, v: 5, pathnames: [] }),
+          ],
+          [
+            makeUpdate({ startTs: 50, v: 6, pathnames: [] }),
+            makeUpdate({ startTs: LATER, v: 7, pathnames: [] }),
+            makeUpdate({ startTs: LATER + 10, v: 8, pathnames: ['main.tex'] }),
+            makeUpdate({ startTs: LATER + 20, v: 9, pathnames: ['main.tex'] }),
+            makeUpdate({ startTs: LATER + 30, v: 10, pathnames: [] }),
+          ],
+        ])
+
+        expectSummaries('should skip empty updates', {}, [
+          makeSummary({
+            startTs: LATER + 10,
+            endTs: LATER + 30,
+            fromV: 8,
+            toV: 11,
+          }),
+          makeSummary({ startTs: 0, endTs: 30, fromV: 1, toV: 8 }),
+        ])
+      })
+
       describe('history resync updates', function () {
         setupChunks([
           [