From fc90db231c111487baf65c58e2a2edff12e0fb51 Mon Sep 17 00:00:00 2001 From: Domagoj Kriskovic Date: Tue, 23 Jan 2024 14:52:10 +0100 Subject: [PATCH] Add CommentList to StringFileData (#16568) * Add CommentList to StringFileData * added more types * use toRaw * using Map rather than array for comments * using Range class * Comment with ranges:Range[] * Revert "Comment with ranges:Range[]" This reverts commit 0783b1837562600637db03cc70c620129061c797. * Comment with ranges:Range[] * remove isDeleted * commentList.toRaw() * using toRaw * commentId to id * Revert "using toRaw" This reverts commit 0c04ca5836f3befd5ec027bad5bf722e8b27f36c. * fix merge * make comment map internal to CommentList * remove unused type * fix parameter name in StringFileData * import types more consistently * more consistent type def GitOrigin-RevId: 2be2225819d8e8ebcf90d08def280377205cb9ec --- libraries/overleaf-editor-core/lib/file.js | 10 ++ .../lib/file_data/comment.js | 36 +++++ .../lib/file_data/comment_list.js | 68 +++++++++ .../lib/file_data/index.js | 11 ++ .../lib/file_data/range.js | 23 +++ .../lib/file_data/string_file_data.js | 26 +++- libraries/overleaf-editor-core/lib/types.ts | 10 ++ .../test/comments_list.test.js | 138 ++++++++++++++++++ .../overleaf-editor-core/test/file.test.js | 5 + .../test/string_file_data.test.js | 66 +++++++++ 10 files changed, 390 insertions(+), 3 deletions(-) create mode 100644 libraries/overleaf-editor-core/lib/file_data/comment.js create mode 100644 libraries/overleaf-editor-core/lib/file_data/comment_list.js create mode 100644 libraries/overleaf-editor-core/lib/file_data/range.js create mode 100644 libraries/overleaf-editor-core/test/comments_list.test.js diff --git a/libraries/overleaf-editor-core/lib/file.js b/libraries/overleaf-editor-core/lib/file.js index 4dfd0aa78c..c3849728b4 100644 --- a/libraries/overleaf-editor-core/lib/file.js +++ b/libraries/overleaf-editor-core/lib/file.js @@ -12,6 +12,7 @@ const StringFileData = require('./file_data/string_file_data') * @typedef {import("./blob")} Blob * @typedef {import("./types").BlobStore} BlobStore * @typedef {import("./types").StringFileRawData} StringFileRawData + * @typedef {import("./types").CommentRawData} CommentRawData * @typedef {import("./operation/text_operation")} TextOperation */ @@ -183,6 +184,15 @@ class File { this.data.edit(textOperation) } + /** + * Get the comments for this file. + * + * @return {CommentRawData[]} + */ + getComments() { + return this.data.getComments() + } + /** * Clone a file. * diff --git a/libraries/overleaf-editor-core/lib/file_data/comment.js b/libraries/overleaf-editor-core/lib/file_data/comment.js new file mode 100644 index 0000000000..83ca6d9229 --- /dev/null +++ b/libraries/overleaf-editor-core/lib/file_data/comment.js @@ -0,0 +1,36 @@ +const Range = require('./range') + +/** + * @typedef {import("../types").CommentRawData} CommentRawData + */ + +class Comment { + /** + * @param {Range[]} ranges + * @param {boolean} [resolved] + */ + constructor(ranges, resolved = false) { + this.ranges = ranges + this.resolved = resolved + } + + toRaw() { + return { + resolved: this.resolved, + ranges: this.ranges.map(range => range.toRaw()), + } + } + + /** + * @param {CommentRawData} rawComment + * @returns {Comment} + */ + static fromRaw(rawComment) { + return new Comment( + rawComment.ranges.map(range => Range.fromRaw(range)), + rawComment.resolved + ) + } +} + +module.exports = Comment diff --git a/libraries/overleaf-editor-core/lib/file_data/comment_list.js b/libraries/overleaf-editor-core/lib/file_data/comment_list.js new file mode 100644 index 0000000000..b44971afeb --- /dev/null +++ b/libraries/overleaf-editor-core/lib/file_data/comment_list.js @@ -0,0 +1,68 @@ +const Comment = require('./comment') + +/** + * @typedef {import("../types").CommentRawData} CommentRawData + */ + +class CommentList { + /** + * @param {Map} comments + */ + constructor(comments) { + this.comments = comments + } + + /** + * @returns {CommentRawData[]} + */ + getComments() { + return Array.from(this.comments).map(([commentId, comment]) => { + return { + id: commentId, + ...comment.toRaw(), + } + }) + } + + /** + * @param {string} id + * @returns {Comment | undefined} + */ + getComment(id) { + return this.comments.get(id) + } + + /** + * @param {string} id + * @param {Comment} newComment + */ + add(id, newComment) { + const existing = this.getComment(id) + if (existing) { + // todo: merge/split ranges + existing.ranges = newComment.ranges + } else { + this.comments.set(id, newComment) + } + } + + /** + * @param {string} id + */ + delete(id) { + return this.comments.delete(id) + } + + /** + * @param {CommentRawData[]} rawComments + */ + static fromRaw(rawComments) { + const comments = new Map() + for (const rawComment of rawComments) { + comments.set(rawComment.id, Comment.fromRaw(rawComment)) + } + return new CommentList(comments) + } +} + +module.exports = CommentList diff --git a/libraries/overleaf-editor-core/lib/file_data/index.js b/libraries/overleaf-editor-core/lib/file_data/index.js index 2f8d7144dc..2abb7d4974 100644 --- a/libraries/overleaf-editor-core/lib/file_data/index.js +++ b/libraries/overleaf-editor-core/lib/file_data/index.js @@ -15,6 +15,7 @@ let StringFileData = null /** * @typedef {import("../types").BlobStore} BlobStore + * @typedef {import("../types").CommentRawData} CommentRawData */ /** @@ -147,6 +148,16 @@ class FileData { async store(blobStore) { throw new Error('store not implemented for ' + JSON.stringify(this)) } + + /** + * @see File#getComments + * @function + * @return {CommentRawData[]} + * @abstract + */ + getComments() { + throw new Error('getComments not implemented for ' + JSON.stringify(this)) + } } module.exports = FileData diff --git a/libraries/overleaf-editor-core/lib/file_data/range.js b/libraries/overleaf-editor-core/lib/file_data/range.js new file mode 100644 index 0000000000..11690e6d5e --- /dev/null +++ b/libraries/overleaf-editor-core/lib/file_data/range.js @@ -0,0 +1,23 @@ +class Range { + /** + * @param {number} pos + * @param {number} length + */ + constructor(pos, length) { + this.pos = pos + this.length = length + } + + toRaw() { + return { + pos: this.pos, + length: this.length + } + } + + static fromRaw(raw) { + return new Range(raw.pos, raw.length) + } +} + +module.exports = Range diff --git a/libraries/overleaf-editor-core/lib/file_data/string_file_data.js b/libraries/overleaf-editor-core/lib/file_data/string_file_data.js index 209fa27ce6..0ddf7e65ce 100644 --- a/libraries/overleaf-editor-core/lib/file_data/string_file_data.js +++ b/libraries/overleaf-editor-core/lib/file_data/string_file_data.js @@ -3,23 +3,31 @@ const assert = require('check-types').assert const FileData = require('./') +const CommentList = require('./comment_list') /** * @typedef {import("../types").StringFileRawData} StringFileRawData + * @typedef {import("../types").CommentRawData} CommentRawData */ class StringFileData extends FileData { /** * @param {string} content + * @param {CommentRawData[]} [rawComments] */ - constructor(content) { + constructor(content, rawComments = []) { super() assert.string(content) this.content = content + this.comments = CommentList.fromRaw(rawComments) } + /** + * @param {StringFileRawData} raw + * @returns {StringFileData} + */ static fromRaw(raw) { - return new StringFileData(raw.content) + return new StringFileData(raw.content, raw.comments || []) } /** @@ -27,7 +35,14 @@ class StringFileData extends FileData { * @returns {StringFileRawData} */ toRaw() { - return { content: this.content } + const raw = { content: this.content } + + const comments = this.getComments() + if (comments.length) { + raw.comments = comments + } + + return raw } /** @inheritdoc */ @@ -55,6 +70,11 @@ class StringFileData extends FileData { this.content = textOperation.apply(this.content) } + /** @inheritdoc */ + getComments() { + return this.comments.getComments() + } + /** @inheritdoc */ async toEager() { return this diff --git a/libraries/overleaf-editor-core/lib/types.ts b/libraries/overleaf-editor-core/lib/types.ts index ca5d083798..110f9b6eec 100644 --- a/libraries/overleaf-editor-core/lib/types.ts +++ b/libraries/overleaf-editor-core/lib/types.ts @@ -5,8 +5,18 @@ export type BlobStore = { putString(content: string): Promise } +export type CommentRawData = { + id: string + ranges: { + pos: number + length: number + }[] + resolved?: boolean +} + export type StringFileRawData = { content: string + comments?: CommentRawData[] } export type RawV2DocVersions = Record diff --git a/libraries/overleaf-editor-core/test/comments_list.test.js b/libraries/overleaf-editor-core/test/comments_list.test.js new file mode 100644 index 0000000000..15e764a38b --- /dev/null +++ b/libraries/overleaf-editor-core/test/comments_list.test.js @@ -0,0 +1,138 @@ +'use strict' + +const { expect } = require('chai') +const CommentList = require('../lib/file_data/comment_list') +const Comment = require('../lib/file_data/comment') +const Range = require('../lib/file_data/range') + +describe('commentList', function () { + it('checks if toRaw() returns a correct comment list', function () { + const commentList = new CommentList( + new Map([ + ['comm1', new Comment([new Range(5, 10)])], + ['comm2', new Comment([new Range(20, 5)])], + ['comm3', new Comment([new Range(30, 15)])], + ]) + ) + + expect(commentList.getComments()).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, + }, + ]) + }) + + it('should get a comment by id', function () { + const commentList = new CommentList( + new Map([ + ['comm1', new Comment([new Range(5, 10)])], + ['comm3', new Comment([new Range(30, 15)])], + ['comm2', new Comment([new Range(20, 5)])], + ]) + ) + + const comment = commentList.getComment('comm2') + expect(comment?.toRaw()).to.eql({ + ranges: [ + { + pos: 20, + length: 5, + }, + ], + resolved: false, + }) + }) + + it('should add new comment to the list', function () { + const commentList = new CommentList( + new Map([ + ['comm1', new Comment([new Range(5, 10)])], + ['comm2', new Comment([new Range(20, 5)])], + ['comm3', new Comment([new Range(30, 15)])], + ]) + ) + + commentList.add('comm4', new Comment([new Range(40, 10)])) + expect(commentList.getComments()).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: 'comm4', + ranges: [{ pos: 40, length: 10 }], + resolved: false, + }, + ]) + }) + + it('should override existing if a new comment has the same id', function () { + const commentList = new CommentList( + new Map([ + ['comm1', new Comment([new Range(5, 10)])], + ['comm2', new Comment([new Range(20, 5)])], + ['comm3', new Comment([new Range(30, 15)])], + ]) + ) + + commentList.add('comm2', new Comment([new Range(40, 10)])) + expect(commentList.getComments()).to.eql([ + { id: 'comm1', ranges: [{ pos: 5, length: 10 }], resolved: false }, + { + id: 'comm2', + ranges: [{ pos: 40, length: 10 }], + resolved: false, + }, + { + id: 'comm3', + ranges: [{ pos: 30, length: 15 }], + resolved: false, + }, + ]) + }) + + it('should delete a comment from the list', function () { + const commentList = new CommentList( + new Map([ + ['comm1', new Comment([new Range(5, 10)])], + ['comm2', new Comment([new Range(20, 5)])], + ['comm3', new Comment([new Range(30, 15)])], + ]) + ) + + commentList.delete('comm3') + expect(commentList.getComments()).to.eql([ + { id: 'comm1', ranges: [{ pos: 5, length: 10 }], resolved: false }, + { id: 'comm2', ranges: [{ pos: 20, length: 5 }], resolved: false }, + ]) + }) + + it('should not throw an error if comment id does not exist', function () { + const commentList = new CommentList( + new Map([ + ['comm1', new Comment([new Range(5, 10)])], + ['comm2', new Comment([new Range(20, 5)])], + ['comm3', new Comment([new Range(30, 15)])], + ]) + ) + + commentList.delete('comm5') + + expect(commentList.getComments()).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, + }, + ]) + }) +}) diff --git a/libraries/overleaf-editor-core/test/file.test.js b/libraries/overleaf-editor-core/test/file.test.js index 7cbac0462b..06b11c4f63 100644 --- a/libraries/overleaf-editor-core/test/file.test.js +++ b/libraries/overleaf-editor-core/test/file.test.js @@ -88,4 +88,9 @@ describe('File', function () { expect(clone.getStringLength()).to.equal(0) }) }) + + it('getComments() returns an empty comment list', function () { + const file = File.fromString('foo') + expect(file.getComments()).to.eql([]) + }) }) 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 566c838924..1c37954412 100644 --- a/libraries/overleaf-editor-core/test/string_file_data.test.js +++ b/libraries/overleaf-editor-core/test/string_file_data.test.js @@ -34,4 +34,70 @@ describe('StringFileData', function () { expect(fileData.getByteLength()).to.equal(longString.length - 1) expect(fileData.getStringLength()).to.equal(longString.length - 1) }) + + it('getComments() should return an empty array', function () { + const fileData = new StringFileData('test') + expect(fileData.getComments()).to.eql([]) + }) + + it('creates StringFileData with comments', function () { + const fileData = new StringFileData('test', [ + { + id: 'comm1', + ranges: [ + { + pos: 5, + length: 10, + }, + ], + }, + { + id: 'comm2', + ranges: [ + { + pos: 20, + length: 5, + }, + ], + }, + ]) + + expect(fileData.getComments()).to.eql([ + { id: 'comm1', ranges: [{ pos: 5, length: 10 }], resolved: false }, + { id: 'comm2', ranges: [{ pos: 20, length: 5 }], resolved: false }, + ]) + }) + + it('fromRaw() should create StringFileData with comments', function () { + const fileData = StringFileData.fromRaw({ + content: 'test', + comments: [ + { + id: 'comm1', + ranges: [ + { + pos: 5, + length: 10, + }, + ], + resolved: false, + }, + { + id: 'comm2', + ranges: [ + { + pos: 20, + length: 5, + }, + ], + resolved: true, + }, + ], + }) + + expect(fileData.getComments()).to.eql([ + { id: 'comm1', ranges: [{ pos: 5, length: 10 }], resolved: false }, + { id: 'comm2', ranges: [{ pos: 20, length: 5 }], resolved: true }, + ]) + }) })