5

django-mptt seems determined to drive me out of my mind. I'm trying to do something relatively simple: I'm going to delete a node, and need to do something reasonable with the node's children. Namely, I'd like to move them up one level so they're children of their current parent's parent.

That is, if the tree looks like:

 Root
  |
Grandpa
  |
Father
|    |
C1   C2

I'm going to delete Father, and would like C1 and C2 to be children of Grandpa.

Here's the code I'm using:

class Node(models.Model):
    first_name   = models.CharField(max_length=80, blank=True)
    parent       = models.ForeignKey('self', null=True, blank=True, related_name='children')

    def reparent_children(self, parent):
        print "Reparenting"
        for child in self.get_children():
            print "Working on", child.first_name, "to parent", parent.email
            parent = Node.objects.get(id=parent.id)
            child.move_to(parent, 'last-child')
            child.save()

So I'd call:

father.reparent_children(grandpa)
father.parent = None
father.save()

This works - almost. The children report their parents as Grandpa:

c1.parent == grandpa  # True

Grandpa counts C1 and C2 among its children

c1 in grandpa.children.all()   # True

However, Root disowns these kids.

c1.get_root() == father  # c1's root is father, instead of Root

c1 in root.get_descendants()  # False

How do I get the children to move and their root not get corrupted?

Parand
  • 102,950
  • 48
  • 151
  • 186
  • 1
    are you sure that "father.parent = None" is the right way to delete a node? – mawimawi Jun 14 '10 at 20:11
  • In this case I'm not actually deleting the node - I'm archiving it. I'd like to remove it from the tree. You have a good point though, I'm not actually removing it from the tree here. – Parand Jun 14 '10 at 20:21
  • Looks like setting the parent to None and saving is actually the way to remove a node from a tree (according to the mptt test cases), so that looks right. – Parand Jun 14 '10 at 21:56
  • Another thing: it's helpful to have `Node` inherit from `MPTTModel` and `NodeManager` inherit from `TreeManager` – Sam Bobel Oct 06 '17 at 19:29

2 Answers2

6

The internal lft and rght values will change the first time you save a child (i.e. the final line of your reparent_children method). save() doesn't update instances you may have lying around. I think a safe way to do this would be to refetch them from the database each time, like this:

def reparent_children(self, parent):
    print "Reparenting"
    for child in self.get_children():
        print "Working on", child.first_name, "to parent", parent.email
        parent = Node.objects.get(id=parent.id)
        current_child = Node.objects.get(id = child.id)
        current_child.move_to(parent, 'last-child')
        current_child.save()

I had similar problems a while back, and that approach solved my problem.

Community
  • 1
  • 1
Dominic Rodger
  • 97,747
  • 36
  • 197
  • 212
  • 6
    Dominic, I ended up at this method, and it *seems* to work, although with django-mptt I end up constantly questioning my own sanity. I don't know if I've actually fixed the issue or hidden it away somewhere else. – Parand Jun 15 '10 at 17:26
  • Had similar issue in Django application. Was able to resolve using `parent.refresh_from_db()`. – jdsantiagojr Mar 17 '21 at 16:37
1

This library has really confused me these last few days -- move_to does not really seem to do what I want it to, and my tree keeps getting out of sync. I came up with a solution that I'm more confident in, at the expense of speed and nontraditional-ness.

It revolves around the manager method partial_rebuild here.

def delete_node(self):
    if not self.parent:
        print("Should not delete root node, confusing behavior follows")
        return
    tree_id = self.tree_id
    parent = self.parent

    for child in self.get_children():
        child.parent = parent
        child.save()

    self.delete()
    Node.objects.partial_rebuild(tree_id)

You can replace child.parent = parent with child.move_node(parent) if you'd like

Sam Bobel
  • 1,784
  • 1
  • 15
  • 26