Skip to content

Commit

Permalink
Only save pending updates for annotations in the focused group
Browse files Browse the repository at this point in the history
Since we discard all pending updates and deletions when switching
groups, discard create/update notifications about annotations in
unfocused groups as soon as we receive them.

This fixes the issue where the 'Apply Update' badge counted updates
from other groups, with the result that clicking the badge did not
show any changes.
  • Loading branch information
robertknight committed Sep 16, 2016
1 parent e7d5410 commit 3dc467c
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 22 deletions.
24 changes: 13 additions & 11 deletions h/static/scripts/streamer.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,16 +51,17 @@ function Streamer($rootScope, annotationMapper, features, groups, session, setti
var action = message.options.action;
var annotations = message.payload;

if (annotations.length === 0) {
return;
}

switch (action) {
case 'create':
case 'update':
case 'past':
annotations.forEach(function (ann) {
pendingUpdates[ann.id] = ann;
// Only include annotations from the focused group, since we reload all
// annotations and discard pending updates and deletions when switching
// groups
if (ann.group === groups.focused().id) {
pendingUpdates[ann.id] = ann;
}
});
break;
case 'delete':
Expand Down Expand Up @@ -173,16 +174,17 @@ function Streamer($rootScope, annotationMapper, features, groups, session, setti
};

function applyPendingUpdates() {
var updates = Object.values(pendingUpdates).filter(function (ann) {
// Ignore updates to annotations that are not in the focused group
return ann.group === groups.focused().id;
});
var updates = Object.values(pendingUpdates);
var deletions = Object.keys(pendingDeletions).map(function (id) {
return {id: id};
});

annotationMapper.loadAnnotations(updates);
annotationMapper.unloadAnnotations(deletions);
if (updates.length) {
annotationMapper.loadAnnotations(updates);
}
if (deletions.length) {
annotationMapper.unloadAnnotations(deletions);
}

pendingUpdates = {};
pendingDeletions = {};
Expand Down
24 changes: 13 additions & 11 deletions h/static/scripts/test/streamer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ var fixtures = {
},
payload: [{
id: 'an-id',
group: 'public',
}],
},
};
Expand Down Expand Up @@ -109,9 +108,7 @@ describe('Streamer', function () {
};

fakeGroups = {
focused: function () {
return {id: 'public'};
},
focused: sinon.stub().returns({id: 'public'}),
};

fakeSession = {
Expand Down Expand Up @@ -200,6 +197,12 @@ describe('Streamer', function () {
fakeWebSocket.notify(fixtures.deleteNotification);
assert.ok(fakeAnnotationMapper.unloadAnnotations.calledOnce);
});

it('ignores notifications about annotations in unfocused groups', function () {
fakeGroups.focused.returns({id: 'private'});
fakeWebSocket.notify(fixtures.createNotification);
assert.notCalled(fakeAnnotationMapper.loadAnnotations);
});
});

context('when realtime updates are deferred', function () {
Expand All @@ -212,6 +215,12 @@ describe('Streamer', function () {
assert.equal(activeStreamer.countPendingUpdates(), 1);
});

it('does not save pending updates for annotations in unfocused groups', function () {
fakeGroups.focused.returns({id: 'private'});
fakeWebSocket.notify(fixtures.createNotification);
assert.equal(activeStreamer.countPendingUpdates(), 0);
});

it('saves pending deletions', function () {
var id = fixtures.deleteNotification.payload[0].id;
fakeWebSocket.notify(fixtures.deleteNotification);
Expand Down Expand Up @@ -256,13 +265,6 @@ describe('Streamer', function () {
fixtures.createNotification.payload);
});

it('does not apply pending updates for annotations in unfocused groups', function () {
fakeWebSocket.notify(fixtures.createNotification);
fakeGroups.focused = function () { return {id: 'private'}; };
activeStreamer.applyPendingUpdates();
assert.calledWith(fakeAnnotationMapper.loadAnnotations, []);
});

it('applies pending deletions', function () {
fakeWebSocket.notify(fixtures.deleteNotification);
activeStreamer.applyPendingUpdates();
Expand Down

0 comments on commit 3dc467c

Please sign in to comment.