From 5cb27f69d77a76f8aff60956c607f274349b0b11 Mon Sep 17 00:00:00 2001 From: Domagoj Kriskovic Date: Mon, 26 Feb 2024 14:51:03 +0100 Subject: [PATCH] [overleaf-editor-core] SetCommentStateOperation (#17056) GitOrigin-RevId: 6efb6e3c9cb4b0cb9fdbc522bc57b1c1934a5062 --- libraries/overleaf-editor-core/index.js | 2 + libraries/overleaf-editor-core/lib/comment.js | 9 +- .../lib/file_data/comment_list.js | 9 +- .../lib/operation/add_comment_operation.js | 10 + .../lib/operation/edit_no_operation.js | 22 +- .../lib/operation/edit_operation_builder.js | 49 ++- .../operation/edit_operation_transformer.js | 164 +++++--- .../lib/operation/index.js | 6 +- .../operation/set_comment_state_operation.js | 112 ++++++ libraries/overleaf-editor-core/lib/types.ts | 3 + .../test/comments_list.test.js | 9 +- .../test/edit_operation.test.js | 99 ++++- .../test/operation.test.js | 361 +++++++++++++++++- 13 files changed, 766 insertions(+), 89 deletions(-) create mode 100644 libraries/overleaf-editor-core/lib/operation/set_comment_state_operation.js diff --git a/libraries/overleaf-editor-core/index.js b/libraries/overleaf-editor-core/index.js index 2e66720579..e5f363441b 100644 --- a/libraries/overleaf-editor-core/index.js +++ b/libraries/overleaf-editor-core/index.js @@ -14,6 +14,7 @@ const History = require('./lib/history') const Label = require('./lib/label') const AddFileOperation = require('./lib/operation/add_file_operation') const MoveFileOperation = require('./lib/operation/move_file_operation') +const SetCommentStateOperation = require('./lib/operation/set_comment_state_operation') const EditFileOperation = require('./lib/operation/edit_file_operation') const EditNoOperation = require('./lib/operation/edit_no_operation') const SetFileMetadataOperation = require('./lib/operation/set_file_metadata_operation') @@ -45,6 +46,7 @@ exports.History = History exports.Label = Label exports.AddFileOperation = AddFileOperation exports.MoveFileOperation = MoveFileOperation +exports.SetCommentStateOperation = SetCommentStateOperation exports.EditFileOperation = EditFileOperation exports.EditNoOperation = EditNoOperation exports.SetFileMetadataOperation = SetFileMetadataOperation diff --git a/libraries/overleaf-editor-core/lib/comment.js b/libraries/overleaf-editor-core/lib/comment.js index 59bbf1c8a3..3c6729075c 100644 --- a/libraries/overleaf-editor-core/lib/comment.js +++ b/libraries/overleaf-editor-core/lib/comment.js @@ -114,9 +114,10 @@ class Comment { /** * * @param {TextOperation} operation + * @param {string} commentId * @returns {Comment} */ - applyTextOperation(operation) { + applyTextOperation(operation, commentId) { /** @type {Comment} */ let comment = this let cursor = 0 @@ -124,7 +125,11 @@ class Comment { if (op instanceof RetainOp) { cursor += op.length } else if (op instanceof InsertOp) { - comment = comment.applyInsert(cursor, op.insertion.length) + comment = comment.applyInsert( + cursor, + op.insertion.length, + op.commentIds?.includes(commentId) + ) cursor += op.insertion.length } else if (op instanceof RemoveOp) { comment = comment.applyDelete(new Range(cursor, op.length)) diff --git a/libraries/overleaf-editor-core/lib/file_data/comment_list.js b/libraries/overleaf-editor-core/lib/file_data/comment_list.js index a925d0ece9..b7be65b5dc 100644 --- a/libraries/overleaf-editor-core/lib/file_data/comment_list.js +++ b/libraries/overleaf-editor-core/lib/file_data/comment_list.js @@ -39,14 +39,7 @@ class CommentList { * @param {Comment} newComment */ add(id, newComment) { - const existingComment = this.getComment(id) - if (existingComment) { - const resolved = existingComment.resolved && newComment.resolved - const mergedRanges = [...existingComment.ranges, ...newComment.ranges] - this.comments.set(id, new Comment(mergedRanges, resolved)) - } else { - this.comments.set(id, newComment) - } + this.comments.set(id, newComment) } /** 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 73b38e8c43..0e15aebc94 100644 --- a/libraries/overleaf-editor-core/lib/operation/add_comment_operation.js +++ b/libraries/overleaf-editor-core/lib/operation/add_comment_operation.js @@ -60,6 +60,8 @@ class AddCommentOperation extends EditOperation { (other instanceof AddCommentOperation && this.commentId === other.commentId) || (other instanceof core.DeleteCommentOperation && + this.commentId === other.commentId) || + (other instanceof core.SetCommentStateOperation && this.commentId === other.commentId) ) } @@ -84,6 +86,14 @@ class AddCommentOperation extends EditOperation { return other } + if ( + other instanceof core.SetCommentStateOperation && + other.commentId === this.commentId + ) { + const comment = new Comment(this.comment.ranges, other.resolved) + return new AddCommentOperation(this.commentId, comment) + } + throw new Error( `Trying to compose AddCommentOperation with ${other?.constructor?.name}.` ) diff --git a/libraries/overleaf-editor-core/lib/operation/edit_no_operation.js b/libraries/overleaf-editor-core/lib/operation/edit_no_operation.js index f2e99ccc84..bd14db868b 100644 --- a/libraries/overleaf-editor-core/lib/operation/edit_no_operation.js +++ b/libraries/overleaf-editor-core/lib/operation/edit_no_operation.js @@ -1,5 +1,25 @@ const EditOperation = require('./edit_operation') -class EditNoOperation extends EditOperation {} +class EditNoOperation extends EditOperation { + /** + * @inheritdoc + * @param {StringFileData} fileData + */ + apply(fileData) {} + + /** + * @inheritdoc + * @returns {object} + */ + toJSON() { + return { + noOp: true, + } + } + + static fromJSON() { + return new EditNoOperation() + } +} module.exports = EditNoOperation diff --git a/libraries/overleaf-editor-core/lib/operation/edit_operation_builder.js b/libraries/overleaf-editor-core/lib/operation/edit_operation_builder.js index 2ec278de4a..6f27746fc0 100644 --- a/libraries/overleaf-editor-core/lib/operation/edit_operation_builder.js +++ b/libraries/overleaf-editor-core/lib/operation/edit_operation_builder.js @@ -4,11 +4,14 @@ * @typedef {import('../types').RawTextOperation} RawTextOperation * @typedef {import('../types').RawAddCommentOperation} RawAddCommentOperation * @typedef {import('../types').RawDeleteCommentOperation} RawDeleteCommentOperation + * @typedef {import('../types').RawSetCommentStateOperation} RawSetCommentStateOperation * @typedef {import('../types').RawEditOperation} RawEditOperation */ const DeleteCommentOperation = require('./delete_comment_operation') const AddCommentOperation = require('./add_comment_operation') const TextOperation = require('./text_operation') +const SetCommentStateOperation = require('./set_comment_state_operation') +const EditNoOperation = require('./edit_no_operation') class EditOperationBuilder { /** @@ -26,32 +29,66 @@ class EditOperationBuilder { if (isRawDeleteCommentOperation(raw)) { return DeleteCommentOperation.fromJSON(raw) } + if (isRawSetCommentStateOperation(raw)) { + return SetCommentStateOperation.fromJSON(raw) + } + if (isRawEditNoOperation(raw)) { + return EditNoOperation.fromJSON() + } throw new Error('Unsupported operation in EditOperationBuilder.fromJSON') } } /** - * @param {*} raw + * @param {unknown} raw * @returns {raw is RawTextOperation} */ function isTextOperation(raw) { - return raw?.textOperation !== undefined + return raw !== null && typeof raw === 'object' && 'textOperation' in raw } /** - * @param {*} raw + * @param {unknown} raw * @returns {raw is RawAddCommentOperation} */ function isRawAddCommentOperation(raw) { - return raw?.commentId && Array.isArray(raw.ranges) + return ( + raw !== null && + typeof raw === 'object' && + 'commentId' in raw && + 'ranges' in raw && + Array.isArray(raw.ranges) + ) } /** - * @param {*} raw + * @param {unknown} raw * @returns {raw is RawDeleteCommentOperation} */ function isRawDeleteCommentOperation(raw) { - return raw?.deleteComment + return raw !== null && typeof raw === 'object' && 'deleteComment' in raw +} + +/** + * @param {unknown} raw + * @returns {raw is RawSetCommentStateOperation} + */ +function isRawSetCommentStateOperation(raw) { + return ( + raw !== null && + typeof raw === 'object' && + 'commentId' in raw && + 'resolved' in raw && + typeof raw.resolved === 'boolean' + ) +} + +/** + * @param {unknown} raw + * @returns {raw is RawEditNoOperation} + */ +function isRawEditNoOperation(raw) { + return raw !== null && typeof raw === 'object' && 'noOp' in raw } module.exports = EditOperationBuilder diff --git a/libraries/overleaf-editor-core/lib/operation/edit_operation_transformer.js b/libraries/overleaf-editor-core/lib/operation/edit_operation_transformer.js index 8d75402bc8..8bf1d187d9 100644 --- a/libraries/overleaf-editor-core/lib/operation/edit_operation_transformer.js +++ b/libraries/overleaf-editor-core/lib/operation/edit_operation_transformer.js @@ -1,5 +1,6 @@ // @ts-check const core = require('../..') +const Comment = require('../comment') const EditNoOperation = require('./edit_no_operation') const TextOperation = require('./text_operation') /** @typedef {import('./edit_operation')} EditOperation */ @@ -12,63 +13,99 @@ class EditOperationTransformer { * @returns {[EditOperation, EditOperation]} */ static transform(a, b) { - const { AddCommentOperation, DeleteCommentOperation } = core + const { + AddCommentOperation, + DeleteCommentOperation, + SetCommentStateOperation, + } = core if (a instanceof EditNoOperation || b instanceof EditNoOperation) { return [a, b] } - if (a instanceof TextOperation && b instanceof TextOperation) { - return TextOperation.transform(a, b) - } + const transformers = [ + createTransformer(TextOperation, TextOperation, TextOperation.transform), + createTransformer(TextOperation, DeleteCommentOperation, noConflict), + createTransformer(TextOperation, SetCommentStateOperation, noConflict), + createTransformer(TextOperation, AddCommentOperation, (a, b) => { + // apply the text operation to the comment + const movedComment = b.comment.applyTextOperation(a, b.commentId) + return [a, new AddCommentOperation(b.commentId, movedComment)] + }), + createTransformer(AddCommentOperation, AddCommentOperation, (a, b) => { + if (a.commentId === b.commentId) { + return [new EditNoOperation(), b] + } + return [a, b] + }), + createTransformer(AddCommentOperation, DeleteCommentOperation, (a, b) => { + if (a.commentId === b.commentId) { + // delete wins + return [new EditNoOperation(), b] + } + return [a, b] + }), + createTransformer( + AddCommentOperation, + SetCommentStateOperation, + (a, b) => { + if (a.commentId === b.commentId) { + const mergedComment = new Comment(a.comment.ranges, b.resolved) + const newA = new AddCommentOperation(a.commentId, mergedComment) + return [newA, b] + } + return [a, b] + } + ), + createTransformer( + DeleteCommentOperation, + DeleteCommentOperation, + (a, b) => { + if (a.commentId === b.commentId) { + // if both operations delete the same comment, we can ignore both + return [new EditNoOperation(), new EditNoOperation()] + } + return [a, b] + } + ), + createTransformer( + DeleteCommentOperation, + SetCommentStateOperation, + (a, b) => { + if (a.commentId === b.commentId) { + // delete wins + return [a, new EditNoOperation()] + } + return [a, b] + } + ), + createTransformer( + SetCommentStateOperation, + SetCommentStateOperation, + (a, b) => { + if (a.commentId !== b.commentId) { + return [a, b] + } - if (a instanceof TextOperation && b instanceof DeleteCommentOperation) { - return [a, b] - } + if (a.resolved === b.resolved) { + return [new EditNoOperation(), new EditNoOperation()] + } - if (a instanceof DeleteCommentOperation && b instanceof TextOperation) { - return [a, b] - } + const shouldResolve = a.resolved && b.resolved + if (a.resolved === shouldResolve) { + return [a, new EditNoOperation()] + } else { + return [new EditNoOperation(), b] + } + } + ), + ] - if (a instanceof AddCommentOperation && b instanceof TextOperation) { - const comment = a.comment.applyTextOperation(b) - return [new AddCommentOperation(a.commentId, comment), b] - } - - if (a instanceof TextOperation && b instanceof AddCommentOperation) { - const comment = b.comment.applyTextOperation(a) - return [a, new AddCommentOperation(b.commentId, comment)] - } - - if (a instanceof AddCommentOperation && b instanceof AddCommentOperation) { - return [a, b] - } - if ( - a instanceof DeleteCommentOperation && - b instanceof AddCommentOperation - ) { - if (a.commentId === b.commentId) { - return [a, new EditNoOperation()] + for (const transformer of transformers) { + const result = transformer(a, b) + if (result) { + return result } - return [a, b] - } - if ( - a instanceof AddCommentOperation && - b instanceof DeleteCommentOperation - ) { - if (a.commentId === b.commentId) { - return [new EditNoOperation(), b] - } - return [a, b] - } - if ( - a instanceof DeleteCommentOperation && - b instanceof DeleteCommentOperation - ) { - if (a.commentId === b.commentId) { - return [new EditNoOperation(), new EditNoOperation()] - } - return [a, b] } throw new Error( @@ -77,4 +114,35 @@ class EditOperationTransformer { } } +/** + * @template {EditOperation} X + * @template {EditOperation} Y + * @param {new(...args: any[]) => X} ClassA + * @param {new(...args: any[]) => Y} ClassB + * @param {(a: X, b: Y) => [EditOperation, EditOperation]} transformer + * @returns {(a: EditOperation, b: EditOperation) => [EditOperation, EditOperation] | false} + */ +function createTransformer(ClassA, ClassB, transformer) { + return (a, b) => { + if (a instanceof ClassA && b instanceof ClassB) { + return transformer(a, b) + } + if (b instanceof ClassA && a instanceof ClassB) { + const [bPrime, aPrime] = transformer(b, a) + return [aPrime, bPrime] + } + return false + } +} + +/** + * + * @param {EditOperation} a + * @param {EditOperation} b + * @returns {[EditOperation, EditOperation]} + */ +function noConflict(a, b) { + return [a, b] +} + module.exports = EditOperationTransformer diff --git a/libraries/overleaf-editor-core/lib/operation/index.js b/libraries/overleaf-editor-core/lib/operation/index.js index 2826c58a13..fedfdee250 100644 --- a/libraries/overleaf-editor-core/lib/operation/index.js +++ b/libraries/overleaf-editor-core/lib/operation/index.js @@ -32,7 +32,11 @@ class Operation { if ('file' in raw) { return AddFileOperation.fromRaw(raw) } - if ('textOperation' in raw || 'commentId' in raw) { + if ( + 'textOperation' in raw || + 'commentId' in raw || + 'deleteComment' in raw + ) { return EditFileOperation.fromRaw(raw) } if ('newPathname' in raw) { diff --git a/libraries/overleaf-editor-core/lib/operation/set_comment_state_operation.js b/libraries/overleaf-editor-core/lib/operation/set_comment_state_operation.js new file mode 100644 index 0000000000..9d9313add1 --- /dev/null +++ b/libraries/overleaf-editor-core/lib/operation/set_comment_state_operation.js @@ -0,0 +1,112 @@ +// @ts-check +const core = require('../../index') +const Comment = require('../comment') +const EditNoOperation = require('./edit_no_operation') +const EditOperation = require('./edit_operation') + +/** + * @typedef {import('./delete_comment_operation')} DeleteCommentOperation + * @typedef {import('../types').CommentRawData} CommentRawData + * @typedef {import('../types').RawSetCommentStateOperation} RawSetCommentStateOperation + * @typedef {import('../file_data/string_file_data')} StringFileData + */ + +/** + * @extends EditOperation + */ +class SetCommentStateOperation extends EditOperation { + /** + * @param {string} commentId + * @param {boolean} resolved + */ + constructor(commentId, resolved) { + super() + this.commentId = commentId + this.resolved = resolved + } + + /** + * + * @returns {RawSetCommentStateOperation} + */ + toJSON() { + return { + resolved: this.resolved, + commentId: this.commentId, + } + } + + /** + * @param {StringFileData} fileData + */ + apply(fileData) { + const comment = fileData.comments.getComment(this.commentId) + if (comment) { + const newComment = new Comment(comment.ranges, this.resolved) + fileData.comments.add(this.commentId, newComment) + } + } + + /** + * + * @returns {SetCommentStateOperation | EditNoOperation} + */ + invert(previousState) { + const comment = previousState.comments.getComment(this.commentId) + if (!comment) { + return new EditNoOperation() + } + + return new SetCommentStateOperation(this.commentId, comment.resolved) + } + + /** + * @inheritdoc + * @param {EditOperation} other + * @returns {boolean} + */ + canBeComposedWith(other) { + return ( + (other instanceof SetCommentStateOperation && + this.commentId === other.commentId) || + (other instanceof core.DeleteCommentOperation && + this.commentId === other.commentId) + ) + } + + /** + * @inheritdoc + * @param {EditOperation} other + * @returns {EditOperation} + */ + compose(other) { + if ( + other instanceof SetCommentStateOperation && + other.commentId === this.commentId + ) { + return other + } + + if ( + other instanceof core.DeleteCommentOperation && + other.commentId === this.commentId + ) { + return other + } + + throw new Error( + `Trying to compose SetCommentStateOperation with ${other?.constructor?.name}.` + ) + } + + /** + * @inheritdoc + * @param {RawSetCommentStateOperation} raw + * @returns {SetCommentStateOperation} + */ + static fromJSON(raw) { + return new SetCommentStateOperation(raw.commentId, raw.resolved) + } +} + +module.exports = SetCommentStateOperation diff --git a/libraries/overleaf-editor-core/lib/types.ts b/libraries/overleaf-editor-core/lib/types.ts index 11ba6baf7b..5e1b97988e 100644 --- a/libraries/overleaf-editor-core/lib/types.ts +++ b/libraries/overleaf-editor-core/lib/types.ts @@ -63,7 +63,10 @@ export type RawAddCommentOperation = CommentRawData & { commentId: string } export type RawDeleteCommentOperation = { deleteComment: string } +export type RawSetCommentStateOperation = { commentId: string; resolved: boolean } + export type RawEditOperation = | RawTextOperation | RawAddCommentOperation | RawDeleteCommentOperation + | RawSetCommentStateOperation diff --git a/libraries/overleaf-editor-core/test/comments_list.test.js b/libraries/overleaf-editor-core/test/comments_list.test.js index 8ea8a73176..6c29b79873 100644 --- a/libraries/overleaf-editor-core/test/comments_list.test.js +++ b/libraries/overleaf-editor-core/test/comments_list.test.js @@ -74,7 +74,7 @@ describe('commentList', function () { ]) }) - it('should add range to existing if a new comment has the same id', function () { + it('should overwrite existing comment if new one is added', function () { const commentList = new CommentList( new Map([ ['comm1', new Comment([new Range(5, 10)], false)], @@ -87,13 +87,10 @@ describe('commentList', function () { commentList.add('comm2', new Comment([new Range(40, 10)], true)) expect(commentList.getComments()).to.eql([ - { id: 'comm1', ranges: [{ pos: 5, length: 10 }], resolved: false }, + { id: 'comm1', ranges: [{ pos: 5, length: 10 }], resolved: true }, { id: 'comm2', - ranges: [ - { pos: 20, length: 5 }, - { pos: 40, length: 10 }, - ], + ranges: [{ pos: 40, length: 10 }], resolved: true, }, { diff --git a/libraries/overleaf-editor-core/test/edit_operation.test.js b/libraries/overleaf-editor-core/test/edit_operation.test.js index 82d5208ec7..86ed54403d 100644 --- a/libraries/overleaf-editor-core/test/edit_operation.test.js +++ b/libraries/overleaf-editor-core/test/edit_operation.test.js @@ -7,6 +7,7 @@ const randomTextOperation = require('./support/random_text_operation') const random = require('./support/random') const AddCommentOperation = require('../lib/operation/add_comment_operation') const DeleteCommentOperation = require('../lib/operation/delete_comment_operation') +const SetCommentStateOperation = require('../lib/operation/set_comment_state_operation') const Comment = require('../lib/comment') const Range = require('../lib/range') const EditNoOperation = require('../lib/operation/edit_no_operation') @@ -40,7 +41,7 @@ describe('EditOperationTransformer', function () { const a = new AddCommentOperation('comm1', new Comment([new Range(0, 1)])) const b = new AddCommentOperation('comm1', new Comment([new Range(2, 3)])) const [aPrime, bPrime] = EditOperationTransformer.transform(a, b) - expect(aPrime).to.be.an.instanceof(AddCommentOperation) + expect(aPrime).to.be.an.instanceof(EditNoOperation) expect(bPrime).to.be.an.instanceof(AddCommentOperation) }) @@ -192,6 +193,65 @@ describe('EditOperationTransformer', function () { expect(bPrime).to.be.an.instanceof(DeleteCommentOperation) expect(bPrime.toJSON()).to.eql(b.toJSON()) }) + + it('Transforms SetCommentStateOperation and TextOperation', function () { + const a = new TextOperation().retain(9).insert(' world') + const b = new SetCommentStateOperation('comm1', true) + + const [aPrime, bPrime] = EditOperationTransformer.transform(a, b) + expect(aPrime).to.be.an.instanceof(TextOperation) + expect(aPrime.toJSON()).to.eql(a.toJSON()) + expect(bPrime).to.be.an.instanceof(SetCommentStateOperation) + expect(bPrime.toJSON()).to.eql(b.toJSON()) + }) + + it('Transforms SetCommentStateOperation and AddCommentOperation', function () { + const a = new AddCommentOperation('comm1', new Comment([new Range(0, 1)])) + const b = new SetCommentStateOperation('comm1', true) + + const [aPrime, bPrime] = EditOperationTransformer.transform(a, b) + expect(aPrime).to.be.an.instanceof(AddCommentOperation) + expect(aPrime.toJSON()).to.deep.eql({ + commentId: 'comm1', + ranges: [{ pos: 0, length: 1 }], + resolved: true, + }) + expect(bPrime).to.be.an.instanceof(SetCommentStateOperation) + expect(bPrime.toJSON()).to.deep.eql(b.toJSON()) + }) + + it('Transforms SetCommentStateOperation and DeleteCommentOperation', function () { + const a = new DeleteCommentOperation('comm1') + const b = new SetCommentStateOperation('comm1', true) + + const [aPrime, bPrime] = EditOperationTransformer.transform(a, b) + expect(aPrime).to.be.an.instanceof(DeleteCommentOperation) + expect(aPrime.toJSON()).to.deep.eql(a.toJSON()) + expect(bPrime).to.be.an.instanceof(EditNoOperation) + }) + + it('Transforms SetCommentStateOperation and SetCommentStateOperation', function () { + const a = new SetCommentStateOperation('comm1', false) + const b = new SetCommentStateOperation('comm1', true) + + const [aPrime, bPrime] = EditOperationTransformer.transform(a, b) + expect(aPrime.toJSON()).to.deep.eql({ + commentId: 'comm1', + resolved: false, + }) + expect(bPrime).to.be.an.instanceof(EditNoOperation) + }) + + it('Transforms two SetCommentStateOperation with different commentId', function () { + const a = new SetCommentStateOperation('comm1', false) + const b = new SetCommentStateOperation('comm2', true) + + const [aPrime, bPrime] = EditOperationTransformer.transform(a, b) + expect(aPrime).to.be.an.instanceof(SetCommentStateOperation) + expect(aPrime.toJSON()).to.deep.eql(a.toJSON()) + expect(bPrime).to.be.an.instanceof(SetCommentStateOperation) + expect(bPrime.toJSON()).to.deep.eql(b.toJSON()) + }) }) describe('EditOperationBuilder', function () { @@ -204,6 +264,43 @@ describe('EditOperationBuilder', function () { expect(op.toJSON()).to.deep.equal(raw) }) + it('Constructs AddCommentOperation from JSON', function () { + const raw = { + commentId: 'comm1', + ranges: [{ pos: 0, length: 1 }], + resolved: false, + } + const op = EditOperationBuilder.fromJSON(raw) + expect(op).to.be.an.instanceof(AddCommentOperation) + expect(op.toJSON()).to.deep.equal(raw) + }) + + it('Constructs DeleteCommentOperation from JSON', function () { + const raw = { + deleteComment: 'comm1', + } + const op = EditOperationBuilder.fromJSON(raw) + expect(op).to.be.an.instanceof(DeleteCommentOperation) + expect(op.toJSON()).to.deep.equal(raw) + }) + + it('Constructs SetCommentStateOperation from JSON', function () { + const raw = { + commentId: 'comm1', + resolved: true, + } + const op = EditOperationBuilder.fromJSON(raw) + expect(op).to.be.an.instanceof(SetCommentStateOperation) + expect(op.toJSON()).to.deep.equal(raw) + }) + + it('Constructs EditNoOperation from JSON', function () { + const raw = { noOp: true } + const op = EditOperationBuilder.fromJSON(raw) + expect(op).to.be.an.instanceof(EditNoOperation) + expect(op.toJSON()).to.deep.equal(raw) + }) + it('Throws error for unsupported operation', function () { const raw = { unsupportedOperation: { diff --git a/libraries/overleaf-editor-core/test/operation.test.js b/libraries/overleaf-editor-core/test/operation.test.js index a4a823cf63..bf4c5b9922 100644 --- a/libraries/overleaf-editor-core/test/operation.test.js +++ b/libraries/overleaf-editor-core/test/operation.test.js @@ -4,6 +4,7 @@ const _ = require('lodash') const { expect } = require('chai') const ot = require('..') +const StringFileData = require('../lib/file_data/string_file_data') const File = ot.File const AddFileOperation = ot.AddFileOperation const MoveFileOperation = ot.MoveFileOperation @@ -11,6 +12,9 @@ const EditFileOperation = ot.EditFileOperation const NoOperation = ot.NoOperation const Operation = ot.Operation const TextOperation = ot.TextOperation +const AddCommentOperation = ot.AddCommentOperation +const DeleteCommentOperation = ot.DeleteCommentOperation +const SetCommentStateOperation = ot.SetCommentStateOperation const Snapshot = ot.Snapshot describe('Operation', function () { @@ -71,7 +75,12 @@ describe('Operation', function () { }, expectNoTransform() { - expect(this.operations).to.eql(this.primeOperations) + expect(this.operations).to.deep.eql(this.primeOperations) + return this + }, + + expectTransform() { + expect(this.operations).to.not.deep.eql(this.primeOperations) return this }, @@ -84,11 +93,12 @@ describe('Operation', function () { expect(this.snapshot.countFiles()).to.equal(_.size(files)) _.forOwn(files, (expectedFile, pathname) => { if (_.isString(expectedFile)) { - expectedFile = { content: expectedFile, metadata: {} } + expectedFile = { content: expectedFile, metadata: {}, comments: [] } } const file = this.snapshot.getFile(pathname) expect(file.getContent()).to.equal(expectedFile.content) expect(file.getMetadata()).to.eql(expectedFile.metadata) + expect(file.getComments()).to.deep.equal(expectedFile.comments) }) return this }, @@ -240,7 +250,10 @@ describe('Operation', function () { makeOneFileSnapshot() ) .expectNoTransform() - .expectFiles({ foo: { content: '', metadata: testMetadata }, bar: 'a' }) + .expectFiles({ + foo: { content: '', metadata: testMetadata, comments: [] }, + bar: 'a', + }) .expectSymmetry() }) @@ -251,7 +264,9 @@ describe('Operation', function () { Operation.setFileMetadata('foo', testMetadata), makeEmptySnapshot() ) - .expectFiles({ foo: { content: 'x', metadata: testMetadata } }) + .expectFiles({ + foo: { content: 'x', metadata: testMetadata, comments: [] }, + }) .expectSymmetry() }) @@ -574,7 +589,10 @@ describe('Operation', function () { makeTwoFileSnapshot() ) .expectNoTransform() - .expectFiles({ bar: { content: 'a', metadata: testMetadata }, baz: '' }) + .expectFiles({ + bar: { content: 'a', metadata: testMetadata, comments: [] }, + baz: '', + }) .expectSymmetry() }) @@ -585,7 +603,9 @@ describe('Operation', function () { Operation.setFileMetadata('foo', testMetadata), makeOneFileSnapshot() ) - .expectFiles({ bar: { content: '', metadata: testMetadata } }) + .expectFiles({ + bar: { content: '', metadata: testMetadata, comments: [] }, + }) .expectSymmetry() }) @@ -597,7 +617,7 @@ describe('Operation', function () { makeTwoFileSnapshot() ) // move wins - .expectFiles({ bar: { content: '', metadata: {} } }) + .expectFiles({ bar: { content: '', metadata: {}, comments: [] } }) .expectSymmetry() }) @@ -608,7 +628,9 @@ describe('Operation', function () { Operation.setFileMetadata('foo', testMetadata), makeOneFileSnapshot() ) - .expectFiles({ foo: { content: '', metadata: testMetadata } }) + .expectFiles({ + foo: { content: '', metadata: testMetadata, comments: [] }, + }) .expectSymmetry() }) @@ -674,8 +696,8 @@ describe('Operation', function () { ) .expectNoTransform() .expectFiles({ - foo: { content: 'x', metadata: {} }, - bar: { content: 'a', metadata: testMetadata }, + foo: { content: 'x', metadata: {}, comments: [] }, + bar: { content: 'a', metadata: testMetadata, comments: [] }, }) .expectSymmetry() }) @@ -688,7 +710,9 @@ describe('Operation', function () { makeOneFileSnapshot() ) .expectNoTransform() - .expectFiles({ foo: { content: 'x', metadata: testMetadata } }) + .expectFiles({ + foo: { content: 'x', metadata: testMetadata, comments: [] }, + }) .expectSymmetry() }) @@ -700,8 +724,8 @@ describe('Operation', function () { ) .expectNoTransform() .expectFiles({ - foo: { content: '', metadata: { baz: 1 } }, - bar: { content: 'a', metadata: { baz: 2 } }, + foo: { content: '', metadata: { baz: 1 }, comments: [] }, + bar: { content: 'a', metadata: { baz: 2 }, comments: [] }, }) .expectSymmetry() }) @@ -713,10 +737,10 @@ describe('Operation', function () { makeOneFileSnapshot() ) // second op wins - .expectFiles({ foo: { content: '', metadata: { baz: 2 } } }) + .expectFiles({ foo: { content: '', metadata: { baz: 2 }, comments: [] } }) .swap() // first op wins - .expectFiles({ foo: { content: '', metadata: { baz: 1 } } }) + .expectFiles({ foo: { content: '', metadata: { baz: 1 }, comments: [] } }) }) it('transforms SetFileMetadata-RemoveFile with no conflict', function () { @@ -727,7 +751,9 @@ describe('Operation', function () { makeTwoFileSnapshot() ) .expectNoTransform() - .expectFiles({ foo: { content: '', metadata: testMetadata } }) + .expectFiles({ + foo: { content: '', metadata: testMetadata, comments: [] }, + }) .expectSymmetry() }) @@ -747,4 +773,307 @@ describe('Operation', function () { foo: 'test', }) }) + + describe('EditFile sub operations', function () { + it('transforms AddCommentOperation-AddCommentOperation', function () { + runConcurrently( + Operation.editFile( + 'foo', + AddCommentOperation.fromJSON({ + commentId: '1', + ranges: [ + { + pos: 10, + length: 2, + }, + ], + }) + ), + Operation.editFile( + 'foo', + AddCommentOperation.fromJSON({ + commentId: '1', + ranges: [ + { + pos: 0, + length: 1, + }, + ], + }) + ), + makeOneFileSnapshot() + ) + .expectTransform() + .expectFiles({ + foo: { + content: '', + metadata: {}, + comments: [ + { + id: '1', + ranges: [ + { + pos: 0, + length: 1, + }, + ], + resolved: false, + }, + ], + }, + }) + }) + + it('transforms TextOperation-AddCommentOperation', function () { + runConcurrently( + Operation.editFile( + 'foo', + TextOperation.fromJSON({ textOperation: ['xyz'] }) + ), + Operation.editFile( + 'foo', + AddCommentOperation.fromJSON({ + commentId: '1', + ranges: [ + { + pos: 0, + length: 1, + }, + ], + }) + ), + makeOneFileSnapshot() + ) + .expectTransform() + .expectFiles({ + foo: { + content: 'xyz', + metadata: {}, + comments: [ + { id: '1', ranges: [{ pos: 3, length: 1 }], resolved: false }, + ], + }, + }) + .expectSymmetry() + }) + + it('transforms TextOperation-AddCommentOperation (insert with commentId)', function () { + runConcurrently( + Operation.editFile( + 'foo', + TextOperation.fromJSON({ + textOperation: [{ i: 'xyz', commentIds: ['1'] }], + }) + ), + Operation.editFile( + 'foo', + AddCommentOperation.fromJSON({ + commentId: '1', + ranges: [ + { + pos: 0, + length: 1, + }, + ], + }) + ), + makeOneFileSnapshot() + ) + .expectTransform() + .expectFiles({ + foo: { + content: 'xyz', + metadata: {}, + comments: [ + { id: '1', ranges: [{ pos: 0, length: 4 }], resolved: false }, + ], + }, + }) + .expectSymmetry() + }) + + it('transforms AddCommentOperation-SetCommentStateOperation', function () { + runConcurrently( + Operation.editFile( + 'foo', + AddCommentOperation.fromJSON({ + commentId: '1', + ranges: [{ pos: 1, length: 2 }], + }) + ), + Operation.editFile( + 'foo', + SetCommentStateOperation.fromJSON({ + commentId: '1', + resolved: true, + }) + ), + makeOneFileSnapshot() + ) + .expectTransform() + .expectFiles({ + foo: { + content: '', + metadata: {}, + comments: [ + { id: '1', ranges: [{ pos: 1, length: 2 }], resolved: true }, + ], + }, + }) + .expectSymmetry() + }) + + it('transforms AddCommentOperation-DeleteCommentOperation ', function () { + runConcurrently( + Operation.editFile( + 'foo', + AddCommentOperation.fromJSON({ + commentId: '1', + ranges: [{ pos: 1, length: 2 }], + }) + ), + Operation.editFile( + 'foo', + DeleteCommentOperation.fromJSON({ deleteComment: '1' }) + ), + makeOneFileSnapshot() + ) + .expectTransform() + .expectFiles({ + foo: { + content: '', + metadata: {}, + comments: [], + }, + }) + .expectSymmetry() + }) + + it('transforms DeleteCommentOperation-SetCommentStateOperation ', function () { + runConcurrently( + Operation.editFile( + 'foo', + DeleteCommentOperation.fromJSON({ deleteComment: '1' }) + ), + Operation.editFile( + 'foo', + SetCommentStateOperation.fromJSON({ + commentId: '1', + resolved: true, + }) + ), + makeOneFileSnapshot() + ) + .expectTransform() + .expectFiles({ + foo: { + content: '', + metadata: {}, + comments: [], + }, + }) + .expectSymmetry() + }) + + it('transforms DeleteCommentOperation-DeleteCommentOperation ', function () { + runConcurrently( + Operation.editFile( + 'foo', + DeleteCommentOperation.fromJSON({ deleteComment: '1' }) + ), + Operation.editFile( + 'foo', + DeleteCommentOperation.fromJSON({ deleteComment: '1' }) + ), + makeOneFileSnapshot() + ) + .expectTransform() + .expectFiles({ + foo: { + content: '', + metadata: {}, + comments: [], + }, + }) + .expectSymmetry() + }) + + it('transforms SetCommentStateOperation-SetCommentStateOperation to resolved comment', function () { + const snapshot = makeEmptySnapshot() + const file = new File( + new StringFileData('xyz', [ + { id: '1', ranges: [{ pos: 0, length: 3 }] }, + ]) + ) + snapshot.addFile('foo', file) + + runConcurrently( + Operation.editFile( + 'foo', + SetCommentStateOperation.fromJSON({ + commentId: '1', + resolved: true, + }) + ), + Operation.editFile( + 'foo', + SetCommentStateOperation.fromJSON({ + commentId: '1', + resolved: true, + }) + ), + snapshot + ) + .expectTransform() + .expectFiles({ + foo: { + content: 'xyz', + metadata: {}, + comments: [ + { id: '1', ranges: [{ pos: 0, length: 3 }], resolved: true }, + ], + }, + }) + .expectSymmetry() + }) + + it('transforms SetCommentStateOperation-SetCommentStateOperation to unresolved comment', function () { + const snapshot = makeEmptySnapshot() + const file = new File( + new StringFileData('xyz', [ + { id: '1', ranges: [{ pos: 0, length: 3 }] }, + ]) + ) + snapshot.addFile('foo', file) + + runConcurrently( + Operation.editFile( + 'foo', + SetCommentStateOperation.fromJSON({ + commentId: '1', + resolved: true, + }) + ), + Operation.editFile( + 'foo', + SetCommentStateOperation.fromJSON({ + commentId: '1', + resolved: false, + }) + ), + snapshot + ) + .expectTransform() + .expectFiles({ + foo: { + content: 'xyz', + metadata: {}, + comments: [ + { id: '1', ranges: [{ pos: 0, length: 3 }], resolved: false }, + ], + }, + }) + .expectSymmetry() + }) + }) })