From 5630b134bb33932e51471c220e8bc717210b6ae3 Mon Sep 17 00:00:00 2001
From: David Davis <daviddavis@redhat.com>
Date: Mon, 22 Apr 2013 17:32:02 -0400
Subject: [PATCH] Moving before_destroy callbacks because of rails/rails#3458

---
 src/app/models/kt_environment.rb |  6 ++++--
 src/app/models/permission.rb     |  6 ++----
 src/app/models/user.rb           |  8 +++++---
 src/test/models/user_test.rb     | 14 ++++++++++++++
 4 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/src/app/models/kt_environment.rb b/src/app/models/kt_environment.rb
index f5e6dba059f..c2c73b3b6c8 100644
--- a/src/app/models/kt_environment.rb
+++ b/src/app/models/kt_environment.rb
@@ -21,6 +21,10 @@ class KTEnvironment < ActiveRecord::Base
   include Ext::PermissionTagCleanup
   acts_as_reportable
 
+  # RAILS3458: before_destroys before associations. see http://tinyurl.com/rails3458
+  before_destroy :confirm_last_env
+  before_destroy :delete_default_view_version
+
   belongs_to :organization, :inverse_of => :environments
   has_many :activation_keys, :dependent => :destroy, :foreign_key => :environment_id
   has_and_belongs_to_many :priors, {:class_name => "KTEnvironment", :foreign_key => :environment_id,
@@ -68,8 +72,6 @@ def <<(*items)
   validates_with Validators::PathDescendentsValidator
 
   after_create :create_default_content_view_version
-  before_destroy :confirm_last_env
-  before_destroy :delete_default_view_version
 
   after_destroy :unset_users_with_default
    ERROR_CLASS_NAME = "Environment"
diff --git a/src/app/models/permission.rb b/src/app/models/permission.rb
index 62f2186997a..1cc648f4c96 100644
--- a/src/app/models/permission.rb
+++ b/src/app/models/permission.rb
@@ -11,6 +11,8 @@
 # http://www.gnu.org/licenses/old-licenses/gpl-2.0.txt.
 
 class Permission < ActiveRecord::Base
+  before_destroy :check_locked # RAILS3458: must be before dependent associations http://tinyurl.com/rails3458
+
   belongs_to :resource_type
   belongs_to :organization
   belongs_to :role, :inverse_of => :permissions
@@ -24,11 +26,7 @@ class Permission < ActiveRecord::Base
   validates :name, :presence => true
   validates_with Validators::NonHtmlNameValidator, :attributes => :name
   validates_with Validators::KatelloDescriptionFormatValidator, :attributes => :description
-
   validates_uniqueness_of :name, :scope => [:organization_id, :role_id], :message => N_("Label has already been taken")
-
-  before_destroy :check_locked
-
   validates_with Validators::PermissionValidator
   validates_presence_of :resource_type
 
diff --git a/src/app/models/user.rb b/src/app/models/user.rb
index dbc698fa3ce..81eac2013e5 100644
--- a/src/app/models/user.rb
+++ b/src/app/models/user.rb
@@ -29,6 +29,10 @@ class User < ActiveRecord::Base
   scope :hidden, where(:hidden => true)
   scope :visible, where(:hidden => false)
 
+  # RAILS3458: THIS CHECK MUST BE THE FIRST before_destroy AND
+  # PROCEED DEPENDENT ASSOCIATIONS tinyurl.com/rails3458
+  before_destroy :not_last_super_user?, :destroy_own_role
+
   has_many :roles_users
   has_many :roles, :through => :roles_users, :before_remove => :super_admin_check, :uniq => true, :extend => RolesPermissions::UserOwnRole
   validates_with Validators::OwnRolePresenceValidator, :attributes => :roles
@@ -60,8 +64,6 @@ class User < ActiveRecord::Base
   after_validation :setup_remote_id
   before_save   :hash_password, :setup_preferences
   after_save :create_or_update_default_system_registration_permission
-  # THIS CHECK MUST BE THE FIRST before_destroy
-  before_destroy :is_last_super_user?, :destroy_own_role
 
   # hash the password before creating or updateing the record
   def hash_password
@@ -74,7 +76,7 @@ def setup_preferences
     self.preferences = HashWithIndifferentAccess.new unless self.preferences
   end
 
-  def is_last_super_user?
+  def not_last_super_user?
     if !User.current.nil?
       if self.id == User.current.id
         self.errors.add(:base, _("Cannot delete currently logged user"))
diff --git a/src/test/models/user_test.rb b/src/test/models/user_test.rb
index e98b2045531..59052e95f64 100644
--- a/src/test/models/user_test.rb
+++ b/src/test/models/user_test.rb
@@ -418,4 +418,18 @@ def test_default_env_removed
     assert_nil    @user.default_environment
   end
 
+  def test_before_destroy
+    # RAILS3458: Check that before_destroy callback is executed first http://tinyurl.com/rails3458
+    User.stubs(:current).returns(@user)
+    SearchFavorite.create!(:params => "abc",
+                           :path => "/garlic_naan",
+                           :user_id => @user.id
+                          )
+    @user.search_favorites.reload
+    refute_empty @user.search_favorites
+    refute @user.destroy
+
+    refute_empty @user.search_favorites
+  end
+
 end