From 41cb8ca2edb3217100119a5b61f0438c108b1e5e Mon Sep 17 00:00:00 2001 From: Denis Talakevich Date: Fri, 10 Nov 2017 12:01:02 +0200 Subject: [PATCH] correct handling of errors with source/pointer "/data" correct handling of source/parameter --- lib/json_api_client/error_collector.rb | 29 ++++--- lib/json_api_client/resource.rb | 7 +- test/unit/error_collector_test.rb | 101 ++++++++++++++++++++++++- 3 files changed, 121 insertions(+), 16 deletions(-) diff --git a/lib/json_api_client/error_collector.rb b/lib/json_api_client/error_collector.rb index e18931c4..5fbc777c 100644 --- a/lib/json_api_client/error_collector.rb +++ b/lib/json_api_client/error_collector.rb @@ -33,18 +33,27 @@ def detail end def source_parameter - source.fetch(:parameter) do - source[:pointer] ? - source[:pointer].split("/").last : - nil - end + source[:parameter] end def source_pointer - source.fetch(:pointer) do - source[:parameter] ? - "/data/attributes/#{source[:parameter]}" : - nil + source[:pointer] + end + + def error_key + if source_pointer && source_pointer != "/data" + source_pointer.split("/").last + else + "base" + end + end + + def error_msg + msg = title || detail || "invalid" + if source_parameter + "#{source_parameter} #{msg}" + else + msg end end @@ -74,7 +83,7 @@ def full_messages def [](source) map do |error| - error.source_parameter == source + error.error_key == source end end diff --git a/lib/json_api_client/resource.rb b/lib/json_api_client/resource.rb index d00fdd8e..6517c806 100644 --- a/lib/json_api_client/resource.rb +++ b/lib/json_api_client/resource.rb @@ -411,11 +411,8 @@ def save if last_result_set.has_errors? last_result_set.errors.each do |error| - if error.source_parameter - errors.add(self.class.key_formatter.unformat(error.source_parameter), error.title || error.detail) - else - errors.add(:base, error.title || error.detail) - end + key = self.class.key_formatter.unformat(error.error_key) + errors.add(key, error.error_msg) end false else diff --git a/test/unit/error_collector_test.rb b/test/unit/error_collector_test.rb index 4985436b..6db9f08d 100644 --- a/test/unit/error_collector_test.rb +++ b/test/unit/error_collector_test.rb @@ -94,6 +94,9 @@ def test_can_handle_errors assert article.errors.present? assert_equal 2, article.errors.size + assert_equal ["Email address is invalid."], article.errors[:email_address] + assert_equal ["Title already taken"], article.errors[:base] + error = article.last_result_set.errors.first assert_equal "1234-abcd", error.id assert_equal "http://example.com/help/errors/1337", error.about @@ -102,7 +105,7 @@ def test_can_handle_errors assert_equal "Email address is invalid.", error.title assert_equal "Email address 'bar' is not a valid email address.", error.detail assert_equal "/data/attributes/email_address", error.source_pointer - assert_equal "email_address", error.source_parameter + assert_nil error.source_parameter assert error.meta.is_a?(JsonApiClient::MetaData) assert_equal "asdf", error.meta.qwer @@ -118,6 +121,100 @@ def test_can_handle_errors assert error.meta.is_a?(JsonApiClient::MetaData) end + def test_can_handle_generic_error + stub_request(:post, "http://example.com/articles") + .with(headers: {content_type: "application/vnd.api+json", accept: "application/vnd.api+json"}, body: { + data: { + type: "articles", + attributes: { + title: "Rails is Omakase", + email_address: "bar" + } + } + }.to_json) + .to_return(headers: {content_type: "application/vnd.api+json"}, body: { + errors: [ + { + id: "1234-abcd", + status: "422", + code: "1337", + title: "You can't create this record", + detail: "You can't create this record", + source: { + pointer: "/data" + } + } + ] + }.to_json) + + article = Article.create({ + title: "Rails is Omakase", + email_address: "bar" + }) + refute article.persisted? + assert article.errors.present? + assert_equal 1, article.errors.size + + assert_equal ["You can't create this record"], article.errors[:base] + + error = article.last_result_set.errors.first + assert_equal "1234-abcd", error.id + assert_nil error.about + assert_equal "422", error.status + assert_equal "1337", error.code + assert_equal "You can't create this record", error.title + assert_equal "You can't create this record", error.detail + assert_equal "/data", error.source_pointer + assert_nil error.source_parameter + end + + def test_can_handle_parameter_error + stub_request(:post, "http://example.com/articles") + .with(headers: {content_type: "application/vnd.api+json", accept: "application/vnd.api+json"}, body: { + data: { + type: "articles", + attributes: { + title: "Rails is Omakase", + email_address: "bar" + } + } + }.to_json) + .to_return(headers: {content_type: "application/vnd.api+json"}, body: { + errors: [ + { + id: "1234-abcd", + status: "400", + code: "1337", + title: "bar is required", + detail: "bar include is required for creation", + source: { + parameter: "include" + } + } + ] + }.to_json) + + article = Article.create({ + title: "Rails is Omakase", + email_address: "bar" + }) + refute article.persisted? + assert article.errors.present? + assert_equal 1, article.errors.size + + assert_equal ["include bar is required"], article.errors[:base] + + error = article.last_result_set.errors.first + assert_equal "1234-abcd", error.id + assert_nil error.about + assert_equal "400", error.status + assert_equal "1337", error.code + assert_equal "bar is required", error.title + assert_equal "bar include is required for creation", error.detail + assert_nil error.source_pointer + assert_equal "include", error.source_parameter + end + def test_can_handle_explicit_null_error_values stub_request(:post, "http://example.com/articles") .with(headers: {content_type: "application/vnd.api+json", accept: "application/vnd.api+json"}, body: { @@ -152,6 +249,8 @@ def test_can_handle_explicit_null_error_values assert article.errors.present? assert_equal 1, article.errors.size + assert_equal ["invalid"], article.errors[:base] + error = article.last_result_set.errors.first assert_equal "1337", error.id