Commit 9479612f authored by Robert Knight's avatar Robert Knight

Fix real-time update action processing

When store modules were refactored to only receive their local state in
selectors, the code in `store/modules/real-time-updates` which directly
uses selectors from other modules rather than calling
`store.<selectorMethod>(...)` was not updated to pass in the appropriate
part of the state.

Fixing this resolves two problems:

 - An error when processing notifications of deleted annotations from
   the server. These notifications are currently not getting delivered
   due to a server-side issue (see https://github.com/hypothesis/h/pull/6214).

 - Filtering notifications of new or updated annotations not excluding
   annotations from groups that are not currently focused.

   Fixing this second issue has the side effect that it _appears_ to fix
   https://github.com/hypothesis/support/issues/137 from the user's
   point of view. I say _appears to_ because the backend is still
   incorrectly delivering notifications that the user shouldn't be able
   to see. That is fixed by https://github.com/hypothesis/h/pull/6202
parent 235c6b3f
...@@ -98,9 +98,12 @@ function receiveRealTimeUpdates({ ...@@ -98,9 +98,12 @@ function receiveRealTimeUpdates({
// focused group, since we only display annotations from the focused // focused group, since we only display annotations from the focused
// group and reload all annotations and discard pending updates // group and reload all annotations and discard pending updates
// when switching groups. // when switching groups.
const groupState = getState().groups;
const routeState = getState().route;
if ( if (
ann.group === groups.selectors.focusedGroupId(getState()) || ann.group === groups.selectors.focusedGroupId(groupState) ||
route.selectors.route(getState()) !== 'sidebar' route.selectors.route(routeState) !== 'sidebar'
) { ) {
pendingUpdates[/** @type {string} */ (ann.id)] = ann; pendingUpdates[/** @type {string} */ (ann.id)] = ann;
} }
...@@ -116,7 +119,8 @@ function receiveRealTimeUpdates({ ...@@ -116,7 +119,8 @@ function receiveRealTimeUpdates({
// even if the annotation is from the current group, it might be for a // even if the annotation is from the current group, it might be for a
// new annotation (saved in pendingUpdates and removed above), that has // new annotation (saved in pendingUpdates and removed above), that has
// not yet been loaded. // not yet been loaded.
if (annotations.selectors.annotationExists(getState(), id)) { const annotationsState = getState().annotations;
if (annotations.selectors.annotationExists(annotationsState, id)) {
pendingDeletions[id] = true; pendingDeletions[id] = true;
} }
}); });
......
...@@ -16,9 +16,20 @@ describe('sidebar/store/modules/real-time-updates', () => { ...@@ -16,9 +16,20 @@ describe('sidebar/store/modules/real-time-updates', () => {
let store; let store;
beforeEach(() => { beforeEach(() => {
fakeAnnotationExists = sinon.stub().returns(true); fakeAnnotationExists = sinon.stub().callsFake(state => {
fakeFocusedGroupId = sinon.stub().returns('group-1'); assert.equal(state, store.getState().annotations);
fakeRoute = sinon.stub().returns('sidebar'); return true;
});
fakeFocusedGroupId = sinon.stub().callsFake(state => {
assert.equal(state, store.getState().groups);
return 'group-1';
});
fakeRoute = sinon.stub().callsFake(state => {
assert.equal(state, store.getState().route);
return 'sidebar';
});
store = createStore( store = createStore(
[realTimeUpdates, annotations, selection], [realTimeUpdates, annotations, selection],
......
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