Merge pull request #328 from gasman/feature/subpage_types_niceties

Additional UX/workflow niceties when subpage_types is in use

wagtail/tests/models.py

@@ -303,6 +303,9 @@ class StandardChild(Page):
     pass
 
 class BusinessIndex(Page):
+    subpage_types = ['tests.BusinessChild', 'tests.BusinessSubIndex']
+
+class BusinessSubIndex(Page):
     subpage_types = ['tests.BusinessChild']
 
 class BusinessChild(Page):

wagtail/wagtailadmin/tests/test_pages_views.py

@@ -1,5 +1,5 @@
 from django.test import TestCase
-from wagtail.tests.models import SimplePage, EventPage, StandardIndex, StandardChild, BusinessIndex, BusinessChild
+from wagtail.tests.models import SimplePage, EventPage, StandardIndex, StandardChild, BusinessIndex, BusinessChild, BusinessSubIndex
 from wagtail.tests.utils import unittest, WagtailTestUtils
 from wagtail.wagtailcore.models import Page, PageRevision
 from django.core.urlresolvers import reverse
@@ -771,41 +771,87 @@ class TestSubpageBusinessRules(TestCase, WagtailTestUtils):
         # Find root page
         self.root_page = Page.objects.get(id=2)
 
-        # Add standard page
+        # Add standard page (allows subpages of any type)
         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
+        # Add business page (allows BusinessChild and BusinessSubIndex as subpages)
         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)
 
-        # Add business child
+        # Add business child (allows no subpages)
         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)
 
+        # Add business subindex (allows only BusinessChild as subpages)
+        self.business_subindex = BusinessSubIndex()
+        self.business_subindex.title = "Business Subindex"
+        self.business_subindex.slug = "business-subindex"
+        self.business_index.add_child(instance=self.business_subindex)
+
         # Login
         self.login()
 
     def test_standard_subpage(self):
-        response = self.client.get(reverse('wagtailadmin_pages_add_subpage', args=(self.standard_index.id, )))
+        add_subpage_url = reverse('wagtailadmin_pages_add_subpage', args=(self.standard_index.id, ))
+
+        # explorer should contain a link to 'add child page'
+        response = self.client.get(reverse('wagtailadmin_explore', args=(self.standard_index.id, )))
+        self.assertEqual(response.status_code, 200)
+        self.assertContains(response, add_subpage_url)
+
+        # add_subpage should give us the full set of page types to choose
+        response = self.client.get(add_subpage_url)
         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, )))
+        add_subpage_url = reverse('wagtailadmin_pages_add_subpage', args=(self.business_index.id, ))
+
+        # explorer should contain a link to 'add child page'
+        response = self.client.get(reverse('wagtailadmin_explore', args=(self.business_index.id, )))
+        self.assertEqual(response.status_code, 200)
+        self.assertContains(response, add_subpage_url)
+
+        # add_subpage should give us a cut-down set of page types to choose
+        response = self.client.get(add_subpage_url)
         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, )))
+        add_subpage_url = reverse('wagtailadmin_pages_add_subpage', args=(self.business_child.id, ))
+
+        # explorer should not contain a link to 'add child page', as this page doesn't accept subpages
+        response = self.client.get(reverse('wagtailadmin_explore', args=(self.business_child.id, )))
         self.assertEqual(response.status_code, 200)
-        self.assertNotContains(response, 'Standard Child')
-        self.assertEqual(0, len(response.context['page_types']))
+        self.assertNotContains(response, add_subpage_url)
+
+        # this also means that fetching add_subpage is blocked at the permission-check level
+        response = self.client.get(reverse('wagtailadmin_pages_add_subpage', args=(self.business_child.id, )))
+        self.assertEqual(response.status_code, 403)
+
+    def test_cannot_add_invalid_subpage_type(self):
+        # cannot add SimplePage as a child of BusinessIndex, as SimplePage is not present in subpage_types
+        response = self.client.get(reverse('wagtailadmin_pages_create', args=('tests', 'simplepage', self.business_index.id)))
+        self.assertEqual(response.status_code, 403)
+
+        # likewise for BusinessChild which has an empty subpage_types list
+        response = self.client.get(reverse('wagtailadmin_pages_create', args=('tests', 'simplepage', self.business_child.id)))
+        self.assertEqual(response.status_code, 403)
+
+        # but we can add a BusinessChild to BusinessIndex
+        response = self.client.get(reverse('wagtailadmin_pages_create', args=('tests', 'businesschild', self.business_index.id)))
+        self.assertEqual(response.status_code, 200)
+
+    def test_not_prompted_for_page_type_when_only_one_choice(self):
+        response = self.client.get(reverse('wagtailadmin_pages_add_subpage', args=(self.business_subindex.id, )))
+        # BusinessChild is the only valid subpage type of BusinessSubIndex, so redirect straight there
+        self.assertRedirects(response, reverse('wagtailadmin_pages_create', args=('tests', 'businesschild', self.business_subindex.id)))

wagtail/wagtailadmin/views/pages.py

@@ -12,7 +12,7 @@ from wagtail.wagtailadmin.edit_handlers import TabbedInterface, ObjectList
 from wagtail.wagtailadmin.forms import SearchForm
 from wagtail.wagtailadmin import tasks, hooks
 
-from wagtail.wagtailcore.models import Page, PageRevision, get_page_types
+from wagtail.wagtailcore.models import Page, PageRevision
 
 
 @permission_required('wagtailadmin.access_admin')
@@ -57,6 +57,12 @@ def add_subpage(request, parent_page_id):
 
     page_types = sorted(parent_page.clean_subpage_types(), key=lambda pagetype: pagetype.name.lower())
 
+    if len(page_types) == 1:
+        # Only one page type is available - redirect straight to the create form rather than
+        # making the user choose
+        content_type = page_types[0]
+        return redirect('wagtailadmin_pages_create', content_type.app_label, content_type.model, parent_page.id)
+
     return render(request, 'wagtailadmin/pages/add_subpage.html', {
         'parent_page': parent_page,
         'page_types': page_types,
@@ -109,15 +115,11 @@ def create(request, content_type_app_name, content_type_model_name, parent_page_
     except ContentType.DoesNotExist:
         raise Http404
 
-    page_class = content_type.model_class()
-
     # page must be in the list of allowed subpage types for this parent ID
-    # == Restriction temporarily relaxed so that as superusers we can add index pages and things -
-    # == TODO: reinstate this for regular editors when we have distinct user types
-    #
-    # if page_class not in parent_page.clean_subpage_types():
-    #     messages.error(request, "Sorry, you do not have access to create a page of type '%s' here." % content_type.name)
-    #     return redirect('wagtailadmin_pages_select_type')
+    if content_type not in parent_page.clean_subpage_types():
+        raise PermissionDenied
+
+    page_class = content_type.model_class()
 
     page = page_class(owner=request.user)
     edit_handler_class = get_page_edit_handler(page_class)

wagtail/wagtailcore/models.py

@@ -843,6 +843,8 @@ class PagePermissionTester(object):
     def can_add_subpage(self):
         if not self.user.is_active:
             return False
+        if not self.page.specific_class.clean_subpage_types():  # this page model has an empty subpage_types list, so no subpages are allowed
+            return False
         return self.user.is_superuser or ('add' in self.permissions)
 
     def can_edit(self):
@@ -897,10 +899,13 @@ class PagePermissionTester(object):
         """
         Niggly special case for creating and publishing a page in one go.
         Differs from can_publish in that we want to be able to publish subpages of root, but not
-        to be able to publish root itself
+        to be able to publish root itself. (Also, can_publish_subpage returns false if the page
+        does not allow subpages at all.)
         """
         if not self.user.is_active:
             return False
+        if not self.page.specific_class.clean_subpage_types():  # this page model has an empty subpage_types list, so no subpages are allowed
+            return False
 
         return self.user.is_superuser or ('publish' in self.permissions)