ruby-grape / grape-active_model_serializers

User active_model_serializers with Grape
MIT License
140 stars 68 forks source link

add custom error formatter #76

Closed xn closed 7 years ago

xn commented 7 years ago

Would y'all be interested in having an error formatter that would allow for a jsonapi response?

If so, I can add some tests and documentation to this.

dblock commented 7 years ago

I think the answer is yes, we want a custom formatter for error messages, please.

xn commented 7 years ago

Kk, leaving later today to go camping. I'll finish this out Monday or so when I get back.

xn commented 7 years ago

@dblock Rubocop is going to complain about the method signature of #call. I have no control over that. What should I do?

xn commented 7 years ago

I don't know what you want to do about 0.8. Please advise.

dblock commented 7 years ago

For rubocop run rubocop -a ; rubocop --auto-gen-config.

xn commented 7 years ago

kk, constraining to 1.0 works, but breaks the others.

dblock commented 7 years ago

Oh I see. Make this conditional, in the case Grape::JSON is defined you can use that, otherwise use MultiJson, which was required in older versions of Grape.

xn commented 7 years ago

Finally! I think this is ready to go.

xn commented 7 years ago

This should be it.

dblock commented 7 years ago

Code looks great. You still have to add something to README!

xn commented 7 years ago

Would this work?

Tell your API to use Grape::Formatter::ActiveModelSerializers

class API < Grape::API
  format :json
  formatter :json, Grape::Formatter::ActiveModelSerializers

  # Serializes errors with ActiveModel::Serializer::ErrorSerializer if an ActiveModel.
  # Serializer conforms to format. ex: json, jsonapi
  error_formatter :json, Grape::Formatter::ActiveModelSerializers
end
dblock commented 7 years ago

Something like that, an explain what these output.

xn commented 7 years ago

You want me to document the behavior of ActiveModel::Serializer::ErrorSerializer?

dblock commented 7 years ago

Yes, something should explain what's happening with this new code that wasn't happening before.

xn commented 7 years ago

Does this work?

class API < Grape::API
  format :json
  formatter :json, Grape::Formatter::ActiveModelSerializers

  # Serializes errors with ActiveModel::Serializer::ErrorSerializer if an ActiveModel.
  # Serializer conforms to the adapter, ex: json, jsonapi.
  # So an error formatted with a jsonapi formatter would render as per:
  # http://jsonapi.org/format/#error-objects
  error_formatter :json, Grape::Formatter::ActiveModelSerializers
end
dblock commented 7 years ago

Great! As long as there's something in the README.

xn commented 7 years ago

That is from the readme.

xn commented 7 years ago

Under: Tell your API to use Grape::Formatter::ActiveModelSerializers

dblock commented 7 years ago

Oh you're saying we documented it but it was never implemented (sorry, I'm just cheerleading this repo :)).

I'll just merge this, any interest in doing the next release and helping out with this project?

dblock commented 7 years ago

And thanks for hanging on with my many nitpicky comments!

xn commented 7 years ago

No, that was a miscommunication. Let me do a quick PR for the Docs.