Merge pull request #1837 from gasman/fix/disallow-choosing-root

Add flag to AdminPageChooser to specify whether the root page can be chosen

docs/reference/pages/panels.rst

@@ -94,7 +94,7 @@ FieldRowPanel
 PageChooserPanel
 ----------------
 
-.. class:: PageChooserPanel(field_name, page_type=None)
+.. class:: PageChooserPanel(field_name, page_type=None, can_choose_root=False)
 
     You can explicitly link :class:`~wagtail.wagtailcore.models.Page`-derived models together using the :class:`~wagtail.wagtailcore.models.Page` model and ``PageChooserPanel``.
 
@@ -117,10 +117,13 @@ PageChooserPanel
                 PageChooserPanel('related_page', 'demo.PublisherPage'),
             ]
 
-    ``PageChooserPanel`` takes two arguments: a field name and an optional page type. Specifying a page type (in the form of an ``"appname.modelname"`` string) will filter the chooser to display only pages of that type. A list or tuple of page types can also be passed in, to allow choosing a page that matches any of those page types::
+    ``PageChooserPanel`` takes one required argument, the field name. Optionally, specifying a page type (in the form of an ``"appname.modelname"`` string) will filter the chooser to display only pages of that type. A list or tuple of page types can also be passed in, to allow choosing a page that matches any of those page types::
 
         PageChooserPanel('related_page', ['demo.PublisherPage', 'demo.AuthorPage'])
 
+    Passing ``can_choose_root=True`` will allow the editor to choose the tree root as a page. Normally this would be undesirable, since the tree root is never a usable page, but in some specialised cases it may be appropriate; for example, a page with an automatic "related articles" feed could use a PageChooserPanel to select which subsection articles will be taken from, with the root corresponding to 'everywhere'.
+
+
 ImageChooserPanel
 -----------------
 

docs/topics/streamfield.rst

@@ -188,7 +188,13 @@ PageChooserBlock
 
 ``wagtail.wagtailcore.blocks.PageChooserBlock``
 
-A control for selecting a page object, using Wagtail's page browser. The keyword argument ``required`` is accepted.
+A control for selecting a page object, using Wagtail's page browser. The following keyword arguments are accepted:
+
+``required`` (default: True)
+  If true, the field cannot be left blank.
+
+``can_choose_root`` (default: False)
+  If true, the editor can choose the tree root as a page. Normally this would be undesirable, since the tree root is never a usable page, but in some specialised cases it may be appropriate; for example, a block providing a feed of related articles could use a PageChooserBlock to select which subsection articles will be taken from, with the root corresponding to 'everywhere'.
 
 DocumentChooserBlock
 ~~~~~~~~~~~~~~~~~~~~

wagtail/wagtailadmin/edit_handlers.py

@@ -552,7 +552,7 @@ class BasePageChooserPanel(BaseChooserPanel):
     @classmethod
     def widget_overrides(cls):
         return {cls.field_name: widgets.AdminPageChooser(
-            content_type=cls.target_content_type())}
+            content_type=cls.target_content_type(), can_choose_root=cls.can_choose_root)}
 
     @classmethod
     def target_content_type(cls):
@@ -579,7 +579,7 @@ class BasePageChooserPanel(BaseChooserPanel):
 
 
 class PageChooserPanel(object):
-    def __init__(self, field_name, page_type=None):
+    def __init__(self, field_name, page_type=None, can_choose_root=False):
         self.field_name = field_name
 
         if page_type:
@@ -590,12 +590,14 @@ class PageChooserPanel(object):
             page_type = []
 
         self.page_type = page_type
+        self.can_choose_root = can_choose_root
 
     def bind_to_model(self, model):
         return type(str('_PageChooserPanel'), (BasePageChooserPanel,), {
             'model': model,
             'field_name': self.field_name,
             'page_type': self.page_type,
+            'can_choose_root': self.can_choose_root,
         })
 
 

wagtail/wagtailadmin/forms.py

@@ -116,7 +116,7 @@ class CopyForm(forms.Form):
         self.fields['new_parent_page'] = forms.ModelChoiceField(
             initial=self.page.get_parent(),
             queryset=Page.objects.all(),
-            widget=AdminPageChooser(),
+            widget=AdminPageChooser(can_choose_root=True),
             label=_("New parent page"),
             help_text=_("This copy will be a child of this given parent page.")
         )

wagtail/wagtailadmin/static_src/wagtailadmin/js/page-chooser.js

@@ -1,4 +1,4 @@
-function createPageChooser(id, pageTypes, openAtParentId) {
+function createPageChooser(id, pageTypes, openAtParentId, canChooseRoot) {
     var chooserElement = $('#' + id + '-chooser');
     var pageTitle = chooserElement.find('.title');
     var input = $('#' + id);
@@ -10,9 +10,14 @@ function createPageChooser(id, pageTypes, openAtParentId) {
             initialUrl += openAtParentId + '/';
         }
 
+        var urlParams = {page_type: pageTypes.join(',')};
+        if (canChooseRoot) {
+            urlParams.can_choose_root = 'true';
+        }
+
         ModalWorkflow({
             url: initialUrl,
-            urlParams: { page_type: pageTypes.join(',') },
+            urlParams: urlParams,
             responses: {
                 pageChosen: function(pageData) {
                     input.val(pageData.id);

wagtail/wagtailadmin/tests/test_edit_handlers.py

@@ -365,11 +365,25 @@ class TestPageChooserPanel(TestCase):
 
     def test_render_js_init(self):
         result = self.page_chooser_panel.render_as_field()
-        expected_js = 'createPageChooser("{id}", ["{model}"], {parent});'.format(
+        expected_js = 'createPageChooser("{id}", ["{model}"], {parent}, false);'.format(
             id="id_page", model="wagtailcore.page", parent=self.events_index_page.id)
 
         self.assertIn(expected_js, result)
 
+    def test_render_js_init_with_can_choose_root_true(self):
+        # construct an alternative page chooser panel object, with can_choose_root=True
+        MyPageChooserPanel = PageChooserPanel('page', can_choose_root=True).bind_to_model(PageChooserModel)
+        PageChooserForm = MyPageChooserPanel.get_form_class(PageChooserModel)
+
+        form = PageChooserForm(instance=self.test_instance)
+        page_chooser_panel = MyPageChooserPanel(instance=self.test_instance, form=form)
+        result = page_chooser_panel.render_as_field()
+
+        # the canChooseRoot flag on createPageChooser should now be true
+        expected_js = 'createPageChooser("{id}", ["{model}"], {parent}, true);'.format(
+            id="id_page", model="wagtailcore.page", parent=self.events_index_page.id)
+        self.assertIn(expected_js, result)
+
     def test_get_chosen_item(self):
         result = self.page_chooser_panel.get_chosen_item()
         self.assertEqual(result, self.christmas_page)
@@ -408,7 +422,7 @@ class TestPageChooserPanel(TestCase):
         page_chooser_panel = self.MyPageChooserPanel(instance=self.test_instance, form=form)
 
         result = page_chooser_panel.render_as_field()
-        expected_js = 'createPageChooser("{id}", ["{model}"], {parent});'.format(
+        expected_js = 'createPageChooser("{id}", ["{model}"], {parent}, false);'.format(
             id="id_page", model="tests.eventpage", parent=self.events_index_page.id)
 
         self.assertIn(expected_js, result)
@@ -422,7 +436,7 @@ class TestPageChooserPanel(TestCase):
         page_chooser_panel = self.MyPageChooserPanel(instance=self.test_instance, form=form)
 
         result = page_chooser_panel.render_as_field()
-        expected_js = 'createPageChooser("{id}", ["{model}"], {parent});'.format(
+        expected_js = 'createPageChooser("{id}", ["{model}"], {parent}, false);'.format(
             id="id_page", model="tests.eventpage", parent=self.events_index_page.id)
 
         self.assertIn(expected_js, result)

wagtail/wagtailadmin/tests/test_page_chooser.py

@@ -27,6 +27,22 @@ class TestChooserBrowse(TestCase, WagtailTestUtils):
         self.assertTemplateUsed(response, 'wagtailadmin/chooser/browse.html')
 
 
+class TestCanChooseRootFlag(TestCase, WagtailTestUtils):
+    def setUp(self):
+        self.login()
+
+    def get(self, params={}):
+        return self.client.get(reverse('wagtailadmin_choose_page'), params)
+
+    def test_cannot_choose_root_by_default(self):
+        response = self.get()
+        self.assertNotContains(response, '/admin/pages/1/edit/')
+
+    def test_can_choose_root(self):
+        response = self.get({'can_choose_root': 'true'})
+        self.assertContains(response, '/admin/pages/1/edit/')
+
+
 class TestChooserBrowseChild(TestCase, WagtailTestUtils):
     def setUp(self):
         self.root_page = Page.objects.get(id=2)

wagtail/wagtailadmin/tests/test_widgets.py

@@ -32,7 +32,7 @@ class TestAdminPageChooserWidget(TestCase):
         widget = widgets.AdminPageChooser()
 
         js_init = widget.render_js_init('test-id', 'test', None)
-        self.assertEqual(js_init, "createPageChooser(\"test-id\", [\"wagtailcore.page\"], null);")
+        self.assertEqual(js_init, "createPageChooser(\"test-id\", [\"wagtailcore.page\"], null, false);")
 
     def test_render_html_with_value(self):
         widget = widgets.AdminPageChooser()
@@ -44,7 +44,7 @@ class TestAdminPageChooserWidget(TestCase):
         widget = widgets.AdminPageChooser()
 
         js_init = widget.render_js_init('test-id', 'test', self.child_page)
-        self.assertEqual(js_init, "createPageChooser(\"test-id\", [\"wagtailcore.page\"], %d);" % self.root_page.id)
+        self.assertEqual(js_init, "createPageChooser(\"test-id\", [\"wagtailcore.page\"], %d, false);" % self.root_page.id)
 
     # def test_render_html_init_with_content_type omitted as HTML does not
     # change when selecting a content type
@@ -54,7 +54,7 @@ class TestAdminPageChooserWidget(TestCase):
         widget = widgets.AdminPageChooser(content_type=content_type)
 
         js_init = widget.render_js_init('test-id', 'test', None)
-        self.assertEqual(js_init, "createPageChooser(\"test-id\", [\"tests.simplepage\"], null);")
+        self.assertEqual(js_init, "createPageChooser(\"test-id\", [\"tests.simplepage\"], null, false);")
 
     def test_render_js_init_with_multiple_content_types(self):
         content_types = [
@@ -65,4 +65,10 @@ class TestAdminPageChooserWidget(TestCase):
         widget = widgets.AdminPageChooser(content_type=content_types)
 
         js_init = widget.render_js_init('test-id', 'test', None)
-        self.assertEqual(js_init, "createPageChooser(\"test-id\", [\"tests.simplepage\", \"tests.eventpage\"], null);")
+        self.assertEqual(js_init, "createPageChooser(\"test-id\", [\"tests.simplepage\", \"tests.eventpage\"], null, false);")
+
+    def test_render_js_init_with_can_choose_root(self):
+        widget = widgets.AdminPageChooser(can_choose_root=True)
+
+        js_init = widget.render_js_init('test-id', 'test', self.child_page)
+        self.assertEqual(js_init, "createPageChooser(\"test-id\", [\"wagtailcore.page\"], %d, true);" % self.root_page.id)

wagtail/wagtailadmin/views/chooser.py

@@ -69,8 +69,10 @@ def browse(request, parent_page_id=None):
     else:
         desired_classes = (Page, )
 
+    can_choose_root = request.GET.get('can_choose_root', False)
+
     # Parent page can be chosen if it is a instance of desired_classes
-    parent_page.can_choose = issubclass(parent_page.specific_class or Page, desired_classes)
+    parent_page.can_choose = issubclass(parent_page.specific_class or Page, desired_classes) and (can_choose_root or not parent_page.is_root())
 
     # Pagination
     # We apply pagination first so we don't need to walk the entire list

wagtail/wagtailadmin/widgets.py

@@ -122,9 +122,10 @@ class AdminPageChooser(AdminChooser):
     choose_another_text = _('Choose another page')
     link_to_chosen_text = _('Edit this page')
 
-    def __init__(self, content_type=None, **kwargs):
+    def __init__(self, content_type=None, can_choose_root=False, **kwargs):
         super(AdminPageChooser, self).__init__(**kwargs)
         self._content_type = content_type
+        self.can_choose_root = can_choose_root
 
     @cached_property
     def target_content_types(self):
@@ -166,7 +167,7 @@ class AdminPageChooser(AdminChooser):
 
         parent = page.get_parent() if page else None
 
-        return "createPageChooser({id}, {content_type}, {parent});".format(
+        return "createPageChooser({id}, {content_type}, {parent}, {can_choose_root});".format(
             id=json.dumps(id_),
             content_type=json.dumps([
                 '{app}.{model}'.format(
@@ -174,4 +175,6 @@ class AdminPageChooser(AdminChooser):
                     model=content_type.model)
                 for content_type in self.target_content_types
             ]),
-            parent=json.dumps(parent.id if parent else None))
+            parent=json.dumps(parent.id if parent else None),
+            can_choose_root=('true' if self.can_choose_root else 'false')
+        )

wagtail/wagtailcore/blocks/field_block.py

@@ -366,6 +366,10 @@ class ChooserBlock(FieldBlock):
 
 
 class PageChooserBlock(ChooserBlock):
+    def __init__(self, can_choose_root=False, **kwargs):
+        self.can_choose_root = can_choose_root
+        super(PageChooserBlock, self).__init__(**kwargs)
+
     @cached_property
     def target_model(self):
         from wagtail.wagtailcore.models import Page  # TODO: allow limiting to specific page types
@@ -374,7 +378,7 @@ class PageChooserBlock(ChooserBlock):
     @cached_property
     def widget(self):
         from wagtail.wagtailadmin.widgets import AdminPageChooser
-        return AdminPageChooser
+        return AdminPageChooser(can_choose_root=self.can_choose_root)
 
     def render_basic(self, value):
         if value:

wagtail/wagtailcore/tests/test_blocks.py

@@ -1314,6 +1314,7 @@ class TestPageChooserBlock(TestCase):
 
         empty_form_html = block.render_form(None, 'page')
         self.assertIn('<input id="page" name="page" placeholder="" type="hidden" />', empty_form_html)
+        self.assertIn('createPageChooser("page", ["wagtailcore.page"], null, false);', empty_form_html)
 
         christmas_page = Page.objects.get(slug='christmas')
         christmas_form_html = block.render_form(christmas_page, 'page')
@@ -1321,6 +1322,11 @@ class TestPageChooserBlock(TestCase):
         self.assertIn(expected_html, christmas_form_html)
         self.assertIn("pick a page, any page", christmas_form_html)
 
+    def test_form_render_with_can_choose_root(self):
+        block = blocks.PageChooserBlock(help_text="pick a page, any page", can_choose_root=True)
+        empty_form_html = block.render_form(None, 'page')
+        self.assertIn('createPageChooser("page", ["wagtailcore.page"], null, true);', empty_form_html)
+
     def test_form_response(self):
         block = blocks.PageChooserBlock()
         christmas_page = Page.objects.get(slug='christmas')

wagtail/wagtailusers/forms.py

@@ -235,7 +235,7 @@ class GroupForm(forms.ModelForm):
 
 class GroupPagePermissionForm(forms.ModelForm):
     page = forms.ModelChoiceField(queryset=Page.objects.all(),
-        widget=AdminPageChooser(show_edit_link=False))
+        widget=AdminPageChooser(show_edit_link=False, can_choose_root=True))
 
     class Meta:
         model = GroupPagePermission