From dadc548f81150df5f995850cbd908d87d6f40c57 Mon Sep 17 00:00:00 2001
From: James Allen <james@sharelatex.com>
Date: Thu, 12 Jan 2017 10:04:50 +0100
Subject: [PATCH] Ignore comment updates

---
 .../app/coffee/DiffGenerator.coffee           |  3 +++
 .../app/coffee/UpdateCompressor.coffee        |  6 +++--
 .../coffee/AppendingUpdatesTests.coffee       | 22 +++++++++++++++++++
 .../UpdateCompressorTests.coffee              | 16 ++++++++++++++
 4 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/services/track-changes/app/coffee/DiffGenerator.coffee b/services/track-changes/app/coffee/DiffGenerator.coffee
index c9c8e1d4bd..8738621f67 100644
--- a/services/track-changes/app/coffee/DiffGenerator.coffee
+++ b/services/track-changes/app/coffee/DiffGenerator.coffee
@@ -47,6 +47,9 @@ module.exports = DiffGenerator =
 
 		else if op.d?
 			return content.slice(0, op.p) + op.d + content.slice(op.p)
+		
+		else
+			return content
 
 	rewindUpdates: (content, updates) ->
 		for update in updates.reverse()
diff --git a/services/track-changes/app/coffee/UpdateCompressor.coffee b/services/track-changes/app/coffee/UpdateCompressor.coffee
index 3ae5adf8fb..9e11b281e6 100644
--- a/services/track-changes/app/coffee/UpdateCompressor.coffee
+++ b/services/track-changes/app/coffee/UpdateCompressor.coffee
@@ -24,7 +24,9 @@ module.exports = UpdateCompressor =
 	convertToSingleOpUpdates: (updates) ->
 		splitUpdates = []
 		for update in updates
-			if update.op.length == 0
+			# Reject any non-insert or delete ops, i.e. comments
+			ops = update.op.filter (o) -> o.i? or o.d?
+			if ops.length == 0
 				splitUpdates.push
 					op: UpdateCompressor.NOOP
 					meta:
@@ -33,7 +35,7 @@ module.exports = UpdateCompressor =
 						user_id:  update.meta.user_id
 					v: update.v
 			else
-				for op in update.op
+				for op in ops
 					splitUpdates.push
 						op: op
 						meta:
diff --git a/services/track-changes/test/acceptance/coffee/AppendingUpdatesTests.coffee b/services/track-changes/test/acceptance/coffee/AppendingUpdatesTests.coffee
index 3d4bf14465..69d378fd9d 100644
--- a/services/track-changes/test/acceptance/coffee/AppendingUpdatesTests.coffee
+++ b/services/track-changes/test/acceptance/coffee/AppendingUpdatesTests.coffee
@@ -233,6 +233,28 @@ describe "Appending doc ops to the history", ->
 			expect(@updates[0].pack[0].v).to.equal 3
 			expect(@updates[0].pack[1].v).to.equal 4
 
+	describe "when there is a comment update", ->
+		before (done) ->
+			@project_id = ObjectId().toString()
+			@doc_id = ObjectId().toString()
+			@user_id = ObjectId().toString()
+			MockWebApi.projects[@project_id] = features: versioning: false
+			TrackChangesClient.pushRawUpdates @project_id, @doc_id, [{
+				op: [{ c: "foo", p: 3 }, {d: "bar", p: 6}]
+				meta: { ts: Date.now(), user_id: @user_id }
+				v: 3
+			}], (error) =>
+				throw error if error?
+				TrackChangesClient.flushAndGetCompressedUpdates @project_id, @doc_id, (error, @updates) =>
+					throw error if error?
+					done()
+
+		it "should ignore the comment op", ->
+			expect(@updates[0].pack[0].op).to.deep.equal [{d: "bar", p: 6}]
+
+		it "should insert the correct version numbers into mongo", ->
+			expect(@updates[0].pack[0].v).to.equal 3
+
 	describe "when the project has versioning enabled", ->
 		before (done) ->
 			@project_id = ObjectId().toString()
diff --git a/services/track-changes/test/unit/coffee/UpdateCompressor/UpdateCompressorTests.coffee b/services/track-changes/test/unit/coffee/UpdateCompressor/UpdateCompressorTests.coffee
index 05fe923ab3..b4488c17bc 100644
--- a/services/track-changes/test/unit/coffee/UpdateCompressor/UpdateCompressorTests.coffee
+++ b/services/track-changes/test/unit/coffee/UpdateCompressor/UpdateCompressorTests.coffee
@@ -54,6 +54,22 @@ describe "UpdateCompressor", ->
 				v: 42
 			}]
 
+		it "should ignore comment ops", ->
+			expect(@UpdateCompressor.convertToSingleOpUpdates [{
+				op:   [ @op1 = { p: 0, i: "Foo" }, @op2 = { p: 9, c: "baz"}, @op3 = { p: 6, i: "bar"} ]
+				meta: { ts: @ts1, user_id: @user_id }
+				v: 42
+			}])
+			.to.deep.equal [{
+				op: @op1,
+				meta: { start_ts: @ts1, end_ts: @ts1, user_id: @user_id },
+				v: 42
+			}, {
+				op: @op3,
+				meta: { start_ts: @ts1, end_ts: @ts1, user_id: @user_id },
+				v: 42
+			}]
+
 	describe "concatUpdatesWithSameVersion", ->
 		it "should concat updates with the same version", ->
 			expect(@UpdateCompressor.concatUpdatesWithSameVersion [{