Merge pull request #236 from igorkasyanchuk/234-problem-with-conditio…

…nal-validations

[Matcher] Bugfix when several validator are used on the same attribute (#234)

lib/active_storage_validations/matchers/concerns/contextable.rb

@@ -17,19 +17,31 @@ def on(context)
       private
 
       def is_context_valid?
-        return true if !@context && !(attribute_validator && attribute_validator.options[:on])
+        return true if !@context && attribute_validators.none? { |validator| validator.options[:on] }
 
-        raise ArgumentError, "This validator matcher needs the #on option to work since its validator has one" if !@context
+        raise ArgumentError, "This validator matcher needs the #on option to work since its validator has one" if !@context && attribute_validators.all? { |validator| validator.options[:on] }
         raise ArgumentError, "This validator matcher option only allows a symbol or an array" if !(@context.is_a?(Symbol) || @context.is_a?(Array))
 
-        if @context.is_a?(Array) && attribute_validator.options[:on].is_a?(Array)
-          @context.to_set == attribute_validator.options[:on].to_set
-        elsif @context.is_a?(Symbol) && attribute_validator.options[:on].is_a?(Symbol)
-          @context == attribute_validator.options[:on]
-        else
-          false
+        if @context.is_a?(Array)
+          (validator_contexts & @context.map(&:to_s)) == validator_contexts || raise_context_not_listed_error
+        elsif @context.is_a?(Symbol)
+          validator_contexts.include?(@context.to_s) || raise_context_not_listed_error
         end
       end
+
+      def validator_contexts
+        attribute_validators.map do |validator|
+          case validator.options[:on]
+          when Array then validator.options[:on].map { |context| context.to_s }
+          when NilClass then nil
+          else validator.options[:on].to_s
+          end
+        end.flatten.compact
+      end
+
+      def raise_context_not_listed_error
+        raise ArgumentError, "One of the provided contexts to the #on method is not found in any of the listed contexts for this attribute"
+      end
     end
   end
 end

lib/active_storage_validations/matchers/concerns/validatable.rb

@@ -26,7 +26,7 @@ def is_valid?
       def available_errors
         [
           *validator_class::ERROR_TYPES,
-          *error_from_custom_message
+          *errors_from_custom_messages
         ].compact
       end
 
@@ -40,8 +40,14 @@ def attribute_validator
         end
       end
 
-      def error_from_custom_message
-        attribute_validator.options[:message]
+      def attribute_validators
+        @subject.class.validators_on(@attribute_name).select do |validator|
+          validator.class == validator_class
+        end
+      end
+
+      def errors_from_custom_messages
+        attribute_validators.map { |validator| validator.options[:message] }
       end
     end
   end

test/dummy/app/models/aspect_ratio/matcher.rb

@@ -11,6 +11,8 @@
 #
 
 class AspectRatio::Matcher < ApplicationRecord
+  include Validatable
+
   ActiveStorageValidations::AspectRatioValidator::NAMED_ASPECT_RATIOS.each do |aspect_ratio|
     has_one_attached :"allowing_one_#{aspect_ratio}"
     validates :"allowing_one_#{aspect_ratio}", aspect_ratio: aspect_ratio
@@ -28,10 +30,17 @@ class AspectRatio::Matcher < ApplicationRecord
   validates :with_context_symbol, aspect_ratio: :square, on: :update
   has_one_attached :with_context_array
   validates :with_context_array, aspect_ratio: :square, on: %i[update custom]
+  has_one_attached :with_several_validators_and_contexts
+  validates :with_several_validators_and_contexts, aspect_ratio: :square, on: :update
+  validates :with_several_validators_and_contexts, aspect_ratio: :square, on: :custom
 
   has_one_attached :as_instance
   validates :as_instance, aspect_ratio: :square
 
+  has_one_attached :validatable_different_error_messages
+  validates :validatable_different_error_messages, aspect_ratio: { with: :portrait, message: 'Custom message 1' }, if: :title_is_quo_vadis?
+  validates :validatable_different_error_messages, aspect_ratio: { with: :square, message: 'Custom message 2' }, if: :title_is_american_psycho?
+
   has_one_attached :failure_message
   validates :failure_message, aspect_ratio: :square
   has_one_attached :failure_message_when_negated

test/dummy/app/models/attached/matcher.rb

@@ -11,6 +11,8 @@
 #
 
 class Attached::Matcher < ApplicationRecord
+  include Validatable
+
   has_one_attached :required
   validates :required, attached: true
 
@@ -21,10 +23,17 @@ class Attached::Matcher < ApplicationRecord
   validates :with_context_symbol, attached: true, on: :update
   has_one_attached :with_context_array
   validates :with_context_array, attached: true, on: %i[update custom]
+  has_one_attached :with_several_validators_and_contexts
+  validates :with_several_validators_and_contexts, attached: true, on: :update
+  validates :with_several_validators_and_contexts, attached: true, on: :custom
 
   has_one_attached :as_instance
   validates :as_instance, attached: true
 
+  has_one_attached :validatable_different_error_messages
+  validates :validatable_different_error_messages, attached: { message: 'Custom message 1' }, if: :title_is_quo_vadis?
+  validates :validatable_different_error_messages, attached: { message: 'Custom message 2' }, if: :title_is_american_psycho?
+
   has_one_attached :failure_message
   validates :failure_message, attached: true
   has_one_attached :failure_message_when_negated

test/dummy/app/models/concerns/validatable.rb

@@ -0,0 +1,17 @@
+module Validatable
+  extend ActiveSupport::Concern
+
+  private
+
+  def title_is_quo_vadis?
+    return false if self.title.blank?
+
+    self.title == "Quo vadis"
+  end
+
+  def title_is_american_psycho?
+    return false if self.title.blank?
+
+    self.title == "American Psycho"
+  end
+end

test/dummy/app/models/content_type/matcher.rb

@@ -11,6 +11,8 @@
 #
 
 class ContentType::Matcher < ApplicationRecord
+  include Validatable
+
   has_one_attached :allowing_one
   validates :allowing_one, content_type: :png
   has_one_attached :allowing_several
@@ -28,10 +30,17 @@ class ContentType::Matcher < ApplicationRecord
   validates :with_context_symbol, content_type: :png, on: :update
   has_one_attached :with_context_array
   validates :with_context_array, content_type: :png, on: %i[update custom]
+  has_one_attached :with_several_validators_and_contexts
+  validates :with_several_validators_and_contexts, content_type: :png, on: :update
+  validates :with_several_validators_and_contexts, content_type: :png, on: :custom
 
   has_one_attached :as_instance
   validates :as_instance, content_type: :png
 
+  has_one_attached :validatable_different_error_messages
+  validates :validatable_different_error_messages, content_type: { with: :pdf, message: 'Custom message 1' }, if: :title_is_quo_vadis?
+  validates :validatable_different_error_messages, content_type: { with: :png, message: 'Custom message 2' }, if: :title_is_american_psycho?
+
   has_one_attached :failure_message
   validates :failure_message, content_type: :png
   has_one_attached :failure_message_when_negated

test/dummy/app/models/dimension/matcher.rb

@@ -11,6 +11,8 @@
 #
 
 class Dimension::Matcher < ApplicationRecord
+  include Validatable
+
   %i(width height).each do |dimension|
     has_one_attached :"#{dimension}_exact"
     has_one_attached :"#{dimension}_in"
@@ -53,10 +55,17 @@ class Dimension::Matcher < ApplicationRecord
   validates :with_context_symbol, dimension: { width: 150, height: 150 }, on: :update
   has_one_attached :with_context_array
   validates :with_context_array, dimension: { width: 150, height: 150 }, on: %i[update custom]
+  has_one_attached :with_several_validators_and_contexts
+  validates :with_several_validators_and_contexts, dimension: { width: 150, height: 150 }, on: :update
+  validates :with_several_validators_and_contexts, dimension: { width: 150, height: 150 }, on: :custom
 
   has_one_attached :as_instance
   validates :as_instance, dimension: { width: 150, height: 150 }
 
+  has_one_attached :validatable_different_error_messages
+  validates :validatable_different_error_messages, dimension: { width: 150, message: 'Custom message 1' }, if: :title_is_quo_vadis?
+  validates :validatable_different_error_messages, dimension: { width: 150, message: 'Custom message 2' }, if: :title_is_american_psycho?
+
   has_one_attached :failure_message
   validates :failure_message, dimension: { width: 150, height: 150 }
   has_one_attached :failure_message_when_negated

test/dummy/app/models/size/matcher.rb

@@ -11,6 +11,8 @@
 #
 
 class Size::Matcher < ApplicationRecord
+  include Validatable
+
   has_one_attached :less_than
   has_one_attached :less_than_or_equal_to
   has_one_attached :greater_than
@@ -43,10 +45,17 @@ class Size::Matcher < ApplicationRecord
   validates :with_context_symbol, size: { less_than_or_equal_to: 5.megabytes }, on: :update
   has_one_attached :with_context_array
   validates :with_context_array, size: { less_than_or_equal_to: 5.megabytes }, on: %i[update custom]
+  has_one_attached :with_several_validators_and_contexts
+  validates :with_several_validators_and_contexts, size: { less_than_or_equal_to: 5.megabytes }, on: :update
+  validates :with_several_validators_and_contexts, size: { less_than_or_equal_to: 5.megabytes }, on: :custom
 
   has_one_attached :as_instance
   validates :as_instance, size: { less_than_or_equal_to: 5.megabytes }
 
+  has_one_attached :validatable_different_error_messages
+  validates :validatable_different_error_messages, size: { less_than: 20.megabytes, message: 'Custom message 1' }, if: :title_is_quo_vadis?
+  validates :validatable_different_error_messages, size: { less_than: 10.megabytes, message: 'Custom message 2' }, if: :title_is_american_psycho?
+
   has_one_attached :failure_message
   validates :failure_message, size: { less_than_or_equal_to: 5.megabytes }
   has_one_attached :failure_message_when_negated

test/matchers/aspect_ratio_validator_matcher_test.rb

@@ -2,6 +2,7 @@
 
 require 'test_helper'
 require 'matchers/shared_examples/checks_if_is_a_valid_active_storage_attribute'
+require 'matchers/shared_examples/checks_if_is_valid'
 require 'matchers/shared_examples/has_valid_rspec_message_methods'
 require 'matchers/shared_examples/works_with_allow_blank'
 require 'matchers/shared_examples/works_with_both_instance_and_class'
@@ -11,8 +12,9 @@
 describe ActiveStorageValidations::Matchers::AspectRatioValidatorMatcher do
   include MatcherHelpers
 
-  include HasValidRspecMessageMethods
   include ChecksIfIsAValidActiveStorageAttribute
+  include ChecksIfIsValid
+  include HasValidRspecMessageMethods
   include WorksWithBothInstanceAndClass
 
   let(:matcher) { ActiveStorageValidations::Matchers::AspectRatioValidatorMatcher.new(model_attribute) }

test/matchers/attached_validator_matcher_test.rb

@@ -2,6 +2,7 @@
 
 require 'test_helper'
 require 'matchers/shared_examples/checks_if_is_a_valid_active_storage_attribute'
+require 'matchers/shared_examples/checks_if_is_valid'
 require 'matchers/shared_examples/has_valid_rspec_message_methods'
 require 'matchers/shared_examples/works_with_both_instance_and_class'
 require 'matchers/shared_examples/works_with_context'
@@ -10,8 +11,9 @@
 describe ActiveStorageValidations::Matchers::AttachedValidatorMatcher do
   include MatcherHelpers
 
-  include HasValidRspecMessageMethods
   include ChecksIfIsAValidActiveStorageAttribute
+  include ChecksIfIsValid
+  include HasValidRspecMessageMethods
   include WorksWithBothInstanceAndClass
 
   let(:matcher) { ActiveStorageValidations::Matchers::AttachedValidatorMatcher.new(model_attribute) }

test/matchers/content_type_validator_matcher_test.rb

@@ -2,6 +2,7 @@
 
 require 'test_helper'
 require 'matchers/shared_examples/checks_if_is_a_valid_active_storage_attribute'
+require 'matchers/shared_examples/checks_if_is_valid'
 require 'matchers/shared_examples/has_valid_rspec_message_methods'
 require 'matchers/shared_examples/works_with_allow_blank'
 require 'matchers/shared_examples/works_with_both_instance_and_class'
@@ -11,8 +12,9 @@
 describe ActiveStorageValidations::Matchers::ContentTypeValidatorMatcher do
   include MatcherHelpers
 
-  include HasValidRspecMessageMethods
   include ChecksIfIsAValidActiveStorageAttribute
+  include ChecksIfIsValid
+  include HasValidRspecMessageMethods
   include WorksWithBothInstanceAndClass
 
   let(:matcher) { ActiveStorageValidations::Matchers::ContentTypeValidatorMatcher.new(model_attribute) }

test/matchers/dimension_validator_matcher_test.rb

@@ -2,6 +2,7 @@
 
 require 'test_helper'
 require 'matchers/shared_examples/checks_if_is_a_valid_active_storage_attribute'
+require 'matchers/shared_examples/checks_if_is_valid'
 require 'matchers/shared_examples/has_valid_rspec_message_methods'
 require 'matchers/shared_examples/works_with_allow_blank'
 require 'matchers/shared_examples/works_with_both_instance_and_class'
@@ -141,8 +142,9 @@ module OnlyMatchWhenExactValues
 describe ActiveStorageValidations::Matchers::DimensionValidatorMatcher do
   include MatcherHelpers
 
-  include HasValidRspecMessageMethods
   include ChecksIfIsAValidActiveStorageAttribute
+  include ChecksIfIsValid
+  include HasValidRspecMessageMethods
   include WorksWithBothInstanceAndClass
 
   let(:matcher) { ActiveStorageValidations::Matchers::DimensionValidatorMatcher.new(model_attribute) }

test/matchers/shared_examples/checks_if_is_valid.rb

@@ -0,0 +1,28 @@
+module ChecksIfIsValid
+  extend ActiveSupport::Concern
+
+  included do
+    describe 'Edge cases' do
+      describe 'when the validator is used several times on the same attributes' do
+        describe 'and is provided with different error messages' do
+          before do
+            case validator_sym
+            when :aspect_ratio then matcher.allowing(:square)
+            when :attached then matcher
+            when :content_type then matcher.rejecting('image/jpg')
+            when :dimension then matcher.width(150)
+            when :size then matcher.less_than(10.megabytes)
+            end
+          end
+
+          subject { matcher }
+
+          let(:model_attribute) { :validatable_different_error_messages }
+          let(:instance) { klass.new(title: "American Psycho") }
+
+          it { is_expected_to_match_for(instance) }
+        end
+      end
+    end
+  end
+end