Commit 25358a00 authored by Robert Knight's avatar Robert Knight

Initialize route state in store before fetching groups

The behavior of `GroupsService.load` varies depending on whether the current
route is the sidebar or not. During sidebar startup, `GroupsService.load` was
called before `RouterService.sync`. As a result the route was `null` during the
initial call to `GroupsService._loadGroupsForUserAndDocument` and hence the
service did not execute the sidebar-specific code path to wait for the document
URI to become known at that point.

During the first `GroupsService._loadGroupsForUserAndDocument` call,
`_setupAutoReload` would set up a watcher that would react to changes in
`store.mainURI()`. This watcher would fire after the main guest frame connects.
Depending on when that happens, requests from the initial `GroupsService.load`
request could still be in-flight. If the second set of groups API calls
completed first, followed by the first set of groups API calls, then the final
set of loaded groups might reflect the _first_ call to `GroupsService.load`,
where the document URI was not used (because the route was unknown).

This commit fixes the issue by re-arranging the sidebar startup sequence to
initialize the router service before other services.

Fixes https://github.com/hypothesis/support/issues/79
parent 6a6e3ce7
...@@ -64,6 +64,8 @@ if (configFromSidebar.sentry && envOk) { ...@@ -64,6 +64,8 @@ if (configFromSidebar.sentry && envOk) {
disableOpenerForExternalLinks(document.body); disableOpenerForExternalLinks(document.body);
/** /**
* Configure the Hypothesis API client.
*
* @inject * @inject
*/ */
function setupApi(api: APIService, streamer: StreamerService) { function setupApi(api: APIService, streamer: StreamerService) {
...@@ -71,19 +73,22 @@ function setupApi(api: APIService, streamer: StreamerService) { ...@@ -71,19 +73,22 @@ function setupApi(api: APIService, streamer: StreamerService) {
} }
/** /**
* Perform the initial fetch of groups and user profile and then set the initial * Update the route in the store based on the initial URL.
* route to match the current URL.
* *
* @inject * @inject
*/ */
function setupRoute( function syncRoute(router: RouterService) {
groups: GroupsService, router.sync();
session: SessionService, }
router: RouterService,
) { /**
* Perform the initial fetch of groups and user profile.
*
* @inject
*/
function loadGroupsAndProfile(groups: GroupsService, session: SessionService) {
groups.load(); groups.load();
session.load(); session.load();
router.sync();
} }
/** /**
...@@ -170,9 +175,14 @@ function startApp(settings: SidebarSettings, appEl: HTMLElement) { ...@@ -170,9 +175,14 @@ function startApp(settings: SidebarSettings, appEl: HTMLElement) {
.register('settings', { value: settings }); .register('settings', { value: settings });
// Initialize services. // Initialize services.
//
// We sync the route with the initial URL as the first step, because
// initialization of other services may depend on the route (eg. enabling
// sidebar-only behavior).
container.run(syncRoute);
container.run(initServices); container.run(initServices);
container.run(setupApi); container.run(setupApi);
container.run(setupRoute); container.run(loadGroupsAndProfile);
container.run(startRPCServer); container.run(startRPCServer);
container.run(setupFrameSync); container.run(setupFrameSync);
......
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