Browse files

Moving tree branches now updates cached full_names correctly.

Because we cache a bit of information on each AbstractTopic node (the
full_name attribute), this must be updated when a subtree is moved.
Since subclasses of AbstractTopic may have similar requirements, there's
also a facility to pass in other callables which are called for each
child node to do similar updates prior to saving the node.
  • Loading branch information...
1 parent 77ff245 commit 4e9b59353505bdb6e7b686b137a74d274b1dc96a Malcolm Tredinnick committed May 17, 2009
Showing with 44 additions and 0 deletions.
  1. +27 −0 acacia/
  2. +17 −0 acacia/
@@ -80,6 +80,33 @@ def save(self, *args, **kwargs):
return super(AbstractTopic, self).save(*args, **kwargs)
+ def move(self, target, pos=None, update_funcs=()):
+ """
+ Moves current node and all descendents to a new position in the tree.
+ If "update_funcs" is given, it's a list of callables that are passed
+ child nodes as the first argument. Subclasses can use this list to add
+ their own hooks for things needing updating after moves.
+ """
+ # A little runtime efficiency is sacrificed here for the sake of code
+ # simplicity. The default treebeard move() method uses custom SQL to
+ # update the paths on all the children. This class has to also update
+ # all the full_name attributes. The latter is done as a separate query
+ # for each child, on the grounds that moving is far less common than
+ # reading for these types of hierarchies.
+ super(AbstractTopic, self).move(target, pos)
+ # After call to super's move(), "self" no longer has valid path
+ # information, so have to refetch the data from the database.
+ for node in self.__class__.objects.get(
+ # FIXME: Since I already know the ancestor names at this point,
+ # being able to pass them into _set_full_name() would be an
+ # improvement.
+ node._set_full_name()
+ for func in update_funcs:
+ func(node)
class Topic(AbstractTopic):
The basic concrete class for a topic node. API details are defined by the
@@ -133,6 +133,23 @@ def test_unicode(self):
expected = [u"x", u"x/y", u"x/y/c"]
self.assertEquals(result, expected)
+ def test_change_parent(self):
+ """
+ Tests that setting a new parent for a node with children causes the
+ children's full names to be updated appropriately.
+ """
+ # Move the "x" in "a/x" to be a child of "a/b" (so it becomes "a/b/x").
+ target = models.Topic.objects.get_by_full_name("a/b")
+ node = models.Topic.objects.get_by_full_name("a/x")
+ node.move(target, "sorted-child")
+ # The "a/x/c" node should have been moved to "a/b/x/c" as part of this.
+ models.Topic.objects.get_by_full_name("a/b/x/c")
+ # It will no longer be accessible under its old name.
+ self.assertRaises(models.Topic.DoesNotExist,
+ models.Topic.objects.get_by_full_name, "a/x/c")
class TreebeardTests(BaseTestSetup, test.TestCase):
Tests for my local modification/overrides to the default treebeard

0 comments on commit 4e9b593

Please sign in to comment.