From 834d8d0356573b9f47e63a1b910cfa8f3d815e51 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Thu, 23 May 2019 22:09:13 -0600 Subject: [PATCH] Add allow_nil to presence matcher Co-authored-by: cjmao --- NEWS.md | 2 + .../matchers/active_model/qualifiers.rb | 1 + .../active_model/qualifiers/allow_nil.rb | 26 ++++++ .../validate_presence_of_matcher.rb | 71 +++++++++++---- .../validate_presence_of_matcher_spec.rb | 88 ++++++++++++++++++- 5 files changed, 170 insertions(+), 18 deletions(-) create mode 100644 lib/shoulda/matchers/active_model/qualifiers/allow_nil.rb diff --git a/NEWS.md b/NEWS.md index 2ad60bfe3..c7f52d699 100644 --- a/NEWS.md +++ b/NEWS.md @@ -16,9 +16,11 @@ existing features that broke with the upgrade. ([#1193]) * Add support for expression indexes (Rails 5, Postgres only) to `have_db_index`. ([#1211]) +* Add `allow_nil` to the `validate_presence_of` matcher. ([#1100]) [#1193]: https://github.com/thoughtbot/shoulda-matchers/pull/1193 [#1211]: https://github.com/thoughtbot/shoulda-matchers/pull/1211 +[#1100]: https://github.com/thoughtbot/shoulda-matchers/pull/1100 # 4.0.1 diff --git a/lib/shoulda/matchers/active_model/qualifiers.rb b/lib/shoulda/matchers/active_model/qualifiers.rb index 5a2772044..7af3d448a 100644 --- a/lib/shoulda/matchers/active_model/qualifiers.rb +++ b/lib/shoulda/matchers/active_model/qualifiers.rb @@ -8,5 +8,6 @@ module Qualifiers end end +require_relative 'qualifiers/allow_nil' require_relative 'qualifiers/ignore_interference_by_writer' require_relative 'qualifiers/ignoring_interference_by_writer' diff --git a/lib/shoulda/matchers/active_model/qualifiers/allow_nil.rb b/lib/shoulda/matchers/active_model/qualifiers/allow_nil.rb new file mode 100644 index 000000000..0e0bc30a0 --- /dev/null +++ b/lib/shoulda/matchers/active_model/qualifiers/allow_nil.rb @@ -0,0 +1,26 @@ +module Shoulda + module Matchers + module ActiveModel + module Qualifiers + # @private + module AllowNil + def initialize(*args) + super + @expects_to_allow_nil = false + end + + def allow_nil + @expects_to_allow_nil = true + self + end + + protected + + def expects_to_allow_nil? + @expects_to_allow_nil + end + end + end + end + end +end diff --git a/lib/shoulda/matchers/active_model/validate_presence_of_matcher.rb b/lib/shoulda/matchers/active_model/validate_presence_of_matcher.rb index ee5bed882..f87337b01 100644 --- a/lib/shoulda/matchers/active_model/validate_presence_of_matcher.rb +++ b/lib/shoulda/matchers/active_model/validate_presence_of_matcher.rb @@ -57,6 +57,27 @@ module ActiveModel # # #### Qualifiers # + # ##### allow_nil + # + # Use `allow_nil` if your model has an optional attribute. + # + # class Robot + # include ActiveModel::Model + # attr_accessor :nickname + # + # validates_presence_of :nickname, allow_nil: true + # end + # + # # RSpec + # RSpec.describe Robot, type: :model do + # it { should validate_presence_of(:nickname).allow_nil } + # end + # + # # Minitest (Shoulda) + # class RobotTest < ActiveSupport::TestCase + # should validate_presence_of(:nickname).allow_nil + # end + # # ##### on # # Use `on` if your validation applies only under a certain context. @@ -111,6 +132,8 @@ def validate_presence_of(attr) # @private class ValidatePresenceOfMatcher < ValidationMatcher + include Qualifiers::AllowNil + def initialize(attribute) super @expected_message = :blank @@ -122,9 +145,16 @@ def matches?(subject) possibly_ignore_interference_by_writer if secure_password_being_validated? - disallows_and_double_checks_value_of!(blank_value, @expected_message) + ignore_interference_by_writer.default_to(when: :blank?) + + disallowed_values.all? do |value| + disallows_and_double_checks_value_of!(value) + end else - disallows_original_or_typecast_value?(blank_value, @expected_message) + (!expects_to_allow_nil? || allows_value_of(nil)) && + disallowed_values.all? do |value| + disallows_original_or_typecast_value?(value) + end end end @@ -134,9 +164,16 @@ def does_not_match?(subject) possibly_ignore_interference_by_writer if secure_password_being_validated? - allows_and_double_checks_value_of!(blank_value, @expected_message) + ignore_interference_by_writer.default_to(when: :blank?) + + disallowed_values.any? do |value| + allows_and_double_checks_value_of!(value) + end else - allows_original_or_typecast_value?(blank_value, @expected_message) + (expects_to_allow_nil? && !allows_value_of(nil)) || + disallowed_values.any? do |value| + allows_original_or_typecast_value?(value) + end end end @@ -157,31 +194,35 @@ def possibly_ignore_interference_by_writer end end - def allows_and_double_checks_value_of!(value, message) - allows_value_of(value, message) + def allows_and_double_checks_value_of!(value) + allows_value_of(value, @expected_message) rescue ActiveModel::AllowValueMatcher::AttributeChangedValueError raise ActiveModel::CouldNotSetPasswordError.create(@subject.class) end - def allows_original_or_typecast_value?(value, message) - allows_value_of(blank_value, @expected_message) + def allows_original_or_typecast_value?(value) + allows_value_of(value, @expected_message) end - def disallows_and_double_checks_value_of!(value, message) - disallows_value_of(value, message) + def disallows_and_double_checks_value_of!(value) + disallows_value_of(value, @expected_message) rescue ActiveModel::AllowValueMatcher::AttributeChangedValueError raise ActiveModel::CouldNotSetPasswordError.create(@subject.class) end - def disallows_original_or_typecast_value?(value, message) - disallows_value_of(blank_value, @expected_message) + def disallows_original_or_typecast_value?(value) + disallows_value_of(value, @expected_message) end - def blank_value + def disallowed_values if collection? - [] + [Array.new] else - nil + [''].tap do |disallowed| + if !expects_to_allow_nil? + disallowed << nil + end + end end end diff --git a/spec/unit/shoulda/matchers/active_model/validate_presence_of_matcher_spec.rb b/spec/unit/shoulda/matchers/active_model/validate_presence_of_matcher_spec.rb index 38afda32b..4302b75ff 100644 --- a/spec/unit/shoulda/matchers/active_model/validate_presence_of_matcher_spec.rb +++ b/spec/unit/shoulda/matchers/active_model/validate_presence_of_matcher_spec.rb @@ -66,7 +66,7 @@ message = <<-MESSAGE Expected Example to validate that :attr cannot be empty/falsy, but this could not be proved. - After setting :attr to ‹nil›, the matcher expected the Example to be + After setting :attr to ‹""›, the matcher expected the Example to be invalid, but it was valid instead. MESSAGE @@ -124,7 +124,7 @@ def model_creator message = <<-MESSAGE Expected Example to validate that :attr cannot be empty/falsy, but this could not be proved. - After setting :attr to ‹nil›, the matcher expected the Example to be + After setting :attr to ‹""›, the matcher expected the Example to be invalid, but it was valid instead. MESSAGE @@ -355,7 +355,7 @@ def model_creator validates_presence_of :foo def foo=(value) - super(Array.wrap(value)) + super([]) end end @@ -363,6 +363,84 @@ def foo=(value) end end + context 'qualified with allow_nil' do + context 'when validating a model with a presence validator' do + context 'and it is specified with allow_nil: true' do + it 'matches in the positive' do + record = validating_presence(allow_nil: true) + expect(record).to matcher.allow_nil + end + + it 'does not match in the negative' do + record = validating_presence(allow_nil: true) + + assertion = -> { expect(record).not_to matcher.allow_nil } + + expect(&assertion).to fail_with_message(<<-MESSAGE) +Expected Example not to validate that :attr cannot be empty/falsy, but +this could not be proved. + After setting :attr to ‹""›, the matcher expected the Example to be + valid, but it was invalid instead, producing these validation errors: + + * attr: ["can't be blank"] + MESSAGE + end + end + + context 'and it is not specified with allow_nil: true' do + it 'does not match in the positive' do + record = validating_presence + + assertion = lambda do + expect(record).to matcher.allow_nil + end + + message = <<-MESSAGE +Expected Example to validate that :attr cannot be empty/falsy, but this +could not be proved. + After setting :attr to ‹nil›, the matcher expected the Example to be + valid, but it was invalid instead, producing these validation errors: + + * attr: ["can't be blank"] + MESSAGE + + expect(&assertion).to fail_with_message(message) + end + end + + it 'matches in the negative' do + record = validating_presence + + expect(record).not_to matcher.allow_nil + end + end + + context 'when validating a model without a presence validator' do + it 'does not match in the positive' do + record = without_validating_presence + + assertion = lambda do + expect(record).to matcher.allow_nil + end + + message = <<-MESSAGE +Expected Example to validate that :attr cannot be empty/falsy, but this +could not be proved. + After setting :attr to ‹""›, the matcher expected the Example to be + invalid, but it was valid instead. + MESSAGE + + expect(&assertion).to fail_with_message(message) + end + + it 'matches in the negative' do + record = without_validating_presence + + expect(record).not_to matcher.allow_nil + end + end + end + def matcher validate_presence_of(:attr) end @@ -373,6 +451,10 @@ def validating_presence(options = {}) end.new end + def without_validating_presence + define_model(:example, attr: :string).new + end + def active_model(&block) define_active_model_class('Example', accessors: [:attr], &block).new end