Drop custom get_ancestors() & introduce get_[ancestors|descendants]_and_itself()

Templates need access to both ancestors and descendants of a
category, and ideally both including the category itself or not.
Previously, get_ancestors used to default to including the category
itself, which required slicing in templates. Also, get_ancestors is a
treebeard function and I wouldn't bet that it doesn't interact with
treebeard logic.

Now, the two treebeard functions get_ancestors and get_descendants
return a queryset, and get_[ancestors|descendants]_and_itself return a
list that includes the category itself.

docs/source/releases/v0.8.rst

@@ -330,6 +330,11 @@ Misc
   expected. There's currently no frontend or dashboard support for it, as there
   is no good default behaviour.
 
+* ``Category.get_ancestors`` used to return a list of ancestors and would
+  default to include itself. For consistency with get_descendants and to avoid
+  having to slice the results in templates, it now returns a queryset of the
+  ancestors; use ``Category.get_ancestors_and_itself`` for the old behaviour.
+
 .. _rewritten: https://github.com/tangentlabs/django-oscar/commit/d8b4dbfed17be90846ea4bc47b5f7b39ad944c24
 
 Basket line stockrecords

oscar/apps/catalogue/abstract_models.py

@@ -149,11 +149,21 @@ class AbstractCategory(MP_Node):
         reloaded_self.update_slug()
         reloaded_self.update_children_slugs()
 
-    def get_ancestors(self, include_self=True):
-        ancestors = list(super(AbstractCategory, self).get_ancestors())
-        if include_self:
-            ancestors.append(self)
-        return ancestors
+    def get_ancestors_and_self(self):
+        """
+        Gets ancestors and includes itself. Use treebeard's get_ancestors
+        if you don't want to include the category itself. It's a separate
+        function as it's commonly used in templates.
+        """
+        return list(self.get_ancestors()) + [self]
+
+    def get_descendants_and_self(self):
+        """
+        Gets descendants and includes itself. Use treebeard's get_descendants
+        if you don't want to include the category itself. It's a separate
+        function as it's commonly used in templates.
+        """
+        return list(self.get_descendants()) + [self]
 
     def get_absolute_url(self):
         return reverse('catalogue:category',

oscar/apps/catalogue/views.py

@@ -121,9 +121,7 @@ class FacetedProductCategoryView(ListView):
             request, *args, **kwargs)
 
     def get_queryset(self):
-        # We build a list of this category and all descendents (TODO put this
-        # method onto the category itself as it's needed in a few places).
-        categories = list(self.category.get_descendants()) + [self.category]
+        categories = self.category.get_descendants_and_self()
         # We use 'narrow' API to ensure Solr's 'fq' filtering is used as
         # opposed to filtering using 'q'.
         pattern = ' OR '.join(['"%s"' % c.full_name for c in categories])
@@ -196,7 +194,7 @@ class ProductCategoryView(ListView):
         """
         Return a list of the current category and it's ancestors
         """
-        return list(self.category.get_descendants()) + [self.category]
+        return self.category.get_descendants_and_self()
 
     def get_summary(self):
         """

oscar/apps/dashboard/catalogue/views.py

@@ -403,7 +403,7 @@ class CategoryDetailListView(generic.DetailView):
         ctx = super(CategoryDetailListView, self).get_context_data(*args,
                                                                    **kwargs)
         ctx['child_categories'] = self.object.get_children()
-        ctx['ancestors'] = self.object.get_ancestors()
+        ctx['ancestors'] = self.object.get_ancestors_and_self()
         return ctx
 
 

oscar/templates/oscar/catalogue/browse.html

@@ -24,7 +24,7 @@
         {% if not category %}
             <li class="active">{% trans "All products" %}</li>
         {% else %}
-            {% for category in category.get_ancestors|slice:":-1" %}
+            {% for category in category.get_ancestors %}
                 <li>
                     <a href="{{ category.get_absolute_url }}">{{ category.name }}</a>
                     <span class="divider">/</span>

oscar/templates/oscar/catalogue/category.html

@@ -24,7 +24,7 @@
         {% if not category %}
             <li class="active">{% trans "All products" %}</li>
         {% else %}
-            {% for category in category.get_ancestors|slice:":-1" %}
+            {% for category in category.get_ancestors %}
                 <li>
                     <a href="{{ category.get_absolute_url }}">{{ category.name }}</a>
                     <span class="divider">/</span>

oscar/templates/oscar/catalogue/detail.html

@@ -24,7 +24,7 @@
         <span class="divider">/</span>
     </li>
     {% with category=product.categories.all.0 %}
-        {% for c in category.get_ancestors %}
+        {% for c in category.get_ancestors_and_self %}
         <li>
             <a href="{{ c.get_absolute_url }}">{{ c.name }}</a>
             <span class="divider">/</span>

oscar/templates/oscar/catalogue/reviews/review_detail.html

@@ -12,7 +12,7 @@
             <span class="divider">/</span>
         </li>
         {% with category=product.categories.all.0 %}
-            {% for c in category.get_ancestors %}
+            {% for c in category.get_ancestors_and_self %}
                 <li>
                     <a href="{{ c.get_absolute_url }}">{{ c.name }}</a>
                     <span class="divider">/</span>

oscar/templates/oscar/catalogue/reviews/review_list.html

@@ -14,7 +14,7 @@
             <span class="divider">/</span>
         </li>
         {% with category=product.categories.all.0 %}
-            {% for c in category.get_ancestors %}
+            {% for c in category.get_ancestors_and_self %}
                 <li>
                     <a href="{{ c.get_absolute_url }}">{{ c.name }}</a>
                     <span class="divider">/</span>