From a713895440657907fd3e39cfafda3a7ad8daf4df Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Tue, 17 Jun 2014 22:14:23 +0100 Subject: [PATCH 1/4] get_page_types no longer required in wagtailadmin.views.pages --- wagtail/wagtailadmin/views/pages.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wagtail/wagtailadmin/views/pages.py b/wagtail/wagtailadmin/views/pages.py index 916c373ebb..c60a1800e4 100644 --- a/wagtail/wagtailadmin/views/pages.py +++ b/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') From a4b60715b99135ffe7bc452d312f043125a1d685 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Tue, 17 Jun 2014 22:27:11 +0100 Subject: [PATCH 2/4] Check that the content type passed to wagtailadmin.pages.create is valid according to subpage_types --- wagtail/wagtailadmin/tests/test_pages_views.py | 13 +++++++++++++ wagtail/wagtailadmin/views/pages.py | 12 ++++-------- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/wagtail/wagtailadmin/tests/test_pages_views.py b/wagtail/wagtailadmin/tests/test_pages_views.py index 09e6b6f5cb..986bb260cf 100644 --- a/wagtail/wagtailadmin/tests/test_pages_views.py +++ b/wagtail/wagtailadmin/tests/test_pages_views.py @@ -719,3 +719,16 @@ class TestSubpageBusinessRules(TestCase, WagtailTestUtils): self.assertEqual(response.status_code, 200) self.assertNotContains(response, 'Standard Child') self.assertEqual(0, len(response.context['page_types'])) + + 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) diff --git a/wagtail/wagtailadmin/views/pages.py b/wagtail/wagtailadmin/views/pages.py index c60a1800e4..559adb9224 100644 --- a/wagtail/wagtailadmin/views/pages.py +++ b/wagtail/wagtailadmin/views/pages.py @@ -111,15 +111,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) From 6bfe82f5e59ca375725b2769dc5b2564d3d19e2c Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Tue, 17 Jun 2014 22:53:54 +0100 Subject: [PATCH 3/4] Make can_add_subpage and can_publish_subpage permission checks return False for page models that disallow subpages; this ensures that 'add child page' links are not shown --- .../wagtailadmin/tests/test_pages_views.py | 32 ++++++++++++++++--- wagtail/wagtailcore/models.py | 7 +++- 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/wagtail/wagtailadmin/tests/test_pages_views.py b/wagtail/wagtailadmin/tests/test_pages_views.py index 986bb260cf..5a2958fb52 100644 --- a/wagtail/wagtailadmin/tests/test_pages_views.py +++ b/wagtail/wagtailadmin/tests/test_pages_views.py @@ -703,22 +703,44 @@ class TestSubpageBusinessRules(TestCase, WagtailTestUtils): 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 diff --git a/wagtail/wagtailcore/models.py b/wagtail/wagtailcore/models.py index 5c3f9d1c51..d7d23c9889 100644 --- a/wagtail/wagtailcore/models.py +++ b/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) From eddb060c8d3ea0701fc7ff651127a0f6611eb295 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Tue, 17 Jun 2014 23:08:30 +0100 Subject: [PATCH 4/4] Bypass 'choose a page type' screen when there is only one available choice in subpage_types --- wagtail/tests/models.py | 3 +++ .../wagtailadmin/tests/test_pages_views.py | 19 +++++++++++++++---- wagtail/wagtailadmin/views/pages.py | 6 ++++++ 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/wagtail/tests/models.py b/wagtail/tests/models.py index f798129dc3..ca6d411dac 100644 --- a/wagtail/tests/models.py +++ b/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): diff --git a/wagtail/wagtailadmin/tests/test_pages_views.py b/wagtail/wagtailadmin/tests/test_pages_views.py index 5a2958fb52..5f4b0021c3 100644 --- a/wagtail/wagtailadmin/tests/test_pages_views.py +++ b/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 @@ -681,24 +681,30 @@ 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() @@ -754,3 +760,8 @@ class TestSubpageBusinessRules(TestCase, WagtailTestUtils): # 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))) diff --git a/wagtail/wagtailadmin/views/pages.py b/wagtail/wagtailadmin/views/pages.py index 559adb9224..f12a08fe6c 100644 --- a/wagtail/wagtailadmin/views/pages.py +++ b/wagtail/wagtailadmin/views/pages.py @@ -59,6 +59,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,