diff --git a/DESIGN.md b/DESIGN.md index f652328..98e554d 100644 --- a/DESIGN.md +++ b/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. diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 64f28a0..a903eee 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -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 diff --git a/app/javascript/application.js b/app/javascript/application.js index e799f32..a679411 100644 --- a/app/javascript/application.js +++ b/app/javascript/application.js @@ -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]|$))(?\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", diff --git a/config/initializers/core_ext.rb b/config/initializers/core_ext.rb index 149e454..ca7c887 100644 --- a/config/initializers/core_ext.rb +++ b/config/initializers/core_ext.rb @@ -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 diff --git a/config/locales/en.yml b/config/locales/en.yml index 2ba7680..8ae69ed 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -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: diff --git a/lib/core_ext/active_model/validations/numericality_validates_precision.rb b/lib/core_ext/active_model/validations/numericality_validates_precision.rb new file mode 100644 index 0000000..ca8d710 --- /dev/null +++ b/lib/core_ext/active_model/validations/numericality_validates_precision.rb @@ -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 diff --git a/lib/core_ext/active_model/validations/numericality_validates_precision_and_scale.rb b/lib/core_ext/active_model/validations/numericality_validates_precision_and_scale.rb deleted file mode 100644 index 42be128..0000000 --- a/lib/core_ext/active_model/validations/numericality_validates_precision_and_scale.rb +++ /dev/null @@ -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 diff --git a/lib/core_ext/float.rb b/lib/core_ext/float.rb index 47d3a48..b9491ed 100644 --- a/lib/core_ext/float.rb +++ b/lib/core_ext/float.rb @@ -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 diff --git a/test/application_system_test_case.rb b/test/application_system_test_case.rb index 05f217f..972340d 100644 --- a/test/application_system_test_case.rb +++ b/test/application_system_test_case.rb @@ -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 diff --git a/test/system/units_test.rb b/test/system/units_test.rb index 7015f90..54c92dd 100644 --- a/test/system/units_test.rb +++ b/test/system/units_test.rb @@ -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