From 16d3d59bc122acca581be9923904f015084a1dfb Mon Sep 17 00:00:00 2001
From: Eric Mc Sween <5454374+emcsween@users.noreply.github.com>
Date: Tue, 8 Oct 2024 07:17:41 -0400
Subject: [PATCH] Merge pull request #20869 from
 overleaf/em-ranges-tracker-duplicate-ids

Fix handling of duplicate change ids in RangesTracker

GitOrigin-RevId: 1a75d1dc7e7931c459d2d004973f34d931100d33
---
 libraries/ranges-tracker/index.cjs            | 32 ++++---------------
 libraries/ranges-tracker/package.json         |  1 +
 libraries/ranges-tracker/test/notests.js      |  1 -
 .../test/unit/ranges-tracker-test.js          | 29 +++++++++++++++++
 package-lock.json                             |  2 ++
 5 files changed, 39 insertions(+), 26 deletions(-)
 delete mode 100644 libraries/ranges-tracker/test/notests.js
 create mode 100644 libraries/ranges-tracker/test/unit/ranges-tracker-test.js

diff --git a/libraries/ranges-tracker/index.cjs b/libraries/ranges-tracker/index.cjs
index 30860e6f56..8d3cb840c0 100644
--- a/libraries/ranges-tracker/index.cjs
+++ b/libraries/ranges-tracker/index.cjs
@@ -139,22 +139,9 @@ class RangesTracker {
     return change
   }
 
-  getChanges(changeIds) {
-    const changesResponse = []
-    const idsMap = {}
-
-    for (const changeId of changeIds) {
-      idsMap[changeId] = true
-    }
-
-    for (const change of this.changes) {
-      if (idsMap[change.id]) {
-        delete idsMap[change.id]
-        changesResponse.push(change)
-      }
-    }
-
-    return changesResponse
+  getChanges(ids) {
+    const idSet = new Set(ids)
+    return this.changes.filter(change => idSet.has(change.id))
   }
 
   removeChangeId(changeId) {
@@ -165,20 +152,15 @@ class RangesTracker {
     this._removeChange(change)
   }
 
-  removeChangeIds(changeToRemoveIds) {
-    if (changeToRemoveIds == null || changeToRemoveIds.length === 0) {
+  removeChangeIds(ids) {
+    if (ids == null || ids.length === 0) {
       return
     }
-    const removeChangeId = {}
-    for (const changeId of changeToRemoveIds) {
-      removeChangeId[changeId] = true
-    }
 
+    const idSet = new Set(ids)
     const remainingChanges = []
-
     for (const change of this.changes) {
-      if (removeChangeId[change.id]) {
-        delete removeChangeId[change.id]
+      if (idSet.has(change.id)) {
         this._markAsDirty(change, 'change', 'removed')
       } else {
         remainingChanges.push(change)
diff --git a/libraries/ranges-tracker/package.json b/libraries/ranges-tracker/package.json
index 2df71e76b4..9bf65ef84b 100644
--- a/libraries/ranges-tracker/package.json
+++ b/libraries/ranges-tracker/package.json
@@ -19,6 +19,7 @@
     "types:check": "tsc --noEmit"
   },
   "devDependencies": {
+    "chai": "^4.3.6",
     "mocha": "^10.2.0",
     "typescript": "^5.0.4"
   }
diff --git a/libraries/ranges-tracker/test/notests.js b/libraries/ranges-tracker/test/notests.js
deleted file mode 100644
index 62307e456d..0000000000
--- a/libraries/ranges-tracker/test/notests.js
+++ /dev/null
@@ -1 +0,0 @@
-// There are no tests yet
diff --git a/libraries/ranges-tracker/test/unit/ranges-tracker-test.js b/libraries/ranges-tracker/test/unit/ranges-tracker-test.js
new file mode 100644
index 0000000000..75d7024d09
--- /dev/null
+++ b/libraries/ranges-tracker/test/unit/ranges-tracker-test.js
@@ -0,0 +1,29 @@
+const { expect } = require('chai')
+const RangesTracker = require('../..')
+
+describe('RangesTracker', function () {
+  describe('with duplicate change ids', function () {
+    beforeEach(function () {
+      this.changes = [
+        { id: 'id1', op: { p: 1, i: 'hello' } },
+        { id: 'id2', op: { p: 10, i: 'world' } },
+        { id: 'id3', op: { p: 20, i: '!!!' } },
+        { id: 'id1', op: { p: 30, d: 'duplicate' } },
+      ]
+      this.rangesTracker = new RangesTracker(this.changes, this.comments)
+    })
+
+    it('getChanges() returns all changes with the given ids', function () {
+      expect(this.rangesTracker.getChanges(['id1', 'id2'])).to.deep.equal([
+        this.changes[0],
+        this.changes[1],
+        this.changes[3],
+      ])
+    })
+
+    it('removeChangeIds() removes all changes with the given ids', function () {
+      this.rangesTracker.removeChangeIds(['id1', 'id2'])
+      expect(this.rangesTracker.changes).to.deep.equal([this.changes[2]])
+    })
+  })
+})
diff --git a/package-lock.json b/package-lock.json
index 7bbc1387e2..0244c9d490 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -471,6 +471,7 @@
     "libraries/ranges-tracker": {
       "name": "@overleaf/ranges-tracker",
       "devDependencies": {
+        "chai": "^4.3.6",
         "mocha": "^10.2.0",
         "typescript": "^5.0.4"
       }
@@ -52023,6 +52024,7 @@
     "@overleaf/ranges-tracker": {
       "version": "file:libraries/ranges-tracker",
       "requires": {
+        "chai": "^4.3.6",
         "mocha": "^10.2.0",
         "typescript": "^5.0.4"
       }