From 9a89efa5e8f0799da22139a50a4c64a2bbdff638 Mon Sep 17 00:00:00 2001 From: Thomas Romera Date: Tue, 19 Jul 2016 11:06:45 +0200 Subject: [PATCH] Fixes a problem of ambiguous table names when using only_deleted method and joining tables that have a scope on `with_deleted` Update homepage in gemspec The homepage is supposed to be where you can find the code. It is displayed on rubygems.org page. Currently it will redirect you back to the same page. Should point at this github repo. Update README to use proper version for Rails 5 Version 2.2.0 Ignore failures from all jruby's on travis Add explicit language about dependent: :destroy Update CHANGELOG.md update ruby and rails versions Use ActiveSupport.on_load to correctly re-open ActiveRecord::Base. https://github.com/rubysherpas/paranoia/issues/335 Touch record on paranoia-destroy. Fixes #296 Touch record on destroy by leveraging the paranoia_destroy_attributes. Applied the same to the restore-method as this eliminates the extra query. --- lib/paranoia.rb | 5 ++-- test/paranoia_test.rb | 57 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 59 insertions(+), 3 deletions(-) diff --git a/lib/paranoia.rb b/lib/paranoia.rb index 489d2d62..69ca3a67 100644 --- a/lib/paranoia.rb +++ b/lib/paranoia.rb @@ -35,8 +35,9 @@ def only_deleted # some deleted rows will hold a null value in the paranoia column # these will not match != sentinel value because "NULL != value" is # NULL under the sql standard - quoted_paranoia_column = connection.quote_column_name(paranoia_column) - with_deleted.where("#{quoted_paranoia_column} IS NULL OR #{quoted_paranoia_column} != ?", paranoia_sentinel_value) + # Scoping with the table_name is mandatory to avoid ambiguous errors when joining tables. + scoped_quoted_paranoia_column = "#{self.table_name}.#{connection.quote_column_name(paranoia_column)}" + with_deleted.where("#{scoped_quoted_paranoia_column} IS NULL OR #{scoped_quoted_paranoia_column} != ?", paranoia_sentinel_value) end alias_method :deleted, :only_deleted diff --git a/test/paranoia_test.rb b/test/paranoia_test.rb index a1bceeee..e8133411 100644 --- a/test/paranoia_test.rb +++ b/test/paranoia_test.rb @@ -40,6 +40,8 @@ def setup! 'unparanoid_unique_models' => 'name VARCHAR(32), paranoid_with_unparanoids_id INTEGER', 'active_column_models' => 'deleted_at DATETIME, active BOOLEAN', 'active_column_model_with_uniqueness_validations' => 'name VARCHAR(32), deleted_at DATETIME, active BOOLEAN', + 'paranoid_model_with_belongs_to_active_column_model_with_has_many_relationships' => 'name VARCHAR(32), deleted_at DATETIME, active BOOLEAN, active_column_model_with_has_many_relationship_id INTEGER', + 'active_column_model_with_has_many_relationships' => 'name VARCHAR(32), deleted_at DATETIME, active BOOLEAN', 'without_default_scope_models' => 'deleted_at DATETIME' }.each do |table_name, columns_as_sql_string| ActiveRecord::Base.connection.execute "CREATE TABLE #{table_name} (id INTEGER NOT NULL PRIMARY KEY, #{columns_as_sql_string})" @@ -182,8 +184,11 @@ def test_scoping_behavior_for_paranoid_models p2 = ParanoidModel.create(:parent_model => parent2) p1.destroy p2.destroy + assert_equal 0, parent1.paranoid_models.count assert_equal 1, parent1.paranoid_models.only_deleted.count + + assert_equal 2, ParanoidModel.only_deleted.joins(:parent_model).count assert_equal 1, parent1.paranoid_models.deleted.count assert_equal 0, parent1.paranoid_models.without_deleted.count p3 = ParanoidModel.create(:parent_model => parent1) @@ -192,6 +197,17 @@ def test_scoping_behavior_for_paranoid_models assert_equal [p1,p3], parent1.paranoid_models.with_deleted end + def test_only_deleted_with_joins + c1 = ActiveColumnModelWithHasManyRelationship.create(name: 'Jacky') + c2 = ActiveColumnModelWithHasManyRelationship.create(name: 'Thomas') + p1 = ParanoidModelWithBelongsToActiveColumnModelWithHasManyRelationship.create(name: 'Hello', active_column_model_with_has_many_relationship: c1) + + c1.destroy + assert_equal 1, ActiveColumnModelWithHasManyRelationship.count + assert_equal 1, ActiveColumnModelWithHasManyRelationship.only_deleted.count + assert_equal 1, ActiveColumnModelWithHasManyRelationship.only_deleted.joins(:paranoid_model_with_belongs_to_active_column_model_with_has_many_relationships).count + end + def test_destroy_behavior_for_custom_column_models model = CustomColumnModel.new assert_equal 0, model.class.count @@ -1105,6 +1121,45 @@ def paranoia_destroy_attributes end end +class ActiveColumnModelWithHasManyRelationship < ActiveRecord::Base + has_many :paranoid_model_with_belongs_to_active_column_model_with_has_many_relationships + acts_as_paranoid column: :active, sentinel_value: true + + def paranoia_restore_attributes + { + deleted_at: nil, + active: true + } + end + + def paranoia_destroy_attributes + { + deleted_at: current_time_from_proper_timezone, + active: nil + } + end +end + +class ParanoidModelWithBelongsToActiveColumnModelWithHasManyRelationship < ActiveRecord::Base + belongs_to :active_column_model_with_has_many_relationship + + acts_as_paranoid column: :active, sentinel_value: true + + def paranoia_restore_attributes + { + deleted_at: nil, + active: true + } + end + + def paranoia_destroy_attributes + { + deleted_at: current_time_from_proper_timezone, + active: nil + } + end +end + class NonParanoidModel < ActiveRecord::Base end @@ -1208,4 +1263,4 @@ class ParanoidBelongsTo < ActiveRecord::Base acts_as_paranoid belongs_to :paranoid_has_one end -end +end \ No newline at end of file