forked from fixin.me/fixin.me
Check for out of range float values
Remove unsupported attribute `[maxlength]` from `input[number]`.
This commit is contained in:
17
DESIGN.md
17
DESIGN.md
@@ -57,3 +57,20 @@ whenever a change is considered, to avoid regressions.
|
||||
* proper application level constraints should prevent unhandled database
|
||||
exception occurences, e.g `ActiveRecord::InvalidForeignKey` for operations
|
||||
performed through Models (i.e. not `#delete_all` etc.)
|
||||
|
||||
### User interface
|
||||
* values provided by user have to be exactly preserved, avoiding any precision
|
||||
loss due to e.g. type casting
|
||||
* when it's not possible to store value without information loss, feedback has to
|
||||
be given at the time of entering value
|
||||
* values can be received using either:
|
||||
* `input[number]` - validates: number format, value against `[min]` and
|
||||
`[max]` attributes; cannot validate precision (no `[pattern]` support);
|
||||
or
|
||||
* `input[text]` - can validate number format and precision with
|
||||
`[pattern]`; cannot validate range (no `[min]`/`[max]` support),
|
||||
* none of the above prevents misinterpretation of values between 0 and
|
||||
Float::MIN as 0.0 - this has to be checked manually,
|
||||
* assuming that user input with _precision > DIG_ is as rare as in
|
||||
the range _(0, MIN)_, the decision is to choose input which requires
|
||||
the least/simplest additional code.
|
||||
|
||||
@@ -101,22 +101,14 @@ module ApplicationHelper
|
||||
end
|
||||
|
||||
def number_field(method, options = {})
|
||||
attr_type = object.type_for_attribute(method)
|
||||
case attr_type.type
|
||||
case object.type_for_attribute(method).type
|
||||
when :float, :double
|
||||
options[:value] = object.public_send(method)&.to_scientific
|
||||
options[:step] ||= :any
|
||||
options[:min] ||= Float::MIN_15
|
||||
options[:max] ||= Float::MAX_15
|
||||
# Longest possible number (written not using exponent):
|
||||
# sign (1), leading 0 (1), dot (1), exponent 0s (307), digits (15).
|
||||
# This is only upper bound, which cannot guarantee the number won't fall
|
||||
# out of range.
|
||||
# TODO: add `[pattern]` to limit precision and (possibly) replace `[maxlength]`?
|
||||
# NOTE: `[pattern]` is unavailable on `input[type=number]` and `[min]/[max]` is
|
||||
# unavailable on `input[type=text]`.
|
||||
options[:maxlength] ||= 3 + Float::MIN_10_EXP + Float::DIG
|
||||
options[:min] ||= Float::MIN_15.to_scientific
|
||||
options[:max] ||= Float::MAX_15.to_scientific
|
||||
options[:oninput] = 'numberValidate(event)'
|
||||
options[:size] ||= 6
|
||||
options[:step] ||= :any
|
||||
options[:value] = object.public_send("#{method}_before_type_cast")
|
||||
end
|
||||
super
|
||||
end
|
||||
|
||||
@@ -53,6 +53,27 @@ function formValidate(event) {
|
||||
}
|
||||
window.formValidate = formValidate
|
||||
|
||||
// Pattern for number precision verification. Should accept all number formats
|
||||
// accepted by Kernel#Float. Lookahead checks the number of significant digits
|
||||
// in mantissa, following expression makes sure there is only one dot.
|
||||
const numberRegExp = /^\-?(?=(?:\.?0)*(?:\.?\d){1,15}?(?:\.?0)*\.?(?:[eE]|$))(?<mantissa>\d*\.?\d+|\d+\.)(?:[eE]\-?\d{1,3})?$/
|
||||
|
||||
// Validation of input[type=number] value precision.
|
||||
function numberValidate(event) {
|
||||
var input = event.target
|
||||
|
||||
input.setCustomValidity("")
|
||||
if (!input.validity.valid) return
|
||||
|
||||
var match = numberRegExp.exec(input.value)
|
||||
if (!match ||
|
||||
(parseFloat(input.value) == 0.0 && match.groups['mantissa'].match(/[1-9]/))) {
|
||||
// TODO: move message to locales
|
||||
input.setCustomValidity("Please select a value within range of double precision floating-point format.")
|
||||
}
|
||||
}
|
||||
window.numberValidate = numberValidate
|
||||
|
||||
|
||||
// Turbo stream actions.
|
||||
const FORM_CONTROLS = ["BUTTON", "FIELDSET", "INPUT", "OPTGROUP", "OPTION", "SELECT",
|
||||
|
||||
@@ -15,7 +15,7 @@ end
|
||||
|
||||
ActiveSupport.on_load :active_record do
|
||||
ActiveModel::Validations::NumericalityValidator
|
||||
.prepend CoreExt::ActiveModel::Validations::NumericalityValidatesPrecisionAndScale
|
||||
.prepend CoreExt::ActiveModel::Validations::NumericalityValidatesPrecision
|
||||
|
||||
# Temporary patch for https://github.com/rails/rails/pull/54658
|
||||
Arel::TreeManager::StatementMethods
|
||||
|
||||
@@ -5,7 +5,7 @@ en:
|
||||
without_tz: "%Y-%m-%d %H:%M"
|
||||
errors:
|
||||
messages:
|
||||
precision_exceeded: must not exceed %{value} significant digits
|
||||
out_of_range: exceeds range of double precision floating-point format
|
||||
activerecord:
|
||||
attributes:
|
||||
quantity:
|
||||
|
||||
@@ -0,0 +1,13 @@
|
||||
module CoreExt::ActiveModel::Validations::NumericalityValidatesPrecision
|
||||
def validate_each(record, attr_name, value, **kwargs)
|
||||
super(record, attr_name, value, **kwargs)
|
||||
|
||||
# Check if value can be represented using Float data type.
|
||||
# BigDecimal is used as a reference due to its arbitrary precision.
|
||||
if record.class.type_for_attribute(attr_name).type == :float && !value.is_a?(Float)
|
||||
unless value.to_d == value.to_f.to_d(kwargs[:precision])
|
||||
record.errors.add(attr_name, :out_of_range, **filtered_options(value))
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
@@ -1,19 +0,0 @@
|
||||
module CoreExt::ActiveModel::Validations::NumericalityValidatesPrecisionAndScale
|
||||
def validate_each(record, attr_name, value, ...)
|
||||
super(record, attr_name, value, ...)
|
||||
|
||||
if options[:precision] || options[:scale]
|
||||
attr_type = record.class.type_for_attribute(attr_name)
|
||||
# For conversion of 'value' to BigDecimal 'ndigits' is not supplied intentionally,
|
||||
# to avoid silent rounding. It is only required for conversion from Float and
|
||||
# Rational, which should not happen.
|
||||
value = BigDecimal(value) unless value.is_a? BigDecimal
|
||||
if options[:precision] && (value.precision > attr_type.precision)
|
||||
record.errors.add(attr_name, :precision_exceeded, **filtered_options(attr_type.precision))
|
||||
end
|
||||
if options[:scale] && (value.scale > attr_type.scale)
|
||||
record.errors.add(attr_name, :scale_exceeded, **filtered_options(attr_type.scale))
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
@@ -50,5 +50,6 @@ class Float
|
||||
# TODO: change MIN_15 to MIN.ceil(MIN_10_EXP - DIG) after #ceil fix in Ruby
|
||||
# v4.0.5: https://bugs.ruby-lang.org/issues/22079
|
||||
MIN_15 = MIN.ceil(-(MIN_10_EXP - 1))
|
||||
MAX_15 = MAX.floor(-(MAX_10_EXP - DIG + 1))
|
||||
# MAX is an integer.
|
||||
MAX_15 = MAX.floor(-(MAX_10_EXP - DIG + 1)).to_f
|
||||
end
|
||||
|
||||
@@ -93,6 +93,10 @@ class ApplicationSystemTestCase < ActionDispatch::SystemTestCase
|
||||
XPath.descendant(:td).where(header_xp.boolean & position_xp)
|
||||
end
|
||||
|
||||
expression_filter(:fillable) do |xpath, value|
|
||||
value ? xpath.where(XPath.descendant(:input, :select, :textarea)) : xpath
|
||||
end
|
||||
|
||||
node_filter(:with) do |node, expected|
|
||||
value =
|
||||
case expected
|
||||
|
||||
@@ -101,8 +101,29 @@ class UnitsTest < ApplicationSystemTestCase
|
||||
end
|
||||
|
||||
test "create fails with out of range multiplier" do
|
||||
# TODO: multiplier with exponent > max, < min, precision > DIG, value <= 0
|
||||
assert true
|
||||
sign_in(user: users.select { |u| u.confirmed? && !u.units.empty? }.sample)
|
||||
all(:link, exact_text: link_labels[:new_subunit]).sample.click
|
||||
|
||||
multipliers = [
|
||||
"a", # * not a number (NaN)
|
||||
"1e#{Float::MAX_10_EXP + 1}", # * value too big (Infinity)
|
||||
# * value too big (> max), N/A
|
||||
"-1e#{Float::MAX_10_EXP + 1}", # * value too small (-Infinity)
|
||||
"-1", # * value too small (< min)
|
||||
"0", # * -"-
|
||||
"1." + "0" * (Float::DIG - 1) + "1", # * precision too big
|
||||
"1" + "0" * (Float::DIG - 1) + "1", # * -"-
|
||||
"0.1" + "0" * (Float::DIG - 1) + "1", # * -"-
|
||||
"1e#{Float::MIN_10_EXP - Float::DIG - 1}", # * precision too big (MIN > value > 0)
|
||||
]
|
||||
|
||||
field = find(:table_cell, column(:multiplier), fillable: true).find_field
|
||||
assert_not_matches_selector field, ':invalid'
|
||||
|
||||
multipliers.shuffle.each do |multiplier|
|
||||
field.fill_in with: multiplier
|
||||
assert_matches_selector field, ':invalid'
|
||||
end
|
||||
end
|
||||
|
||||
test "create updates view in order" do
|
||||
|
||||
Reference in New Issue
Block a user