Unverified Commit 398663fa authored by Robert Knight's avatar Robert Knight Committed by GitHub

Merge pull request #1212 from hypothesis/move-pending-update-state-to-store

Move real-time update state to store
parents f08e7eae 9c2a6849
...@@ -59,8 +59,7 @@ function AnnotationController( ...@@ -59,8 +59,7 @@ function AnnotationController(
permissions, permissions,
serviceUrl, serviceUrl,
session, session,
settings, settings
streamer
) { ) {
const self = this; const self = this;
let newlyCreatedByHighlightButton; let newlyCreatedByHighlightButton;
...@@ -525,7 +524,7 @@ function AnnotationController( ...@@ -525,7 +524,7 @@ function AnnotationController(
}; };
this.isDeleted = function() { this.isDeleted = function() {
return streamer.hasPendingDeletion(self.annotation.id); return store.hasPendingDeletion(self.annotation.id);
}; };
this.isHiddenByModerator = function() { this.isHiddenByModerator = function() {
......
...@@ -49,8 +49,7 @@ function HypothesisAppController( ...@@ -49,8 +49,7 @@ function HypothesisAppController(
groups, groups,
serviceUrl, serviceUrl,
session, session,
settings, settings
streamer
) { ) {
const self = this; const self = this;
...@@ -184,9 +183,6 @@ function HypothesisAppController( ...@@ -184,9 +183,6 @@ function HypothesisAppController(
session.logout(); session.logout();
}; };
this.countPendingUpdates = streamer.countPendingUpdates;
this.applyPendingUpdates = streamer.applyPendingUpdates;
} }
module.exports = { module.exports = {
......
...@@ -115,7 +115,6 @@ describe('annotation', function() { ...@@ -115,7 +115,6 @@ describe('annotation', function() {
let fakeSettings; let fakeSettings;
let fakeApi; let fakeApi;
let fakeBridge; let fakeBridge;
let fakeStreamer;
let sandbox; let sandbox;
beforeEach(() => { beforeEach(() => {
...@@ -188,6 +187,7 @@ describe('annotation', function() { ...@@ -188,6 +187,7 @@ describe('annotation', function() {
}; };
fakeStore = { fakeStore = {
hasPendingDeletion: sinon.stub(),
updateFlagStatus: sandbox.stub().returns(true), updateFlagStatus: sandbox.stub().returns(true),
// draft store // draft store
countDrafts: sandbox.stub().returns(0), countDrafts: sandbox.stub().returns(0),
...@@ -253,10 +253,6 @@ describe('annotation', function() { ...@@ -253,10 +253,6 @@ describe('annotation', function() {
call: sinon.stub(), call: sinon.stub(),
}; };
fakeStreamer = {
hasPendingDeletion: sinon.stub(),
};
$provide.value('analytics', fakeAnalytics); $provide.value('analytics', fakeAnalytics);
$provide.value('annotationMapper', fakeAnnotationMapper); $provide.value('annotationMapper', fakeAnnotationMapper);
$provide.value('store', fakeStore); $provide.value('store', fakeStore);
...@@ -268,7 +264,6 @@ describe('annotation', function() { ...@@ -268,7 +264,6 @@ describe('annotation', function() {
$provide.value('session', fakeSession); $provide.value('session', fakeSession);
$provide.value('serviceUrl', fakeServiceUrl); $provide.value('serviceUrl', fakeServiceUrl);
$provide.value('settings', fakeSettings); $provide.value('settings', fakeSettings);
$provide.value('streamer', fakeStreamer);
}) })
); );
...@@ -860,13 +855,13 @@ describe('annotation', function() { ...@@ -860,13 +855,13 @@ describe('annotation', function() {
describe('#isDeleted', function() { describe('#isDeleted', function() {
it('returns true if the annotation has been marked as deleted', function() { it('returns true if the annotation has been marked as deleted', function() {
const controller = createDirective().controller; const controller = createDirective().controller;
fakeStreamer.hasPendingDeletion.returns(true); fakeStore.hasPendingDeletion.returns(true);
assert.equal(controller.isDeleted(), true); assert.equal(controller.isDeleted(), true);
}); });
it('returns false if the annotation has not been marked as deleted', function() { it('returns false if the annotation has not been marked as deleted', function() {
const controller = createDirective().controller; const controller = createDirective().controller;
fakeStreamer.hasPendingDeletion.returns(false); fakeStore.hasPendingDeletion.returns(false);
assert.equal(controller.isDeleted(), false); assert.equal(controller.isDeleted(), false);
}); });
}); });
......
...@@ -25,7 +25,6 @@ describe('sidebar.components.hypothesis-app', function() { ...@@ -25,7 +25,6 @@ describe('sidebar.components.hypothesis-app', function() {
let fakeRoute = null; let fakeRoute = null;
let fakeServiceUrl = null; let fakeServiceUrl = null;
let fakeSettings = null; let fakeSettings = null;
let fakeStreamer = null;
let fakeWindow = null; let fakeWindow = null;
let sandbox = null; let sandbox = null;
...@@ -111,10 +110,6 @@ describe('sidebar.components.hypothesis-app', function() { ...@@ -111,10 +110,6 @@ describe('sidebar.components.hypothesis-app', function() {
fakeServiceUrl = sinon.stub(); fakeServiceUrl = sinon.stub();
fakeSettings = {}; fakeSettings = {};
fakeStreamer = {
countPendingUpdates: sinon.stub(),
applyPendingUpdates: sinon.stub(),
};
fakeBridge = { fakeBridge = {
call: sandbox.stub(), call: sandbox.stub(),
}; };
...@@ -129,7 +124,6 @@ describe('sidebar.components.hypothesis-app', function() { ...@@ -129,7 +124,6 @@ describe('sidebar.components.hypothesis-app', function() {
$provide.value('session', fakeSession); $provide.value('session', fakeSession);
$provide.value('settings', fakeSettings); $provide.value('settings', fakeSettings);
$provide.value('bridge', fakeBridge); $provide.value('bridge', fakeBridge);
$provide.value('streamer', fakeStreamer);
$provide.value('groups', fakeGroups); $provide.value('groups', fakeGroups);
$provide.value('$route', fakeRoute); $provide.value('$route', fakeRoute);
$provide.value('$routeParams', fakeParams); $provide.value('$routeParams', fakeParams);
......
...@@ -12,17 +12,23 @@ const UserMenu = require('../user-menu'); ...@@ -12,17 +12,23 @@ const UserMenu = require('../user-menu');
describe('TopBar', () => { describe('TopBar', () => {
const fakeSettings = {}; const fakeSettings = {};
let fakeIsThirdPartyService;
let fakeStore; let fakeStore;
let fakeStreamer;
let fakeIsThirdPartyService;
beforeEach(() => { beforeEach(() => {
fakeIsThirdPartyService = sinon.stub().returns(false); fakeIsThirdPartyService = sinon.stub().returns(false);
fakeStore = { fakeStore = {
filterQuery: sinon.stub().returns(null), filterQuery: sinon.stub().returns(null),
pendingUpdateCount: sinon.stub().returns(0),
setFilterQuery: sinon.stub(), setFilterQuery: sinon.stub(),
}; };
fakeStreamer = {
applyPendingUpdates: sinon.stub(),
};
TopBar.$imports.$mock({ TopBar.$imports.$mock({
'../store/use-store': callback => callback(fakeStore), '../store/use-store': callback => callback(fakeStore),
'../util/is-third-party-service': fakeIsThirdPartyService, '../util/is-third-party-service': fakeIsThirdPartyService,
...@@ -44,35 +50,35 @@ describe('TopBar', () => { ...@@ -44,35 +50,35 @@ describe('TopBar', () => {
function createTopBar(props = {}) { function createTopBar(props = {}) {
const auth = { status: 'unknown' }; const auth = { status: 'unknown' };
return shallow( return shallow(
<TopBar auth={auth} isSidebar={true} settings={fakeSettings} {...props} /> <TopBar
auth={auth}
isSidebar={true}
settings={fakeSettings}
streamer={fakeStreamer}
{...props}
/>
).dive(); // Dive through `withServices` wrapper. ).dive(); // Dive through `withServices` wrapper.
} }
it('shows the pending update count', () => { it('shows the pending update count', () => {
const wrapper = createTopBar({ fakeStore.pendingUpdateCount.returns(1);
pendingUpdateCount: 1, const wrapper = createTopBar();
});
const applyBtn = applyUpdateBtn(wrapper); const applyBtn = applyUpdateBtn(wrapper);
assert.isTrue(applyBtn.exists()); assert.isTrue(applyBtn.exists());
}); });
it('does not show the pending update count when there are no updates', () => { it('does not show the pending update count when there are no updates', () => {
const wrapper = createTopBar({ const wrapper = createTopBar();
pendingUpdateCount: 0,
});
const applyBtn = applyUpdateBtn(wrapper); const applyBtn = applyUpdateBtn(wrapper);
assert.isFalse(applyBtn.exists()); assert.isFalse(applyBtn.exists());
}); });
it('applies updates when clicked', () => { it('applies updates when clicked', () => {
const onApplyPendingUpdates = sinon.stub(); fakeStore.pendingUpdateCount.returns(1);
const wrapper = createTopBar({ const wrapper = createTopBar();
pendingUpdateCount: 1,
onApplyPendingUpdates,
});
const applyBtn = applyUpdateBtn(wrapper); const applyBtn = applyUpdateBtn(wrapper);
applyBtn.simulate('click'); applyBtn.simulate('click');
assert.called(onApplyPendingUpdates); assert.called(fakeStreamer.applyPendingUpdates);
}); });
it('shows Help Panel when help icon is clicked', () => { it('shows Help Panel when help icon is clicked', () => {
......
...@@ -23,14 +23,13 @@ const UserMenu = require('./user-menu'); ...@@ -23,14 +23,13 @@ const UserMenu = require('./user-menu');
function TopBar({ function TopBar({
auth, auth,
isSidebar, isSidebar,
onApplyPendingUpdates,
onLogin, onLogin,
onLogout, onLogout,
onSharePage, onSharePage,
onShowHelpPanel, onShowHelpPanel,
onSignUp, onSignUp,
pendingUpdateCount,
settings, settings,
streamer,
}) { }) {
const useCleanTheme = settings.theme === 'clean'; const useCleanTheme = settings.theme === 'clean';
const showSharePageButton = !isThirdPartyService(settings); const showSharePageButton = !isThirdPartyService(settings);
...@@ -39,6 +38,10 @@ function TopBar({ ...@@ -39,6 +38,10 @@ function TopBar({
const filterQuery = useStore(store => store.filterQuery()); const filterQuery = useStore(store => store.filterQuery());
const setFilterQuery = useStore(store => store.setFilterQuery); const setFilterQuery = useStore(store => store.setFilterQuery);
const pendingUpdateCount = useStore(store => store.pendingUpdateCount());
const applyPendingUpdates = () => streamer.applyPendingUpdates();
const loginControl = ( const loginControl = (
<Fragment> <Fragment>
{auth.status === 'unknown' && ( {auth.status === 'unknown' && (
...@@ -89,7 +92,7 @@ function TopBar({ ...@@ -89,7 +92,7 @@ function TopBar({
{pendingUpdateCount > 0 && ( {pendingUpdateCount > 0 && (
<a <a
className="top-bar__apply-update-btn" className="top-bar__apply-update-btn"
onClick={onApplyPendingUpdates} onClick={applyPendingUpdates}
title={`Show ${pendingUpdateCount} new/updated ${ title={`Show ${pendingUpdateCount} new/updated ${
pendingUpdateCount === 1 ? 'annotation' : 'annotations' pendingUpdateCount === 1 ? 'annotation' : 'annotations'
}`} }`}
...@@ -161,16 +164,11 @@ TopBar.propTypes = { ...@@ -161,16 +164,11 @@ TopBar.propTypes = {
/** Callback invoked when user clicks "Sign up" button. */ /** Callback invoked when user clicks "Sign up" button. */
onSignUp: propTypes.func, onSignUp: propTypes.func,
/** Count of updates received via WebSocket that have not been applied. */
pendingUpdateCount: propTypes.number,
/** Called when user clicks button to apply pending real-time updates. */
onApplyPendingUpdates: propTypes.func,
// Services // Services
settings: propTypes.object, settings: propTypes.object,
streamer: propTypes.object,
}; };
TopBar.injectedProps = ['settings']; TopBar.injectedProps = ['settings', 'streamer'];
module.exports = withServices(TopBar); module.exports = withServices(TopBar);
...@@ -5,7 +5,6 @@ const uuid = require('node-uuid'); ...@@ -5,7 +5,6 @@ const uuid = require('node-uuid');
const warnOnce = require('../../shared/warn-once'); const warnOnce = require('../../shared/warn-once');
const events = require('../events');
const Socket = require('../websocket'); const Socket = require('../websocket');
/** /**
...@@ -41,23 +40,6 @@ function Streamer( ...@@ -41,23 +40,6 @@ function Streamer(
// established. // established.
const configMessages = {}; const configMessages = {};
// The streamer maintains a set of pending updates and deletions which have
// been received via the WebSocket but not yet applied to the contents of the
// app.
//
// This state should be managed as part of the global app state in
// store, but that is currently difficult because applying updates
// requires filtering annotations against the focused group (information not
// currently stored in the app state) and triggering events in order to update
// the annotations displayed in the page.
// Map of ID -> updated annotation for updates that have been received over
// the WS but not yet applied
let pendingUpdates = {};
// Set of IDs of annotations which have been deleted but for which the
// deletion has not yet been applied
let pendingDeletions = {};
function handleAnnotationNotification(message) { function handleAnnotationNotification(message) {
const action = message.options.action; const action = message.options.action;
const annotations = message.payload; const annotations = message.payload;
...@@ -66,31 +48,10 @@ function Streamer( ...@@ -66,31 +48,10 @@ function Streamer(
case 'create': case 'create':
case 'update': case 'update':
case 'past': case 'past':
annotations.forEach(function(ann) { store.receiveRealTimeUpdates({ updatedAnnotations: annotations });
// In the sidebar, only save pending updates for annotations in the
// focused group, since we only display annotations from the focused
// group and reload all annotations and discard pending updates
// when switching groups.
if (ann.group === groups.focused().id || !store.isSidebar()) {
pendingUpdates[ann.id] = ann;
}
});
break; break;
case 'delete': case 'delete':
annotations.forEach(function(ann) { store.receiveRealTimeUpdates({ deletedAnnotations: annotations });
// Discard any pending but not-yet-applied updates for this annotation
delete pendingUpdates[ann.id];
// If we already have this annotation loaded, then record a pending
// deletion. We do not check the group of the annotation here because a)
// that information is not included with deletion notifications and b)
// even if the annotation is from the current group, it might be for a
// new annotation (saved in pendingUpdates and removed above), that has
// not yet been loaded.
if (store.annotationExists(ann.id)) {
pendingDeletions[ann.id] = true;
}
});
break; break;
} }
...@@ -257,61 +218,20 @@ function Streamer( ...@@ -257,61 +218,20 @@ function Streamer(
} }
function applyPendingUpdates() { function applyPendingUpdates() {
const updates = Object.values(pendingUpdates); const updates = Object.values(store.pendingUpdates());
const deletions = Object.keys(pendingDeletions).map(function(id) {
return { id: id };
});
if (updates.length) { if (updates.length) {
annotationMapper.loadAnnotations(updates); annotationMapper.loadAnnotations(updates);
} }
const deletions = Object.keys(store.pendingDeletions()).map(id => ({ id }));
if (deletions.length) { if (deletions.length) {
annotationMapper.unloadAnnotations(deletions); annotationMapper.unloadAnnotations(deletions);
} }
pendingUpdates = {}; store.clearPendingUpdates();
pendingDeletions = {};
}
function countPendingUpdates() {
return (
Object.keys(pendingUpdates).length + Object.keys(pendingDeletions).length
);
}
function hasPendingDeletion(id) {
return pendingDeletions.hasOwnProperty(id);
}
function removePendingUpdates(event, anns) {
if (!Array.isArray(anns)) {
anns = [anns];
}
anns.forEach(function(ann) {
delete pendingUpdates[ann.id];
delete pendingDeletions[ann.id];
});
}
function clearPendingUpdates() {
pendingUpdates = {};
pendingDeletions = {};
} }
const updateEvents = [
events.ANNOTATION_DELETED,
events.ANNOTATION_UPDATED,
events.ANNOTATIONS_UNLOADED,
];
updateEvents.forEach(function(event) {
$rootScope.$on(event, removePendingUpdates);
});
$rootScope.$on(events.GROUP_FOCUSED, clearPendingUpdates);
this.applyPendingUpdates = applyPendingUpdates; this.applyPendingUpdates = applyPendingUpdates;
this.countPendingUpdates = countPendingUpdates;
this.hasPendingDeletion = hasPendingDeletion;
this.clientId = clientId; this.clientId = clientId;
this.configMessages = configMessages; this.configMessages = configMessages;
this.connect = connect; this.connect = connect;
......
...@@ -2,7 +2,6 @@ ...@@ -2,7 +2,6 @@
const EventEmitter = require('tiny-emitter'); const EventEmitter = require('tiny-emitter');
const events = require('../../events');
const unroll = require('../../../shared/test/util').unroll; const unroll = require('../../../shared/test/util').unroll;
const Streamer = require('../streamer'); const Streamer = require('../streamer');
...@@ -122,12 +121,16 @@ describe('Streamer', function() { ...@@ -122,12 +121,16 @@ describe('Streamer', function() {
fakeStore = { fakeStore = {
annotationExists: sinon.stub().returns(false), annotationExists: sinon.stub().returns(false),
isSidebar: sinon.stub().returns(true), clearPendingUpdates: sinon.stub(),
getState: sinon.stub().returns({ getState: sinon.stub().returns({
session: { session: {
userid: 'jim@hypothes.is', userid: 'jim@hypothes.is',
}, },
}), }),
isSidebar: sinon.stub().returns(true),
pendingUpdates: sinon.stub().returns({}),
pendingDeletions: sinon.stub().returns({}),
receiveRealTimeUpdates: sinon.stub(),
}; };
fakeGroups = { fakeGroups = {
...@@ -278,20 +281,17 @@ describe('Streamer', function() { ...@@ -278,20 +281,17 @@ describe('Streamer', function() {
fakeStore.isSidebar.returns(false); fakeStore.isSidebar.returns(false);
}); });
it('does not defer updates', function() { it('applies updates immediately', function() {
fakeWebSocket.notify(fixtures.createNotification); const [ann] = fixtures.createNotification.payload;
fakeStore.pendingUpdates.returns({
assert.calledWith( [ann.id]: ann,
fakeAnnotationMapper.loadAnnotations, });
fixtures.createNotification.payload
);
});
it('applies updates from all groups', function() {
fakeGroups.focused.returns({ id: 'private' });
fakeWebSocket.notify(fixtures.createNotification); fakeWebSocket.notify(fixtures.createNotification);
assert.calledWith(fakeStore.receiveRealTimeUpdates, {
updatedAnnotations: [ann],
});
assert.calledWith( assert.calledWith(
fakeAnnotationMapper.loadAnnotations, fakeAnnotationMapper.loadAnnotations,
fixtures.createNotification.payload fixtures.createNotification.payload
...@@ -302,56 +302,37 @@ describe('Streamer', function() { ...@@ -302,56 +302,37 @@ describe('Streamer', function() {
context('when the app is the sidebar', function() { context('when the app is the sidebar', function() {
it('saves pending updates', function() { it('saves pending updates', function() {
fakeWebSocket.notify(fixtures.createNotification); fakeWebSocket.notify(fixtures.createNotification);
assert.equal(activeStreamer.countPendingUpdates(), 1); assert.calledWith(fakeStore.receiveRealTimeUpdates, {
}); updatedAnnotations: fixtures.createNotification.payload,
});
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 if the annotation is loaded', function() {
const id = fixtures.deleteNotification.payload[0].id;
fakeStore.annotationExists.returns(true);
fakeWebSocket.notify(fixtures.deleteNotification);
assert.isTrue(activeStreamer.hasPendingDeletion(id));
assert.equal(activeStreamer.countPendingUpdates(), 1);
}); });
it('discards pending deletions if the annotation is not loaded', function() { it('saves pending deletions', function() {
const id = fixtures.deleteNotification.payload[0].id;
fakeStore.annotationExists.returns(false);
fakeWebSocket.notify(fixtures.deleteNotification); fakeWebSocket.notify(fixtures.deleteNotification);
assert.calledWith(fakeStore.receiveRealTimeUpdates, {
assert.isFalse(activeStreamer.hasPendingDeletion(id)); deletedAnnotations: fixtures.deleteNotification.payload,
}); });
it('saves one pending update per annotation', function() {
fakeWebSocket.notify(fixtures.createNotification);
fakeWebSocket.notify(fixtures.updateNotification);
assert.equal(activeStreamer.countPendingUpdates(), 1);
}); });
it('discards pending updates if an unloaded annotation is deleted', function() { it('does not apply updates immediately', function() {
fakeStore.annotationExists.returns(false); const ann = fixtures.createNotification.payload;
fakeStore.pendingUpdates.returns({
[ann.id]: ann,
});
fakeWebSocket.notify(fixtures.createNotification); fakeWebSocket.notify(fixtures.createNotification);
fakeWebSocket.notify(fixtures.deleteNotification);
assert.equal(activeStreamer.countPendingUpdates(), 0);
});
it('does not apply updates immediately', function() {
fakeWebSocket.notify(fixtures.createNotification);
assert.notCalled(fakeAnnotationMapper.loadAnnotations); assert.notCalled(fakeAnnotationMapper.loadAnnotations);
}); });
it('does not apply deletions immediately', function() { it('does not apply deletions immediately', function() {
const ann = fixtures.deleteNotification.payload;
fakeStore.pendingDeletions.returns({
[ann.id]: true,
});
fakeWebSocket.notify(fixtures.deleteNotification); fakeWebSocket.notify(fixtures.deleteNotification);
assert.notCalled(fakeAnnotationMapper.unloadAnnotations); assert.notCalled(fakeAnnotationMapper.unloadAnnotations);
}); });
}); });
...@@ -364,18 +345,16 @@ describe('Streamer', function() { ...@@ -364,18 +345,16 @@ describe('Streamer', function() {
}); });
it('applies pending updates', function() { it('applies pending updates', function() {
fakeWebSocket.notify(fixtures.createNotification); fakeStore.pendingUpdates.returns({ 'an-id': { id: 'an-id' } });
activeStreamer.applyPendingUpdates(); activeStreamer.applyPendingUpdates();
assert.calledWith( assert.calledWith(fakeAnnotationMapper.loadAnnotations, [
fakeAnnotationMapper.loadAnnotations, { id: 'an-id' },
fixtures.createNotification.payload ]);
);
}); });
it('applies pending deletions', function() { it('applies pending deletions', function() {
fakeStore.annotationExists.returns(true); fakeStore.pendingDeletions.returns({ 'an-id': true });
fakeWebSocket.notify(fixtures.deleteNotification);
activeStreamer.applyPendingUpdates(); activeStreamer.applyPendingUpdates();
assert.calledWithMatch( assert.calledWithMatch(
...@@ -387,56 +366,7 @@ describe('Streamer', function() { ...@@ -387,56 +366,7 @@ describe('Streamer', function() {
it('clears the set of pending updates', function() { it('clears the set of pending updates', function() {
fakeWebSocket.notify(fixtures.createNotification); fakeWebSocket.notify(fixtures.createNotification);
activeStreamer.applyPendingUpdates(); activeStreamer.applyPendingUpdates();
assert.equal(activeStreamer.countPendingUpdates(), 0); assert.calledWith(fakeStore.clearPendingUpdates);
});
});
describe('when annotations are unloaded, updated or deleted', function() {
const changeEvents = [
{ event: events.ANNOTATION_DELETED },
{ event: events.ANNOTATION_UPDATED },
{ event: events.ANNOTATIONS_UNLOADED },
];
beforeEach(function() {
createDefaultStreamer();
return activeStreamer.connect();
});
unroll(
'discards pending updates when #event occurs',
function(testCase) {
fakeWebSocket.notify(fixtures.createNotification);
assert.equal(activeStreamer.countPendingUpdates(), 1);
fakeRootScope.$broadcast(testCase.event, { id: 'an-id' });
assert.equal(activeStreamer.countPendingUpdates(), 0);
},
changeEvents
);
unroll(
'discards pending deletions when #event occurs',
function(testCase) {
fakeStore.annotationExists.returns(true);
fakeWebSocket.notify(fixtures.deleteNotification);
fakeRootScope.$broadcast(testCase.event, { id: 'an-id' });
assert.isFalse(activeStreamer.hasPendingDeletion('an-id'));
},
changeEvents
);
});
describe('when the focused group changes', function() {
it('clears pending updates and deletions', function() {
createDefaultStreamer();
return activeStreamer.connect().then(function() {
fakeWebSocket.notify(fixtures.createNotification);
fakeRootScope.$broadcast(events.GROUP_FOCUSED);
assert.equal(activeStreamer.countPendingUpdates(), 0);
});
}); });
}); });
......
...@@ -41,6 +41,7 @@ const drafts = require('./modules/drafts'); ...@@ -41,6 +41,7 @@ const drafts = require('./modules/drafts');
const frames = require('./modules/frames'); const frames = require('./modules/frames');
const links = require('./modules/links'); const links = require('./modules/links');
const groups = require('./modules/groups'); const groups = require('./modules/groups');
const realTimeUpdates = require('./modules/real-time-updates');
const selection = require('./modules/selection'); const selection = require('./modules/selection');
const session = require('./modules/session'); const session = require('./modules/session');
const viewer = require('./modules/viewer'); const viewer = require('./modules/viewer');
...@@ -93,6 +94,7 @@ function store($rootScope, settings) { ...@@ -93,6 +94,7 @@ function store($rootScope, settings) {
frames, frames,
links, links,
groups, groups,
realTimeUpdates,
selection, selection,
session, session,
viewer, viewer,
......
'use strict';
/**
* This module contains state related to real-time updates received via the
* WebSocket connection to h's real-time API.
*/
const { createSelector } = require('reselect');
const { actionTypes } = require('../util');
const { selectors: annotationSelectors } = require('./annotations');
const { selectors: groupSelectors } = require('./groups');
const { selectors: viewerSelectors } = require('./viewer');
function init() {
return {
// Map of ID -> updated annotation for updates that have been received over
// the WebSocket but not yet applied
pendingUpdates: {},
// Set of IDs of annotations which have been deleted but for which the
// deletion has not yet been applied
pendingDeletions: {},
};
}
const update = {
RECEIVE_REAL_TIME_UPDATES(
state,
{ updatedAnnotations = [], deletedAnnotations = [] }
) {
const pendingUpdates = { ...state.pendingUpdates };
const pendingDeletions = { ...state.pendingDeletions };
updatedAnnotations.forEach(ann => {
// In the sidebar, only save pending updates for annotations in the
// focused group, since we only display annotations from the focused
// group and reload all annotations and discard pending updates
// when switching groups.
if (
ann.group === groupSelectors.focusedGroupId(state) ||
!viewerSelectors.isSidebar(state)
) {
pendingUpdates[ann.id] = ann;
}
});
deletedAnnotations.forEach(ann => {
// Discard any pending but not-yet-applied updates for this annotation
delete pendingUpdates[ann.id];
// If we already have this annotation loaded, then record a pending
// deletion. We do not check the group of the annotation here because a)
// that information is not included with deletion notifications and b)
// even if the annotation is from the current group, it might be for a
// new annotation (saved in pendingUpdates and removed above), that has
// not yet been loaded.
if (annotationSelectors.annotationExists(state, ann.id)) {
pendingDeletions[ann.id] = true;
}
});
return { pendingUpdates, pendingDeletions };
},
CLEAR_PENDING_UPDATES() {
return { pendingUpdates: {}, pendingDeletions: {} };
},
ADD_ANNOTATIONS(state, { annotations }) {
// Discard any pending updates which conflict with an annotation added
// locally or fetched via an API call.
//
// If there is a conflicting local update/remote delete then we keep
// the pending delete. The UI should prevent the user from editing an
// annotation that has been deleted on the server.
const pendingUpdates = { ...state.pendingUpdates };
annotations.forEach(ann => delete pendingUpdates[ann.id]);
return { pendingUpdates };
},
REMOVE_ANNOTATIONS(state, { annotations }) {
// Discard any pending updates which conflict with an annotation removed
// locally.
const pendingUpdates = { ...state.pendingUpdates };
const pendingDeletions = { ...state.pendingDeletions };
annotations.forEach(ann => {
delete pendingUpdates[ann.id];
delete pendingDeletions[ann.id];
});
return { pendingUpdates, pendingDeletions };
},
FOCUS_GROUP() {
// When switching groups we clear and re-fetch all annotations, so discard
// any pending updates.
return { pendingUpdates: {}, pendingDeletions: {} };
},
};
const actions = actionTypes(update);
/**
* Record pending updates representing changes on the server that the client
* has been notified about but has not yet applied.
*
* @param {Object} args
* @param {Annotation[]} args.updatedAnnotations
* @param {Annotation[]} args.deletedAnnotations
*/
function receiveRealTimeUpdates({ updatedAnnotations, deletedAnnotations }) {
return {
type: actions.RECEIVE_REAL_TIME_UPDATES,
updatedAnnotations,
deletedAnnotations,
};
}
/**
* Clear the queue of real-time updates which have been received but not applied.
*/
function clearPendingUpdates() {
return {
type: actions.CLEAR_PENDING_UPDATES,
};
}
/**
* Return added or updated annotations received via the WebSocket
* which have not been applied to the local state.
*
* @return {{[id: string]: Annotation}}
*/
function pendingUpdates(state) {
return state.pendingUpdates;
}
/**
* Return IDs of annotations which have been deleted on the server but not
* yet removed from the local state.
*
* @return {{[id: string]: boolean}}
*/
function pendingDeletions(state) {
return state.pendingDeletions;
}
/**
* Return a total count of pending updates and deletions.
*/
const pendingUpdateCount = createSelector(
state => [state.pendingUpdates, state.pendingDeletions],
([pendingUpdates, pendingDeletions]) =>
Object.keys(pendingUpdates).length + Object.keys(pendingDeletions).length
);
/**
* Return true if an annotation has been deleted on the server but the deletion
* has not yet been applied.
*/
function hasPendingDeletion(state, id) {
return state.pendingDeletions.hasOwnProperty(id);
}
module.exports = {
init,
update,
actions: {
receiveRealTimeUpdates,
clearPendingUpdates,
},
selectors: {
hasPendingDeletion,
pendingDeletions,
pendingUpdates,
pendingUpdateCount,
},
};
'use strict';
const createStore = require('../../create-store');
const annotations = require('../annotations');
const groups = require('../groups');
const realTimeUpdates = require('../real-time-updates');
const { removeAnnotations } = annotations.actions;
const { focusGroup } = groups.actions;
describe('sidebar/store/modules/real-time-updates', () => {
let fakeAnnotationExists;
let fakeFocusedGroupId;
let fakeIsSidebar;
let store;
beforeEach(() => {
fakeAnnotationExists = sinon.stub().returns(true);
fakeFocusedGroupId = sinon.stub().returns('group-1');
fakeIsSidebar = sinon.stub().returns(true);
store = createStore([realTimeUpdates]);
realTimeUpdates.$imports.$mock({
'./annotations': {
selectors: { annotationExists: fakeAnnotationExists },
},
'./groups': {
selectors: { focusedGroupId: fakeFocusedGroupId },
},
'./viewer': {
selectors: { isSidebar: fakeIsSidebar },
},
});
});
afterEach(() => {
realTimeUpdates.$imports.$restore();
});
function addPendingUpdates(store) {
const updates = [
{ id: 'updated-ann', group: 'group-1' },
{ id: 'created-ann', group: 'group-1' },
];
store.receiveRealTimeUpdates({
updatedAnnotations: updates,
});
return updates;
}
function addPendingDeletions(store) {
const deletions = [{ id: 'deleted-ann' }];
store.receiveRealTimeUpdates({
deletedAnnotations: deletions,
});
return deletions;
}
describe('receiveRealTimeUpdates', () => {
it("adds pending updates where the focused group matches the annotation's group", () => {
addPendingUpdates(store);
assert.deepEqual(store.pendingUpdates(), {
'updated-ann': { id: 'updated-ann', group: 'group-1' },
'created-ann': { id: 'created-ann', group: 'group-1' },
});
});
it("does not add pending updates if the focused group does not match the annotation's group", () => {
fakeFocusedGroupId.returns('other-group');
addPendingUpdates(store);
assert.deepEqual(store.pendingUpdates(), {});
});
it('always adds pending updates in the stream where there is no focused group', () => {
fakeFocusedGroupId.returns(null);
fakeIsSidebar.returns(false);
addPendingUpdates(store);
assert.deepEqual(store.pendingUpdates(), {
'updated-ann': { id: 'updated-ann', group: 'group-1' },
'created-ann': { id: 'created-ann', group: 'group-1' },
});
});
it('adds pending deletions if the annotation exists locally', () => {
fakeAnnotationExists.returns(true);
addPendingDeletions(store);
assert.deepEqual(store.pendingDeletions(), {
'deleted-ann': true,
});
});
it('does not add pending deletions if the annotation does not exist locally', () => {
fakeAnnotationExists.returns(false);
addPendingDeletions(store);
assert.deepEqual(store.pendingDeletions(), {});
});
});
describe('clearPendingUpdates', () => {
it('clears pending updates', () => {
addPendingUpdates(store);
store.clearPendingUpdates();
assert.deepEqual(store.pendingUpdates(), {});
});
it('clears pending deletions', () => {
addPendingDeletions(store);
store.clearPendingUpdates();
assert.deepEqual(store.pendingDeletions(), {});
});
});
describe('pendingUpdateCount', () => {
it('returns the total number of pending updates', () => {
const updates = addPendingUpdates(store);
const deletes = addPendingDeletions(store);
assert.deepEqual(
store.pendingUpdateCount(),
updates.length + deletes.length
);
});
});
it('clears pending updates when annotations are added/updated', () => {
const updates = addPendingUpdates(store);
// Dispatch `ADD_ANNOTATIONS` directly here rather than using
// the `addAnnotations` action creator because that has side effects.
store.dispatch({ type: 'ADD_ANNOTATIONS', annotations: updates });
assert.deepEqual(store.pendingUpdateCount(), 0);
});
it('clears pending updates when annotations are removed', () => {
const updates = addPendingUpdates(store);
const deletions = addPendingDeletions(store);
store.dispatch(removeAnnotations([...updates, ...deletions]));
assert.equal(store.pendingUpdateCount(), 0);
});
it('clears pending updates when focused group changes', () => {
addPendingUpdates(store);
addPendingDeletions(store);
store.dispatch(focusGroup('123'));
assert.deepEqual(store.pendingUpdateCount(), 0);
});
describe('hasPendingDeletion', () => {
it('returns false if there are no pending deletions', () => {
assert.equal(store.hasPendingDeletion('deleted-ann'), false);
});
it('returns true if there are pending deletions', () => {
addPendingDeletions(store);
assert.equal(store.hasPendingDeletion('deleted-ann'), true);
});
});
});
...@@ -6,9 +6,7 @@ ...@@ -6,9 +6,7 @@
on-logout="vm.logout()" on-logout="vm.logout()"
on-share-page="vm.share()" on-share-page="vm.share()"
on-show-help-panel="vm.showHelpPanel()" on-show-help-panel="vm.showHelpPanel()"
is-sidebar="::vm.isSidebar" is-sidebar="::vm.isSidebar">
pending-update-count="vm.countPendingUpdates()"
on-apply-pending-updates="vm.applyPendingUpdates()">
</top-bar> </top-bar>
<div class="content"> <div class="content">
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment