diff --git a/wagtail/admin/forms/models.py b/wagtail/admin/forms/models.py index 260b25c69e..14ef36aab0 100644 --- a/wagtail/admin/forms/models.py +++ b/wagtail/admin/forms/models.py @@ -126,15 +126,27 @@ class WagtailAdminModelForm( def __init__(self, *args, **kwargs): # keep hold of the `for_user` kwarg as well as passing it on to PermissionedForm self.for_user = kwargs.get("for_user") + self.deferred_required_fields = [] super().__init__(*args, **kwargs) def defer_required_fields(self): + if self.deferred_required_fields: + # defer_required_fields has already been called + return + for field_name in self._meta.defer_required_on_fields: try: - self.fields[field_name].required = False + if self.fields[field_name].required: + self.fields[field_name].required = False + self.deferred_required_fields.append(field_name) except KeyError: pass + def restore_required_fields(self): + for field_name in self.deferred_required_fields: + self.fields[field_name].required = True + self.deferred_required_fields = [] + class Meta: formfield_callback = formfield_for_dbfield diff --git a/wagtail/admin/tests/pages/test_create_page.py b/wagtail/admin/tests/pages/test_create_page.py index 0f02384c49..3aea7541fd 100644 --- a/wagtail/admin/tests/pages/test_create_page.py +++ b/wagtail/admin/tests/pages/test_create_page.py @@ -621,6 +621,58 @@ class TestPageCreation(WagtailTestUtils, TestCase): self.assertEqual(page.date_from, None) self.assertFalse(page.live) + def test_required_asterisk_on_reshowing_form(self): + """ + If a form is reshown due to a validation error elsewhere, fields whose validation + was deferred should still show the required asterisk. + """ + post_data = { + "title": "Event page", + "date_from": "", + "slug": "event-page", + "audience": "public", + "location": "", + "cost": "Free", + "signup_link": "Not a valid URL", + "carousel_items-TOTAL_FORMS": 0, + "carousel_items-INITIAL_FORMS": 0, + "carousel_items-MIN_NUM_FORMS": 0, + "carousel_items-MAX_NUM_FORMS": 0, + "speakers-TOTAL_FORMS": 0, + "speakers-INITIAL_FORMS": 0, + "speakers-MIN_NUM_FORMS": 0, + "speakers-MAX_NUM_FORMS": 0, + "related_links-TOTAL_FORMS": 0, + "related_links-INITIAL_FORMS": 0, + "related_links-MIN_NUM_FORMS": 0, + "related_links-MAX_NUM_FORMS": 0, + "head_counts-TOTAL_FORMS": 0, + "head_counts-INITIAL_FORMS": 0, + "head_counts-MIN_NUM_FORMS": 0, + "head_counts-MAX_NUM_FORMS": 0, + } + response = self.client.post( + reverse( + "wagtailadmin_pages:add", + args=("tests", "eventpage", self.root_page.id), + ), + post_data, + ) + self.assertEqual(response.status_code, 200) + + # Empty fields should not cause a validation error, but the invalid URL should + self.assertNotContains(response, "This field is required.") + self.assertContains(response, "Enter a valid URL.", count=1) + + # Asterisks should still show against required fields + soup = self.get_soup(response.content) + self.assertTrue( + soup.select_one('label[for="id_date_from"] > span.w-required-mark') + ) + self.assertTrue( + soup.select_one('label[for="id_location"] > span.w-required-mark') + ) + def test_cannot_publish_eventpage_post_with_blank_start_date(self): """ EventPage.date_from has null=True and blank=False; the latter is enforced when diff --git a/wagtail/admin/tests/pages/test_edit_page.py b/wagtail/admin/tests/pages/test_edit_page.py index 2b647972c2..183340d4ec 100644 --- a/wagtail/admin/tests/pages/test_edit_page.py +++ b/wagtail/admin/tests/pages/test_edit_page.py @@ -578,6 +578,58 @@ class TestPageEdit(WagtailTestUtils, TestCase): self.assertEqual(response.status_code, 200) self.assertContains(response, "This field is required.") + def test_required_asterisk_on_reshowing_form(self): + """ + If a form is reshown due to a validation error elsewhere, fields whose validation + was deferred should still show the required asterisk. + """ + post_data = { + "title": "Event page", + "date_from": "", + "slug": "event-page", + "audience": "public", + "location": "", + "cost": "Free", + "signup_link": "Not a valid URL", + "carousel_items-TOTAL_FORMS": 0, + "carousel_items-INITIAL_FORMS": 0, + "carousel_items-MIN_NUM_FORMS": 0, + "carousel_items-MAX_NUM_FORMS": 0, + "speakers-TOTAL_FORMS": 0, + "speakers-INITIAL_FORMS": 0, + "speakers-MIN_NUM_FORMS": 0, + "speakers-MAX_NUM_FORMS": 0, + "related_links-TOTAL_FORMS": 0, + "related_links-INITIAL_FORMS": 0, + "related_links-MIN_NUM_FORMS": 0, + "related_links-MAX_NUM_FORMS": 0, + "head_counts-TOTAL_FORMS": 0, + "head_counts-INITIAL_FORMS": 0, + "head_counts-MIN_NUM_FORMS": 0, + "head_counts-MAX_NUM_FORMS": 0, + } + response = self.client.post( + reverse( + "wagtailadmin_pages:edit", + args=[self.event_page.id], + ), + post_data, + ) + self.assertEqual(response.status_code, 200) + + # Empty fields should not cause a validation error, but the invalid URL should + self.assertNotContains(response, "This field is required.") + self.assertContains(response, "Enter a valid URL.", count=1) + + # Asterisks should still show against required fields + soup = self.get_soup(response.content) + self.assertTrue( + soup.select_one('label[for="id_date_from"] > span.w-required-mark') + ) + self.assertTrue( + soup.select_one('label[for="id_location"] > span.w-required-mark') + ) + def test_page_edit_post_when_locked(self): # Tests that trying to edit a locked page results in an error diff --git a/wagtail/admin/tests/test_forms.py b/wagtail/admin/tests/test_forms.py index 794fb8d6b3..7592500f9f 100644 --- a/wagtail/admin/tests/test_forms.py +++ b/wagtail/admin/tests/test_forms.py @@ -56,3 +56,13 @@ class TestDeferRequiredFields(TestCase): ) form.defer_required_fields() self.assertTrue(form.is_valid()) + + form = AdvertForm( + { + "url": "https://www.example.com", + "text": "", + } + ) + form.defer_required_fields() + form.restore_required_fields() + self.assertFalse(form.is_valid()) diff --git a/wagtail/admin/views/generic/mixins.py b/wagtail/admin/views/generic/mixins.py index 72fdc84966..a9c24ed372 100644 --- a/wagtail/admin/views/generic/mixins.py +++ b/wagtail/admin/views/generic/mixins.py @@ -751,8 +751,12 @@ class CreateEditViewOptionalFeaturesMixin: # If saving as draft, do not enforce full validation if self.saving_as_draft and isinstance(form, WagtailAdminModelForm): form.defer_required_fields() + form_is_valid = form.is_valid() + form.restore_required_fields() + else: + form_is_valid = form.is_valid() - if form.is_valid(): + if form_is_valid: return self.form_valid(form) else: return self.form_invalid(form) diff --git a/wagtail/admin/views/pages/create.py b/wagtail/admin/views/pages/create.py index 74b6f1d3cf..838367dede 100644 --- a/wagtail/admin/views/pages/create.py +++ b/wagtail/admin/views/pages/create.py @@ -157,6 +157,7 @@ class CreateView(WagtailAdminTemplateMixin, HookResponseMixin, View): if self.form.is_valid(): return self.form_valid(self.form) else: + self.form.restore_required_fields() return self.form_invalid(self.form) @cached_property diff --git a/wagtail/admin/views/pages/edit.py b/wagtail/admin/views/pages/edit.py index c013646744..21bd3bb25e 100644 --- a/wagtail/admin/views/pages/edit.py +++ b/wagtail/admin/views/pages/edit.py @@ -495,6 +495,7 @@ class EditView(WagtailAdminTemplateMixin, HookResponseMixin, View): if self.form.is_valid() and not self.locked_for_user: return self.form_valid(self.form) else: + self.form.restore_required_fields() return self.form_invalid(self.form) def workflow_action_is_valid(self): diff --git a/wagtail/snippets/tests/test_snippets.py b/wagtail/snippets/tests/test_snippets.py index 43a4d13dcc..c731d0eb4b 100644 --- a/wagtail/snippets/tests/test_snippets.py +++ b/wagtail/snippets/tests/test_snippets.py @@ -1401,6 +1401,25 @@ class TestCreateDraftStateSnippet(WagtailTestUtils, TestCase): self.assertEqual(log_entry.revision, snippet.latest_revision) self.assertEqual(log_entry.label, f"DraftStateModel object ({snippet.pk})") + def test_required_asterisk_on_reshowing_form(self): + """ + If a form is reshown due to a validation error elsewhere, fields whose validation + was deferred should still show the required asterisk. + """ + response = self.client.post( + reverse("some_namespace:add"), + {"text": "", "country_code": "UK", "some_number": "meef"}, + ) + + self.assertEqual(response.status_code, 200) + + # The empty text should not cause a validation error, but the invalid number should + self.assertNotContains(response, "This field is required.") + self.assertContains(response, "Enter a whole number.", count=1) + + soup = self.get_soup(response.content) + self.assertTrue(soup.select_one('label[for="id_text"] > span.w-required-mark')) + def test_create_will_not_publish_invalid_snippet(self): response = self.post( post_data={"text": "", "action-publish": "Publish"}, @@ -2374,6 +2393,30 @@ class TestEditDraftStateSnippet(BaseTestSnippetEditView): f"DraftStateCustomPrimaryKeyModel object ({self.test_snippet.pk})", ) + def test_required_asterisk_on_reshowing_form(self): + """ + If a form is reshown due to a validation error elsewhere, fields whose validation + was deferred should still show the required asterisk. + """ + snippet = FullFeaturedSnippet.objects.create( + text="Hello world", + country_code="UK", + some_number=42, + ) + response = self.client.post( + reverse("some_namespace:edit", args=[snippet.pk]), + {"text": "", "country_code": "UK", "some_number": "meef"}, + ) + + self.assertEqual(response.status_code, 200) + + # The empty text should not cause a validation error, but the invalid number should + self.assertNotContains(response, "This field is required.") + self.assertContains(response, "Enter a whole number.", count=1) + + soup = self.get_soup(response.content) + self.assertTrue(soup.select_one('label[for="id_text"] > span.w-required-mark')) + def test_cannot_publish_invalid(self): # Connect a mock signal handler to published signal mock_handler = mock.MagicMock() diff --git a/wagtail/test/testapp/models.py b/wagtail/test/testapp/models.py index 03a227a423..07f7879b24 100644 --- a/wagtail/test/testapp/models.py +++ b/wagtail/test/testapp/models.py @@ -1255,6 +1255,8 @@ class FullFeaturedSnippet( for_concrete_model=False, ) + panels = ["text", "country_code", "some_number"] + search_fields = [ index.SearchField("text"), index.AutocompleteField("text"),