From d9e9469f4d6affdbe39109259be14fdc9e8859f6 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Tue, 16 Feb 2016 16:41:33 +0100 Subject: [PATCH 1/4] Added support to show error message of object was not able to create. --- .../_application_controller_form.coffee | 13 +++++++++++++ .../app/controllers/getting_started.coffee | 8 ++++---- .../javascripts/app/controllers/signup.coffee | 7 +++++-- .../javascripts/app/lib/app_post/ajax.coffee | 1 + .../javascripts/app/lib/spine/ajax.coffee | 18 ++++++++++++++++-- .../app/models/_application_model.coffee | 15 ++++++++++++++- app/controllers/application_controller.rb | 19 ++++++++++++++----- app/controllers/users_controller.rb | 6 +++--- 8 files changed, 70 insertions(+), 17 deletions(-) diff --git a/app/assets/javascripts/app/controllers/_application_controller_form.coffee b/app/assets/javascripts/app/controllers/_application_controller_form.coffee index e33e18f45..c5b64db52 100644 --- a/app/assets/javascripts/app/controllers/_application_controller_form.coffee +++ b/app/assets/javascripts/app/controllers/_application_controller_form.coffee @@ -12,6 +12,10 @@ class App.ControllerForm extends App.Controller # set empty class attributes if needed if !@form @form = @formGen() + + # add alert placeholder + @form.prepend('') + if !@model @model = {} if !@attributes @@ -27,9 +31,18 @@ class App.ControllerForm extends App.Controller @form.find('textarea').trigger('change') @form.find('select').trigger('change') + # remove alert on input + @form.on('input', @hideAlert) + @finishForm = true @form + showAlert: (message) => + @form.find('.alert').removeClass('hide').html( App.i18n.translateContent( message ) ) + + hideAlert: => + @form.find('.alert').addClass('hide').html() + html: => @form.html() diff --git a/app/assets/javascripts/app/controllers/getting_started.coffee b/app/assets/javascripts/app/controllers/getting_started.coffee index 3b05f7af4..42ee9acb3 100644 --- a/app/assets/javascripts/app/controllers/getting_started.coffee +++ b/app/assets/javascripts/app/controllers/getting_started.coffee @@ -228,11 +228,11 @@ class Admin extends App.ControllerContent ) @Config.set('system_init_done', true) - fail: (data) => + fail: (settings, details) => @formEnable(e) App.Event.trigger 'notify', { type: 'error' - msg: App.i18n.translateContent( 'Can\'t create user!' ) + msg: App.i18n.translateContent( details.error_human || 'Can\'t create user!' ) timeout: 2500 } ) @@ -984,11 +984,11 @@ class Agent extends App.ControllerContent # rerender page @render() - fail: (data) => + fail: (settings, details) => @formEnable(e) App.Event.trigger 'notify', { type: 'error' - msg: App.i18n.translateContent( 'Can\'t create user!' ) + msg: App.i18n.translateContent( details.error_human || 'Can\'t create user!' ) timeout: 2500 } ) diff --git a/app/assets/javascripts/app/controllers/signup.coffee b/app/assets/javascripts/app/controllers/signup.coffee index 24105b24d..4400949f0 100644 --- a/app/assets/javascripts/app/controllers/signup.coffee +++ b/app/assets/javascripts/app/controllers/signup.coffee @@ -24,7 +24,7 @@ class Index extends App.ControllerContent @html App.view('signup')() - new App.ControllerForm( + @form = new App.ControllerForm( el: @el.find('form') model: App.User screen: 'signup' @@ -34,7 +34,7 @@ class Index extends App.ControllerContent cancel: -> @navigate '#login' - submit: (e) -> + submit: (e) => e.preventDefault() @formDisable(e) @params = @formParam(e.target) @@ -67,6 +67,9 @@ class Index extends App.ControllerContent success: @success error: @error ) + fail: (settings, details) => + @formEnable(e) + @form.showAlert(details.error_human || details.error || 'Unable to update object!') ) success: (data, status, xhr) => diff --git a/app/assets/javascripts/app/lib/app_post/ajax.coffee b/app/assets/javascripts/app/lib/app_post/ajax.coffee index 9f2cf72c8..8bfb5e5db 100644 --- a/app/assets/javascripts/app/lib/app_post/ajax.coffee +++ b/app/assets/javascripts/app/lib/app_post/ajax.coffee @@ -73,6 +73,7 @@ class _ajaxSingleton # do not show any error message with code 401/404 (handled by controllers) return if status is 401 return if status is 404 + return if status is 422 # do not show any error message with code 502 return if status is 502 diff --git a/app/assets/javascripts/app/lib/spine/ajax.coffee b/app/assets/javascripts/app/lib/spine/ajax.coffee index ed2639878..1370780db 100644 --- a/app/assets/javascripts/app/lib/spine/ajax.coffee +++ b/app/assets/javascripts/app/lib/spine/ajax.coffee @@ -166,7 +166,14 @@ class Collection extends Base failResponse: (options) => (xhr, statusText, error, settings) => @model.trigger('ajaxError', null, xhr, statusText, error, settings) - options.fail?.call(@model, settings) + # add errors to calllback + @record.trigger('ajaxError', @record, xhr, statusText, error, settings) + #options.fail?.call(@model, settings) + detailsRaw = xhr.responseText + if !_.isEmpty(detailsRaw) + details = JSON.parse(detailsRaw) + options.fail?.call(@record, settings, details) + # /add errors to calllback class Singleton extends Base constructor: (@record) -> @@ -230,8 +237,15 @@ class Singleton extends Base switch settings.type when 'POST' then @createFailed() when 'DELETE' then @destroyFailed() + # add errors to calllback @record.trigger('ajaxError', @record, xhr, statusText, error, settings) - options.fail?.call(@record, settings) + #options.fail?.call(@record, settings) + detailsRaw = xhr.responseText + if !_.isEmpty(detailsRaw) + details = JSON.parse(detailsRaw) + options.fail?.call(@record, settings, details) + @record.trigger('remove', @record) + # /add errors to calllback createFailed: -> @record.remove(clear: true) diff --git a/app/assets/javascripts/app/models/_application_model.coffee b/app/assets/javascripts/app/models/_application_model.coffee index 39a284abc..d1d182880 100644 --- a/app/assets/javascripts/app/models/_application_model.coffee +++ b/app/assets/javascripts/app/models/_application_model.coffee @@ -307,7 +307,7 @@ class App.Model extends Spine.Model # subscribe and render data / fetch new data if triggered @bind( - 'refresh change' + 'refresh change remove' (items) => App.Log.debug('Model', "local collection refresh/change #{@className}", items) for key, callback of @SUBSCRIPTION_COLLECTION @@ -376,6 +376,19 @@ class App.Model extends Spine.Model item = App[ @className ]._fillUp( item ) callback(item, 'change') ) + @bind( + 'remove' + (items) => + + # check if result is array or singel item + if !_.isArray(items) + items = [items] + App.Log.debug('Model', "local remove #{@className}", items) + for item in items + for key, callback of App[ @className ].SUBSCRIPTION_ITEM[ item.id ] + item = App[ @className ]._fillUp( item ) + callback(item, 'remove') + ) @changeTable = {} @bind( diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 6f4cc4170..665abdbed 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -313,7 +313,7 @@ class ApplicationController < ActionController::Base rescue => e logger.error e.message logger.error e.backtrace.inspect - render json: { error: e.message }, status: :unprocessable_entity + render json: model_match_error(e.message), status: :unprocessable_entity end def model_create_render_item (generic_object) @@ -335,7 +335,7 @@ class ApplicationController < ActionController::Base rescue => e logger.error e.message logger.error e.backtrace.inspect - render json: { error: e.message }, status: :unprocessable_entity + render json: model_match_error(e.message), status: :unprocessable_entity end def model_update_render_item (generic_object) @@ -349,7 +349,7 @@ class ApplicationController < ActionController::Base rescue => e logger.error e.message logger.error e.backtrace.inspect - render json: { error: e.message }, status: :unprocessable_entity + render json: model_match_error(e.message), status: :unprocessable_entity end def model_destory_render_item () @@ -369,7 +369,7 @@ class ApplicationController < ActionController::Base rescue => e logger.error e.message logger.error e.backtrace.inspect - render json: { error: e.message }, status: :unprocessable_entity + render json: model_match_error(e.message), status: :unprocessable_entity end def model_show_render_item (generic_object) @@ -382,11 +382,20 @@ class ApplicationController < ActionController::Base rescue => e logger.error e.message logger.error e.backtrace.inspect - render json: { error: e.message }, status: :unprocessable_entity + render json: model_match_error(e.message), status: :unprocessable_entity end def model_index_render_result (generic_objects) render json: generic_objects, status: :ok end + def model_match_error (error) + data = { + error: error + } + if error =~ /(already exists|duplicate key)/i + data[:error_human] = 'Object already exists!' + end + data + end end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 0a82973ba..f23a357b7 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -77,7 +77,7 @@ class UsersController < ApplicationController # check if feature is enabled if !Setting.get('user_create_account') - render json: { error: 'Feature not enabled!' }, status: :unprocessable_entity + render json: { error_human: 'Feature not enabled!' }, status: :unprocessable_entity return end @@ -117,7 +117,7 @@ class UsersController < ApplicationController if user.email exists = User.where(email: user.email.downcase).first if exists - render json: { error: 'User already exists!' }, status: :unprocessable_entity + render json: { error_human: 'User already exists!' }, status: :unprocessable_entity return end end @@ -180,7 +180,7 @@ class UsersController < ApplicationController user_new = User.find(user.id) render json: user_new, status: :created rescue => e - render json: { error: e.message }, status: :unprocessable_entity + render json: model_match_error(e.message), status: :unprocessable_entity end end From 3bd759e23f91b181e629411bb9640acdfac466c2 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Tue, 16 Feb 2016 19:58:41 +0100 Subject: [PATCH 2/4] Fixed test. --- test/controllers/user_organization_controller_test.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/controllers/user_organization_controller_test.rb b/test/controllers/user_organization_controller_test.rb index bce04cc70..7378a5ef2 100644 --- a/test/controllers/user_organization_controller_test.rb +++ b/test/controllers/user_organization_controller_test.rb @@ -80,8 +80,8 @@ class UserOrganizationControllerTest < ActionDispatch::IntegrationTest post '/api/v1/users', {}, @headers assert_response(422) result = JSON.parse(@response.body) - assert(result['error']) - assert_equal('Feature not enabled!', result['error']) + assert(result['error_human']) + assert_equal('Feature not enabled!', result['error_human']) # already existing user with enabled feature Setting.set('user_create_account', true) @@ -89,8 +89,8 @@ class UserOrganizationControllerTest < ActionDispatch::IntegrationTest post '/api/v1/users', params.to_json, @headers assert_response(422) result = JSON.parse(@response.body) - assert(result['error']) - assert_equal('User already exists!', result['error']) + assert(result['error_human']) + assert_equal('User already exists!', result['error_human']) # create user with enabled feature params = { firstname: 'Me First', lastname: 'Me Last', email: 'new_here@example.com' } From a20c6e438ac86e11936257e1777bed2cc58a7226 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Tue, 16 Feb 2016 20:00:27 +0100 Subject: [PATCH 3/4] Added support to show error message of object was not able to create. --- .../_application_controller_generic.coffee | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/app/assets/javascripts/app/controllers/_application_controller_generic.coffee b/app/assets/javascripts/app/controllers/_application_controller_generic.coffee index 5d344dafb..63df1d627 100644 --- a/app/assets/javascripts/app/controllers/_application_controller_generic.coffee +++ b/app/assets/javascripts/app/controllers/_application_controller_generic.coffee @@ -6,13 +6,13 @@ class App.ControllerGenericNew extends App.ControllerModal content: => @head = @pageData.object - controller = new App.ControllerForm( + @controller = new App.ControllerForm( model: App[ @genericObject ] params: @item screen: @screen || 'edit' autofocus: true ) - controller.form + @controller.form onSubmit: (e) -> params = @formParam(e.target) @@ -39,9 +39,10 @@ class App.ControllerGenericNew extends App.ControllerModal ui.callback(item) ui.close() - fail: -> - ui.log 'errors' - ui.close() + fail: (settings, details) -> + ui.log 'errors', details + ui.formEnable(e) + ui.controller.showAlert(details.error_human || details.error || 'Unable to create object!') ) class App.ControllerGenericEdit extends App.ControllerModal @@ -54,13 +55,13 @@ class App.ControllerGenericEdit extends App.ControllerModal @item = App[ @genericObject ].find( @id ) @head = @pageData.object - controller = new App.ControllerForm( + @controller = new App.ControllerForm( model: App[ @genericObject ] params: @item screen: @screen || 'edit' autofocus: true ) - controller.form + @controller.form onSubmit: (e) -> params = @formParam(e.target) @@ -85,9 +86,10 @@ class App.ControllerGenericEdit extends App.ControllerModal ui.callback(item) ui.close() - fail: -> + fail: (settings, details) -> ui.log 'errors' - ui.close() + ui.formEnable(e) + ui.controller.showAlert(details.error_human || details.error || 'Unable to update object!') ) class App.ControllerGenericIndex extends App.Controller From 2d88c25b36ad092c596c8bc61870a2fd053c89c9 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Tue, 16 Feb 2016 20:34:37 +0100 Subject: [PATCH 4/4] Added preferences permission check. --- .../_application_controller_generic.coffee | 39 ++++++++++---- test/browser/prefereces_test.rb | 54 +++++++++++++++++++ 2 files changed, 83 insertions(+), 10 deletions(-) diff --git a/app/assets/javascripts/app/controllers/_application_controller_generic.coffee b/app/assets/javascripts/app/controllers/_application_controller_generic.coffee index 63df1d627..b6c4b06e7 100644 --- a/app/assets/javascripts/app/controllers/_application_controller_generic.coffee +++ b/app/assets/javascripts/app/controllers/_application_controller_generic.coffee @@ -353,25 +353,44 @@ class App.ControllerNavSidbar extends App.ControllerContent @params = params - # get groups + # get accessable groups + roles = App.Session.get('roles') groups = App.Config.get(@configKey) groupsUnsorted = [] - for key, value of groups - if !value.controller - groupsUnsorted.push value + for key, item of groups + if !item.controller + if !item.role + groupsUnsorted.push item + else + match = _.include(item.role, 'Anybody') + if !match + for role in roles + if !match + match = _.include(item.role, role.name) + if match + groupsUnsorted.push item - @groupsSorted = _.sortBy( groupsUnsorted, (item) -> return item.prio ) + @groupsSorted = _.sortBy(groupsUnsorted, (item) -> return item.prio) # get items of group for group in @groupsSorted items = App.Config.get(@configKey) itemsUnsorted = [] - for key, value of items - if value.controller - if value.parent is group.target - itemsUnsorted.push value + for key, item of items + if item.parent is group.target + if item.controller + if !item.role + itemsUnsorted.push item + else + match = _.include(item.role, 'Anybody') + if !match + for role in roles + if !match + match = _.include(item.role, role.name) + if match + itemsUnsorted.push item - group.items = _.sortBy( itemsUnsorted, (item) -> return item.prio ) + group.items = _.sortBy(itemsUnsorted, (item) -> return item.prio) # check last selected item selectedItem = undefined diff --git a/test/browser/prefereces_test.rb b/test/browser/prefereces_test.rb index 8ea3d87ad..1a3f53b4f 100644 --- a/test/browser/prefereces_test.rb +++ b/test/browser/prefereces_test.rb @@ -2,6 +2,60 @@ require 'browser_test_helper' class PreferencesTest < TestCase + def test_permission_agent + @browser = browser_instance + login( + username: 'master@example.com', + password: 'test', + url: browser_url, + ) + click( css: 'a[href="#current_user"]' ) + click( css: 'a[href="#profile"]' ) + match( + css: '.content .NavBarProfile', + value: 'Password', + ) + match( + css: '.content .NavBarProfile', + value: 'Language', + ) + match( + css: '.content .NavBarProfile', + value: 'Notifications', + ) + match( + css: '.content .NavBarProfile', + value: 'Calendar', + ) + end + + def test_permission_customer + @browser = browser_instance + login( + username: 'nicole.braun@zammad.org', + password: 'test', + url: browser_url, + ) + click( css: 'a[href="#current_user"]' ) + click( css: 'a[href="#profile"]' ) + match( + css: '.content .NavBarProfile', + value: 'Password', + ) + match( + css: '.content .NavBarProfile', + value: 'Language', + ) + match_not( + css: '.content .NavBarProfile', + value: 'Notifications', + ) + match_not( + css: '.content .NavBarProfile', + value: 'Calendar', + ) + end + def test_preferences @browser = browser_instance login(