Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Recursively destroy descendants #90

Open
brendon opened this Issue · 4 comments

3 participants

@brendon

I noticed that ancestry destroys descendants arbitrarily by calling descendants on the destroyed object. This breaks down in situations that require an orderly deconstruction of the tree and its nodes associations (starting from the bottom and working up_. I'd like to table this instead:

before_destroy :destroy_descendants

def destroy_descendants
  children.each { |child| child.destroy }
end

I thought of children.destroy_all but that didn't have the same effect for some reason :)

Fortunately for me, the above works with the default orphan strategy anyway, but just wanted to raise it for discussion.

@brendon

Just wanted to bump this. Does anyone else think this is a shortcoming that needs to be fixed?

@StefanH
Collaborator

You can implement your own destroy strategy by recursing through the tree and destroying, like: (pseudo-code)

def orderly_destroy node
  node.children.each ( orderly_destroy node )
  node.destroy
end

Would that work for you?

@brendon

That's correct, and I have implemented that for my project though more like this:

  def apply_orphan_strategy
    unless new_record?
      children.all.each { |child| child.destroy }
    end
  end

It's been a while since I wrote that but from memory I'm overriding apply_orphan_strategy so the necessity to destroy self isn't required in the above code as it's already happening.

The main problem I see with ancestry's destroy strategy as it stands is that it discounts the very real possibility that destroy hooks on descendants are going to try save related records in the same branch of the tree. Depending on the order of this happening those related records might already have been deleted which will raise an error of some sort. At least that's how I remember it.

I guess if no one else has experienced this then I'll just close this, but if you think it's a valid case for a patch then I'd gladly submit one.

@miguelgraz

I second that, have that issue with deleting a node with children, it seems quite clear for me that the gem has to destroy the nodes from bottom (leaves) and working up, I too managed to write a quick and dirty monkey patch to fix that and would be great to submit a PR, except that it seems to have quite some PRs waiting without an answer :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.