Merge pull request #18716 from overleaf/em-tracked-delete-undo

Fix translation of tracked deletes

GitOrigin-RevId: 4124db6953cbed46eea61f62118fc8e1ddfff4a0

services/document-updater/app/js/UpdateManager.js

@@ -346,11 +346,18 @@ const UpdateManager = {
         }
         if (isDelete(op)) {
           docLength -= op.d.length
-          if (!update.meta.tc || op.u) {
-            // This is either a regular delete or a tracked insert rejection.
-            // It will be translated to a delete in history.  Tracked deletes
-            // are translated into retains and don't change the history doc
-            // length.
+          if (update.meta.tc) {
+            // This is a tracked delete. It will be translated into a retain in
+            // history, except any enclosed tracked inserts, which will be
+            // translated into regular deletes.
+            for (const change of op.trackedChanges ?? []) {
+              if (change.type === 'insert') {
+                historyDocLength -= change.length
+              }
+            }
+          } else {
+            // This is a regular delete.  It will be translated to a delete in
+            // history.
             historyDocLength -= op.d.length
           }
         }

services/document-updater/test/unit/js/UpdateManager/UpdateManagerTests.js

@@ -540,7 +540,11 @@ describe('UpdateManager', function () {
           op: [
             { d: 'qux', p: 4 },
             { i: 'bazbaz', p: 14 },
-            { d: 'bong', p: 28, u: true },
+            {
+              d: 'bong',
+              p: 28,
+              trackedChanges: [{ type: 'insert', offset: 0, length: 4 }],
+            },
           ],
           meta: {
             tc: 'tracking-info',
@@ -589,7 +593,11 @@ describe('UpdateManager', function () {
           op: [
             { d: 'qux', p: 4 },
             { i: 'bazbaz', p: 14 },
-            { d: 'bong', p: 28, u: true },
+            {
+              d: 'bong',
+              p: 28,
+              trackedChanges: [{ type: 'insert', offset: 0, length: 4 }],
+            },
           ],
           meta: {
             pathname: this.pathname,
@@ -647,7 +655,11 @@ describe('UpdateManager', function () {
           op: [
             { d: 'qux', p: 4 },
             { i: 'bazbaz', p: 14 },
-            { d: 'bong', p: 28, u: true },
+            {
+              d: 'bong',
+              p: 28,
+              trackedChanges: [{ type: 'insert', offset: 0, length: 4 }],
+            },
           ],
           meta: {
             pathname: this.pathname,

services/project-history/app/js/UpdateCompressor.js

@@ -60,7 +60,7 @@ function adjustLengthByOp(length, op, opts = {}) {
       return length + op.i.length
     }
   } else if ('d' in op && op.d != null) {
-    if (opts.tracked && op.u == null) {
+    if (opts.tracked) {
       // Tracked delete: will be translated into a retain, except where it overlaps tracked inserts.
       for (const change of op.trackedChanges ?? []) {
         if (change.type === 'insert') {

services/project-history/app/js/UpdateTranslator.js

@@ -357,7 +357,7 @@ class OperationsBuilder {
       for (const change of changes) {
         if (change.offset > offset) {
           // Handle the portion before the tracked change
-          if (update.meta.tc != null && op.u == null) {
+          if (update.meta.tc != null) {
             // This is a tracked delete
             this.retain(change.offset - offset, {
               tracking: {
@@ -385,7 +385,7 @@ class OperationsBuilder {
       }
       if (offset < op.d.length) {
         // Handle the portion after the last tracked change
-        if (update.meta.tc != null && op.u == null) {
+        if (update.meta.tc != null) {
           // This is a tracked delete
           this.retain(op.d.length - offset, {
             tracking: {

services/project-history/test/unit/js/UpdateCompressor/UpdateCompressorTests.js

@@ -199,7 +199,11 @@ describe('UpdateCompressor', function () {
               { p: 22, d: 'apple' }, // doc_length doesn't change
               { p: 12, i: 'melon', u: true }, // doc_length += 5
               { p: 18, i: 'banana', u: true, trackedDeleteRejection: true }, // doc_length doesn't change
-              { p: 8, d: 'pineapple', u: true }, // doc_length -= 9
+              {
+                p: 8,
+                d: 'pineapple',
+                trackedChanges: [{ type: 'insert', offset: 0, length: 9 }],
+              }, // doc_length -= 9
               { p: 11, i: 'fruit salad' },
             ],
             meta: { ...meta, doc_length: 20, history_doc_length: 30 },
@@ -228,7 +232,11 @@ describe('UpdateCompressor', function () {
           v: 42,
         },
         {
-          op: { p: 8, d: 'pineapple', u: true },
+          op: {
+            p: 8,
+            d: 'pineapple',
+            trackedChanges: [{ type: 'insert', offset: 0, length: 9 }],
+          },
           meta: { ...meta, doc_length: 41 },
           v: 42,
         },

services/project-history/test/unit/js/UpdateTranslator/UpdateTranslatorTests.js

@@ -1069,7 +1069,11 @@ describe('UpdateTranslator', function () {
                 { i: 'inserted', p: 5 },
                 { d: 'deleted', p: 20 },
                 { i: 'rejected deletion', p: 30, trackedDeleteRejection: true },
-                { d: 'rejected insertion', p: 50, u: true },
+                {
+                  d: 'rejected insertion',
+                  p: 50,
+                  trackedChanges: [{ type: 'insert', offset: 0, length: 18 }],
+                },
               ],
               v: this.version,
               meta: {