correct handling of errors with source/pointer "/data" (#277)

correct handling of source/parameter

Author: senid231

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
 

lib/json_api_client/resource.rb

@@ -502,11 +502,8 @@ def relationships_for_serialization
 
     def fill_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
     end
   end

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