Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Numericality validation regression in 5.2.1 #33651

Closed
zarqman opened this issue Aug 19, 2018 · 2 comments
Closed

Numericality validation regression in 5.2.1 #33651

zarqman opened this issue Aug 19, 2018 · 2 comments

Comments

@zarqman
Copy link
Contributor

zarqman commented Aug 19, 2018

Steps to reproduce

https://gist.github.com/zarqman/075179677c21c7aa58d7a7ba3376bac9

Expected behavior

validates :quantity, numericality: {greater_than: 0} should return /not a number/ when given a value of "invalid".

This is how it worked up through 5.2.0.

Actual behavior

In 5.2.1, non-numeric user input, such as "invalid" is silently morphed to 0 (numeric zero) and then validated as such, incorrectly returning /must be greater than 0/ instead.

System configuration

Rails version: 5.2.1

Ruby version: 2.5.1

Mongoid version: 7.0.1

Other notes

Because mongoid relies on before_type_cast and does not implement came_from_user?, the change to eliminate use of before_type_cast in Rails 5.2.1 is a breaking change.

A post-5.2.1 commit (#33603) addresses a different regression from the change in 5.2.1, but does not resolve this one. Additionally, that commit might be incorrect in that it uses read_attribute(attr_name) as a fallback, preempting use of value which was produced using read_attribute_for_validation(attr_name) (which would seem preferable since raw_value is being used for validation). It's possible read_attribute_for_validation itself needs to fallback to read_attribute instead of send (whether in ActiveModel or ActiveRecord, I don't know).

It's possible this is a mongoid issue (although since it broke in the Rails 5.2.1 patch-level release, it's arguably still a Rails issue, at least until 6.0).

Also filed Mongoid bug https://jira.mongodb.org/browse/MONGOID-4604

@edzhelyov
Copy link

edzhelyov commented Aug 21, 2018

Upgrading to 5.2.1 broke my specs as well. I test ActiveAttr modles which internally relies on ActiveModel validations. I have custom specs as well as shoulda-matchers which rely on this behavior as well.
The matcher validate_numericality_of adds tests that specifically add non numeric or float values and expect to see not an integer error.
So this is not only a mongoid issue.

PS. The CHANGELOG for ActiveModel 5.2.1 version is empty, but that not correct. Can we update it?

kamipo added a commit to kamipo/rails that referenced this issue Aug 23, 2018
… Active Record

The purpose of fe9547b is to work type casting to value from database.

But that was caused not to use the value before type cast even except
Active Record.

There we never guarantees that the value before type cast was going to
the used in this validation, but we should not change the behavior
unless there is some particular reason.

To restore original behavior, still use the value before type cast if
`came_from_user?` is undefined (i.e. except Active Record).

Fixes rails#33651.
Fixes rails#33686.
@rst
Copy link

rst commented Aug 23, 2018

This also affects code which wraps ActiveRecord objects with form presenters (e.g., to support manipulation of related AR objects as a unit). It can be worked around there by having the presenter delegate the _came_from_user? stuff for the relevant attributes (and, oddly, read_attribute_before_type_cast), but it would be nice if those workarounds don't have to turn permanent...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants