From 73f8c16601e51480d007dca5a33f2035293bdd23 Mon Sep 17 00:00:00 2001
From: eileencodes <eileencodes@gmail.com>
Date: Wed, 24 Feb 2016 13:45:36 -0500
Subject: [PATCH] Ensure suppressor runs before validations

I ran into an issue where validations on a suppressed record were
causing validation errors to be thrown on a record that was never going
to be saved.

There isn't a reason to run the validations on a record that doesn't
matter.

This change moves the suppressor up the chain to be run on the `save` or
`save!` in the validations rather than in persistence. The issue with
running it when we hit persistence is that the validations are run
first, then we hit persistance, and then we hit the suppressor. The
suppressor comes first.

The change to the test was required since I added the
`validates_presence_of` validations. Adding this alone was enough to
demonstrate the issue. I added a new test to demonstrate the new
behavior is explict.
---
 activerecord/CHANGELOG.md                    |  7 +++++++
 activerecord/lib/active_record/suppressor.rb |  6 +++++-
 activerecord/test/cases/suppressor_test.rb   | 13 ++++++++++++-
 activerecord/test/models/notification.rb     |  1 +
 4 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index c1a8803457..39fe217777 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,10 @@
+*   Ensure that the Suppressor runs before validations.
+
+    This moves the suppressor up to be run before validations rather than after
+    validations. There's no reason to validate a record you aren't planning on saving.
+
+    *Eileen M. Uchitelle*
+
 ## Rails 5.0.0.beta3 (February 24, 2016) ##
 
 *   Ensure that mutations of the array returned from `ActiveRecord::Relation#to_a`
diff --git a/activerecord/lib/active_record/suppressor.rb b/activerecord/lib/active_record/suppressor.rb
index b3644bf569..8ec4b48d31 100644
--- a/activerecord/lib/active_record/suppressor.rb
+++ b/activerecord/lib/active_record/suppressor.rb
@@ -37,7 +37,11 @@ def suppress(&block)
       end
     end
 
-    def create_or_update(*args) # :nodoc:
+    def save(*) # :nodoc:
+      SuppressorRegistry.suppressed[self.class.name] ? true : super
+    end
+
+    def save!(*) # :nodoc:
       SuppressorRegistry.suppressed[self.class.name] ? true : super
     end
   end
diff --git a/activerecord/test/cases/suppressor_test.rb b/activerecord/test/cases/suppressor_test.rb
index 72c5c16555..7d44e36419 100644
--- a/activerecord/test/cases/suppressor_test.rb
+++ b/activerecord/test/cases/suppressor_test.rb
@@ -46,7 +46,18 @@ def test_resumes_saving_after_suppression_complete
     Notification.suppress { UserWithNotification.create! }
 
     assert_difference -> { Notification.count } do
-      Notification.create!
+      Notification.create!(message: "New Comment")
+    end
+  end
+
+  def test_suppresses_validations_on_create
+    assert_no_difference -> { Notification.count } do
+      Notification.suppress do
+        User.create
+        User.create!
+        User.new.save
+        User.new.save!
+      end
     end
   end
 end
diff --git a/activerecord/test/models/notification.rb b/activerecord/test/models/notification.rb
index b4b4b8f1b6..82edc64b68 100644
--- a/activerecord/test/models/notification.rb
+++ b/activerecord/test/models/notification.rb
@@ -1,2 +1,3 @@
 class Notification < ActiveRecord::Base
+  validates_presence_of :message
 end