diff --git a/services/project-history/app/js/SnapshotManager.js b/services/project-history/app/js/SnapshotManager.js index 4dce9673b2..fd94c64a7f 100644 --- a/services/project-history/app/js/SnapshotManager.js +++ b/services/project-history/app/js/SnapshotManager.js @@ -124,7 +124,6 @@ async function getRangesSnapshot(projectId, version, pathname) { ) const docUpdaterCompatibleComments = [] for (const comment of comments) { - trackedDeletionOffset = 0 let trackedDeletionIndex = 0 if (comment.ranges.length === 0) { // Translate detached comments into zero length comments at position 0 @@ -138,69 +137,60 @@ async function getRangesSnapshot(projectId, version, pathname) { }) continue } - for (const commentRange of comment.ranges) { - let commentRangeContent = '' - let offsetFromOverlappingRangeAtStart = 0 - while ( - trackedDeletionIndex < trackedDeletions.length && - trackedDeletions[trackedDeletionIndex].range.start < - commentRange.start && - trackedDeletions[trackedDeletionIndex].range.end <= commentRange.start - ) { - // Skip over tracked deletions that are before the current comment range - trackedDeletionOffset += - trackedDeletions[trackedDeletionIndex].range.length - trackedDeletionIndex++ + + // Consider a multiple range comment as a single comment that joins all its + // ranges + const commentStart = comment.ranges[0].start + const commentEnd = comment.ranges[comment.ranges.length - 1].end + + let commentContent = '' + // Docupdater position + let position = commentStart + while (trackedDeletions[trackedDeletionIndex]?.range.end <= commentStart) { + // Skip over tracked deletions that are before the current comment range + position -= trackedDeletions[trackedDeletionIndex].range.length + trackedDeletionIndex++ + } + + if (trackedDeletions[trackedDeletionIndex]?.range.start < commentStart) { + // There's overlap with a tracked deletion, move the position left and + // truncate the overlap + position -= + commentStart - trackedDeletions[trackedDeletionIndex].range.start + } + + // Cursor in the history content + let cursor = commentStart + while (cursor < commentEnd) { + const trackedDeletion = trackedDeletions[trackedDeletionIndex] + if (!trackedDeletion || trackedDeletion.range.start >= commentEnd) { + // We've run out of relevant tracked changes + commentContent += content.slice(cursor, commentEnd) + break + } + if (trackedDeletion.range.start > cursor) { + // There's a gap between the current cursor and the tracked deletion + commentContent += content.slice(cursor, trackedDeletion.range.start) } - if ( - trackedDeletions[trackedDeletionIndex]?.range.start < commentRange.start - ) { - // There's overlap with a tracked deletion, move the position left and - // truncate the overlap - offsetFromOverlappingRangeAtStart = - commentRange.start - - trackedDeletions[trackedDeletionIndex].range.start - } - - // The position of the comment in the document after tracked deletions - const position = - commentRange.start - - trackedDeletionOffset - - offsetFromOverlappingRangeAtStart - - let cursor = commentRange.start - while (cursor < commentRange.end) { - const trackedDeletion = trackedDeletions[trackedDeletionIndex] - if ( - !trackedDeletion || - trackedDeletion.range.start >= commentRange.end - ) { - // We've run out of relevant tracked changes - commentRangeContent += content.slice(cursor, commentRange.end) - break - } - if (trackedDeletion.range.start > cursor) { - // There's a gap between the current cursor and the tracked deletion - commentRangeContent += content.slice( - cursor, - trackedDeletion.range.start - ) - } + if (trackedDeletion.range.end <= commentEnd) { // Skip to the end of the tracked delete cursor = trackedDeletion.range.end trackedDeletionIndex++ - trackedDeletionOffset += trackedDeletion.range.length + } else { + // We're done with that comment + break } - docUpdaterCompatibleComments.push({ - op: { - p: position, - c: commentRangeContent, - t: comment.id, - resolved: comment.resolved, - }, - }) } + docUpdaterCompatibleComments.push({ + op: { + p: position, + c: commentContent, + t: comment.id, + resolved: comment.resolved, + }, + id: comment.id, + }) } return { diff --git a/services/project-history/test/unit/js/SnapshotManager/SnapshotManagerTests.js b/services/project-history/test/unit/js/SnapshotManager/SnapshotManagerTests.js index 7a10d33b2b..d6c52058c1 100644 --- a/services/project-history/test/unit/js/SnapshotManager/SnapshotManagerTests.js +++ b/services/project-history/test/unit/js/SnapshotManager/SnapshotManagerTests.js @@ -723,10 +723,10 @@ Four five six\ }, }, { - // 'er th' + // 'er the la' range: { pos: 28, - length: 5, + length: 9, }, tracking: { type: 'delete', @@ -754,10 +754,23 @@ Four five six\ pos: 26, length: 4, }, + // 'lazy' + { + pos: 35, + length: 4, + }, ], resolved: false, }, { id: 'comment-2', ranges: [], resolved: true }, + { + id: 'comment-3', + ranges: [ + // 'q' + { pos: 4, length: 1 }, + ], + resolved: true, + }, ], }) this.data = await this.SnapshotManager.promises.getRangesSnapshot( @@ -769,36 +782,29 @@ Four five six\ it('should move the comment to the start of the tracked delete and remove overlapping text', function () { expect(this.data.comments[0].op.p).to.eq(2) - expect(this.data.comments[0].op.c).to.eq('ck') - }) - - it('should remove overlapping text in middle of comment', function () { - expect(this.data.comments[1].op.p).to.eq(5) - expect(this.data.comments[1].op.c).to.eq('bown') - }) - - it('should remove overlapping text at end of comment', function () { - expect(this.data.comments[2].op.p).to.eq(20) - expect(this.data.comments[2].op.c).to.eq('ov') + expect(this.data.comments[0].op.c).to.eq('ck bown fox jumps ovzy') }) it('should put resolved status in op', function () { expect(this.data.comments[0].op.resolved).to.be.false - expect(this.data.comments[1].op.resolved).to.be.false - expect(this.data.comments[2].op.resolved).to.be.false - expect(this.data.comments[3].op.resolved).to.be.true + expect(this.data.comments[1].op.resolved).to.be.true + expect(this.data.comments[2].op.resolved).to.be.true }) it('should include thread id', function () { expect(this.data.comments[0].op.t).to.eq('comment-1') - expect(this.data.comments[1].op.t).to.eq('comment-1') - expect(this.data.comments[2].op.t).to.eq('comment-1') - expect(this.data.comments[3].op.t).to.eq('comment-2') + expect(this.data.comments[1].op.t).to.eq('comment-2') + expect(this.data.comments[2].op.t).to.eq('comment-3') }) - it('should translated detached comment to zero length op', function () { - expect(this.data.comments[3].op.p).to.eq(0) - expect(this.data.comments[3].op.c).to.eq('') + it('should translate detached comment to zero length op', function () { + expect(this.data.comments[1].op.p).to.eq(0) + expect(this.data.comments[1].op.c).to.eq('') + }) + + it('should position a comment entirely in a tracked delete next to the tracked delete', function () { + expect(this.data.comments[2].op.p).to.eq(2) + expect(this.data.comments[2].op.c).to.eq('') }) }) @@ -936,43 +942,21 @@ Four five six\ comments: [ { op: { - c: '', + c: 'brown fox jumps over the ', p: 4, t: 'comment-1', resolved: false, }, + id: 'comment-1', }, { op: { - c: 'brown', - p: 4, - t: 'comment-1', - resolved: false, - }, - }, - { - op: { - c: '', - p: 29, - t: 'comment-1', - resolved: false, - }, - }, - { - op: { - c: 'the', + c: 'the brown fox jumps over the', p: 0, t: 'comment-2', resolved: true, }, - }, - { - op: { - c: 'the', - p: 25, - t: 'comment-2', - resolved: true, - }, + id: 'comment-2', }, ], })