Commit 233450db authored by Robert Knight's avatar Robert Knight

Only wait for document URI before fetching groups in sidebar

Fixes a regression after 36c46608 where the stream and single
annotation page views failed to load.

36c46608 made the groups service wait for the host page's document URI
to be determined before making a call to the `/api/groups` endpoint
passing that URI as a parameter. In the stream (`/stream`) and single
annotation page routes, there is no host page and so
the promise returned by `getDocumentUriForGroupSearch` never resolved.
Avoid waiting for this URI in that case and avoid passing the
`document_uri` parameter to the `/api/groups` endpoint.

This commit introduces a new `isSidebar` test rather than using
`annotationUI.isSidebar` because that piece of app state is not yet
initialized at the point when `groups.load()` is called in the route's
`resolve` function in src/sidebar/index.js.
parent 607a2023
......@@ -17,7 +17,7 @@ var events = require('./events');
var { awaitStateChange } = require('./util/state-util');
// @ngInject
function groups(annotationUI, localStorage, serviceUrl, session, $rootScope, store) {
function groups(annotationUI, isSidebar, localStorage, serviceUrl, session, $rootScope, store) {
// The currently focused group. This is the group that's shown as selected in
// the groups dropdown, the annotations displayed are filtered to only ones
// that belong to this group, and any new annotations that the user creates
......@@ -48,8 +48,16 @@ function groups(annotationUI, localStorage, serviceUrl, session, $rootScope, sto
* the attached frames.
*/
function load() {
return getDocumentUriForGroupSearch().then(uri => {
return store.groups.list({ document_uri: uri });
var uri = Promise.resolve(null);
if (isSidebar) {
uri = getDocumentUriForGroupSearch();
}
return uri.then(uri => {
var params = {};
if (uri) {
params.document_uri = uri;
}
return store.groups.list(params);
}).then(gs => {
$rootScope.$apply(() => {
var focGroup = focused();
......
......@@ -61,6 +61,9 @@ var resolve = {
},
};
var isSidebar = !(window.location.pathname.startsWith('/stream') ||
window.location.pathname.startsWith('/a/'));
// @ngInject
function configureLocation($locationProvider) {
// Use HTML5 history
......@@ -211,6 +214,7 @@ module.exports = angular.module('h', [
.value('ExcerptOverflowMonitor', require('./util/excerpt-overflow-monitor'))
.value('OAuthClient', require('./util/oauth-client'))
.value('VirtualThreadList', require('./virtual-thread-list'))
.value('isSidebar', isSidebar)
.value('random', require('./util/random'))
.value('raven', require('./raven'))
.value('serviceConfig', serviceConfig)
......
......@@ -10,7 +10,10 @@ var util = require('./util');
function init() {
return {
// Flag that indicates whether the app is the sidebar and connected to
// a page where annotations are being shown in context
// a page where annotations are being shown in context.
//
// Note that this flag is not available early in the lifecycle of the
// application.
isSidebar: true,
visibleHighlights: false,
......
'use strict';
var events = require('../events');
var fakeReduxStore = require('./fake-redux-store');
var groups = require('../groups');
var unroll = require('../../shared/test/util').unroll;
......@@ -13,6 +14,7 @@ var sessionWithThreeGroups = function() {
describe('groups', function() {
var fakeAnnotationUI;
var fakeIsSidebar;
var fakeSession;
var fakeStore;
var fakeLocalStorage;
......@@ -23,10 +25,15 @@ describe('groups', function() {
beforeEach(function() {
sandbox = sinon.sandbox.create();
fakeAnnotationUI = {
searchUris: sinon.stub().returns(['http://example.org']),
};
fakeAnnotationUI = fakeReduxStore({
searchUris: ['http://example.org'],
},{
searchUris() {
return this.getState().searchUris;
},
});
fakeSession = sessionWithThreeGroups();
fakeIsSidebar = true;
fakeLocalStorage = {
getItem: sandbox.stub(),
setItem: sandbox.stub(),
......@@ -68,7 +75,7 @@ describe('groups', function() {
});
function service() {
return groups(fakeAnnotationUI, fakeLocalStorage, fakeServiceUrl, fakeSession,
return groups(fakeAnnotationUI, fakeIsSidebar, fakeLocalStorage, fakeServiceUrl, fakeSession,
fakeRootScope, fakeStore);
}
......@@ -120,6 +127,34 @@ describe('groups', function() {
assert.equal(svc.focused().id, 'id3');
});
});
context('in the sidebar', () => {
it('waits for the document URL to be determined', () => {
var svc = service();
fakeAnnotationUI.setState({ searchUris: [] });
var loaded = svc.load();
fakeAnnotationUI.setState({ searchUris: ['https://asite.com'] });
return loaded.then(() => {
assert.calledWith(fakeStore.groups.list, { document_uri: 'https://asite.com' });
});
});
});
context('in the stream and single annotation page', () => {
beforeEach(() => {
fakeIsSidebar = false;
});
it('does not wait for the document URL', () => {
fakeAnnotationUI.setState({ searchUris: [] });
var svc = service();
return svc.load().then(() => {
assert.calledWith(fakeStore.groups.list, {});
});
});
});
});
describe('#get() method', function() {
......
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