Merge pull request #379 from Erowlin/fix_ambiguous_table_names_on_only_deleted_joins

Fixes a problem of ambiguous table names when using only_deleted meth…

lib/paranoia.rb

@@ -35,8 +35,9 @@ module Paranoia
       # some deleted rows will hold a null value in the paranoia column
       # some deleted rows will hold a null value in the paranoia column
       # these will not match != sentinel value because "NULL != value" is
       # these will not match != sentinel value because "NULL != value" is
       # NULL under the sql standard
       # 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
     end
     alias_method :deleted, :only_deleted
     alias_method :deleted, :only_deleted
 
 

test/paranoia_test.rb

@@ -40,6 +40,8 @@ def setup!
     'unparanoid_unique_models' => 'name VARCHAR(32), paranoid_with_unparanoids_id INTEGER',
     'unparanoid_unique_models' => 'name VARCHAR(32), paranoid_with_unparanoids_id INTEGER',
     'active_column_models' => 'deleted_at DATETIME, active BOOLEAN',
     'active_column_models' => 'deleted_at DATETIME, active BOOLEAN',
     'active_column_model_with_uniqueness_validations' => 'name VARCHAR(32), 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'
     'without_default_scope_models' => 'deleted_at DATETIME'
   }.each do |table_name, columns_as_sql_string|
   }.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})"
     ActiveRecord::Base.connection.execute "CREATE TABLE #{table_name} (id INTEGER NOT NULL PRIMARY KEY, #{columns_as_sql_string})"
@@ -182,8 +184,11 @@ class ParanoiaTest < test_framework
     p2 = ParanoidModel.create(:parent_model => parent2)
     p2 = ParanoidModel.create(:parent_model => parent2)
     p1.destroy
     p1.destroy
     p2.destroy
     p2.destroy
+    
     assert_equal 0, parent1.paranoid_models.count
     assert_equal 0, parent1.paranoid_models.count
     assert_equal 1, parent1.paranoid_models.only_deleted.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 1, parent1.paranoid_models.deleted.count
     assert_equal 0, parent1.paranoid_models.without_deleted.count
     assert_equal 0, parent1.paranoid_models.without_deleted.count
     p3 = ParanoidModel.create(:parent_model => parent1)
     p3 = ParanoidModel.create(:parent_model => parent1)
@@ -192,6 +197,17 @@ class ParanoiaTest < test_framework
     assert_equal [p1,p3], parent1.paranoid_models.with_deleted
     assert_equal [p1,p3], parent1.paranoid_models.with_deleted
   end
   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
   def test_destroy_behavior_for_custom_column_models
     model = CustomColumnModel.new
     model = CustomColumnModel.new
     assert_equal 0, model.class.count
     assert_equal 0, model.class.count
@@ -1112,6 +1128,45 @@ class ActiveColumnModelWithUniquenessValidation < ActiveRecord::Base
   end
   end
 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
 class NonParanoidModel < ActiveRecord::Base
 end
 end