From ec55c8130299067eda77b5f97c9c2a934e53399b Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Fri, 9 Sep 2016 13:29:33 +0200 Subject: [PATCH] Improved error codes on model validation. --- app/controllers/application_controller.rb | 7 ++++++- app/models/application_model.rb | 18 +++++++++--------- app/models/ticket.rb | 4 ++++ test/controllers/tickets_controller_test.rb | 8 ++++---- 4 files changed, 23 insertions(+), 14 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index af5720033..2a91a648a 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -21,6 +21,8 @@ class ApplicationController < ActionController::Base rescue_from StandardError, with: :server_error rescue_from ExecJS::RuntimeError, with: :server_error rescue_from ActiveRecord::RecordNotFound, with: :not_found + rescue_from ActiveRecord::StatementInvalid, with: :unprocessable_entity + rescue_from ActiveRecord::RecordInvalid, with: :unprocessable_entity rescue_from ArgumentError, with: :unprocessable_entity rescue_from Exceptions::UnprocessableEntity, with: :unprocessable_entity rescue_from Exceptions::NotAuthorized, with: :unauthorized @@ -378,7 +380,7 @@ class ApplicationController < ActionController::Base params.delete(:form_id) # check min. params - raise 'Need at least article: { body: "some text" }' if !params[:body] + raise Exceptions::UnprocessableEntity, 'Need at least article: { body: "some text" }' if !params[:body] # fill default values if params[:type_id].empty? && params[:type].empty? @@ -639,6 +641,9 @@ class ApplicationController < ActionController::Base data = { error: error } + if error =~ /Validation failed: (.+?)(,|$)/i + data[:error_human] = $1 + end if error =~ /(already exists|duplicate key|duplicate entry)/i data[:error_human] = 'Object already exists!' end diff --git a/app/models/application_model.rb b/app/models/application_model.rb index bb29ff5d6..5b5a4a61b 100644 --- a/app/models/application_model.rb +++ b/app/models/application_model.rb @@ -76,7 +76,7 @@ returns end if params.nil? - raise "No params for #{self}!" + raise ArgumentError, "No params for #{self}!" end data = {} @@ -131,7 +131,7 @@ returns # complain if we found no reference if !lookup - raise "No value found for '#{assoc.name}' with id #{item_id.inspect}" + raise ArgumentError, "No value found for '#{assoc.name}' with id #{item_id.inspect}" end list.push item_id } @@ -165,7 +165,7 @@ returns # complain if we found no reference if !lookup - raise "No lookup value found for '#{assoc.name}': #{value.inspect}" + raise ArgumentError, "No lookup value found for '#{assoc.name}': #{value.inspect}" end list.push lookup.id } @@ -350,7 +350,7 @@ returns lookup = class_object.lookup(email: value) end else - raise "String is needed as ref value #{value.inspect} for '#{assoc.name}'" + raise ArgumentError, "String is needed as ref value #{value.inspect} for '#{assoc.name}'" end else lookup = class_object.lookup(name: value) @@ -358,7 +358,7 @@ returns # complain if we found no reference if !lookup - raise "No lookup value found for '#{assoc.name}': #{value.inspect}" + raise ArgumentError, "No lookup value found for '#{assoc.name}': #{value.inspect}" end # release data value @@ -393,7 +393,7 @@ returns lookup = class_object.lookup(email: item) end else - raise "String is needed in array ref as ref value #{value.inspect} for '#{assoc.name}'" + raise ArgumentError, "String is needed in array ref as ref value #{value.inspect} for '#{assoc.name}'" end else lookup = class_object.lookup(name: item) @@ -401,7 +401,7 @@ returns # complain if we found no reference if !lookup - raise "No lookup value found for '#{assoc.name}': #{item.inspect}" + raise ArgumentError, "No lookup value found for '#{assoc.name}': #{item.inspect}" end lookup_ids.push lookup.id } @@ -626,7 +626,7 @@ returns return end - raise 'Need name, id, login or email for lookup()' + raise ArgumentError, 'Need name, id, login or email for lookup()' end =begin @@ -801,7 +801,7 @@ returns record.save return record else - raise 'Need name, login, email or locale for create_or_update()' + raise ArgumentError, 'Need name, login, email or locale for create_or_update()' end end diff --git a/app/models/ticket.rb b/app/models/ticket.rb index 3e8e49c8c..52d8e698a 100644 --- a/app/models/ticket.rb +++ b/app/models/ticket.rb @@ -20,6 +20,10 @@ class Ticket < ApplicationModel before_update :check_defaults, :check_title, :reset_pending_time, :check_escalation_update before_destroy :destroy_dependencies + validates :group_id, presence: true + validates :priority_id, presence: true + validates :state_id, presence: true + notify_clients_support latest_change_support diff --git a/test/controllers/tickets_controller_test.rb b/test/controllers/tickets_controller_test.rb index c07826e98..ccfcac63f 100644 --- a/test/controllers/tickets_controller_test.rb +++ b/test/controllers/tickets_controller_test.rb @@ -62,10 +62,10 @@ class TicketsControllerTest < ActionDispatch::IntegrationTest }, } post '/api/v1/tickets', params.to_json, @headers.merge('Authorization' => credentials) - assert_response(500) + assert_response(422) result = JSON.parse(@response.body) assert_equal(Hash, result.class) - assert_equal('Attribute \'group_id\' required!', result['error_human']) + assert_equal('Group can\'t be blank', result['error_human']) end test '01.02 ticket create with agent - wrong group' do @@ -81,7 +81,7 @@ class TicketsControllerTest < ActionDispatch::IntegrationTest }, } post '/api/v1/tickets', params.to_json, @headers.merge('Authorization' => credentials) - assert_response(500) + assert_response(422) result = JSON.parse(@response.body) assert_equal(Hash, result.class) assert_equal('No lookup value found for \'group\': "not_existing"', result['error']) @@ -98,7 +98,7 @@ class TicketsControllerTest < ActionDispatch::IntegrationTest article: {}, } post '/api/v1/tickets', params.to_json, @headers.merge('Authorization' => credentials) - assert_response(500) + assert_response(422) result = JSON.parse(@response.body) assert_equal(Hash, result.class) assert_equal('Need at least article: { body: "some text" }', result['error'])