opendev/storyboard-webclient
Code Issues Proposed changes

Error message & notification handling

Browse Source 
This patch adds the notification module, a mostly-self contained way
of surfacing errors, warnings and notifications to the user. These
can either be initiated by the application, or can be received from
the server. Among other things, the impact of this will be that
the user will be notified if one of our HTTP requests fails. This
fixes the situation where storyboard would 'fail silently' when
someone is trying to accomplish something.

- New notification module.
- Notifications directive, for a list of all notifications that
made it past filters.
- New Notification service that handles all error broadcasts in the
system.
- New severity directive.
- HTTP Error broadcaster now uses the notification service.
- Session now uses Notification service to handle 401 errors.
- Added new notification filters to strip out 200's and template load
requests.

Change-Id: I4aaa50404560d1c1f14f639e3ac68b2ac2d4380c
This commit is contained in:
Michael Krotscheck 2014-06-11 11:21:06 -07:00
parent 03437e26ca
commit d17fe0362c
16 changed files with 531 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
src/app/auth/service/session.js
View File

@ -20,7 +20,7 @@
*/
angular.module('sb.auth').factory('Session',
function (SessionState, AccessToken, $rootScope, $log, $q, $state, User,
RefreshManager) {
RefreshManager, Notification) {
'use strict';
/**
@ -68,10 +68,10 @@ angular.module('sb.auth').factory('Session',
* validate a token after a long break in using StoryBoard.
* Even if refresh is not necessary right now the tryRefresh method
* will just resolve immediately.
*/
*/
var deferred = $q.defer();
RefreshManager.tryRefresh().then(function() {
RefreshManager.tryRefresh().then(function () {
var id = AccessToken.getIdToken();
User.read({id: id},
@ -109,16 +109,21 @@ angular.module('sb.auth').factory('Session',
*/
initializeSession();
// If we ever encounter a 401 error, make sure the session is destroyed.
$rootScope.$on('http_401', function ($log) {
RefreshManager.tryRefresh().then(
function () {
$log.info('Token refreshsed on 401');
}, function() {
$log.info('Could not refresh token. Destroying session');
destroySession();
});
});
// We're using -1 as the priority, to ensure that this is intercepted
// before anything else happens.
Notification.intercept(function (message) {
if (message.type === 'http' && message.message === 401) {
RefreshManager.tryRefresh().then(
function () {
$log.info('Token refreshed on 401');
}, function () {
$log.info('Could not refresh token. ' +
'Destroying session');
destroySession();
});
return true; // Stop processing this notification.
}
}, -1);
// Expose the methods for this service.
return {

69
src/app/notification/controller/notifications_controller.js Normal file
View File

@ -0,0 +1,69 @@
/*
* Copyright (c) 2014 Hewlett-Packard Development Company, L.P.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License. You may obtain
* a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*/
/**
* This module acts as the central routing point for all errors that occur
* within storyboard.
*/
angular.module('sb.notification').controller('NotificationsController',
function ($scope, Notification) {
'use strict';
var defaultDisplayCount = 5;
$scope.displayCount = defaultDisplayCount;
$scope.notifications = [];
/**
* Remove a notification from the display list.
*
* @param notification
*/
$scope.remove = function (notification) {
var idx = $scope.notifications.indexOf(notification);
if (idx > -1) {
$scope.notifications.splice(idx, 1);
}
// If the notification list length drops below default, make
// sure we reset the limit.
if ($scope.notifications.length <= defaultDisplayCount) {
$scope.displayCount = defaultDisplayCount;
}
};
/**
* Reveal more notifications, either current count + 5 or the total
* number of messages, whichever is smaller.
*/
$scope.showMore = function () {
// Set this to something big.
$scope.displayCount = Math.min($scope.notifications.length,
$scope.displayCount + 5);
};
/**
* Set up a notification subscriber, and make sure it's removed when
* the scope is destroyed.
*/
$scope.$on('$destroy', Notification.subscribe(
function (notification) {
$scope.notifications.push(notification);
}
)
);
});

30
src/app/notification/directive/notifications.js Normal file
View File

@ -0,0 +1,30 @@
/*
* Copyright (c) 2014 Hewlett-Packard Development Company, L.P.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License. You may obtain
* a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*/
/**
* This directive is a notification list renderer with all the trimmings.
* Errors broadcast throughout the system will be collected and displayed here.
*/
angular.module('sb.notification').directive('notifications',
function () {
'use strict';
return {
restrict: 'E',
templateUrl: 'app/templates/error/notifications.html',
controller: 'NotificationsController'
};
});

21
src/app/notification/module.js Normal file
View File

@ -0,0 +1,21 @@
/*
* Copyright (c) 2014 Hewlett-Packard Development Company, L.P.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License. You may obtain
* a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*/
/**
* This module acts as the central routing point for all errors that occur
* within storyboard.
*/
angular.module('sb.notification', []);

27
src/app/notification/provider/severity.js Normal file
View File

@ -0,0 +1,27 @@
/*
* Copyright (c) 2014 Hewlett-Packard Development Company, L.P.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License. You may obtain
* a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*/
/**
* A list of severity levels used within this module.
*/
angular.module('sb.notification').constant('Severity', {
ERROR: 'error',
WARNING: 'warning',
INFO: 'info',
SUCCESS: 'success'
});

136
src/app/notification/service/notification_service.js Normal file
View File

@ -0,0 +1,136 @@
/*
* Copyright (c) 2014 Hewlett-Packard Development Company, L.P.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License. You may obtain
* a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*/
/**
* The centralized notification service, aka the central routing point for all
* broadcast notifications. You use it by registering interceptors and
* subscribers, and handling any messages that are sent().
*
* Interceptors are intended to be both filters and decorators, where
* individual components can handle messages before either terminating
* the dispatch chain, or passing them on to the next interceptor. In this
* fashion it is easy to handle specific messages in one context while
* other messages in another.
*
* Subscribers are processors that handle all messages vetted by our
* interceptors.
*/
angular.module('sb.notification').factory('Notification',
function ($log, Severity) {
'use strict';
var subscribers = [];
var interceptors = [];
return {
/**
* Send a notification.
*
* @param type A type identifier, such as a string. Use this for
* your subscribers to determine what kind of a message you're
* working with.
* @param message A human readable message for this notification.
* @param severity The severity of this message, any of the
* constants provided in Severity.
* @param cause The cause of this message, perhaps a large amount
* of debugging information.
* @param callback If this message prompts the user to do
* something, then pass a function here and it'll be rendered
* in the message.
* @param callbackLabel A custom label for the callback.
*/
send: function (type, message, severity, cause, callback,
callbackLabel) {
// Return the type.
if (!type || !message) {
$log.warn('Invoked Notification.send() without a type' +
' or message.');
return;
}
// sanitize our data.
var n = {
'type': type,
'message': message,
'severity': severity || Severity.INFO,
'cause': cause || null,
'callback': callback || null,
'callbackLabel': callbackLabel || null,
'date': new Date()
};
// Iterate through the interceptors.
for (var i = 0; i < interceptors.length; i++) {
if (!!interceptors[i].method(n)) {
return;
}
}
// Iterate through the subscribers.
for (var j = 0; j < subscribers.length; j++) {
subscribers[j](n);
}
},
/**
* Add a message interceptor to the notification system, in order
* to determine which messages you'd like to handle.
*
* @param interceptor A method that accepts a notification. You can
* return true from the interceptor method to indicate that this
* message has been handled and should not be processed further.
* @param priority An optional priority (default 999).
* Interceptors with a lower priority will go first.
* @returns {Function} A method that may be called to remove the
* interceptor at a later time.
*/
intercept: function (interceptor, priority) {
var i = {
'priority': priority || 999,
'method': interceptor
};
// Add and sort the interceptors. We're using unshift here so
// that the sorting algorithm ends up being a single-pass
// bubble sort.
interceptors.unshift(i);
interceptors.sort(function (a, b) {
return a.priority - b.priority;
});
return function () {
interceptors.remove(i);
};
},
/**
* Subscribe to all messages that make it through our interceptors.
*
* @param subscriber A subscriber method that receives a
* notification.
* @returns {Function} A method that may be called to remove the
* subscriber at a later time.
*/
subscribe: function (subscriber) {
subscribers.push(subscriber);
return function () {
subscribers.remove(subscriber);
};
}
};
});

19
src/app/services/http/http_error_broadcaster.js
View File

@ -16,8 +16,8 @@
/**
* An HTTP request interceptor that broadcasts response status codes to the
* rest of the application as events. These events are broadcast before the
* error response itself is passed back to the receiving closure, so please
* rest of the application as notifications. These events are broadcast before
* the error response itself is passed back to the receiving closure, so please
* keep that in mind as you base your application logic on it.
*
* @author Michael Krotscheck
@ -25,24 +25,24 @@
angular.module('sb.services')
// Create an HTTP Error Broadcaster that intercepts requests and lets the
// rest of the application know about what happened.
.factory('httpErrorBroadcaster', function ($q, $rootScope) {
.factory('httpErrorBroadcaster',
function ($q, $rootScope, Notification, Severity) {
'use strict';
function sendEvent(status, body) {
function sendEvent(severity, response) {
// Only send an event if a status is passed.
if (!!status) {
$rootScope.$broadcast('http_' + status, body || {});
if (!!response.status) {
Notification.send('http', response.status, severity, response);
}
}
return {
/**
* Handle a success response.
*/
response: function (response) {
if (!!response) {
sendEvent(response.status);
sendEvent(Severity.SUCCESS, response);
}
return response;
},
@ -51,9 +51,8 @@ angular.module('sb.services')
* Handle a fail response.
*/
responseError: function (response) {
if (!!response) {
sendEvent(response.status, response.data);
sendEvent(Severity.ERROR, response);
}
return $q.reject(response);

2
src/app/services/module.js
View File

@ -19,4 +19,4 @@
* used by the storyboard client. Its resources are available via injection to
* any module that declares it as a dependency.
*/
angular.module('sb.services', ['ngResource']);
angular.module('sb.services', ['ngResource', 'sb.notification']);

92
src/app/services/notification/http_message_filter.js Normal file
View File

@ -0,0 +1,92 @@
/*
* Copyright (c) 2014 Hewlett-Packard Development Company, L.P.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License. You may obtain
* a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*/
/**
* Notification interceptors for this library.
*/
angular.module('sb.services')
.run(function (Notification) {
'use strict';
/**
* Template load requests are done via $http, so we need to filter
* those out first.
*/
function filterTemplateRequests(message) {
if (message.type !== 'http') {
return;
}
var request = message.cause;
var url = request.config.url;
if (url.substr(-5) === '.html') {
return true;
}
}
/**
* A notification interceptor that filters successful HTTP requests.
* It's registered at priority 999 (the lowest) so that other
* interceptors can get access to this message first (ex: statistics).
*/
function filterSuccessful(message) {
var response = message.cause;
if (message.type !== 'http' || !response) {
return;
}
// All 200 requests are filtered out.
if (response.status === 200) {
return true;
}
}
/**
* A notification interceptor that rewrites HTTP status codes to
* human readable messages.
*/
function rewriteHttpStatus(message) {
if (message.type !== 'http') {
// Do nothing.
return;
}
var httpStatus = message.message;
var request = message.cause;
if (!httpStatus || !request || !request.data) {
return;
}
var data = request.data;
var method = request.config.method;
var url = request.config.url;
message.message = httpStatus + ': ' + method + ' ' + url + ': ';
if (data.hasOwnProperty('faultstring')) {
message.message += data.faultstring;
} else {
message.message += 'No error details available.';
}
}
// Apply the interceptors.
Notification.intercept(filterTemplateRequests, -1);
Notification.intercept(filterSuccessful, 999);
Notification.intercept(rewriteHttpStatus, 1000);
});

4
src/app/storyboard/module.js
View File

@ -24,8 +24,8 @@
*/
angular.module('storyboard',
[ 'sb.services', 'sb.templates', 'sb.dashboard', 'sb.pages', 'sb.projects',
'sb.auth', 'sb.story', 'sb.profile', 'ui.router', 'ui.bootstrap',
'monospaced.elastic'])
'sb.auth', 'sb.story', 'sb.profile', 'sb.notification', 'ui.router',
'ui.bootstrap', 'monospaced.elastic'])
.config(function ($provide, $urlRouterProvider, $locationProvider,
$httpProvider, msdElasticConfig) {
'use strict';

44
src/app/templates/error/notifications.html Normal file
View File

@ -0,0 +1,44 @@
<div class="notification-container">
<div ng-repeat="n in notifications | limitTo: displayCount"
ng-switch="n.severity">
<div ng-switch-when="error"
class="alert alert-danger">
<div ng-include
src="'/inline/notification_detail.html'"></div>
</div>
<div ng-switch-when="warning"
class="alert alert-warning">
<div ng-include
src="'/inline/notification_detail.html'"></div>
</div>
<div ng-switch-when="info"
class="alert alert-info">
<div ng-include
src="'/inline/notification_detail.html'"></div>
</div>
<div ng-switch-when="success"
class="alert alert-success">
<div ng-include
src="'/inline/notification_detail.html'"></div>
</div>
</div>
<div ng-show="notifications.length &gt; displayCount"
class="alert alert-info">
Showing {{displayCount}} of {{notifications.length}}
messages.
<a href=""
class="alert-link pull-right"
ng-click="showMore()">
<i class="fa fa-arrow-circle-down"></i>
Show More
</a>
</div>
</div>
<script type="text/ng-template" id="/inline/notification_detail.html">
<button type="button" class="close" ng-click="remove(n)">
&times;
</button>
{{n.message}}
</script>

9
src/index.html
View File

@ -47,9 +47,10 @@
<script src="js/templates.js"></script>
</head>
<body ng-class="{'logged-in' : isLoggedIn}">
<header ng-include src="'app/templates/header_menu.html'"></header>
<div ng-include src="'app/templates/side_menu.html'"></div>
<div ui-view class="main"></div>
</footer>
<header ng-include src="'app/templates/header_menu.html'"></header>
<div ng-include src="'app/templates/side_menu.html'"></div>
<div ui-view class="main"></div>
<notifications></notifications>
</body>
</html>

51
src/theme/base/notification.less Normal file
View File

@ -0,0 +1,51 @@
/*
* Copyright (c) 2014 Hewlett-Packard Development Company, L.P.
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License. You may obtain
* a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*/
/**
* Styles specific to error display.
*/
.notification-container {
position: absolute;
z-index: 2000;
@media (max-width: @screen-xs-max) {
top: @error-container-margin-vertical;
right: @error-container-margin-horizontal;
left: @error-container-margin-horizontal;
}
@media (min-width: @screen-sm-min) and (max-width: @screen-sm-max) {
top: @error-container-margin-vertical;
right: @error-container-margin-horizontal;
width: @error-container-max-width;
}
@media (min-width: @screen-md-min) and (max-width: @screen-md-max) {
top: @error-container-margin-vertical;
right: @error-container-margin-horizontal;
width: @error-container-max-width;
}
@media (min-width: @screen-lg-min) {
top: @error-container-margin-vertical;
right: @error-container-margin-horizontal;
width: @error-container-max-width;
}
.alert {
box-shadow: 3px 3px 10px rgba(0, 0, 0, .5)
}
}

1
src/theme/main.less
View File

@ -31,6 +31,7 @@
// Add our own custom icon font.
@import './base/custom_font_icons.less';
// Module specific styles
@import './base/notification.less';
@import './base/body.less';
@import './base/logged_in.less';
@import './base/auth.less';

7
src/theme/storyboard/theme.less
View File

@ -29,6 +29,13 @@
@font-size-h5: (@font-size-base);
@font-size-h6: (ceil(@font-size-base * 0.85));
// Error messages
// --------------
@error-container-margin-horizontal: 10px;
@error-container-margin-vertical: 10px;
@error-container-max-width: 400px;
// Navbar
// -------------------------
// Basics of a navbar

36
test/unit/services/http/http_error_broadcaster_test.js
View File

@ -21,7 +21,7 @@
describe('httpErrorBroadcaster', function () {
'use strict';
var $rootScope, $httpBackend, $resource, MockResource;
var notification, $httpBackend, $resource, MockResource;
var errorResponse = {
error_code: 404,
@ -35,31 +35,31 @@ describe('httpErrorBroadcaster', function () {
inject(function ($injector) {
// Capture various providers for later use.
$rootScope = $injector.get('$rootScope');
notification = $injector.get('Notification');
$httpBackend = $injector.get('$httpBackend');
$resource = $injector.get('$resource');
MockResource = $resource('/foo/:id', {id: '@id'});
});
// Start listening to the broadcast method.
spyOn($rootScope, '$broadcast');
});
it('should capture events on the $rootScope', function (done) {
it('should dispatch events to the Notification framework',
function () {
// Start listening to the broadcast method.
spyOn(notification, 'send').and.callFake(
function(type, code, severity) {
expect(type).toEqual('http');
expect(code).toEqual(553);
expect(severity).toEqual('error');
}
);
// Prepare the HTTP Backend
$httpBackend.when('GET', '/foo/99')
.respond(553, JSON.stringify(errorResponse));
// Prepare the HTTP Backend
$httpBackend.when('GET', '/foo/99')
.respond(553, JSON.stringify(errorResponse));
// Handle a result.
function handleResult() {
expect($rootScope.$broadcast)
.toHaveBeenCalledWith('http_553', errorResponse);
done();
}
MockResource.get({'id': 99}, handleResult, handleResult);
$httpBackend.flush();
});
MockResource.get({'id': 99});
$httpBackend.flush();
});
});