Unverified Commit 0c29751a authored by Sheetal Umesh Kumar's avatar Sheetal Umesh Kumar Committed by GitHub

Merge pull request #674 from hypothesis/only-wait-for-doc-uri-in-sidebar

Only wait for document URI before fetching groups in sidebar
parents 607a2023 233450db
...@@ -17,7 +17,7 @@ var events = require('./events'); ...@@ -17,7 +17,7 @@ var events = require('./events');
var { awaitStateChange } = require('./util/state-util'); var { awaitStateChange } = require('./util/state-util');
// @ngInject // @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 currently focused group. This is the group that's shown as selected in
// the groups dropdown, the annotations displayed are filtered to only ones // the groups dropdown, the annotations displayed are filtered to only ones
// that belong to this group, and any new annotations that the user creates // 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 ...@@ -48,8 +48,16 @@ function groups(annotationUI, localStorage, serviceUrl, session, $rootScope, sto
* the attached frames. * the attached frames.
*/ */
function load() { function load() {
return getDocumentUriForGroupSearch().then(uri => { var uri = Promise.resolve(null);
return store.groups.list({ document_uri: uri }); if (isSidebar) {
uri = getDocumentUriForGroupSearch();
}
return uri.then(uri => {
var params = {};
if (uri) {
params.document_uri = uri;
}
return store.groups.list(params);
}).then(gs => { }).then(gs => {
$rootScope.$apply(() => { $rootScope.$apply(() => {
var focGroup = focused(); var focGroup = focused();
......
...@@ -61,6 +61,9 @@ var resolve = { ...@@ -61,6 +61,9 @@ var resolve = {
}, },
}; };
var isSidebar = !(window.location.pathname.startsWith('/stream') ||
window.location.pathname.startsWith('/a/'));
// @ngInject // @ngInject
function configureLocation($locationProvider) { function configureLocation($locationProvider) {
// Use HTML5 history // Use HTML5 history
...@@ -211,6 +214,7 @@ module.exports = angular.module('h', [ ...@@ -211,6 +214,7 @@ module.exports = angular.module('h', [
.value('ExcerptOverflowMonitor', require('./util/excerpt-overflow-monitor')) .value('ExcerptOverflowMonitor', require('./util/excerpt-overflow-monitor'))
.value('OAuthClient', require('./util/oauth-client')) .value('OAuthClient', require('./util/oauth-client'))
.value('VirtualThreadList', require('./virtual-thread-list')) .value('VirtualThreadList', require('./virtual-thread-list'))
.value('isSidebar', isSidebar)
.value('random', require('./util/random')) .value('random', require('./util/random'))
.value('raven', require('./raven')) .value('raven', require('./raven'))
.value('serviceConfig', serviceConfig) .value('serviceConfig', serviceConfig)
......
...@@ -10,7 +10,10 @@ var util = require('./util'); ...@@ -10,7 +10,10 @@ var util = require('./util');
function init() { function init() {
return { return {
// Flag that indicates whether the app is the sidebar and connected to // 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, isSidebar: true,
visibleHighlights: false, visibleHighlights: false,
......
'use strict'; 'use strict';
var events = require('../events'); var events = require('../events');
var fakeReduxStore = require('./fake-redux-store');
var groups = require('../groups'); var groups = require('../groups');
var unroll = require('../../shared/test/util').unroll; var unroll = require('../../shared/test/util').unroll;
...@@ -13,6 +14,7 @@ var sessionWithThreeGroups = function() { ...@@ -13,6 +14,7 @@ var sessionWithThreeGroups = function() {
describe('groups', function() { describe('groups', function() {
var fakeAnnotationUI; var fakeAnnotationUI;
var fakeIsSidebar;
var fakeSession; var fakeSession;
var fakeStore; var fakeStore;
var fakeLocalStorage; var fakeLocalStorage;
...@@ -23,10 +25,15 @@ describe('groups', function() { ...@@ -23,10 +25,15 @@ describe('groups', function() {
beforeEach(function() { beforeEach(function() {
sandbox = sinon.sandbox.create(); sandbox = sinon.sandbox.create();
fakeAnnotationUI = { fakeAnnotationUI = fakeReduxStore({
searchUris: sinon.stub().returns(['http://example.org']), searchUris: ['http://example.org'],
}; },{
searchUris() {
return this.getState().searchUris;
},
});
fakeSession = sessionWithThreeGroups(); fakeSession = sessionWithThreeGroups();
fakeIsSidebar = true;
fakeLocalStorage = { fakeLocalStorage = {
getItem: sandbox.stub(), getItem: sandbox.stub(),
setItem: sandbox.stub(), setItem: sandbox.stub(),
...@@ -68,7 +75,7 @@ describe('groups', function() { ...@@ -68,7 +75,7 @@ describe('groups', function() {
}); });
function service() { function service() {
return groups(fakeAnnotationUI, fakeLocalStorage, fakeServiceUrl, fakeSession, return groups(fakeAnnotationUI, fakeIsSidebar, fakeLocalStorage, fakeServiceUrl, fakeSession,
fakeRootScope, fakeStore); fakeRootScope, fakeStore);
} }
...@@ -120,6 +127,34 @@ describe('groups', function() { ...@@ -120,6 +127,34 @@ describe('groups', function() {
assert.equal(svc.focused().id, 'id3'); 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() { 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