Skip to content

Commit

Permalink
feat: show new notification status when international sends fails (#2079
Browse files Browse the repository at this point in the history
)

* 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
  • Loading branch information
whabanks authored Feb 25, 2025
1 parent 1351852 commit c3e6559
Show file tree
Hide file tree
Showing 9 changed files with 94 additions and 32 deletions.
13 changes: 12 additions & 1 deletion app/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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"),
Expand All @@ -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"),
Expand Down
2 changes: 1 addition & 1 deletion app/main/views/service_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.<br>" "Learn more in the <a href='{}'>API documentation</a>."
"This feature is only available when sending through the API.<br>Learn more in the <a href='{}'>API documentation</a>."
).format(documentation_url("send", section="sending-a-file-by-email"))

if form.validate_on_submit():
Expand Down
2 changes: 1 addition & 1 deletion app/templates/components/table.html
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@
{% if notification.status|format_notification_status_as_url(notification.notification_type) %}
<a href="{{ 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) %}
</a>
{% endif %}
Expand Down
2 changes: 1 addition & 1 deletion app/templates/partials/notifications/status.html
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<a href="{{ 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.template.template_type, notification.provider_response, notification.feedback_subtype, notification.feedback_reason
) }}
{% if notification.status|format_notification_status_as_url(notification.notification_type) %}
</a>
Expand Down
1 change: 1 addition & 0 deletions app/translations/csv/fr.csv
Original file line number Diff line number Diff line change
Expand Up @@ -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"
2 changes: 2 additions & 0 deletions tests/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
55 changes: 39 additions & 16 deletions tests/app/main/views/test_activity.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
),
Expand All @@ -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(
Expand Down
45 changes: 33 additions & 12 deletions tests/app/main/views/test_notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -83,6 +102,7 @@ def test_notification_status_page_shows_details_new_statuses(
key_type,
notification_status,
provider_response,
feedback_reason,
expected_status,
app_,
):
Expand All @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -4807,6 +4807,7 @@ def create_notification(
personalisation=None,
content=None,
notification_provider_response=None,
feedback_reason=None,
):
noti = notification_json(
service_id,
Expand All @@ -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
Expand Down Expand Up @@ -4846,6 +4848,7 @@ def create_notifications(
template_type="sms",
rows=5,
status=None,
feedback_reason=None,
subject="subject",
content="content",
client_reference=None,
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit c3e6559

Please sign in to comment.