serialize_errors support claims for activelmodel are wrong

[HoneyryderChuck]

From https://github.com/fotinakis/jsonapi-serializers/blob/master/lib/jsonapi-serializers/serializer.rb#L340-L356

The problem is, the lib assumes that the errors hash is an activemodel hash if it responds to #to_hash and #full_messages. First there's nowhere in the lint spec from activel model regarding these methods, so it's a false assumption. Second, many objects implement #to_hash without arguments, and this gem assumes one hash argument with support for :full_messages key, which is also not part of the spec.

These assumptions make this gem fail for the sequel ORM, which is ActiveModel-compliant, has an errors object, but this doesn't support #to_hash with arguments.

I think the lib should add suport explicitly for active record error hashes and fallback to something else.

[fotinakis]

Perhaps @green-arrow or @mwpastore can comment on this, I believe they use Sequel with this library.

[mwpastore]

@TiagoCardoso1983 Are you using the :active_model Sequel plugin, then?

[HoneyryderChuck]

@mwpastore yes.

[mwpastore]

@TiagoCardoso1983 And forgive me for asking a stupid question, but do you actually need that plugin? 😄

[HoneyryderChuck]

@mwpastore yes. Did you ever try to use sequel in rails with all those helpers spread across? I did.

[mwpastore]

Ah, okay, so you're doing sequel-rails. I haven't encountered this problem because I don't use the :active_model plugin, and I tend to stay away from Rails in general, but this discrepancy you've noted is interesting. Since it's not in the ActiveModel spec, I doubt jeremyevans will be open to changing the behavior of Sequel. If you disable this check in jsonapi-serializers does it serialize your errors the way you want it to?

I don't want to drive traffic away from jsonapi-serializers (because it's a great project that I use all the time) but you might want to take a quick look at jsonapi-resources. It's more Rails-centric and I know it's been proofed-of-concept with sequel-rails.

[HoneyryderChuck]

let's just say that this is a migration path to move a certain app out of the rails. Yes, I'm using sequel-rails, which doesn't include activemodel by default (I'm doing it because I needed it for the helpers). If I'd choose to stay on the rails, I'd probably have gone with activemodel-serializers (which by the way, is also not activemodel-lint complaint, it actually has its own linter, because there is no one true activemodel(?) :) ).

I was just of the impression that this activemodel thing was to be able to build on top of APIs agreed upon by the ORMs. Sadly a lot of use-cases fall out of the activemodel scope, and errors is just one of them. Which is alright, there is no "activemodel" in this gem's name.

The problem is more about relying on object duck-typing to imply it's an expected type of errors container. If you want to fallback to a solution which adheres to activerecord that's fine, but you could just use defined?(ActiveModel) and errors.is_a?(ActiveModel::Errors) instead of checking if it responds to those two methods. I've worked around this by transforming sequel errors object into an hash this gem could work with.

[fotinakis]

Is there a way to write our error handling to support sequel-rails ActiveModel errors as well as Rails ActiveModel errors at the same time?

If not, I'm fine with a more explicit type-check to replace the duck-typing check in this case—I usually just prefer duck-typing so I write it that way, but if it's causing this many problems with Sequel then happy to accept a PR to make it more explicit.

[HoneyryderChuck]

@fotinakis, I'd saved myself the work of supporting the sequel errors object. sequel is a fairly stable gem if you decide to go down that path, but you'll be subject to support in case sequel rewrites the API of the errors object. In fact, you'll have worries if you claim sequel-compatibility and someone is still on sequel 2/3 (I don't know if the API changed, but then again, no guarantees).

I think this gem could try to see if ActiveModel is loaded and the errors object is of the expected type, and then fallback to the array of hashes. Duck-typing won't do, because of the similar method but different expectations. The main Readme does express the ActiveRecord case, so I'd just keep it there, remove the active model claims (if you mean the spec-not-the-gem) and maybe document how this could be done for other, ex. sequel (I could provide you with a snippet, if you prefer this).

[bgentry]

Funny, I also just ran into this. I had issues with ActiveModelSerializers' ErrorSerializer as well and ended up hacking it myself to get it working:

module ActiveModel
  class Serializer
    class ErrorSerializer < ActiveModel::Serializer
      # @return [Hash<field_name,Array<error_message>>]
      def as_json
        # this line was changed away from `object.messages`
        object.errors
      end

      def success?
        false
      end
    end
  end
end

👍 for an approach that falls back to an ActiveModel-compliant error serializer

Edit: the format that results from the AMS error serializer is this:

{"errors": [{"source": {"pointer": "/data/attributes/name"}, "detail": "is not present"}]}

[fotinakis]

@bgentry ActiveModelSerializers is unrelated to this project, I think your hack might be unrelated? I might be crazy.

[bgentry]

@fotinakis Yes, it is for a different project. I was offering it up as an example of what a similar project does to handle errors, and how I was able to make their error serializer work with Sequel errors. Presumably a similar solution would be feasible for jsonapi-serializers.

[fotinakis]

Gotcha, thanks!

[fotinakis]

It's curious that AMS had a similar issue.

[bgentry]

@fotinakis yeah, out of the box AMS is not compatible with Sequel errors either. I made that tweak late last night and didn't get around to submitting a PR because I ran into other issues with AMS and started looking elsewhere.