:if should not fire on validations when not in context with :on

3 files changed, 17 insertions, 3 deletions

diff --git a/activemodel/lib/active_model/validations.rb b/activemodel/lib/active_model/validations.rb
index d968609e67..5e567307f3 100644
--- a/activemodel/lib/active_model/validations.rb
+++ b/activemodel/lib/active_model/validations.rb
@@ -133,7 +133,7 @@ module ActiveModel
         if options.key?(:on)
           options = options.dup
           options[:if] = Array.wrap(options[:if])
-          options[:if] << "validation_context == :#{options[:on]}"
+          options[:if].unshift("validation_context == :#{options[:on]}")
         end
         args << options
         set_callback(:validate, *args, &block)
diff --git a/activemodel/lib/active_model/validations/callbacks.rb b/activemodel/lib/active_model/validations/callbacks.rb
index adc2867ad0..f187d6f378 100644
--- a/activemodel/lib/active_model/validations/callbacks.rb
+++ b/activemodel/lib/active_model/validations/callbacks.rb
@@ -31,7 +31,7 @@ module ActiveModel
           options = args.last
           if options.is_a?(Hash) && options[:on]
             options[:if] = Array.wrap(options[:if])
-            options[:if] << "self.validation_context == :#{options[:on]}"
+            options[:if].unshift("self.validation_context == :#{options[:on]}")
           end
           set_callback(:validation, :before, *args, &block)
         end
@@ -41,7 +41,7 @@ module ActiveModel
           options[:prepend] = true
           options[:if] = Array.wrap(options[:if])
           options[:if] << "!halted"
-          options[:if] << "self.validation_context == :#{options[:on]}" if options[:on]
+          options[:if].unsfhit("self.validation_context == :#{options[:on]}") if options[:on]
           set_callback(:validation, :after, *(args << options), &block)
         end
       end
diff --git a/activemodel/test/cases/validations_test.rb b/activemodel/test/cases/validations_test.rb
index 2f36195627..0b50acf913 100644
--- a/activemodel/test/cases/validations_test.rb
+++ b/activemodel/test/cases/validations_test.rb
@@ -212,6 +212,20 @@ class ValidationsTest < ActiveModel::TestCase
     assert_equal 'is too short (minimum is 2 characters)', t.errors[key][0]
   end
 
+  def test_validaton_with_if_and_on
+    Topic.validates_presence_of :title, :if => Proc.new{|x| x.author_name = "bad"; true }, :on => :update
+
+    t = Topic.new(:title => "")
+
+    # If block should not fire
+    assert t.valid?
+    assert t.author_name.nil?
+
+    # If block should fire
+    assert t.invalid?(:update)
+    assert t.author_name == "bad"
+  end
+
   def test_invalid_should_be_the_opposite_of_valid
     Topic.validates_presence_of :title