Merge pull request #7 from sharelatex/sk-missing-users

Handle missing users

services/track-changes/app/coffee/UpdatesManager.coffee

@@ -188,10 +188,11 @@ module.exports = UpdatesManager =
 		for update in updates
 			earliestUpdate = summarizedUpdates[summarizedUpdates.length - 1]
 			if earliestUpdate and earliestUpdate.meta.start_ts - update.meta.end_ts < @TIME_BETWEEN_DISTINCT_UPDATES
-				if update.meta.user?
+				# check if the user in this update is already present in the earliest update,
+				# if not, add them to the users list of the earliest update
 				userExists = false
 				for user in earliestUpdate.meta.users
-						if user.id == update.meta.user.id
+					if (!user and !update.meta.user) or (user?.id == update.meta.user?.id)
 						userExists = true
 						break
 				if !userExists
@@ -220,11 +221,7 @@ module.exports = UpdatesManager =
 				newUpdate.docs[update.doc_id.toString()] =
 					fromV: update.v
 					toV: update.v
-
-				if update.meta.user?
 				newUpdate.meta.users.push update.meta.user
-
 				summarizedUpdates.push newUpdate
 
 		return summarizedUpdates
-

services/track-changes/app/coffee/WebApiManager.coffee

@@ -14,6 +14,7 @@ module.exports = WebApiManager =
 			if error?
 				return callback(error)
 			if res.statusCode == 404
+				logger.log url: url, "got 404 from web api"
 				return callback null, null
 			if res.statusCode >= 200 and res.statusCode < 300
 				return callback null, body

services/track-changes/test/acceptance/coffee/GettingUpdatesTests.coffee

@@ -15,6 +15,7 @@ describe "Getting updates", ->
 		@now = Date.now()
 		@to = @now
 		@user_id = ObjectId().toString()
+		@deleted_user_id = 'deleted_user'
 		@doc_id = ObjectId().toString()
 		@project_id = ObjectId().toString()
 
@@ -44,6 +45,7 @@ describe "Getting updates", ->
 				meta: { ts: @now - (9 - i) * @hours, user_id: @user_id }
 				v: 2 * i + 2
 			}
+		@updates[0].meta.user_id = @deleted_user_id
 
 		TrackChangesClient.pushRawUpdates @project_id, @doc_id, @updates, (error) =>
 			throw error if error?
@@ -91,7 +93,6 @@ describe "Getting updates", ->
 					users: [@user]
 			}]
 
-
 	describe "getting updates beyond the end of the database", ->
 		before (done) ->
 			TrackChangesClient.getUpdates @project_id, { before: @to - 8 * @hours + 1, min_count: 30 }, (error, body) =>
@@ -115,6 +116,5 @@ describe "Getting updates", ->
 				meta:
 					start_ts: @to - 9 * @hours - 2 * @minutes
 					end_ts: @to - 9 * @hours
-					users: [@user]
+					users: [@user, null]
 			}]
-

services/track-changes/test/acceptance/coffee/helpers/MockWebApi.coffee

@@ -7,7 +7,7 @@ module.exports = MockWebApi =
 	projects: {}
 
 	getUser: (user_id, callback = (error) ->) ->
-		callback null, @users[user_id]
+		callback null, @users[user_id] or null
 
 	getProject: (project_id, callback = (error, project) ->) ->
 		callback null, @projects[project_id]

services/track-changes/test/unit/coffee/UpdatesManager/UpdatesManagerTests.coffee

@@ -627,3 +627,91 @@ describe "UpdatesManager", ->
 					start_ts: @now
 					end_ts:   @now + 50
 			}]
+
+		it "should include null user values", ->
+			result = @UpdatesManager._summarizeUpdates [{
+				doc_id: "doc-id-1"
+				meta:
+					user: @user_1
+					start_ts: @now + 20
+					end_ts:   @now + 30
+				v: 5
+			}, {
+				doc_id: "doc-id-1"
+				meta:
+					user: null
+					start_ts: @now
+					end_ts:   @now + 10
+				v: 4
+			}]
+			expect(result).to.deep.equal [{
+				docs:
+					"doc-id-1":
+						fromV: 4
+						toV: 5
+				meta:
+					users: [@user_1, null]
+					start_ts: @now
+					end_ts:   @now + 30
+			}]
+
+		it "should include null user values, when the null is earlier in the updates list", ->
+			result = @UpdatesManager._summarizeUpdates [{
+				doc_id: "doc-id-1"
+				meta:
+					user: null
+					start_ts: @now
+					end_ts:   @now + 10
+				v: 4
+			}, {
+				doc_id: "doc-id-1"
+				meta:
+					user: @user_1
+					start_ts: @now + 20
+					end_ts:   @now + 30
+				v: 5
+			}]
+			expect(result).to.deep.equal [{
+				docs:
+					"doc-id-1":
+						fromV: 4
+						toV: 5
+				meta:
+					users: [null, @user_1]
+					start_ts: @now
+					end_ts:   @now + 30
+			}]
+
+		it "should roll several null user values into one", ->
+			result = @UpdatesManager._summarizeUpdates [{
+				doc_id: "doc-id-1"
+				meta:
+					user: @user_1
+					start_ts: @now + 20
+					end_ts:   @now + 30
+				v: 5
+			}, {
+				doc_id: "doc-id-1"
+				meta:
+					user: null
+					start_ts: @now
+					end_ts:   @now + 10
+				v: 4
+			}, {
+				doc_id: "doc-id-1"
+				meta:
+					user: null
+					start_ts: @now + 2
+					end_ts:   @now + 4
+				v: 4
+			}]
+			expect(result).to.deep.equal [{
+				docs:
+					"doc-id-1":
+						fromV: 4
+						toV: 5
+				meta:
+					users: [@user_1, null]
+					start_ts: @now
+					end_ts:   @now + 30
+			}]

services/track-changes/test/unit/coffee/WebApiManager/WebApiManagerTests.coffee

@@ -72,6 +72,16 @@ describe "WebApiManager", ->
 					.calledWith(new Error("web returned failure status code: 500"))
 					.should.equal true
 
+		describe "when the user cannot be found", ->
+			beforeEach ->
+				@request.get = sinon.stub().callsArgWith(1, null, {statusCode: 404}, "nothing")
+				@WebApiManager.getUserInfo @user_id, @callback
+
+			it "should return a null value", ->
+				@callback
+					.calledWith(null, null)
+					.should.equal true
+
 
 	describe "getProjectDetails", ->
 		describe "successfully", ->