From 670d82b72ce4a50e918033170388f1ef23cf24da Mon Sep 17 00:00:00 2001 From: Jonathan Bobel Date: Fri, 25 Apr 2025 10:23:28 -0400 Subject: [PATCH 01/10] Adjusting back text for all steps in send message process --- app/main/views/send.py | 61 +++++++++++++------- app/templates/views/check/ok.html | 2 +- app/templates/views/edit-sms-template.html | 3 +- app/templates/views/notifications/check.html | 2 +- app/templates/views/send-test.html | 2 +- app/templates/views/send.html | 5 +- 6 files changed, 50 insertions(+), 25 deletions(-) diff --git a/app/main/views/send.py b/app/main/views/send.py index e3b3ed0a9..ab5a4c6dc 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -476,7 +476,7 @@ def send_one_off_step(service_id, template_id, step_index): "views/send-test.html", page_title=get_send_test_page_title( template.template_type, - entering_recipient=not session["recipient"], + entering_recipient=(step_index == 0), name=template.name, ), template=template, @@ -755,31 +755,52 @@ def get_back_link( preview=False, ): if preview: - return url_for( - "main.check_notification", - service_id=service_id, - template_id=template.id, - ) + return { + "href": { + "url": url_for( + "main.check_notification", + service_id=service_id, + template_id=template.id, + ), + "text": "Back to preview" + }, + "html": "Back to preview" + } if step_index == 0: if should_skip_template_page(template._template): - return url_for( - ".choose_template", - service_id=service_id, - ) + return { + "href": { + "url": url_for(".choose_template", service_id=service_id), + "text": "Back to all templates" + }, + "html": "Back to all templates" + } else: - return url_for( - ".view_template", + return { + "href": { + "url": url_for(".view_template", service_id=service_id, template_id=template.id), + "text": "Back to confirm your template" + }, + "html": "Back to confirm your template" + } + + back_to_text = ( + "Back to select recipients" if step_index == 1 else "Back to message personalization" + ) + + return { + "href": { + "url": url_for( + "main.send_one_off_step", service_id=service_id, template_id=template.id, - ) - - return url_for( - "main.send_one_off_step", - service_id=service_id, - template_id=template.id, - step_index=step_index - 1, - ) + step_index=step_index - 1, + ), + "text": back_to_text + }, + "html": back_to_text + } def get_skip_link(step_index, template): diff --git a/app/templates/views/check/ok.html b/app/templates/views/check/ok.html index 71324b724..02c903d0e 100644 --- a/app/templates/views/check/ok.html +++ b/app/templates/views/check/ok.html @@ -12,7 +12,7 @@ {% block backLink %} - {{ usaBackLink({ "href": back_link }) }} + {{ usaBackLink(back_link) }} {% endblock %} {% block maincolumn_content %} diff --git a/app/templates/views/edit-sms-template.html b/app/templates/views/edit-sms-template.html index 87dba61e4..8dc8448a1 100644 --- a/app/templates/views/edit-sms-template.html +++ b/app/templates/views/edit-sms-template.html @@ -11,7 +11,8 @@ {% block backLink %} {{ usaBackLink({ - "href": url_for('main.choose_template', service_id=current_service.id, template_folder_id=template_folder_id) if template_folder_id else url_for('main.choose_template', service_id=current_service.id) + "href": url_for('main.choose_template', service_id=current_service.id, template_folder_id=template_folder_id) if template_folder_id else url_for('main.choose_template', service_id=current_service.id), + "html": "Back to all templates" }) }} {% endblock %} diff --git a/app/templates/views/notifications/check.html b/app/templates/views/notifications/check.html index 8646d1b7f..6566c395c 100644 --- a/app/templates/views/notifications/check.html +++ b/app/templates/views/notifications/check.html @@ -9,7 +9,7 @@ {% endblock %} {% block backLink %} - {{ usaBackLink({ "href": back_link }) }} + {{ usaBackLink(back_link) }} {% endblock %} {% block maincolumn_content %} diff --git a/app/templates/views/send-test.html b/app/templates/views/send-test.html index 3f1161f6b..2dc94efb7 100644 --- a/app/templates/views/send-test.html +++ b/app/templates/views/send-test.html @@ -9,7 +9,7 @@ {% endblock %} {% block backLink %} - {{ usaBackLink({ "href": back_link }) }} + {{ usaBackLink(back_link) }} {% endblock %} diff --git a/app/templates/views/send.html b/app/templates/views/send.html index 219c16cc8..2250b5199 100644 --- a/app/templates/views/send.html +++ b/app/templates/views/send.html @@ -12,7 +12,10 @@ {% block backLink %} - {{ usaBackLink(params) }} + {{ usaBackLink({ + "href": url_for('main.send_one_off_step', service_id=current_service.id, template_id=template.id, step_index=0), + "html": "Back to select recipients" + }) }} {% endblock %} {% set phone_numbers = [ From fb5f7d969e60d2ff9ab3b993d9ff44c6fcc5506a Mon Sep 17 00:00:00 2001 From: Jonathan Bobel Date: Fri, 25 Apr 2025 10:35:25 -0400 Subject: [PATCH 02/10] 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 b6f6132ee..2643fdf49 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -756,7 +756,7 @@ def get_back_link( preview=False, ): if preview: - return { + return { "href": { "url": url_for( "main.check_notification", From 350526f4adac5ce24c6ec34b2e63c60d634cfd74 Mon Sep 17 00:00:00 2001 From: Jonathan Bobel Date: Fri, 25 Apr 2025 11:39:10 -0400 Subject: [PATCH 03/10] Update test_send.py --- tests/app/main/views/test_send.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index e3617d9f8..27956f299 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -1449,7 +1449,12 @@ def test_send_one_off_offers_link_to_upload( back_link = page.select_one(".usa-back-link") link = page.select_one("form a") - assert back_link.text.strip() == "Back" + assert back_link.text.strip() in { + "Back to all templates", + "Back to confirm your template", + "Back to select recipients", + "Back to message personalization" + } assert link.text.strip() == "Upload a list of phone numbers" assert link["href"] == url_for( @@ -2284,9 +2289,13 @@ def test_check_messages_back_link( **extra_args, ) - assert (page.find_all("a", {"class": "usa-back-link"})[0]["href"]) == expected_url( - service_id=SERVICE_ONE_ID, template_id=fake_uuid - ) + # assert (page.find_all("a", {"class": "usa-back-link"})[0]["href"]) == expected_url( + # service_id=SERVICE_ONE_ID, template_id=fake_uuid + # ) + + actual_href = page.find_all("a", {"class": "usa-back-link"})[0]["href"] + expected_href = expected_url(service_id=SERVICE_ONE_ID, template_id=fake_uuid) + assert actual_href == expected_href @pytest.mark.parametrize( From cd40580231339725aa4ea939757991e562faaebb Mon Sep 17 00:00:00 2001 From: Jonathan Bobel Date: Fri, 25 Apr 2025 16:20:21 -0400 Subject: [PATCH 04/10] Working through some tests --- app/main/views/send.py | 73 +++++++++++++------ .../components/components/back-link/macro.njk | 2 +- .../components/back-link/template.njk | 4 +- app/templates/views/check/preview.html | 2 +- .../views/notifications/preview.html | 4 +- tests/app/main/views/test_send.py | 21 +++--- 6 files changed, 70 insertions(+), 36 deletions(-) diff --git a/app/main/views/send.py b/app/main/views/send.py index 2643fdf49..ba098173e 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -476,7 +476,7 @@ def send_one_off_step(service_id, template_id, step_index): "views/send-test.html", page_title=get_send_test_page_title( template.template_type, - entering_recipient=(step_index == 0), + entering_recipient=not session["recipient"], name=template.name, ), template=template, @@ -561,24 +561,47 @@ def _check_messages(service_id, template_id, upload_id, preview_row, **kwargs): ) if request.args.get("from_test"): - # TODO: may not be required after letters code removed - back_link = url_for( - "main.send_one_off", service_id=service_id, template_id=template.id - ) - back_link_from_preview = url_for( - "main.send_one_off", service_id=service_id, template_id=template.id - ) + back_link = { + "href": { + "url": url_for( + "main.send_one_off", service_id=service_id, template_id=template.id + ), + "text": "Back to message personalization" + }, + "html": "Back to message personalization" + } + back_link_from_preview = { + "href": { + "url": url_for( + "main.send_one_off", service_id=service_id, template_id=template.id + ), + "text": "Back to message personalization" + }, + "html": "Back to message personalization" + } choose_time_form = None else: - back_link = url_for( - "main.send_messages", service_id=service_id, template_id=template.id - ) - back_link_from_preview = url_for( - "main.check_messages", - service_id=service_id, - template_id=template.id, - upload_id=upload_id, - ) + back_link = { + "href": { + "url": url_for( + "main.send_messages", service_id=service_id, template_id=template.id + ), + "text": "Back to upload a file" + }, + "html": "Back to upload a file" + } + back_link_from_preview = { + "href": { + "url": url_for( + "main.check_messages", + service_id=service_id, + template_id=template.id, + upload_id=upload_id, + ), + "text": "Back to check messages" + }, + "html": "Back to check messages" + } choose_time_form = ChooseTimeForm() if preview_row < 2: @@ -763,16 +786,19 @@ def get_back_link( service_id=service_id, template_id=template.id, ), - "text": "Back to preview" + "text": "Back to select delivery time" }, - "html": "Back to preview" + "html": "Back to select delivery time" } if step_index == 0: if should_skip_template_page(template._template): return { "href": { - "url": url_for(".choose_template", service_id=service_id), + "url": url_for( + ".choose_template", + service_id=service_id, + ), "text": "Back to all templates" }, "html": "Back to all templates" @@ -780,12 +806,17 @@ def get_back_link( else: return { "href": { - "url": url_for(".view_template", service_id=service_id, template_id=template.id), + "url": url_for( + ".view_template", + service_id=service_id, + template_id=template.id, + ), "text": "Back to confirm your template" }, "html": "Back to confirm your template" } + # fallback for other steps back_to_text = ( "Back to select recipients" if step_index == 1 else "Back to message personalization" ) diff --git a/app/templates/components/components/back-link/macro.njk b/app/templates/components/components/back-link/macro.njk index d232c1b2b..e52d92fff 100644 --- a/app/templates/components/components/back-link/macro.njk +++ b/app/templates/components/components/back-link/macro.njk @@ -1,3 +1,3 @@ {% macro usaBackLink(params) %} - {%- include "./template.njk" -%} + {%- include "./template.njk" with context %} {% endmacro %} diff --git a/app/templates/components/components/back-link/template.njk b/app/templates/components/components/back-link/template.njk index 340b3ca22..84344ab35 100644 --- a/app/templates/components/components/back-link/template.njk +++ b/app/templates/components/components/back-link/template.njk @@ -1,4 +1,6 @@ diff --git a/app/templates/views/check/preview.html b/app/templates/views/check/preview.html index 091ba345f..0d8d65f98 100644 --- a/app/templates/views/check/preview.html +++ b/app/templates/views/check/preview.html @@ -13,7 +13,7 @@ {% block backLink %} - {{ usaBackLink({ "href": back_link_from_preview }) }} + {{ usaBackLink(www.google.com) }} {% endblock %} {% block maincolumn_content %} diff --git a/app/templates/views/notifications/preview.html b/app/templates/views/notifications/preview.html index 536288d9a..c870ba539 100644 --- a/app/templates/views/notifications/preview.html +++ b/app/templates/views/notifications/preview.html @@ -16,9 +16,9 @@ {% block backLink %} {% if help %} - {{ usaBackLink({ "href": back_link }) }} + {{ usaBackLink(back_link) }} {% else %} - {{ usaBackLink({ "href": back_link_from_preview }) }} + {{ usaBackLink(back_link_from_preview) }} {% endif %} {% endblock %} diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index 27956f299..6e6a5233e 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -1451,9 +1451,7 @@ def test_send_one_off_offers_link_to_upload( assert back_link.text.strip() in { "Back to all templates", - "Back to confirm your template", - "Back to select recipients", - "Back to message personalization" + "Back to confirm your template" } assert link.text.strip() == "Upload a list of phone numbers" @@ -1524,8 +1522,6 @@ 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").text.strip() == "name" @@ -2289,12 +2285,10 @@ def test_check_messages_back_link( **extra_args, ) - # assert (page.find_all("a", {"class": "usa-back-link"})[0]["href"]) == expected_url( - # service_id=SERVICE_ONE_ID, template_id=fake_uuid - # ) - actual_href = page.find_all("a", {"class": "usa-back-link"})[0]["href"] expected_href = expected_url(service_id=SERVICE_ONE_ID, template_id=fake_uuid) + + assert actual_href != "#", "Back link href fell back to '#' — missing correct back_link in view" assert actual_href == expected_href @@ -2723,8 +2717,15 @@ def test_preview_notification_shows_preview( template_id=fake_uuid, _expected_status=200, ) + assert page.h1.text.strip() == "Preview for sending" - assert (page.find_all("a", {"class": "usa-back-link"})[0]["href"]) == url_for( + back_link = page.find_all("a", {"class": "usa-back-link"})[0] + assert back_link is not None + + # The real rendered attribute + href = back_link["href"] + + assert href == url_for( "main.check_notification", service_id=service_one["id"], template_id=fake_uuid, From fb20ea6e1c210d4cb2505f8bc80d2f20e96db781 Mon Sep 17 00:00:00 2001 From: Jonathan Bobel Date: Mon, 28 Apr 2025 11:23:00 -0400 Subject: [PATCH 05/10] 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 ba098173e..61e0c0083 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -932,7 +932,7 @@ def _check_notification(service_id, template_id, exception=None, **kwargs): if (not session.get("recipient")) or not all_placeholders_in_session( template.placeholders ): - raise PermanentRedirect(back_link) + raise PermanentRedirect(back_link["href"]["url"]) template.values = get_recipient_and_placeholders_from_session( template.template_type From 7410a005194a460a46f8da9d5365ac8d10e1430c Mon Sep 17 00:00:00 2001 From: Jonathan Bobel Date: Mon, 28 Apr 2025 11:53:01 -0400 Subject: [PATCH 06/10] Update tour.py --- app/main/views/tour.py | 41 +++++++++++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/app/main/views/tour.py b/app/main/views/tour.py index 0e4b5f344..d6a26ea60 100644 --- a/app/main/views/tour.py +++ b/app/main/views/tour.py @@ -139,14 +139,21 @@ def tour_step(service_id, template_id, step_index): def _get_tour_step_back_link(service_id, template_id, step_index): if step_index == 1: - return url_for(".begin_tour", service_id=service_id, template_id=template_id) - - return url_for( - ".tour_step", - service_id=service_id, - template_id=template_id, - step_index=step_index - 1, - ) + return { + "href": { + "url": url_for('main.begin_tour', service_id=service_id, template_id=template_id), + "text": "Back to tour start" + }, + "html": "Back to tour start" + } + else: + return { + "href": { + "url": url_for('main.tour_step', service_id=service_id, template_id=template_id, step_index=step_index-1), + "text": "Back to previous step" + }, + "html": "Back to previous step" + } @main.route( @@ -183,12 +190,18 @@ def check_tour_notification(service_id, template_id): ) ) - back_link = url_for( - ".tour_step", - service_id=current_service.id, - template_id=template_id, - step_index=len(placeholders), - ) + back_link = { + "href": { + "url": url_for( + "main.tour_step", + service_id=current_service.id, + template_id=template_id, + step_index=len(placeholders), + ), + "text": "Back to previous step" + }, + "html": "Back to previous step" + } template.values = get_recipient_and_placeholders_from_session( template.template_type From e04806b1eb51fc8de24395d55c14826811515fd0 Mon Sep 17 00:00:00 2001 From: Jonathan Bobel Date: Mon, 28 Apr 2025 11:59:48 -0400 Subject: [PATCH 07/10] Update tour.py Formatting --- app/main/views/tour.py | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/app/main/views/tour.py b/app/main/views/tour.py index d6a26ea60..57ab12125 100644 --- a/app/main/views/tour.py +++ b/app/main/views/tour.py @@ -141,19 +141,27 @@ def _get_tour_step_back_link(service_id, template_id, step_index): if step_index == 1: return { "href": { - "url": url_for('main.begin_tour', service_id=service_id, template_id=template_id), + "url": url_for('main.begin_tour', + service_id=service_id, + template_id=template_id + ), "text": "Back to tour start" }, "html": "Back to tour start" } else: return { - "href": { - "url": url_for('main.tour_step', service_id=service_id, template_id=template_id, step_index=step_index-1), - "text": "Back to previous step" - }, - "html": "Back to previous step" - } + "href": { + "url": url_for( + "main.tour_step", + service_id=service_id, + template_id=template_id, + step_index=step_index - 1, + ), + "text": "Back to previous step", + }, + "html": "Back to previous step", + } @main.route( From bf03391b6d6182d30f3114370f70e489b20a83b6 Mon Sep 17 00:00:00 2001 From: Jonathan Bobel Date: Mon, 28 Apr 2025 12:17:28 -0400 Subject: [PATCH 08/10] Update tour.py --- app/main/views/tour.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/app/main/views/tour.py b/app/main/views/tour.py index 57ab12125..31bb16803 100644 --- a/app/main/views/tour.py +++ b/app/main/views/tour.py @@ -151,17 +151,17 @@ def _get_tour_step_back_link(service_id, template_id, step_index): } else: return { - "href": { - "url": url_for( - "main.tour_step", - service_id=service_id, - template_id=template_id, - step_index=step_index - 1, - ), - "text": "Back to previous step", - }, - "html": "Back to previous step", - } + "href": { + "url": url_for( + "main.tour_step", + service_id=service_id, + template_id=template_id, + step_index=step_index - 1, + ), + "text": "Back to previous step", + }, + "html": "Back to previous step", + } @main.route( From 6c1648257797bd230613473912d44cc41a17e2d6 Mon Sep 17 00:00:00 2001 From: Jonathan Bobel Date: Mon, 28 Apr 2025 13:00:31 -0400 Subject: [PATCH 09/10] Update tour.py --- app/main/views/tour.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/main/views/tour.py b/app/main/views/tour.py index 31bb16803..e253167c8 100644 --- a/app/main/views/tour.py +++ b/app/main/views/tour.py @@ -141,7 +141,8 @@ def _get_tour_step_back_link(service_id, template_id, step_index): if step_index == 1: return { "href": { - "url": url_for('main.begin_tour', + "url": url_for( + 'main.begin_tour', service_id=service_id, template_id=template_id ), From 4279467b164448de2643d33bb6824a560812db7b Mon Sep 17 00:00:00 2001 From: Jonathan Bobel Date: Tue, 29 Apr 2025 12:27:01 -0400 Subject: [PATCH 10/10] Update preview.html Missed some test code --- app/templates/views/check/preview.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/templates/views/check/preview.html b/app/templates/views/check/preview.html index 0d8d65f98..8a1e6189d 100644 --- a/app/templates/views/check/preview.html +++ b/app/templates/views/check/preview.html @@ -13,7 +13,7 @@ {% block backLink %} - {{ usaBackLink(www.google.com) }} + {{ usaBackLink(back_link_from_preview) }} {% endblock %} {% block maincolumn_content %}