Merge pull request #18747 from overleaf/em-update-compressor-comment-ids

Propagate commentIds in UpdateCompressor

GitOrigin-RevId: 03315146c2dc816d02e69594df44d0d25f7952ca
This commit is contained in:
Eric Mc Sween 2024-06-06 08:50:43 -04:00 committed by Copybot
parent a732130633
commit c070c036af
2 changed files with 88 additions and 3 deletions

View file

@ -300,7 +300,8 @@ function _concatTwoUpdates(firstUpdate, secondUpdate) {
firstOp.i != null &&
secondOp.i != null &&
secondOpInsideFirstOp &&
combinedLengthUnderLimit
combinedLengthUnderLimit &&
insertOpsInsideSameComments(firstOp, secondOp)
) {
return [
mergeUpdatesWithOp(firstUpdate, secondUpdate, {
@ -379,6 +380,10 @@ function _concatTwoUpdates(firstUpdate, secondUpdate) {
if (firstOp.u && secondOp.u) {
op.u = true
}
if ('i' in op && secondOp.commentIds != null) {
// Make sure that commentIds metadata is propagated to inserts
op.commentIds = secondOp.commentIds
}
return mergeUpdatesWithOp(firstUpdate, secondUpdate, op)
}
)
@ -436,3 +441,31 @@ export function diffAsShareJsOps(before, after) {
}
return ops
}
/**
* Checks if two insert ops are inside the same comments
*
* @param {InsertOp} op1
* @param {InsertOp} op2
* @returns {boolean}
*/
function insertOpsInsideSameComments(op1, op2) {
const commentIds1 = op1.commentIds
const commentIds2 = op2.commentIds
if (commentIds1 == null && commentIds2 == null) {
// None are inside comments
return true
}
if (
commentIds1 != null &&
commentIds2 != null &&
commentIds1.every(id => commentIds2.includes(id)) &&
commentIds2.every(id => commentIds1.includes(id))
) {
// Both are inside the same comments
return true
}
return false
}

View file

@ -817,6 +817,57 @@ describe('UpdateCompressor', function () {
},
])
})
it('should not merge inserts inside different comments', function () {
expect(
this.UpdateCompressor.compressUpdates([
{
op: { p: 3, i: 'foo', hpos: 13, commentIds: ['comment-id-1'] },
meta: { ts: this.ts1, user_id: this.user_id },
v: 42,
},
{
op: { p: 6, i: 'bar', hpos: 16 },
meta: { ts: this.ts2, user_id: this.user_id },
v: 43,
},
])
).to.deep.equal([
{
op: { p: 3, i: 'foo', hpos: 13, commentIds: ['comment-id-1'] },
meta: { ts: this.ts1, user_id: this.user_id },
v: 42,
},
{
op: { p: 6, i: 'bar', hpos: 16 },
meta: { ts: this.ts2, user_id: this.user_id },
v: 43,
},
])
})
it('should propagate the commentIds property', function () {
expect(
this.UpdateCompressor.compressUpdates([
{
op: { p: 3, i: 'foo', hpos: 13, commentIds: ['comment-id-1'] },
meta: { ts: this.ts1, user_id: this.user_id },
v: 42,
},
{
op: { p: 6, i: 'bar', hpos: 16, commentIds: ['comment-id-1'] },
meta: { ts: this.ts2, user_id: this.user_id },
v: 43,
},
])
).to.deep.equal([
{
op: { p: 3, i: 'foobar', hpos: 13, commentIds: ['comment-id-1'] },
meta: { ts: this.ts1, user_id: this.user_id },
v: 43,
},
])
})
})
describe('delete - delete', function () {
@ -1174,7 +1225,7 @@ describe('UpdateCompressor', function () {
).to.deep.equal([])
})
it('should preserver history metadata', function () {
it('should preserve history metadata', function () {
expect(
this.UpdateCompressor.compressUpdates([
{
@ -1191,6 +1242,7 @@ describe('UpdateCompressor', function () {
p: 3,
i: 'one 2 three four five six seven eight',
hpos: 13,
commentIds: ['comment-1'],
},
meta: { ts: this.ts2, user_id: this.user_id, doc_length: 100 },
v: 43,
@ -1203,7 +1255,7 @@ describe('UpdateCompressor', function () {
v: 43,
},
{
op: { p: 7, i: '2', hpos: 17 },
op: { p: 7, i: '2', hpos: 17, commentIds: ['comment-1'] },
meta: { ts: this.ts1, user_id: this.user_id, doc_length: 97 },
v: 43,
},