From 14379aac45a86cbb8ca65cf037528a48d3c8abf9 Mon Sep 17 00:00:00 2001 From: Matheus Sales Date: Sat, 1 Oct 2022 10:09:35 -0300 Subject: [PATCH] Add `in: range` matcher to validate_numericality_of Closes: #1493 In Rails 7 was added a new option to validate numericality. You can use `in: range` to specify a range to validate an attribute. ```ruby class User < ApplicationRecord validates :age, numericality: { greater_than_or_equal_to: 18, less_than_or_equal_to: 65 } end class User < ApplicationRecord validates :age, numericality: { in: 18..65 } end ``` In this commit we are adding the support matcher to this new functionality, while also making a refactor on the numericality matchers that use the concept of submatchers. We've created a new class (`NumericalityMatchers::Submatcher`) that's been used by `NumericalityMatchers::RangeMatcher` and `NumericalityMatchers::ComparisonMatcher`, this new class wil handle shared logic regarding having submatchers that will check if the parent matcher is valid or not. Our new class `Numericality::Matchers::RangeMatcher` is using as submatchers two `NumericalityMatchers::ComparisonMatcher` instances to avoid creating new logic to handle this new option and also to replicate what was being used before this option existed in Rails (see example above) In this commit we are adding: * NumericalityMatchers::RangeMatcher file to support the new `in: range` option. * Specs on ValidateNumericalityOfMatcherSpec file for the new supported option, only running on rails_versions > 7. * NumericalityMatchers::Submatchers file to handle having submatchers inside a matcher file. * Refactors to NumericalityMatchers::ComparisonMatcher. --- lib/shoulda/matchers/active_model.rb | 2 + .../comparison_matcher.rb | 47 ++--- .../numericality_matchers/range_matcher.rb | 71 ++++++++ .../numericality_matchers/submatchers.rb | 43 +++++ .../validate_numericality_of_matcher.rb | 46 +++++ spec/support/unit/helpers/rails_versions.rb | 4 + .../validate_numericality_of_matcher_spec.rb | 164 +++++++++++++++++- 7 files changed, 336 insertions(+), 41 deletions(-) create mode 100644 lib/shoulda/matchers/active_model/numericality_matchers/range_matcher.rb create mode 100644 lib/shoulda/matchers/active_model/numericality_matchers/submatchers.rb diff --git a/lib/shoulda/matchers/active_model.rb b/lib/shoulda/matchers/active_model.rb index e4a0ad7bb..c9f20d49b 100644 --- a/lib/shoulda/matchers/active_model.rb +++ b/lib/shoulda/matchers/active_model.rb @@ -26,6 +26,8 @@ require 'shoulda/matchers/active_model/numericality_matchers/odd_number_matcher' require 'shoulda/matchers/active_model/numericality_matchers/even_number_matcher' require 'shoulda/matchers/active_model/numericality_matchers/only_integer_matcher' +require 'shoulda/matchers/active_model/numericality_matchers/range_matcher' +require 'shoulda/matchers/active_model/numericality_matchers/submatchers' require 'shoulda/matchers/active_model/errors' require 'shoulda/matchers/active_model/have_secure_password_matcher' diff --git a/lib/shoulda/matchers/active_model/numericality_matchers/comparison_matcher.rb b/lib/shoulda/matchers/active_model/numericality_matchers/comparison_matcher.rb index 8d41b92db..2bea2de52 100644 --- a/lib/shoulda/matchers/active_model/numericality_matchers/comparison_matcher.rb +++ b/lib/shoulda/matchers/active_model/numericality_matchers/comparison_matcher.rb @@ -1,3 +1,5 @@ +require 'active_support/core_ext/module/delegation' + module Shoulda module Matchers module ActiveModel @@ -31,6 +33,8 @@ class ComparisonMatcher < ValidationMatcher }, }.freeze + delegate :failure_message, :failure_message_when_negated, to: :submatchers + def initialize(numericality_matcher, value, operator) super(nil) unless numericality_matcher.respond_to? :diff_to_compare @@ -72,49 +76,24 @@ def expects_custom_validation_message? def matches?(subject) @subject = subject - all_bounds_correct? - end - - def failure_message - last_failing_submatcher.failure_message - end - - def failure_message_when_negated - last_failing_submatcher.failure_message_when_negated + submatchers.matches?(subject) end def comparison_description "#{comparison_expectation} #{@value}" end - private - - def all_bounds_correct? - failing_submatchers.empty? - end - - def failing_submatchers - submatchers_and_results. - select { |x| !x[:matched] }. - map { |x| x[:matcher] } - end - - def last_failing_submatcher - failing_submatchers.last - end - def submatchers - @_submatchers ||= - comparison_combos.map do |diff, submatcher_method_name| - matcher = __send__(submatcher_method_name, diff, nil) - matcher.with_message(@message, values: { count: @value }) - matcher - end + @_submatchers ||= NumericalityMatchers::Submatchers.new(build_submatchers) end - def submatchers_and_results - @_submatchers_and_results ||= submatchers.map do |matcher| - { matcher: matcher, matched: matcher.matches?(@subject) } + private + + def build_submatchers + comparison_combos.map do |diff, submatcher_method_name| + matcher = __send__(submatcher_method_name, diff, nil) + matcher.with_message(@message, values: { count: @value }) + matcher end end diff --git a/lib/shoulda/matchers/active_model/numericality_matchers/range_matcher.rb b/lib/shoulda/matchers/active_model/numericality_matchers/range_matcher.rb new file mode 100644 index 000000000..03d823cd8 --- /dev/null +++ b/lib/shoulda/matchers/active_model/numericality_matchers/range_matcher.rb @@ -0,0 +1,71 @@ +require 'active_support/core_ext/module/delegation' + +module Shoulda + module Matchers + module ActiveModel + module NumericalityMatchers + # @private + class RangeMatcher < ValidationMatcher + OPERATORS = [:>=, :<=].freeze + + delegate :failure_message, to: :submatchers + + def initialize(numericality_matcher, attribute, range) + super(attribute) + unless numericality_matcher.respond_to? :diff_to_compare + raise ArgumentError, 'numericality_matcher is invalid' + end + + @numericality_matcher = numericality_matcher + @range = range + @attribute = attribute + end + + def matches?(subject) + @subject = subject + submatchers.matches?(subject) + end + + def simple_description + description = '' + + if expects_strict? + description << ' strictly' + end + + description + + "disallow :#{attribute} from being a number that is not " + + range_description + end + + def range_description + "from #{Shoulda::Matchers::Util.inspect_range(@range)}" + end + + def submatchers + @_submatchers ||= NumericalityMatchers::Submatchers.new(build_submatchers) + end + + private + + def build_submatchers + submatcher_combos.map do |value, operator| + build_comparison_submatcher(value, operator) + end + end + + def submatcher_combos + @range.minmax.zip(OPERATORS) + end + + def build_comparison_submatcher(value, operator) + NumericalityMatchers::ComparisonMatcher.new(@numericality_matcher, value, operator). + for(@attribute). + with_message(@message). + on(@context) + end + end + end + end + end +end diff --git a/lib/shoulda/matchers/active_model/numericality_matchers/submatchers.rb b/lib/shoulda/matchers/active_model/numericality_matchers/submatchers.rb new file mode 100644 index 000000000..c7d0a9d63 --- /dev/null +++ b/lib/shoulda/matchers/active_model/numericality_matchers/submatchers.rb @@ -0,0 +1,43 @@ +module Shoulda + module Matchers + module ActiveModel + module NumericalityMatchers + # @private + class Submatchers + def initialize(submatchers) + @submatchers = submatchers + end + + def matches?(subject) + @subject = subject + failing_submatchers.empty? + end + + def failure_message + last_failing_submatcher.failure_message + end + + def failure_message_when_negated + last_failing_submatcher.failure_message_when_negated + end + + def add(submatcher) + @submatchers << submatcher + end + + def last_failing_submatcher + failing_submatchers.last + end + + private + + def failing_submatchers + @_failing_submatchers ||= @submatchers.reject do |submatcher| + submatcher.matches?(@subject) + end + end + end + end + end + end +end diff --git a/lib/shoulda/matchers/active_model/validate_numericality_of_matcher.rb b/lib/shoulda/matchers/active_model/validate_numericality_of_matcher.rb index 1b2c446fe..f10834e52 100644 --- a/lib/shoulda/matchers/active_model/validate_numericality_of_matcher.rb +++ b/lib/shoulda/matchers/active_model/validate_numericality_of_matcher.rb @@ -276,6 +276,33 @@ module ActiveModel # should validate_numericality_of(:birth_day).odd # end # + # ##### is_in + # + # Use `is_in` to test usage of the `:in` option. + # This asserts that the attribute can take a number which is contained + # in the given range. + # + # class Person + # include ActiveModel::Model + # attr_accessor :legal_age + # + # validates_numericality_of :birth_month, in: 1..12 + # end + # + # # RSpec + # RSpec.describe Person, type: :model do + # it do + # should validate_numericality_of(:birth_month). + # is_in(1..12) + # end + # end + # + # # Minitest (Shoulda) + # class PersonTest < ActiveSupport::TestCase + # should validate_numericality_of(:birth_month). + # is_in(1..12) + # end + # # ##### with_message # # Use `with_message` if you are using a custom validation message. @@ -426,6 +453,13 @@ def is_other_than(value) self end + def is_in(range) + prepare_submatcher( + NumericalityMatchers::RangeMatcher.new(self, @attribute, range), + ) + self + end + def with_message(message) @expects_custom_validation_message = true @expected_message = message @@ -457,6 +491,10 @@ def simple_description description << "validate that :#{@attribute} looks like " description << Shoulda::Matchers::Util.a_or_an(full_allowed_type) + if range_description.present? + description << " #{range_description}" + end + if comparison_descriptions.present? description << " #{comparison_descriptions}" end @@ -673,6 +711,14 @@ def submatcher_comparison_descriptions end end + def range_description + range_submatcher = @submatchers.detect do |submatcher| + submatcher.respond_to? :range_description + end + + range_submatcher&.range_description + end + def model @subject.class end diff --git a/spec/support/unit/helpers/rails_versions.rb b/spec/support/unit/helpers/rails_versions.rb index e7831c24c..44ac3f59b 100644 --- a/spec/support/unit/helpers/rails_versions.rb +++ b/spec/support/unit/helpers/rails_versions.rb @@ -10,5 +10,9 @@ def self.configure_example_group(example_group) def rails_version Tests::Version.new(Rails::VERSION::STRING) end + + def rails_oldest_version_supported + 5.2 + end end end diff --git a/spec/unit/shoulda/matchers/active_model/validate_numericality_of_matcher_spec.rb b/spec/unit/shoulda/matchers/active_model/validate_numericality_of_matcher_spec.rb index d1d5c6d44..92dfb2dbe 100644 --- a/spec/unit/shoulda/matchers/active_model/validate_numericality_of_matcher_spec.rb +++ b/spec/unit/shoulda/matchers/active_model/validate_numericality_of_matcher_spec.rb @@ -69,26 +69,34 @@ def all_qualifiers # rubocop:disable Metrics/MethodLength validation_name: :on, validation_value: :customizable, }, + { + category: :range, + name: :is_in, + argument: 1..10, + validation_name: :in, + validation_value: 1..10, + rails_version: 7.0, + }, ] end def qualifiers_under(category) - all_qualifiers.select do |qualifier| + all_available_qualifiers.select do |qualifier| qualifier[:category] == category end end def mutually_exclusive_qualifiers - qualifiers_under(:cardinality) + qualifiers_under(:comparison) + qualifiers_under(:cardinality) + qualifiers_under(:comparison) + qualifiers_under(:range) end def non_mutually_exclusive_qualifiers - all_qualifiers - mutually_exclusive_qualifiers + all_available_qualifiers - mutually_exclusive_qualifiers end - def validations_by_qualifier - all_qualifiers.each_with_object({}) do |qualifier, hash| - hash[qualifier[:name]] = qualifier[:validation_name] + def all_available_qualifiers + all_qualifiers.filter do |qualifier| + rails_version >= qualifier.fetch(:rails_version, rails_oldest_version_supported) end end @@ -2065,6 +2073,144 @@ def validation_matcher_scenario_args end end + if rails_version >= 7.0 + context 'qualified with in' do + context 'validating with in' do + it 'accepts' do + record = build_record_validating_numericality( + in: 1..10, + ) + expect(record).to validate_numericality.is_in(1..10) + end + + it 'rejects when used in the negative' do + record = build_record_validating_numericality( + in: 1..10, + ) + + assertion = lambda do + expect(record).not_to validate_numericality.is_in(1..10) + end + + expect(&assertion).to fail_with_message(<<~MESSAGE) + Expected Example not to validate that :attr looks like a number from ‹1› + to ‹10›, but this could not be proved. + After setting :attr to ‹"abcd"›, the matcher expected the Example to + be valid, but it was invalid instead, producing these validation + errors: + + * attr: ["is not a number"] + MESSAGE + end + + it_supports( + 'ignoring_interference_by_writer', + tests: { + reject_if_qualified_but_changing_value_interferes: { + model_name: 'Example', + attribute_name: :attr, + changing_values_with: :next_value, + expected_message: <<-MESSAGE.strip, +Expected Example to validate that :attr looks like a number from ‹1› to +‹10›, but this could not be proved. + After setting :attr to ‹"10"› -- which was read back as ‹"11"› -- the + matcher expected the Example to be valid, but it was invalid instead, + producing these validation errors: + + * attr: ["must be in 1..10"] + + As indicated in the message above, :attr seems to be changing certain + values as they are set, and this could have something to do with why + this test is failing. If you've overridden the writer method for this + attribute, then you may need to change it to make this test pass, or + do something else entirely. + MESSAGE + }, + }, + ) do + def validation_matcher_scenario_args + super.deep_merge( + validation_options: { in: 1..10 }, + ) + end + + def configure_validation_matcher(matcher) + matcher.is_in(1..10) + end + end + + context 'when the attribute is a virtual attribute in an ActiveRecord model' do + it 'accepts' do + record = build_record_validating_numericality_of_virtual_attribute( + in: 1..10, + ) + expect(record).to validate_numericality. + is_in(1..10) + end + end + + context 'when the column is an integer column' do + it 'accepts (and does not raise an error)' do + record = build_record_validating_numericality( + column_type: :integer, + in: 1..10, + ) + + expect(record). + to validate_numericality. + is_in(1..10) + end + end + + context 'when the column is a float column' do + it 'accepts (and does not raise an error)' do + record = build_record_validating_numericality( + column_type: :float, + in: 1..10, + ) + + expect(record). + to validate_numericality. + is_in(1..10) + end + end + + context 'when the column is a decimal column' do + it 'accepts (and does not raise an error)' do + record = build_record_validating_numericality( + column_type: :decimal, + in: 1..10, + ) + + expect(record). + to validate_numericality. + is_in(1..10) + end + end + end + + context 'not validating with in' do + it 'rejects since it does not disallow numbers that are not in the range specified' do + record = build_record_validating_numericality + + assertion = lambda do + expect(record).to validate_numericality. + is_in(1..10) + end + + message = <<-MESSAGE +Expected Example to validate that :attr looks like a number from ‹1› to +‹10›, but this could not be proved. + After setting :attr to ‹"11"›, the matcher expected the Example to be + invalid, but it was valid instead. + MESSAGE + + expect(&assertion).to fail_with_message(message) + end + end + end + end + def build_validation_options(args) combination = args.fetch(:for) @@ -2080,7 +2226,11 @@ def apply_qualifiers!(args) combination.each do |qualifier| args = self.class.default_qualifier_arguments.fetch(qualifier[:name]) - matcher.__send__(qualifier[:name], *args) + if args + matcher.__send__(qualifier[:name], args) + else + matcher.__send__(qualifier[:name]) + end end end