From 6c9d4fb52285496c339d29d7acfafb70fb882541 Mon Sep 17 00:00:00 2001 From: Eric Mc Sween <5454374+emcsween@users.noreply.github.com> Date: Wed, 5 Jun 2024 10:25:17 -0400 Subject: [PATCH] Merge pull request #18716 from overleaf/em-tracked-delete-undo Fix translation of tracked deletes GitOrigin-RevId: 4124db6953cbed46eea61f62118fc8e1ddfff4a0 --- .../document-updater/app/js/UpdateManager.js | 17 ++++++++++++----- .../js/UpdateManager/UpdateManagerTests.js | 18 +++++++++++++++--- .../project-history/app/js/UpdateCompressor.js | 2 +- .../project-history/app/js/UpdateTranslator.js | 4 ++-- .../UpdateCompressor/UpdateCompressorTests.js | 12 ++++++++++-- .../UpdateTranslator/UpdateTranslatorTests.js | 6 +++++- 6 files changed, 45 insertions(+), 14 deletions(-) diff --git a/services/document-updater/app/js/UpdateManager.js b/services/document-updater/app/js/UpdateManager.js index e7cc69c1f4..dff81f9fa4 100644 --- a/services/document-updater/app/js/UpdateManager.js +++ b/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 } } diff --git a/services/document-updater/test/unit/js/UpdateManager/UpdateManagerTests.js b/services/document-updater/test/unit/js/UpdateManager/UpdateManagerTests.js index 84ea05ba91..16ee0b12e1 100644 --- a/services/document-updater/test/unit/js/UpdateManager/UpdateManagerTests.js +++ b/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, diff --git a/services/project-history/app/js/UpdateCompressor.js b/services/project-history/app/js/UpdateCompressor.js index 8b7bca80b1..4c9b3108fa 100644 --- a/services/project-history/app/js/UpdateCompressor.js +++ b/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') { diff --git a/services/project-history/app/js/UpdateTranslator.js b/services/project-history/app/js/UpdateTranslator.js index ade0fce53f..dfd3f90ee8 100644 --- a/services/project-history/app/js/UpdateTranslator.js +++ b/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: { diff --git a/services/project-history/test/unit/js/UpdateCompressor/UpdateCompressorTests.js b/services/project-history/test/unit/js/UpdateCompressor/UpdateCompressorTests.js index 1338cfa4ee..95dbf3f676 100644 --- a/services/project-history/test/unit/js/UpdateCompressor/UpdateCompressorTests.js +++ b/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, }, diff --git a/services/project-history/test/unit/js/UpdateTranslator/UpdateTranslatorTests.js b/services/project-history/test/unit/js/UpdateTranslator/UpdateTranslatorTests.js index cbab154db2..f285fb4c24 100644 --- a/services/project-history/test/unit/js/UpdateTranslator/UpdateTranslatorTests.js +++ b/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: {