Merge pull request #23519 from overleaf/em-remove-ordered-rejections-flag

Remove orderedRejections flag on ops

GitOrigin-RevId: df464023e7e6c3b5899e6a424f423a374fd33df6

libraries/ranges-tracker/index.cjs

@@ -325,7 +325,7 @@ class RangesTracker {
           movedChanges.push(change)
         } else if (opStart === changeStart) {
           if (
-            !(op.orderedRejections && alreadyMerged) &&
+            !alreadyMerged &&
             undoing &&
             change.op.d.length >= op.i.length &&
             change.op.d.slice(0, op.i.length) === op.i
@@ -342,13 +342,11 @@ class RangesTracker {
             }
             alreadyMerged = true
 
-            if (op.orderedRejections) {
-              // Any tracked delete that came before this tracked delete
-              // rejection was moved after the incoming insert. Move them back
-              // so that they appear before the tracked delete rejection.
-              for (const trackedDelete of trackedDeletesAtOpPosition) {
-                trackedDelete.op.p -= opLength
-              }
+            // Any tracked delete that came before this tracked delete
+            // rejection was moved after the incoming insert. Move them back
+            // so that they appear before the tracked delete rejection.
+            for (const trackedDelete of trackedDeletesAtOpPosition) {
+              trackedDelete.op.p -= opLength
             }
           } else {
             // We're not rejecting that tracked delete. Move it after the
@@ -656,8 +654,6 @@ class RangesTracker {
   _addOp(op, metadata) {
     // Don't take a reference to the existing op since we'll modify this in place with future changes
     op = this._clone(op)
-    // TODO: Remove this when the orderedRejections transition is over
-    delete op.orderedRejections
     const change = {
       id: this.newId(),
       op,

libraries/ranges-tracker/test/unit/ranges-tracker-test.js

@@ -136,62 +136,31 @@ describe('RangesTracker', function () {
       this.rangesTracker = new RangesTracker(this.changes, this.comments)
     })
 
-    describe('with the orderedRejections flag', function () {
-      it('preserves the text order when rejecting changes', function () {
-        this.rangesTracker.applyOp(
-          { p: 50, i: 'this one', u: true, orderedRejections: true },
-          { user_id: 'user-id' }
-        )
-        expect(this.rangesTracker.changes).to.deep.equal([
-          { id: 'id1', op: { p: 33, d: 'before' } },
-          { id: 'id2', op: { p: 50, d: 'right before' } },
-          { id: 'id4', op: { p: 58, d: 'right after' } },
-          { id: 'id5', op: { p: 83, d: 'long after' } },
-        ])
-      })
-
-      it('moves all tracked deletes after the insert if not rejecting changes', function () {
-        this.rangesTracker.applyOp(
-          { p: 50, i: 'some other text', u: true, orderedRejections: true },
-          { user_id: 'user-id' }
-        )
-        expect(this.rangesTracker.changes).to.deep.equal([
-          { id: 'id1', op: { p: 33, d: 'before' } },
-          { id: 'id2', op: { p: 65, d: 'right before' } },
-          { id: 'id3', op: { p: 65, d: 'this one' } },
-          { id: 'id4', op: { p: 65, d: 'right after' } },
-          { id: 'id5', op: { p: 90, d: 'long after' } },
-        ])
-      })
+    it('preserves the text order when rejecting changes', function () {
+      this.rangesTracker.applyOp(
+        { p: 50, i: 'this one', u: true },
+        { user_id: 'user-id' }
+      )
+      expect(this.rangesTracker.changes).to.deep.equal([
+        { id: 'id1', op: { p: 33, d: 'before' } },
+        { id: 'id2', op: { p: 50, d: 'right before' } },
+        { id: 'id4', op: { p: 58, d: 'right after' } },
+        { id: 'id5', op: { p: 83, d: 'long after' } },
+      ])
     })
 
-    describe('without the orderedRejections flag', function () {
-      it('puts the insert before tracked deletes when rejecting changes', function () {
-        this.rangesTracker.applyOp(
-          { p: 50, i: 'this one', u: true },
-          { user_id: 'user-id' }
-        )
-        expect(this.rangesTracker.changes).to.deep.equal([
-          { id: 'id1', op: { p: 33, d: 'before' } },
-          { id: 'id2', op: { p: 58, d: 'right before' } },
-          { id: 'id4', op: { p: 58, d: 'right after' } },
-          { id: 'id5', op: { p: 83, d: 'long after' } },
-        ])
-      })
-
-      it('moves all tracked deletes after the insert if not rejecting changes', function () {
-        this.rangesTracker.applyOp(
-          { p: 50, i: 'some other text', u: true },
-          { user_id: 'user-id' }
-        )
-        expect(this.rangesTracker.changes).to.deep.equal([
-          { id: 'id1', op: { p: 33, d: 'before' } },
-          { id: 'id2', op: { p: 65, d: 'right before' } },
-          { id: 'id3', op: { p: 65, d: 'this one' } },
-          { id: 'id4', op: { p: 65, d: 'right after' } },
-          { id: 'id5', op: { p: 90, d: 'long after' } },
-        ])
-      })
+    it('moves all tracked deletes after the insert if not rejecting changes', function () {
+      this.rangesTracker.applyOp(
+        { p: 50, i: 'some other text', u: true, orderedRejections: true },
+        { user_id: 'user-id' }
+      )
+      expect(this.rangesTracker.changes).to.deep.equal([
+        { id: 'id1', op: { p: 33, d: 'before' } },
+        { id: 'id2', op: { p: 65, d: 'right before' } },
+        { id: 'id3', op: { p: 65, d: 'this one' } },
+        { id: 'id4', op: { p: 65, d: 'right after' } },
+        { id: 'id5', op: { p: 90, d: 'long after' } },
+      ])
     })
   })
 
@@ -207,31 +176,16 @@ describe('RangesTracker', function () {
       this.rangesTracker = new RangesTracker(this.changes, this.comments)
     })
 
-    describe('with the orderedRejections flag', function () {
-      it('removes only the first matching tracked delete', function () {
-        this.rangesTracker.applyOp(
-          { p: 10, i: 'giraffe', u: true, orderedRejections: true },
-          { user_id: 'user-id' }
-        )
-        expect(this.rangesTracker.changes).to.deep.equal([
-          { id: 'id1', op: { p: 10, d: 'cat' } },
-          { id: 'id3', op: { p: 17, d: 'cat' } },
-          { id: 'id4', op: { p: 17, d: 'giraffe' } },
-        ])
-      })
-    })
-
-    describe('without the orderedRejections flag', function () {
-      it('removes all matching tracked delete', function () {
-        this.rangesTracker.applyOp(
-          { p: 10, i: 'giraffe', u: true },
-          { user_id: 'user-id' }
-        )
-        expect(this.rangesTracker.changes).to.deep.equal([
-          { id: 'id1', op: { p: 17, d: 'cat' } },
-          { id: 'id3', op: { p: 17, d: 'cat' } },
-        ])
-      })
+    it('removes only the first matching tracked delete', function () {
+      this.rangesTracker.applyOp(
+        { p: 10, i: 'giraffe', u: true },
+        { user_id: 'user-id' }
+      )
+      expect(this.rangesTracker.changes).to.deep.equal([
+        { id: 'id1', op: { p: 10, d: 'cat' } },
+        { id: 'id3', op: { p: 17, d: 'cat' } },
+        { id: 'id4', op: { p: 17, d: 'giraffe' } },
+      ])
     })
   })
 
@@ -258,40 +212,20 @@ describe('RangesTracker', function () {
       this.rangesTracker = new RangesTracker(this.changes, this.comments)
     })
 
-    describe('with the orderedRejections flag', function () {
-      it('places a tracked insert at the same position before both the delete and the insert', function () {
-        this.rangesTracker.track_changes = true
-        this.rangesTracker.applyOp(
-          { p: 10, i: 'incoming', orderedRejections: true },
-          { user_id: 'user-id' }
-        )
-        expect(
-          this.rangesTracker.changes.map(change => change.op)
-        ).to.deep.equal([
-          { p: 5, d: 'before' },
-          { p: 10, i: 'incoming' },
-          { p: 18, d: 'delete' },
-          { p: 18, i: 'insert' },
-        ])
-      })
-    })
-
-    describe('without the orderedRejections flag', function () {
-      it('places a tracked insert at the same position before both the delete and the insert', function () {
-        this.rangesTracker.track_changes = true
-        this.rangesTracker.applyOp(
-          { p: 10, i: 'incoming' },
-          { user_id: 'user-id' }
-        )
-        expect(
-          this.rangesTracker.changes.map(change => change.op)
-        ).to.deep.equal([
+    it('places a tracked insert at the same position before both the delete and the insert', function () {
+      this.rangesTracker.track_changes = true
+      this.rangesTracker.applyOp(
+        { p: 10, i: 'incoming' },
+        { user_id: 'user-id' }
+      )
+      expect(this.rangesTracker.changes.map(change => change.op)).to.deep.equal(
+        [
           { p: 5, d: 'before' },
           { p: 10, i: 'incoming' },
           { p: 18, d: 'delete' },
           { p: 18, i: 'insert' },
-        ])
-      })
+        ]
+      )
     })
   })
 })

services/web/frontend/js/vendor/libs/sharejs.js

@@ -698,7 +698,6 @@ export const { Doc } = (() => {
         op.u = true;
         // TODO: This flag is temporary. It is only necessary while we change
         // the behaviour of tracked delete rejections in RangesTracker
-        op.orderedRejections = true;
         op.fixedRemoveChange = true;
       }
       op = [op];