Merge pull request #26279 from overleaf/em-compose-tracking-props

Merge tracked inserts and deletes during composition

GitOrigin-RevId: f8cfcf79aef7cb3e7acaecf7c3baa69d71a4efa9

libraries/overleaf-editor-core/lib/file_data/clear_tracking_props.js

@@ -1,7 +1,7 @@
 // @ts-check
 
 /**
- * @import { ClearTrackingPropsRawData } from '../types'
+ * @import { ClearTrackingPropsRawData, TrackingDirective } from '../types'
  */
 
 class ClearTrackingProps {
@@ -11,12 +11,27 @@ class ClearTrackingProps {
 
   /**
    * @param {any} other
-   * @returns {boolean}
+   * @returns {other is ClearTrackingProps}
    */
   equals(other) {
     return other instanceof ClearTrackingProps
   }
 
+  /**
+   * @param {TrackingDirective} other
+   * @returns {other is ClearTrackingProps}
+   */
+  canMergeWith(other) {
+    return other instanceof ClearTrackingProps
+  }
+
+  /**
+   * @param {TrackingDirective} other
+   */
+  mergeWith(other) {
+    return this
+  }
+
   /**
    * @returns {ClearTrackingPropsRawData}
    */

libraries/overleaf-editor-core/lib/file_data/tracking_props.js

@@ -62,6 +62,35 @@ class TrackingProps {
       this.ts.getTime() === other.ts.getTime()
     )
   }
+
+  /**
+   * Are these tracking props compatible with the other tracking props for merging
+   * ranges?
+   *
+   * @param {TrackingDirective} other
+   * @returns {other is TrackingProps}
+   */
+  canMergeWith(other) {
+    if (!(other instanceof TrackingProps)) {
+      return false
+    }
+    return this.type === other.type && this.userId === other.userId
+  }
+
+  /**
+   * Merge two tracking props
+   *
+   * Assumes that `canMerge(other)` returns true
+   *
+   * @param {TrackingDirective} other
+   */
+  mergeWith(other) {
+    if (!this.canMergeWith(other)) {
+      throw new Error('Cannot merge with incompatible tracking props')
+    }
+    const ts = this.ts <= other.ts ? this.ts : other.ts
+    return new TrackingProps(this.type, this.userId, ts)
+  }
 }
 
 module.exports = TrackingProps

libraries/overleaf-editor-core/lib/operation/scan_op.js

@@ -175,7 +175,7 @@ class InsertOp extends ScanOp {
       return false
     }
     if (this.tracking) {
-      if (!this.tracking.equals(other.tracking)) {
+      if (!other.tracking || !this.tracking.canMergeWith(other.tracking)) {
         return false
       }
     } else if (other.tracking) {
@@ -198,7 +198,10 @@ class InsertOp extends ScanOp {
       throw new Error('Cannot merge with incompatible operation')
     }
     this.insertion += other.insertion
-    // We already have the same tracking info and commentIds
+    if (this.tracking != null && other.tracking != null) {
+      this.tracking = this.tracking.mergeWith(other.tracking)
+    }
+    // We already have the same commentIds
   }
 
   /**
@@ -306,9 +309,13 @@ class RetainOp extends ScanOp {
       return false
     }
     if (this.tracking) {
-      return this.tracking.equals(other.tracking)
+      if (!other.tracking || !this.tracking.canMergeWith(other.tracking)) {
+        return false
+      }
+    } else if (other.tracking) {
+      return false
     }
-    return !other.tracking
+    return true
   }
 
   /**
@@ -319,6 +326,9 @@ class RetainOp extends ScanOp {
       throw new Error('Cannot merge with incompatible operation')
     }
     this.length += other.length
+    if (this.tracking != null && other.tracking != null) {
+      this.tracking = this.tracking.mergeWith(other.tracking)
+    }
   }
 
   /**

libraries/overleaf-editor-core/test/scan_op.test.js

@@ -107,7 +107,7 @@ describe('RetainOp', function () {
     expect(op1.equals(new RetainOp(3))).to.be.true
   })
 
-  it('cannot merge with another RetainOp if tracking info is different', function () {
+  it('cannot merge with another RetainOp if the tracking user is different', function () {
     const op1 = new RetainOp(
       4,
       new TrackingProps('insert', 'user1', new Date('2024-01-01T00:00:00.000Z'))
@@ -120,14 +120,14 @@ describe('RetainOp', function () {
     expect(() => op1.mergeWith(op2)).to.throw(Error)
   })
 
-  it('can merge with another RetainOp if tracking info is the same', function () {
+  it('can merge with another RetainOp if the tracking user is the same', function () {
     const op1 = new RetainOp(
       4,
       new TrackingProps('insert', 'user1', new Date('2024-01-01T00:00:00.000Z'))
     )
     const op2 = new RetainOp(
       4,
-      new TrackingProps('insert', 'user1', new Date('2024-01-01T00:00:00.000Z'))
+      new TrackingProps('insert', 'user1', new Date('2024-01-01T00:00:01.000Z'))
     )
     op1.mergeWith(op2)
     expect(
@@ -310,7 +310,7 @@ describe('InsertOp', function () {
     expect(() => op1.mergeWith(op2)).to.throw(Error)
   })
 
-  it('cannot merge with another InsertOp if tracking info is different', function () {
+  it('cannot merge with another InsertOp if tracking user is different', function () {
     const op1 = new InsertOp(
       'a',
       new TrackingProps('insert', 'user1', new Date('2024-01-01T00:00:00.000Z'))
@@ -323,7 +323,7 @@ describe('InsertOp', function () {
     expect(() => op1.mergeWith(op2)).to.throw(Error)
   })
 
-  it('can merge with another InsertOp if tracking and comment info is the same', function () {
+  it('can merge with another InsertOp if tracking user and comment info is the same', function () {
     const op1 = new InsertOp(
       'a',
       new TrackingProps(
@@ -338,7 +338,7 @@ describe('InsertOp', function () {
       new TrackingProps(
         'insert',
         'user1',
-        new Date('2024-01-01T00:00:00.000Z')
+        new Date('2024-01-01T00:00:01.000Z')
       ),
       ['1', '2']
     )