diff --git a/services/track-changes/app/coffee/UpdatesManager.coffee b/services/track-changes/app/coffee/UpdatesManager.coffee index 92357fc2dc..9bea5ba177 100644 --- a/services/track-changes/app/coffee/UpdatesManager.coffee +++ b/services/track-changes/app/coffee/UpdatesManager.coffee @@ -188,14 +188,15 @@ 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? - userExists = false - for user in earliestUpdate.meta.users - if user.id == update.meta.user.id - userExists = true - break - if !userExists - earliestUpdate.meta.users.push 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 and !update.meta.user) or (user?.id == update.meta.user?.id) + userExists = true + break + if !userExists + earliestUpdate.meta.users.push update.meta.user doc_id = update.doc_id.toString() doc = earliestUpdate.docs[doc_id] @@ -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 - + newUpdate.meta.users.push update.meta.user summarizedUpdates.push newUpdate return summarizedUpdates - diff --git a/services/track-changes/app/coffee/WebApiManager.coffee b/services/track-changes/app/coffee/WebApiManager.coffee index 9264862fa3..d7441682e3 100644 --- a/services/track-changes/app/coffee/WebApiManager.coffee +++ b/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 diff --git a/services/track-changes/test/acceptance/coffee/GettingUpdatesTests.coffee b/services/track-changes/test/acceptance/coffee/GettingUpdatesTests.coffee index 6452649fc4..7c94bcdc34 100644 --- a/services/track-changes/test/acceptance/coffee/GettingUpdatesTests.coffee +++ b/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] }] - diff --git a/services/track-changes/test/acceptance/coffee/helpers/MockWebApi.coffee b/services/track-changes/test/acceptance/coffee/helpers/MockWebApi.coffee index f7cfc9e2e9..002d944dfd 100644 --- a/services/track-changes/test/acceptance/coffee/helpers/MockWebApi.coffee +++ b/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] diff --git a/services/track-changes/test/unit/coffee/UpdatesManager/UpdatesManagerTests.coffee b/services/track-changes/test/unit/coffee/UpdatesManager/UpdatesManagerTests.coffee index fdc19360b7..fc8853f2ab 100644 --- a/services/track-changes/test/unit/coffee/UpdatesManager/UpdatesManagerTests.coffee +++ b/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 + }] diff --git a/services/track-changes/test/unit/coffee/WebApiManager/WebApiManagerTests.coffee b/services/track-changes/test/unit/coffee/WebApiManager/WebApiManagerTests.coffee index aec51ab2ca..177a7b26f0 100644 --- a/services/track-changes/test/unit/coffee/WebApiManager/WebApiManagerTests.coffee +++ b/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", ->