From bc99de529be8259e71a334fb4295f775233b10e7 Mon Sep 17 00:00:00 2001
From: Robert Clark <robert@bablmedia.com>
Date: Tue, 13 May 2014 12:46:20 -0400
Subject: [PATCH 1/9] page type business rules

---
 wagtail/tests/models.py                       | 12 +++++++
 .../wagtailadmin/pages/add_subpage.html       |  4 +--
 wagtail/wagtailadmin/tests.py                 | 35 ++++++++++++++++++-
 wagtail/wagtailadmin/views/pages.py           |  4 +--
 wagtail/wagtailcore/models.py                 | 10 ++++--
 5 files changed, 56 insertions(+), 9 deletions(-)

diff --git a/wagtail/tests/models.py b/wagtail/tests/models.py
index 9124cb253..6c4b45f31 100644
--- a/wagtail/tests/models.py
+++ b/wagtail/tests/models.py
@@ -196,3 +196,15 @@ EventIndex.content_panels = [
     FieldPanel('title', classname="full title"),
     FieldPanel('intro', classname="full"),
 ]
+
+class StandardIndex(Page):
+    pass
+
+class StandardChild(Page):
+    pass
+
+class BusinessIndex(Page):
+    subpage_types = ['tests.BusinessChild']
+
+class BusinessChild(Page):
+    pass
diff --git a/wagtail/wagtailadmin/templates/wagtailadmin/pages/add_subpage.html b/wagtail/wagtailadmin/templates/wagtailadmin/pages/add_subpage.html
index 757c013b5..1158cc857 100644
--- a/wagtail/wagtailadmin/templates/wagtailadmin/pages/add_subpage.html
+++ b/wagtail/wagtailadmin/templates/wagtailadmin/pages/add_subpage.html
@@ -12,9 +12,9 @@
     <div class="nice-padding">
         <p>{% trans "Choose which type of page you'd like to create." %}</p>
 
-        {% if all_page_types %}
+        {% if page_types %}
             <ul class="listing">
-                {% for content_type in all_page_types %}
+                {% for content_type in page_types %}
                     <li>
                         <div class="row row-flush">
                             <div class="col6">
diff --git a/wagtail/wagtailadmin/tests.py b/wagtail/wagtailadmin/tests.py
index 435f29a9c..59e112b32 100644
--- a/wagtail/wagtailadmin/tests.py
+++ b/wagtail/wagtailadmin/tests.py
@@ -1,6 +1,6 @@
 from django.test import TestCase
 import unittest2 as unittest
-from wagtail.tests.models import SimplePage, EventPage
+from wagtail.tests.models import SimplePage, EventPage, StandardIndex, StandardChild, BusinessIndex, BusinessChild
 from wagtail.tests.utils import login
 from wagtail.wagtailcore.models import Page
 from django.core.urlresolvers import reverse
@@ -326,3 +326,36 @@ class TestEditorHooks(TestCase):
         self.assertEqual(response.status_code, 200)
         self.assertContains(response, '<link rel="stylesheet" href="/path/to/my/custom.css">')
         self.assertContains(response, '<script src="/path/to/my/custom.js"></script>')
+
+
+class TestSubpageBusinessRules(TestCase):
+    def setUp(self):
+        # Find root page
+        self.root_page = Page.objects.get(id=2)
+
+        # Add standard page
+        self.standard_index = StandardIndex()
+        self.standard_index.title = "Standard Index"
+        self.standard_index.slug = "standard-index"
+        self.root_page.add_child(instance=self.standard_index)
+
+        # Add business page
+        self.business_index = BusinessIndex()
+        self.business_index.title = "Business Index"
+        self.business_index.slug = "business-index"
+        self.root_page.add_child(instance=self.business_index)
+
+        # Login
+        login(self.client)
+
+    def test_standard_subpage(self):
+        response = self.client.get(reverse('wagtailadmin_pages_add_subpage', args=(self.standard_index.id, )))
+        self.assertEqual(response.status_code, 200)
+        self.assertContains(response, 'Standard Child')
+        self.assertContains(response, 'Business Child')
+
+    def test_business_subpage(self):
+        response = self.client.get(reverse('wagtailadmin_pages_add_subpage', args=(self.business_index.id, )))
+        self.assertEqual(response.status_code, 200)
+        self.assertNotContains(response, 'Standard Child')
+        self.assertContains(response, 'Business Child')
diff --git a/wagtail/wagtailadmin/views/pages.py b/wagtail/wagtailadmin/views/pages.py
index 6be731c3b..56c1294f9 100644
--- a/wagtail/wagtailadmin/views/pages.py
+++ b/wagtail/wagtailadmin/views/pages.py
@@ -68,13 +68,11 @@ def add_subpage(request, parent_page_id):
     if not parent_page.permissions_for_user(request.user).can_add_subpage():
         raise PermissionDenied
 
-    page_types = sorted([ContentType.objects.get_for_model(model_class) for model_class in parent_page.clean_subpage_types()], key=lambda pagetype: pagetype.name.lower())
-    all_page_types = sorted(get_page_types(), key=lambda pagetype: pagetype.name.lower())
+    page_types = sorted(parent_page.clean_subpage_types(), key=lambda pagetype: pagetype.name.lower())
 
     return render(request, 'wagtailadmin/pages/add_subpage.html', {
         'parent_page': parent_page,
         'page_types': page_types,
-        'all_page_types': all_page_types,
     })
 
 
diff --git a/wagtail/wagtailcore/models.py b/wagtail/wagtailcore/models.py
index 441c74a28..0fc1b3350 100644
--- a/wagtail/wagtailcore/models.py
+++ b/wagtail/wagtailcore/models.py
@@ -484,7 +484,11 @@ class Page(MP_Node, ClusterableModel, Indexed):
             where required
         """
         if cls._clean_subpage_types is None:
-            res = []
+            if len(cls.subpage_types) == 0:
+                res = get_page_types()
+            else:
+                res = []
+            
             for page_type in cls.subpage_types:
                 if isinstance(page_type, basestring):
                     try:
@@ -496,13 +500,13 @@ class Page(MP_Node, ClusterableModel, Indexed):
 
                     model = get_model(app_label, model_name)
                     if model:
-                        res.append(model)
+                        res.append(ContentType.objects.get_for_model(model))
                     else:
                         raise NameError(_("name '{0}' (used in subpage_types list) is not defined.").format(page_type))
 
                 else:
                     # assume it's already a model class
-                    res.append(page_type)
+                    res.append(ContentType.objects.get_for_model(page_type))
 
             cls._clean_subpage_types = res
 

From db6bb920fb367540b9d5ac31209336c342624c54 Mon Sep 17 00:00:00 2001
From: Karl Hobley <karl@torchbox.com>
Date: Mon, 16 Jun 2014 14:57:33 +0100
Subject: [PATCH 2/9] Set treebeard attributes in preview on create view. Fixes
 #320

---
 wagtail/wagtailadmin/views/pages.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/wagtail/wagtailadmin/views/pages.py b/wagtail/wagtailadmin/views/pages.py
index 69e6b7f4f..584613dea 100644
--- a/wagtail/wagtailadmin/views/pages.py
+++ b/wagtail/wagtailadmin/views/pages.py
@@ -364,6 +364,10 @@ def preview_on_create(request, content_type_app_name, content_type_model_name, p
         parent_page = get_object_or_404(Page, id=parent_page_id).specific
         page.set_url_path(parent_page)
 
+        # Set treebeard attributes
+        page.depth = parent_page.depth + 1
+        page.path = Page._get_children_path_interval(parent_page.path)[1]
+
         # This view will generally be invoked as an AJAX request; as such, in the case of
         # an error Django will return a plaintext response. This isn't what we want, since
         # we will be writing the response back to an HTML page regardless of success or

From 4136bb5053165076bdcb7ac5e19ed71ee18fb5cf Mon Sep 17 00:00:00 2001
From: Karl Hobley <karl@torchbox.com>
Date: Mon, 16 Jun 2014 15:15:38 +0100
Subject: [PATCH 3/9] Added tests for db6bb92

---
 wagtail/wagtailadmin/tests/test_pages_views.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/wagtail/wagtailadmin/tests/test_pages_views.py b/wagtail/wagtailadmin/tests/test_pages_views.py
index 12c42f3e4..31301f70f 100644
--- a/wagtail/wagtailadmin/tests/test_pages_views.py
+++ b/wagtail/wagtailadmin/tests/test_pages_views.py
@@ -188,6 +188,11 @@ class TestPageCreation(TestCase, WagtailTestUtils):
         self.assertTemplateUsed(response, 'tests/simple_page.html')
         self.assertContains(response, "New page!")
 
+        # Check that the treebeard attributes were set correctly on the page object
+        self.assertEqual(response.context['self'].depth, self.root_page.depth + 1)
+        self.assertTrue(response.context['self'].path.startswith(self.root_page.path))
+        self.assertEqual(response.context['self'].get_parent(), self.root_page)
+
 
 class TestPageEdit(TestCase, WagtailTestUtils):
     def setUp(self):

From 5987f2843a4f06663391ff379f33f7b73e90f3cd Mon Sep 17 00:00:00 2001
From: Matt Westcott <matt@west.co.tt>
Date: Tue, 17 Jun 2014 14:56:44 +0100
Subject: [PATCH 4/9] remove redundant imports from wagtailcore/models.py

---
 wagtail/wagtailcore/models.py | 2 --
 1 file changed, 2 deletions(-)

diff --git a/wagtail/wagtailcore/models.py b/wagtail/wagtailcore/models.py
index 48875f09a..983661158 100644
--- a/wagtail/wagtailcore/models.py
+++ b/wagtail/wagtailcore/models.py
@@ -1,5 +1,3 @@
-import sys
-import os
 from StringIO import StringIO
 from urlparse import urlparse
 

From a2254148e908b0c68eae7e8f615e97dd39f16e83 Mon Sep 17 00:00:00 2001
From: Matt Westcott <matt@west.co.tt>
Date: Tue, 17 Jun 2014 20:40:55 +0100
Subject: [PATCH 5/9] Remove subpage_types logic from is_navigable and
 get_navigation_menu_items. It's now clear that we will never mandate the use
 of subpage_types, so we can't use its presence or absence as an indicator of
 whether or not a page is appropriate to be included in navigation menus.

---
 wagtail/wagtailcore/models.py | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/wagtail/wagtailcore/models.py b/wagtail/wagtailcore/models.py
index 983661158..cefbe2b88 100644
--- a/wagtail/wagtailcore/models.py
+++ b/wagtail/wagtailcore/models.py
@@ -414,10 +414,10 @@ class Page(MP_Node, ClusterableModel, Indexed):
     def is_navigable(self):
         """
         Return true if it's meaningful to browse subpages of this page -
-        i.e. it currently has subpages, or its page type indicates that sub-pages are supported,
+        i.e. it currently has subpages,
         or it's at the top level (this rule necessary for empty out-of-the-box sites to have working navigation)
         """
-        return (not self.is_leaf()) or (self.content_type_id not in get_leaf_page_content_type_ids()) or self.depth == 2
+        return (not self.is_leaf()) or self.depth == 2
 
     def get_other_siblings(self):
         # get sibling pages excluding self
@@ -654,13 +654,8 @@ class Page(MP_Node, ClusterableModel, Indexed):
 
 def get_navigation_menu_items():
     # Get all pages that appear in the navigation menu: ones which have children,
-    # or are a non-leaf type (indicating that they *could* have children),
     # or are at the top-level (this rule required so that an empty site out-of-the-box has a working menu)
-    navigable_content_type_ids = get_navigable_page_content_type_ids()
-    if navigable_content_type_ids:
-        pages = Page.objects.filter(Q(content_type__in=navigable_content_type_ids)|Q(depth=2)|Q(numchild__gt=0)).order_by('path')
-    else:
-        pages = Page.objects.filter(Q(depth=2)|Q(numchild__gt=0)).order_by('path')
+    pages = Page.objects.filter(Q(depth=2)|Q(numchild__gt=0)).order_by('path')
 
     # Turn this into a tree structure:
     #     tree_node = (page, children)

From 8d3ac405c42cd880b36d88f1140a594510da9824 Mon Sep 17 00:00:00 2001
From: Matt Westcott <matt@west.co.tt>
Date: Tue, 17 Jun 2014 21:10:18 +0100
Subject: [PATCH 6/9] Deprecate get_leaf_page_content_type_ids and
 get_navigable_page_content_type_ids, as they have incorrect semantics for
 page subclasses that don't define subpage_types. Replace them with a naive
 implementation that doesn't require metaclass hacks

---
 wagtail/wagtailcore/models.py | 46 ++++++++++++++++-------------------
 1 file changed, 21 insertions(+), 25 deletions(-)

diff --git a/wagtail/wagtailcore/models.py b/wagtail/wagtailcore/models.py
index cefbe2b88..1100b3d06 100644
--- a/wagtail/wagtailcore/models.py
+++ b/wagtail/wagtailcore/models.py
@@ -1,5 +1,6 @@
 from StringIO import StringIO
 from urlparse import urlparse
+import warnings
 
 from modelcluster.models import ClusterableModel
 
@@ -99,30 +100,29 @@ def get_page_types():
     return _PAGE_CONTENT_TYPES
 
 
-LEAF_PAGE_MODEL_CLASSES = []
-_LEAF_PAGE_CONTENT_TYPE_IDS = []
-
-
 def get_leaf_page_content_type_ids():
-    global _LEAF_PAGE_CONTENT_TYPE_IDS
-    if len(_LEAF_PAGE_CONTENT_TYPE_IDS) != len(LEAF_PAGE_MODEL_CLASSES):
-        _LEAF_PAGE_CONTENT_TYPE_IDS = [
-            ContentType.objects.get_for_model(cls).id for cls in LEAF_PAGE_MODEL_CLASSES
-        ]
-    return _LEAF_PAGE_CONTENT_TYPE_IDS
-
-
-NAVIGABLE_PAGE_MODEL_CLASSES = []
-_NAVIGABLE_PAGE_CONTENT_TYPE_IDS = []
-
+    warnings.warn("""
+        get_leaf_page_content_type_ids is deprecated, as it treats pages without an explicit subpage_types
+        setting as 'leaf' pages. Code that calls get_leaf_page_content_type_ids must be rewritten to avoid
+        this incorrect assumption.
+    """, DeprecationWarning)
+    return [
+        content_type.id
+        for content_type in get_page_types()
+        if not getattr(content_type.model_class(), 'subpage_types', None)
+    ]
 
 def get_navigable_page_content_type_ids():
-    global _NAVIGABLE_PAGE_CONTENT_TYPE_IDS
-    if len(_NAVIGABLE_PAGE_CONTENT_TYPE_IDS) != len(NAVIGABLE_PAGE_MODEL_CLASSES):
-        _NAVIGABLE_PAGE_CONTENT_TYPE_IDS = [
-            ContentType.objects.get_for_model(cls).id for cls in NAVIGABLE_PAGE_MODEL_CLASSES
-        ]
-    return _NAVIGABLE_PAGE_CONTENT_TYPE_IDS
+    warnings.warn("""
+        get_navigable_page_content_type_ids is deprecated, as it treats pages without an explicit subpage_types
+        setting as 'leaf' pages. Code that calls get_navigable_page_content_type_ids must be rewritten to avoid
+        this incorrect assumption.
+    """, DeprecationWarning)
+    return [
+        content_type.id
+        for content_type in get_page_types()
+        if getattr(content_type.model_class(), 'subpage_types', None)
+    ]
 
 
 class PageManager(models.Manager):
@@ -207,10 +207,6 @@ class PageBase(models.base.ModelBase):
         if not cls.is_abstract:
             # register this type in the list of page content types
             PAGE_MODEL_CLASSES.append(cls)
-        if cls.subpage_types:
-            NAVIGABLE_PAGE_MODEL_CLASSES.append(cls)
-        else:
-            LEAF_PAGE_MODEL_CLASSES.append(cls)
 
 
 class Page(MP_Node, ClusterableModel, Indexed):

From 9071a8e0043f9041244a66b1baa388eef3ecafa5 Mon Sep 17 00:00:00 2001
From: Matt Westcott <matt@west.co.tt>
Date: Tue, 17 Jun 2014 21:48:48 +0100
Subject: [PATCH 7/9] Tweak subpage_types logic so that subpage_types=None (or
 undefined) means 'no restriction on subpages', freeing up [] to mean 'no
 subpages allowed'

---
 wagtail/tests/models.py                       |  2 +-
 .../wagtailadmin/tests/test_pages_views.py    | 12 ++++++
 wagtail/wagtailcore/models.py                 | 40 +++++++++----------
 3 files changed, 32 insertions(+), 22 deletions(-)

diff --git a/wagtail/tests/models.py b/wagtail/tests/models.py
index a85cedcfa..f798129dc 100644
--- a/wagtail/tests/models.py
+++ b/wagtail/tests/models.py
@@ -306,4 +306,4 @@ class BusinessIndex(Page):
     subpage_types = ['tests.BusinessChild']
 
 class BusinessChild(Page):
-    pass
+    subpage_types = []
diff --git a/wagtail/wagtailadmin/tests/test_pages_views.py b/wagtail/wagtailadmin/tests/test_pages_views.py
index 9e51c5681..5a7107226 100644
--- a/wagtail/wagtailadmin/tests/test_pages_views.py
+++ b/wagtail/wagtailadmin/tests/test_pages_views.py
@@ -688,6 +688,12 @@ class TestSubpageBusinessRules(TestCase, WagtailTestUtils):
         self.business_index.slug = "business-index"
         self.root_page.add_child(instance=self.business_index)
 
+        # Add business child
+        self.business_child = BusinessChild()
+        self.business_child.title = "Business Child"
+        self.business_child.slug = "business-child"
+        self.business_index.add_child(instance=self.business_child)
+
         # Login
         self.login()
 
@@ -702,3 +708,9 @@ class TestSubpageBusinessRules(TestCase, WagtailTestUtils):
         self.assertEqual(response.status_code, 200)
         self.assertNotContains(response, 'Standard Child')
         self.assertContains(response, 'Business Child')
+
+    def test_business_child_subpage(self):
+        response = self.client.get(reverse('wagtailadmin_pages_add_subpage', args=(self.business_child.id, )))
+        self.assertEqual(response.status_code, 200)
+        self.assertNotContains(response, 'Standard Child')
+        self.assertEqual(0, len(response.context['page_types']))
diff --git a/wagtail/wagtailcore/models.py b/wagtail/wagtailcore/models.py
index 1100b3d06..5c3f9d1c5 100644
--- a/wagtail/wagtailcore/models.py
+++ b/wagtail/wagtailcore/models.py
@@ -253,9 +253,6 @@ class Page(MP_Node, ClusterableModel, Indexed):
     def __unicode__(self):
         return self.title
 
-    # by default pages do not allow any kind of subpages
-    subpage_types = []
-
     is_abstract = True  # don't offer Page in the list of page types a superuser can create
 
     def set_url_path(self, parent):
@@ -478,29 +475,30 @@ class Page(MP_Node, ClusterableModel, Indexed):
             where required
         """
         if cls._clean_subpage_types is None:
-            if len(cls.subpage_types) == 0:
+            subpage_types = getattr(cls, 'subpage_types', None)
+            if subpage_types is None:
+                # if subpage_types is not specified on the Page class, allow all page types as subpages
                 res = get_page_types()
             else:
                 res = []
-            
-            for page_type in cls.subpage_types:
-                if isinstance(page_type, basestring):
-                    try:
-                        app_label, model_name = page_type.split(".")
-                    except ValueError:
-                        # If we can't split, assume a model in current app
-                        app_label = cls._meta.app_label
-                        model_name = page_type
+                for page_type in cls.subpage_types:
+                    if isinstance(page_type, basestring):
+                        try:
+                            app_label, model_name = page_type.split(".")
+                        except ValueError:
+                            # If we can't split, assume a model in current app
+                            app_label = cls._meta.app_label
+                            model_name = page_type
+
+                        model = get_model(app_label, model_name)
+                        if model:
+                            res.append(ContentType.objects.get_for_model(model))
+                        else:
+                            raise NameError(_("name '{0}' (used in subpage_types list) is not defined.").format(page_type))
 
-                    model = get_model(app_label, model_name)
-                    if model:
-                        res.append(ContentType.objects.get_for_model(model))
                     else:
-                        raise NameError(_("name '{0}' (used in subpage_types list) is not defined.").format(page_type))
-
-                else:
-                    # assume it's already a model class
-                    res.append(ContentType.objects.get_for_model(page_type))
+                        # assume it's already a model class
+                        res.append(ContentType.objects.get_for_model(page_type))
 
             cls._clean_subpage_types = res
 

From 8f9298f2b4960cb68b72caf851030479859136c7 Mon Sep 17 00:00:00 2001
From: Matt Westcott <matt@west.co.tt>
Date: Tue, 17 Jun 2014 22:03:11 +0100
Subject: [PATCH 8/9] Changelog / contributors entry for #243

---
 CHANGELOG.txt    | 1 +
 CONTRIBUTORS.rst | 1 +
 2 files changed, 2 insertions(+)

diff --git a/CHANGELOG.txt b/CHANGELOG.txt
index 6c28594a9..ddd41d76e 100644
--- a/CHANGELOG.txt
+++ b/CHANGELOG.txt
@@ -8,6 +8,7 @@ Changelog
  * Snippets are now ordered alphabetically
  * Removed the "More" section from the admin menu
  * Added pagination to page listings in admin
+ * Support for setting a subpage_types property on page models, to define which page types are allowed as subpages
  * Fix: Animated GIFs are now coalesced before resizing
  * Fix: Wand backend clones images before modifying them
  * Fix: Admin breadcrumb now positioned correctly on mobile
diff --git a/CONTRIBUTORS.rst b/CONTRIBUTORS.rst
index d7222307d..a3f9c7221 100644
--- a/CONTRIBUTORS.rst
+++ b/CONTRIBUTORS.rst
@@ -28,6 +28,7 @@ Contributors
 * Ben Margolis
 * Tom Talbot
 * Jeffrey Hearn
+* Robert Clark
 
 Translators
 ===========

From 9c0f4212e339fc0aa1c8b94f05778f65be4dc536 Mon Sep 17 00:00:00 2001
From: Karl Hobley <karl@torchbox.com>
Date: Wed, 18 Jun 2014 15:49:02 +0100
Subject: [PATCH 9/9] Only allow logged in users to access the userbar views

---
 wagtail/wagtailadmin/views/userbar.py | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/wagtail/wagtailadmin/views/userbar.py b/wagtail/wagtailadmin/views/userbar.py
index 6e6ef3492..41776f684 100644
--- a/wagtail/wagtailadmin/views/userbar.py
+++ b/wagtail/wagtailadmin/views/userbar.py
@@ -1,9 +1,13 @@
 from django.shortcuts import render
+from django.contrib.auth.decorators import permission_required
 
 from wagtail.wagtailadmin.userbar import EditPageItem, AddPageItem, ApproveModerationEditPageItem, RejectModerationEditPageItem
 from wagtail.wagtailadmin import hooks
 from wagtail.wagtailcore.models import Page, PageRevision
 
+
+
+@permission_required('wagtailadmin.access_admin')
 def for_frontend(request, page_id):
     items = [
         EditPageItem(Page.objects.get(id=page_id)),
@@ -24,6 +28,8 @@ def for_frontend(request, page_id):
         'items': rendered_items,
     })
 
+
+@permission_required('wagtailadmin.access_admin')
 def for_moderation(request, revision_id):
     items = [
         EditPageItem(PageRevision.objects.get(id=revision_id).page),
@@ -44,4 +50,4 @@ def for_moderation(request, revision_id):
     # Render the edit bird
     return render(request, 'wagtailadmin/userbar/base.html', {
         'items': rendered_items,
-    })
\ No newline at end of file
+    })