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

Fix numericality validator to still use value before type cast except Active Record #33654

Merged
merged 1 commit into from Aug 28, 2018

Conversation

@kamipo
Copy link
Member

@kamipo kamipo commented Aug 19, 2018

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 #33651.

@edzhelyov
Copy link

@edzhelyov edzhelyov commented Aug 21, 2018

While this might close #33651 it won't affect the regression that I have when using ActiveAttr.

This is basically what I have:

class User
  include ActiveAttr::Model

  attribute :account_crn, type: Integer
end

describe User do
  it { should validate_numericality_of(:account_crn) }

  it 'validates account CRN numericality' do
    user.account_crn = 1.1

    user.validate

    user.errors.should be_added :account_crn, :not_an_integer
  end
end

The shoulda-matchers test is broken and my custom one. I will work out to provide a runnable example tomorrow.

@edzhelyov
Copy link

@edzhelyov edzhelyov commented Aug 22, 2018

I've filled an issue #33686 with the problems that I have after fe9547b. Which this PR won't fix.

@cgriego
Copy link
Contributor

@cgriego cgriego commented Aug 23, 2018

For maximum backwards compatibility, if #{attr_name}_came_from_user? isn't defined then ActiveModel should use #{attr_name}_before_type_cast like it did before instead of read_attribute_before_type_cast.

The read_attribute_before_type_cast method is an ActiveRecord interface, but the ActiveModel convention (using ActiveModel::AttributeMethods) is that #{attr_name}_before_type_cast is the public interface backed by an attribute_before_type_cast method. ActiveRecord makes attribute_before_type_cast private and exposes its own read_attribute_before_type_cast publicly in addition to #{attr_name}_before_type_cast.

… 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.
@kamipo kamipo force-pushed the fix_numericality_validator_2 branch from e330577 to 47a6d78 Aug 23, 2018
@kamipo
Copy link
Member Author

@kamipo kamipo commented Aug 23, 2018

You are right. I've restored to use #{attr_name}_before_type_cast in this method.

@kamipo kamipo merged commit 37075aa into rails:master Aug 28, 2018
2 checks passed
@kamipo kamipo deleted the fix_numericality_validator_2 branch Aug 28, 2018
kamipo added a commit that referenced this issue Aug 28, 2018
Fix numericality validator to still use value before type cast except Active Record
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants