Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes a problem of ambiguous table names when using only_deleted method and joining tables that have a scope on with_deleted #346

22 changes: 13 additions & 9 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,24 @@ language: ruby
cache: bundler
rvm:
- 2.1.10
- 2.2.5
- 2.3.1
- jruby-9.1.0.0
- 2.2.6
- 2.3.3
- jruby-9.1.6.0

env:
matrix:
- RAILS='~> 4.1.15'
- RAILS='~> 4.2.6'
- RAILS='~> 5.0.0'
- RAILS='~> 4.1.16'
- RAILS='~> 4.2.7.1'
- RAILS='~> 5.0.0.1'

matrix:
exclude:
- env: RAILS='~> 5.0.0'
- env: RAILS='~> 5.0.0.1'
rvm: 2.1.10
allow_failures:
- env: RAILS='~> 5.0.0'
rvm: jruby-9.1.0.0
- env: RAILS='~> 4.1.16'
rvm: jruby-9.1.6.0
- env: RAILS='~> 4.2.7.1'
rvm: jruby-9.1.6.0
- env: RAILS='~> 5.0.0.1'
rvm: jruby-9.1.6.0
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# paranoia Changelog

## 2.2.0 (unreleased)
## 2.2.0 (2016-10-21)

* Ruby 2.0 or greater is required
* Rails 5.0.0.beta1.1 support [@pigeonworks](https://github.com/pigeonworks) [@halostatue](https://github.com/halostatue) and [@gagalago](https://github.com/gagalago)
Expand Down
77 changes: 74 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ When your app is using Paranoia, calling `destroy` on an ActiveRecord object doe

If you wish to actually destroy an object you may call `really_destroy!`. **WARNING**: This will also *really destroy* all `dependent: :destroy` records, so please aim this method away from face when using.

If a record has `has_many` associations defined AND those associations have `dependent: :destroy` set on them, then they will also be soft-deleted if `acts_as_paranoid` is set, otherwise the normal destroy will be called.
If a record has `has_many` associations defined AND those associations have `dependent: :destroy` set on them, then they will also be soft-deleted if `acts_as_paranoid` is set, otherwise the normal destroy will be called. ***See [Destroying through association callbacks](#destroying-through-association-callbacks) for clarifying examples.***

## Getting Started Video
Setup and basic usage of the paranoia gem
Expand All @@ -20,10 +20,10 @@ For Rails 3, please use version 1 of Paranoia:
gem "paranoia", "~> 1.0"
```

For Rails 4 or 5, please use version 2 of Paranoia:
For Rails 4 and 5, please use version 2 of Paranoia (2.2 or greater required for rails 5):

``` ruby
gem "paranoia", "~> 2.0"
gem "paranoia", "~> 2.2"
```

Of course you can install this from GitHub as well from one of these examples:
Expand Down Expand Up @@ -245,6 +245,77 @@ class Client < ActiveRecord::Base
end
```

##### Destroying through association callbacks

When dealing with `dependent: :destroy` associations and `acts_as_paranoid`, it's important to remember that whatever method is called on the parent model will be called on the child model. For example, given both models of an association have `acts_as_paranoid` defined:

``` ruby
class Client < ActiveRecord::Base
acts_as_paranoid

has_many :emails, dependent: :destroy
end

class Email < ActiveRecord::Base
acts_as_paranoid

belongs_to :client
end
```

When we call `destroy` on the parent `client`, it will call `destroy` on all of its associated children `emails`:

``` ruby
>> client.emails.count
# => 5
>> client.destroy
# => client
>> client.deleted_at
# => [current timestamp]
>> Email.where(client_id: client.id).count
# => 0
>> Email.with_deleted.where(client_id: client.id).count
# => 5
```

Similarly, when we call `really_destroy!` on the parent `client`, then each child `email` will also have `really_destroy!` called:

``` ruby
>> client.emails.count
# => 5
>> client.id
# => 12345
>> client.really_destroy!
# => client
>> Client.find 12345
# => ActiveRecord::RecordNotFound
>> Email.with_deleted.where(client_id: client.id).count
# => 0
```

However, if the child model `Email` does not have `acts_as_paranoid` set, then calling `destroy` on the parent `client` will also call `destroy` on each child `email`, thereby actually destroying them:

``` ruby
class Client < ActiveRecord::Base
acts_as_paranoid

has_many :emails, dependent: :destroy
end

class Email < ActiveRecord::Base
belongs_to :client
end

>> client.emails.count
# => 5
>> client.destroy
# => client
>> Email.where(client_id: client.id).count
# => 0
>> Email.with_deleted.where(client_id: client.id).count
# => NoMethodError: undefined method `with_deleted' for #<Class:0x0123456>
```

## Acts As Paranoid Migration

You can replace the older `acts_as_paranoid` methods as follows:
Expand Down
88 changes: 47 additions & 41 deletions lib/paranoia.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -110,7 +111,6 @@ def restore!(opts = {})
if (noop_if_frozen && [email protected]?) || !noop_if_frozen
write_attribute paranoia_column, paranoia_sentinel_value
update_columns(paranoia_restore_attributes)
touch
end
restore_associated_records if opts[:recursive]
end
Expand Down Expand Up @@ -154,13 +154,17 @@ def really_destroy!
def paranoia_restore_attributes
{
paranoia_column => paranoia_sentinel_value
}
}.merge(timestamp_attributes_with_current_time)
end

def paranoia_destroy_attributes
{
paranoia_column => current_time_from_proper_timezone
}
}.merge(timestamp_attributes_with_current_time)
end

def timestamp_attributes_with_current_time
timestamp_attributes_for_update_in_model.each_with_object({}) { |attr,hash| hash[attr] = current_time_from_proper_timezone }
end

# restore associated records that have been soft deleted when
Expand Down Expand Up @@ -205,56 +209,58 @@ def restore_associated_records
end
end

class ActiveRecord::Base
def self.acts_as_paranoid(options={})
alias_method :really_destroyed?, :destroyed?
alias_method :really_delete, :delete
alias_method :destroy_without_paranoia, :destroy
ActiveSupport.on_load(:active_record) do
class ActiveRecord::Base
def self.acts_as_paranoid(options={})
alias_method :really_destroyed?, :destroyed?
alias_method :really_delete, :delete
alias_method :destroy_without_paranoia, :destroy

include Paranoia
class_attribute :paranoia_column, :paranoia_sentinel_value
include Paranoia
class_attribute :paranoia_column, :paranoia_sentinel_value

self.paranoia_column = (options[:column] || :deleted_at).to_s
self.paranoia_sentinel_value = options.fetch(:sentinel_value) { Paranoia.default_sentinel_value }
def self.paranoia_scope
where(paranoia_column => paranoia_sentinel_value)
end
class << self; alias_method :without_deleted, :paranoia_scope end
self.paranoia_column = (options[:column] || :deleted_at).to_s
self.paranoia_sentinel_value = options.fetch(:sentinel_value) { Paranoia.default_sentinel_value }
def self.paranoia_scope
where(paranoia_column => paranoia_sentinel_value)
end
class << self; alias_method :without_deleted, :paranoia_scope end

unless options[:without_default_scope]
default_scope { paranoia_scope }
end
unless options[:without_default_scope]
default_scope { paranoia_scope }
end

before_restore {
self.class.notify_observers(:before_restore, self) if self.class.respond_to?(:notify_observers)
}
after_restore {
self.class.notify_observers(:after_restore, self) if self.class.respond_to?(:notify_observers)
}
end
before_restore {
self.class.notify_observers(:before_restore, self) if self.class.respond_to?(:notify_observers)
}
after_restore {
self.class.notify_observers(:after_restore, self) if self.class.respond_to?(:notify_observers)
}
end

# Please do not use this method in production.
# Pretty please.
def self.I_AM_THE_DESTROYER!
# TODO: actually implement spelling error fixes
# Please do not use this method in production.
# Pretty please.
def self.I_AM_THE_DESTROYER!
# TODO: actually implement spelling error fixes
puts %Q{
Sharon: "There should be a method called I_AM_THE_DESTROYER!"
Ryan: "What should this method do?"
Sharon: "It should fix all the spelling errors on the page!"
}
end
end

def self.paranoid? ; false ; end
def paranoid? ; self.class.paranoid? ; end
def self.paranoid? ; false ; end
def paranoid? ; self.class.paranoid? ; end

private
private

def paranoia_column
self.class.paranoia_column
end
def paranoia_column
self.class.paranoia_column
end

def paranoia_sentinel_value
self.class.paranoia_sentinel_value
def paranoia_sentinel_value
self.class.paranoia_sentinel_value
end
end
end

Expand Down
2 changes: 1 addition & 1 deletion lib/paranoia/version.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
module Paranoia
VERSION = "2.2.0.pre"
VERSION = "2.2.0"
end
2 changes: 1 addition & 1 deletion paranoia.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ Gem::Specification.new do |s|
s.platform = Gem::Platform::RUBY
s.authors = %w([email protected])
s.email = %w([email protected] [email protected])
s.homepage = "http://rubygems.org/gems/paranoia"
s.homepage = "https://github.com/rubysherpas/paranoia"
s.license = 'MIT'
s.summary = "Paranoia is a re-implementation of acts_as_paranoid for Rails 3, 4, and 5, using much, much, much less code."
s.description = <<-DSC
Expand Down
64 changes: 63 additions & 1 deletion test/paranoia_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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})"
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -774,6 +790,13 @@ def test_validates_uniqueness_still_works_on_non_deleted_records
refute b.valid?
end

def test_updated_at_modification_on_destroy
paranoid_model = ParanoidModelWithTimestamp.create(:parent_model => ParentModel.create, :updated_at => 1.day.ago)
assert paranoid_model.updated_at < 10.minutes.ago
paranoid_model.destroy
assert paranoid_model.updated_at > 10.minutes.ago
end

def test_updated_at_modification_on_restore
parent1 = ParentModel.create
pt1 = ParanoidModelWithTimestamp.create(:parent_model => parent1)
Expand Down Expand Up @@ -1105,6 +1128,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

Expand Down Expand Up @@ -1208,4 +1270,4 @@ class ParanoidBelongsTo < ActiveRecord::Base
acts_as_paranoid
belongs_to :paranoid_has_one
end
end
end