Permalink
Browse files

Finish port to use django-mptt, including lots of extra tests.

  • Loading branch information...
1 parent 8ae6abc commit ada61896d3b6a2918a329f199cb8ad1de2dd9db4 @malcolmt committed Jun 6, 2010
Showing with 241 additions and 212 deletions.
  1. +2 −105 acacia/admin.py
  2. +63 −0 acacia/managers.py
  3. +48 −92 acacia/models.py
  4. +4 −0 acacia/signals.py
  5. +124 −15 acacia/tests.py
View
@@ -1,109 +1,6 @@
-from django import forms
from django.contrib import admin
-import models
+from acacia import models
-class TopicForm(forms.ModelForm):
- """
- Instead of displaying the standard model fields in the admin, display the
- name field and a synthesised "parent" field. The TopicAdmin class then
- converts uses these fields to make the appropriate modifications to the
- underlying Topic instances.
- """
- parent = forms.ModelChoiceField(models.Topic.objects.all(), required=False,
- empty_label="<no parent>")
-
- class Meta:
- model = models.Topic
- fields = ("name", "parent")
-
- def __init__(self, *args, **kwargs):
- """
- In addition to normal ModelForm initialisation, sets the initial value
- of the "parent" form field to be correct for any passed in instance and
- removes any descendants of the initial instance as possible parent
- choices.
- """
- super(TopicForm, self).__init__(*args, **kwargs)
- if self.instance.pk is not None:
- parent = self.instance.get_parent()
- parent_field = self.fields["parent"]
- parent_field.initial = parent and parent.pk or None
- parent_field.queryset = models.Topic.objects.exclude(
- path__startswith=self.instance.path)
-
- def save_m2m(self):
- """
- The admin expects to have indirectly called
- TopicForm.save(commit=False) and will then call this method (it's
- created as a result of commit=False). So it has to be stubbed out, but
- we don't need to do anything here.
- """
- pass
-
-
-class TopicAdmin(admin.ModelAdmin):
- form = TopicForm
-
- def save_form(self, request, form, change):
- """
- Returns the updated and unsaved instance of the model being changed or
- added. This instance isn't fully complete in our case, since attributes
- are updated when it is saved (as part of the tree structure).
- """
- name = form.cleaned_data["name"]
- parent = form.cleaned_data.get("parent")
- if change:
- # Changing an existing instance.
- instance = form.instance
- instance.name = name
- else:
- # Adding a new instance.
- instance = models.Topic(name=name)
-
- # The full name is set to the *new* full_name, so that it appears
- # correctly in the history logs, for example.
- if parent:
- instance.full_name = "%s%s%s" % (parent.full_name,
- models.Topic.separator, name)
- else:
- instance.full_name = name
- return instance
-
- def save_model(self, request, instance, form, change):
- """
- Add the new instance node to the tree at the appropriate point.
- """
- parent = form.cleaned_data.get("parent")
- if change:
- # Changing an existing topic.
- moved = False
- if instance.depth != 1:
- if not parent:
- # Move a non-root node to the root.
- root = models.Topic.get_first_root_node()
- instance.move(root, "sorted-sibling")
- moved = True
- elif parent.id != instance.get_parent().id:
- # Move a non-root node to a different, non-root, parent.
- instance.move(parent, "sorted-child")
- moved = True
- else:
- if parent:
- # Move a root node to a lower point in the tree.
- instance.move(parent, "sorted-child")
- moved = True
- if moved:
- # Refresh from the database, as any moves will have changed the
- # depth and path values.
- instance = models.Topic.objects.get(pk=instance.pk)
- instance.save()
- else:
- # Adding a new instance.
- if not parent:
- models.Topic.add_root(instance)
- else:
- parent.add_child(instance)
-
-admin.site.register(models.Topic, TopicAdmin)
+admin.site.register(models.Topic)
View
@@ -0,0 +1,63 @@
+"""
+Custom manager for working with topic hierarchies.
+"""
+
+from django.db import models
+
+class TopicManager(models.Manager):
+ """
+ Some useful methods that operate on topics as a whole. Mostly for locating
+ information about a Topic based on its full name.
+ """
+ def get_by_full_name(self, full_name):
+ """
+ Returns the topic with the given full name.
+
+ Raises Topic.DoesNotExist if there is no tag with 'full_name'.
+ """
+ # Ensure foo//bar is the same as foo/bar. Nice to have.
+ sep = self.model.separator
+ pieces = [o for o in full_name.split(sep) if o]
+ for candidate in self.filter(level=len(pieces)-1, name=pieces[-1]):
+ parents = candidate.get_ancestors().values_list("name", flat=True)
+ if list(parents) == pieces[:-1]:
+ return candidate
+ raise self.model.DoesNotExist
+
+ def get_subtree(self, full_name):
+ """
+ Returns a list containing the tag with the given full name and all tags
+ with this tag as an ancestor. The first item in the list will be the
+ tag with the passed in long name.
+
+ Raises Topic.DoesNotExist if there is no tag with 'long_name'.
+ """
+ return self.get_by_full_name(full_name).get_descendants(True)
+
+ def get_or_create_by_full_name(self, full_name):
+ """
+ Retrieves a topic with the given full_name. If the topic doesn't exist,
+ it is created (along with all the necessary parent topics). This is
+ analogous to Django's get_or_create() manager method, with additional
+ logic to handle long names.
+
+ Returns a pair: the topic object and a boolean flag indicating whether
+ or not a new object was created.
+ """
+ try:
+ node = self.get_by_full_name(full_name)
+ return node, False
+ except self.model.DoesNotExist:
+ pass
+
+ # TODO: Feels like I should be able to do this with fewer queries.
+ pieces = full_name.rsplit(self.model.separator, 1)
+ if len(pieces) == 1:
+ return self.model.create(name=pieces[0]), True
+ parent, created = self.get_or_create_by_full_name(pieces[0])
+ if not pieces[1]:
+ # full_name ended with a trailing separator (e.g. /foo/bar/).
+ return parent, created
+ node = self.create(name=pieces[-1], parent=parent)
+ return node, True
+
View
@@ -2,70 +2,14 @@
A hierarchical topics scheme.
The important piece here is the hierarchy. This is not tagging -- which tends
-to be viewed as something flat by the kids these days.
+to be viewed as something flat by kids these days. We have no time for such a
+shallow view on information organisation here.
"""
-from django.db import models
-
import mptt
+from django.db import models
-
-class TopicManager(models.Manager):
- """
- Some useful methods that operate on topics as a whole. Mostly for locating
- information about a Topic based on its full name.
- """
- def get_by_full_name(self, full_name):
- """
- Returns the topic with the given full name.
-
- Raises Topic.DoesNotExist if there is no tag with 'full_name'.
- """
- # Ensure foo//bar is the same as foo/bar. Nice to have.
- sep = self.model.separator
- pieces = [o for o in full_name.split(sep) if o]
- for candidate in self.filter(level=len(pieces)-1, name=pieces[-1]):
- parents = candidate.get_ancestors().values_list("name", flat=True)
- if list(parents) == pieces[:-1]:
- return candidate
- raise self.model.DoesNotExist
-
- def get_subtree(self, full_name):
- """
- Returns a list containing the tag with the given full name and all tags
- with this tag as an ancestor. The first item in the list will be the
- tag with the passed in long name.
-
- Raises Topic.DoesNotExist if there is no tag with 'long_name'.
- """
- return self.get_by_full_name(full_name).get_descendants(True)
-
- def get_or_create_by_full_name(self, full_name):
- """
- Retrieves a topic with the given full_name. If the topic doesn't exist,
- it is created (along with all the necessary parent topics). This is
- analogous to Django's get_or_create() manager method, with additional
- logic to handle long names.
-
- Returns a pair: the topic object and a boolean flag indicating whether
- or not a new object was created.
- """
- try:
- node = self.get_by_full_name(full_name)
- return node, False
- except self.model.DoesNotExist:
- pass
-
- # TODO: Feels like I should be able to do this with less queries.
- pieces = full_name.rsplit(self.model.separator, 1)
- if len(pieces) == 1:
- return self.model.create(name=pieces[0]), True
- parent, created = self.get_or_create_by_full_name(pieces[0])
- if not pieces[1]:
- # full_name ended with a trailing separator (e.g. /foo/bar/).
- return parent, created
- node = self.create(name=pieces[-1], parent=parent)
- return node, True
+from acacia import managers, signals
class AbstractTopic(models.Model):
@@ -80,56 +24,68 @@ class AbstractTopic(models.Model):
name = models.CharField(max_length=50)
parent = models.ForeignKey("self", null=True, blank=True,
related_name="children")
- # The level of this node in the tree it belongs to (used by mptt).
- level = models.PositiveIntegerField()
- objects = TopicManager()
+ objects = managers.TopicManager()
separator = u"/"
class Meta:
+ # pylint: disable-msg=W0232
abstract = True
+ unique_together = [("name", "parent")]
def __unicode__(self):
return self.full_name()
def full_name(self):
- if not hasattr(self, "_full_name_cache"):
- if self.level:
+ # pylint: disable-msg=W0201,E0203
+ if (not hasattr(self, "_full_name_cache") or
+ self.parent_id != self._cached_parent):
+ if self.parent is not None:
parents = self.separator.join(
self.get_ancestors().values_list("name", flat=True))
self._full_name_cache = u"%s%s%s" % (parents, self.separator,
self.name)
else:
self._full_name_cache = self.name
+ self._cached_parent = self.parent_id
return self._full_name_cache
- # FIXME: Implement this in a reasonable fashion for mptt-based trees.
- ##def move_to(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(pk=self.pk).get_tree():
- ## # 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)
- ## node.save()
+ def merge_to(self, parent):
+ """
+ A variant on mptt's move_to() method that merges any overlapping
+ portions of the move. If any nodes are to be merged, a pre_merge signal
+ is emitted before the move, with a list (old_id, new_id) pairs for
+ nodes that will be merged rather than moved. (For efficiency reasons,
+ no signal is emitted if no nodes are to be merged.)
+ """
+ manager = self.__class__.objects
+ try:
+ merge_node = manager.get(parent=parent, name=self.name)
+ except self.DoesNotExist:
+ self.move_to(parent)
+ return
+ squash = [(self.id, merge_node.id)]
+ examine = [(self, merge_node)]
+ to_move = []
+ while examine:
+ node, merge_node = examine.pop()
+ children = node.get_children()
+ child_names = [obj.name for obj in children]
+ conflicts = {}
+ for obj in manager.filter(parent=merge_node, name__in=child_names):
+ conflicts[obj.name] = obj
+ for child in children:
+ if child.name in conflicts:
+ squash.append((child.id, conflicts[child.name].id))
+ examine.append((child, conflicts[child.name]))
+ else:
+ to_move.append((child, merge_node.id))
+ if squash:
+ signals.pre_merge.send(sender=self, merge_pairs=squash)
+ for child, target_parent_id in to_move:
+ target_parent = manager.get(id=target_parent_id)
+ child.move_to(target_parent)
+ self.delete()
class Topic(AbstractTopic):
"""
View
@@ -0,0 +1,4 @@
+from django import dispatch
+
+pre_merge = dispatch.Signal(providing_args=["merge_pairs"])
+
Oops, something went wrong.

0 comments on commit ada6189

Please sign in to comment.