ActsAsParanoid / acts_as_paranoid

ActiveRecord plugin allowing you to hide and restore records without actually deleting them.
MIT License
1.45k stars 190 forks source link

Add support for recursive delete all associations #326

Open ryan-secondsight opened 3 months ago

ryan-secondsight commented 3 months ago

When having nested associations that use delete_all, the grandchildren of the associations do not get deleted. This can result in some orphaned records.

This PR adds an optional configuration to automatically cascade delete dependent relationships where the top level relationship is a delete_all.

For instance:

class ParanoidParent < ActiveRecord::Base
  acts_as_paranoid(handle_delete_all_associations: true)

  has_many :paranoid_children, dependent: :delete_all
end

class ParanoidChild < ActiveRecord::Base
  acts_as_paranoid(handle_delete_all_associations: true)

  belongs_to :paranoid_parent
  has_many :paranoid_grandchildren, dependent: :delete_all
end

class ParanoidGrandchild < ActiveRecord::Base
  acts_as_paranoid(handle_delete_all_associations: true)

  belongs_to :paranoid_child
end

When calling parent.destroy, the Grandchildren are not deleted. This PR would cascade the delete_all through to the children. I have found this behavior helpful when there are thousands of parent.children, and each of those children have grandchildren.

This PR also handles the recovery of the associations as well.

If this idea gets approved, I can clean up the Rubocop violations and flesh out any tests, if required.

mvz commented 3 months ago

Hi @ryan-secondsight, I don't think standard Rails deletes the grandchildren in this case. At least, if I remove the paranoid configuration from your test file the grandchildren are not deleted. Therefore, it wouldn't make sense for acts_as_paranoid to do so. If you want to cascade the removal of the parent to the grandchildren, you should probably use dependent: :destroy.

ryan-secondsight commented 3 months ago

Fair enough -- vanilla delete_all doesn't fire any callbacks so it's not typically expected.

There are too many records to do a dependent: :destroy (it would take several minutes, whereas this takes about a second), though maybe dependent: :destroy_async would work. That would require another acts_as_paranoid patch.

Thank you for the response.

ryan-secondsight commented 3 months ago

Hello @mvz -- thank you again for your consideration in this PR.

I have been reflecting on your comments about standard Rails delete_all on grandchildren, and I found one way that Rails allows for this behavior natively: by updating the migration to use references and passing the correct configuration to enable cascading deletes, upon deleting the Parent, all of the Children and Grandchildren are also deleted.

I have updated the PR's migrations to include this behavior -- if you are to remove paranoid from the models and delete the parent, you can see that the children and grandchildren are also deleted.

I believe this feature will be helpful in cases where it is not feasible to use dependent: :destroy, such as where cascading deletes have been used to hard-delete hundreds of thousands of records. I am including this functionality as a flag that must be enabled separately, as to not alter the default behavior of acts_as_paranoid.

Given this, would you be open to reconsidering accepting the merge request?

mvz commented 1 week ago

@ryan-secondsight your use case makes a lot of sense. I'll need to think a bit to decide the best way to support this.