From 1029ef316e4503141b2c41317ff2ea1e35fed1d8 Mon Sep 17 00:00:00 2001 From: Mariana Bedran Lesche Date: Fri, 12 Jun 2026 17:01:44 -0300 Subject: [PATCH 1/6] Change form_errors format for inline field error display --- l10n/en/cms/contact.ftl | 4 +- springfield/cms/models/pages.py | 25 +++-- .../blocks/form_fields/checkbox_field.html | 5 +- .../form_fields/checkbox_group_field.html | 5 +- .../cms/blocks/form_fields/email_field.html | 5 +- .../cms/blocks/form_fields/phone_field.html | 5 +- .../cms/blocks/form_fields/select_field.html | 5 +- .../cms/blocks/form_fields/text_field.html | 5 +- .../blocks/form_fields/textarea_field.html | 5 +- .../cms/templates/cms/contact_page.html | 4 +- springfield/cms/tests/test_contact_page.py | 94 +++++++++++++++++-- 11 files changed, 129 insertions(+), 33 deletions(-) diff --git a/l10n/en/cms/contact.ftl b/l10n/en/cms/contact.ftl index ad51570b4..0f46aad6e 100644 --- a/l10n/en/cms/contact.ftl +++ b/l10n/en/cms/contact.ftl @@ -6,9 +6,7 @@ ## Error messages -# Variables -# $field (string) The required form field that wasn't filled -contact-form-error-required-field = You must fill out the { $field } field. +contact-form-error-required = This field is required. contact-form-error-sending = There was an error sending your message. Please try again. contact-form-error-empty = Please fill out the form. contact-form-submit = Submit diff --git a/springfield/cms/models/pages.py b/springfield/cms/models/pages.py index 8a08c945a..b5e69f57b 100644 --- a/springfield/cms/models/pages.py +++ b/springfield/cms/models/pages.py @@ -1863,9 +1863,7 @@ def clean(self): def get_context(self, request, *args, **kwargs): context = super().get_context(request, *args, **kwargs) - form_errors = getattr(request, "form_errors", None) - if form_errors: - context["form_errors"] = form_errors + context["form_errors"] = getattr(request, "form_errors", {}) if getattr(request, "form_success", False): context["form_success"] = True context["form_data"] = getattr(request, "form_data", {}) @@ -1899,7 +1897,7 @@ def serve(self, request, *args, **kwargs): f"Basket API returned {api_response.status_code} for path {self.basket_api_path}", level="error", ) - request.form_errors = [ftl_lazy("contact-form-error-sending", ftl_files=self.ftl_files)] + request.form_errors = {"__all__": [ftl_lazy("contact-form-error-sending", ftl_files=self.ftl_files)]} request.form_data = self._get_form_data_for_context(request.POST) response = super().serve(request, *args, **kwargs) add_never_cache_headers(response) @@ -1912,7 +1910,7 @@ def serve(self, request, *args, **kwargs): f"Basket API request failed for path {self.basket_api_path}", level="error", ) - request.form_errors = [ftl_lazy("contact-form-error-sending", ftl_files=self.ftl_files)] + request.form_errors = {"__all__": [ftl_lazy("contact-form-error-sending", ftl_files=self.ftl_files)]} request.form_data = self._get_form_data_for_context(request.POST) response = super().serve(request, *args, **kwargs) add_never_cache_headers(response) @@ -1928,7 +1926,7 @@ def serve(self, request, *args, **kwargs): "Failed to send contact form email", level="error", ) - request.form_errors = [ftl_lazy("contact-form-error-sending", ftl_files=self.ftl_files)] + request.form_errors = {"__all__": [ftl_lazy("contact-form-error-sending", ftl_files=self.ftl_files)]} request.form_data = self._get_form_data_for_context(request.POST) response = super().serve(request, *args, **kwargs) add_never_cache_headers(response) @@ -1981,12 +1979,14 @@ def _get_form_data_for_context(self, post_data): def validate_form_data(self, post_data): """Validate submitted form data against the field configuration. - Returns a list of error messages. An empty list means the data is valid. + Returns a dict matching Django's ErrorDict shape: + {identifier: [msg], ..., "__all__": [global_msg]} + An empty dict means the data is valid. """ if post_data.get("office_fax", ""): - return [ftl_lazy("contact-form-error-sending", ftl_files=self.ftl_files)] + return {"__all__": [ftl_lazy("contact-form-error-sending", ftl_files=self.ftl_files)]} - errors = [] + errors = {} has_any_data = False for field in self.form_fields: @@ -1995,7 +1995,6 @@ def validate_form_data(self, post_data): value = field.value identifier = value["internal_identifier"] - label = value["label"] is_required = value.get("required", False) if field.block_type == "checkbox_group_field": @@ -2007,10 +2006,10 @@ def validate_form_data(self, post_data): has_any_data = True if is_required and not submitted: - errors.append(ftl_lazy("contact-form-error-required-field", ftl_files=self.ftl_files, field=label)) + errors[identifier] = [ftl_lazy("contact-form-error-required", ftl_files=self.ftl_files)] - if not has_any_data: - errors.append(ftl_lazy("contact-form-error-empty", ftl_files=self.ftl_files)) + if not has_any_data and not errors: + errors.setdefault("__all__", []).append(ftl_lazy("contact-form-error-empty", ftl_files=self.ftl_files)) return errors diff --git a/springfield/cms/templates/cms/blocks/form_fields/checkbox_field.html b/springfield/cms/templates/cms/blocks/form_fields/checkbox_field.html index 345f1246e..cd926ef9c 100644 --- a/springfield/cms/templates/cms/blocks/form_fields/checkbox_field.html +++ b/springfield/cms/templates/cms/blocks/form_fields/checkbox_field.html @@ -4,9 +4,12 @@ file, You can obtain one at https://mozilla.org/MPL/2.0/. #} -
+
+ {% for error in form_errors.get(value.internal_identifier, []) %} +

{{ error }}

+ {% endfor %}
diff --git a/springfield/cms/templates/cms/blocks/form_fields/checkbox_group_field.html b/springfield/cms/templates/cms/blocks/form_fields/checkbox_group_field.html index e17095e1d..7e0a46500 100644 --- a/springfield/cms/templates/cms/blocks/form_fields/checkbox_group_field.html +++ b/springfield/cms/templates/cms/blocks/form_fields/checkbox_group_field.html @@ -4,7 +4,7 @@ file, You can obtain one at https://mozilla.org/MPL/2.0/. #} -
+
{{ value.label }}{% if value.required %} {% endif %} {% for option in value.options %} {% endfor %} + {% for error in form_errors.get(value.internal_identifier, []) %} +

{{ error }}

+ {% endfor %}
diff --git a/springfield/cms/templates/cms/blocks/form_fields/email_field.html b/springfield/cms/templates/cms/blocks/form_fields/email_field.html index e111088b4..4eda77c6a 100644 --- a/springfield/cms/templates/cms/blocks/form_fields/email_field.html +++ b/springfield/cms/templates/cms/blocks/form_fields/email_field.html @@ -4,9 +4,12 @@ file, You can obtain one at https://mozilla.org/MPL/2.0/. #} -
+
+ {% for error in form_errors.get(value.internal_identifier, []) %} +

{{ error }}

+ {% endfor %}
diff --git a/springfield/cms/templates/cms/blocks/form_fields/phone_field.html b/springfield/cms/templates/cms/blocks/form_fields/phone_field.html index 280e66ff0..08f2f2805 100644 --- a/springfield/cms/templates/cms/blocks/form_fields/phone_field.html +++ b/springfield/cms/templates/cms/blocks/form_fields/phone_field.html @@ -4,9 +4,12 @@ file, You can obtain one at https://mozilla.org/MPL/2.0/. #} -
+
+ {% for error in form_errors.get(value.internal_identifier, []) %} +

{{ error }}

+ {% endfor %}
diff --git a/springfield/cms/templates/cms/blocks/form_fields/select_field.html b/springfield/cms/templates/cms/blocks/form_fields/select_field.html index 8ce570766..fbe140ce5 100644 --- a/springfield/cms/templates/cms/blocks/form_fields/select_field.html +++ b/springfield/cms/templates/cms/blocks/form_fields/select_field.html @@ -4,7 +4,7 @@ file, You can obtain one at https://mozilla.org/MPL/2.0/. #} -
+
@@ -14,4 +14,7 @@ {% endfor %} + {% for error in form_errors.get(value.internal_identifier, []) %} +

{{ error }}

+ {% endfor %}
diff --git a/springfield/cms/templates/cms/blocks/form_fields/text_field.html b/springfield/cms/templates/cms/blocks/form_fields/text_field.html index 0cebf8559..e966224f5 100644 --- a/springfield/cms/templates/cms/blocks/form_fields/text_field.html +++ b/springfield/cms/templates/cms/blocks/form_fields/text_field.html @@ -4,9 +4,12 @@ file, You can obtain one at https://mozilla.org/MPL/2.0/. #} -
+
+ {% for error in form_errors.get(value.internal_identifier, []) %} +

{{ error }}

+ {% endfor %}
diff --git a/springfield/cms/templates/cms/blocks/form_fields/textarea_field.html b/springfield/cms/templates/cms/blocks/form_fields/textarea_field.html index bc2dced8c..e16251eb8 100644 --- a/springfield/cms/templates/cms/blocks/form_fields/textarea_field.html +++ b/springfield/cms/templates/cms/blocks/form_fields/textarea_field.html @@ -4,9 +4,12 @@ file, You can obtain one at https://mozilla.org/MPL/2.0/. #} -
+
+ {% for error in form_errors.get(value.internal_identifier, []) %} +

{{ error }}

+ {% endfor %}
diff --git a/springfield/cms/templates/cms/contact_page.html b/springfield/cms/templates/cms/contact_page.html index 1fd7c828d..019811c34 100644 --- a/springfield/cms/templates/cms/contact_page.html +++ b/springfield/cms/templates/cms/contact_page.html @@ -41,10 +41,10 @@
{% csrf_token %} - {% if form_errors %} + {% if form_errors.get("__all__") %}
    - {% for error in form_errors %} + {% for error in form_errors["__all__"] %}
  • {{ error }}
    • {% endfor %}
diff --git a/springfield/cms/tests/test_contact_page.py b/springfield/cms/tests/test_contact_page.py index 18c731af7..84aea0604 100644 --- a/springfield/cms/tests/test_contact_page.py +++ b/springfield/cms/tests/test_contact_page.py @@ -150,7 +150,7 @@ def test_contact_page_post_errors_is_never_cached( resp = page.serve(request) assert resp.status_code == 200 - assert "Please fill out the form." in resp.text + assert "This field is required." in resp.text cache_control = resp.get("Cache-Control", "") assert "no-store" in cache_control mock_email_class.assert_not_called() @@ -256,7 +256,7 @@ def test_contact_page_post_missing_required( minimal_site: Site, # noqa: F811 rf: RequestFactory, ) -> None: - """Test that a POST missing required fields re-renders with errors.""" + """Test that a POST missing required fields re-renders with inline errors.""" index_page = minimal_site.root_page form_field_variants = get_form_field_variants() thank_you_page = _create_thank_you_page(index_page) @@ -283,9 +283,8 @@ def test_contact_page_post_missing_required( page_content = resp.text assert resp.status_code == 200 - assert "You must fill out the First Name field." in page_content - assert "You must fill out the Business Email field." in page_content - assert "You must fill out the Company Size field." in page_content + # Error text appears inline multiple times (once per missing required field) + assert page_content.count("This field is required.") >= 3 mock_email_class.assert_not_called() @@ -345,7 +344,7 @@ def test_contact_page_post_empty_submission( thank_you_page = _create_thank_you_page(index_page) # Use only optional fields so required-field validation doesn't trigger first - optional_fields = [form_field_variants[1], form_field_variants[3]] # company, phone + optional_fields = [form_field_variants[8], form_field_variants[10]] # message (textarea), opt_in (checkbox) page = ContactPage( title="Contact Empty Test", @@ -855,7 +854,7 @@ def test_textarea_field_required_validates( resp = page.serve(request) assert resp.status_code == 200 - assert "You must fill out the Message field." in resp.content.decode() + assert "This field is required." in resp.content.decode() @patch("springfield.cms.models.pages.EmailMessage") @@ -900,13 +899,92 @@ def test_textarea_field_value_in_email( assert "Hello, I have a question about your product." in email_body +def test_validate_form_data_per_field_errors( + minimal_site: Site, # noqa: F811 +) -> None: + """validate_form_data returns a dict keyed by identifier for missing required fields.""" + index_page = minimal_site.root_page + thank_you_page = _create_thank_you_page(index_page) + page = ContactPage( + title="Validate Test", + slug="validate-test", + form_fields=get_form_field_variants()[:2], # first_name and last_name, both required + to_email_address="test@example.com", + redirect_to=thank_you_page, + ) + index_page.add_child(instance=page) + page.save_revision().publish() + + post_data = QueryDict("") # empty POST + errors = page.validate_form_data(post_data) + + assert "first_name" in errors + assert "last_name" in errors + assert "__all__" not in errors # no global errors when individual fields are caught + assert errors["first_name"] == ["This field is required."] + + +def test_validate_form_data_valid_returns_empty_dict( + minimal_site: Site, # noqa: F811 +) -> None: + """validate_form_data returns {} (falsy) for a valid submission.""" + index_page = minimal_site.root_page + thank_you_page = _create_thank_you_page(index_page) + page = ContactPage( + title="Validate Valid Test", + slug="validate-valid-test", + form_fields=get_form_field_variants()[:2], + to_email_address="test@example.com", + redirect_to=thank_you_page, + ) + index_page.add_child(instance=page) + page.save_revision().publish() + + post_data = QueryDict("first_name=Jane&last_name=Doe") + errors = page.validate_form_data(post_data) + + assert errors == {} + + +def test_validate_form_data_empty_form( + minimal_site: Site, # noqa: F811 +) -> None: + """validate_form_data adds '__all__' error when form has no data at all.""" + index_page = minimal_site.root_page + thank_you_page = _create_thank_you_page(index_page) + + # Only optional fields so required-field errors don't fire + optional_fields = [ + { + "type": "text_field", + "value": {"internal_identifier": "message", "label": "Message", "required": False}, + "id": "text-field-msg", + } + ] + page = ContactPage( + title="Validate Empty Test", + slug="validate-empty-test", + form_fields=optional_fields, + to_email_address="test@example.com", + redirect_to=thank_you_page, + ) + index_page.add_child(instance=page) + page.save_revision().publish() + + post_data = QueryDict("") + errors = page.validate_form_data(post_data) + + assert "__all__" in errors + assert len(errors["__all__"]) == 1 + + def test_validation_error_strings_are_translatable() -> None: """validate_form_data returns lazy strings so errors can be localised at render time.""" page = ContactPage(to_email_address="test@example.com") # An empty submission with no configured fields triggers the "fill out the form" error errors = page.validate_form_data({}) assert errors - assert isinstance(errors[0], Promise), "Validation errors must use gettext_lazy for i18n support" + assert isinstance(errors["__all__"][0], Promise), "Validation errors must use gettext_lazy for i18n support" # ============================================================================ From d129e870e93499dbb80749d1fc8f41e48b5016ae Mon Sep 17 00:00:00 2001 From: Mariana Bedran Lesche Date: Fri, 12 Jun 2026 17:21:22 -0300 Subject: [PATCH 2/6] Add no-JS notification and inline field error tests --- .../cms/templates/cms/contact_page.html | 6 ++ springfield/cms/tests/test_contact_page.py | 55 +++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/springfield/cms/templates/cms/contact_page.html b/springfield/cms/templates/cms/contact_page.html index 019811c34..4798ee97b 100644 --- a/springfield/cms/templates/cms/contact_page.html +++ b/springfield/cms/templates/cms/contact_page.html @@ -14,6 +14,12 @@ {% block content %}
+ + {% if page.intro %} {% for block in page.intro %} {% set block_level = 1 %} diff --git a/springfield/cms/tests/test_contact_page.py b/springfield/cms/tests/test_contact_page.py index 84aea0604..c4c6dd7ca 100644 --- a/springfield/cms/tests/test_contact_page.py +++ b/springfield/cms/tests/test_contact_page.py @@ -1356,3 +1356,58 @@ def test_form_persistence_checkbox_field( assert resp.status_code == 200 assert 'value="on" checked' in content + + +@patch("springfield.cms.models.pages.EmailMessage") +def test_inline_field_error_html( + mock_email_class, + minimal_site: Site, # noqa: F811 + rf: RequestFactory, +) -> None: + """When a required field is missing, the field wrapper gets fl-field-error and a message.""" + index_page = minimal_site.root_page + thank_you_page = _create_thank_you_page(index_page) + + page = ContactPage( + title="Inline Error Test", + slug="inline-error-test", + form_fields=get_form_field_variants()[:1], # first_name only, required + to_email_address="test@example.com", + redirect_to=thank_you_page, + ) + index_page.add_child(instance=page) + page.save_revision().publish() + + request = rf.post(page.relative_url(minimal_site), {}) + resp = page.serve(request) + content = resp.text + + assert resp.status_code == 200 + assert "fl-field-wrap fl-field-error" in content + assert "fl-field-error-message" in content + assert "This field is required." in content + + +def test_no_js_notification_present( + minimal_site: Site, # noqa: F811 + rf: RequestFactory, +) -> None: + """The contact page renders a noscript notification with orange color.""" + index_page = minimal_site.root_page + thank_you_page = _create_thank_you_page(index_page) + + page = ContactPage( + title="NoJS Test", + slug="nojs-test", + to_email_address="test@example.com", + redirect_to=thank_you_page, + ) + index_page.add_child(instance=page) + page.save_revision().publish() + + request = rf.get(page.relative_url(minimal_site)) + resp = page.serve(request) + content = resp.text + + assert "