Fix conditional validator evaluation for Class subjects and lambda arity

CHANGELOG.md

@@ -1,8 +1,11 @@
 ### 4.3.0 (Next)
 
-* [#247](https://github.com/mongoid/mongoid-rspec/pull/247): Migrate to danger-pr-comment workflow - [@dblock](https://github.com/dblock).
+* [#249](https://github.com/mongoid/mongoid-rspec/pull/249): Gate conditional `:if`/`:unless` validator support behind a `.check_conditions` method - [@johnnyshields](https://github.com/johnnyshields).
+* [#249](https://github.com/mongoid/mongoid-rspec/pull/249): Raise informative error if conditional `:if`/`:unless` validator is used with a class subject - [@johnnyshields](https://github.com/johnnyshields).
+* [#249](https://github.com/mongoid/mongoid-rspec/pull/249): Support Arrays and Procs with non-zero arity for `:if`/`:unless` validator - [@johnnyshields](https://github.com/johnnyshields).
 * [#248](https://github.com/mongoid/mongoid-rspec/pull/248): Add frozen_string_literal: true to all files and enforce rubocop - [@johnnyshields](https://github.com/johnnyshields).
 * [#248](https://github.com/mongoid/mongoid-rspec/pull/248): Various small CI fixes - [@johnnyshields](https://github.com/johnnyshields).
+* [#247](https://github.com/mongoid/mongoid-rspec/pull/247): Migrate to danger-pr-comment workflow - [@dblock](https://github.com/dblock).
 * Your contribution here.
 
 ### 4.2.0 (2024/06/04)

lib/matchers/validations.rb

@@ -59,28 +59,37 @@ def with_message(message)
           self
         end
 
+        def check_conditions
+          @check_conditions = true
+          self
+        end
+
         private
 
         def if_condition_matches?(actual, validator)
+          return true unless @check_conditions
           return true unless validator.options[:if]
 
-          check_condition actual, validator.options[:if]
+          check_condition(actual, validator.options[:if])
         end
 
         def unless_condition_matches?(actual, validator)
+          return true unless @check_conditions
           return true unless validator.options[:unless]
 
-          !check_condition actual, validator.options[:unless]
+          !check_condition(actual, validator.options[:unless])
         end
 
         def check_condition(actual, filter)
-          raise ArgumentError, 'Spec subject must be object instance when testing validators with if/unless condition.' if actual.is_a?(Class)
+          raise ArgumentError, 'Spec subject must be an instance when using .check_conditions' if actual.is_a?(Class)
 
           case filter
           when Symbol
-            actual.send filter
+            actual.send(filter)
           when ::Proc
-            actual.instance_exec(&filter)
+            filter.arity.zero? ? actual.instance_exec(&filter) : filter.call(actual)
+          when Array
+            filter.all? { |f| check_condition(actual, f) }
           else
             raise ArgumentError, "Unexpected filter: #{filter.inspect}"
           end

spec/models/article.rb

@@ -14,6 +14,8 @@ class Article
   field :status, type: Symbol
   field :deletion_date, type: DateTime, default: nil
   field :reviewer, type: String, default: nil
+  field :editor, type: String, default: nil
+  field :summary, type: String, default: nil
 
   embeds_many :comments, cascade_callbacks: true, inverse_of: :article
   embeds_one :permalink, inverse_of: :linkable, class_name: 'Permalink'
@@ -37,6 +39,14 @@ class Article
 
   validates_absence_of :comments, unless: :allow_comments if Mongoid::Compatibility::Version.mongoid4_or_newer?
 
+  validates_presence_of :editor, if: ->(article) { article.status == :approved }
+
+  validates_presence_of :summary, if: [:published?, ->(article) { article.status == :approved }]
+
+  def published?
+    published == true
+  end
+
   index({title: 1 }, { unique: true, background: true})
   index({ published: 1 })
 

spec/unit/validations_spec.rb

@@ -64,59 +64,117 @@
 
 if Mongoid::Compatibility::Version.mongoid4_or_newer?
   RSpec.describe 'Conditional validations' do
-    describe 'validations with if condition using symbol' do
-      context 'when the condition is met' do
-        subject { User.new(role: 'admin') }
-
-        it { is_expected.to validate_length_of(:password).greater_than(20) }
+    describe 'without .check_conditions (default)' do
+      it 'finds validators regardless of conditions when subject is a Class' do
+        expect(User).to validate_length_of(:password).greater_than(20)
       end
 
-      context 'when the condition is not met' do
-        subject { User.new(role: 'member') }
+      it 'finds validators with unless conditions when subject is a Class' do
+        expect(Article).to validate_presence_of(:reviewer)
+      end
 
-        it { is_expected.not_to validate_length_of(:password) }
+      it 'finds validators regardless of conditions when subject is an instance' do
+        expect(User.new(role: 'member')).to validate_length_of(:password).greater_than(20)
       end
     end
 
-    describe 'validations with if condition using lambda' do
-      context 'when the condition is met' do
-        subject { User.new(role: 'moderator') }
+    describe 'with .check_conditions' do
+      describe 'if condition using symbol' do
+        context 'when the condition is met' do
+          subject { User.new(role: 'admin') }
+
+          it { is_expected.to validate_length_of(:password).greater_than(20).check_conditions }
+        end
 
-        it { is_expected.to validate_length_of(:password).greater_than(10) }
+        context 'when the condition is not met' do
+          subject { User.new(role: 'member') }
+
+          it { is_expected.not_to validate_length_of(:password).check_conditions }
+        end
       end
 
-      context 'when the condition is not met' do
-        subject { User.new(role: 'member') }
+      describe 'if condition using lambda' do
+        context 'when the condition is met' do
+          subject { User.new(role: 'moderator') }
+
+          it { is_expected.to validate_length_of(:password).greater_than(10).check_conditions }
+        end
 
-        it { is_expected.not_to validate_length_of(:password) }
+        context 'when the condition is not met' do
+          subject { User.new(role: 'member') }
+
+          it { is_expected.not_to validate_length_of(:password).check_conditions }
+        end
       end
-    end
 
-    describe 'validations with unless condition using symbol' do
-      context 'when the condition is met' do
-        subject { Article.new(allow_comments: false) }
+      describe 'unless condition using symbol' do
+        context 'when the condition is met' do
+          subject { Article.new(allow_comments: false) }
+
+          it { is_expected.to validate_absence_of(:comments).check_conditions }
+        end
 
-        it { is_expected.to validate_absence_of(:comments) }
+        context 'when the condition is not met' do
+          subject { Article.new(allow_comments: true) }
+
+          it { is_expected.not_to validate_absence_of(:comments).check_conditions }
+        end
       end
 
-      context 'when the condition is not met' do
-        subject { Article.new(allow_comments: true) }
+      describe 'unless condition using lambda' do
+        context 'when the condition is met' do
+          subject { Article.new(status: :rejected) }
+
+          it { is_expected.to validate_presence_of(:reviewer).check_conditions }
+        end
 
-        it { is_expected.not_to validate_absence_of(:comments) }
+        context 'when the condition is not met' do
+          subject { Article.new(status: :pending) }
+
+          it { is_expected.not_to validate_presence_of(:reviewer).check_conditions }
+        end
       end
-    end
 
-    describe 'validations with unless condition using lambda' do
-      context 'when the condition is met' do
-        subject { Article.new(status: :rejected) }
+      describe 'lambda with explicit argument' do
+        context 'when the condition is met' do
+          subject { Article.new(status: :approved) }
+
+          it { is_expected.to validate_presence_of(:editor).check_conditions }
+        end
 
-        it { is_expected.to validate_presence_of(:reviewer) }
+        context 'when the condition is not met' do
+          subject { Article.new(status: :pending) }
+
+          it { is_expected.not_to validate_presence_of(:editor).check_conditions }
+        end
       end
 
-      context 'when the condition is not met' do
-        subject { Article.new(status: :pending) }
+      describe 'array of conditions (symbol + lambda)' do
+        context 'when all conditions are met' do
+          subject { Article.new(published: true, status: :approved) }
+
+          it { is_expected.to validate_presence_of(:summary).check_conditions }
+        end
+
+        context 'when symbol condition is not met' do
+          subject { Article.new(published: false, status: :approved) }
+
+          it { is_expected.not_to validate_presence_of(:summary).check_conditions }
+        end
+
+        context 'when lambda condition is not met' do
+          subject { Article.new(published: true, status: :pending) }
+
+          it { is_expected.not_to validate_presence_of(:summary).check_conditions }
+        end
+      end
 
-        it { is_expected.not_to validate_presence_of(:reviewer) }
+      describe 'raises error with Class subject' do
+        it 'raises ArgumentError when subject is a Class' do
+          expect do
+            expect(User).to validate_length_of(:password).check_conditions
+          end.to raise_error(ArgumentError, /must be an instance/)
+        end
       end
     end
   end