From 655f9b50aeaa886557e9a84220380a5e45b294bf Mon Sep 17 00:00:00 2001 From: Mathias Jakobsen Date: Wed, 15 May 2024 10:24:25 +0100 Subject: [PATCH] Merge pull request #18309 from overleaf/mj-add-comment-resolved [overleaf-editor-core] Omit resolved: false for comments serialisation GitOrigin-RevId: 909f20efd8f3c3e50d40e09366951d317a4c31bf --- libraries/overleaf-editor-core/lib/comment.js | 7 +- .../lib/operation/add_comment_operation.js | 7 +- .../test/add_comment_operation.test.js | 6 -- .../test/comments_list.test.js | 76 ++++++++----------- .../test/edit_operation.test.js | 6 -- .../test/hash_file_data.test.js | 1 - .../test/lazy_string_file_data.test.js | 6 +- .../test/operation.test.js | 13 +--- .../test/string_file_data.test.js | 7 +- .../test/text_operation.test.js | 14 ++-- .../test/acceptance/js/SendingUpdatesTests.js | 1 - .../test/acceptance/js/SyncTests.js | 1 - .../UpdateTranslator/UpdateTranslatorTests.js | 3 - 13 files changed, 55 insertions(+), 93 deletions(-) diff --git a/libraries/overleaf-editor-core/lib/comment.js b/libraries/overleaf-editor-core/lib/comment.js index 6523382754..baf0814f51 100644 --- a/libraries/overleaf-editor-core/lib/comment.js +++ b/libraries/overleaf-editor-core/lib/comment.js @@ -149,11 +149,14 @@ class Comment { * @returns {CommentRawData} */ toRaw() { - return { + const raw = { id: this.id, - resolved: this.resolved, ranges: this.ranges.map(range => range.toRaw()), } + if (this.resolved) { + raw.resolved = true + } + return raw } /** diff --git a/libraries/overleaf-editor-core/lib/operation/add_comment_operation.js b/libraries/overleaf-editor-core/lib/operation/add_comment_operation.js index 700caba180..6e50d4d51e 100644 --- a/libraries/overleaf-editor-core/lib/operation/add_comment_operation.js +++ b/libraries/overleaf-editor-core/lib/operation/add_comment_operation.js @@ -38,11 +38,14 @@ class AddCommentOperation extends EditOperation { * @returns {RawAddCommentOperation} */ toJSON() { - return { + const raw = { commentId: this.commentId, ranges: this.ranges.map(range => range.toRaw()), - resolved: this.resolved, } + if (this.resolved) { + raw.resolved = true + } + return raw } /** diff --git a/libraries/overleaf-editor-core/test/add_comment_operation.test.js b/libraries/overleaf-editor-core/test/add_comment_operation.test.js index 9dc20085e7..72d43d6c5a 100644 --- a/libraries/overleaf-editor-core/test/add_comment_operation.test.js +++ b/libraries/overleaf-editor-core/test/add_comment_operation.test.js @@ -21,7 +21,6 @@ describe('AddCommentOperation', function () { const op = new AddCommentOperation('123', [new Range(0, 1)]) expect(op.toJSON()).to.eql({ commentId: '123', - resolved: false, ranges: [ { pos: 0, @@ -39,7 +38,6 @@ describe('AddCommentOperation', function () { { id: '123', ranges: [{ pos: 0, length: 1 }], - resolved: false, }, ]) }) @@ -54,10 +52,8 @@ describe('AddCommentOperation', function () { { id: '123', ranges: [{ pos: 0, length: 1 }], - resolved: false, }, ]) - const invertedOp = op.invert(initialFileData) invertedOp.apply(fileData) expect(fileData.getComments().toRaw()).to.eql([]) @@ -68,7 +64,6 @@ describe('AddCommentOperation', function () { { id: '123', ranges: [{ pos: 0, length: 1 }], - resolved: false, }, ] @@ -97,7 +92,6 @@ describe('AddCommentOperation', function () { { id: '123', ranges: [{ pos: 0, length: 1 }], - resolved: false, }, ] diff --git a/libraries/overleaf-editor-core/test/comments_list.test.js b/libraries/overleaf-editor-core/test/comments_list.test.js index c7c4765a7e..d687863c90 100644 --- a/libraries/overleaf-editor-core/test/comments_list.test.js +++ b/libraries/overleaf-editor-core/test/comments_list.test.js @@ -15,12 +15,11 @@ describe('commentList', function () { ]) expect(commentList.toRaw()).to.eql([ - { id: 'comm1', ranges: [{ pos: 5, length: 10 }], resolved: false }, - { id: 'comm2', ranges: [{ pos: 20, length: 5 }], resolved: false }, + { id: 'comm1', ranges: [{ pos: 5, length: 10 }] }, + { id: 'comm2', ranges: [{ pos: 20, length: 5 }] }, { id: 'comm3', ranges: [{ pos: 30, length: 15 }], - resolved: false, }, ]) }) @@ -41,7 +40,6 @@ describe('commentList', function () { length: 5, }, ], - resolved: false, }) }) @@ -54,17 +52,15 @@ describe('commentList', function () { commentList.add(new Comment('comm4', [new Range(40, 10)])) expect(commentList.toRaw()).to.eql([ - { id: 'comm1', ranges: [{ pos: 5, length: 10 }], resolved: false }, - { id: 'comm2', ranges: [{ pos: 20, length: 5 }], resolved: false }, + { id: 'comm1', ranges: [{ pos: 5, length: 10 }] }, + { id: 'comm2', ranges: [{ pos: 20, length: 5 }] }, { id: 'comm3', ranges: [{ pos: 30, length: 15 }], - resolved: false, }, { id: 'comm4', ranges: [{ pos: 40, length: 10 }], - resolved: false, }, ]) }) @@ -89,7 +85,6 @@ describe('commentList', function () { { id: 'comm3', ranges: [{ pos: 30, length: 15 }], - resolved: false, }, ]) }) @@ -103,8 +98,8 @@ describe('commentList', function () { commentList.delete('comm3') expect(commentList.toRaw()).to.eql([ - { id: 'comm1', ranges: [{ pos: 5, length: 10 }], resolved: false }, - { id: 'comm2', ranges: [{ pos: 20, length: 5 }], resolved: false }, + { id: 'comm1', ranges: [{ pos: 5, length: 10 }] }, + { id: 'comm2', ranges: [{ pos: 20, length: 5 }] }, ]) }) @@ -118,12 +113,11 @@ describe('commentList', function () { commentList.delete('comm5') expect(commentList.toRaw()).to.eql([ - { id: 'comm1', ranges: [{ pos: 5, length: 10 }], resolved: false }, - { id: 'comm2', ranges: [{ pos: 20, length: 5 }], resolved: false }, + { id: 'comm1', ranges: [{ pos: 5, length: 10 }] }, + { id: 'comm2', ranges: [{ pos: 20, length: 5 }] }, { id: 'comm3', ranges: [{ pos: 30, length: 15 }], - resolved: false, }, ]) }) @@ -149,8 +143,8 @@ describe('commentList', function () { commentList.applyInsert(new Range(15, 5), { commentIds: ['comm1'] }) expect(commentList.toRaw()).to.eql([ - { id: 'comm1', ranges: [{ pos: 5, length: 15 }], resolved: false }, - { id: 'comm2', ranges: [{ pos: 20, length: 10 }], resolved: false }, + { id: 'comm1', ranges: [{ pos: 5, length: 15 }] }, + { id: 'comm2', ranges: [{ pos: 20, length: 10 }] }, ]) }) @@ -168,8 +162,8 @@ describe('commentList', function () { commentList.applyInsert(new Range(15, 5), { commentIds: ['comm2'] }) expect(commentList.toRaw()).to.eql([ - { id: 'comm1', ranges: [{ pos: 5, length: 10 }], resolved: false }, - { id: 'comm2', ranges: [{ pos: 15, length: 15 }], resolved: false }, + { id: 'comm1', ranges: [{ pos: 5, length: 10 }] }, + { id: 'comm2', ranges: [{ pos: 15, length: 15 }] }, ]) }) }) @@ -188,8 +182,8 @@ describe('commentList', function () { commentList.applyDelete(new Range(10, 10)) // 10-19 expect(commentList.toRaw()).to.eql([ - { id: 'comm1', ranges: [{ pos: 5, length: 5 }], resolved: false }, - { id: 'comm2', ranges: [{ pos: 10, length: 5 }], resolved: false }, + { id: 'comm1', ranges: [{ pos: 5, length: 5 }] }, + { id: 'comm2', ranges: [{ pos: 10, length: 5 }] }, ]) }) @@ -212,9 +206,9 @@ describe('commentList', function () { commentList.applyInsert(new Range(7, 5), { commentIds: ['comm1'] }) expect(commentList.toRaw()).to.eql([ - { id: 'comm1', ranges: [{ pos: 5, length: 15 }], resolved: false }, - { id: 'comm2', ranges: [{ pos: 25, length: 5 }], resolved: false }, - { id: 'comm3', ranges: [{ pos: 35, length: 15 }], resolved: false }, + { id: 'comm1', ranges: [{ pos: 5, length: 15 }] }, + { id: 'comm2', ranges: [{ pos: 25, length: 5 }] }, + { id: 'comm3', ranges: [{ pos: 35, length: 15 }] }, ]) }) @@ -242,7 +236,6 @@ describe('commentList', function () { { pos: 5, length: 2 }, { pos: 12, length: 8 }, ], - resolved: false, }, { id: 'comm2', @@ -250,9 +243,8 @@ describe('commentList', function () { { pos: 7, length: 5 }, { pos: 25, length: 5 }, ], - resolved: false, }, - { id: 'comm3', ranges: [{ pos: 35, length: 15 }], resolved: false }, + { id: 'comm3', ranges: [{ pos: 35, length: 15 }] }, ]) }) @@ -279,7 +271,6 @@ describe('commentList', function () { { id: 'comm1', ranges: [{ pos: 5, length: 20 }], - resolved: false, }, { id: 'comm2', @@ -287,9 +278,8 @@ describe('commentList', function () { { pos: 7, length: 5 }, { pos: 25, length: 5 }, ], - resolved: false, }, - { id: 'comm3', ranges: [{ pos: 35, length: 15 }], resolved: false }, + { id: 'comm3', ranges: [{ pos: 35, length: 15 }] }, ]) }) @@ -311,9 +301,9 @@ describe('commentList', function () { commentList.applyInsert(new Range(16, 5)) expect(commentList.toRaw()).to.eql([ - { id: 'comm1', ranges: [{ pos: 5, length: 10 }], resolved: false }, - { id: 'comm2', ranges: [{ pos: 25, length: 5 }], resolved: false }, - { id: 'comm3', ranges: [{ pos: 35, length: 15 }], resolved: false }, + { id: 'comm1', ranges: [{ pos: 5, length: 10 }] }, + { id: 'comm2', ranges: [{ pos: 25, length: 5 }] }, + { id: 'comm3', ranges: [{ pos: 35, length: 15 }] }, ]) }) @@ -335,9 +325,9 @@ describe('commentList', function () { commentList.applyInsert(new Range(50, 5)) expect(commentList.toRaw()).to.eql([ - { id: 'comm1', ranges: [{ pos: 5, length: 10 }], resolved: false }, - { id: 'comm2', ranges: [{ pos: 20, length: 5 }], resolved: false }, - { id: 'comm3', ranges: [{ pos: 30, length: 15 }], resolved: false }, + { id: 'comm1', ranges: [{ pos: 5, length: 10 }] }, + { id: 'comm2', ranges: [{ pos: 20, length: 5 }] }, + { id: 'comm3', ranges: [{ pos: 30, length: 15 }] }, ]) }) @@ -359,9 +349,9 @@ describe('commentList', function () { commentList.applyDelete(new Range(0, 4)) expect(commentList.toRaw()).to.eql([ - { id: 'comm1', ranges: [{ pos: 1, length: 10 }], resolved: false }, - { id: 'comm2', ranges: [{ pos: 16, length: 5 }], resolved: false }, - { id: 'comm3', ranges: [{ pos: 26, length: 15 }], resolved: false }, + { id: 'comm1', ranges: [{ pos: 1, length: 10 }] }, + { id: 'comm2', ranges: [{ pos: 16, length: 5 }] }, + { id: 'comm3', ranges: [{ pos: 26, length: 15 }] }, ]) }) @@ -376,7 +366,7 @@ describe('commentList', function () { commentList.applyDelete(new Range(0, 6)) expect(commentList.toRaw()).to.eql([ - { id: 'comm1', ranges: [{ pos: 0, length: 9 }], resolved: false }, + { id: 'comm1', ranges: [{ pos: 0, length: 9 }] }, ]) }) @@ -389,7 +379,7 @@ describe('commentList', function () { ]) commentList.applyDelete(new Range(7, 10)) expect(commentList.toRaw()).to.eql([ - { id: 'comm1', ranges: [{ pos: 5, length: 2 }], resolved: false }, + { id: 'comm1', ranges: [{ pos: 5, length: 2 }] }, ]) }) @@ -402,7 +392,7 @@ describe('commentList', function () { ]) commentList.applyDelete(new Range(6, 2)) expect(commentList.toRaw()).to.eql([ - { id: 'comm1', ranges: [{ pos: 5, length: 8 }], resolved: false }, + { id: 'comm1', ranges: [{ pos: 5, length: 8 }] }, ]) }) }) @@ -428,13 +418,11 @@ describe('commentList', function () { { id: 'comm1', ranges: [{ pos: 5, length: 10 }], - resolved: false, }, - { id: 'comm2', ranges: [], resolved: false }, + { id: 'comm2', ranges: [] }, { id: 'comm3', ranges: [{ pos: 20, length: 15 }], - resolved: false, }, ]) }) diff --git a/libraries/overleaf-editor-core/test/edit_operation.test.js b/libraries/overleaf-editor-core/test/edit_operation.test.js index 161e8e9f73..56570f0863 100644 --- a/libraries/overleaf-editor-core/test/edit_operation.test.js +++ b/libraries/overleaf-editor-core/test/edit_operation.test.js @@ -104,7 +104,6 @@ describe('EditOperationTransformer', function () { expect(bPrime.toJSON()).to.eql({ commentId: 'comm1', ranges: [{ pos: 16, length: 3 }], - resolved: false, }) }) @@ -122,7 +121,6 @@ describe('EditOperationTransformer', function () { expect(aPrime.toJSON()).to.eql({ commentId: 'comm1', ranges: [{ pos: 16, length: 3 }], - resolved: false, }) }) @@ -140,7 +138,6 @@ describe('EditOperationTransformer', function () { expect(bPrime.toJSON()).to.eql({ commentId: 'comm1', ranges: [], - resolved: false, }) }) @@ -159,7 +156,6 @@ describe('EditOperationTransformer', function () { expect(bPrime.toJSON()).to.eql({ commentId: 'comm1', ranges: [{ pos: 8, length: 1 }], - resolved: false, }) }) @@ -178,7 +174,6 @@ describe('EditOperationTransformer', function () { expect(bPrime.toJSON()).to.eql({ commentId: 'comm1', ranges: [{ pos: 12, length: 1 }], - resolved: false, }) }) @@ -267,7 +262,6 @@ describe('EditOperationBuilder', function () { const raw = { commentId: 'comm1', ranges: [{ pos: 0, length: 1 }], - resolved: false, } const op = EditOperationBuilder.fromJSON(raw) expect(op).to.be.an.instanceof(AddCommentOperation) diff --git a/libraries/overleaf-editor-core/test/hash_file_data.test.js b/libraries/overleaf-editor-core/test/hash_file_data.test.js index be644b5e26..cf7ff56738 100644 --- a/libraries/overleaf-editor-core/test/hash_file_data.test.js +++ b/libraries/overleaf-editor-core/test/hash_file_data.test.js @@ -87,7 +87,6 @@ describe('HashFileData', function () { { id: 'comment-1', ranges: [{ pos: 1, length: 4 }], - resolved: false, }, ] const fileData = new HashFileData(this.fileHash, this.rangesHash) diff --git a/libraries/overleaf-editor-core/test/lazy_string_file_data.test.js b/libraries/overleaf-editor-core/test/lazy_string_file_data.test.js index 048f9a17f6..4c9f4aa497 100644 --- a/libraries/overleaf-editor-core/test/lazy_string_file_data.test.js +++ b/libraries/overleaf-editor-core/test/lazy_string_file_data.test.js @@ -26,9 +26,7 @@ describe('LazyStringFileData', function () { .withArgs(this.fileHash) .resolves('the quick brown fox') this.blobStore.getObject.withArgs(this.rangesHash).resolves({ - comments: [ - { id: 'foo', ranges: [{ pos: 0, length: 3 }], resolved: false }, - ], + comments: [{ id: 'foo', ranges: [{ pos: 0, length: 3 }] }], trackedChanges: [ { range: { pos: 4, length: 5 }, @@ -121,7 +119,7 @@ describe('LazyStringFileData', function () { expect(eagerString).to.be.instanceOf(EagerStringFileData) expect(eagerString.getContent()).to.equal('the quick brown fox') expect(eagerString.getComments().toRaw()).to.deep.equal([ - { id: 'foo', ranges: [{ pos: 0, length: 3 }], resolved: false }, + { id: 'foo', ranges: [{ pos: 0, length: 3 }] }, ]) expect(eagerString.trackedChanges.toRaw()).to.deep.equal([ { diff --git a/libraries/overleaf-editor-core/test/operation.test.js b/libraries/overleaf-editor-core/test/operation.test.js index 71c0af3dc3..c2edc66f6e 100644 --- a/libraries/overleaf-editor-core/test/operation.test.js +++ b/libraries/overleaf-editor-core/test/operation.test.js @@ -819,7 +819,6 @@ describe('Operation', function () { length: 1, }, ], - resolved: false, }, ], }, @@ -851,9 +850,7 @@ describe('Operation', function () { foo: { content: 'xyz', metadata: {}, - comments: [ - { id: '1', ranges: [{ pos: 3, length: 1 }], resolved: false }, - ], + comments: [{ id: '1', ranges: [{ pos: 3, length: 1 }] }], }, }) .expectSymmetry() @@ -886,9 +883,7 @@ describe('Operation', function () { foo: { content: 'xyz', metadata: {}, - comments: [ - { id: '1', ranges: [{ pos: 0, length: 4 }], resolved: false }, - ], + comments: [{ id: '1', ranges: [{ pos: 0, length: 4 }] }], }, }) .expectSymmetry() @@ -1070,9 +1065,7 @@ describe('Operation', function () { foo: { content: 'xyz', metadata: {}, - comments: [ - { id: '1', ranges: [{ pos: 0, length: 3 }], resolved: false }, - ], + comments: [{ id: '1', ranges: [{ pos: 0, length: 3 }] }], }, }) .expectSymmetry() diff --git a/libraries/overleaf-editor-core/test/string_file_data.test.js b/libraries/overleaf-editor-core/test/string_file_data.test.js index 802be1e39e..45a0337da1 100644 --- a/libraries/overleaf-editor-core/test/string_file_data.test.js +++ b/libraries/overleaf-editor-core/test/string_file_data.test.js @@ -64,8 +64,8 @@ describe('StringFileData', function () { ]) expect(fileData.getComments().toRaw()).to.eql([ - { id: 'comm1', ranges: [{ pos: 5, length: 10 }], resolved: false }, - { id: 'comm2', ranges: [{ pos: 20, length: 5 }], resolved: false }, + { id: 'comm1', ranges: [{ pos: 5, length: 10 }] }, + { id: 'comm2', ranges: [{ pos: 20, length: 5 }] }, ]) }) @@ -81,7 +81,6 @@ describe('StringFileData', function () { length: 10, }, ], - resolved: false, }, { id: 'comm2', @@ -97,7 +96,7 @@ describe('StringFileData', function () { }) expect(fileData.getComments().toRaw()).to.eql([ - { id: 'comm1', ranges: [{ pos: 5, length: 10 }], resolved: false }, + { id: 'comm1', ranges: [{ pos: 5, length: 10 }] }, { id: 'comm2', ranges: [{ pos: 20, length: 5 }], resolved: true }, ]) }) diff --git a/libraries/overleaf-editor-core/test/text_operation.test.js b/libraries/overleaf-editor-core/test/text_operation.test.js index abb3753ad5..b017f08d36 100644 --- a/libraries/overleaf-editor-core/test/text_operation.test.js +++ b/libraries/overleaf-editor-core/test/text_operation.test.js @@ -369,7 +369,7 @@ describe('TextOperation', function () { ) ).to.deep.equal({ content: 'foo baz', - comments: [{ id: 'comment1', ranges: [], resolved: false }], + comments: [{ id: 'comment1', ranges: [] }], }) }) @@ -473,8 +473,8 @@ describe('TextOperation', function () { ).to.deep.equal({ content: 'foo baaar bbbaz', comments: [ - { id: 'comment1', ranges: [{ pos: 4, length: 5 }], resolved: false }, - { id: 'comment2', ranges: [{ pos: 10, length: 5 }], resolved: false }, + { id: 'comment1', ranges: [{ pos: 4, length: 5 }] }, + { id: 'comment2', ranges: [{ pos: 10, length: 5 }] }, ], }) }) @@ -722,9 +722,7 @@ describe('TextOperation', function () { ) ).to.deep.equal({ content: 'foo qux baz', - comments: [ - { id: 'comment1', ranges: [{ pos: 4, length: 4 }], resolved: false }, - ], + comments: [{ id: 'comment1', ranges: [{ pos: 4, length: 4 }] }], }) }) @@ -751,9 +749,7 @@ describe('TextOperation', function () { ) ).to.deep.equal({ content: 'foo qux corge bar baz', - comments: [ - { id: 'comment1', ranges: [{ pos: 4, length: 13 }], resolved: false }, - ], + comments: [{ id: 'comment1', ranges: [{ pos: 4, length: 13 }] }], }) }) diff --git a/services/project-history/test/acceptance/js/SendingUpdatesTests.js b/services/project-history/test/acceptance/js/SendingUpdatesTests.js index 6d25eff6af..7ab1dde6e4 100644 --- a/services/project-history/test/acceptance/js/SendingUpdatesTests.js +++ b/services/project-history/test/acceptance/js/SendingUpdatesTests.js @@ -100,7 +100,6 @@ function olAddCommentOperation(doc, commentId, pos, length) { pathname: doc.pathname.replace(/^\//, ''), // Strip leading / commentId, ranges: [{ pos, length }], - resolved: false, } } diff --git a/services/project-history/test/acceptance/js/SyncTests.js b/services/project-history/test/acceptance/js/SyncTests.js index 3d6a4b8f45..e284674a9d 100644 --- a/services/project-history/test/acceptance/js/SyncTests.js +++ b/services/project-history/test/acceptance/js/SyncTests.js @@ -543,7 +543,6 @@ describe('Syncing with web and doc-updater', function () { pathname: 'main.tex', commentId, ranges: [{ pos: 1, length: 10 }], - resolved: false, }, ], origin: { kind: 'test-origin' }, diff --git a/services/project-history/test/unit/js/UpdateTranslator/UpdateTranslatorTests.js b/services/project-history/test/unit/js/UpdateTranslator/UpdateTranslatorTests.js index 447281ada1..74ae65b67f 100644 --- a/services/project-history/test/unit/js/UpdateTranslator/UpdateTranslatorTests.js +++ b/services/project-history/test/unit/js/UpdateTranslator/UpdateTranslatorTests.js @@ -814,13 +814,11 @@ describe('UpdateTranslator', function () { pathname: 'main.tex', commentId: 'comment-id-1', ranges: [{ pos: 5, length: 12 }], - resolved: false, }, { pathname: 'main.tex', commentId: 'comment-id-2', ranges: [{ pos: 7, length: 15 }], - resolved: false, }, { pathname: 'main.tex', @@ -999,7 +997,6 @@ describe('UpdateTranslator', function () { pathname: 'main.tex', commentId: 'comment-id', ranges: [{ pos: 11, length: 14 }], - resolved: false, }, ], v2Authors: [this.user_id],