Skip to content

Commit

Permalink
Fix Errors#from_array refactor sorting the attributes keys
Browse files Browse the repository at this point in the history
  • Loading branch information
salrepe committed Jun 21, 2012
1 parent da3236c commit d8864c4
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 11 deletions.
3 changes: 1 addition & 2 deletions lib/active_resource/validations.rb
Expand Up @@ -15,12 +15,11 @@ def from_array(messages, save_cache = false)
clear unless save_cache clear unless save_cache
humanized_attributes = Hash[@base.attributes.keys.map { |attr_name| [attr_name.humanize, attr_name] }] humanized_attributes = Hash[@base.attributes.keys.map { |attr_name| [attr_name.humanize, attr_name] }]
messages.each do |message| messages.each do |message|
attr_message = humanized_attributes.keys.detect do |attr_name| attr_message = humanized_attributes.keys.sort_by { |a| -a.length }.detect do |attr_name|
if message[0, attr_name.size + 1] == "#{attr_name} " if message[0, attr_name.size + 1] == "#{attr_name} "
add humanized_attributes[attr_name], message[(attr_name.size + 1)..-1] add humanized_attributes[attr_name], message[(attr_name.size + 1)..-1]
end end
end end

self[:base] << message if attr_message.nil? self[:base] << message if attr_message.nil?
end end
end end
Expand Down
26 changes: 17 additions & 9 deletions test/cases/base_errors_test.rb
Expand Up @@ -4,8 +4,8 @@
class BaseErrorsTest < ActiveSupport::TestCase class BaseErrorsTest < ActiveSupport::TestCase
def setup def setup
ActiveResource::HttpMock.respond_to do |mock| ActiveResource::HttpMock.respond_to do |mock|
mock.post "/people.xml", {}, %q(<?xml version="1.0" encoding="UTF-8"?><errors><error>Age can't be blank</error><error>Name can't be blank</error><error>Name must start with a letter</error><error>Person quota full for today.</error></errors>), 422, {'Content-Type' => 'application/xml; charset=utf-8'} mock.post "/people.xml", {}, %q(<?xml version="1.0" encoding="UTF-8"?><errors><error>Age can't be blank</error><error>Name can't be blank</error><error>Name must start with a letter</error><error>Person quota full for today.</error><error>Phone work can't be blank</error><error>Phone is not valid</error></errors>), 422, {'Content-Type' => 'application/xml; charset=utf-8'}
mock.post "/people.json", {}, %q({"errors":{"age":["can't be blank"],"name":["can't be blank", "must start with a letter"],"person":["quota full for today."]}}), 422, {'Content-Type' => 'application/json; charset=utf-8'} mock.post "/people.json", {}, %q({"errors":{"age":["can't be blank"],"name":["can't be blank", "must start with a letter"],"person":["quota full for today."],"phone_work":["can't be blank"],"phone":["is not valid"]}}), 422, {'Content-Type' => 'application/json; charset=utf-8'}
end end
end end


Expand All @@ -21,7 +21,7 @@ def test_should_parse_json_and_xml_errors
[ :json, :xml ].each do |format| [ :json, :xml ].each do |format|
invalid_user_using_format(format) do invalid_user_using_format(format) do
assert_kind_of ActiveResource::Errors, @person.errors assert_kind_of ActiveResource::Errors, @person.errors
assert_equal 4, @person.errors.size assert_equal 6, @person.errors.size
end end
end end
end end
Expand All @@ -43,6 +43,8 @@ def test_should_parse_errors_to_individual_attributes
assert @person.errors[:name].any? assert @person.errors[:name].any?
assert_equal ["can't be blank"], @person.errors[:age] assert_equal ["can't be blank"], @person.errors[:age]
assert_equal ["can't be blank", "must start with a letter"], @person.errors[:name] assert_equal ["can't be blank", "must start with a letter"], @person.errors[:name]
assert_equal ["can't be blank"], @person.errors[:phone_work]
assert_equal ["is not valid"], @person.errors[:phone]
assert_equal ["Person quota full for today."], @person.errors[:base] assert_equal ["Person quota full for today."], @person.errors[:base]
end end
end end
Expand Down Expand Up @@ -76,14 +78,16 @@ def test_should_format_full_errors
assert full.include?("Name can't be blank") assert full.include?("Name can't be blank")
assert full.include?("Name must start with a letter") assert full.include?("Name must start with a letter")
assert full.include?("Person quota full for today.") assert full.include?("Person quota full for today.")
assert full.include?("Phone is not valid")
assert full.include?("Phone work can't be blank")
end end
end end
end end


def test_should_mark_as_invalid_when_content_type_is_unavailable_in_response_header def test_should_mark_as_invalid_when_content_type_is_unavailable_in_response_header
ActiveResource::HttpMock.respond_to do |mock| ActiveResource::HttpMock.respond_to do |mock|
mock.post "/people.xml", {}, %q(<?xml version="1.0" encoding="UTF-8"?><errors><error>Age can't be blank</error><error>Name can't be blank</error><error>Name must start with a letter</error><error>Person quota full for today.</error></errors>), 422, {} mock.post "/people.xml", {}, %q(<?xml version="1.0" encoding="UTF-8"?><errors><error>Age can't be blank</error><error>Name can't be blank</error><error>Name must start with a letter</error><error>Person quota full for today.</error><error>Phone work can't be blank</error><error>Phone is not valid</error></errors>), 422, {}
mock.post "/people.json", {}, %q({"errors":{"age":["can't be blank"],"name":["can't be blank", "must start with a letter"],"person":["quota full for today."]}}), 422, {} mock.post "/people.json", {}, %q({"errors":{"age":["can't be blank"],"name":["can't be blank", "must start with a letter"],"person":["quota full for today."],"phone_work":["can't be blank"],"phone":["is not valid"]}}), 422, {}
end end


[ :json, :xml ].each do |format| [ :json, :xml ].each do |format|
Expand All @@ -95,29 +99,33 @@ def test_should_mark_as_invalid_when_content_type_is_unavailable_in_response_hea


def test_should_parse_json_string_errors_with_an_errors_key def test_should_parse_json_string_errors_with_an_errors_key
ActiveResource::HttpMock.respond_to do |mock| ActiveResource::HttpMock.respond_to do |mock|
mock.post "/people.json", {}, %q({"errors":["Age can't be blank", "Name can't be blank", "Name must start with a letter", "Person quota full for today."]}), 422, {'Content-Type' => 'application/json; charset=utf-8'} mock.post "/people.json", {}, %q({"errors":["Age can't be blank", "Name can't be blank", "Name must start with a letter", "Person quota full for today.", "Phone work can't be blank", "Phone is not valid"]}), 422, {'Content-Type' => 'application/json; charset=utf-8'}
end end


assert_deprecated(/as an array/) do assert_deprecated(/as an array/) do
invalid_user_using_format(:json) do invalid_user_using_format(:json) do
assert @person.errors[:name].any? assert @person.errors[:name].any?
assert_equal ["can't be blank"], @person.errors[:age] assert_equal ["can't be blank"], @person.errors[:age]
assert_equal ["can't be blank", "must start with a letter"], @person.errors[:name] assert_equal ["can't be blank", "must start with a letter"], @person.errors[:name]
assert_equal ["is not valid"], @person.errors[:phone]
assert_equal ["can't be blank"], @person.errors[:phone_work]
assert_equal ["Person quota full for today."], @person.errors[:base] assert_equal ["Person quota full for today."], @person.errors[:base]
end end
end end
end end


def test_should_parse_3_1_style_json_errors def test_should_parse_3_1_style_json_errors
ActiveResource::HttpMock.respond_to do |mock| ActiveResource::HttpMock.respond_to do |mock|
mock.post "/people.json", {}, %q({"age":["can't be blank"],"name":["can't be blank", "must start with a letter"],"person":["quota full for today."]}), 422, {'Content-Type' => 'application/json; charset=utf-8'} mock.post "/people.json", {}, %q({"age":["can't be blank"],"name":["can't be blank", "must start with a letter"],"person":["quota full for today."],"phone_work":["can't be blank"],"phone":["is not valid"]}), 422, {'Content-Type' => 'application/json; charset=utf-8'}
end end


assert_deprecated(/without a root/) do assert_deprecated(/without a root/) do
invalid_user_using_format(:json) do invalid_user_using_format(:json) do
assert @person.errors[:name].any? assert @person.errors[:name].any?
assert_equal ["can't be blank"], @person.errors[:age] assert_equal ["can't be blank"], @person.errors[:age]
assert_equal ["can't be blank", "must start with a letter"], @person.errors[:name] assert_equal ["can't be blank", "must start with a letter"], @person.errors[:name]
assert_equal ["is not valid"], @person.errors[:phone]
assert_equal ["can't be blank"], @person.errors[:phone_work]
assert_equal ["Person quota full for today."], @person.errors[:base] assert_equal ["Person quota full for today."], @person.errors[:base]
end end
end end
Expand All @@ -127,11 +135,11 @@ def test_should_parse_3_1_style_json_errors
def invalid_user_using_format(mime_type_reference) def invalid_user_using_format(mime_type_reference)
previous_format = Person.format previous_format = Person.format
Person.format = mime_type_reference Person.format = mime_type_reference
@person = Person.new(:name => '', :age => '') @person = Person.new(:name => '', :age => '', :phone => '', :phone_work => '')
assert_equal false, @person.save assert_equal false, @person.save


yield yield
ensure ensure
Person.format = previous_format Person.format = previous_format
end end
end end

0 comments on commit d8864c4

Please sign in to comment.