From adab5f13389668dc4a890a6b0f48ca7e6548fe1a Mon Sep 17 00:00:00 2001 From: Jonathan Bobel Date: Mon, 24 Feb 2025 12:29:58 -0500 Subject: [PATCH 01/28] 2214 Client-side validation --- app/assets/javascripts/validation.js | 47 +++++++++++++++++++ .../uswds/_uswds-theme-custom-styles.scss | 3 +- .../components/components/input/template.njk | 22 +++++---- app/templates/views/send-test.html | 2 + gulpfile.js | 2 +- 5 files changed, 65 insertions(+), 11 deletions(-) create mode 100644 app/assets/javascripts/validation.js diff --git a/app/assets/javascripts/validation.js b/app/assets/javascripts/validation.js new file mode 100644 index 000000000..28a360380 --- /dev/null +++ b/app/assets/javascripts/validation.js @@ -0,0 +1,47 @@ +// document.addEventListener("DOMContentLoaded", function () { +// const form = document.querySelector(".send-one-off-form"); +// const phoneInput = document.getElementById("phone-number"); + +// // Try to get the error element +// let phoneError = document.getElementById("phone-number-error"); + +// // If not found, create it +// if (!phoneError) { +// phoneError = document.createElement("span"); +// phoneError.id = "phone-number-error"; +// phoneError.classList.add("usa-error-message"); +// phoneError.style.display = "none"; // Keep it hidden initially +// phoneInput.insertAdjacentElement("afterend", phoneError); +// } + +// form.addEventListener("submit", function (event) { +// let isValid = true; + +// if (phoneInput.value.trim() === "") { +// showError(phoneInput, phoneError, "Phone number cannot be empty."); +// isValid = false; +// } + +// if (!isValid) event.preventDefault(); +// }); + +// // Remove error when user starts typing +// phoneInput.addEventListener("input", function () { +// if (phoneInput.value.trim() !== "") { +// hideError(phoneInput, phoneError); +// } +// }); + +// function showError(input, errorElement, message) { +// errorElement.textContent = message; +// errorElement.style.display = "block"; +// input.classList.add("usa-input--error"); +// input.setAttribute("aria-describedby", errorElement.id); +// } + +// function hideError(input, errorElement) { +// errorElement.style.display = "none"; +// input.classList.remove("usa-input--error"); +// input.removeAttribute("aria-describedby"); +// } +// }); diff --git a/app/assets/sass/uswds/_uswds-theme-custom-styles.scss b/app/assets/sass/uswds/_uswds-theme-custom-styles.scss index ebce95061..0348530c6 100644 --- a/app/assets/sass/uswds/_uswds-theme-custom-styles.scss +++ b/app/assets/sass/uswds/_uswds-theme-custom-styles.scss @@ -288,7 +288,8 @@ td.table-empty-message { @include u-width('mobile-lg'); margin-top: units(2); } - input#search { + input#search-by-name { + margin-top: units(1); width: 100%; border: 1px solid color('gray-60'); } diff --git a/app/templates/components/components/input/template.njk b/app/templates/components/components/input/template.njk index 4ea649dce..2693e4be2 100644 --- a/app/templates/components/components/input/template.njk +++ b/app/templates/components/components/input/template.njk @@ -12,7 +12,7 @@ classes: params.label.classes, isPageHeading: params.label.isPageHeading, attributes: params.label.attributes, - for: params.id + for: params.text }) | indent(2) | trim }} {% if params.hint %} {% set hintId = params.id + '-hint' %} @@ -26,7 +26,7 @@ }) | indent(2) | trim }} {% endif %} {% if params.errorMessage %} - {% set errorId = params.id + '-error' %} + {% set errorId = params.label.text + '-error' %} {% set describedBy = describedBy + ' ' + errorId if describedBy else errorId %} {{ usaErrorMessage({ id: errorId, @@ -37,12 +37,16 @@ visuallyHiddenText: params.errorMessage.visuallyHiddenText, }) | indent(2) | trim }} {% endif %} - diff --git a/app/templates/views/send-test.html b/app/templates/views/send-test.html index fd5eb63db..faa71873b 100644 --- a/app/templates/views/send-test.html +++ b/app/templates/views/send-test.html @@ -37,6 +37,8 @@ data_kwargs={'force-focus': True} ) %}
+ {% set extra_class = "extra-tracking" if form.placeholder_value.label.text == "phone number" else "" %} + {% set placeholder_id = "phone number" if form.placeholder_value.label.text == "phone number" else "" %}
{{ form.placeholder_value(param_extensions={"classes": "", "id": "phone-number"}) }}
diff --git a/gulpfile.js b/gulpfile.js index e1bf8ba5c..f6389f3c9 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -82,7 +82,7 @@ const javascripts = () => { paths.src + 'javascripts/totalMessagesChart.js', paths.src + 'javascripts/activityChart.js', paths.src + 'javascripts/sidenav.js', - + paths.src + 'javascripts/validation.js', ]) .pipe(plugins.prettyerror()) .pipe( From 70dae9c208861b19266f39c723721a9f5f32d0d7 Mon Sep 17 00:00:00 2001 From: Jonathan Bobel Date: Mon, 24 Feb 2025 14:18:00 -0500 Subject: [PATCH 02/28] Update test_organizations.py Adjusting a test --- tests/app/main/views/organizations/test_organizations.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/app/main/views/organizations/test_organizations.py b/tests/app/main/views/organizations/test_organizations.py index 0e781ab82..1055b959f 100644 --- a/tests/app/main/views/organizations/test_organizations.py +++ b/tests/app/main/views/organizations/test_organizations.py @@ -817,7 +817,7 @@ def test_manage_org_users_should_show_live_search_if_more_than_7_users( textbox = page.select_one("[data-module=autofocus] .usa-input") assert "value" not in textbox - assert textbox["name"] == "search" + assert textbox["name"] == "search-by-name-or-email-address" # data-module=autofocus is set on a containing element so it # shouldn’t also be set on the textbox itself assert "data-module" not in textbox From 0fa8c571c82d7aa36075ff12daf6766918b1e213 Mon Sep 17 00:00:00 2001 From: Jonathan Bobel Date: Mon, 24 Feb 2025 14:50:29 -0500 Subject: [PATCH 03/28] Update test_organizations.py Fixing a test --- tests/app/main/views/organizations/test_organizations.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/app/main/views/organizations/test_organizations.py b/tests/app/main/views/organizations/test_organizations.py index 1055b959f..0f4ee3dd6 100644 --- a/tests/app/main/views/organizations/test_organizations.py +++ b/tests/app/main/views/organizations/test_organizations.py @@ -101,7 +101,7 @@ def test_page_to_create_new_organization( (input["type"], input["name"], input.get("value")) for input in page.select("input") ] == [ - ("text", "name", None), + ("text", "organization-name", None), ("radio", "organization_type", "federal"), ("radio", "organization_type", "state"), # ('radio', 'organization_type', 'nhs_central'), @@ -824,7 +824,7 @@ def test_manage_org_users_should_show_live_search_if_more_than_7_users( assert not page.select_one("[data-force-focus]") assert textbox["class"] == ["usa-input"] assert ( - normalize_spaces(page.select_one("label[for=search]").text) + normalize_spaces(page.select_one("label[class=usa-label]").text) == "Search by name or email address" ) From 91c2ae6733c561f86d80c379e94bb77212173668 Mon Sep 17 00:00:00 2001 From: Jonathan Bobel Date: Mon, 24 Feb 2025 15:27:51 -0500 Subject: [PATCH 04/28] Update template.njk Reverting this change --- app/templates/components/components/input/template.njk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/templates/components/components/input/template.njk b/app/templates/components/components/input/template.njk index 2693e4be2..0b89fe729 100644 --- a/app/templates/components/components/input/template.njk +++ b/app/templates/components/components/input/template.njk @@ -40,7 +40,7 @@ Date: Mon, 24 Feb 2025 15:34:52 -0500 Subject: [PATCH 05/28] Update test_organizations.py Fixing tests --- tests/app/main/views/organizations/test_organizations.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/app/main/views/organizations/test_organizations.py b/tests/app/main/views/organizations/test_organizations.py index 0f4ee3dd6..a83e817fe 100644 --- a/tests/app/main/views/organizations/test_organizations.py +++ b/tests/app/main/views/organizations/test_organizations.py @@ -101,7 +101,7 @@ def test_page_to_create_new_organization( (input["type"], input["name"], input.get("value")) for input in page.select("input") ] == [ - ("text", "organization-name", None), + ("text", "name", None), ("radio", "organization_type", "federal"), ("radio", "organization_type", "state"), # ('radio', 'organization_type', 'nhs_central'), @@ -817,7 +817,7 @@ def test_manage_org_users_should_show_live_search_if_more_than_7_users( textbox = page.select_one("[data-module=autofocus] .usa-input") assert "value" not in textbox - assert textbox["name"] == "search-by-name-or-email-address" + assert textbox["name"] == "search" # data-module=autofocus is set on a containing element so it # shouldn’t also be set on the textbox itself assert "data-module" not in textbox From 2bb318493a8472615cea86afaddee9ba05a03334 Mon Sep 17 00:00:00 2001 From: Jonathan Bobel Date: Mon, 24 Feb 2025 16:11:42 -0500 Subject: [PATCH 06/28] Fixing tests, adding dynamic slug --- app/__init__.py | 10 ++++++++++ app/templates/components/components/input/template.njk | 2 +- .../views/service_settings/test_service_settings.py | 4 +++- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/app/__init__.py b/app/__init__.py index 54248bda0..9129da961 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -616,6 +616,8 @@ def setup_event_handlers(): def add_template_filters(application): + application.add_template_filter(slugify) + for fn in [ format_auth_type, format_billions, @@ -673,3 +675,11 @@ def init_jinja(application): ] jinja_loader = jinja2.FileSystemLoader(template_folders) application.jinja_loader = jinja_loader + +import re + +def slugify(text): + """ + Converts text to lowercase, replaces spaces with hyphens, and removes invalid characters. + """ + return re.sub(r'[^a-z0-9-]', '', re.sub(r'\s+', '-', text.lower())) diff --git a/app/templates/components/components/input/template.njk b/app/templates/components/components/input/template.njk index 0b89fe729..ce4544a9a 100644 --- a/app/templates/components/components/input/template.njk +++ b/app/templates/components/components/input/template.njk @@ -39,7 +39,7 @@ {% endif %} Date: Tue, 25 Feb 2025 11:14:15 -0500 Subject: [PATCH 07/28] Update __init__.py Moved the import statement --- app/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/__init__.py b/app/__init__.py index 9129da961..84abe7ba4 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -1,6 +1,7 @@ import os import pathlib import secrets +import re from functools import partial from time import monotonic from urllib.parse import unquote, urlparse, urlunparse @@ -676,7 +677,6 @@ def init_jinja(application): jinja_loader = jinja2.FileSystemLoader(template_folders) application.jinja_loader = jinja_loader -import re def slugify(text): """ From bb0d9a25b7a2ce990ae9685b702285f2b0b36fe7 Mon Sep 17 00:00:00 2001 From: Jonathan Bobel Date: Tue, 25 Feb 2025 11:36:53 -0500 Subject: [PATCH 08/28] Update __init__.py Fixing import --- app/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/__init__.py b/app/__init__.py index 84abe7ba4..be7f08146 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -1,7 +1,7 @@ import os import pathlib -import secrets import re +import secrets from functools import partial from time import monotonic from urllib.parse import unquote, urlparse, urlunparse From 2b07517e9cf7eb832b82f0916ee73cc9be172450 Mon Sep 17 00:00:00 2001 From: Jonathan Bobel Date: Tue, 25 Feb 2025 12:23:56 -0500 Subject: [PATCH 09/28] Fixing tests --- .../main/views/service_settings/test_service_settings.py | 4 +++- tests/app/main/views/test_activity.py | 9 ++++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/tests/app/main/views/service_settings/test_service_settings.py b/tests/app/main/views/service_settings/test_service_settings.py index a6fddc16c..f3d7bb59e 100644 --- a/tests/app/main/views/service_settings/test_service_settings.py +++ b/tests/app/main/views/service_settings/test_service_settings.py @@ -1310,6 +1310,8 @@ def test_shows_delete_link_for_error_on_post_request_for_edit_email_reply_to_add _expected_status=200, ) + print(page.prettify()) + assert page.select_one(".usa-back-link").text.strip() == "Back" assert page.select_one(".usa-back-link")["href"] == url_for( ".service_email_reply_to", @@ -1320,7 +1322,7 @@ def test_shows_delete_link_for_error_on_post_request_for_edit_email_reply_to_add == "Error: Enter a valid email address" ) assert ( - page.select_one("input#email_address").get("value") + page.select_one("input#reply-to-email-address").get("value") == "not a valid email address" ) diff --git a/tests/app/main/views/test_activity.py b/tests/app/main/views/test_activity.py index 8d583223e..1110855f3 100644 --- a/tests/app/main/views/test_activity.py +++ b/tests/app/main/views/test_activity.py @@ -388,7 +388,7 @@ def test_search_recipient_form( query_dict = parse_qs(url.query) assert query_dict == {} - assert page.select_one("label[for=to]").text.strip() == expected_search_box_label + assert page.select_one("label:contains('Search by')").text.strip() == expected_search_box_label recipient_inputs = page.select("input[name=to]") assert len(recipient_inputs) == 2 @@ -421,7 +421,7 @@ def test_api_users_are_told_they_can_search_by_reference_when_service_has_api_ke service_id=SERVICE_ONE_ID, message_type=message_type, ) - assert page.select_one("label[for=to]").text.strip() == expected_search_box_label + assert page.select_one("label:contains('Search by')").text.strip() == expected_search_box_label @pytest.mark.parametrize( @@ -448,7 +448,10 @@ def test_api_users_are_not_told_they_can_search_by_reference_when_service_has_no service_id=SERVICE_ONE_ID, message_type=message_type, ) - assert page.select_one("label[for=to]").text.strip() == expected_search_box_label + + print(page.prettify()) + + assert page.select_one("label:contains('Search by')").text.strip() == expected_search_box_label def test_should_show_notifications_for_a_service_with_next_previous( From 951cb0fef9eea33a24839618857ad961e66c8818 Mon Sep 17 00:00:00 2001 From: Jonathan Bobel Date: Tue, 25 Feb 2025 12:28:39 -0500 Subject: [PATCH 10/28] Update test_activity.py --- tests/app/main/views/test_activity.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/app/main/views/test_activity.py b/tests/app/main/views/test_activity.py index 1110855f3..02bed98fa 100644 --- a/tests/app/main/views/test_activity.py +++ b/tests/app/main/views/test_activity.py @@ -449,8 +449,6 @@ def test_api_users_are_not_told_they_can_search_by_reference_when_service_has_no message_type=message_type, ) - print(page.prettify()) - assert page.select_one("label:contains('Search by')").text.strip() == expected_search_box_label From becdfc79d2afd23acfabb08d8c34fa38567e1067 Mon Sep 17 00:00:00 2001 From: Jonathan Bobel Date: Tue, 25 Feb 2025 12:47:54 -0500 Subject: [PATCH 11/28] Update test_service_settings.py Removing a print --- tests/app/main/views/service_settings/test_service_settings.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/app/main/views/service_settings/test_service_settings.py b/tests/app/main/views/service_settings/test_service_settings.py index f3d7bb59e..71a926b34 100644 --- a/tests/app/main/views/service_settings/test_service_settings.py +++ b/tests/app/main/views/service_settings/test_service_settings.py @@ -1310,8 +1310,6 @@ def test_shows_delete_link_for_error_on_post_request_for_edit_email_reply_to_add _expected_status=200, ) - print(page.prettify()) - assert page.select_one(".usa-back-link").text.strip() == "Back" assert page.select_one(".usa-back-link")["href"] == url_for( ".service_email_reply_to", From df4979277a34d0f1afad6b8b6cfeb5107cd5ff25 Mon Sep 17 00:00:00 2001 From: Jonathan Bobel Date: Tue, 25 Feb 2025 13:58:49 -0500 Subject: [PATCH 12/28] Adjusting tests --- tests/app/main/views/test_manage_users.py | 8 ++++++-- tests/app/main/views/test_register.py | 4 ++-- tests/app/main/views/test_send.py | 4 +++- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/tests/app/main/views/test_manage_users.py b/tests/app/main/views/test_manage_users.py index 2f6b3a226..b7cd4195e 100644 --- a/tests/app/main/views/test_manage_users.py +++ b/tests/app/main/views/test_manage_users.py @@ -197,7 +197,7 @@ def test_should_show_live_search_if_more_than_7_users( "usa-input", ] assert ( - normalize_spaces(page.select_one("label[for=search]").text) + normalize_spaces(page.select_one("label:contains('Search by')").text) == "Search by name or email address" ) @@ -366,6 +366,8 @@ def test_user_with_no_mobile_number_cant_be_set_to_sms_auth( user_id=sample_uuid(), ) + + sms_auth_radio_button = page.select_one('input[value="sms_auth"]') assert sms_auth_radio_button.has_attr("disabled") == sms_option_disabled assert normalize_spaces( @@ -445,9 +447,11 @@ def test_invite_user_has_correct_email_field( platform_admin_user, ): client_request.login(platform_admin_user) + page = client_request.get("main.invite_user", service_id=SERVICE_ONE_ID) + print(page.prettify()) # Print the full HTML response email_field = client_request.get( "main.invite_user", service_id=SERVICE_ONE_ID - ).select_one("#email_address") + ).select_one("#email-address") assert email_field["spellcheck"] == "false" assert "autocomplete" not in email_field diff --git a/tests/app/main/views/test_register.py b/tests/app/main/views/test_register.py index a55307a2b..64ff5bd4c 100644 --- a/tests/app/main/views/test_register.py +++ b/tests/app/main/views/test_register.py @@ -17,8 +17,8 @@ def test_render_register_returns_template_with_form(client_request, mocker): page = client_request.get_url("/register") assert page.find("input", attrs={"name": "auth_type"}).attrs["value"] == "sms_auth" - assert page.select_one("#email_address")["spellcheck"] == "false" - assert page.select_one("#email_address")["autocomplete"] == "email" + assert page.select_one("#email-address")["spellcheck"] == "false" + assert page.select_one("#email-address")["autocomplete"] == "email" assert page.select_one("#password")["autocomplete"] == "new-password" assert "Create an account" in page.text diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index 0ab770c18..d957fa393 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -1519,9 +1519,11 @@ def test_link_to_upload_not_offered_when_entering_personalisation( step_index=1, ) + # print(page.prettify()) # Print the full HTML response + # We’re entering personalization assert page.select_one("input[type=text]")["name"] == "placeholder_value" - assert page.select_one("label[for=phone-number]").text.strip() == "name" + assert page.select_one("label").text.strip() == "name" # No ‘Upload’ link shown assert len(page.select("main a")) == 0 assert "Upload" not in page.select_one("main").text From ed7935220130fd3ef502069b176c5c979dd90615 Mon Sep 17 00:00:00 2001 From: Jonathan Bobel Date: Tue, 25 Feb 2025 14:18:51 -0500 Subject: [PATCH 13/28] Update test_manage_users.py Removed print --- tests/app/main/views/test_manage_users.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/app/main/views/test_manage_users.py b/tests/app/main/views/test_manage_users.py index b7cd4195e..69aed9337 100644 --- a/tests/app/main/views/test_manage_users.py +++ b/tests/app/main/views/test_manage_users.py @@ -448,7 +448,6 @@ def test_invite_user_has_correct_email_field( ): client_request.login(platform_admin_user) page = client_request.get("main.invite_user", service_id=SERVICE_ONE_ID) - print(page.prettify()) # Print the full HTML response email_field = client_request.get( "main.invite_user", service_id=SERVICE_ONE_ID ).select_one("#email-address") From 06c099b567ca128bbbffd89fb3befd903bf64056 Mon Sep 17 00:00:00 2001 From: Jonathan Bobel Date: Wed, 26 Feb 2025 11:20:04 -0500 Subject: [PATCH 14/28] Update test_manage_users.py --- tests/app/main/views/test_manage_users.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/app/main/views/test_manage_users.py b/tests/app/main/views/test_manage_users.py index 69aed9337..7987ecb90 100644 --- a/tests/app/main/views/test_manage_users.py +++ b/tests/app/main/views/test_manage_users.py @@ -367,7 +367,6 @@ def test_user_with_no_mobile_number_cant_be_set_to_sms_auth( ) - sms_auth_radio_button = page.select_one('input[value="sms_auth"]') assert sms_auth_radio_button.has_attr("disabled") == sms_option_disabled assert normalize_spaces( @@ -447,7 +446,6 @@ def test_invite_user_has_correct_email_field( platform_admin_user, ): client_request.login(platform_admin_user) - page = client_request.get("main.invite_user", service_id=SERVICE_ONE_ID) email_field = client_request.get( "main.invite_user", service_id=SERVICE_ONE_ID ).select_one("#email-address") From 69457c873da9190df2f0aac3406f1b45a78246ef Mon Sep 17 00:00:00 2001 From: Jonathan Bobel Date: Wed, 26 Feb 2025 11:41:30 -0500 Subject: [PATCH 15/28] Update test_manage_users.py --- tests/app/main/views/test_manage_users.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/app/main/views/test_manage_users.py b/tests/app/main/views/test_manage_users.py index 7987ecb90..7b0b1d9e4 100644 --- a/tests/app/main/views/test_manage_users.py +++ b/tests/app/main/views/test_manage_users.py @@ -366,7 +366,6 @@ def test_user_with_no_mobile_number_cant_be_set_to_sms_auth( user_id=sample_uuid(), ) - sms_auth_radio_button = page.select_one('input[value="sms_auth"]') assert sms_auth_radio_button.has_attr("disabled") == sms_option_disabled assert normalize_spaces( From 4225559677958c739dc5af13bb7e4d13dba12bcf Mon Sep 17 00:00:00 2001 From: Jonathan Bobel Date: Wed, 26 Feb 2025 11:57:26 -0500 Subject: [PATCH 16/28] Update test_manage_users.py --- tests/app/main/views/test_manage_users.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/app/main/views/test_manage_users.py b/tests/app/main/views/test_manage_users.py index 7b0b1d9e4..d586f23c0 100644 --- a/tests/app/main/views/test_manage_users.py +++ b/tests/app/main/views/test_manage_users.py @@ -372,7 +372,6 @@ def test_user_with_no_mobile_number_cant_be_set_to_sms_auth( page.select_one("label[for=login_authentication-0]").text ) == normalize_spaces(expected_label) - @pytest.mark.parametrize( ("endpoint", "extra_args", "expected_checkboxes"), [ From 0daf140b39f8557f921412873a126bb8754ba38c Mon Sep 17 00:00:00 2001 From: Jonathan Bobel Date: Wed, 26 Feb 2025 12:09:34 -0500 Subject: [PATCH 17/28] Update test_manage_users.py --- tests/app/main/views/test_manage_users.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/app/main/views/test_manage_users.py b/tests/app/main/views/test_manage_users.py index d586f23c0..7b0b1d9e4 100644 --- a/tests/app/main/views/test_manage_users.py +++ b/tests/app/main/views/test_manage_users.py @@ -372,6 +372,7 @@ def test_user_with_no_mobile_number_cant_be_set_to_sms_auth( page.select_one("label[for=login_authentication-0]").text ) == normalize_spaces(expected_label) + @pytest.mark.parametrize( ("endpoint", "extra_args", "expected_checkboxes"), [ From e769d421be858392e2cf3be75c08b4e2a045a4f8 Mon Sep 17 00:00:00 2001 From: Jonathan Bobel Date: Wed, 26 Feb 2025 12:19:15 -0500 Subject: [PATCH 18/28] Update test_tour.py --- tests/app/main/views/test_tour.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/app/main/views/test_tour.py b/tests/app/main/views/test_tour.py index c6316d0a7..f55f19529 100644 --- a/tests/app/main/views/test_tour.py +++ b/tests/app/main/views/test_tour.py @@ -163,6 +163,8 @@ def test_should_show_empty_text_box( "main.tour_step", service_id=SERVICE_ONE_ID, template_id=fake_uuid, step_index=1 ) + print(page.prettify()) + textbox = page.select_one( "[data-module=autofocus][data-force-focus=True] .usa-input" ) @@ -175,7 +177,7 @@ def test_should_show_empty_text_box( # shouldn’t also be set on the textbox itself assert "data-module" not in textbox - assert normalize_spaces(page.select_one("label[for=phone-number]").text) == "one" + assert normalize_spaces(page.select_one("label").text) == "one" def test_should_prefill_answers_for_get_tour_step( From ffda07b6bdf49685cc907d02e62f2b66cc4bead5 Mon Sep 17 00:00:00 2001 From: Jonathan Bobel Date: Wed, 26 Feb 2025 12:24:18 -0500 Subject: [PATCH 19/28] Update test_tour.py --- tests/app/main/views/test_tour.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/app/main/views/test_tour.py b/tests/app/main/views/test_tour.py index f55f19529..9a1774a13 100644 --- a/tests/app/main/views/test_tour.py +++ b/tests/app/main/views/test_tour.py @@ -163,8 +163,6 @@ def test_should_show_empty_text_box( "main.tour_step", service_id=SERVICE_ONE_ID, template_id=fake_uuid, step_index=1 ) - print(page.prettify()) - textbox = page.select_one( "[data-module=autofocus][data-force-focus=True] .usa-input" ) From f031f4bf18d64c3218743377cf93dadb985573a3 Mon Sep 17 00:00:00 2001 From: Jonathan Bobel Date: Wed, 26 Feb 2025 12:53:06 -0500 Subject: [PATCH 20/28] Update template.njk Experimenting with setting the "for" --- app/templates/components/components/label/template.njk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/templates/components/components/label/template.njk b/app/templates/components/components/label/template.njk index d2510fc59..72d914428 100644 --- a/app/templates/components/components/label/template.njk +++ b/app/templates/components/components/label/template.njk @@ -2,7 +2,7 @@ {% set labelHtml %} {% endset %} From a5146fc58affedc0801847fca0e66330f6c53acb Mon Sep 17 00:00:00 2001 From: Jonathan Bobel Date: Thu, 27 Feb 2025 14:21:43 -0500 Subject: [PATCH 21/28] Fixing tests --- .../views/organizations/test_organizations.py | 15 +++------- .../service_settings/test_service_settings.py | 28 +++++++++++++++---- 2 files changed, 26 insertions(+), 17 deletions(-) diff --git a/tests/app/main/views/organizations/test_organizations.py b/tests/app/main/views/organizations/test_organizations.py index a83e817fe..f04937e9b 100644 --- a/tests/app/main/views/organizations/test_organizations.py +++ b/tests/app/main/views/organizations/test_organizations.py @@ -982,20 +982,13 @@ def test_view_organization_settings( page = client_request.get(endpoint, org_id=organization_one["id"]) radios = page.select("input[type=radio]") + labels = page.select("label.usa-radio__label") # Select all radio labels in order for index, option in enumerate(expected_options): option_values = { "value": radios[index]["value"], - "label": normalize_spaces( - page.select_one("label[for={}]".format(radios[index]["id"])).text - ), + "label": normalize_spaces(labels[index].text), # Match labels using index } - if "hint" in option: - option_values["hint"] = normalize_spaces( - page.select_one( - "label[for={}] + .usa-hint".format(radios[index]["id"]) - ).text - ) assert option_values == option if expected_selected: @@ -1082,7 +1075,7 @@ def test_update_organization_sector_sends_service_id_data_to_api_client( client_request.post( "main.edit_organization_type", org_id=organization_one["id"], - _data={"organization_type": "federal"}, + _data={"organization_type": "state"}, _expected_status=302, _expected_redirect=url_for( "main.organization_settings", @@ -1093,7 +1086,7 @@ def test_update_organization_sector_sends_service_id_data_to_api_client( mock_update_organization.assert_called_once_with( organization_one["id"], cached_service_ids=["12345", "67890", SERVICE_ONE_ID], - organization_type="federal", + organization_type="state", ) diff --git a/tests/app/main/views/service_settings/test_service_settings.py b/tests/app/main/views/service_settings/test_service_settings.py index 71a926b34..d7155f709 100644 --- a/tests/app/main/views/service_settings/test_service_settings.py +++ b/tests/app/main/views/service_settings/test_service_settings.py @@ -466,14 +466,30 @@ def test_show_switch_service_to_count_as_live_page( "main.service_switch_count_as_live", service_id=SERVICE_ONE_ID, ) - assert page.select_one("[checked]")["value"] == selected - assert ( - page.select_one( - "label[for={}]".format(page.select_one("[checked]")["id"]) - ).text.strip() - == labelled + + client_request.login(platform_admin_user) + page = client_request.get( + "main.service_switch_count_as_live", + service_id=SERVICE_ONE_ID, ) + # Find the checked radio button + checked_input = page.select_one("[checked]") + + # Ensure we actually found a checked input + assert checked_input is not None, "No checked radio button found" + + # Check that the selected value is as expected + assert checked_input["value"] == selected + + # Find all labels + labels = page.select("label.usa-radio__label") + + # Extract label text and see if it matches the expected label + label_texts = [label.text.strip() for label in labels] + + assert labelled in label_texts, f"Expected label '{labelled}' not found. Found labels: {label_texts}" + @pytest.mark.parametrize( ("post_data", "expected_persisted_value"), From f4ca59796421370e84464df83b78390d61492fc0 Mon Sep 17 00:00:00 2001 From: Jonathan Bobel Date: Fri, 28 Feb 2025 11:31:47 -0500 Subject: [PATCH 22/28] Updates to validation.js and it's test - all are passing --- app/assets/javascripts/validation.js | 124 ++++++++++++++++++--------- tests/javascripts/validation.test.js | 62 ++++++++++++++ 2 files changed, 147 insertions(+), 39 deletions(-) create mode 100644 tests/javascripts/validation.test.js diff --git a/app/assets/javascripts/validation.js b/app/assets/javascripts/validation.js index 28a360380..d7ec7c5e2 100644 --- a/app/assets/javascripts/validation.js +++ b/app/assets/javascripts/validation.js @@ -1,47 +1,93 @@ -// document.addEventListener("DOMContentLoaded", function () { -// const form = document.querySelector(".send-one-off-form"); -// const phoneInput = document.getElementById("phone-number"); +function showError(input, errorElement, message) { + errorElement.textContent = ""; // Clear existing message + errorElement.style.display = "block"; -// // Try to get the error element -// let phoneError = document.getElementById("phone-number-error"); + // Small delay to ensure screen readers pick up the change + setTimeout(() => { + errorElement.textContent = message; + }, 10); -// // If not found, create it -// if (!phoneError) { -// phoneError = document.createElement("span"); -// phoneError.id = "phone-number-error"; -// phoneError.classList.add("usa-error-message"); -// phoneError.style.display = "none"; // Keep it hidden initially -// phoneInput.insertAdjacentElement("afterend", phoneError); -// } + input.classList.add("usa-input--error"); + input.setAttribute("aria-describedby", errorElement.id); +} -// form.addEventListener("submit", function (event) { -// let isValid = true; +function hideError(input, errorElement) { + errorElement.style.display = "none"; + input.classList.remove("usa-input--error"); + input.removeAttribute("aria-describedby"); +} -// if (phoneInput.value.trim() === "") { -// showError(phoneInput, phoneError, "Phone number cannot be empty."); -// isValid = false; -// } +function getFieldLabel(input) { + const label = document.querySelector(`label[for="${input.id}"]`); + return label ? label.textContent.trim() : "This field"; +} -// if (!isValid) event.preventDefault(); -// }); +// Attach validation logic to forms +function attachValidation() { + const forms = document.querySelectorAll("form"); + forms.forEach((form) => { + const inputs = form.querySelectorAll("input, textarea, select"); -// // Remove error when user starts typing -// phoneInput.addEventListener("input", function () { -// if (phoneInput.value.trim() !== "") { -// hideError(phoneInput, phoneError); -// } -// }); + form.addEventListener("submit", function (event) { + let isValid = true; + let firstInvalidInput = null; -// function showError(input, errorElement, message) { -// errorElement.textContent = message; -// errorElement.style.display = "block"; -// input.classList.add("usa-input--error"); -// input.setAttribute("aria-describedby", errorElement.id); -// } + inputs.forEach((input) => { + const errorId = input.id ? `${input.id}-error` : `${input.name}-error`; + let errorElement = document.getElementById(errorId); -// function hideError(input, errorElement) { -// errorElement.style.display = "none"; -// input.classList.remove("usa-input--error"); -// input.removeAttribute("aria-describedby"); -// } -// }); + if (!errorElement) { + errorElement = document.createElement("span"); + errorElement.id = errorId; + errorElement.classList.add("usa-error-message"); + errorElement.setAttribute("aria-live", "polite"); + input.insertAdjacentElement("afterend", errorElement); + } + + if (input.type === "radio") { + // Find all radio buttons with the same name + const radioGroup = document.querySelectorAll(`input[name="${input.name}"]`); + const isChecked = Array.from(radioGroup).some(radio => radio.checked); + + if (!isChecked) { + showError(input, errorElement, `Error: ${getFieldLabel(input)} must be selected.`); + isValid = false; + if (!firstInvalidInput) { + firstInvalidInput = input; + } + } + } else if (input.value.trim() === "") { + showError(input, errorElement, `Error: ${getFieldLabel(input)} is required.`); + isValid = false; + if (!firstInvalidInput) { + firstInvalidInput = input; + } + } + }); + + if (!isValid) { + event.preventDefault(); + if (firstInvalidInput) firstInvalidInput.focus(); + } + }); + + inputs.forEach((input) => { + input.addEventListener("input", function () { + const errorElement = document.getElementById(`${input.id}-error`); + if (input.value.trim() !== "" && errorElement) { + hideError(input, errorElement); + } + }); + }); + }); +} + +// Automatically attach validation only in the browser +if (typeof window !== "undefined") { + document.addEventListener("DOMContentLoaded", attachValidation); +} + +// ✅ Check if we're in a Node.js environment (for Jest) before using `module.exports` +if (typeof module !== "undefined" && typeof module.exports !== "undefined") { + module.exports = { showError, hideError, getFieldLabel, attachValidation }; +} diff --git a/tests/javascripts/validation.test.js b/tests/javascripts/validation.test.js new file mode 100644 index 000000000..dc4bae435 --- /dev/null +++ b/tests/javascripts/validation.test.js @@ -0,0 +1,62 @@ +const { showError, hideError, getFieldLabel, attachValidation } = require("../../app/assets/javascripts/validation.js"); + +describe("Form Validation", () => { + let form, input, submitButton; + + beforeEach(() => { + document.body.innerHTML = ` +
+ + + +
+ `; + + form = document.querySelector(".test-form"); + input = document.getElementById("test-input"); + submitButton = form.querySelector("button"); + + // Manually attach validation logic for Jest + attachValidation(); + }); + + afterEach(() => { + document.body.innerHTML = ""; // Clean up DOM after each test + }); + + test("Displays an error message when input is empty", async () => { + form.dispatchEvent(new Event("submit", { bubbles: true })); + + // Wait for the timeout to complete + await new Promise(resolve => setTimeout(resolve, 20)); + + const errorMessage = document.getElementById("test-input-error"); + expect(errorMessage).not.toBeNull(); + expect(errorMessage.textContent).toBe("Error: Test Input is required."); + expect(input.classList.contains("usa-input--error")).toBe(true); +}); + + test("Removes error message when input is filled", () => { + // Trigger validation first + form.dispatchEvent(new Event("submit", { bubbles: true })); + + // Simulate user typing to remove the error + input.value = "Some text"; + input.dispatchEvent(new Event("input", { bubbles: true })); + + const errorMessage = document.getElementById("test-input-error"); + expect(errorMessage).not.toBeNull(); + expect(errorMessage.style.display).toBe("none"); + expect(input.classList.contains("usa-input--error")).toBe(false); + }); + + test("Focus moves to first invalid input", async () => { + const spy = jest.spyOn(input, "focus"); + + form.dispatchEvent(new Event("submit", { bubbles: true })); + + await new Promise((resolve) => setTimeout(resolve, 10)); // Allow DOM updates + + expect(spy).toHaveBeenCalled(); + }); +}); From 94f881477e22a95df2d8904a907ea5e6cb9b53cf Mon Sep 17 00:00:00 2001 From: Jonathan Bobel Date: Mon, 3 Mar 2025 13:33:45 -0500 Subject: [PATCH 23/28] Focusing validation on individual form with one input --- app/assets/javascripts/validation.js | 2 +- app/main/views/send.py | 2 +- app/templates/components/file-upload.html | 2 +- app/templates/components/radios.html | 2 +- app/templates/components/select-input.html | 2 +- app/templates/components/textbox.html | 2 +- tests/javascripts/validation.test.js | 4 ++-- 7 files changed, 8 insertions(+), 8 deletions(-) diff --git a/app/assets/javascripts/validation.js b/app/assets/javascripts/validation.js index d7ec7c5e2..bd556f0c4 100644 --- a/app/assets/javascripts/validation.js +++ b/app/assets/javascripts/validation.js @@ -24,7 +24,7 @@ function getFieldLabel(input) { // Attach validation logic to forms function attachValidation() { - const forms = document.querySelectorAll("form"); + const forms = document.querySelectorAll("form.send-one-off-form"); forms.forEach((form) => { const inputs = form.querySelectorAll("input, textarea, select"); diff --git a/app/main/views/send.py b/app/main/views/send.py index 2b36e5723..d627497a1 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -167,7 +167,7 @@ def send_messages(service_id, template_id): # just show the first error, as we don't expect the form to have more # than one, since it only has one field first_field_errors = list(form.errors.values())[0] - error_message = '' + error_message = '' error_message = f"{error_message}{first_field_errors[0]}" error_message = f"{error_message}" error_message = Markup(error_message) diff --git a/app/templates/components/file-upload.html b/app/templates/components/file-upload.html index 3c613a2c3..27989ee8f 100644 --- a/app/templates/components/file-upload.html +++ b/app/templates/components/file-upload.html @@ -20,7 +20,7 @@ {% endif %} {% if field.errors and show_errors %} - + {{ field.errors[0] }} {% endif %} diff --git a/app/templates/components/radios.html b/app/templates/components/radios.html index f3db32cf7..b629bb871 100644 --- a/app/templates/components/radios.html +++ b/app/templates/components/radios.html @@ -32,7 +32,7 @@ {{ field.label.text }} {% if field.errors %} - + {{ field.errors[0] }} {% endif %} diff --git a/app/templates/components/select-input.html b/app/templates/components/select-input.html index 47ec19c5d..be84aa232 100644 --- a/app/templates/components/select-input.html +++ b/app/templates/components/select-input.html @@ -52,7 +52,7 @@ {% endif %} {% if field.errors %} - + {{ field.errors[0] }} {% endif %} diff --git a/app/templates/components/textbox.html b/app/templates/components/textbox.html index fa92d0cf8..de0f93b9f 100644 --- a/app/templates/components/textbox.html +++ b/app/templates/components/textbox.html @@ -32,7 +32,7 @@
{% endif %} {% if field.errors %} - + Error: {% if not safe_error_message %}{{ field.errors[0] }}{% else %}{{ field.errors[0]|safe }}{% endif %} diff --git a/tests/javascripts/validation.test.js b/tests/javascripts/validation.test.js index dc4bae435..fb6b1a67e 100644 --- a/tests/javascripts/validation.test.js +++ b/tests/javascripts/validation.test.js @@ -5,14 +5,14 @@ describe("Form Validation", () => { beforeEach(() => { document.body.innerHTML = ` -
+
`; - form = document.querySelector(".test-form"); + form = document.querySelector(".send-one-off-form"); input = document.getElementById("test-input"); submitButton = form.querySelector("button"); From b1b4611c555b8a6c0c5b1a515a22a79a16298f66 Mon Sep 17 00:00:00 2001 From: Jonathan Bobel Date: Tue, 4 Mar 2025 11:24:50 -0500 Subject: [PATCH 24/28] Update checks.yml Seeing if this fixes the a11y scan --- .github/workflows/checks.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index 87d7b338e..97e081798 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -177,7 +177,7 @@ jobs: cmd_options: "-I" a11y-scan: - runs-on: ubuntu-20.04 + runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 - uses: ./.github/actions/setup-project From 7144eaa00d0a094809be1592d1a467a4e4cc1fca Mon Sep 17 00:00:00 2001 From: Jonathan Bobel Date: Tue, 4 Mar 2025 14:04:35 -0500 Subject: [PATCH 25/28] Update .pa11yci --- .pa11yci | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.pa11yci b/.pa11yci index 1fde9f2d8..8a0718cb5 100644 --- a/.pa11yci +++ b/.pa11yci @@ -1,7 +1,10 @@ { "defaults": { "standard": "WCAG2AA", - "runners": ["htmlcs"], + "runners": ["htmlcs"], + "chromeLaunchConfig": { + "executablePath": "/usr/bin/google-chrome" + }, "concurrency": 1, "hideElements": [ "nav > ol a", From 16966149958e198a29e43ebaf8bfd6f734ef7bb9 Mon Sep 17 00:00:00 2001 From: Jonathan Bobel Date: Thu, 6 Mar 2025 15:00:21 -0500 Subject: [PATCH 26/28] Update activityChart.test.js --- tests/javascripts/activityChart.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/javascripts/activityChart.test.js b/tests/javascripts/activityChart.test.js index 4e0393e7e..85e245149 100644 --- a/tests/javascripts/activityChart.test.js +++ b/tests/javascripts/activityChart.test.js @@ -199,7 +199,7 @@ test('Fetches data and creates chart and table correctly', async () => { const data = await fetchData('service'); - expect(global.fetch).toHaveBeenCalledWith(`/services/${currentServiceId}/daily-stats.json?timezone=UTC`); + expect(global.fetch).toHaveBeenCalledWith(`/services/${currentServiceId}/daily-stats.json?timezone=America%2FNew_York`); expect(data).toEqual(mockResponse); const labels = Object.keys(mockResponse).map(dateString => { From bd96cc1d20350ab314f10810ba7312f5ae23e512 Mon Sep 17 00:00:00 2001 From: Jonathan Bobel Date: Thu, 6 Mar 2025 15:19:27 -0500 Subject: [PATCH 27/28] Update activityChart.test.js Hmm this failed locally but not when committed. Reverting. --- tests/javascripts/activityChart.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/javascripts/activityChart.test.js b/tests/javascripts/activityChart.test.js index 85e245149..4e0393e7e 100644 --- a/tests/javascripts/activityChart.test.js +++ b/tests/javascripts/activityChart.test.js @@ -199,7 +199,7 @@ test('Fetches data and creates chart and table correctly', async () => { const data = await fetchData('service'); - expect(global.fetch).toHaveBeenCalledWith(`/services/${currentServiceId}/daily-stats.json?timezone=America%2FNew_York`); + expect(global.fetch).toHaveBeenCalledWith(`/services/${currentServiceId}/daily-stats.json?timezone=UTC`); expect(data).toEqual(mockResponse); const labels = Object.keys(mockResponse).map(dateString => { From 32c797e10b2c61be9e4118ec3fc3046e3b5110fc Mon Sep 17 00:00:00 2001 From: Jonathan Bobel Date: Thu, 6 Mar 2025 15:56:47 -0500 Subject: [PATCH 28/28] Update send.py --- app/main/views/send.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/main/views/send.py b/app/main/views/send.py index d627497a1..194ee55ef 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -167,7 +167,7 @@ def send_messages(service_id, template_id): # just show the first error, as we don't expect the form to have more # than one, since it only has one field first_field_errors = list(form.errors.values())[0] - error_message = '' + error_message = '' error_message = f"{error_message}{first_field_errors[0]}" error_message = f"{error_message}" error_message = Markup(error_message)