diff --git a/libraries/overleaf-editor-core/lib/file_data/clear_tracking_props.js b/libraries/overleaf-editor-core/lib/file_data/clear_tracking_props.js new file mode 100644 index 0000000000..ad735fbf5c --- /dev/null +++ b/libraries/overleaf-editor-core/lib/file_data/clear_tracking_props.js @@ -0,0 +1,28 @@ +// @ts-check + +/** + * @typedef {import('../types').ClearTrackingPropsRawData} ClearTrackingPropsRawData + */ + +class ClearTrackingProps { + constructor() { + this.type = 'none' + } + + /** + * @param {any} other + * @returns {boolean} + */ + equals(other) { + return other instanceof ClearTrackingProps + } + + /** + * @returns {ClearTrackingPropsRawData} + */ + toRaw() { + return { type: 'none' } + } +} + +module.exports = ClearTrackingProps diff --git a/libraries/overleaf-editor-core/lib/file_data/tracked_change_list.js b/libraries/overleaf-editor-core/lib/file_data/tracked_change_list.js index dd402690c4..fd4f016ed9 100644 --- a/libraries/overleaf-editor-core/lib/file_data/tracked_change_list.js +++ b/libraries/overleaf-editor-core/lib/file_data/tracked_change_list.js @@ -1,10 +1,11 @@ // @ts-check const Range = require('../range') const TrackedChange = require('./tracked_change') +const TrackingProps = require('../file_data/tracking_props') /** + * @typedef {import("../types").TrackingDirective} TrackingDirective * @typedef {import("../types").TrackedChangeRawData} TrackedChangeRawData - * @typedef {import("../file_data/tracking_props")} TrackingProps */ class TrackedChangeList { @@ -211,7 +212,7 @@ class TrackedChangeList { /** * @param {number} cursor * @param {number} length - * @param {{tracking?: TrackingProps}} opts + * @param {{tracking?: TrackingDirective}} opts */ applyRetain(cursor, length, opts = {}) { // If there's no tracking info, leave everything as-is @@ -263,7 +264,7 @@ class TrackedChangeList { newTrackedChanges.push(trackedChange) } } - if (opts.tracking?.type === 'delete' || opts.tracking?.type === 'insert') { + if (opts.tracking instanceof TrackingProps) { // This is a new tracked change const newTrackedChange = new TrackedChange(retainedRange, opts.tracking) newTrackedChanges.push(newTrackedChange) diff --git a/libraries/overleaf-editor-core/lib/file_data/tracking_props.js b/libraries/overleaf-editor-core/lib/file_data/tracking_props.js index a03246b521..5b2f3ae326 100644 --- a/libraries/overleaf-editor-core/lib/file_data/tracking_props.js +++ b/libraries/overleaf-editor-core/lib/file_data/tracking_props.js @@ -6,14 +6,14 @@ class TrackingProps { /** * - * @param {'insert' | 'delete' | 'none'} type + * @param {'insert' | 'delete'} type * @param {string} userId * @param {Date} ts */ constructor(type, userId, ts) { /** * @readonly - * @type {'insert' | 'delete' | 'none'} + * @type {'insert' | 'delete'} */ this.type = type /** diff --git a/libraries/overleaf-editor-core/lib/operation/scan_op.js b/libraries/overleaf-editor-core/lib/operation/scan_op.js index dbdb3cce84..fd27d55662 100644 --- a/libraries/overleaf-editor-core/lib/operation/scan_op.js +++ b/libraries/overleaf-editor-core/lib/operation/scan_op.js @@ -5,6 +5,7 @@ const { InvalidInsertionError, UnprocessableError, } = require('../errors') +const ClearTrackingProps = require('../file_data/clear_tracking_props') const TrackingProps = require('../file_data/tracking_props') /** @@ -13,6 +14,7 @@ const TrackingProps = require('../file_data/tracking_props') * @typedef {import('../types').RawInsertOp} RawInsertOp * @typedef {import('../types').RawRetainOp} RawRetainOp * @typedef {import('../types').RawRemoveOp} RawRemoveOp + * @typedef {import('../types').TrackingDirective} TrackingDirective */ class ScanOp { @@ -218,7 +220,7 @@ class InsertOp extends ScanOp { class RetainOp extends ScanOp { /** * @param {number} length - * @param {TrackingProps | undefined} tracking + * @param {TrackingDirective | undefined} tracking */ constructor(length, tracking = undefined) { super() @@ -227,7 +229,7 @@ class RetainOp extends ScanOp { } /** @type {number} */ this.length = length - /** @type {TrackingProps | undefined} */ + /** @type {TrackingDirective | undefined} */ this.tracking = tracking } @@ -263,7 +265,11 @@ class RetainOp extends ScanOp { throw new Error('retain operation must have a number property') } if (op.tracking) { - return new RetainOp(op.r, TrackingProps.fromRaw(op.tracking)) + const tracking = + op.tracking.type === 'none' + ? new ClearTrackingProps() + : TrackingProps.fromRaw(op.tracking) + return new RetainOp(op.r, tracking) } return new RetainOp(op.r) } diff --git a/libraries/overleaf-editor-core/lib/operation/text_operation.js b/libraries/overleaf-editor-core/lib/operation/text_operation.js index b2f13cb63f..25f297bf1f 100644 --- a/libraries/overleaf-editor-core/lib/operation/text_operation.js +++ b/libraries/overleaf-editor-core/lib/operation/text_operation.js @@ -26,12 +26,15 @@ const { TooLongError, } = require('../errors') const Range = require('../range') +const ClearTrackingProps = require('../file_data/clear_tracking_props') const TrackingProps = require('../file_data/tracking_props') + /** * @typedef {import('../file_data/string_file_data')} StringFileData * @typedef {import('../types').RawTextOperation} RawTextOperation * @typedef {import('../operation/scan_op').ScanOp} ScanOp * @typedef {import('../file_data/tracked_change_list')} TrackedChangeList + * @typedef {import('../types').TrackingDirective} TrackingDirective */ /** @@ -91,7 +94,7 @@ class TextOperation extends EditOperation { /** * Skip over a given number of characters. * @param {number | {r: number}} n - * @param {{tracking?: TrackingProps}} opts + * @param {{tracking?: TrackingDirective}} opts * @returns {TextOperation} */ retain(n, opts = {}) { @@ -376,11 +379,7 @@ class TextOperation extends EditOperation { let removeTrackingInfoIfNeeded if (op.tracking) { - removeTrackingInfoIfNeeded = new TrackingProps( - 'none', - op.tracking.userId, - op.tracking.ts - ) + removeTrackingInfoIfNeeded = new ClearTrackingProps() } for (const trackedChange of previousRanges) { @@ -575,10 +574,15 @@ class TextOperation extends EditOperation { } } else if (op1 instanceof InsertOp && op2 instanceof RetainOp) { const opts = { - // Prefer the latter tracking info - tracking: op2.tracking ?? op1.tracking, commentIds: op1.commentIds, } + if (op2.tracking instanceof TrackingProps) { + // Prefer the tracking info on the second operation + opts.tracking = op2.tracking + } else if (!(op2.tracking instanceof ClearTrackingProps)) { + // The second operation does not cancel the first operation's tracking + opts.tracking = op1.tracking + } if (op1.insertion.length > op2.length) { operation.insert(op1.insertion.slice(0, op2.length), opts) op1 = new InsertOp( @@ -692,9 +696,9 @@ class TextOperation extends EditOperation { // Simple case: retain/retain // If both have tracking info, we use the one from op1 - /** @type {TrackingProps | undefined} */ + /** @type {TrackingProps | ClearTrackingProps | undefined} */ let operation1primeTracking - /** @type {TrackingProps | undefined} */ + /** @type {TrackingProps | ClearTrackingProps | undefined} */ let operation2primeTracking if (op1.tracking) { operation1primeTracking = op1.tracking diff --git a/libraries/overleaf-editor-core/lib/types.ts b/libraries/overleaf-editor-core/lib/types.ts index 4c63928983..82a6accf5c 100644 --- a/libraries/overleaf-editor-core/lib/types.ts +++ b/libraries/overleaf-editor-core/lib/types.ts @@ -1,4 +1,6 @@ import Blob from './blob' +import TrackingProps from './file_data/tracking_props' +import ClearTrackingProps from './file_data/clear_tracking_props' export type BlobStore = { getBlob(hash: string): Promise @@ -32,11 +34,17 @@ export type TrackedChangeRawData = { } export type TrackingPropsRawData = { - type: 'insert' | 'delete' | 'none' + type: 'insert' | 'delete' userId: string ts: string } +export type ClearTrackingPropsRawData = { + type: 'none' +} + +export type TrackingDirective = TrackingProps | ClearTrackingProps + export type StringFileRawData = { content: string comments?: CommentRawData[] @@ -58,7 +66,7 @@ export type RawRetainOp = | { r: number commentIds?: string[] - tracking?: TrackingPropsRawData + tracking?: TrackingPropsRawData | ClearTrackingPropsRawData } | number diff --git a/libraries/overleaf-editor-core/test/support/random_text_operation.js b/libraries/overleaf-editor-core/test/support/random_text_operation.js index 4caf49bda3..a600cdc852 100644 --- a/libraries/overleaf-editor-core/test/support/random_text_operation.js +++ b/libraries/overleaf-editor-core/test/support/random_text_operation.js @@ -1,4 +1,5 @@ const TrackingProps = require('../../lib/file_data/tracking_props') +const ClearTrackingProps = require('../../lib/file_data/clear_tracking_props') const TextOperation = require('../../lib/operation/text_operation') const random = require('./random') @@ -19,7 +20,7 @@ function randomTextOperation(str, commentIds) { const trackedChange = Math.random() < 0.1 ? new TrackingProps( - random.element(['insert', 'delete', 'none']), + random.element(['insert', 'delete']), random.element(['user1', 'user2', 'user3']), new Date( random.element([ @@ -41,6 +42,8 @@ function randomTextOperation(str, commentIds) { }) } else if (r < 0.4) { operation.remove(l) + } else if (r < 0.5) { + operation.retain(l, { tracking: new ClearTrackingProps() }) } else { operation.retain(l, { tracking: trackedChange }) } diff --git a/libraries/overleaf-editor-core/test/text_operation.test.js b/libraries/overleaf-editor-core/test/text_operation.test.js index b017f08d36..fa9bc62dc3 100644 --- a/libraries/overleaf-editor-core/test/text_operation.test.js +++ b/libraries/overleaf-editor-core/test/text_operation.test.js @@ -15,6 +15,7 @@ const TextOperation = ot.TextOperation const StringFileData = require('../lib/file_data/string_file_data') const { RetainOp, InsertOp, RemoveOp } = require('../lib/operation/scan_op') const TrackingProps = require('../lib/file_data/tracking_props') +const ClearTrackingProps = require('../lib/file_data/clear_tracking_props') describe('TextOperation', function () { const numTrials = 500 @@ -496,11 +497,7 @@ describe('TextOperation', function () { new TextOperation() .retain(4) .retain(4, { - tracking: TrackingProps.fromRaw({ - ts: '2024-01-01T00:00:00.000Z', - type: 'none', - userId: 'user2', - }), + tracking: new ClearTrackingProps(), }) .retain(3) ) @@ -508,6 +505,42 @@ describe('TextOperation', function () { content: 'foo bar baz', }) }) + + it('it removes the tracking from an insert if operation 2 resolves it', function () { + expect( + compose( + new StringFileData('foo bar baz'), + new TextOperation() + .retain(4) + .insert('quux ', { + tracking: TrackingProps.fromRaw({ + ts: '2023-01-01T00:00:00.000Z', + type: 'insert', + userId: 'user1', + }), + }) + .retain(7), + new TextOperation() + .retain(6) + .retain(5, { + tracking: new ClearTrackingProps(), + }) + .retain(5) + ) + ).to.deep.equal({ + content: 'foo quux bar baz', + trackedChanges: [ + { + range: { pos: 4, length: 2 }, + tracking: { + ts: '2023-01-01T00:00:00.000Z', + type: 'insert', + userId: 'user1', + }, + }, + ], + }) + }) }) describe('transform', function () { diff --git a/libraries/overleaf-editor-core/test/tracked_change_list.test.js b/libraries/overleaf-editor-core/test/tracked_change_list.test.js index 04dcdf450f..b72caf2423 100644 --- a/libraries/overleaf-editor-core/test/tracked_change_list.test.js +++ b/libraries/overleaf-editor-core/test/tracked_change_list.test.js @@ -1,6 +1,7 @@ // @ts-check const TrackedChangeList = require('../lib/file_data/tracked_change_list') const TrackingProps = require('../lib/file_data/tracking_props') +const ClearTrackingProps = require('../lib/file_data/clear_tracking_props') const { expect } = require('chai') /** @typedef {import('../lib/types').TrackedChangeRawData} TrackedChangeRawData */ @@ -766,11 +767,7 @@ describe('TrackedChangeList', function () { }, ]) trackedChanges.applyRetain(0, 10, { - tracking: TrackingProps.fromRaw({ - type: 'none', - userId: 'user1', - ts: '2024-01-01T00:00:00.000Z', - }), + tracking: new ClearTrackingProps(), }) expect(trackedChanges.length).to.equal(0) expect(trackedChanges.toRaw()).to.deep.equal([]) @@ -788,11 +785,7 @@ describe('TrackedChangeList', function () { }, ]) trackedChanges.applyRetain(0, 10, { - tracking: TrackingProps.fromRaw({ - type: 'none', - userId: 'user2', - ts: '2024-01-01T00:00:00.000Z', - }), + tracking: new ClearTrackingProps(), }) expect(trackedChanges.length).to.equal(0) expect(trackedChanges.toRaw()).to.deep.equal([]) @@ -810,11 +803,7 @@ describe('TrackedChangeList', function () { }, ]) trackedChanges.applyRetain(0, 10, { - tracking: TrackingProps.fromRaw({ - type: 'none', - userId: 'user1', - ts: '2024-01-01T00:00:00.000Z', - }), + tracking: new ClearTrackingProps(), }) expect(trackedChanges.length).to.equal(0) expect(trackedChanges.toRaw()).to.deep.equal([]) @@ -832,11 +821,7 @@ describe('TrackedChangeList', function () { }, ]) trackedChanges.applyRetain(0, 10, { - tracking: TrackingProps.fromRaw({ - type: 'none', - userId: 'user2', - ts: '2024-01-01T00:00:00.000Z', - }), + tracking: new ClearTrackingProps(), }) expect(trackedChanges.length).to.equal(0) expect(trackedChanges.toRaw()).to.deep.equal([]) diff --git a/services/document-updater/app/js/RangesManager.js b/services/document-updater/app/js/RangesManager.js index 8bf8e5a6ad..4846850782 100644 --- a/services/document-updater/app/js/RangesManager.js +++ b/services/document-updater/app/js/RangesManager.js @@ -208,11 +208,7 @@ const RangesManager = { op = { p: change.op.p, r: change.op.i, - tracking: { - type: 'none', - userId: change.metadata.user_id, - ts: change.metadata.ts, - }, + tracking: { type: 'none' }, } if (unacceptedDeletes > 0) { op.hpos = op.p + unacceptedDeletes diff --git a/services/document-updater/app/js/types.ts b/services/document-updater/app/js/types.ts index eb18f030c7..a4b61a73a4 100644 --- a/services/document-updater/app/js/types.ts +++ b/services/document-updater/app/js/types.ts @@ -1,4 +1,7 @@ -import { TrackingPropsRawData } from 'overleaf-editor-core/lib/types' +import { + TrackingPropsRawData, + ClearTrackingPropsRawData, +} from 'overleaf-editor-core/lib/types' /** * An update coming from the editor @@ -97,7 +100,7 @@ export type HistoryInsertOp = InsertOp & { export type HistoryRetainOp = RetainOp & { hpos?: number - tracking?: TrackingPropsRawData + tracking?: TrackingPropsRawData | ClearTrackingPropsRawData } export type HistoryDeleteOp = DeleteOp & { diff --git a/services/document-updater/test/unit/js/RangesManager/RangesManagerTests.js b/services/document-updater/test/unit/js/RangesManager/RangesManagerTests.js index b932a43835..564c8cb679 100644 --- a/services/document-updater/test/unit/js/RangesManager/RangesManagerTests.js +++ b/services/document-updater/test/unit/js/RangesManager/RangesManagerTests.js @@ -682,11 +682,7 @@ describe('RangesManager', function () { { r: 'lorem', p: 0, - tracking: { - type: 'none', - userId: this.user_id, - ts: ranges.changes[0].metadata.ts, - }, + tracking: { type: 'none' }, }, ], }, @@ -702,11 +698,7 @@ describe('RangesManager', function () { { r: 'ipsum', p: 15, - tracking: { - type: 'none', - userId: this.user_id, - ts: ranges.changes[1].metadata.ts, - }, + tracking: { type: 'none' }, }, ], }, @@ -868,11 +860,7 @@ describe('RangesManager', function () { r: 'xxx ', p: 6, hpos: 11, - tracking: { - type: 'none', - userId: this.user_id, - ts: ranges.changes[1].metadata.ts, - }, + tracking: { type: 'none' }, }, ], }, diff --git a/services/project-history/app/js/UpdateTranslator.js b/services/project-history/app/js/UpdateTranslator.js index cc118505c2..c272394f95 100644 --- a/services/project-history/app/js/UpdateTranslator.js +++ b/services/project-history/app/js/UpdateTranslator.js @@ -309,11 +309,7 @@ class OperationsBuilder { if (isInsert(op)) { if (op.trackedDeleteRejection) { this.retain(op.i.length, { - tracking: { - type: 'none', - userId: update.meta.user_id, - ts: new Date(update.meta.ts).toISOString(), - }, + tracking: { type: 'none' }, }) } else { const opts = {} diff --git a/services/project-history/app/js/types.ts b/services/project-history/app/js/types.ts index ec93ad340e..5614657836 100644 --- a/services/project-history/app/js/types.ts +++ b/services/project-history/app/js/types.ts @@ -140,11 +140,13 @@ export type RawOrigin = { kind: string } -export type TrackingProps = { - type: 'insert' | 'delete' | 'none' - userId: string - ts: string -} +export type TrackingProps = + | { type: 'none' } + | { + type: 'insert' | 'delete' + userId: string + ts: string + } export type RawScanOp = | number diff --git a/services/project-history/test/unit/js/ChunkTranslator/ChunkTranslatorTests.js b/services/project-history/test/unit/js/ChunkTranslator/ChunkTranslatorTests.js index 28b232df55..92d012465c 100644 --- a/services/project-history/test/unit/js/ChunkTranslator/ChunkTranslatorTests.js +++ b/services/project-history/test/unit/js/ChunkTranslator/ChunkTranslatorTests.js @@ -2142,11 +2142,7 @@ describe('ChunkTranslator', function () { 4, // Hell|o [planet ]world, this is a test { r: 4, - tracking: { - type: 'none', - userId: this.author1.id, - ts: '2024-01-01T00:00:00.000Z', - }, + tracking: { type: 'none' }, }, // Hello pl|[anet ]world, this is a test { r: 3, @@ -2375,11 +2371,7 @@ describe('ChunkTranslator', function () { 2, // Heo |{TEST }planet world, [this] is a test { r: 5, - tracking: { - type: 'none', - userId: '65f1baeb3cee7c3563fbd1d1', - ts: '2024-03-27T13:20:23.253Z', - }, + tracking: { type: 'none' }, }, // Heo TEST| planet world, [this] is a test 14, // Heo TEST planet world, |[this] is a test -4, // Heo TEST planet world, | is a test @@ -2557,11 +2549,7 @@ describe('ChunkTranslator', function () { -2, // He|o planet world, [this] {TEST }is a test { r: 39, - tracking: { - type: 'none', - userId: this.author1.id, - ts: this.date.toISOString(), - }, + tracking: { type: 'none' }, }, ], // He|o planet world, this TEST is a test }, diff --git a/services/project-history/test/unit/js/UpdateCompressor/UpdateCompressorTests.js b/services/project-history/test/unit/js/UpdateCompressor/UpdateCompressorTests.js index 8a6da56a97..1338cfa4ee 100644 --- a/services/project-history/test/unit/js/UpdateCompressor/UpdateCompressorTests.js +++ b/services/project-history/test/unit/js/UpdateCompressor/UpdateCompressorTests.js @@ -106,7 +106,7 @@ describe('UpdateCompressor', function () { (this.op2 = { p: 9, r: 'baz', - tracking: { type: 'none', ts: this.ts1, userId: this.user_id }, + tracking: { type: 'none' }, }), (this.op3 = { p: 6, i: 'bar' }), ], diff --git a/services/project-history/test/unit/js/UpdateTranslator/UpdateTranslatorTests.js b/services/project-history/test/unit/js/UpdateTranslator/UpdateTranslatorTests.js index 74ae65b67f..ce8e2e7a8f 100644 --- a/services/project-history/test/unit/js/UpdateTranslator/UpdateTranslatorTests.js +++ b/services/project-history/test/unit/js/UpdateTranslator/UpdateTranslatorTests.js @@ -625,11 +625,7 @@ describe('UpdateTranslator', function () { { p: 3, r: 'lo', - tracking: { - type: 'none', - ts: this.timestamp, - userId: this.user_id, - }, + tracking: { type: 'none' }, }, ], v: this.version, @@ -658,11 +654,7 @@ describe('UpdateTranslator', function () { 3, { r: 2, - tracking: { - type: 'none', - ts: this.timestamp, - userId: this.user_id, - }, + tracking: { type: 'none' }, }, 15, ], @@ -1044,11 +1036,7 @@ describe('UpdateTranslator', function () { 3, { r: 3, - tracking: { - type: 'none', - userId: this.user_id, - ts: new Date(this.timestamp).toISOString(), - }, + tracking: { type: 'none' }, }, 14, ], @@ -1123,11 +1111,7 @@ describe('UpdateTranslator', function () { 3, { r: 17, - tracking: { - type: 'none', - userId: this.user_id, - ts: new Date(this.timestamp).toISOString(), - }, + tracking: { type: 'none' }, }, 3, -18,