Skip to content

Commit

Permalink
Fix app initialization race when issuing annotation search query
Browse files Browse the repository at this point in the history
If the initial annotation search query was constructed
before the user's session state had been retrieved,
the groups list was empty and groups.focused() returned null
in WidgetController._loadAnnotationsFrom

This commit resolves the issue by adding a 'groups.ready()'
function which returns a promise once the groups list is ready.

It currently just returns the session.load().$promise.

Fixes #2590
  • Loading branch information
robertknight committed Oct 7, 2015
1 parent 227efd6 commit 7326a1e
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 16 deletions.
8 changes: 8 additions & 0 deletions h/static/scripts/groups.js
Expand Up @@ -31,7 +31,15 @@ function groups(localStorage, session, $rootScope, features) {
}
};

/** Returns a promise which resolves once the list of groups
* has been loaded.
*/
function ready() {
return session.load().$promise;
}

return {
ready: ready,
all: all,
get: get,

Expand Down
18 changes: 17 additions & 1 deletion h/static/scripts/test/widget-controller-test.coffee
@@ -1,5 +1,7 @@
{module, inject} = angular.mock

Promise = require('core-js/library/es6/promise')

describe 'WidgetController', ->
$scope = null
fakeAnnotationMapper = null
Expand Down Expand Up @@ -60,7 +62,8 @@ describe 'WidgetController', ->
}

fakeGroups = {
focused: -> {id: 'foo'}
focused: -> {id: 'foo'},
ready: -> Promise.resolve(),
}

$provide.value 'annotationMapper', fakeAnnotationMapper
Expand Down Expand Up @@ -92,3 +95,16 @@ describe 'WidgetController', ->
assert.calledWith(loadSpy, [40..59])
assert.calledWith(loadSpy, [60..79])
assert.calledWith(loadSpy, [80..99])

it.only 'should defer loading annotations until groups service is ready', ->
ready = null
groupsReady = new Promise((resolve) -> ready = resolve)
fakeGroups.ready = -> groupsReady

fakeCrossFrame.frames.push({uri: 'http://example.com'})
$scope.$digest()
assert.callCount(fakeAnnotationMapper.loadAnnotations, 0)
ready()
groupsReady.then(->
assert.callCount(fakeAnnotationMapper.loadAnnotations, 1)
)
34 changes: 19 additions & 15 deletions h/static/scripts/widget-controller.coffee
Expand Up @@ -17,21 +17,25 @@ module.exports = class WidgetController
loaded = []

_loadAnnotationsFrom = (query, offset) =>
queryCore =
limit: @chunkSize
offset: offset
sort: 'created'
order: 'asc'
group: groups.focused().id
q = angular.extend(queryCore, query)

store.SearchResource.get q, (results) ->
total = results.total
offset += results.rows.length
if offset < total
_loadAnnotationsFrom query, offset

annotationMapper.loadAnnotations(results.rows)
groups.ready().then( ->
queryCore =
limit: @chunkSize
offset: offset
sort: 'created'
order: 'asc'
group: groups.focused().id
q = angular.extend(queryCore, query)

store.SearchResource.get q, (results) ->
total = results.total
offset += results.rows.length
if offset < total
_loadAnnotationsFrom query, offset

annotationMapper.loadAnnotations(results.rows)
).catch((e) ->
throw e
)

loadAnnotations = (frames) ->
for f in frames
Expand Down

0 comments on commit 7326a1e

Please sign in to comment.