Support Rails 6.1 #88
Conversation
In Rails 6.1, `messages` is an instance of `ActiveModel::DeprecationHandlingMessageArray`. So can't handle with the Reform's `filter_for`.
| @@ -143,7 +143,8 @@ def to_s | |||
|
|
|||
| def add(key, error_text) | |||
| # use rails magic to get the correct error_text and make sure we still update details and fields | |||
| text = @amv_errors.add(key, error_text) | |||
| error = @amv_errors.add(key, error_text) | |||
| error = [error.message] if error.respond_to?(:message) | |||
There was a problem hiding this comment.
What's happening when error doesn't have a message method? Is that Rails < 6.1? I'd refrain from introducing #respond_to in TRB gems. https://youtu.be/mjsnd8dJbew?t=1113
There was a problem hiding this comment.
Unfortunately, in the case of Rails < 6.1, error doesn't have a message method and causes an error.
NoMethodError: undefined method `message' for ["error_text"]:Array
lib/reform/form/active_model/validations.rb:147:in `add'
I'd refrain from introducing #respond_to in TRB gems
Oh, I see. So what do you think about checking Rails' version?
There was a problem hiding this comment.
I updated to use is_a? instead of respond_to?.
Now `ActiveModel::Errors#add` returns an instance of `ActiveModel::Error`. So can't use it as is.
`ActiveModel::Errors#messages` now only returns error attributes.
y-yagi
force-pushed
the
support-rails61
branch
from
4f89aa9
to
bc73f89
Compare
|
Hi, is there anything blocking this PR from being merged and released? I would very much like to upgrade our app to rails 6.1 and this is the final blocker. Thanks. |
|
Yup. This is the thing that is also preventing us from upgrading to Rails 6.1 as well |
|
Awesome, thanks! |
|
Yup, we're now happily upgraded to rails 6.1. Many thanks @y-yagi @seuros and @apotonick! |
| nested_errors = v.select { |attr_key, val| attr_key.is_a?(Integer) && val.is_a?(Array) && val.any? } | ||
| v = nested_errors.to_a if nested_errors.any? | ||
| end | ||
| v.is_a?(Array) || v.class.to_s == "ActiveModel::DeprecationHandlingMessageArray" |
There was a problem hiding this comment.
I did similar as a monkey patch to get our 6.1 upgrade in. Curious why v.class.to_s vs v.is_a?(ActiveModel::DeprecationHandlingMessageArray)?
is_a?() seems more direct than stringifying the class name and comparing the string.
There was a problem hiding this comment.
v.is_a?(ActiveModel::DeprecationHandlingMessageArray doesn' work on Rails <= 6.0. It will raise NameError.
The behavior of Active Model errors was changed by rails/rails#32313.
This PR has fixed to accommodate that change.
Fixes #86.