Add is_public to Category and hide non-public categories from frontend views.

docs/source/releases/v2.1.rst

@@ -31,6 +31,16 @@ What's new in Oscar 2.1?
   Django doesn't generate migrations if a project modifies the ``OSCAR_IMAGE_FOLDER``
   to specify a custom directory structure for uploaded images.
 
+- ``catalogue.Category`` now has an ``is_public`` boolean field that serves a
+  similar purpose to ``catalogue.Product.is_public`` - i.e., hides them from
+  public views. The ``Category`` model also now has a custom manager
+  that provides a ``browsable()`` queryset method that excludes non-public
+  categories. This changes requires a database migration.
+
+  Category hierarchy implies that the children of any non-public category are
+  also non-public. This is enforced through an ``ancestors_are_public`` field
+  on the ``Category`` model.
+
 Communications app
 ------------------
 
@@ -125,6 +135,8 @@ Minor changes
   in Django's form, but it was possible to send a password reset email to
   unintended recipients because of unicode character collision.
 
+- ``catalogue.Product.is_public`` is now an indexed field. This change requires
+  a database migration.
 
 Dependency changes
 ~~~~~~~~~~~~~~~~~~

src/oscar/apps/catalogue/abstract_models.py

@@ -11,7 +11,9 @@ from django.core.exceptions import ImproperlyConfigured, ValidationError
 from django.core.files.base import File
 from django.core.validators import RegexValidator
 from django.db import models
-from django.db.models import Count, Sum
+from django.db.models import Count, Exists, OuterRef, Sum
+from django.db.models.fields import Field
+from django.db.models.lookups import StartsWith
 from django.urls import reverse
 from django.utils.functional import cached_property
 from django.utils.html import strip_tags
@@ -21,7 +23,7 @@ from django.utils.translation import gettext_lazy as _
 from django.utils.translation import pgettext_lazy
 from treebeard.mp_tree import MP_Node
 
-from oscar.core.loading import get_class, get_model
+from oscar.core.loading import get_class, get_classes, get_model
 from oscar.core.utils import slugify
 from oscar.core.validators import non_python_keyword
 from oscar.models.fields import AutoSlugField, NullCharField
@@ -29,11 +31,44 @@ from oscar.models.fields.slugfield import SlugField
 from oscar.utils.models import get_image_upload_path
 
 BrowsableProductManager = get_class('catalogue.managers', 'BrowsableProductManager')
-ProductQuerySet = get_class('catalogue.managers', 'ProductQuerySet')
+CategoryQuerySet, ProductQuerySet = get_classes(
+    'catalogue.managers', ['CategoryQuerySet', 'ProductQuerySet'])
 ProductAttributesContainer = get_class(
     'catalogue.product_attributes', 'ProductAttributesContainer')
 
 
+class ReverseStartsWith(StartsWith):
+    """
+    Adds a new lookup method to the django query language, that allows the
+    following syntax::
+
+        henk__rstartswith="koe"
+
+    The regular version of startswith::
+
+        henk__startswith="koe"
+
+     Would be about the same as the python statement::
+
+        henk.startswith("koe")
+
+    ReverseStartsWith will flip the right and left hand side of the expression,
+    effectively making this the same query as::
+
+    "koe".startswith(henk)
+    """
+    def process_rhs(self, compiler, connection):
+        return super().process_lhs(compiler, connection)
+
+    def process_lhs(self, compiler, connection, lhs=None):
+        if lhs is not None:
+            raise Exception("Flipped process_lhs does not accept lhs argument")
+        return super().process_rhs(compiler, connection)
+
+
+Field.register_lookup(ReverseStartsWith, "rstartswith")
+
+
 class AbstractProductClass(models.Model):
     """
     Used for defining options and attributes for a subset of products.
@@ -97,9 +132,23 @@ class AbstractCategory(MP_Node):
                               null=True, max_length=255)
     slug = SlugField(_('Slug'), max_length=255, db_index=True)
 
+    is_public = models.BooleanField(
+        _('Is public'),
+        default=True,
+        db_index=True,
+        help_text=_("Show this category in search results and catalogue listings."))
+
+    ancestors_are_public = models.BooleanField(
+        _('Ancestor categories are public'),
+        default=True,
+        db_index=True,
+        help_text=_("The ancestors of this category are public"))
+
     _slug_separator = '/'
     _full_name_separator = ' > '
 
+    objects = CategoryQuerySet.as_manager()
+
     def __str__(self):
         return self.full_name
 
@@ -158,9 +207,34 @@ class AbstractCategory(MP_Node):
         """
         if not self.slug:
             self.slug = self.generate_slug()
-
         super().save(*args, **kwargs)
 
+    def set_ancestors_are_public(self):
+        # Update ancestors_are_public for the sub tree.
+        # note: This doesn't trigger a new save for each instance, rather
+        # just a SQL update.
+        included_in_non_public_subtree = self.__class__.objects.filter(
+            is_public=False, path__rstartswith=OuterRef("path"), depth__lt=OuterRef("depth")
+        )
+        self.get_descendants_and_self().update(
+            ancestors_are_public=Exists(
+                included_in_non_public_subtree.values("id"), negated=True))
+
+        # Correctly populate ancestors_are_public
+        self.refresh_from_db()
+
+    @classmethod
+    def fix_tree(cls, destructive=False):
+        super().fix_tree(destructive)
+        for node in cls.get_root_nodes():
+            # ancestors_are_public *must* be True for root nodes, or all trees
+            # will become non-public
+            if not node.ancestors_are_public:
+                node.ancestors_are_public = True
+                node.save()
+            else:
+                node.set_ancestors_are_public()
+
     def get_ancestors_and_self(self):
         """
         Gets ancestors and includes itself. Use treebeard's get_ancestors
@@ -268,6 +342,7 @@ class AbstractProduct(models.Model):
     is_public = models.BooleanField(
         _('Is public'),
         default=True,
+        db_index=True,
         help_text=_("Show this product in search results and catalogue listings."))
 
     upc = NullCharField(

src/oscar/apps/catalogue/managers.py

@@ -3,6 +3,7 @@ from collections import defaultdict
 from django.db import models
 from django.db.models import Exists, OuterRef
 from django.db.models.constants import LOOKUP_SEP
+from treebeard.mp_tree import MP_NodeQuerySet
 
 from oscar.core.decorators import deprecated
 from oscar.core.loading import get_model
@@ -144,3 +145,12 @@ class BrowsableProductManager(ProductManager):
     @deprecated
     def get_queryset(self):
         return super().get_queryset().browsable()
+
+
+class CategoryQuerySet(MP_NodeQuerySet):
+
+    def browsable(self):
+        """
+        Excludes non-public categories
+        """
+        return self.filter(is_public=True, ancestors_are_public=True)

src/oscar/apps/catalogue/migrations/0018_auto_20191220_0920.py

@@ -0,0 +1,28 @@
+# Generated by Django 2.2.9 on 2019-12-20 09:20
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('catalogue', '0017_auto_20190816_0938'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='category',
+            name='ancestors_are_public',
+            field=models.BooleanField(db_index=True, default=True, help_text='The ancestors of this category are public', verbose_name='Ancestor categories are public'),
+        ),
+        migrations.AddField(
+            model_name='category',
+            name='is_public',
+            field=models.BooleanField(db_index=True, default=True, help_text='Show this category in search results and catalogue listings.', verbose_name='Is public'),
+        ),
+        migrations.AlterField(
+            model_name='product',
+            name='is_public',
+            field=models.BooleanField(db_index=True, default=True, help_text='Show this product in search results and catalogue listings.', verbose_name='Is public'),
+        ),
+    ]

src/oscar/apps/catalogue/receivers.py

@@ -1,16 +1,18 @@
 # -*- coding: utf-8 -*-
-
 from django.conf import settings
+from django.db.models.signals import post_delete, post_save
+from django.dispatch import receiver
+
+from oscar.core.loading import get_model
+
+Category = get_model("catalogue", "Category")
+
 
 if settings.OSCAR_DELETE_IMAGE_FILES:
     from django.db import models
-    from django.db.models.signals import post_delete
-
-    from oscar.core.loading import get_model
     from oscar.core.thumbnails import get_thumbnailer
 
     ProductImage = get_model('catalogue', 'ProductImage')
-    Category = get_model('catalogue', 'Category')
 
     def delete_image_files(sender, instance, **kwargs):
         """
@@ -28,3 +30,8 @@ if settings.OSCAR_DELETE_IMAGE_FILES:
     models_with_images = [ProductImage, Category]
     for sender in models_with_images:
         post_delete.connect(delete_image_files, sender=sender)
+
+
+@receiver(post_save, sender=Category, dispatch_uid='set_ancestors_are_public')
+def post_save_set_ancestors_are_public(sender, instance, **kwargs):
+    instance.set_ancestors_are_public()

src/oscar/apps/catalogue/views.py

@@ -158,6 +158,11 @@ class ProductCategoryView(TemplateView):
     def get(self, request, *args, **kwargs):
         # Fetch the category; return 404 or redirect as needed
         self.category = self.get_category()
+
+        # Allow staff members so they can test layout etc.
+        if not self.is_viewable(self.category, request):
+            raise Http404()
+
         potential_redirect = self.redirect_if_necessary(
             request.path, self.category)
         if potential_redirect is not None:
@@ -172,6 +177,9 @@ class ProductCategoryView(TemplateView):
 
         return super().get(request, *args, **kwargs)
 
+    def is_viewable(self, category, request):
+        return category.is_public or request.user.is_staff
+
     def get_category(self):
         return get_object_or_404(Category, pk=self.kwargs['pk'])
 

src/oscar/apps/dashboard/catalogue/forms.py

@@ -26,7 +26,8 @@ ProductSelect = get_class('dashboard.catalogue.widgets', 'ProductSelect')
 
 CategoryForm = movenodeform_factory(
     Category,
-    fields=['name', 'description', 'image'])
+    fields=['name', 'description', 'image', 'is_public'],
+    exclude=['ancestors_are_public'])
 
 
 class ProductClassSelectForm(forms.Form):

src/oscar/apps/offer/abstract_models.py

@@ -7,8 +7,6 @@ from django.conf import settings
 from django.core import exceptions
 from django.db import models
 from django.db.models import Exists, OuterRef
-from django.db.models.fields import Field
-from django.db.models.lookups import StartsWith
 from django.db.models.query import Q
 from django.template.defaultfilters import date as date_filter
 from django.urls import reverse
@@ -29,41 +27,6 @@ ZERO_DISCOUNT = get_class('offer.results', 'ZERO_DISCOUNT')
 load_proxy, unit_price = get_classes('offer.utils', ['load_proxy', 'unit_price'])
 
 
-class ReverseStartsWith(StartsWith):
-    """
-    Adds a new lookup method to the django query language, that allows the
-    following syntax::
-
-        henk_rstartswith="koe"
-
-    The regular version of startswith::
-
-        henk_startswith="koe"
-
-     Would be about the same as the python statement::
-
-        henk.startswith("koe")
-
-    ReverseStartsWith will flip the right and left hand side of the expression,
-    effectively making this the same query as::
-
-    "koe".startswith(henk)
-
-    This is used by the range query below, where we need to flip select children
-    based on that their depth starts with the depth string of the parent.
-    """
-    def process_rhs(self, compiler, connection):
-        return super().process_lhs(compiler, connection)
-
-    def process_lhs(self, compiler, connection, lhs=None):
-        if lhs is not None:
-            raise Exception("Flipped process_lhs does not accept lhs argument")
-        return super().process_rhs(compiler, connection)
-
-
-Field.register_lookup(ReverseStartsWith, "rstartswith")
-
-
 class BaseOfferMixin(models.Model):
     class Meta:
         abstract = True

src/oscar/templatetags/category_tags.py

@@ -94,6 +94,8 @@ def get_annotated_list(depth=None, parent=None):
     if max_depth is not None:
         categories = categories.filter(depth__lte=max_depth)
 
+    categories = categories.browsable()
+
     info = CheapCategoryInfo(parent, url="")
 
     for node in categories:

tests/_site/apps/catalogue/migrations/0018_auto_20191220_0920.py

@@ -0,0 +1,28 @@
+# Generated by Django 2.2.9 on 2019-12-20 09:20
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('catalogue', '0017_auto_20190816_0938'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='category',
+            name='ancestors_are_public',
+            field=models.BooleanField(db_index=True, default=True, help_text='The ancestors of this category are public', verbose_name='Ancestor categories are public'),
+        ),
+        migrations.AddField(
+            model_name='category',
+            name='is_public',
+            field=models.BooleanField(db_index=True, default=True, help_text='Show this category in search results and catalogue listings.', verbose_name='Is public'),
+        ),
+        migrations.AlterField(
+            model_name='product',
+            name='is_public',
+            field=models.BooleanField(db_index=True, default=True, help_text='Show this product in search results and catalogue listings.', verbose_name='Is public'),
+        ),
+    ]

tests/functional/catalogue/test_catalogue.py

@@ -122,3 +122,42 @@ class TestProductCategoryView(WebTestCase):
         response = self.app.get(wrong_url)
         self.assertEqual(http_client.MOVED_PERMANENTLY, response.status_code)
         self.assertTrue(self.category.get_absolute_url() in response.location)
+
+    def test_is_public_off(self):
+        category = Category.add_root(name="Foobars", is_public=False)
+        response = self.app.get(category.get_absolute_url(), expect_errors=True)
+        self.assertEqual(http_client.NOT_FOUND, response.status_code)
+        return category
+
+    def test_is_public_on(self):
+        category = Category.add_root(name="Barfoos", is_public=True)
+        response = self.app.get(category.get_absolute_url())
+        self.assertEqual(http_client.OK, response.status_code)
+        return category
+
+    def test_browsable_contains_public_child(self):
+        "If the parent is public the child should be in browsable if it is public as well"
+        cat = self.test_is_public_on()
+        child = cat.add_child(name="Koe", is_public=True)
+        self.assertTrue(child in Category.objects.all().browsable())
+
+        child.is_public = False
+        child.save()
+        self.assertTrue(child not in Category.objects.all().browsable())
+
+    def test_browsable_hides_public_child(self):
+        "If the parent is not public the child should not be in browsable evn if it is public"
+        cat = self.test_is_public_off()
+        child = cat.add_child(name="Koe", is_public=True)
+        self.assertTrue(child not in Category.objects.all().browsable())
+
+    def test_is_public_child(self):
+        cat = self.test_is_public_off()
+        child = cat.add_child(name="Koe", is_public=True)
+        response = self.app.get(child.get_absolute_url())
+        self.assertEqual(http_client.OK, response.status_code)
+
+        child.is_public = False
+        child.save()
+        response = self.app.get(child.get_absolute_url(), expect_errors=True)
+        self.assertEqual(http_client.NOT_FOUND, response.status_code)