From f26f2c62b63203b51bccb49ff17eb420937341e5 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Tue, 3 May 2016 12:35:30 +0100 Subject: [PATCH] Handle blank forms in page / collection permission formsets - fixes #2389 (#2540) --- wagtail/wagtailadmin/forms.py | 9 +++++++-- wagtail/wagtailusers/forms.py | 9 +++++++-- wagtail/wagtailusers/tests.py | 20 ++++++++++++++++++++ 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/wagtail/wagtailadmin/forms.py b/wagtail/wagtailadmin/forms.py index 3bcf802c3..4711bfc07 100644 --- a/wagtail/wagtailadmin/forms.py +++ b/wagtail/wagtailadmin/forms.py @@ -418,7 +418,9 @@ class BaseGroupCollectionMemberPermissionFormSet(forms.BaseFormSet): collections = [ form.cleaned_data['collection'] for form in self.forms - if form not in self.deleted_forms + # need to check for presence of 'collection' in cleaned_data, + # because a completely blank form passes validation + if form not in self.deleted_forms and 'collection' in form.cleaned_data ] if len(set(collections)) != len(collections): # collections list contains duplicates @@ -435,7 +437,10 @@ class BaseGroupCollectionMemberPermissionFormSet(forms.BaseFormSet): ) # get a set of (collection, permission) tuples for all ticked permissions - forms_to_save = [form for form in self.forms if form not in self.deleted_forms] + forms_to_save = [ + form for form in self.forms + if form not in self.deleted_forms and 'collection' in form.cleaned_data + ] final_permission_records = set() for form in forms_to_save: diff --git a/wagtail/wagtailusers/forms.py b/wagtail/wagtailusers/forms.py index 0ef5117e6..089353657 100644 --- a/wagtail/wagtailusers/forms.py +++ b/wagtail/wagtailusers/forms.py @@ -295,7 +295,9 @@ class BaseGroupPagePermissionFormSet(forms.BaseFormSet): pages = [ form.cleaned_data['page'] for form in self.forms - if form not in self.deleted_forms + # need to check for presence of 'page' in cleaned_data, + # because a completely blank form passes validation + if form not in self.deleted_forms and 'page' in form.cleaned_data ] if len(set(pages)) != len(pages): # pages list contains duplicates @@ -309,7 +311,10 @@ class BaseGroupPagePermissionFormSet(forms.BaseFormSet): ) # get a set of (page, permission_type) tuples for all ticked permissions - forms_to_save = [form for form in self.forms if form not in self.deleted_forms] + forms_to_save = [ + form for form in self.forms + if form not in self.deleted_forms and 'page' in form.cleaned_data + ] final_permission_records = set() for form in forms_to_save: diff --git a/wagtail/wagtailusers/tests.py b/wagtail/wagtailusers/tests.py index 40577c7fc..12df1bf51 100644 --- a/wagtail/wagtailusers/tests.py +++ b/wagtail/wagtailusers/tests.py @@ -312,6 +312,26 @@ class TestGroupCreateView(TestCase, WagtailTestUtils): ) ) + def test_can_submit_blank_permission_form(self): + # the formsets for page / collection permissions should gracefully + # handle (and ignore) forms that have been left entirely blank + response = self.post({ + 'name': "test group", + 'page_permissions-0-page': [''], + 'page_permissions-TOTAL_FORMS': ['1'], + 'document_permissions-0-collection': [''], + 'document_permissions-TOTAL_FORMS': ['1'], + }) + + self.assertRedirects(response, reverse('wagtailusers_groups:index')) + # The test group now exists, with no page / document permissions + new_group = Group.objects.get(name='test group') + self.assertEqual(new_group.page_permissions.all().count(), 0) + self.assertEqual( + new_group.collection_permissions.filter(permission=self.add_doc_permission).count(), + 0 + ) + class TestGroupEditView(TestCase, WagtailTestUtils): def setUp(self):