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

Use round(scale) in ActiveModel NumericalityValidator #41019

Merged
merged 1 commit into from Jan 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 7 additions & 1 deletion activemodel/lib/active_model/validations/numericality.rb
Expand Up @@ -71,6 +71,8 @@ def parse_as_number(raw_value, precision, scale, option = nil)
raw_value if raw_value.is_a?(Range)
elsif raw_value.is_a?(Float)
parse_float(raw_value, precision, scale)
elsif raw_value.is_a?(BigDecimal)
Copy link
Contributor Author

@intrip intrip Jan 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when I was adding the missing tests I've realized that we didn't round BigDecimal when scale was given, therefore I've added this.

round(raw_value, scale)
elsif raw_value.is_a?(Numeric)
raw_value
elsif is_integer?(raw_value)
Expand All @@ -81,7 +83,11 @@ def parse_as_number(raw_value, precision, scale, option = nil)
end

def parse_float(raw_value, precision, scale)
(scale ? raw_value.truncate(scale) : raw_value).to_d(precision)
round(raw_value, scale).to_d(precision)
end

def round(raw_value, scale)
scale ? raw_value.round(scale) : raw_value
end

def is_number?(raw_value, precision, scale)
Expand Down
Expand Up @@ -103,6 +103,21 @@ def test_virtual_attribute_with_scale
assert_not_predicate subject, :valid?
end

def test_virtual_attribute_with_precision_and_scale
model_class.attribute(:virtual_decimal_number, :decimal, precision: 4, scale: 2)
model_class.validates_numericality_of(
:virtual_decimal_number, less_than_or_equal_to: 99.99
)

subject = model_class.new(virtual_decimal_number: 99.994)
assert_equal 99.99.to_d(4), subject.virtual_decimal_number
assert_predicate subject, :valid?

subject = model_class.new(virtual_decimal_number: 99.999)
assert_equal 100.00.to_d(4), subject.virtual_decimal_number
assert_not_predicate subject, :valid?
end

def test_aliased_attribute
model_class.validates_numericality_of(:new_bank_balance, greater_or_equal_than: 0)

Expand Down
14 changes: 11 additions & 3 deletions activerecord/test/cases/validations_test.rb
Expand Up @@ -187,9 +187,17 @@ def self.model_name
validates_numericality_of :wibble, greater_than_or_equal_to: BigDecimal("97.18")
end

assert_not_predicate klass.new(wibble: "97.179"), :valid?
assert_not_predicate klass.new(wibble: 97.179), :valid?
assert_not_predicate klass.new(wibble: BigDecimal("97.179")), :valid?
Comment on lines -190 to -192
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These assert_not_predicate were originally odd/incorrect since the typecasted value 97.18 is valid for greater_than_or_equal_to: BigDecimal("97.18").

Can you expand these to test both rounding up and down like #41019 (comment)?

["97.179", 97.179, BigDecimal("97.179")].each do |raw_value|
subject = klass.new(wibble: raw_value)
assert_equal 97.18.to_d(4), subject.wibble
assert_predicate subject, :valid?
end

["97.174", 97.174, BigDecimal("97.174")].each do |raw_value|
subject = klass.new(wibble: raw_value)
assert_equal 97.17.to_d(4), subject.wibble
assert_not_predicate subject, :valid?
end
end

def test_numericality_validator_wont_be_affected_by_custom_getter
Expand Down