Add response errors to ActiveModel using ActiveModel::Errors

[lleger]

What
This commit adds any response errors (such as those from calling save) to the ActiveModel::Error collection on the resource.

Why
Doing so enables accessing the errors hash via the errors attribute and allows the use of helper methods, such as full_messages. This also makes the Her::Model act more like an ActiveRecord object.

How
If the response errors are in an AR-like hash, then they're iterated over and added to the model via add on ActiveRecord::Error. For example:

# from remote model
{ errors => @user.errors }.to_json

# parsed in @response_errors
{ :errors => { :name => ["should not be blank"] } }

This PR includes passing tests and has been integrated into a production app. This was briefly discussed in #138.

Example

class User
  include Her::Model
  # validates :name, presence: true (on remote model)
end

@user = User.new
@user.save

@user.errors # => { :name => ["can't be blank"] }
@user.errors.any? # => true
@user.errors.full_messages # => ["Name can't be blank"]

[lleger]

Some of the tests randomly failed on 1.8.7. Fixed in 0f27448, so it looks like we're all green now.

[pencil]

I'm not sure if we should mix the local validation messages with the remote validation messages... @remiprev, your thoughts on this one?

[remi]

I also think that we shouldn't mix local and remote errors.

Validations defined through ActiveModel::Validations go into #errors (like any ActiveModel-like object) and errors returned by the response go into #response_errors (this is the behavior added by Her).

[lleger]

I'm not sure I understand the dichotomy. If the error happens locally thanks to an ActiveModel validation on the Her model or remotely on the actual ActiveRecord resource, I still want to be able to display those errors in the same place and in the same way.

I don't think this does anything more than what Her is trying to do, it just enhances the functionality that's already there. I'm already getting the errors from the response, but now I get them in a nicely wrapped instance that's familiar and that I'm already interfacing with for the ActiveModel validations.

That being said, is there some version of this functionality that you would consider merging? For example, if instead of adding it to the base errors we transformed the response_errors hash into an ActiveModel::Error object, would that be agreeable?

I believe this is good functionality and I'm open to alternative implementations in order to see its way into the codebase.

[anicholson]

+1 to @lleger , I don't see the dichotomy either.

[pencil]

Guess you are right. But we can't be sure, that the errors from the server are always a Hash. What if they are just a simple Array. Or even worse: A String?

[lleger]

We perhaps could ignore any ill-formatted errors, but as a matter of convention I'd imagine, if you're sending the errors back over the wire, then it'd make sense to send them back as a hash (otherwise there'd be no relation between the error message and the attribute).

[pencil]

I'm sorry, but it doesn't work that way. We can not rely on something because "it would make sense". her is designed to work with as many APIs as possible and we can therefore not enforce such strong policies.

[lleger]

I'm struggling to consider a way to implement this if the response errors aren't formatted as a hash. It would be difficult to relate an error message to an attribute if we get an array or a string. Do you have any recommendations?

It currently works fine with how Rails and AR serialize a failed validation back to JSON, but that's the only API I've tested it with.

[lleger]

We've had some lively discussion on this, and I'd like to see it get merged in, so I'm pinging y'all back to see if there's some manner we can get this functionality into Her.

[pencil]

The suggested solution is only compatible with how Rails encodes the errors. Other APIs may use other formats and I can't think of an easy way to tackle this without having a confusing programming API.

As initially mentioned by me and @remiprev, mixing local and remote validation messages will open Pandora's box. Not only because of the issues we discussed so far. It would probably also be quite confusing for the programmers.

I think using custom code to handle this in your application as needed is the right approach.

[lleger]

Unfortunate—was really hoping we could get this merged. It's a splendid addition for our usage. In case anyone finds a similar need, we'll keep our fork maintained.

[noctivityinc]

@lleger I know this is an old issue but I completely agree with you. Id like to try to integrate the same logic using a callback but unfortunately the callback is cancelled if an error occurs. How did you eventually handle this?

[lleger]

@noctivityinc To be honest, this is quite old. We abandoned Her not long after this in favor of greener pastures. Our fork has the code we ended up using, though, and you're free to use it if it's helpful. Good luck!

[noctivityinc]

Thanks man. What did you decided to use in place of Her for consuming RESTful APIs in Rails, or are you using something else now on the front end that is better suited? Best, Josh

…

On Thu, Oct 18, 2018 at 8:56 PM Logan Leger ***@***.***> wrote: @noctivityinc <https://github.com/noctivityinc> To be honest, this is quite old. We abandoned Her not long after this in favor of greener pastures. Our fork has the code we ended up using, though, and you're free to use it if it's helpful. Good luck! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#178 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAE0AvTJX5hJWwCVGf4fQTReF0edLNrHks5umSNLgaJpZM4BEqDx> .

[lleger]

We ended up rewriting the app in question to use a React frontend that just straight consumed a Rails API. Much better experience than trying to consume an API on the backend and push back a response.

[noctivityinc]

Thanks. Ive been thinking of learning React for that exact purpose. I wrote the backend Rails API, which works great, but consuming it in a Rails app with pseudo ActiveRecord code is proving to be *interesting*

…

On Thu, Oct 18, 2018 at 9:02 PM Logan Leger ***@***.***> wrote: We ended up rewriting the app in question to use a React frontend that just straight consumed a Rails API. Much better experience than trying to consume an API on the backend and push back a response. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#178 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAE0ArrjF47U19HicsRWMa9MI4QzdaX7ks5umSSugaJpZM4BEqDx> .