Przeglądaj źródła

[overleaf-editor-core] SetCommentStateOperation (#17056)

GitOrigin-RevId: 6efb6e3c9cb4b0cb9fdbc522bc57b1c1934a5062
Domagoj Kriskovic 2 lat temu
rodzic
commit
5cb27f69d7

+ 2 - 0
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

+ 7 - 2
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))

+ 1 - 8
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)
   }
 
   /**

+ 10 - 0
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}.`
     )

+ 21 - 1
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

+ 43 - 6
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

+ 116 - 48
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]
-    }
-
-    if (a instanceof AddCommentOperation && b instanceof TextOperation) {
-      const comment = a.comment.applyTextOperation(b)
-      return [new AddCommentOperation(a.commentId, comment), b]
-    }
+          const shouldResolve = a.resolved && b.resolved
+          if (a.resolved === shouldResolve) {
+            return [a, new EditNoOperation()]
+          } else {
+            return [new EditNoOperation(), 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

+ 5 - 1
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) {

+ 112 - 0
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

+ 3 - 0
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

+ 3 - 6
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,
       },
       {

+ 98 - 1
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: {

+ 345 - 16
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()
+    })
+  })
 })