diff --git a/app/assets/stylesheets/layout/_work_package.sass b/app/assets/stylesheets/layout/_work_package.sass index 4226d60ef7..70b3e1f2b0 100644 --- a/app/assets/stylesheets/layout/_work_package.sass +++ b/app/assets/stylesheets/layout/_work_package.sass @@ -32,31 +32,21 @@ #content $flash-margins-padding: 10px - // HACK: workaround to ensure correct height applied to child elements - // the dom looks like this + // HACK: workaround to ensure correct height applied to child elements. + // The dom looks like this: // flash (generated from rails - if it was generated when rendering the page) - // flash (generated from angular - exists always but is hidden with ng-hide if not needed) // div[ui-view] (main content we want to adjust the height for) // div style="clear:both;" (is pushed to overflow - of no relevance) // - // There can be at most two flash messages visible on the page. - // We need to adjust the hight of the div[ui-view] depending on the amount of flash messages. - // // This makes use of more specific rules overwriting less specific ones. // Per default, the height is always 100% - .flash + div[ui-view] + & > div[ui-view] height: 100% - // If there is only one flash message shown: // Subtract the height of the flash message - .flash:not(.ng-hide) ~ div[ui-view] + .flash ~ div[ui-view] height: calc(100% - #{($content-flash-height + $flash-margins-padding)}) - // If there are two flash messages shown: - // Subtract the height of the two flash messages - .flash:not(.ng-hide) ~ .flash:not(.ng-hide) ~ div[ui-view] - height: calc(100% - #{2 * ($content-flash-height + $flash-margins-padding)}) - // HACK: workaround to ensure correct height applied to child elements #work-packages-index height: 100% diff --git a/app/views/layouts/angular.html.erb b/app/views/layouts/angular.html.erb index 06fe86891b..d425084463 100644 --- a/app/views/layouts/angular.html.erb +++ b/app/views/layouts/angular.html.erb @@ -122,7 +122,6 @@ See doc/COPYRIGHT.rdoc for more details. ng-class="{ 'hidden-navigation': !showNavigation }">

<%= l(:label_content) %>

<%= render_flash_messages %> -
<%= call_hook :view_layouts_base_content %>
 
diff --git a/app/views/layouts/base.html.erb b/app/views/layouts/base.html.erb index ee944f7559..c6d306b8ba 100644 --- a/app/views/layouts/base.html.erb +++ b/app/views/layouts/base.html.erb @@ -118,7 +118,7 @@ See doc/COPYRIGHT.rdoc for more details. ng-class="{ 'hidden-navigation': !showNavigation }">

<%= l(:label_content) %>

<%= render_flash_messages %> - + <%= render :partial => 'layouts/action_menu' %> <%= yield %> diff --git a/frontend/app/ui_components/flash-message-directive.js b/frontend/app/services/api-notifications-service.js similarity index 55% rename from frontend/app/ui_components/flash-message-directive.js rename to frontend/app/services/api-notifications-service.js index ab717f3060..da89be84e9 100644 --- a/frontend/app/ui_components/flash-message-directive.js +++ b/frontend/app/services/api-notifications-service.js @@ -26,37 +26,26 @@ // See doc/COPYRIGHT.rdoc for more details. //++ -// TODO move to UI components -module.exports = function($rootScope, $timeout, ConfigurationService) { +module.exports = function(NotificationsService, ApiHelper) { + 'use strict'; - return { - restrict: 'E', - replace: true, - scope: {}, - templateUrl: '/templates/components/flash_message.html', - link: function(scope, element, attrs) { - $rootScope.$on('flashMessage', function(event, message) { - scope.message = message; - scope.flashType = 'notice'; - scope.flashId = 'flash-notice'; + var addError = function(error) { + var messages = ApiHelper.getErrorMessages(error); - var fadeOutTime = attrs.fadeOutTime || 3000; + if (messages.length > 1) { + NotificationsService.addError('', messages); + } + else { + NotificationsService.addError(messages[0]); + } + }; - if (message.isError) { - scope.flashType = "errorExplanation"; - scope.flashId = "errorExplanation"; - } + var addSuccess = function(text) { + NotificationsService.addSuccess(text); + }; - // not using $timeout to allow capybara to not wait until timeout is done with - // scope apply - if (!ConfigurationService.accessibilityModeEnabled() && !message.isPermanent) { - setTimeout(function() { - scope.$apply(function() { - scope.message = undefined; - }); - }, fadeOutTime); - } - }); - } + return { + addError: addError, + addSuccess: addSuccess }; }; diff --git a/frontend/app/services/index.js b/frontend/app/services/index.js index 2c1420b416..114c0be5c5 100644 --- a/frontend/app/services/index.js +++ b/frontend/app/services/index.js @@ -103,6 +103,7 @@ angular.module('openproject.services') 'AuthorisationService', 'EditableFieldsState', 'WorkPackageFieldService', + 'NotificationsService', require('./work-package-service') ]) .service('NotificationsService', [ @@ -110,4 +111,9 @@ angular.module('openproject.services') '$rootScope', require('./notifications-service.js') ]) + .service('ApiNotificationsService', [ + 'NotificationsService', + 'ApiHelper', + require('./api-notifications-service.js') + ]) .service('ConversionService', require('./conversion-service.js')); diff --git a/frontend/app/services/notifications-service.js b/frontend/app/services/notifications-service.js index a9b333cd4a..b21f98070f 100644 --- a/frontend/app/services/notifications-service.js +++ b/frontend/app/services/notifications-service.js @@ -42,12 +42,9 @@ module.exports = function(I18n, $rootScope) { return _.extend(createNotification(message), { type: 'warning' }); }, createErrorNotification = function(message, errors) { - if(!errors) { - throw new Error('Cannot create an error notification without errors!'); - } return _.extend(createNotification(message), { type: 'error', - errors: errors + errors: errors || [] }); }, createWorkPackageUploadNotification = function(message, uploads) { diff --git a/frontend/app/services/work-package-service.js b/frontend/app/services/work-package-service.js index 91143b4505..d72f9246e2 100644 --- a/frontend/app/services/work-package-service.js +++ b/frontend/app/services/work-package-service.js @@ -38,7 +38,8 @@ module.exports = function($http, $q, AuthorisationService, EditableFieldsState, - WorkPackageFieldService + WorkPackageFieldService, + NotificationsService ) { var workPackage; @@ -298,11 +299,9 @@ module.exports = function($http, return response.data; }, function(failedResponse) { - $rootScope.$emit('flashMessage', { - isError: true, - isPermanent: true, - text: I18n.t('js.work_packages.query.errors.unretrievable_query') - }); + NotificationsService.addError( + I18n.t('js.work_packages.query.errors.unretrievable_query') + ); } ); }, @@ -320,18 +319,16 @@ module.exports = function($http, if (defaultHandling) { promise.success(function(data, status) { // TODO wire up to API and process API response - $rootScope.$emit('flashMessage', { - isError: false, - text: I18n.t('js.work_packages.message_successful_bulk_delete') - }); + NotificationsService.addSuccess( + I18n.t('js.work_packages.message_successful_bulk_delete') + ); $rootScope.$emit('workPackagesRefreshRequired'); }) .error(function(data, status) { // TODO wire up to API and processs API response - $rootScope.$emit('flashMessage', { - isError: true, - text: I18n.t('js.work_packages.message_error_during_bulk_delete') - }); + NotificationsService.addError( + I18n.t('js.work_packages.message_error_during_bulk_delete') + ); }); } diff --git a/frontend/app/templates/components/flash_message.html b/frontend/app/templates/components/flash_message.html deleted file mode 100644 index 9940b1f883..0000000000 --- a/frontend/app/templates/components/flash_message.html +++ /dev/null @@ -1,8 +0,0 @@ - diff --git a/frontend/app/time_entries/controllers/index.js b/frontend/app/time_entries/controllers/index.js index 2bea960494..f2ec1e3793 100644 --- a/frontend/app/time_entries/controllers/index.js +++ b/frontend/app/time_entries/controllers/index.js @@ -28,6 +28,6 @@ angular.module('openproject.timeEntries.controllers') .controller('TimeEntriesController', ['$scope', '$http', 'PathHelper', - 'SortService', 'PaginationService', + 'SortService', 'PaginationService', 'NotificationsService', require('./time-entries-controller') ]); diff --git a/frontend/app/time_entries/controllers/time-entries-controller.js b/frontend/app/time_entries/controllers/time-entries-controller.js index a1cf874813..805dfd3f4f 100644 --- a/frontend/app/time_entries/controllers/time-entries-controller.js +++ b/frontend/app/time_entries/controllers/time-entries-controller.js @@ -26,7 +26,7 @@ // See doc/COPYRIGHT.rdoc for more details. //++ -module.exports = function($scope, $http, PathHelper, SortService, PaginationService) { +module.exports = function($scope, $http, PathHelper, SortService, PaginationService, NotificationsService) { $scope.PathHelper = PathHelper; $scope.timeEntries = gon.timeEntries; $scope.totalEntryCount = gon.total_count; @@ -57,11 +57,11 @@ module.exports = function($scope, $http, PathHelper, SortService, PaginationServ $scope.deleteTimeEntry = function(id) { if (window.confirm(I18n.t('js.text_are_you_sure'))) { $http['delete'](PathHelper.timeEntryPath(id)) - .success(function(data, status, headers, config) { + .success(function(data) { var index = 0; for (var i = 0; i < $scope.timeEntries.length; i++) { - if ($scope.timeEntries[i].id == id) { + if ($scope.timeEntries[i].id === id) { index = i; break; } @@ -69,10 +69,10 @@ module.exports = function($scope, $http, PathHelper, SortService, PaginationServ $scope.timeEntries.splice(index, 1); - $scope.$emit('flashMessage', data); + NotificationsService.addSuccess(data.text); }) - .error(function(data, status, headers, config) { - $scope.$emit('flashMessage', data); + .error(function(data) { + NotificationsService.addError(data.text); }); } }; diff --git a/frontend/app/ui_components/index.js b/frontend/app/ui_components/index.js index 3562af24be..a9ec0acfbf 100644 --- a/frontend/app/ui_components/index.js +++ b/frontend/app/ui_components/index.js @@ -49,12 +49,6 @@ angular.module('openproject.uiComponents') .constant('ENTER_KEY', 13) .directive('executeOnEnter', ['ENTER_KEY', require( './execute-on-enter-directive')]) - .directive('flashMessage', [ - '$rootScope', - '$timeout', - 'ConfigurationService', - require('./flash-message-directive') - ]) .directive('expandableSearch', ['ENTER_KEY', require('./expandable-search')]) .directive('focus', ['FocusHelper', require('./focus-directive')]) .constant('FOCUSABLE_SELECTOR', 'a, button, :input, [tabindex], select') diff --git a/frontend/app/work_packages/controllers/dialogs/save.js b/frontend/app/work_packages/controllers/dialogs/save.js index 8bcc1562de..e89e273ccf 100644 --- a/frontend/app/work_packages/controllers/dialogs/save.js +++ b/frontend/app/work_packages/controllers/dialogs/save.js @@ -26,7 +26,14 @@ // See doc/COPYRIGHT.rdoc for more details. //++ -module.exports = function($scope, saveModal, QueryService, AuthorisationService, $state) { +module.exports = function( + $scope, + saveModal, + QueryService, + AuthorisationService, + $state, + NotificationsService + ) { this.name = 'Save'; this.closeMe = saveModal.deactivate; @@ -34,14 +41,23 @@ module.exports = function($scope, saveModal, QueryService, AuthorisationService, $scope.saveQueryAs = function(name) { QueryService.saveQueryAs(name) .then(function(data){ - // push query id to URL without reinitializing work-packages-list-controller - if (data.query) { - $state.go('work-packages.list', { query_id: data.query.id, query: null }, { notify: false }); - AuthorisationService.initModelAuth("query", data.query._links); + + if (data.status.isError){ + NotificationsService.addError(data.status.text); } + else { + // push query id to URL without reinitializing work-packages-list-controller + if (data.query) { + $state.go('work-packages.list', + { query_id: data.query.id, query: null }, + { notify: false }); + AuthorisationService.initModelAuth('query', data.query._links); + } + + saveModal.deactivate(); - saveModal.deactivate(); - $scope.$emit('flashMessage', data.status); + NotificationsService.addSuccess(data.status.text); + } }); }; }; diff --git a/frontend/app/work_packages/controllers/dialogs/settings.js b/frontend/app/work_packages/controllers/dialogs/settings.js index 3003440f5b..be64f65118 100644 --- a/frontend/app/work_packages/controllers/dialogs/settings.js +++ b/frontend/app/work_packages/controllers/dialogs/settings.js @@ -26,7 +26,15 @@ // See doc/COPYRIGHT.rdoc for more details. //++ -module.exports = function($scope, settingsModal, QueryService, AuthorisationService, $rootScope, QUERY_MENU_ITEM_TYPE) { +module.exports = function( + $scope, + settingsModal, + QueryService, + AuthorisationService, + $rootScope, + QUERY_MENU_ITEM_TYPE, + NotificationsService + ) { var query = QueryService.getQuery(); @@ -43,7 +51,7 @@ module.exports = function($scope, settingsModal, QueryService, AuthorisationServ }) .then(function(data) { settingsModal.deactivate(); - $scope.$emit('flashMessage', data.status); + NotificationsService.addSuccess(data.status.text); $rootScope.$broadcast('openproject.layout.renameQueryMenuItem', { itemType: QUERY_MENU_ITEM_TYPE, @@ -52,7 +60,7 @@ module.exports = function($scope, settingsModal, QueryService, AuthorisationServ }); if(data.query) { - AuthorisationService.initModelAuth("query", data.query._links); + AuthorisationService.initModelAuth('query', data.query._links); } }); }; diff --git a/frontend/app/work_packages/controllers/dialogs/share.js b/frontend/app/work_packages/controllers/dialogs/share.js index 2f1207cf91..08944a188f 100644 --- a/frontend/app/work_packages/controllers/dialogs/share.js +++ b/frontend/app/work_packages/controllers/dialogs/share.js @@ -26,7 +26,15 @@ // See doc/COPYRIGHT.rdoc for more details. //++ -module.exports = function($scope, shareModal, QueryService, AuthorisationService, queryMenuItemFactory, PathHelper) { +module.exports = function( + $scope, + shareModal, + QueryService, + AuthorisationService, + queryMenuItemFactory, + PathHelper, + NotificationsService + ) { this.name = 'Share'; this.closeMe = shareModal.deactivate; @@ -38,7 +46,7 @@ module.exports = function($scope, shareModal, QueryService, AuthorisationService function closeAndReport(message) { shareModal.deactivate(); - $scope.$emit('flashMessage', message); + NotificationsService.addSuccess(message.text); } $scope.cannot = AuthorisationService.cannot; @@ -49,11 +57,11 @@ module.exports = function($scope, shareModal, QueryService, AuthorisationService .then(function(data){ messageObject = data.status; if(data.query) { - AuthorisationService.initModelAuth("query", data.query._links); + AuthorisationService.initModelAuth('query', data.query._links); } }) .then(function(data){ - if($scope.query.starred != $scope.shareSettings.starred){ + if($scope.query.starred !== $scope.shareSettings.starred){ QueryService.toggleQueryStarred($scope.query) .then(function(data){ closeAndReport(data.status || messageObject); diff --git a/frontend/app/work_packages/controllers/index.js b/frontend/app/work_packages/controllers/index.js index 71f9988c92..94cc4b20c2 100644 --- a/frontend/app/work_packages/controllers/index.js +++ b/frontend/app/work_packages/controllers/index.js @@ -90,7 +90,7 @@ angular.module('openproject.workPackages.controllers') 'CommonRelationsHandler', 'ChildrenRelationsHandler', 'ParentRelationsHandler', - 'EditableFieldsState', + 'NotificationsService', require('./work-package-details-controller') ]) .controller('WorkPackageNewController', [ @@ -134,6 +134,7 @@ angular.module('openproject.workPackages.controllers') 'PathHelper', 'Query', 'OPERATORS_AND_LABELS_BY_FILTER_TYPE', + 'NotificationsService', require('./work-packages-list-controller') ]) .factory('columnsModal', ['btfModal', function(btfModal) { @@ -197,6 +198,7 @@ angular.module('openproject.workPackages.controllers') 'QueryService', 'AuthorisationService', '$state', + 'NotificationsService', require('./dialogs/save') ]) .factory('settingsModal', ['btfModal', function(btfModal) { @@ -214,6 +216,7 @@ angular.module('openproject.workPackages.controllers') 'AuthorisationService', '$rootScope', 'QUERY_MENU_ITEM_TYPE', + 'NotificationsService', require('./dialogs/settings') ]) .factory('shareModal', ['btfModal', function(btfModal) { @@ -231,6 +234,7 @@ angular.module('openproject.workPackages.controllers') 'AuthorisationService', 'queryMenuItemFactory', 'PathHelper', + 'NotificationsService', require('./dialogs/share') ]) .factory('sortingModal', ['btfModal', function(btfModal) { diff --git a/frontend/app/work_packages/controllers/menus/index.js b/frontend/app/work_packages/controllers/menus/index.js index 0f35a2de44..3e7647eb17 100644 --- a/frontend/app/work_packages/controllers/menus/index.js +++ b/frontend/app/work_packages/controllers/menus/index.js @@ -73,7 +73,9 @@ angular.module('openproject.workPackages') 'AuthorisationService', '$window', '$state', - '$timeout', require('./settings-dropdown-menu-controller') + '$timeout', + 'NotificationsService', + require('./settings-dropdown-menu-controller') ]) .factory('TasksDropdownMenu', [ 'ngContextMenu', diff --git a/frontend/app/work_packages/controllers/menus/settings-dropdown-menu-controller.js b/frontend/app/work_packages/controllers/menus/settings-dropdown-menu-controller.js index 527909b4b5..24adaf88da 100644 --- a/frontend/app/work_packages/controllers/menus/settings-dropdown-menu-controller.js +++ b/frontend/app/work_packages/controllers/menus/settings-dropdown-menu-controller.js @@ -31,7 +31,8 @@ module.exports = function( exportModal, saveModal, settingsModal, shareModal, sortingModal, groupingModal, QueryService, AuthorisationService, - $window, $state, $timeout) { + $window, $state, $timeout, + NotificationsService) { $scope.$watch('query.displaySums', function(newValue) { $timeout(function() { $scope.displaySumsLabel = (newValue) ? I18n.t('js.toolbar.settings.hide_sums') @@ -53,10 +54,15 @@ module.exports = function( if( allowQueryAction(event, 'update') ) { QueryService.saveQuery() .then(function(data){ - $scope.$emit('flashMessage', data.status); - $state.go('work-packages.list', - { 'query_id': $scope.query.id, 'query_props': null }, - { notify: false }); + if (data.status.isError) { + NotificationsService.addError(data.status.text); + } + else { + NotificationsService.addSuccess(data.status.text); + $state.go('work-packages.list', + { 'query_id': $scope.query.id, 'query_props': null }, + { notify: false }); + } }); } } @@ -67,11 +73,15 @@ module.exports = function( if( allowQueryAction(event, 'delete') && preventNewQueryAction(event) && deleteConfirmed() ){ QueryService.deleteQuery() .then(function(data){ - settingsModal.deactivate(); - $scope.$emit('flashMessage', data.status); - $state.go('work-packages.list', - { 'query_id': null, 'query_props': null }, - { reload: true }); + if (data.status.isError) { + NotificationsService.addError(data.status.text); + } + else { + NotificationsService.addSuccess(data.status.text); + $state.go('work-packages.list', + { 'query_id': null, 'query_props': null }, + { reload: true }); + } }); } }; diff --git a/frontend/app/work_packages/controllers/work-package-details-controller.js b/frontend/app/work_packages/controllers/work-package-details-controller.js index 2383f2a8da..b17bb0c150 100644 --- a/frontend/app/work_packages/controllers/work-package-details-controller.js +++ b/frontend/app/work_packages/controllers/work-package-details-controller.js @@ -42,7 +42,8 @@ module.exports = function($scope, WorkPackageService, CommonRelationsHandler, ChildrenRelationsHandler, - ParentRelationsHandler + ParentRelationsHandler, + NotificationsService ) { $scope.$on('$stateChangeSuccess', function(event, toState){ latestTab.registerState(toState.name); @@ -75,14 +76,16 @@ module.exports = function($scope, $scope.$emit('workPackgeLoaded'); function outputMessage(message, isError) { - $scope.$emit('flashMessage', { - isError: !!isError, - text: message - }); + if (!!isError) { + NotificationsService.addError(message); + } + else { + NotificationsService.addSuccess(message); + } } function outputError(error) { - outputMessage(error.message, true); + NotificationsService.addError(error.message); } $scope.outputMessage = outputMessage; // expose to child controllers diff --git a/frontend/app/work_packages/controllers/work-packages-list-controller.js b/frontend/app/work_packages/controllers/work-packages-list-controller.js index 5264ca6458..2fe7eed775 100644 --- a/frontend/app/work_packages/controllers/work-packages-list-controller.js +++ b/frontend/app/work_packages/controllers/work-packages-list-controller.js @@ -30,7 +30,7 @@ module.exports = function($scope, $rootScope, $state, $location, latestTab, I18n, WorkPackagesTableService, WorkPackageService, ProjectService, QueryService, PaginationService, AuthorisationService, UrlParamsHelper, PathHelper, Query, - OPERATORS_AND_LABELS_BY_FILTER_TYPE) { + OPERATORS_AND_LABELS_BY_FILTER_TYPE, NotificationsService) { // Setup function initialSetup() { @@ -84,10 +84,9 @@ module.exports = function($scope, $rootScope, $state, $location, latestTab, return WorkPackageService.getWorkPackages($scope.projectIdentifier, queryFromParams, PaginationService.getPaginationOptions()); } catch(e) { - $scope.$emit('flashMessage', { - isError: true, - text: I18n.t('js.work_packages.query.errors.unretrievable_query') - }); + NotificationsService.addError( + I18n.t('js.work_packages.query.errors.unretrievable_query') + ); clearUrlQueryParams(); return WorkPackageService.getWorkPackages($scope.projectIdentifier); diff --git a/frontend/app/work_packages/directives/inplace_editor/inplace-editor-edit-pane-directive.js b/frontend/app/work_packages/directives/inplace_editor/inplace-editor-edit-pane-directive.js index 11c4a26ff2..d67978faee 100644 --- a/frontend/app/work_packages/directives/inplace_editor/inplace-editor-edit-pane-directive.js +++ b/frontend/app/work_packages/directives/inplace_editor/inplace-editor-edit-pane-directive.js @@ -42,7 +42,7 @@ module.exports = function( if (_.isEmpty(_.keys(errors))) { return; } - var errorMessages = _.map(errors); + var errorMessages = _.flatten(_.map(errors), true); NotificationsService.addError(I18n.t('js.label_validation_error'), errorMessages); }; @@ -157,7 +157,7 @@ module.exports = function( function setFailure(e) { afterError(); EditableFieldsState.errors = { - '_common': ApiHelper.getErrorMessage(e) + '_common': ApiHelper.getErrorMessages(e) }; showErrors(); } diff --git a/frontend/app/work_packages/helpers/api-helper.js b/frontend/app/work_packages/helpers/api-helper.js index be3926e746..526709abea 100644 --- a/frontend/app/work_packages/helpers/api-helper.js +++ b/frontend/app/work_packages/helpers/api-helper.js @@ -28,20 +28,12 @@ module.exports = function() { var ApiHelper = { - handleError: function(scope, error) { - scope.$emit('flashMessage', { - isError: true, - text: ApiHelper.getErrorMessage(error) - }); - }, - - getErrorMessage: function(error) { - if(error.status == 500) { - return error.statusText; + getErrorMessages: function(error) { + if(error.status === 500) { + return [error.statusText]; } else { var response = JSON.parse(error.responseText); var messages = []; - var message; if (ApiHelper.isMultiErrorMessage(response)) { angular.forEach(response._embedded.errors, function(error) { @@ -51,14 +43,12 @@ module.exports = function() { messages.push(response.message); } - message = messages.join(' '); - - return message; + return messages; } }, isMultiErrorMessage: function(error) { - return error.errorIdentifier == 'urn:openproject-org:api:v3:errors:MultipleErrors'; + return error.errorIdentifier === 'urn:openproject-org:api:v3:errors:MultipleErrors'; } }; diff --git a/frontend/app/work_packages/helpers/index.js b/frontend/app/work_packages/helpers/index.js index 9db27a3787..7e4dd52cb3 100644 --- a/frontend/app/work_packages/helpers/index.js +++ b/frontend/app/work_packages/helpers/index.js @@ -27,7 +27,7 @@ //++ angular.module('openproject.workPackages.helpers') - .factory('ApiHelper', require('./api-helper')) + .factory('ApiHelper', ['NotificationsService', require('./api-helper')]) .factory('FiltersHelper', ['I18n', require('./filters-helper')]) .constant('ACTIVE_USER_STATUSES', ['active', 'registered']) .factory('UsersHelper', ['ACTIVE_USER_STATUSES', require('./users-helper')]) diff --git a/frontend/app/work_packages/view_models/children-relations-handler.js b/frontend/app/work_packages/view_models/children-relations-handler.js index 18237652c3..e74979ca52 100644 --- a/frontend/app/work_packages/view_models/children-relations-handler.js +++ b/frontend/app/work_packages/view_models/children-relations-handler.js @@ -26,11 +26,15 @@ // See doc/COPYRIGHT.rdoc for more details. //++ -module.exports = function(PathHelper, CommonRelationsHandler, WorkPackageService) { +module.exports = function( + CommonRelationsHandler, + WorkPackageService, + ApiNotificationsService + ) { function ChildrenRelationsHandler(workPackage, children) { var handler = new CommonRelationsHandler(workPackage, children, undefined); - handler.type = "child"; + handler.type = 'child'; handler.applyCustomExtensions = undefined; handler.canAddRelation = function() { return !!this.workPackage.links.addChild; }; @@ -55,7 +59,7 @@ module.exports = function(PathHelper, CommonRelationsHandler, WorkPackageService scope.updateFocus(index); scope.$emit('workPackageRefreshRequired'); }, function(error) { - ApiHelper.handleError(scope, error); + ApiNotificationsService.addError(error); }); }; diff --git a/frontend/app/work_packages/view_models/common-relations-handler.js b/frontend/app/work_packages/view_models/common-relations-handler.js index 3e616bf425..9676f490ce 100644 --- a/frontend/app/work_packages/view_models/common-relations-handler.js +++ b/frontend/app/work_packages/view_models/common-relations-handler.js @@ -26,7 +26,11 @@ // See doc/COPYRIGHT.rdoc for more details. //++ -module.exports = function($timeout, WorkPackageService, ApiHelper, PathHelper, MAX_AUTOCOMPLETER_ADDITION_ITERATIONS) { +module.exports = function( + $timeout, + WorkPackageService, + ApiNotificationsService + ) { function CommonRelationsHandler(workPackage, relations, relationsId) { @@ -34,7 +38,7 @@ module.exports = function($timeout, WorkPackageService, ApiHelper, PathHelper, M this.relations = relations; this.relationsId = relationsId; - this.type = "relation"; + this.type = 'relation'; this.isSingletonRelation = false; } @@ -56,12 +60,15 @@ module.exports = function($timeout, WorkPackageService, ApiHelper, PathHelper, M }, addRelation: function(scope) { - WorkPackageService.addWorkPackageRelation(this.workPackage, scope.relationToAddId, this.relationsId).then(function(relation) { + WorkPackageService.addWorkPackageRelation(this.workPackage, + scope.relationToAddId, + this.relationsId) + .then(function() { scope.relationToAddId = ''; scope.updateFocus(-1); scope.$emit('workPackageRefreshRequired'); }, function(error) { - ApiHelper.handleError(scope, error); + ApiNotificationsService.addError(error); }); }, @@ -74,7 +81,7 @@ module.exports = function($timeout, WorkPackageService, ApiHelper, PathHelper, M scope.updateFocus(index); scope.$emit('workPackageRefreshRequired'); }, function(error) { - ApiHelper.handleError(scope, error); + ApiNotificationsService.addError(scope, error); }); }, @@ -89,7 +96,7 @@ module.exports = function($timeout, WorkPackageService, ApiHelper, PathHelper, M getRelatedWorkPackage: function(workPackage, relation) { var self = workPackage.links.self.href; - if (relation.links.relatedTo.href == self) { + if (relation.links.relatedTo.href === self) { return relation.links.relatedFrom.fetch(); } else { return relation.links.relatedTo.fetch(); diff --git a/frontend/app/work_packages/view_models/index.js b/frontend/app/work_packages/view_models/index.js index 9215d849a9..1bb4fe92bf 100644 --- a/frontend/app/work_packages/view_models/index.js +++ b/frontend/app/work_packages/view_models/index.js @@ -27,20 +27,21 @@ //++ angular.module('openproject.viewModels') - .constant('MAX_AUTOCOMPLETER_ADDITION_ITERATIONS', 3) .factory('CommonRelationsHandler', [ '$timeout', 'WorkPackageService', - 'ApiHelper', - 'PathHelper', - 'MAX_AUTOCOMPLETER_ADDITION_ITERATIONS', require( - './common-relations-handler') + 'ApiNotificationsService', + require('./common-relations-handler') ]) - .factory('ChildrenRelationsHandler', ['PathHelper', 'CommonRelationsHandler', + .factory('ChildrenRelationsHandler', [ + 'CommonRelationsHandler', 'WorkPackageService', + 'ApiNotificationsService', require('./children-relations-handler') ]) - .factory('ParentRelationsHandler', ['CommonRelationsHandler', - 'WorkPackageService', 'ApiHelper', + .factory('ParentRelationsHandler', [ + 'CommonRelationsHandler', + 'WorkPackageService', + 'ApiNotificationsService', require('./parent-relations-handler') ]); diff --git a/frontend/app/work_packages/view_models/parent-relations-handler.js b/frontend/app/work_packages/view_models/parent-relations-handler.js index 0bdd3d919f..c92f1052a5 100644 --- a/frontend/app/work_packages/view_models/parent-relations-handler.js +++ b/frontend/app/work_packages/view_models/parent-relations-handler.js @@ -26,7 +26,7 @@ // See doc/COPYRIGHT.rdoc for more details. //++ -module.exports = function(CommonRelationsHandler, WorkPackageService, ApiHelper) { +module.exports = function(CommonRelationsHandler, WorkPackageService, ApiNotificationsService) { function ParentRelationsHandler(workPackage, parents, relationsId) { var relations = parents.filter(function(parent) { return parent.props.id !== workPackage.props.id; @@ -52,7 +52,7 @@ module.exports = function(CommonRelationsHandler, WorkPackageService, ApiHelper) scope.updateFocus(-1); scope.$emit('workPackageRefreshRequired'); }, function(error) { - ApiHelper.handleError(scope, error); + ApiNotificationsService.addError(error); }); }; handler.removeRelation = function(scope) { @@ -69,7 +69,7 @@ module.exports = function(CommonRelationsHandler, WorkPackageService, ApiHelper) scope.updateFocus(index); scope.$emit('workPackageRefreshRequired'); }, function(error) { - ApiHelper.handleError(scope, error); + ApiNotificationsService.addError(error); }); }; diff --git a/frontend/public/index.html b/frontend/public/index.html index c1e59e1e62..cbfd9b0976 100644 --- a/frontend/public/index.html +++ b/frontend/public/index.html @@ -85,7 +85,6 @@ See doc/COPYRIGHT.rdoc for more details.

Content

-
 
diff --git a/frontend/tests/unit/tests/services/api-notifications-service-test.js b/frontend/tests/unit/tests/services/api-notifications-service-test.js new file mode 100644 index 0000000000..1ad14a81b5 --- /dev/null +++ b/frontend/tests/unit/tests/services/api-notifications-service-test.js @@ -0,0 +1,91 @@ +//-- copyright +// OpenProject is a project management system. +// Copyright (C) 2012-2015 the OpenProject Foundation (OPF) +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License version 3. +// +// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +// Copyright (C) 2006-2013 Jean-Philippe Lang +// Copyright (C) 2010-2013 the ChiliProject Team +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License +// as published by the Free Software Foundation; either version 2 +// of the License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, write to the Free Software +// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +// +// See doc/COPYRIGHT.rdoc for more details. +//++ + +describe('NotificationsService', function() { + 'use strict'; + var ApiNotificationsService, + NotificationsService, + ApiHelper; + + beforeEach(module('openproject.services')); + + beforeEach(inject(function(_ApiNotificationsService_, _NotificationsService_, _ApiHelper_){ + ApiNotificationsService = _ApiNotificationsService_; + NotificationsService = _NotificationsService_; + ApiHelper = _ApiHelper_; + })); + + describe('#addError', function() { + var error = {}, + messages = []; + + beforeEach(function() { + sinon.spy(NotificationsService, 'addError'); + }); + + describe('with only one error message', function() { + beforeEach(function() { + messages = ['Oh my - Error']; + ApiHelper.getErrorMessages = sinon.stub().returns(messages); + }); + + it('adds the error to the notification service', function() { + ApiNotificationsService.addError(error); + + expect(NotificationsService.addError).to.have.been.calledWith('Oh my - Error'); + }); + }); + + describe('with multiple error messages', function() { + beforeEach(function() { + messages = ['Oh my - Error', 'Hey you - Error']; + ApiHelper.getErrorMessages = sinon.stub().returns(messages); + }); + + it('adds the error to the notification service', function() { + ApiNotificationsService.addError(error); + + expect(NotificationsService.addError).to.have.been.calledWith('', messages); + }); + }); + }); + + describe('#addSuccess', function() { + var message = 'Great success'; + + beforeEach(function() { + sinon.spy(NotificationsService, 'addSuccess'); + }); + + it('delegates to NotificationService', function() { + ApiNotificationsService.addSuccess(message); + + expect(NotificationsService.addSuccess).to.have.been.calledWith(message); + }); + }); +}); diff --git a/frontend/tests/unit/tests/services/notifications-service-test.js b/frontend/tests/unit/tests/services/notifications-service-test.js index f8de32619c..2593793268 100644 --- a/frontend/tests/unit/tests/services/notifications-service-test.js +++ b/frontend/tests/unit/tests/services/notifications-service-test.js @@ -48,18 +48,6 @@ describe('NotificationsService', function() { expect(notification).to.eql({ message: 'warning!', type: 'warning' }); }); - it('should throw an Error if trying to create an error without errors', function() { - expect(function() { - NotificationsService.addError('error!'); - }).to.throw(Error); - }); - - it('should throw an Error if trying to create an upload without uploads', function() { - expect(function() { - NotificationsService.addWorkPackageUpload('themUploads'); - }).to.throw(Error); - }); - it('should be able to create error messages with errors', function() { var notification = NotificationsService.addError('a super cereal error', ['fooo', 'baarr']); expect(notification).to.eql({ @@ -69,7 +57,16 @@ describe('NotificationsService', function() { }); }); - it('should be able to create error messages with errors', function() { + it('should be able to create error messages with only a message', function() { + var notification = NotificationsService.addError('a super cereal error'); + expect(notification).to.eql({ + message: 'a super cereal error', + errors: [], + type: 'error' + }); + }); + + it('should be able to create upload messages with uploads', function() { var notification = NotificationsService.addWorkPackageUpload('uploading...', [0, 1, 2]); expect(notification).to.eql({ message: 'uploading...', @@ -77,4 +74,10 @@ describe('NotificationsService', function() { uploads: [0, 1, 2] }); }); + + it('should throw an Error if trying to create an upload without uploads', function() { + expect(function() { + NotificationsService.addWorkPackageUpload('themUploads'); + }).to.throw(Error); + }); }); diff --git a/frontend/tests/unit/tests/ui_components/flash-message-directive-test.js b/frontend/tests/unit/tests/ui_components/flash-message-directive-test.js deleted file mode 100644 index 005bc31405..0000000000 --- a/frontend/tests/unit/tests/ui_components/flash-message-directive-test.js +++ /dev/null @@ -1,126 +0,0 @@ -//-- copyright -// OpenProject is a project management system. -// Copyright (C) 2012-2015 the OpenProject Foundation (OPF) -// -// This program is free software; you can redistribute it and/or -// modify it under the terms of the GNU General Public License version 3. -// -// OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: -// Copyright (C) 2006-2013 Jean-Philippe Lang -// Copyright (C) 2010-2013 the ChiliProject Team -// -// This program is free software; you can redistribute it and/or -// modify it under the terms of the GNU General Public License -// as published by the Free Software Foundation; either version 2 -// of the License, or (at your option) any later version. -// -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License -// along with this program; if not, write to the Free Software -// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. -// -// See doc/COPYRIGHT.rdoc for more details. -//++ - -/*jshint expr: true*/ - -describe('flashMessage Directive', function() { - var compile, element, rootScope, scope; - - beforeEach(angular.mock.module('openproject.uiComponents', function($provide) { - var configurationService = {}; - - configurationService.accessibilityModeEnabled = sinon.stub().returns(true); - - $provide.constant('ConfigurationService', configurationService); - })); - - beforeEach(module('openproject.templates')); - - beforeEach(inject(function($rootScope, $compile) { - var html = ''; - - element = angular.element(html); - rootScope = $rootScope; - scope = $rootScope.$new(); - - compile = function() { - $compile(element)(scope); - scope.$digest(); - }; - })); - - context('with no message', function() { - beforeEach(function() { - compile(); - }); - - it('should render no message initially', function() { - expect(element.text()).to.be.equal(''); - }); - - it('should be hidden', function() { - expect(element.hasClass('ng-hide')).to.be.true; - }); - }); - - context('with flash messages', function() { - beforeEach(function() { - compile(); - }); - - describe('info message', function() { - var message = { - text: 'für deine Informationen', - isError: false - }; - - beforeEach(function() { - rootScope.$emit('flashMessage', message); - scope.$apply(); - }); - - it('should render message', function() { - expect(element.text().trim()).to.equal('für deine Informationen'); - }); - - it('should be visible', function() { - expect(element.hasClass('ng-hide')).to.be.false; - }); - - it('should style as an info message', function() { - expect(element.attr('class').split(' ')).to - .include.members(['flash', 'icon-notice', 'notice']); - }); - }); - - describe('error message', function() { - var message = { - text: '¡Alerta! WARNING! Achtung!', - isError: true - }; - - beforeEach(function() { - rootScope.$emit('flashMessage', message); - scope.$apply(); - }); - - it('should render message', function() { - expect(element.text().trim()).to.equal('¡Alerta! WARNING! Achtung!'); - }); - - it('should be visible', function() { - expect(element.hasClass('ng-hide')).to.be.false; - }); - - it('should style as an error message', function() { - expect(element.attr('class').split(' ')).to - .include.members(['flash', 'icon-errorExplanation', 'errorExplanation']); - }); - }); - }); -}); diff --git a/frontend/tests/unit/tests/work_packages/controllers/dialogs/settings-test.js b/frontend/tests/unit/tests/work_packages/controllers/dialogs/settings-test.js index 8e4967e52a..fb9888db1e 100644 --- a/frontend/tests/unit/tests/work_packages/controllers/dialogs/settings-test.js +++ b/frontend/tests/unit/tests/work_packages/controllers/dialogs/settings-test.js @@ -29,7 +29,7 @@ /*jshint expr: true*/ describe('SettingsModalController', function() { - var scope, $q, defer, settingsModal, QueryService; + var scope, $q, defer, settingsModal, QueryService, NotificationsService; var ctrl, buildController; beforeEach(module('openproject.workPackages.controllers')); @@ -51,12 +51,16 @@ describe('SettingsModalController', function() { }; settingsModal = { deactivate: angular.noop }; + NotificationsService = { + addSuccess: function() {} + }; buildController = function() { ctrl = $controller("SettingsModalController", { $scope: scope, settingsModal: settingsModal, - QueryService: QueryService + QueryService: QueryService, + NotificationsService: NotificationsService }); }; })); @@ -68,9 +72,14 @@ describe('SettingsModalController', function() { sinon.spy(scope, "$emit"); sinon.spy(settingsModal, "deactivate"); sinon.spy(QueryService, "updateHighlightName"); + sinon.spy(NotificationsService, 'addSuccess'); scope.updateQuery(); - defer.resolve({status: "Query updated!"}); + defer.resolve({ + status: { + text: 'Query updated!' + } + }); scope.$digest(); }); @@ -79,9 +88,8 @@ describe('SettingsModalController', function() { expect(settingsModal.deactivate).to.have.been.called; }); - it('should update the flash message', function() { - expect(scope.$emit).to.have.been.calledWith("flashMessage", - "Query updated!"); + it('should notfify success', function() { + expect(NotificationsService.addSuccess).to.have.been.calledWith('Query updated!'); }); it ('should update the query menu name', function() { diff --git a/frontend/tests/unit/tests/work_packages/helpers/api-helper-test.js b/frontend/tests/unit/tests/work_packages/helpers/api-helper-test.js index 9515591daa..b60bdc3422 100644 --- a/frontend/tests/unit/tests/work_packages/helpers/api-helper-test.js +++ b/frontend/tests/unit/tests/work_packages/helpers/api-helper-test.js @@ -47,53 +47,56 @@ describe('API helper', function() { return error; } - describe('500', function() { - var error = createErrorObject(500, "Internal Server Error"); + describe('.getErrorMessages', function() { + describe('500', function() { + var error = createErrorObject(500, 'Internal Server Error'); - it('should return status text', function() { - expect(ApiHelper.getErrorMessage(error)).to.eq(error.statusText); + it('should return status text in an array', function() { + expect(ApiHelper.getErrorMessages(error)).to.eql([error.statusText]); + }); }); - }); - describe('other codes', function() { - function createApiErrorObject(errorIdentifier, message, multiple) { - var apiError = {}; + describe('other codes', function() { + function createApiErrorObject(errorIdentifier, message, multiple) { + var apiError = {}; - apiError._type = 'Error'; - apiError.errorIdentifier = (multiple) ? 'urn:openproject-org:api:v3:errors:MultipleErrors' : errorIdentifier; - apiError.message = message; + apiError._type = 'Error'; + apiError.errorIdentifier = (multiple) ? + 'urn:openproject-org:api:v3:errors:MultipleErrors' : + errorIdentifier; + apiError.message = message; - if (multiple) { - apiError._embedded = { errors: [] }; + if (multiple) { + apiError._embedded = { errors: [] }; - for (var x=0; x < 2; x++) { - apiError._embedded.errors.push(createApiErrorObject(errorIdentifier, message)); + for (var x=0; x < 2; x++) { + apiError._embedded.errors.push(createApiErrorObject(errorIdentifier, message)); + } } - } - return apiError; - } + return apiError; + } - describe('single error', function() { - var apiError = createApiErrorObject('NotFound', 'Not found.'); - var error = createErrorObject(404, null, JSON.stringify(apiError)); - var expectedResult = 'Not found.'; + describe('single error', function() { + var apiError = createApiErrorObject('NotFound', 'Not found.'); + var error = createErrorObject(404, null, JSON.stringify(apiError)); + var expectedResult = 'Not found.'; - it('should return api error message', function() { - expect(ApiHelper.getErrorMessage(error)).to.eq(expectedResult); + it('should return api error message in an arry', function() { + expect(ApiHelper.getErrorMessages(error)).to.eql([expectedResult]); + }); }); - }); - describe('multiple errors', function() { - var errorMessage = 'This is an error message.'; - var apiError = createApiErrorObject('PropertyIsReadOnly', errorMessage, true); - var error = createErrorObject(404, null, JSON.stringify(apiError)); + describe('multiple errors', function() { + var errorMessage = 'This is an error message.'; + var apiError = createApiErrorObject('PropertyIsReadOnly', errorMessage, true); + var error = createErrorObject(404, null, JSON.stringify(apiError)); - it('should return concatenated api error messages', function() { - var messages = []; - var expectedResult = errorMessage + ' ' + errorMessage; + it('should return an array of messages', function() { + var expectedResult = [errorMessage, errorMessage]; - expect(ApiHelper.getErrorMessage(error)).to.eq(expectedResult); + expect(ApiHelper.getErrorMessages(error)).to.eql(expectedResult); + }); }); }); }); diff --git a/spec/features/menu_items/query_menu_item_spec.rb b/spec/features/menu_items/query_menu_item_spec.rb index 64eb5908fe..656b5eb7eb 100644 --- a/spec/features/menu_items/query_menu_item_spec.rb +++ b/spec/features/menu_items/query_menu_item_spec.rb @@ -27,6 +27,7 @@ #++ require 'spec_helper' +require 'features/page_objects/notification' require 'features/work_packages/shared_contexts' require 'features/work_packages/work_packages_page' @@ -34,6 +35,7 @@ feature 'Query menu items' do let(:user) { FactoryGirl.create :admin } let(:project) { FactoryGirl.create :project } let(:work_packages_page) { WorkPackagesPage.new(project) } + let(:notification) { PageObjects::Notifications.new(page) } def visit_index_page(query) work_packages_page.select_query(query) @@ -72,7 +74,7 @@ feature 'Query menu items' do check 'show_in_menu' click_on 'Save' - expect(page).to have_selector('.flash', text: 'Successful update') + notification.expect_success('Successful update') expect(page).to have_selector('a', text: query.name) end @@ -103,7 +105,7 @@ feature 'Query menu items' do end it 'displaying a success message', js: true do - expect(page).to have_selector('.flash', text: 'Successful update') + notification.expect_success('Successful update') end it 'is renaming and reordering the list', js: true do diff --git a/spec/features/page_objects/notification.rb b/spec/features/page_objects/notification.rb new file mode 100644 index 0000000000..37e370162e --- /dev/null +++ b/spec/features/page_objects/notification.rb @@ -0,0 +1,43 @@ +#-- copyright +# OpenProject is a project management system. +# Copyright (C) 2012-2015 the OpenProject Foundation (OPF) +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See doc/COPYRIGHT.rdoc for more details. +#++ + +module PageObjects + class Notifications + include RSpec::Matchers + + attr_accessor :page + + def initialize(page) + @page = page + end + + def expect_success(message) + expect(page).to have_selector('.notification-box.-success', text: message) + end + end +end