Commit 0d0e47ea authored by Robert Knight's avatar Robert Knight

Avoid redundant view reloads on startup and simplify group focus change handling

 * Remove the logic from app-controller.coffee to focus the Public group
   on logout. This is no longer needed as the 'groups' service automatically
   updates the focused group if the previously focused group is removed.

 * Only emit the SESSION_CHANGED event on updates to the session state,
   not the initial app load, since initialization of the view is deferred
   until the session has already been loaded.

   Since the SESSION_CHANGED handler currently reloads the whole view,
   this avoids reloading the view twice on startup.

   Refactoring handling of SESSION_CHANGED to only update the relevant
   app state is left for a future refactoring.

T-105
parent 2660ef5d
...@@ -42,8 +42,8 @@ module.exports = class AppController ...@@ -42,8 +42,8 @@ module.exports = class AppController
# Reload the view when the focused group changes or the # Reload the view when the focused group changes or the
# list of groups that the user is a member of changes # list of groups that the user is a member of changes
groupChangeEvents = [events.SESSION_CHANGED, events.GROUP_FOCUSED]; reloadEvents = [events.SESSION_CHANGED, events.GROUP_FOCUSED];
groupChangeEvents.forEach((eventName) -> reloadEvents.forEach((eventName) ->
$scope.$on(eventName, (event) -> $scope.$on(eventName, (event) ->
$route.reload() $route.reload()
) )
...@@ -51,14 +51,7 @@ module.exports = class AppController ...@@ -51,14 +51,7 @@ module.exports = class AppController
identity.watch({ identity.watch({
onlogin: (identity) -> $scope.auth.user = auth.userid(identity) onlogin: (identity) -> $scope.auth.user = auth.userid(identity)
onlogout: -> onlogout: -> $scope.auth.user = null
$scope.auth.user = null
# Currently all groups are private so when the user logs out they can
# no longer see the annotations from any group they may have had
# focused. Focus the public group instead, so that they see any public
# annotations in the sidebar.
groups.focus('__world__')
onready: -> $scope.auth.user ?= null onready: -> $scope.auth.user ?= null
}) })
...@@ -84,10 +77,6 @@ module.exports = class AppController ...@@ -84,10 +77,6 @@ module.exports = class AppController
else else
$scope.accountDialog.visible = false $scope.accountDialog.visible = false
# Reload the view if this is not the initial load.
if oldVal isnt undefined
$route.reload()
$scope.login = -> $scope.login = ->
$scope.accountDialog.visible = true $scope.accountDialog.visible = true
identity.request({ identity.request({
......
...@@ -4,6 +4,10 @@ ...@@ -4,6 +4,10 @@
*/ */
module.exports = { module.exports = {
/** Broadcast when the currently selected group changes */
GROUP_FOCUSED: 'groupFocused', GROUP_FOCUSED: 'groupFocused',
/** Broadcast when the session state is updated.
* This event is NOT broadcast after the initial session load.
*/
SESSION_CHANGED: 'sessionChanged', SESSION_CHANGED: 'sessionChanged',
}; };
...@@ -115,6 +115,8 @@ function session($document, $http, $resource, $rootScope, flash) { ...@@ -115,6 +115,8 @@ function session($document, $http, $resource, $rootScope, flash) {
* when new state has been pushed to it by the server. * when new state has been pushed to it by the server.
*/ */
resource.update = function (model) { resource.update = function (model) {
var isInitialLoad = !resource.state.csrf;
// Copy the model data (including the CSRF token) into `resource.state`. // Copy the model data (including the CSRF token) into `resource.state`.
angular.copy(model, resource.state); angular.copy(model, resource.state);
...@@ -122,7 +124,9 @@ function session($document, $http, $resource, $rootScope, flash) { ...@@ -122,7 +124,9 @@ function session($document, $http, $resource, $rootScope, flash) {
lastLoad = {$promise: Promise.resolve(model), $resolved: true}; lastLoad = {$promise: Promise.resolve(model), $resolved: true};
lastLoadTime = Date.now(); lastLoadTime = Date.now();
if (!isInitialLoad) {
$rootScope.$broadcast(events.SESSION_CHANGED); $rootScope.$broadcast(events.SESSION_CHANGED);
}
// Return the model // Return the model
return model; return model;
......
...@@ -73,7 +73,7 @@ function connect($websocket, annotationMapper, groups, session) { ...@@ -73,7 +73,7 @@ function connect($websocket, annotationMapper, groups, session) {
// Listen for updates // Listen for updates
socket.onMessage(function (event) { socket.onMessage(function (event) {
message = JSON.parse(event.data) message = JSON.parse(event.data);
if (!message) { if (!message) {
return; return;
} }
......
...@@ -124,12 +124,6 @@ describe 'AppController', -> ...@@ -124,12 +124,6 @@ describe 'AppController', ->
onlogout() onlogout()
assert.strictEqual($scope.auth.user, null) assert.strictEqual($scope.auth.user, null)
it 'focuses the default group in logout', ->
createController()
{onlogout} = fakeIdentity.watch.args[0][0]
onlogout()
assert.calledWith(fakeGroups.focus, '__world__')
it 'does not show login form for logged in users', -> it 'does not show login form for logged in users', ->
createController() createController()
assert.isFalse($scope.accountDialog.visible) assert.isFalse($scope.accountDialog.visible)
......
...@@ -160,11 +160,23 @@ describe('h:session', function () { ...@@ -160,11 +160,23 @@ describe('h:session', function () {
describe('#update()', function () { describe('#update()', function () {
it('broadcasts an event when the session is updated', function () { it('broadcasts an event when the session is updated', function () {
var sessionChangeCallback = sinon.stub(); var sessionChangeCallback = sinon.stub();
// the initial load should not trigger a SESSION_CHANGED event
$rootScope.$on(events.SESSION_CHANGED, sessionChangeCallback); $rootScope.$on(events.SESSION_CHANGED, sessionChangeCallback);
session.update({ session.update({
groups: [{ groups: [{
id: 'groupid' id: 'groupid'
}] }],
csrf: 'dummytoken',
});
// subsequent loads should trigger a SESSION_CHANGED event
assert.isFalse(sessionChangeCallback.called);
session.update({
groups: [{
id: 'groupid2'
}],
csrf: 'dummytoken'
}); });
assert.calledOnce(sessionChangeCallback); assert.calledOnce(sessionChangeCallback);
}); });
......
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