NumericalityValidator support for activemodel

[ggpasqualino]

In the commit rails/rails@f888fad it was introduced the method record_attribute_changed_in_place? to ActiveModel::Validations::NumericalityValidator which calls record.attribute_changed_in_place?.
The intention was that if this method was added by ActiveRecord::AttributeMethods::Dirty then it would be called.
The problem is that Her::Model doesn't include that and doesn't implement that but it responds to that because it assumes on https://github.com/remiprev/her/blob/master/lib/her/model/attributes.rb#L74 that attribute_changed_in_place is a new attribute.

Her version: 0.8.1
Active model version: 4.2.4

[glappen]

I think I'm running into this also when I try to use "validates_on_numericality_of":

Failure/Error: should validate_presence_of(f)
     ArgumentError:
       wrong number of arguments (2 for 1)
     # /gems/2.2.0/gems/her-0.8.1/lib/her/model/attributes.rb:173:in `attribute?'
     # /gems/2.2.0/gems/activemodel-4.2.1/lib/active_model/attribute_methods.rb:385:in `attribute_changed_in_place?'
     # /gems/2.2.0/gems/activemodel-4.2.1/lib/active_model/validations/numericality.rb:98:in `record_attribute_changed_in_place?'
     # /gems/2.2.0/gems/activemodel-4.2.1/lib/active_model/validations/numericality.rb:26:in `validate_each'
     # /gems/2.2.0/gems/activemodel-4.2.1/lib/active_model/validator.rb:151:in `block in validate'
     # /gems/2.2.0/gems/activemodel-4.2.1/lib/active_model/validator.rb:148:in `each'
     # /gems/2.2.0/gems/activemodel-4.2.1/lib/active_model/validator.rb:148:in `validate'
     # /gems/2.2.0/gems/activesupport-4.2.1/lib/active_support/callbacks.rb:455:in `public_send'
     # /gems/2.2.0/gems/activesupport-4.2.1/lib/active_support/callbacks.rb:455:in `block in make_lambda'
     # /gems/2.2.0/gems/activesupport-4.2.1/lib/active_support/callbacks.rb:192:in `call'
     # /gems/2.2.0/gems/activesupport-4.2.1/lib/active_support/callbacks.rb:192:in `block in simple'
     # /gems/2.2.0/gems/activesupport-4.2.1/lib/active_support/callbacks.rb:504:in `call'
     # /gems/2.2.0/gems/activesupport-4.2.1/lib/active_support/callbacks.rb:504:in `block in call'
     # /gems/2.2.0/gems/activesupport-4.2.1/lib/active_support/callbacks.rb:504:in `each'
     # /gems/2.2.0/gems/activesupport-4.2.1/lib/active_support/callbacks.rb:504:in `call'

If someone can give me a little direction, I'd be happy to help fix this as we need it to be working for our project ASAP. Thanks!

[kritik]

Here's two options to fix it:

1. monkey patch for number-validation https://gist.github.com/kritik/2c98b73a390c557b2d1a. It works for me
2. fix her to use ActiveRecord::AttributeSet

[glappen]

I actually fixed it in Her and I have a pull request pending approval.

[kritik]

Can you add a link here with you pull request?

[glappen]

Yeah, I should have included this before. My pull request is here: #383

[hubert]

hi @glappen first off, thanks for the contribution. i've been on the road for a bit so haven't had much time to review PRs. apologies for the slow reply

after looking at your PR and @kritik suggestion, i fear that a solution of this nature might just paper over the root cause -- that Her::Model is not compliant with ActiveModel. would you want to take a stab at a solution using that approach?

[glappen]

I would be willing to take a stab at it. Can you elaborate a bit more
about what you have in mind, and what would be involved in becoming
ActiveModel compliant? Thanks!
On Nov 6, 2015 9:44 PM, "hubert huang" notifications@github.com wrote:

hi @glappen https://github.com/glappen first off, thanks for the
contribution. i've been on the road for a bit so haven't had much time to
review PRs. apologies for the slow reply

after looking at your PR and @kritik https://github.com/kritik
suggestion, i fear that a solution of this nature might just paper over the
root cause -- that Her::Model is not compliant with ActiveModel. would you
want to take a stab at a solution using that approach?

—
Reply to this email directly or view it on GitHub
#382 (comment).

[glappen]

@hubert - Following up as I haven't heard anything (maybe because I forgot to mention you in my response). Are you suggesting @kritik 1st of 2nd option? monkey patch number validation or implement ActiveRecord::AttributeSet? Can you explain how implementing ActiveRecord::AttributeSet solves the problem here? Thanks!

[hubert]

hi @glappen. i understand your confusion. we def shouldn't be pulling in anything from ActiveRecord. when i looked at it earlier, i must've misread it as ActiveModel. my issue with the solution may be that i don't like the way the situation with record_attributed_in_place is handled in ActiveModel.

let me look a bit more closely and i'll get back to you.