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

@@ -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)

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"
   }

libraries/ranges-tracker/test/notests.js

@@ -1 +0,0 @@
-// There are no tests yet

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]])
+    })
+  })
+})

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"
       }