From c3e6559ccf814ca704a89ca45ada9fd5a7e95d29 Mon Sep 17 00:00:00 2001
From: William B <7444334+whabanks@users.noreply.github.com>
Date: Tue, 25 Feb 2025 10:16:14 -0400
Subject: [PATCH] feat: show new notification status when international sends
fails (#2079)
* feat: show new notification status when international sends fails
* Add missing translation
* Rename pinpoint-failure status to provider-failure
* Update fr.csv
* Update fr.csv
* Update content strings when formatting sms statuses
* Fix code that should not be there yet somehow is
---
app/__init__.py | 13 ++++-
app/main/views/service_settings.py | 2 +-
app/templates/components/table.html | 2 +-
.../partials/notifications/status.html | 2 +-
app/translations/csv/fr.csv | 1 +
tests/__init__.py | 2 +
tests/app/main/views/test_activity.py | 55 +++++++++++++------
tests/app/main/views/test_notifications.py | 45 +++++++++++----
tests/conftest.py | 4 ++
9 files changed, 94 insertions(+), 32 deletions(-)
diff --git a/app/__init__.py b/app/__init__.py
index 976a1eea93..dcdbe1f8b8 100644
--- a/app/__init__.py
+++ b/app/__init__.py
@@ -442,7 +442,7 @@ def format_notification_type(notification_type):
return {"email": "Email", "sms": "SMS", "letter": "Letter"}[notification_type]
-def format_notification_status(status, template_type, provider_response=None, feedback_subtype=None):
+def format_notification_status(status, template_type, provider_response=None, feedback_subtype=None, feedback_reason=None):
if template_type == "sms" and provider_response:
return _(provider_response)
@@ -458,6 +458,16 @@ def _getStatusByBounceSubtype():
else:
return _("No such address")
+ def _get_sms_status_by_feedback_reason():
+ """Return the status of a notification based on the feedback reason"""
+ if feedback_reason:
+ return {
+ "NO_ORIGINATION_IDENTITIES_FOUND": _("GC Notify cannot send text messages to some international numbers"),
+ "DESTINATION_COUNTRY_BLOCKED": _("GC Notify cannot send text messages to some international numbers"),
+ }.get(feedback_reason, _("No such number"))
+ else:
+ return _("No such number")
+
return {
"email": {
"failed": _("Failed"),
@@ -478,6 +488,7 @@ def _getStatusByBounceSubtype():
"technical-failure": _("Tech issue"),
"temporary-failure": _("Carrier issue"),
"permanent-failure": _("No such number"),
+ "provider-failure": _get_sms_status_by_feedback_reason(),
"delivered": _("Delivered"),
"sending": _("In transit"),
"created": _("In transit"),
diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py
index 4f8906c111..7be44987a3 100644
--- a/app/main/views/service_settings.py
+++ b/app/main/views/service_settings.py
@@ -425,7 +425,7 @@ def service_switch_upload_document(service_id):
title = _("Send files by email")
form = ServiceOnOffSettingForm(name=title, enabled=current_service.has_permission("upload_document"))
help = _(
- "This feature is only available when sending through the API.
" "Learn more in the API documentation."
+ "This feature is only available when sending through the API.
Learn more in the API documentation."
).format(documentation_url("send", section="sending-a-file-by-email"))
if form.validate_on_submit():
diff --git a/app/templates/components/table.html b/app/templates/components/table.html
index 46a608cb57..cdf0702f1e 100644
--- a/app/templates/components/table.html
+++ b/app/templates/components/table.html
@@ -181,7 +181,7 @@
{% if notification.status|format_notification_status_as_url(notification.notification_type) %}
{% endif %}
- {{ notification.status|format_notification_status(notification.template.template_type, notification.provider_response, notification.feedback_subtype) }}
+ {{ notification.status|format_notification_status(notification.template.template_type, notification.provider_response, notification.feedback_subtype, notification.feedback_reason) }}
{% if notification.status|format_notification_status_as_url(notification.notification_type) %}
{% endif %}
diff --git a/app/templates/partials/notifications/status.html b/app/templates/partials/notifications/status.html
index 0a79285dd7..9c47c8d2ba 100644
--- a/app/templates/partials/notifications/status.html
+++ b/app/templates/partials/notifications/status.html
@@ -4,7 +4,7 @@
{% endif %}
{{ notification.status|format_notification_status(
- notification.template.template_type, notification.provider_response, notification.feedback_subtype
+ notification.template.template_type, notification.provider_response, notification.feedback_subtype, notification.feedback_reason
) }}
{% if notification.status|format_notification_status_as_url(notification.notification_type) %}
diff --git a/app/translations/csv/fr.csv b/app/translations/csv/fr.csv
index f6311d7608..39d7089f06 100644
--- a/app/translations/csv/fr.csv
+++ b/app/translations/csv/fr.csv
@@ -2122,3 +2122,4 @@
"Ask which team members have permission to invite you. If the team is unsure, from a GC Notify account visit the main menu and select “Team members.” That page:","Demandez quels ou quelles membres de l’équipe ont la permission de vous inviter. Si l’équipe n’en est pas certaine, utilisez un compte Notification GC pour visiter le menu principal et rendez-vous dans la section « Votre équipe ». Cette page comprend :"
"Includes an invitation button at the top of the page, if the member is permitted to send invitations.","un bouton d’invitation au haut de la page si le ou la membre de l’équipe a la permission d’envoyer des invitations; et"
"Lists permitted tasks under the name of each member.","sous le nom de chaque membre de l’équipe, une liste des tâches que cette personne a l’autorisation de réaliser."
+"GC Notify cannot send text messages to some international numbers","Il y a des numéros internationaux auxquels Notification GC ne peut pas envoyer de messages texte"
diff --git a/tests/__init__.py b/tests/__init__.py
index 269238816f..b8d6e19d92 100644
--- a/tests/__init__.py
+++ b/tests/__init__.py
@@ -493,6 +493,7 @@ def notification_json(
to=None,
status=None,
provider_response=None,
+ feedback_reason=None,
sent_at=None,
job_row_number=None,
created_at=None,
@@ -550,6 +551,7 @@ def notification_json(
"sent_at": sent_at,
"status": status,
"provider_response": provider_response,
+ "feedback_reason": feedback_reason,
"created_at": created_at,
"created_by": None,
"updated_at": updated_at,
diff --git a/tests/app/main/views/test_activity.py b/tests/app/main/views/test_activity.py
index 7a5068a515..4769e8376c 100644
--- a/tests/app/main/views/test_activity.py
+++ b/tests/app/main/views/test_activity.py
@@ -610,67 +610,89 @@ def test_big_numbers_and_search_dont_show_for_letters(
@freeze_time("2017-09-27 16:30:00.000000")
@pytest.mark.parametrize(
- "message_type, status, expected_hint_status, single_line",
+ "message_type, status, feedback_reason, expected_hint_status, single_line",
[
- ("email", "created", "In transit since 2017-09-27T16:30:00+00:00", True),
- ("email", "sending", "In transit since 2017-09-27T16:30:00+00:00", True),
+ ("email", "created", None, "In transit since 2017-09-27T16:30:00+00:00", True),
+ ("email", "sending", None, "In transit since 2017-09-27T16:30:00+00:00", True),
(
"email",
"temporary-failure",
+ None,
"Content or inbox issue 16:31:00",
False,
),
- ("email", "permanent-failure", "No such address 16:31:00", False),
- ("email", "delivered", "Delivered 16:31:00", True),
- ("sms", "created", "In transit since 2017-09-27T16:30:00+00:00", True),
- ("sms", "sending", "In transit since 2017-09-27T16:30:00+00:00", True),
+ ("email", "permanent-failure", None, "No such address 16:31:00", False),
+ ("email", "delivered", None, "Delivered 16:31:00", True),
+ ("sms", "created", None, "In transit since 2017-09-27T16:30:00+00:00", True),
+ ("sms", "sending", None, "In transit since 2017-09-27T16:30:00+00:00", True),
(
"sms",
"temporary-failure",
+ None,
"Carrier issue 16:31:00",
False,
),
- ("sms", "permanent-failure", "No such number 16:31:00", False),
- ("sms", "delivered", "Delivered 16:31:00", True),
- ("letter", "created", "2017-09-27T16:30:00+00:00", True),
- ("letter", "pending-virus-check", "2017-09-27T16:30:00+00:00", True),
- ("letter", "sending", "2017-09-27T16:30:00+00:00", True),
- ("letter", "delivered", "2017-09-27T16:30:00+00:00", True),
- ("letter", "received", "2017-09-27T16:30:00+00:00", True),
- ("letter", "accepted", "2017-09-27T16:30:00+00:00", True),
+ ("sms", "permanent-failure", None, "No such number 16:31:00", False),
+ (
+ "sms",
+ "provider-failure",
+ "DESTINATION_COUNTRY_BLOCKED",
+ "GC Notify cannot send text messages to some international numbers 16:31:00",
+ False,
+ ),
+ (
+ "sms",
+ "provider-failure",
+ "NO_ORIGINATION_IDENTITIES_FOUND",
+ "GC Notify cannot send text messages to some international numbers 16:31:00",
+ False,
+ ),
+ ("sms", "delivered", None, "Delivered 16:31:00", True),
+ ("letter", "created", None, "2017-09-27T16:30:00+00:00", True),
+ ("letter", "pending-virus-check", None, "2017-09-27T16:30:00+00:00", True),
+ ("letter", "sending", None, "2017-09-27T16:30:00+00:00", True),
+ ("letter", "delivered", None, "2017-09-27T16:30:00+00:00", True),
+ ("letter", "received", None, "2017-09-27T16:30:00+00:00", True),
+ ("letter", "accepted", None, "2017-09-27T16:30:00+00:00", True),
(
"letter",
"cancelled",
+ None,
"2017-09-27T16:30:00+00:00",
False,
), # The API won’t return cancelled letters
(
"letter",
"permanent-failure",
+ None,
"16:31:00",
False,
), # Deprecated for ‘cancelled’
(
"letter",
"temporary-failure",
+ None,
"2017-09-27T16:30:00+00:00",
False,
), # Not currently a real letter status
(
"letter",
"virus-scan-failed",
+ None,
"Virus detected 2017-09-27T16:30:00+00:00",
False,
),
(
"letter",
"validation-failed",
+ None,
"Validation failed 2017-09-27T16:30:00+00:00",
False,
),
(
"letter",
"technical-failure",
+ None,
"Technical failure 2017-09-27T16:30:00+00:00",
False,
),
@@ -684,12 +706,13 @@ def test_sending_status_hint_displays_correctly_on_notifications_page_new_status
mock_get_service_data_retention,
message_type,
status,
+ feedback_reason,
expected_hint_status,
single_line,
mocker,
app_,
):
- notifications = create_notifications(template_type=message_type, status=status)
+ notifications = create_notifications(template_type=message_type, feedback_reason=feedback_reason, status=status)
mocker.patch("app.notification_api_client.get_notifications_for_service", return_value=notifications)
page = client_request.get(
diff --git a/tests/app/main/views/test_notifications.py b/tests/app/main/views/test_notifications.py
index 16110de5e2..38ffb4fdf7 100644
--- a/tests/app/main/views/test_notifications.py
+++ b/tests/app/main/views/test_notifications.py
@@ -20,49 +20,68 @@
@pytest.mark.parametrize(
- "key_type, notification_status, provider_response, expected_status",
+ "key_type, notification_status, provider_response, feedback_reason, expected_status",
[
- (None, "created", None, "In transit"),
- (None, "sending", None, "In transit"),
- (None, "delivered", None, "Delivered"),
- (None, "failed", None, "Failed"),
+ (None, "created", None, None, "In transit"),
+ (None, "sending", None, None, "In transit"),
+ (None, "delivered", None, None, "Delivered"),
+ (None, "failed", None, None, "Failed"),
(
None,
"temporary-failure",
None,
+ None,
"Carrier issue",
),
- (None, "permanent-failure", None, "No such number"),
- (None, "technical-failure", None, "Tech issue"),
+ (None, "permanent-failure", None, None, "No such number"),
+ (None, "technical-failure", None, None, "Tech issue"),
(
None,
"technical-failure",
"Blocked as spam by phone carrier",
+ None,
"Blocked as spam by phone carrier",
),
(
None,
"permanent-failure",
"The email address is on the GC Notify suppression list",
+ None,
"The email address is on the GC Notify suppression list",
),
(
None,
"permanent-failure",
"Email address is on our email provider suppression list",
+ None,
"Email address is on our email provider suppression list",
),
+ (
+ None,
+ "provider-failure",
+ None,
+ "NO_ORIGINATION_IDENTITIES_FOUND",
+ "GC Notify cannot send text messages to some international numbers",
+ ),
+ (
+ None,
+ "provider-failure",
+ None,
+ "DESTINATION_COUNTRY_BLOCKED",
+ "GC Notify cannot send text messages to some international numbers",
+ ),
(
None,
"temporary-failure",
"Email was rejected because of its attachments",
+ None,
"Email was rejected because of its attachments",
),
- ("team", "delivered", None, "Delivered"),
- ("live", "delivered", None, "Delivered"),
- ("test", "sending", None, "In transit (test)"),
- ("test", "delivered", None, "Delivered (test)"),
- ("test", "permanent-failure", None, "No such number (test)"),
+ ("team", "delivered", None, None, "Delivered"),
+ ("live", "delivered", None, None, "Delivered"),
+ ("test", "sending", None, None, "In transit (test)"),
+ ("test", "delivered", None, None, "Delivered (test)"),
+ ("test", "permanent-failure", None, None, "No such number (test)"),
],
)
@pytest.mark.parametrize(
@@ -83,6 +102,7 @@ def test_notification_status_page_shows_details_new_statuses(
key_type,
notification_status,
provider_response,
+ feedback_reason,
expected_status,
app_,
):
@@ -91,6 +111,7 @@ def test_notification_status_page_shows_details_new_statuses(
notification = create_notification(
notification_status=notification_status,
notification_provider_response=provider_response,
+ feedback_reason=feedback_reason,
key_type=key_type,
)
_mock_get_notification = mocker.patch("app.notification_api_client.get_notification", return_value=notification)
diff --git a/tests/conftest.py b/tests/conftest.py
index 097e13bb15..4a84ec77b5 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -4807,6 +4807,7 @@ def create_notification(
personalisation=None,
content=None,
notification_provider_response=None,
+ feedback_reason=None,
):
noti = notification_json(
service_id,
@@ -4816,6 +4817,7 @@ def create_notification(
postage=postage,
reply_to_text=reply_to_text,
provider_response=notification_provider_response,
+ feedback_reason=feedback_reason,
)["notifications"][0]
noti_content = "hello ((name))" if content is None else content
@@ -4846,6 +4848,7 @@ def create_notifications(
template_type="sms",
rows=5,
status=None,
+ feedback_reason=None,
subject="subject",
content="content",
client_reference=None,
@@ -4873,6 +4876,7 @@ def create_notifications(
template_type=template_type,
client_reference=client_reference,
status=status,
+ feedback_reason=feedback_reason,
created_by_name="Firstname Lastname",
postage=postage,
to=to,