Preserve required asterisks on validation-deferred fields when re-showing form after an error

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
 

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

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
 

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())

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)

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

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):

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()

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"),