From 1eb677c61cc1be05c8e680760883c0255e22a286 Mon Sep 17 00:00:00 2001 From: wbanks Date: Wed, 29 May 2024 15:32:04 -0400 Subject: [PATCH 01/22] Add basic validation to callbacks URLs - Added the [validators](https://yozachar.github.io/pyvalidators/stable/) lib to the project - Callback URLs are now checked for validity - Callback URLs are now tested to determine if they are reachable before saving the callback url - Validate that callback urls return 200 responses --- app/main/forms.py | 2 ++ app/main/validators.py | 40 +++++++++++++++++++++++++++++++++++++- app/main/views/api_keys.py | 1 + poetry.lock | 13 ++++++++++++- pyproject.toml | 1 + 5 files changed, 55 insertions(+), 2 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index e4014caa17..c45a9a58e4 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -43,6 +43,7 @@ LettersNumbersAndFullStopsOnly, NoCommasInPlaceHolders, OnlySMSCharacters, + ValidCallbackUrl, ValidEmail, ValidGovEmail, validate_email_from, @@ -1351,6 +1352,7 @@ class ServiceDeliveryStatusCallbackForm(CallbackForm): validators=[ DataRequired(message=_l("This cannot be empty")), Regexp(regex="^https.*", message=_l("Must be a valid https URL")), + ValidCallbackUrl(), ], ) bearer_token = PasswordFieldShowHasContent( diff --git a/app/main/validators.py b/app/main/validators.py index 238e68cc91..c3d358310c 100644 --- a/app/main/validators.py +++ b/app/main/validators.py @@ -2,6 +2,8 @@ import time import pwnedpasswords +import requests +import validators from flask import current_app from flask_babel import _ from flask_babel import lazy_gettext as _l @@ -11,7 +13,7 @@ from wtforms import ValidationError from wtforms.validators import Email -from app import formatted_list, service_api_client +from app import current_service, formatted_list, service_api_client from app.main._blocked_passwords import blocked_passwords from app.utils import Spreadsheet, email_safe, email_safe_name, is_gov_user @@ -141,6 +143,42 @@ def __call__(self, form, field): raise ValidationError(self.message) +class ValidCallbackUrl: + def __init__(self, message="Enter a valid URL"): + self.message = message + + def __call__(self, form, field): + if field.data: + validate_callback_url(field.data, form.bearer_token.data) + + +def validate_callback_url(service_callback_url, bearer_token): + if not validators.url(service_callback_url): + current_app.logger.warning(f"Invalid callback URL for service: {current_service.id}") + raise ValidationError(_l("URL is invalid")) + + try: + response = requests.post( + url=service_callback_url, + allow_redirects=True, + data={"health_check": "true"}, + headers={"Content-Type": "application/json", "Authorization": f"Bearer {bearer_token}"}, + timeout=5, + ) + if not response.status_code == 200: + current_app.logger.warning( + f"Callback URL for service: {current_service.id} was reachable but returned status code {response.status_code}." + ) + raise ValidationError(_l(f"Callback endpoint response was unsuccessful: {response.status_code}")) + except requests.RequestException as e: + current_app.logger.warning(f"Callback URL not reachable for service: {current_service.id}. Exception: {e}") + raise ValidationError( + _l( + f"We were unable to reach {service_callback_url}. Please verify the service is running and can be reached and try again." + ) + ) + + def validate_email_from(form, field): if email_safe(field.data) != field.data.lower(): # fix their data instead of only warning them diff --git a/app/main/views/api_keys.py b/app/main/views/api_keys.py index d3c9a35277..6a018351c3 100644 --- a/app/main/views/api_keys.py +++ b/app/main/views/api_keys.py @@ -21,6 +21,7 @@ KEY_TYPE_TEST, ) from app.utils import documentation_url, email_safe, user_has_permissions +import validators dummy_bearer_token = "bearer_token_set" diff --git a/poetry.lock b/poetry.lock index dabbf80d30..dcfe881d2a 100644 --- a/poetry.lock +++ b/poetry.lock @@ -2341,6 +2341,17 @@ files = [ [package.dependencies] ua-parser = ">=0.10.0" +[[package]] +name = "validators" +version = "0.28.3" +description = "Python Data Validation for Humans™" +optional = false +python-versions = ">=3.8" +files = [ + {file = "validators-0.28.3-py3-none-any.whl", hash = "sha256:53cafa854f13850156259d9cc479b864ee901f6a96e6b109e6fc33f98f37d99f"}, + {file = "validators-0.28.3.tar.gz", hash = "sha256:c6c79840bcde9ba77b19f6218f7738188115e27830cbaff43264bc4ed24c429d"}, +] + [[package]] name = "webencodings" version = "0.5.1" @@ -2476,4 +2487,4 @@ testing = ["coverage (>=5.0.3)", "zope.event", "zope.testing"] [metadata] lock-version = "2.0" python-versions = "~3.10.9" -content-hash = "ddc978fe2f5c973e21db582949903fa3ba76a5f05f1ef363e4d2c4e4353e4aa1" +content-hash = "0dc6a85b517860088c4ef6882e9b27e5b5667a15804c0da071ebe833f4e1c8f8" diff --git a/pyproject.toml b/pyproject.toml index 9d9d8639ea..a14510b465 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -64,6 +64,7 @@ xlrd = "1.2.0" # this is pinned so that we can continue to support .xslm files; itsdangerous = "2.1.2" # pyup: <1.0.0 newrelic = "8.10.0" # Pinned to 8.10.0 for now, 8.11.0 caused a performance regression: https://gcdigital.slack.com/archives/C012W5K734Y/p1709668046344929 +validators = "^0.28.3" [tool.poetry.group.test.dependencies] isort = "5.13.2" pytest = "7.4.4" From 36fc3d3969ccaab400c5b5a77e9f7e6f933c4c85 Mon Sep 17 00:00:00 2001 From: wbanks Date: Thu, 30 May 2024 14:13:02 -0400 Subject: [PATCH 02/22] Added dynamic hint text to the url field --- app/main/validators.py | 6 +++--- app/main/views/api_keys.py | 8 ++++++++ .../views/api/callbacks/delivery-status-callback.html | 3 +-- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/app/main/validators.py b/app/main/validators.py index c3d358310c..94a68370eb 100644 --- a/app/main/validators.py +++ b/app/main/validators.py @@ -151,7 +151,7 @@ def __call__(self, form, field): if field.data: validate_callback_url(field.data, form.bearer_token.data) - + def validate_callback_url(service_callback_url, bearer_token): if not validators.url(service_callback_url): current_app.logger.warning(f"Invalid callback URL for service: {current_service.id}") @@ -169,12 +169,12 @@ def validate_callback_url(service_callback_url, bearer_token): current_app.logger.warning( f"Callback URL for service: {current_service.id} was reachable but returned status code {response.status_code}." ) - raise ValidationError(_l(f"Callback endpoint response was unsuccessful: {response.status_code}")) + raise ValidationError(_l(f"We were able to reach your callback service but it's response indicated a failure code: {response.status_code}")) except requests.RequestException as e: current_app.logger.warning(f"Callback URL not reachable for service: {current_service.id}. Exception: {e}") raise ValidationError( _l( - f"We were unable to reach {service_callback_url}. Please verify the service is running and can be reached and try again." + f"We were unable to reach your callback service. Please verify that the service is running and can be reached and try again." ) ) diff --git a/app/main/views/api_keys.py b/app/main/views/api_keys.py index 6a018351c3..2c36ea6c8e 100644 --- a/app/main/views/api_keys.py +++ b/app/main/views/api_keys.py @@ -180,6 +180,7 @@ def get_delivery_status_callback_details(): def delivery_status_callback(service_id): delivery_status_callback = get_delivery_status_callback_details() back_link = ".api_callbacks" if current_service.has_permission("inbound_sms") else ".api_integration" + url_hint_txt = "Must start with https://" form = ServiceDeliveryStatusCallbackForm( url=delivery_status_callback.get("url") if delivery_status_callback else "", @@ -215,10 +216,17 @@ def delivery_status_callback(service_id): pass return redirect(url_for(back_link, service_id=service_id)) + elif form.errors: + url_error = form.errors["url"][0] + if "response indicated a failure code:" in url_error: + url_hint_txt = "Your service must be running. Try checking your service's logs for errors." + elif "unable to reach your callback service" in url_error: + url_hint_txt = "Your service must be running and reachable from the internet." return render_template( "views/api/callbacks/delivery-status-callback.html", back_link=back_link, + hint_text=url_hint_txt, form=form, ) diff --git a/app/templates/views/api/callbacks/delivery-status-callback.html b/app/templates/views/api/callbacks/delivery-status-callback.html index ed20b80b18..2b346d5963 100644 --- a/app/templates/views/api/callbacks/delivery-status-callback.html +++ b/app/templates/views/api/callbacks/delivery-status-callback.html @@ -23,12 +23,11 @@

{{ _('Add an endpoint where GC Notify should send POST requests') }}

- {% set hint_txt = _('Must start with https://') %} {% call form_wrapper() %} {{ textbox( form.url, width='w-full', - hint=hint_txt + hint=hint_text ) }}

{{ _('Add a secret value for authentication') }} From 17edaeb0b90debacd01eacae831a2ea14857b768 Mon Sep 17 00:00:00 2001 From: wbanks Date: Fri, 7 Jun 2024 17:19:50 -0400 Subject: [PATCH 03/22] Various fixes - Update error messages - Add logic for catching 5xx and 4xx errors and raise the correct validation error + logging - Fixed ServiceReceiveCallbackMessagesForm validation --- app/main/forms.py | 5 ++-- app/main/validators.py | 30 +++++++++++-------- app/main/views/api_keys.py | 13 ++++---- .../received-text-messages-callback.html | 4 +-- tests/app/main/views/test_api_integration.py | 26 ++++++++++++---- 5 files changed, 50 insertions(+), 28 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index c45a9a58e4..b5fd763aa1 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -1334,7 +1334,8 @@ class ServiceReceiveMessagesCallbackForm(CallbackForm): "URL", validators=[ DataRequired(message=_l("This cannot be empty")), - Regexp(regex="^https.*", message=_l("Must be a valid https URL")), + Regexp(regex="^https.*", message=_l("Enter a URL that starts with https://")), + ValidCallbackUrl(), ], ) bearer_token = PasswordFieldShowHasContent( @@ -1351,7 +1352,7 @@ class ServiceDeliveryStatusCallbackForm(CallbackForm): "URL", validators=[ DataRequired(message=_l("This cannot be empty")), - Regexp(regex="^https.*", message=_l("Must be a valid https URL")), + Regexp(regex="^https.*", message=_l("Enter a URL that starts with https://")), ValidCallbackUrl(), ], ) diff --git a/app/main/validators.py b/app/main/validators.py index 94a68370eb..3ccfa30de1 100644 --- a/app/main/validators.py +++ b/app/main/validators.py @@ -144,18 +144,20 @@ def __call__(self, form, field): class ValidCallbackUrl: - def __init__(self, message="Enter a valid URL"): + def __init__(self, message="Enter a URL that starts with https://"): self.message = message def __call__(self, form, field): if field.data: validate_callback_url(field.data, form.bearer_token.data) - + def validate_callback_url(service_callback_url, bearer_token): if not validators.url(service_callback_url): - current_app.logger.warning(f"Invalid callback URL for service: {current_service.id}") - raise ValidationError(_l("URL is invalid")) + current_app.logger.warning( + f"Unable to create callback for service: {current_service.id}. Error: Invalid callback URL format: URL: {service_callback_url}" + ) + raise ValidationError(_l("Enter a URL that starts with https://")) try: response = requests.post( @@ -165,18 +167,22 @@ def validate_callback_url(service_callback_url, bearer_token): headers={"Content-Type": "application/json", "Authorization": f"Bearer {bearer_token}"}, timeout=5, ) - if not response.status_code == 200: + + if response.status_code >= 500: current_app.logger.warning( - f"Callback URL for service: {current_service.id} was reachable but returned status code {response.status_code}." + f"Unable to create callback for service: {current_service.id} Error: URL was reachable but returned status code {response.status_code} URL: {service_callback_url}" ) - raise ValidationError(_l(f"We were able to reach your callback service but it's response indicated a failure code: {response.status_code}")) - except requests.RequestException as e: - current_app.logger.warning(f"Callback URL not reachable for service: {current_service.id}. Exception: {e}") - raise ValidationError( - _l( - f"We were unable to reach your callback service. Please verify that the service is running and can be reached and try again." + raise ValidationError(_l("Check your service log for errors")) + elif response.status_code < 500 and response.status_code >= 400: + current_app.logger.warning( + f"Unable to create callback for service: {current_service.id} Error: Callback URL not reachable URL: {service_callback_url}" ) + raise ValidationError(_l("Check your service is running and not using a proxy we cannot access")) + except requests.RequestException as e: + current_app.logger.warning( + f"Unable to create callback for service: {current_service.id} Error: Callback URL not reachable URL: {service_callback_url} Exception: {e}" ) + raise ValidationError(_l("Check your service is running and not using a proxy we cannot access")) def validate_email_from(form, field): diff --git a/app/main/views/api_keys.py b/app/main/views/api_keys.py index 2c36ea6c8e..b93fe1bad8 100644 --- a/app/main/views/api_keys.py +++ b/app/main/views/api_keys.py @@ -21,7 +21,6 @@ KEY_TYPE_TEST, ) from app.utils import documentation_url, email_safe, user_has_permissions -import validators dummy_bearer_token = "bearer_token_set" @@ -217,11 +216,7 @@ def delivery_status_callback(service_id): return redirect(url_for(back_link, service_id=service_id)) elif form.errors: - url_error = form.errors["url"][0] - if "response indicated a failure code:" in url_error: - url_hint_txt = "Your service must be running. Try checking your service's logs for errors." - elif "unable to reach your callback service" in url_error: - url_hint_txt = "Your service must be running and reachable from the internet." + url_hint_txt = "Your service must be running and reachable from the internet." return render_template( "views/api/callbacks/delivery-status-callback.html", @@ -250,6 +245,7 @@ def received_text_messages_callback(service_id): url=received_text_messages_callback.get("url") if received_text_messages_callback else "", bearer_token=dummy_bearer_token if received_text_messages_callback else "", ) + url_hint_txt = "Must start with https://" if form.validate_on_submit(): if received_text_messages_callback and form.url.data: @@ -273,8 +269,11 @@ def received_text_messages_callback(service_id): bearer_token=form.bearer_token.data, user_id=current_user.id, ) - return redirect(url_for(".api_callbacks", service_id=service_id)) + return redirect(url_for(".api_callbacks", service_id=service_id)) + elif form.errors: + url_hint_txt = "Your service must be running and reachable from the internet." return render_template( "views/api/callbacks/received-text-messages-callback.html", form=form, + hint_text=url_hint_txt, ) diff --git a/app/templates/views/api/callbacks/received-text-messages-callback.html b/app/templates/views/api/callbacks/received-text-messages-callback.html index 488dad293f..4d68a923d3 100644 --- a/app/templates/views/api/callbacks/received-text-messages-callback.html +++ b/app/templates/views/api/callbacks/received-text-messages-callback.html @@ -18,12 +18,12 @@

{{ _('When you receive a text message in GC Notify, we can forward it to your system. Learn more in the') }} {{ _('callbacks documentation') }} .

- {% set hint_txt = _('Must start with https://') %} + {% call form_wrapper() %} {{ textbox( form.url, width='w-full', - hint=hint_txt + hint=hint_text ) }} {% set hint_txt = _('At least 10 characters') %} {{ textbox( diff --git a/tests/app/main/views/test_api_integration.py b/tests/app/main/views/test_api_integration.py index 4711cad886..292cf7be45 100644 --- a/tests/app/main/views/test_api_integration.py +++ b/tests/app/main/views/test_api_integration.py @@ -1,6 +1,6 @@ import uuid from collections import OrderedDict -from unittest.mock import call +from unittest.mock import Mock, call import pytest from bs4 import BeautifulSoup @@ -507,11 +507,22 @@ def test_should_validate_safelist_items( ], ) @pytest.mark.parametrize( - "url, bearer_token, expected_errors", + "url, bearer_token, response, expected_errors", [ - ("https://example.com", "", "This cannot be empty"), - ("http://not_https.com", "1234567890", "Must be a valid https URL"), - ("https://test.com", "123456789", "Must be at least 10 characters"), + ("https://example.com", "", None, "This cannot be empty"), + ("http://not_https.com", "1234567890", None, "Enter a URL that starts with https://"), + ( + "https://test.com", + "123456789", + {"content": "a", "status_code": 500, "headers": {"a": "a"}}, + "Check your service log for errors Must be at least 10 characters", + ), + ( + "https://test.ee", + "1234567890", + {"content": "a", "status_code": 404, "headers": {"a": "a"}}, + "Check your service is running and not using a proxy we cannot access", + ), ], ) def test_callback_forms_validation( @@ -521,7 +532,9 @@ def test_callback_forms_validation( endpoint, url, bearer_token, + response, expected_errors, + mocker, ): if endpoint == "main.received_text_messages_callback": service_one["permissions"] = ["inbound_sms"] @@ -530,6 +543,9 @@ def test_callback_forms_validation( "url": url, "bearer_token": bearer_token, } + if response: + resp = Mock(content=response["content"], status_code=response["status_code"], headers=response["headers"]) + mocker.patch("app.main.validators.requests.post", return_value=resp) response = client_request.post(endpoint, service_id=service_one["id"], _data=data, _expected_status=200) error_msgs = " ".join(msg.text.strip() for msg in response.select(".error-message")) From a1ae8b2249ff083a4df5eab5b092362b4b0568af Mon Sep 17 00:00:00 2001 From: wbanks Date: Tue, 11 Jun 2024 12:25:09 -0400 Subject: [PATCH 04/22] Fix tests --- app/main/views/api_keys.py | 2 +- tests/app/main/views/test_api_integration.py | 5 +++++ tests/conftest.py | 5 +++++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/app/main/views/api_keys.py b/app/main/views/api_keys.py index b93fe1bad8..5a4edd9f1b 100644 --- a/app/main/views/api_keys.py +++ b/app/main/views/api_keys.py @@ -269,9 +269,9 @@ def received_text_messages_callback(service_id): bearer_token=form.bearer_token.data, user_id=current_user.id, ) - return redirect(url_for(".api_callbacks", service_id=service_id)) elif form.errors: url_hint_txt = "Your service must be running and reachable from the internet." + return redirect(url_for(".api_callbacks", service_id=service_id)) return render_template( "views/api/callbacks/received-text-messages-callback.html", form=form, diff --git a/tests/app/main/views/test_api_integration.py b/tests/app/main/views/test_api_integration.py index 292cf7be45..1c8dc01547 100644 --- a/tests/app/main/views/test_api_integration.py +++ b/tests/app/main/views/test_api_integration.py @@ -723,6 +723,7 @@ def test_create_delivery_status_and_receive_text_message_callbacks( mock_create_service_callback_api, endpoint, fake_uuid, + mock_validate_callback_url, ): if endpoint == "main.received_text_messages_callback": service_one["permissions"] = ["inbound_sms"] @@ -761,6 +762,7 @@ def test_update_delivery_status_callback_details( mock_update_service_callback_api, mock_get_valid_service_callback_api, fake_uuid, + mock_validate_callback_url, ): service_one["service_callback_api"] = [fake_uuid] @@ -787,6 +789,7 @@ def test_update_receive_text_message_callback_details( mock_update_service_inbound_api, mock_get_valid_service_inbound_api, fake_uuid, + mock_validate_callback_url, ): service_one["inbound_api"] = [fake_uuid] service_one["permissions"] = ["inbound_sms"] @@ -814,6 +817,7 @@ def test_update_delivery_status_callback_without_changes_does_not_update( mock_update_service_callback_api, fake_uuid, mock_get_valid_service_callback_api, + mock_validate_callback_url, ): service_one["service_callback_api"] = [fake_uuid] data = {"user_id": fake_uuid, "url": "https://hello2.canada.ca", "bearer_token": "bearer_token_set"} @@ -833,6 +837,7 @@ def test_update_receive_text_message_callback_without_changes_does_not_update( mock_update_service_inbound_api, fake_uuid, mock_get_valid_service_inbound_api, + mock_validate_callback_url, ): service_one["inbound_api"] = [fake_uuid] service_one["permissions"] = ["inbound_sms"] diff --git a/tests/conftest.py b/tests/conftest.py index e7379135cf..c11f803ade 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -3720,6 +3720,11 @@ def mock_get_empty_service_callback_api(mocker): ) +@pytest.fixture(scope="function") +def mock_validate_callback_url(mocker): + return mocker.patch("app.main.validators.requests.post", return_value=Mock(content="a", status_code=200, headers={"a": "a"})) + + @pytest.fixture(scope="function") def mock_create_service_inbound_api(mocker): def _create_service_inbound_api(service_id, url, bearer_token, user_id): From 40c743ab12e91cd143288cbd04ebca310f1e9ee0 Mon Sep 17 00:00:00 2001 From: wbanks Date: Tue, 11 Jun 2024 12:44:21 -0400 Subject: [PATCH 05/22] Add placeholder translations --- app/translations/csv/fr.csv | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/translations/csv/fr.csv b/app/translations/csv/fr.csv index 5d7e45d237..4a5ca08fe8 100644 --- a/app/translations/csv/fr.csv +++ b/app/translations/csv/fr.csv @@ -784,6 +784,7 @@ "When you send an email or text message, find out if GC Notify was able to deliver it with callbacks. Learn more in the callbacks documentation.","Lorsque vous envoyez un courriel ou un message texte, vérifiez si Notification GC a réussi à le livrer avec des fonctions de rappel. Pour en savoir plus, consultez la documentation sur les fonctions de rappel." "Add an endpoint where GC Notify should send POST requests","Ajouter un point de terminaison où Notification GC peut envoyer les requêtes ‘POST’" "Must start with https://","Doit commencer par https://" +"Enter a URL that starts with https://","Entrez une URL qui commence par https://" "Add a secret value for authentication","Ajouter une valeur secrète pour l’authentification" "At least 10 characters","Au moins 10 caractères" "Callbacks for received text messages","Fonctions de rappel pour les messages texte reçus" @@ -1890,4 +1891,6 @@ "You have been signed out.","Déconnexion réussie" "Your session ends after 8 hours of inactivity","Votre session se termine au bout de 8 heures d’inactivité" "total sends in the last 7 days","total des envois au cours des 7 derniers jours" -"Getting started","Découvrez comment fonctionne Notification GC" \ No newline at end of file +"Getting started","Découvrez comment fonctionne Notification GC" +"Check your service log for errors","Vérifiez le journal de service pour les erreurs" +"Check your service is running and not using a proxy we cannot access","Vérifiez que votre service est en cours d'exécution et n'utilise pas un proxy auquel nous ne pouvons pas accéder" From 0046a7de5a84e4f3320d15f90b8c4e524cc252bc Mon Sep 17 00:00:00 2001 From: wbanks Date: Mon, 12 Aug 2024 11:52:08 -0400 Subject: [PATCH 06/22] Consider 5xx responses as valid - We only want to verify that there's a service running at the specified callback URL. - Additionally we are sending a payload the service won't understand how to response to so a 5xx is likely --- app/main/validators.py | 18 ++++++++++++------ app/translations/csv/fr.csv | 1 - 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/app/main/validators.py b/app/main/validators.py index 3ccfa30de1..84c7ffd3fc 100644 --- a/app/main/validators.py +++ b/app/main/validators.py @@ -153,6 +153,17 @@ def __call__(self, form, field): def validate_callback_url(service_callback_url, bearer_token): + """Validates a callback URL, checking that it is https and by sending a POST request to the URL with a health_check parameter. + 4xx responses are considered invalid. 5xx responses are considered valid as it indicates there is at least a service running + at the URL, and we are sending a payload that the service will not understand. + + Args: + service_callback_url (str): The url to validate. + bearer_token (str): The bearer token to use in the request, specified by the user requesting callbacks. + + Raises: + ValidationError: If the URL is not HTTPS or the http response is 4xx. + """ if not validators.url(service_callback_url): current_app.logger.warning( f"Unable to create callback for service: {current_service.id}. Error: Invalid callback URL format: URL: {service_callback_url}" @@ -168,12 +179,7 @@ def validate_callback_url(service_callback_url, bearer_token): timeout=5, ) - if response.status_code >= 500: - current_app.logger.warning( - f"Unable to create callback for service: {current_service.id} Error: URL was reachable but returned status code {response.status_code} URL: {service_callback_url}" - ) - raise ValidationError(_l("Check your service log for errors")) - elif response.status_code < 500 and response.status_code >= 400: + if response.status_code < 500 and response.status_code >= 400: current_app.logger.warning( f"Unable to create callback for service: {current_service.id} Error: Callback URL not reachable URL: {service_callback_url}" ) diff --git a/app/translations/csv/fr.csv b/app/translations/csv/fr.csv index 7a0721173b..27d5437b20 100644 --- a/app/translations/csv/fr.csv +++ b/app/translations/csv/fr.csv @@ -1897,7 +1897,6 @@ "Your session ends after 8 hours of inactivity","Votre session se termine au bout de 8 heures d’inactivité" "total sends in the last 7 days","total des envois au cours des 7 derniers jours" "Getting started","Découvrez comment fonctionne Notification GC" -"Check your service log for errors","Vérifiez le journal de service pour les erreurs" "Check your service is running and not using a proxy we cannot access","Vérifiez que votre service est en cours d'exécution et n'utilise pas un proxy auquel nous ne pouvons pas accéder" "Know your responsibilities","Ayez connaissance de vos responsabilités" "By using GC Notify, you accept our ","En utilisant Notification GC, vous acceptez ses " From 144e65c2ac3ef0f055d63d0031a0a6289cf05f1e Mon Sep 17 00:00:00 2001 From: wbanks Date: Tue, 13 Aug 2024 15:16:41 -0400 Subject: [PATCH 07/22] Improve the callback config UX - There is now a dedicated delete button to remove a callback configuration instead of needing to empty the form and click save to delete a config - Users will receive a confirmation banner before deletion occurs - Saving, creating, and deleting a callback url now provide the user with a confirmation message that the operation succeeded --- app/main/views/api_keys.py | 46 +++++++++++++++++-- .../callbacks/delivery-status-callback.html | 5 +- poetry.lock | 4 +- 3 files changed, 47 insertions(+), 8 deletions(-) diff --git a/app/main/views/api_keys.py b/app/main/views/api_keys.py index 5a4edd9f1b..564f18a073 100644 --- a/app/main/views/api_keys.py +++ b/app/main/views/api_keys.py @@ -171,6 +171,42 @@ def get_delivery_status_callback_details(): return service_api_client.get_service_callback_api(current_service.id, current_service.service_callback_api[0]) +@main.route( + "/services//api/callbacks/delivery-status-callback/delete", + methods=["GET", "POST"] +) +@user_has_permissions("manage_api_keys") +def delete_delivery_status_callback(service_id): + delivery_status_callback = get_delivery_status_callback_details() + back_link = ".api_callbacks" if current_service.has_permission("inbound_sms") else ".api_integration" + url_hint_txt = "Must start with https://" + + if request.method == "POST": + if delivery_status_callback: + service_api_client.delete_service_callback_api( + service_id, + delivery_status_callback["id"], + ) + + flash(_l("Callback configuration deleted"), "default_with_tick") + return redirect(url_for(back_link, service_id=service_id)) + + flash(_l("Are you sure you want to delete this callback?"), "delete") + + form = ServiceDeliveryStatusCallbackForm( + url=delivery_status_callback.get("url") if delivery_status_callback else "", + bearer_token=dummy_bearer_token if delivery_status_callback else "", + ) + + return render_template( + "views/api/callbacks/delivery-status-callback.html", + back_link=".api_callbacks" if current_service.has_permission("inbound_sms") else ".delivery_status_callback", + hint_text=url_hint_txt, + is_deleting=True, + form=form, + ) + + @main.route( "/services//api/callbacks/delivery-status-callback", methods=["GET", "POST"], @@ -196,11 +232,8 @@ def delivery_status_callback(service_id): user_id=current_user.id, callback_api_id=delivery_status_callback.get("id"), ) - elif delivery_status_callback and not form.url.data: - service_api_client.delete_service_callback_api( - service_id, - delivery_status_callback["id"], - ) + flash(_l("Callback to '{}' saved").format(form.url.data.split('https://')[1]), "default_with_tick") + return redirect(url_for(back_link, service_id=service_id)) elif form.url.data: service_api_client.create_service_callback_api( service_id, @@ -208,12 +241,15 @@ def delivery_status_callback(service_id): bearer_token=form.bearer_token.data, user_id=current_user.id, ) + flash(_l("Callback to '{}' created").format(form.url.data.split('https://')[1]), "default_with_tick") + return redirect(url_for(back_link, service_id=service_id)) else: # If no callback is set up and the user chooses to continue # having no callback (ie both fields empty) then there’s # nothing for us to do here pass + flash(_l("Callback to '{}' saved").format(form.url.data.split('https://')[1]), "default_with_tick") return redirect(url_for(back_link, service_id=service_id)) elif form.errors: url_hint_txt = "Your service must be running and reachable from the internet." diff --git a/app/templates/views/api/callbacks/delivery-status-callback.html b/app/templates/views/api/callbacks/delivery-status-callback.html index 2b346d5963..eae494e777 100644 --- a/app/templates/views/api/callbacks/delivery-status-callback.html +++ b/app/templates/views/api/callbacks/delivery-status-callback.html @@ -40,7 +40,10 @@

autocomplete='new-password' ) }} {% set button_txt = _('Save') %} - {{ page_footer(button_txt) }} + {% set display_footer = is_deleting if is_deleting else False %} + {% if not display_footer %} + {{ page_footer(button_txt, delete_link=url_for('.delete_delivery_status_callback', service_id=current_service.id)) }} + {% endif %} {% endcall %} diff --git a/poetry.lock b/poetry.lock index a58f238ed3..6688803512 100644 --- a/poetry.lock +++ b/poetry.lock @@ -2440,7 +2440,6 @@ files = [ ua-parser = ">=0.10.0" [[package]] - name = "validators" version = "0.28.3" description = "Python Data Validation for Humans™" @@ -2449,6 +2448,7 @@ python-versions = ">=3.8" files = [ {file = "validators-0.28.3-py3-none-any.whl", hash = "sha256:53cafa854f13850156259d9cc479b864ee901f6a96e6b109e6fc33f98f37d99f"}, {file = "validators-0.28.3.tar.gz", hash = "sha256:c6c79840bcde9ba77b19f6218f7738188115e27830cbaff43264bc4ed24c429d"}, +] [[package]] name = "virtualenv" @@ -2695,4 +2695,4 @@ testing = ["coverage (>=5.0.3)", "zope.event", "zope.testing"] [metadata] lock-version = "2.0" python-versions = "~3.10.9" -content-hash = "16eae1c9720d9a920fc5dc629d76695c9ec87a30fef7e13bb302756e81a44cde" +content-hash = "e0ac74772b34ecba75fab4bc38dfe09f988fb06c0430d553f87d15623a8fbd82" From eec3e7cd4ea802a0bb0db909357d0d1bc7aa0878 Mon Sep 17 00:00:00 2001 From: wbanks Date: Mon, 9 Sep 2024 12:27:07 -0400 Subject: [PATCH 08/22] Add callback test button - Added a button to the callback config page so users can see the response times of their callback services - Added some translation stubs - The ping to their service takes place as part of the validation --- app/main/forms.py | 209 ++++++++++++------ app/main/validators.py | 40 ++-- app/main/views/api_keys.py | 91 ++++++-- app/templates/components/page-footer.html | 41 ++++ .../callbacks/delivery-status-callback.html | 17 +- app/translations/csv/fr.csv | 4 + 6 files changed, 297 insertions(+), 105 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index 482cd7fe2a..fbf42d06e4 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -23,6 +23,7 @@ SelectField, SelectMultipleField, StringField, + SubmitField, TextAreaField, ValidationError, validators, @@ -65,8 +66,10 @@ def get_time_value_and_label(future_time): return ( future_time.replace(tzinfo=None).isoformat(), "{} at {}".format( - get_human_day(future_time.astimezone(pytz.timezone("Europe/London"))), - get_human_time(future_time.astimezone(pytz.timezone("Europe/London"))), + get_human_day(future_time.astimezone( + pytz.timezone("Europe/London"))), + get_human_time(future_time.astimezone( + pytz.timezone("Europe/London"))), ), ) @@ -254,7 +257,8 @@ class OrganisationTypeField(RadioField): def __init__(self, *args, include_only=None, validators=None, **kwargs): super().__init__( *args, - choices=[(value, label) for value, label in Organisation.TYPES if not include_only or value in include_only], + choices=[(value, label) for value, + label in Organisation.TYPES if not include_only or value in include_only], validators=validators or [], **kwargs, ) @@ -294,18 +298,22 @@ class RadioFieldWithNoneOption(FieldWithNoneOption, RadioField): class NestedFieldMixin: def children(self): # start map with root option as a single child entry - child_map = {None: [option for option in self if option.data == self.NONE_OPTION_VALUE]} + child_map = { + None: [option for option in self if option.data == self.NONE_OPTION_VALUE]} # add entries for all other children for option in self: if option.data == self.NONE_OPTION_VALUE: - child_ids = [folder["id"] for folder in self.all_template_folders if folder["parent_id"] is None] + child_ids = [ + folder["id"] for folder in self.all_template_folders if folder["parent_id"] is None] key = self.NONE_OPTION_VALUE else: - child_ids = [folder["id"] for folder in self.all_template_folders if folder["parent_id"] == option.data] + child_ids = [ + folder["id"] for folder in self.all_template_folders if folder["parent_id"] == option.data] key = option.data - child_map[key] = [option for option in self if option.data in child_ids] + child_map[key] = [ + option for option in self if option.data in child_ids] return child_map @@ -340,10 +348,12 @@ def bind_field(self, form, unbound_field, options): # FieldList simply doesn't support filters. # @see: https://github.com/wtforms/wtforms/issues/148 no_filter_fields = (FieldList, PasswordField) - filters = [strip_whitespace] if not issubclass(unbound_field.field_class, no_filter_fields) else [] + filters = [strip_whitespace] if not issubclass( + unbound_field.field_class, no_filter_fields) else [] filters += unbound_field.kwargs.get("filters", []) bound = unbound_field.bind(form=form, filters=filters, **options) - bound.get_form = weakref.ref(form) # GC won't collect the form if we don't use a weakref + # GC won't collect the form if we don't use a weakref + bound.get_form = weakref.ref(form) return bound @@ -367,11 +377,13 @@ class LoginForm(StripWhitespaceForm): ValidEmail(), ], ) - password = PasswordField(_l("Password"), validators=[DataRequired(message=_l("Enter your password"))]) + password = PasswordField(_l("Password"), validators=[ + DataRequired(message=_l("Enter your password"))]) class RegisterUserForm(StripWhitespaceForm): - name = StringField(_l("Full name"), validators=[DataRequired(message=_l("This cannot be empty"))]) + name = StringField(_l("Full name"), validators=[ + DataRequired(message=_l("This cannot be empty"))]) email_address = email_address() mobile_number = international_phone_number() password = password() @@ -394,7 +406,8 @@ def __init__(self, invited_user): name=guess_name_from_email_address(invited_user.email_address), ) - mobile_number = InternationalPhoneNumber(_l("Mobile number"), validators=[]) + mobile_number = InternationalPhoneNumber( + _l("Mobile number"), validators=[]) service = HiddenField("service") email_address = HiddenField("email_address") auth_type = HiddenField("auth_type", validators=[DataRequired()]) @@ -411,7 +424,8 @@ def __init__(self, invited_org_user): email_address=invited_org_user.email_address, ) - name = StringField("Full name", validators=[DataRequired(message=_l("This cannot be empty"))]) + name = StringField("Full name", validators=[ + DataRequired(message=_l("This cannot be empty"))]) mobile_number = InternationalPhoneNumber( _l("Mobile number"), @@ -440,7 +454,8 @@ def __init__(self, all_template_folders=None, *args, **kwargs): (item["id"], item["name"]) for item in ([{"name": _l("Templates"), "id": None}] + all_template_folders) ] - folder_permissions = NestedCheckboxesField(_l("Folders this team member can see")) + folder_permissions = NestedCheckboxesField( + _l("Folders this team member can see")) login_authentication = RadioField( _l("Sign in using"), @@ -476,7 +491,8 @@ def __init__(self, invalid_email_address, *args, **kwargs): def validate_email_address(self, field): if field.data.lower() == self.invalid_email_address: - raise ValidationError(_l("You cannot send an invitation to yourself")) + raise ValidationError( + _l("You cannot send an invitation to yourself")) class InviteOrgUserForm(StripWhitespaceForm): @@ -488,7 +504,8 @@ def __init__(self, invalid_email_address, *args, **kwargs): def validate_email_address(self, field): if field.data.lower() == self.invalid_email_address: - raise ValidationError(_l("You cannot send an invitation to yourself")) + raise ValidationError( + _l("You cannot send an invitation to yourself")) class TwoFactorForm(StripWhitespaceForm): @@ -651,7 +668,8 @@ class CreateServiceStepCombinedOrganisationForm(StripWhitespaceForm): class CreateServiceStepOtherOrganisationForm(StripWhitespaceForm): other_organisation_name = StringField( _l("Enter name of your group"), - validators=[DataRequired(message=_l("Enter name to continue")), Length(max=500)], + validators=[DataRequired(message=_l( + "Enter name to continue")), Length(max=500)], ) @@ -744,7 +762,8 @@ class EmailMessageLimit(StripWhitespaceForm): class SMSMessageLimit(StripWhitespaceForm): message_limit = IntegerField( _l("Daily text message limit"), - validators=[DataRequired(message=_l("This cannot be empty")), validators.NumberRange(min=1)], + validators=[DataRequired(message=_l( + "This cannot be empty")), validators.NumberRange(min=1)], ) @@ -811,7 +830,8 @@ def validate_template_content(self, field): # TODO: Remove this class when FF_TEMPLATE_CATEGORY is removed class EmailTemplateForm(BaseTemplateForm): - subject = TextAreaField(_l("Subject line of the email"), validators=[DataRequired(message=_l("This cannot be empty"))]) + subject = TextAreaField(_l("Subject line of the email"), validators=[ + DataRequired(message=_l("This cannot be empty"))]) template_content = TextAreaField( _l("Email message"), @@ -824,7 +844,8 @@ class EmailTemplateForm(BaseTemplateForm): # TODO: Remove this class when FF_TEMPLATE_CATEGORY is removed class LetterTemplateForm(EmailTemplateForm): - subject = TextAreaField("Main heading", validators=[DataRequired(message="This cannot be empty")]) + subject = TextAreaField("Main heading", validators=[ + DataRequired(message="This cannot be empty")]) template_content = TextAreaField( "Body", @@ -847,13 +868,15 @@ def __init__(self, other_field_name, other_field_value, *args, **kwargs): def __call__(self, form, field): other_field = form._fields.get(self.other_field_name) if other_field is None: - raise Exception('no field named "%s" in form' % self.other_field_name) + raise Exception('no field named "%s" in form' % + self.other_field_name) if bool(other_field.data and other_field.data == self.other_field_value): super(RequiredIf, self).__call__(form, field) class BaseTemplateFormWithCategory(BaseTemplateForm): - template_category_id = RadioField(_l("Select category"), validators=[DataRequired(message=_l("This cannot be empty"))]) + template_category_id = RadioField(_l("Select category"), validators=[ + DataRequired(message=_l("This cannot be empty"))]) template_category_other = StringField( _l("Describe category"), validators=[RequiredIf("template_category_id", DefaultTemplateCategories.LOW.value)] @@ -874,7 +897,8 @@ def validate_template_content(self, field): class EmailTemplateFormWithCategory(BaseTemplateFormWithCategory): - subject = TextAreaField(_l("Subject line of the email"), validators=[DataRequired(message=_l("This cannot be empty"))]) + subject = TextAreaField(_l("Subject line of the email"), validators=[ + DataRequired(message=_l("This cannot be empty"))]) template_content = TextAreaField( _l("Email message"), @@ -886,7 +910,8 @@ class EmailTemplateFormWithCategory(BaseTemplateFormWithCategory): class LetterTemplateFormWithCategory(EmailTemplateFormWithCategory): - subject = TextAreaField("Main heading", validators=[DataRequired(message="This cannot be empty")]) + subject = TextAreaField("Main heading", validators=[ + DataRequired(message="This cannot be empty")]) template_content = TextAreaField( "Body", @@ -931,7 +956,8 @@ def validate_old_password(self, field): class CsvUploadForm(StripWhitespaceForm): file = FileField( _l("Choose a file"), - validators=[DataRequired(message="Please pick a file"), CsvFileValidator()], + validators=[DataRequired( + message="Please pick a file"), CsvFileValidator()], ) @@ -966,7 +992,8 @@ def __init__(self, *args, **kwargs): self.scheduled_for.choices = [("", "Now")] + [ get_time_value_and_label(hour) for hour in get_next_hours_until(get_furthest_possible_scheduled_time()) ] - self.scheduled_for.categories = get_next_days_until(get_furthest_possible_scheduled_time()) + self.scheduled_for.categories = get_next_days_until( + get_furthest_possible_scheduled_time()) scheduled_for = RadioField( _l("When should we send these messages?"), @@ -976,7 +1003,8 @@ def __init__(self, *args, **kwargs): class CreateKeyForm(StripWhitespaceForm): def __init__(self, existing_keys, *args, **kwargs): - self.existing_key_names = [key["name"].lower() for key in existing_keys if not key["expiry_date"]] + self.existing_key_names = [ + key["name"].lower() for key in existing_keys if not key["expiry_date"]] super().__init__(*args, **kwargs) key_type = RadioField( @@ -985,7 +1013,8 @@ def __init__(self, existing_keys, *args, **kwargs): key_name = StringField( "Description of key", - validators=[DataRequired(message=_l("You need to give the key a name")), Length(max=255)], + validators=[DataRequired(message=_l( + "You need to give the key a name")), Length(max=255)], ) def validate_key_name(self, key_name): @@ -1010,7 +1039,8 @@ def validate_number(self, number): class ContactNotify(StripWhitespaceForm): not_empty = _l("Enter your name") - name = StringField(_l("Your name"), validators=[DataRequired(message=not_empty), Length(max=255)]) + name = StringField(_l("Your name"), validators=[ + DataRequired(message=not_empty), Length(max=255)]) support_type = RadioField( _l("How can we help?"), choices=[ @@ -1026,7 +1056,8 @@ class ContactNotify(StripWhitespaceForm): class ContactMessageStep(ContactNotify): message = TextAreaField( _l("Message"), - validators=[DataRequired(message=_l("You need to enter something if you want to contact us")), Length(max=2000)], + validators=[DataRequired(message=_l( + "You need to enter something if you want to contact us")), Length(max=2000)], ) @@ -1040,7 +1071,8 @@ def __init__(self, *args, **kwargs): branding_style = SelectField() file = FileField_wtf( _l("Upload logo"), - validators=[FileAllowed(["png"], _l("Your logo must be an image in PNG format"))], + validators=[FileAllowed(["png"], _l( + "Your logo must be an image in PNG format"))], ) @@ -1057,7 +1089,8 @@ class Triage(StripWhitespaceForm): class ProviderForm(StripWhitespaceForm): priority = IntegerField( "Priority", - [validators.NumberRange(min=1, max=100, message="Must be between 1 and 100")], + [validators.NumberRange( + min=1, max=100, message="Must be between 1 and 100")], ) @@ -1077,7 +1110,8 @@ class ServiceContactDetailsForm(StripWhitespaceForm): def validate(self, extra_validators=None): if self.contact_details_type.data == "url": - self.url.validators = [DataRequired(), URL(message="Must be a valid URL")] + self.url.validators = [DataRequired(), URL( + message="Must be a valid URL")] elif self.contact_details_type.data == "email_address": self.email_address.validators = [ @@ -1149,7 +1183,8 @@ class ServiceLetterContactBlockForm(StripWhitespaceForm): def validate_letter_contact_block(self, field): line_count = field.data.strip().count("\n") if line_count >= 10: - raise ValidationError("Contains {} lines, maximum is 10".format(line_count + 1)) + raise ValidationError( + "Contains {} lines, maximum is 10".format(line_count + 1)) class OnOffField(RadioField): @@ -1167,7 +1202,8 @@ def __init__(self, label, *args, **kwargs): def process_formdata(self, valuelist): if valuelist: value = valuelist[0] - self.data = (value == "True") if value in ["True", "False"] else value + self.data = (value == "True") if value in [ + "True", "False"] else value class ServiceOnOffSettingForm(StripWhitespaceForm): @@ -1329,14 +1365,16 @@ def populate(self, email_addresses, phone_numbers): form_field[index].data = value email_addresses = FieldList( - EmailFieldInSafelist("", validators=[Optional(), ValidEmail()], default=""), + EmailFieldInSafelist( + "", validators=[Optional(), ValidEmail()], default=""), min_entries=5, max_entries=5, label=_l("Email safelist"), ) phone_numbers = FieldList( - InternationalPhoneNumberInSafelist("", validators=[Optional()], default=""), + InternationalPhoneNumberInSafelist( + "", validators=[Optional()], default=""), min_entries=5, max_entries=5, label=_l("Phone safelist"), @@ -1346,7 +1384,8 @@ def populate(self, email_addresses, phone_numbers): class DateFilterForm(StripWhitespaceForm): start_date = DateField(_l("Start Date"), [validators.optional()]) end_date = DateField(_l("End Date"), [validators.optional()]) - include_from_test_key = BooleanField(_l("Include test keys"), default="checked", false_values={"N"}) + include_from_test_key = BooleanField( + _l("Include test keys"), default="checked", false_values={"N"}) class RequiredDateFilterForm(StripWhitespaceForm): @@ -1357,14 +1396,16 @@ class RequiredDateFilterForm(StripWhitespaceForm): class SearchByNameForm(StripWhitespaceForm): search = SearchField( _l("Search by name"), - validators=[DataRequired("You need to enter full or partial name to search by.")], + validators=[DataRequired( + "You need to enter full or partial name to search by.")], ) class SearchUsersByEmailForm(StripWhitespaceForm): search = SearchField( _l("Search by name or email address"), - validators=[DataRequired(_l("You need to enter full or partial email address to search by."))], + validators=[DataRequired( + _l("You need to enter full or partial email address to search by."))], ) @@ -1416,7 +1457,8 @@ class ServiceReceiveMessagesCallbackForm(CallbackForm): "URL", validators=[ DataRequired(message=_l("This cannot be empty")), - Regexp(regex="^https.*", message=_l("Enter a URL that starts with https://")), + Regexp(regex="^https.*", + message=_l("Enter a URL that starts with https://")), ValidCallbackUrl(), ], ) @@ -1434,7 +1476,8 @@ class ServiceDeliveryStatusCallbackForm(CallbackForm): "URL", validators=[ DataRequired(message=_l("This cannot be empty")), - Regexp(regex="^https.*", message=_l("Enter a URL that starts with https://")), + Regexp(regex="^https.*", + message=_l("Enter a URL that starts with https://")), ValidCallbackUrl(), ], ) @@ -1445,6 +1488,7 @@ class ServiceDeliveryStatusCallbackForm(CallbackForm): Length(min=10, message=_l("Must be at least 10 characters")), ], ) + test_response_time = SubmitField() class InternationalSMSForm(StripWhitespaceForm): @@ -1483,10 +1527,12 @@ def get_placeholder_form_instance( else: field = uk_mobile_number(label=placeholder_name) elif optional_placeholder: - field = StringField(_l("What is the custom content in (({})) ?").format(placeholder_name)) + field = StringField( + _l("What is the custom content in (({})) ?").format(placeholder_name)) elif is_conditional: field = RadioField( - _l("Do you want to include the content in (({})) ?").format(placeholder_name), + _l("Do you want to include the content in (({})) ?").format( + placeholder_name), choices=[ ("yes", _l("Yes")), ("no", _l("No")), @@ -1558,14 +1604,16 @@ class ServiceDataRetentionForm(StripWhitespaceForm): ) days_of_retention = IntegerField( label="Days of retention", - validators=[validators.NumberRange(min=3, max=7, message="Must be between 3 and 7")], + validators=[validators.NumberRange( + min=3, max=7, message="Must be between 3 and 7")], ) class ServiceDataRetentionEditForm(StripWhitespaceForm): days_of_retention = IntegerField( label="Days of retention", - validators=[validators.NumberRange(min=3, max=7, message="Must be between 3 and 7")], + validators=[validators.NumberRange( + min=3, max=7, message="Must be between 3 and 7")], ) @@ -1583,10 +1631,13 @@ def __init__(self, all_service_users=None, *args, **kwargs): super().__init__(*args, **kwargs) if all_service_users is not None: self.users_with_permission.all_service_users = all_service_users - self.users_with_permission.choices = [(item.id, item.name) for item in all_service_users] + self.users_with_permission.choices = [ + (item.id, item.name) for item in all_service_users] - users_with_permission = MultiCheckboxField(_l("Team members who can see this folder")) - name = StringField(_l("Folder name"), validators=[DataRequired(message=_l("This cannot be empty"))]) + users_with_permission = MultiCheckboxField( + _l("Team members who can see this folder")) + name = StringField(_l("Folder name"), validators=[ + DataRequired(message=_l("This cannot be empty"))]) def required_for_ops(*operations): @@ -1613,19 +1664,23 @@ def __init__(self, *args, **kwargs): class AddEmailRecipientsForm(Form): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - self.what_type.choices = [("many_recipients", _l("Many recipients")), ("one_recipient", _l("One recipient"))] + self.what_type.choices = [("many_recipients", _l( + "Many recipients")), ("one_recipient", _l("One recipient"))] what_type = RadioField("") - placeholder_value = email_address(_l("Email address of recipient"), gov_user=False) + placeholder_value = email_address( + _l("Email address of recipient"), gov_user=False) class AddSMSRecipientsForm(Form): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - self.what_type.choices = [("many_recipients", _l("Many recipients")), ("one_recipient", _l("One recipient"))] + self.what_type.choices = [("many_recipients", _l( + "Many recipients")), ("one_recipient", _l("One recipient"))] what_type = RadioField("") - placeholder_value = international_phone_number(_l("Phone number of recipient")) + placeholder_value = international_phone_number( + _l("Phone number of recipient")) class TemplateAndFoldersSelectionForm(Form): @@ -1667,7 +1722,8 @@ def __init__( self.is_move_op = self.is_add_folder_op = False self.move_to.all_template_folders = all_template_folders - self.move_to.choices = [(item["id"], item["name"]) for item in ([self.ALL_TEMPLATES_FOLDER] + all_template_folders)] + self.move_to.choices = [(item["id"], item["name"]) for item in ( + [self.ALL_TEMPLATES_FOLDER] + all_template_folders)] def is_selected(self, template_folder_id): return template_folder_id in (self.templates_and_folders.data or []) @@ -1675,8 +1731,10 @@ def is_selected(self, template_folder_id): def validate(self, extra_validators=None): self.op = request.form.get("operation") - self.is_move_op = self.op in {"move-to-existing-folder", "move-to-new-folder"} - self.is_add_folder_op = self.op in {"add-new-folder", "move-to-new-folder"} + self.is_move_op = self.op in { + "move-to-existing-folder", "move-to-new-folder"} + self.is_add_folder_op = self.op in { + "add-new-folder", "move-to-new-folder"} if not (self.is_add_folder_op or self.is_move_op): return False @@ -1692,7 +1750,8 @@ def get_folder_name(self): templates_and_folders = MultiCheckboxField( "Choose templates or folders", - validators=[required_for_ops("move-to-new-folder", "move-to-existing-folder")], + validators=[required_for_ops( + "move-to-new-folder", "move-to-existing-folder")], ) # if no default set, it is set to None, which process_data transforms to '__NONE__' # this means '__NONE__' (self.ALL_TEMPLATES option) is selected when no form data has been submitted @@ -1702,8 +1761,10 @@ def get_folder_name(self): default="", validators=[required_for_ops("move-to-existing-folder"), Optional()], ) - add_new_folder_name = StringField(_l("Folder name"), validators=[required_for_ops("add-new-folder")]) - move_to_new_folder_name = StringField(_l("Folder name"), validators=[required_for_ops("move-to-new-folder")]) + add_new_folder_name = StringField(_l("Folder name"), validators=[ + required_for_ops("add-new-folder")]) + move_to_new_folder_name = StringField(_l("Folder name"), validators=[ + required_for_ops("move-to-new-folder")]) class ClearCacheForm(StripWhitespaceForm): @@ -1736,7 +1797,8 @@ def from_organisation(cls, org): on_behalf_of_email=org.agreement_signed_on_behalf_of_email_address, ) - version = StringField("Which version of the agreement do you want to accept?") + version = StringField( + "Which version of the agreement do you want to accept?") who = RadioField( "Who are you accepting the agreement for?", @@ -1871,7 +1933,8 @@ class BrandingGOCForm(StripWhitespaceForm): (FieldWithLanguageOptions.ENGLISH_OPTION_VALUE, _l("English-first")), (FieldWithLanguageOptions.FRENCH_OPTION_VALUE, _l("French-first")), ], - validators=[DataRequired(message=_l("You must select an option to continue"))], + validators=[DataRequired(message=_l( + "You must select an option to continue"))], ) def __init__(self, *args, **kwargs): @@ -1888,8 +1951,10 @@ class BrandingPoolForm(StripWhitespaceForm): pool_branding = RadioField( _l("Select alternate logo"), - choices=[], # Choices by default, override to get more refined options. - validators=[DataRequired(message=_l("You must select an option to continue"))], + # Choices by default, override to get more refined options. + choices=[], + validators=[DataRequired(message=_l( + "You must select an option to continue"))], ) def __init__(self, *args, **kwargs): @@ -1905,9 +1970,12 @@ class BrandingRequestForm(StripWhitespaceForm): file (FileField_wtf): Field for uploading the logo file. """ - name = StringField(label=_l("Name of logo"), validators=[DataRequired(message=_l("Enter the name of the logo"))]) - alt_text_en = StringField(label=_l("English"), validators=[DataRequired(message=_l("This cannot be empty"))]) - alt_text_fr = StringField(label=_l("French"), validators=[DataRequired(message=_l("This cannot be empty"))]) + name = StringField(label=_l("Name of logo"), validators=[ + DataRequired(message=_l("Enter the name of the logo"))]) + alt_text_en = StringField(label=_l("English"), validators=[ + DataRequired(message=_l("This cannot be empty"))]) + alt_text_fr = StringField(label=_l("French"), validators=[ + DataRequired(message=_l("This cannot be empty"))]) file = FileField_wtf( label=_l("Prepare your logo"), validators=[ @@ -1917,11 +1985,14 @@ class BrandingRequestForm(StripWhitespaceForm): class TemplateCategoryForm(StripWhitespaceForm): - name_en = StringField("EN", validators=[DataRequired(message=_l("This cannot be empty"))]) - name_fr = StringField("FR", validators=[DataRequired(message=_l("This cannot be empty"))]) + name_en = StringField( + "EN", validators=[DataRequired(message=_l("This cannot be empty"))]) + name_fr = StringField( + "FR", validators=[DataRequired(message=_l("This cannot be empty"))]) description_en = StringField("EN") description_fr = StringField("FR") - hidden = RadioField(_l("Hide category"), choices=[("True", _l("Hide")), ("False", _l("Show"))]) + hidden = RadioField(_l("Hide category"), choices=[ + ("True", _l("Hide")), ("False", _l("Show"))]) sms_sending_vehicle = RadioField( _l("Sending method for text messages"), choices=[("long_code", _l("Long code")), ("short_code", _l("Short code"))] ) diff --git a/app/main/validators.py b/app/main/validators.py index 84c7ffd3fc..5e9e5eef93 100644 --- a/app/main/validators.py +++ b/app/main/validators.py @@ -4,7 +4,7 @@ import pwnedpasswords import requests import validators -from flask import current_app +from flask import current_app, g from flask_babel import _ from flask_babel import lazy_gettext as _l from notifications_utils.field import Field @@ -21,13 +21,15 @@ class Blocklist: def __init__(self, message=None): if not message: - message = _("This password is not allowed. Try a different password.") + message = _( + "This password is not allowed. Try a different password.") self.message = message def __call__(self, form, field): if current_app.config.get("HIPB_ENABLED", None): hibp_bad_password_found = False - for i in range(0, 3): # Try 3 times. If the HIPB API is down then fall back to the old banlist. + # Try 3 times. If the HIPB API is down then fall back to the old banlist. + for i in range(0, 3): try: response = pwnedpasswords.check(field.data) if response > 0: @@ -108,7 +110,8 @@ def __call__(self, form, field): class OnlySMSCharacters: def __call__(self, form, field): - non_sms_characters = sorted(list(SanitiseSMS.get_non_compatible_characters(field.data))) + non_sms_characters = sorted( + list(SanitiseSMS.get_non_compatible_characters(field.data))) if non_sms_characters: raise ValidationError( "You can’t use {} in text messages. {} won’t show up properly on everyone’s phones.".format( @@ -175,20 +178,26 @@ def validate_callback_url(service_callback_url, bearer_token): url=service_callback_url, allow_redirects=True, data={"health_check": "true"}, - headers={"Content-Type": "application/json", "Authorization": f"Bearer {bearer_token}"}, - timeout=5, + headers={"Content-Type": "application/json", + "Authorization": f"Bearer {bearer_token}"}, + timeout=2, ) + g.callback_response_time = response.elapsed.total_seconds() + if response.status_code < 500 and response.status_code >= 400: current_app.logger.warning( f"Unable to create callback for service: {current_service.id} Error: Callback URL not reachable URL: {service_callback_url}" ) - raise ValidationError(_l("Check your service is running and not using a proxy we cannot access")) + raise ValidationError( + _l("Check your service is running and not using a proxy we cannot access")) + except requests.RequestException as e: current_app.logger.warning( f"Unable to create callback for service: {current_service.id} Error: Callback URL not reachable URL: {service_callback_url} Exception: {e}" ) - raise ValidationError(_l("Check your service is running and not using a proxy we cannot access")) + raise ValidationError( + _l("Check your service is running and not using a proxy we cannot access")) def validate_email_from(form, field): @@ -198,18 +207,23 @@ def validate_email_from(form, field): if len(field.data) > 64: raise ValidationError(_l("This cannot exceed 64 characters in length")) # this filler is used because service id is not available when validating a new service to be created - service_id = getattr(form, "service_id", current_app.config["NOTIFY_BAD_FILLER_UUID"]) - unique_name = service_api_client.is_service_email_from_unique(service_id, field.data) + service_id = getattr(form, "service_id", + current_app.config["NOTIFY_BAD_FILLER_UUID"]) + unique_name = service_api_client.is_service_email_from_unique( + service_id, field.data) if not unique_name: raise ValidationError(_l("This email address is already in use")) def validate_service_name(form, field): if len(field.data) > 255: - raise ValidationError(_l("This cannot exceed 255 characters in length")) + raise ValidationError( + _l("This cannot exceed 255 characters in length")) if field.data != email_safe_name(field.data): - raise ValidationError(_l("Make sure we formatted your email address correctly.")) - service_id = getattr(form, "service_id", current_app.config["NOTIFY_BAD_FILLER_UUID"]) + raise ValidationError( + _l("Make sure we formatted your email address correctly.")) + service_id = getattr(form, "service_id", + current_app.config["NOTIFY_BAD_FILLER_UUID"]) unique_name = service_api_client.is_service_name_unique( service_id, field.data, diff --git a/app/main/views/api_keys.py b/app/main/views/api_keys.py index 564f18a073..4cd3d9b478 100644 --- a/app/main/views/api_keys.py +++ b/app/main/views/api_keys.py @@ -1,6 +1,7 @@ -from flask import Markup, abort, flash, redirect, render_template, request, url_for +from flask import Markup, abort, flash, g, redirect, render_template, request, url_for from flask_babel import lazy_gettext as _l from flask_login import current_user +import requests from app import ( api_key_api_client, @@ -28,11 +29,13 @@ @main.route("/services//api") @user_has_permissions("manage_api_keys") def api_integration(service_id): - callbacks_link = ".api_callbacks" if current_service.has_permission("inbound_sms") else ".delivery_status_callback" + callbacks_link = ".api_callbacks" if current_service.has_permission( + "inbound_sms") else ".delivery_status_callback" return render_template( "views/api/index.html", callbacks_link=callbacks_link, - api_notifications=notification_api_client.get_api_notifications_for_service(service_id), + api_notifications=notification_api_client.get_api_notifications_for_service( + service_id), ) @@ -87,7 +90,8 @@ def create_api_key(service_id): disabled_options, option_hints = [], {} if current_service.trial_mode: disabled_options = [KEY_TYPE_NORMAL] - option_hints[KEY_TYPE_NORMAL] = Markup(_l("Not available because your service is in trial mode.")) + option_hints[KEY_TYPE_NORMAL] = Markup( + _l("Not available because your service is in trial mode.")) if current_service.has_permission("letter"): option_hints[KEY_TYPE_TEAM] = "" if form.validate_on_submit(): @@ -119,7 +123,8 @@ def revoke_api_key(service_id, key_id): if request.method == "GET": flash( [ - "{} ‘{}’?".format(_l("Are you sure you want to revoke"), key_name), + "{} ‘{}’?".format( + _l("Are you sure you want to revoke"), key_name), _l("You will not be able to use this API key to connect to GC Notify"), ], "revoke this API key", @@ -137,9 +142,11 @@ def get_apis(): callback_api = None inbound_api = None if current_service.service_callback_api: - callback_api = service_api_client.get_service_callback_api(current_service.id, current_service.service_callback_api[0]) + callback_api = service_api_client.get_service_callback_api( + current_service.id, current_service.service_callback_api[0]) if current_service.inbound_api: - inbound_api = service_api_client.get_service_inbound_api(current_service.id, current_service.inbound_api[0]) + inbound_api = service_api_client.get_service_inbound_api( + current_service.id, current_service.inbound_api[0]) return (callback_api, inbound_api) @@ -161,7 +168,8 @@ def api_callbacks(service_id): return render_template( "views/api/callbacks.html", - received_text_messages_callback=received_text_messages_callback["url"] if received_text_messages_callback else None, + received_text_messages_callback=received_text_messages_callback[ + "url"] if received_text_messages_callback else None, delivery_status_callback=delivery_status_callback["url"] if delivery_status_callback else None, ) @@ -178,7 +186,8 @@ def get_delivery_status_callback_details(): @user_has_permissions("manage_api_keys") def delete_delivery_status_callback(service_id): delivery_status_callback = get_delivery_status_callback_details() - back_link = ".api_callbacks" if current_service.has_permission("inbound_sms") else ".api_integration" + back_link = ".api_callbacks" if current_service.has_permission( + "inbound_sms") else ".api_integration" url_hint_txt = "Must start with https://" if request.method == "POST": @@ -194,13 +203,15 @@ def delete_delivery_status_callback(service_id): flash(_l("Are you sure you want to delete this callback?"), "delete") form = ServiceDeliveryStatusCallbackForm( - url=delivery_status_callback.get("url") if delivery_status_callback else "", + url=delivery_status_callback.get( + "url") if delivery_status_callback else "", bearer_token=dummy_bearer_token if delivery_status_callback else "", ) return render_template( "views/api/callbacks/delivery-status-callback.html", - back_link=".api_callbacks" if current_service.has_permission("inbound_sms") else ".delivery_status_callback", + back_link=".api_callbacks" if current_service.has_permission( + "inbound_sms") else ".delivery_status_callback", hint_text=url_hint_txt, is_deleting=True, form=form, @@ -214,26 +225,49 @@ def delete_delivery_status_callback(service_id): @user_has_permissions("manage_api_keys") def delivery_status_callback(service_id): delivery_status_callback = get_delivery_status_callback_details() - back_link = ".api_callbacks" if current_service.has_permission("inbound_sms") else ".api_integration" + back_link = ".api_callbacks" if current_service.has_permission( + "inbound_sms") else ".api_integration" url_hint_txt = "Must start with https://" form = ServiceDeliveryStatusCallbackForm( - url=delivery_status_callback.get("url") if delivery_status_callback else "", + url=delivery_status_callback.get( + "url") if delivery_status_callback else "", bearer_token=dummy_bearer_token if delivery_status_callback else "", ) if form.validate_on_submit(): + # As part of the ValidCallbackUrl validation, we ping their callback URL to check if it's up and set the response time in g + response_time = "{:.2f} {}".format( + g.callback_response_time, "milliseconds" if g.callback_response_time < 1 else "seconds") + + # Update existing callback if delivery_status_callback and form.url.data: if delivery_status_callback.get("url") != form.url.data or form.bearer_token.data != dummy_bearer_token: + service_api_client.update_service_callback_api( service_id, url=form.url.data, - bearer_token=check_token_against_dummy_bearer(form.bearer_token.data), + bearer_token=check_token_against_dummy_bearer( + form.bearer_token.data), user_id=current_user.id, callback_api_id=delivery_status_callback.get("id"), ) - flash(_l("Callback to '{}' saved").format(form.url.data.split('https://')[1]), "default_with_tick") + + # If the user is just testing their URL, don't send them back to the API Integration page + if request.form.get("button_pressed") == "test_response_time": + flash(_l("Your service '{}' responded in {}").format( + form.url.data, + response_time, + ), "default_with_tick") + return redirect(url_for("main.delivery_status_callback", service_id=service_id)) + + flash(_l("Callback to '{}' saved. Your service responded in '{}'").format( + form.url.data.split('https://')[1], + response_time, + ), "default_with_tick") + return redirect(url_for(back_link, service_id=service_id)) + # Create a new callback elif form.url.data: service_api_client.create_service_callback_api( service_id, @@ -241,7 +275,12 @@ def delivery_status_callback(service_id): bearer_token=form.bearer_token.data, user_id=current_user.id, ) - flash(_l("Callback to '{}' created").format(form.url.data.split('https://')[1]), "default_with_tick") + + flash(_l("Callback to '{}' created. Your service responded in '{}'").format( + form.url.data.split('https://')[1], + response_time, + ), "default_with_tick") + return redirect(url_for(back_link, service_id=service_id)) else: # If no callback is set up and the user chooses to continue @@ -249,13 +288,25 @@ def delivery_status_callback(service_id): # nothing for us to do here pass - flash(_l("Callback to '{}' saved").format(form.url.data.split('https://')[1]), "default_with_tick") + if request.form.get("button_pressed") == "test_response_time": + flash(_l("Your service '{}' responded in {}").format( + form.url.data, + response_time, + ), "default_with_tick") + return redirect(url_for("main.delivery_status_callback", service_id=service_id)) + + flash(_l("Callback to '{}' saved. Your service responded in '{}'").format( + form.url.data.split('https://')[1], + response_time, + ), "default_with_tick") + return redirect(url_for(back_link, service_id=service_id)) elif form.errors: url_hint_txt = "Your service must be running and reachable from the internet." return render_template( "views/api/callbacks/delivery-status-callback.html", + has_callback_config=not delivery_status_callback is None, back_link=back_link, hint_text=url_hint_txt, form=form, @@ -278,7 +329,8 @@ def received_text_messages_callback(service_id): received_text_messages_callback = get_received_text_messages_callback() form = ServiceReceiveMessagesCallbackForm( - url=received_text_messages_callback.get("url") if received_text_messages_callback else "", + url=received_text_messages_callback.get( + "url") if received_text_messages_callback else "", bearer_token=dummy_bearer_token if received_text_messages_callback else "", ) url_hint_txt = "Must start with https://" @@ -289,7 +341,8 @@ def received_text_messages_callback(service_id): service_api_client.update_service_inbound_api( service_id, url=form.url.data, - bearer_token=check_token_against_dummy_bearer(form.bearer_token.data), + bearer_token=check_token_against_dummy_bearer( + form.bearer_token.data), user_id=current_user.id, inbound_api_id=received_text_messages_callback.get("id"), ) diff --git a/app/templates/components/page-footer.html b/app/templates/components/page-footer.html index 13929520a3..144aaaccab 100644 --- a/app/templates/components/page-footer.html +++ b/app/templates/components/page-footer.html @@ -67,3 +67,44 @@ {% endmacro %} + +{% macro sticky_page_footer_two_submit_buttons_and_delete_link( + button1_text="b1", + button1_value="b1", + button2_text=None, + button2_value=None, + delete_link=False, + delete_link_text=_("Delete")) %} +
+
+ + + + {% if button2_text and button2_value %} + + {% endif %} + + {% if delete_link %} + + {{ delete_link_text }} + + {% endif %} +
+
+{% endmacro %} \ No newline at end of file diff --git a/app/templates/views/api/callbacks/delivery-status-callback.html b/app/templates/views/api/callbacks/delivery-status-callback.html index eae494e777..7c5ee805ee 100644 --- a/app/templates/views/api/callbacks/delivery-status-callback.html +++ b/app/templates/views/api/callbacks/delivery-status-callback.html @@ -1,8 +1,9 @@ {% extends "admin_template.html" %} {% from "components/textbox.html" import textbox %} {% from "components/page-header.html" import page_header %} -{% from "components/page-footer.html" import page_footer %} +{% from "components/page-footer.html" import page_footer, sticky_page_footer_two_submit_buttons_and_delete_link %} {% from "components/form.html" import form_wrapper %} +{% from "components/ajax-block.html" import ajax_block %} {% block service_page_title %} {{ _('Callbacks for delivery receipts') }} @@ -39,13 +40,21 @@

hint=hint_txt, autocomplete='new-password' ) }} - {% set button_txt = _('Save') %} + {% set test_response_txt = _('Test response time') if has_callback_config else None %} + {% set test_response_value = _('test_response_time') if has_callback_config else None %} {% set display_footer = is_deleting if is_deleting else False %} {% if not display_footer %} - {{ page_footer(button_txt, delete_link=url_for('.delete_delivery_status_callback', service_id=current_service.id)) }} + {{ sticky_page_footer_two_submit_buttons_and_delete_link( + button1_text=_('Save'), + button1_value=_('save'), + button2_text=test_response_txt, + button2_value=test_response_value, + delete_link=url_for('.delete_delivery_status_callback', service_id=current_service.id), + delete_link_text=_('Delete') + ) }} {% endif %} {% endcall %} - + {% endblock %} diff --git a/app/translations/csv/fr.csv b/app/translations/csv/fr.csv index bbf38984d9..9b561ea0f0 100644 --- a/app/translations/csv/fr.csv +++ b/app/translations/csv/fr.csv @@ -1981,3 +1981,7 @@ "Review your activity","Examinez votre activité" "Set sensitive service","Définir un service sensible" "Suspend Callback","Suspension du rappel" +"Callback to '{}' created. Your service responded in '{}'","(FR) Callback to '{}' created. Your service responded in '{}'" +"Callback to '{}' saved. Your service responded in '{}'","(FR) Callback to '{}' saved. Your service responded in '{}'" +"Your service must be running and reachable from the internet","(FR) Your service must be running and reachable from the internet" +"We were unable to contact your callback service, please try again.","(FR)We were unable to contact your callback service, please try again." \ No newline at end of file From ba989b92f99043cb0bdbd919e862fb2c21f339da Mon Sep 17 00:00:00 2001 From: wbanks Date: Mon, 9 Sep 2024 16:29:54 -0400 Subject: [PATCH 09/22] Unify delivery-status-callback and received-text-messages-callback pages - Fix a few tests --- app/main/views/api_keys.py | 158 ++++++++++++++---- .../callbacks/delivery-status-callback.html | 3 +- .../received-text-messages-callback.html | 20 ++- app/translations/csv/fr.csv | 4 +- tests/app/main/views/test_api_integration.py | 104 +++++++----- tests/conftest.py | 2 +- 6 files changed, 214 insertions(+), 77 deletions(-) diff --git a/app/main/views/api_keys.py b/app/main/views/api_keys.py index 4cd3d9b478..ca020ecd79 100644 --- a/app/main/views/api_keys.py +++ b/app/main/views/api_keys.py @@ -1,7 +1,6 @@ from flask import Markup, abort, flash, g, redirect, render_template, request, url_for from flask_babel import lazy_gettext as _l from flask_login import current_user -import requests from app import ( api_key_api_client, @@ -179,10 +178,7 @@ def get_delivery_status_callback_details(): return service_api_client.get_service_callback_api(current_service.id, current_service.service_callback_api[0]) -@main.route( - "/services//api/callbacks/delivery-status-callback/delete", - methods=["GET", "POST"] -) +@main.route("/services//api/callbacks/delivery-status-callback/delete", methods=["GET", "POST"]) @user_has_permissions("manage_api_keys") def delete_delivery_status_callback(service_id): delivery_status_callback = get_delivery_status_callback_details() @@ -238,12 +234,12 @@ def delivery_status_callback(service_id): if form.validate_on_submit(): # As part of the ValidCallbackUrl validation, we ping their callback URL to check if it's up and set the response time in g response_time = "{:.2f} {}".format( - g.callback_response_time, "milliseconds" if g.callback_response_time < 1 else "seconds") + g.callback_response_time, "milliseconds" if g.callback_response_time < 1 else "seconds" + ) # Update existing callback if delivery_status_callback and form.url.data: if delivery_status_callback.get("url") != form.url.data or form.bearer_token.data != dummy_bearer_token: - service_api_client.update_service_callback_api( service_id, url=form.url.data, @@ -255,16 +251,22 @@ def delivery_status_callback(service_id): # If the user is just testing their URL, don't send them back to the API Integration page if request.form.get("button_pressed") == "test_response_time": - flash(_l("Your service '{}' responded in {}").format( - form.url.data, - response_time, - ), "default_with_tick") + flash( + _l("Your service '{}' responded in {}").format( + form.url.data, + response_time, + ), + "default_with_tick", + ) return redirect(url_for("main.delivery_status_callback", service_id=service_id)) - flash(_l("Callback to '{}' saved. Your service responded in '{}'").format( - form.url.data.split('https://')[1], - response_time, - ), "default_with_tick") + flash( + _l("Callback to '{}' saved. Your service responded in {}").format( + form.url.data.split("https://")[1], + response_time, + ), + "default_with_tick", + ) return redirect(url_for(back_link, service_id=service_id)) # Create a new callback @@ -276,10 +278,13 @@ def delivery_status_callback(service_id): user_id=current_user.id, ) - flash(_l("Callback to '{}' created. Your service responded in '{}'").format( - form.url.data.split('https://')[1], - response_time, - ), "default_with_tick") + flash( + _l("Callback to '{}' created. Your service responded in {}").format( + form.url.data.split("https://")[1], + response_time, + ), + "default_with_tick", + ) return redirect(url_for(back_link, service_id=service_id)) else: @@ -289,16 +294,22 @@ def delivery_status_callback(service_id): pass if request.form.get("button_pressed") == "test_response_time": - flash(_l("Your service '{}' responded in {}").format( - form.url.data, - response_time, - ), "default_with_tick") + flash( + _l("Your service '{}' responded in {}").format( + form.url.data, + response_time, + ), + "default_with_tick", + ) return redirect(url_for("main.delivery_status_callback", service_id=service_id)) - flash(_l("Callback to '{}' saved. Your service responded in '{}'").format( - form.url.data.split('https://')[1], - response_time, - ), "default_with_tick") + flash( + _l("Callback to '{}' saved. Your service responded in {}").format( + form.url.data.split("https://")[1], + response_time, + ), + "default_with_tick", + ) return redirect(url_for(back_link, service_id=service_id)) elif form.errors: @@ -306,7 +317,7 @@ def delivery_status_callback(service_id): return render_template( "views/api/callbacks/delivery-status-callback.html", - has_callback_config=not delivery_status_callback is None, + has_callback_config=delivery_status_callback is not None, back_link=back_link, hint_text=url_hint_txt, form=form, @@ -326,6 +337,8 @@ def get_received_text_messages_callback(): def received_text_messages_callback(service_id): if not current_service.has_permission("inbound_sms"): return redirect(url_for(".api_integration", service_id=service_id)) + back_link = ".api_callbacks" if current_service.has_permission( + "inbound_sms") else ".api_integration" received_text_messages_callback = get_received_text_messages_callback() form = ServiceReceiveMessagesCallbackForm( @@ -336,6 +349,10 @@ def received_text_messages_callback(service_id): url_hint_txt = "Must start with https://" if form.validate_on_submit(): + # As part of the ValidCallbackUrl validation, we ping their callback URL to check if it's up and set the response time in g + response_time = "{:.2f} {}".format( + g.callback_response_time, "milliseconds" if g.callback_response_time < 1 else "seconds" + ) if received_text_messages_callback and form.url.data: if received_text_messages_callback.get("url") != form.url.data or form.bearer_token.data != dummy_bearer_token: service_api_client.update_service_inbound_api( @@ -346,6 +363,26 @@ def received_text_messages_callback(service_id): user_id=current_user.id, inbound_api_id=received_text_messages_callback.get("id"), ) + + # If the user is just testing their URL, don't send them back to the API Integration page + if request.form.get("button_pressed") == "test_response_time": + flash( + _l("Your service '{}' responded in {}").format( + form.url.data, + response_time, + ), + "default_with_tick", + ) + return redirect(url_for("main.received_text_messages_callback", service_id=service_id)) + + flash( + _l("Callback to '{}' saved. Your service responded in {}").format( + form.url.data.split("https://")[1], + response_time, + ), + "default_with_tick", + ) + elif received_text_messages_callback and not form.url.data: service_api_client.delete_service_inbound_api( service_id, @@ -358,11 +395,76 @@ def received_text_messages_callback(service_id): bearer_token=form.bearer_token.data, user_id=current_user.id, ) + flash( + _l("Callback to '{}' created. Your service responded in {}").format( + form.url.data.split("https://")[1], + response_time, + ), + "default_with_tick", + ) + + return redirect(url_for(back_link, service_id=service_id)) elif form.errors: url_hint_txt = "Your service must be running and reachable from the internet." + + if request.form.get("button_pressed") == "test_response_time": + flash( + _l("Your service '{}' responded in {}").format( + form.url.data, + response_time, + ), + "default_with_tick", + ) + return redirect(url_for("main.received_text_messages_callback", service_id=service_id)) + + flash( + _l("Callback to '{}' saved. Your service responded in {}").format( + form.url.data.split("https://")[1], + response_time, + ), + "default_with_tick", + ) + return redirect(url_for(".api_callbacks", service_id=service_id)) return render_template( "views/api/callbacks/received-text-messages-callback.html", + has_callback_config=received_text_messages_callback is not None, form=form, hint_text=url_hint_txt, ) + + +@main.route("/services//api/callbacks/received-text-messages-callback/delete", methods=["GET", "POST"]) +@user_has_permissions("manage_api_keys") +def delete_received_text_messages_callback(service_id): + received_text_messages_callback = get_received_text_messages_callback() + back_link = ".api_callbacks" if current_service.has_permission( + "inbound_sms") else ".api_integration" + url_hint_txt = "Must start with https://" + + if request.method == "POST": + if received_text_messages_callback: + service_api_client.delete_service_inbound_api( + service_id, + received_text_messages_callback["id"], + ) + + flash(_l("Callback configuration deleted"), "default_with_tick") + return redirect(url_for(back_link, service_id=service_id)) + + flash(_l("Are you sure you want to delete this callback?"), "delete") + + form = ServiceReceiveMessagesCallbackForm( + url=received_text_messages_callback.get( + "url") if delivery_status_callback else "", + bearer_token=dummy_bearer_token if received_text_messages_callback else "", + ) + + return render_template( + "views/api/callbacks/delivery-status-callback.html", + back_link=".api_callbacks" if current_service.has_permission( + "inbound_sms") else ".delivery_status_callback", + hint_text=url_hint_txt, + is_deleting=True, + form=form, + ) diff --git a/app/templates/views/api/callbacks/delivery-status-callback.html b/app/templates/views/api/callbacks/delivery-status-callback.html index 7c5ee805ee..211ed01a39 100644 --- a/app/templates/views/api/callbacks/delivery-status-callback.html +++ b/app/templates/views/api/callbacks/delivery-status-callback.html @@ -1,7 +1,7 @@ {% extends "admin_template.html" %} {% from "components/textbox.html" import textbox %} {% from "components/page-header.html" import page_header %} -{% from "components/page-footer.html" import page_footer, sticky_page_footer_two_submit_buttons_and_delete_link %} +{% from "components/page-footer.html" import sticky_page_footer_two_submit_buttons_and_delete_link %} {% from "components/form.html" import form_wrapper %} {% from "components/ajax-block.html" import ajax_block %} @@ -56,5 +56,4 @@

{% endcall %} - {% endblock %} diff --git a/app/templates/views/api/callbacks/received-text-messages-callback.html b/app/templates/views/api/callbacks/received-text-messages-callback.html index 4d68a923d3..49fab276b6 100644 --- a/app/templates/views/api/callbacks/received-text-messages-callback.html +++ b/app/templates/views/api/callbacks/received-text-messages-callback.html @@ -1,7 +1,7 @@ {% extends "admin_template.html" %} {% from "components/textbox.html" import textbox %} {% from "components/page-header.html" import page_header %} -{% from "components/page-footer.html" import page_footer %} +{% from "components/page-footer.html" import sticky_page_footer_two_submit_buttons_and_delete_link %} {% from "components/form.html" import form_wrapper %} {% block service_page_title %} @@ -25,6 +25,9 @@ width='w-full', hint=hint_text ) }} +

+ {{ _('Add a secret value for authentication') }} +

{% set hint_txt = _('At least 10 characters') %} {{ textbox( form.bearer_token, @@ -32,8 +35,19 @@ hint=hint_txt, autocomplete='new-password' ) }} - {% set button_txt = _('Save') %} - {{ page_footer(button_txt) }} + {% set test_response_txt = _('Test response time') if has_callback_config else None %} + {% set test_response_value = _('test_response_time') if has_callback_config else None %} + {% set display_footer = is_deleting if is_deleting else False %} + {% if not display_footer %} + {{ sticky_page_footer_two_submit_buttons_and_delete_link( + button1_text=_('Save'), + button1_value=_('save'), + button2_text=test_response_txt, + button2_value=test_response_value, + delete_link=url_for('.delete_received_text_messages_callback', service_id=current_service.id), + delete_link_text=_('Delete') + ) }} + {% endif %} {% endcall %} diff --git a/app/translations/csv/fr.csv b/app/translations/csv/fr.csv index 9b561ea0f0..20b646950e 100644 --- a/app/translations/csv/fr.csv +++ b/app/translations/csv/fr.csv @@ -1981,7 +1981,7 @@ "Review your activity","Examinez votre activité" "Set sensitive service","Définir un service sensible" "Suspend Callback","Suspension du rappel" -"Callback to '{}' created. Your service responded in '{}'","(FR) Callback to '{}' created. Your service responded in '{}'" -"Callback to '{}' saved. Your service responded in '{}'","(FR) Callback to '{}' saved. Your service responded in '{}'" +"Callback to '{}' created. Your service responded in {}","(FR) Callback to '{}' created. Your service responded in '{}'" +"Callback to '{}' saved. Your service responded in {}","(FR) Callback to '{}' saved. Your service responded in '{}'" "Your service must be running and reachable from the internet","(FR) Your service must be running and reachable from the internet" "We were unable to contact your callback service, please try again.","(FR)We were unable to contact your callback service, please try again." \ No newline at end of file diff --git a/tests/app/main/views/test_api_integration.py b/tests/app/main/views/test_api_integration.py index 1c8dc01547..79101a1266 100644 --- a/tests/app/main/views/test_api_integration.py +++ b/tests/app/main/views/test_api_integration.py @@ -1,5 +1,6 @@ import uuid from collections import OrderedDict +from datetime import timedelta from unittest.mock import Mock, call import pytest @@ -61,7 +62,8 @@ def test_should_show_api_page_with_no_notifications( service_id=SERVICE_ONE_ID, ) rows = page.find_all("div", {"class": "api-notifications-item"}) - assert "When you send messages via the API they’ll appear here." in rows[len(rows) - 1].text.strip() + assert "When you send messages via the API they’ll appear here." in rows[len( + rows) - 1].text.strip() @pytest.mark.parametrize( @@ -80,7 +82,8 @@ def test_letter_notifications_should_have_link_to_view_letter( link_text, ): notifications = create_notifications(template_type=template_type) - mocker.patch("app.notification_api_client.get_api_notifications_for_service", return_value=notifications) + mocker.patch("app.notification_api_client.get_api_notifications_for_service", + return_value=notifications) page = client_request.get( "main.api_integration", service_id=SERVICE_ONE_ID, @@ -94,7 +97,8 @@ def test_should_not_have_link_to_view_letter_for_precompiled_letters_in_virus_st client_request, fake_uuid, mock_has_permissions, mocker, status ): notifications = create_notifications(status=status) - mocker.patch("app.notification_api_client.get_api_notifications_for_service", return_value=notifications) + mocker.patch("app.notification_api_client.get_api_notifications_for_service", + return_value=notifications) page = client_request.get( "main.api_integration", @@ -120,7 +124,8 @@ def test_letter_notifications_should_show_client_reference( shows_ref, ): notifications = create_notifications(client_reference=client_reference) - mocker.patch("app.notification_api_client.get_api_notifications_for_service", return_value=notifications) + mocker.patch("app.notification_api_client.get_api_notifications_for_service", + return_value=notifications) page = client_request.get( "main.api_integration", @@ -171,7 +176,8 @@ def test_should_show_empty_api_keys_page( response = client.get(url_for("main.api_keys", service_id=service_id)) assert response.status_code == 200 - assert "You have not created any API keys yet" in response.get_data(as_text=True) + assert "You have not created any API keys yet" in response.get_data( + as_text=True) assert "Create API key" in response.get_data(as_text=True) mock_get_no_api_keys.assert_called_once_with(service_id) @@ -185,7 +191,8 @@ def test_should_show_api_keys_page( rows = [normalize_spaces(row.text) for row in page.select("main tr")] assert rows[0] == "API keys Action" - assert "another key name 20 total sends in the last 7 days (20 email, 0 sms)" in rows[1] + assert "another key name 20 total sends in the last 7 days (20 email, 0 sms)" in rows[ + 1] assert "Revoke API key some key name" in rows[2] mock_get_api_keys.assert_called_once_with(SERVICE_ONE_ID) @@ -237,12 +244,14 @@ def test_should_show_create_api_key_page( if can_send_letters: service_one["permissions"].append("letter") - mocker.patch("app.service_api_client.get_service", return_value={"data": service_one}) + mocker.patch("app.service_api_client.get_service", + return_value={"data": service_one}) page = client_request.get("main.create_api_key", service_id=SERVICE_ONE_ID) for index, option in enumerate(expected_options): - assert normalize_spaces(page.select(".block-label")[index].text) == option + assert normalize_spaces(page.select( + ".block-label")[index].text) == option def test_should_create_api_key_with_type_normal( @@ -269,7 +278,8 @@ def test_should_create_api_key_with_type_normal( _expected_status=200, ) - assert page.select_one("span.api-key-key").text.strip() == ("{}-{}-{}".format(key_name_fixed, SERVICE_ONE_ID, fake_uuid)) + assert page.select_one("span.api-key-key").text.strip() == ( + "{}-{}-{}".format(key_name_fixed, SERVICE_ONE_ID, fake_uuid)) post.assert_called_once_with( url="/service/{}/api-key".format(SERVICE_ONE_ID), @@ -291,7 +301,8 @@ def test_cant_create_normal_api_key_in_trial_mode( fake_uuid, mocker, ): - mock_post = mocker.patch("app.notify_client.api_key_api_client.ApiKeyApiClient.post") + mock_post = mocker.patch( + "app.notify_client.api_key_api_client.ApiKeyApiClient.post") client_request.post( "main.create_api_key", @@ -378,7 +389,8 @@ def test_should_redirect_after_revoking_api_key( service_id=SERVICE_ONE_ID, ), ) - mock_revoke_api_key.assert_called_once_with(service_id=SERVICE_ONE_ID, key_id=fake_uuid) + mock_revoke_api_key.assert_called_once_with( + service_id=SERVICE_ONE_ID, key_id=fake_uuid) mock_get_api_keys.assert_called_once_with( SERVICE_ONE_ID, ) @@ -443,7 +455,8 @@ def test_should_show_safelist_page( "main.safelist", service_id=SERVICE_ONE_ID, ) - textboxes = page.find_all("input", {"type": "email"}) + page.find_all("input", {"type": "tel"}) + textboxes = page.find_all( + "input", {"type": "email"}) + page.find_all("input", {"type": "tel"}) for index, value in enumerate(["test@example.com"] + [""] * 4 + ["6502532222"] + [""] * 4): assert textboxes[index]["value"] == value @@ -483,11 +496,13 @@ def test_should_validate_safelist_items( page = client_request.post( "main.safelist", service_id=SERVICE_ONE_ID, - _data=OrderedDict([("email_addresses-1", "abc"), ("phone_numbers-0", "123")]), + _data=OrderedDict([("email_addresses-1", "abc"), + ("phone_numbers-0", "123")]), _expected_status=200, ) - assert page.select_one(".banner-title").string.strip() == "There was a problem with your safelist" + assert page.select_one( + ".banner-title").string.strip() == "There was a problem with your safelist" jump_links = page.select(".banner-dangerous a") assert jump_links[0].string.strip() == "Enter valid email addresses" @@ -510,12 +525,13 @@ def test_should_validate_safelist_items( "url, bearer_token, response, expected_errors", [ ("https://example.com", "", None, "This cannot be empty"), - ("http://not_https.com", "1234567890", None, "Enter a URL that starts with https://"), + ("http://not_https.com", "1234567890", None, + "Enter a URL that starts with https://"), ( "https://test.com", "123456789", {"content": "a", "status_code": 500, "headers": {"a": "a"}}, - "Check your service log for errors Must be at least 10 characters", + "Must be at least 10 characters", ), ( "https://test.ee", @@ -544,21 +560,23 @@ def test_callback_forms_validation( "bearer_token": bearer_token, } if response: - resp = Mock(content=response["content"], status_code=response["status_code"], headers=response["headers"]) + resp = Mock( + content=response["content"], status_code=response["status_code"], headers=response["headers"]) mocker.patch("app.main.validators.requests.post", return_value=resp) - response = client_request.post(endpoint, service_id=service_one["id"], _data=data, _expected_status=200) - error_msgs = " ".join(msg.text.strip() for msg in response.select(".error-message")) + response = client_request.post( + endpoint, service_id=service_one["id"], _data=data, _expected_status=200) + error_msgs = " ".join(msg.text.strip() + for msg in response.select(".error-message")) assert error_msgs == expected_errors -@pytest.mark.parametrize("bearer_token", ["", "some-bearer-token"]) @pytest.mark.parametrize( "endpoint, expected_delete_url", [ ( - "main.delivery_status_callback", + "main.delete_delivery_status_callback", "/service/{}/delivery-receipt-api/{}", ), ( @@ -567,12 +585,11 @@ def test_callback_forms_validation( ), ], ) -def test_callback_forms_can_be_cleared( +def test_delete_callback( client_request, service_one, endpoint, expected_delete_url, - bearer_token, mocker, fake_uuid, mock_get_valid_service_callback_api, @@ -586,18 +603,12 @@ def test_callback_forms_can_be_cleared( page = client_request.post( endpoint, service_id=service_one["id"], - _data={ - "url": "", - "bearer_token": bearer_token, - }, - _expected_redirect=url_for( - "main.api_callbacks", - service_id=service_one["id"], - ), + _follow_redirects=True, ) assert not page.select(".error-message") - mocked_delete.assert_called_once_with(expected_delete_url.format(service_one["id"], fake_uuid)) + mocked_delete.assert_called_once_with( + expected_delete_url.format(service_one["id"], fake_uuid)) @pytest.mark.parametrize("bearer_token", ["", "some-bearer-token"]) @@ -667,7 +678,8 @@ def test_callbacks_button_links_straight_to_delivery_status_if_service_has_no_in service_id=service_one["id"], ) - assert page.select(".api-header-links")[2]["href"] == url_for(expected_link, service_id=service_one["id"]) + assert page.select( + ".api-header-links")[2]["href"] == url_for(expected_link, service_id=service_one["id"]) def test_callbacks_page_redirects_to_delivery_status_if_service_has_no_inbound_sms( @@ -682,7 +694,8 @@ def test_callbacks_page_redirects_to_delivery_status_if_service_has_no_inbound_s _follow_redirects=True, ) - assert normalize_spaces(page.select_one("h1").text) == "Callbacks for delivery receipts" + assert normalize_spaces(page.select_one( + "h1").text) == "Callbacks for delivery receipts" @pytest.mark.parametrize( @@ -704,7 +717,8 @@ def test_back_link_directs_to_api_integration_from_delivery_callback_if_no_inbou _follow_redirects=True, ) - assert page.select_one(".back-link")["href"] == url_for(expected_link, service_id=service_one["id"]) + assert page.select_one( + ".back-link")["href"] == url_for(expected_link, service_id=service_one["id"]) @pytest.mark.parametrize( @@ -763,6 +777,7 @@ def test_update_delivery_status_callback_details( mock_get_valid_service_callback_api, fake_uuid, mock_validate_callback_url, + mocker, ): service_one["service_callback_api"] = [fake_uuid] @@ -794,7 +809,8 @@ def test_update_receive_text_message_callback_details( service_one["inbound_api"] = [fake_uuid] service_one["permissions"] = ["inbound_sms"] - data = {"url": "https://test.url.com/", "bearer_token": "1234567890", "user_id": fake_uuid} + data = {"url": "https://test.url.com/", + "bearer_token": "1234567890", "user_id": fake_uuid} client_request.post( "main.received_text_messages_callback", @@ -818,9 +834,11 @@ def test_update_delivery_status_callback_without_changes_does_not_update( fake_uuid, mock_get_valid_service_callback_api, mock_validate_callback_url, + mocker, ): service_one["service_callback_api"] = [fake_uuid] - data = {"user_id": fake_uuid, "url": "https://hello2.canada.ca", "bearer_token": "bearer_token_set"} + data = {"user_id": fake_uuid, "url": "https://hello2.canada.ca", + "bearer_token": "bearer_token_set"} client_request.post( "main.delivery_status_callback", @@ -841,7 +859,8 @@ def test_update_receive_text_message_callback_without_changes_does_not_update( ): service_one["inbound_api"] = [fake_uuid] service_one["permissions"] = ["inbound_sms"] - data = {"user_id": fake_uuid, "url": "https://hello3.canada.ca", "bearer_token": "bearer_token_set"} + data = {"user_id": fake_uuid, "url": "https://hello3.canada.ca", + "bearer_token": "bearer_token_set"} client_request.post( "main.received_text_messages_callback", @@ -889,10 +908,13 @@ def test_callbacks_page_works_when_no_apis_set( service_one["inbound_api"] = inbound_api service_one["service_callback_api"] = service_callback_api - mocker.patch("app.service_api_client.get_service_callback_api", return_value=delivery_url) - mocker.patch("app.service_api_client.get_service_inbound_api", return_value=inbound_url) + mocker.patch("app.service_api_client.get_service_callback_api", + return_value=delivery_url) + mocker.patch("app.service_api_client.get_service_inbound_api", + return_value=inbound_url) - page = client_request.get("main.api_callbacks", service_id=service_one["id"], _follow_redirects=True) + page = client_request.get( + "main.api_callbacks", service_id=service_one["id"], _follow_redirects=True) expected_rows = [ expected_1st_table_row, expected_2nd_table_row, diff --git a/tests/conftest.py b/tests/conftest.py index 47b6567393..3913b28292 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -3736,7 +3736,7 @@ def mock_get_empty_service_callback_api(mocker): @pytest.fixture(scope="function") def mock_validate_callback_url(mocker): - return mocker.patch("app.main.validators.requests.post", return_value=Mock(content="a", status_code=200, headers={"a": "a"})) + return mocker.patch("app.main.validators.requests.post", return_value=Mock(content="a", status_code=200, headers={"a": "a"}, elapsed=timedelta(seconds=0.3))) @pytest.fixture(scope="function") From 193e2639a05bef003aad0a1591fd491ff6b230cc Mon Sep 17 00:00:00 2001 From: wbanks Date: Mon, 9 Sep 2024 16:32:13 -0400 Subject: [PATCH 10/22] formatting fixes --- app/main/forms.py | 201 ++++++++++++++--------------------------- app/main/validators.py | 30 ++---- 2 files changed, 77 insertions(+), 154 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index fbf42d06e4..3786a77a17 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -66,10 +66,8 @@ def get_time_value_and_label(future_time): return ( future_time.replace(tzinfo=None).isoformat(), "{} at {}".format( - get_human_day(future_time.astimezone( - pytz.timezone("Europe/London"))), - get_human_time(future_time.astimezone( - pytz.timezone("Europe/London"))), + get_human_day(future_time.astimezone(pytz.timezone("Europe/London"))), + get_human_time(future_time.astimezone(pytz.timezone("Europe/London"))), ), ) @@ -257,8 +255,7 @@ class OrganisationTypeField(RadioField): def __init__(self, *args, include_only=None, validators=None, **kwargs): super().__init__( *args, - choices=[(value, label) for value, - label in Organisation.TYPES if not include_only or value in include_only], + choices=[(value, label) for value, label in Organisation.TYPES if not include_only or value in include_only], validators=validators or [], **kwargs, ) @@ -298,22 +295,18 @@ class RadioFieldWithNoneOption(FieldWithNoneOption, RadioField): class NestedFieldMixin: def children(self): # start map with root option as a single child entry - child_map = { - None: [option for option in self if option.data == self.NONE_OPTION_VALUE]} + child_map = {None: [option for option in self if option.data == self.NONE_OPTION_VALUE]} # add entries for all other children for option in self: if option.data == self.NONE_OPTION_VALUE: - child_ids = [ - folder["id"] for folder in self.all_template_folders if folder["parent_id"] is None] + child_ids = [folder["id"] for folder in self.all_template_folders if folder["parent_id"] is None] key = self.NONE_OPTION_VALUE else: - child_ids = [ - folder["id"] for folder in self.all_template_folders if folder["parent_id"] == option.data] + child_ids = [folder["id"] for folder in self.all_template_folders if folder["parent_id"] == option.data] key = option.data - child_map[key] = [ - option for option in self if option.data in child_ids] + child_map[key] = [option for option in self if option.data in child_ids] return child_map @@ -348,8 +341,7 @@ def bind_field(self, form, unbound_field, options): # FieldList simply doesn't support filters. # @see: https://github.com/wtforms/wtforms/issues/148 no_filter_fields = (FieldList, PasswordField) - filters = [strip_whitespace] if not issubclass( - unbound_field.field_class, no_filter_fields) else [] + filters = [strip_whitespace] if not issubclass(unbound_field.field_class, no_filter_fields) else [] filters += unbound_field.kwargs.get("filters", []) bound = unbound_field.bind(form=form, filters=filters, **options) # GC won't collect the form if we don't use a weakref @@ -377,13 +369,11 @@ class LoginForm(StripWhitespaceForm): ValidEmail(), ], ) - password = PasswordField(_l("Password"), validators=[ - DataRequired(message=_l("Enter your password"))]) + password = PasswordField(_l("Password"), validators=[DataRequired(message=_l("Enter your password"))]) class RegisterUserForm(StripWhitespaceForm): - name = StringField(_l("Full name"), validators=[ - DataRequired(message=_l("This cannot be empty"))]) + name = StringField(_l("Full name"), validators=[DataRequired(message=_l("This cannot be empty"))]) email_address = email_address() mobile_number = international_phone_number() password = password() @@ -406,8 +396,7 @@ def __init__(self, invited_user): name=guess_name_from_email_address(invited_user.email_address), ) - mobile_number = InternationalPhoneNumber( - _l("Mobile number"), validators=[]) + mobile_number = InternationalPhoneNumber(_l("Mobile number"), validators=[]) service = HiddenField("service") email_address = HiddenField("email_address") auth_type = HiddenField("auth_type", validators=[DataRequired()]) @@ -424,8 +413,7 @@ def __init__(self, invited_org_user): email_address=invited_org_user.email_address, ) - name = StringField("Full name", validators=[ - DataRequired(message=_l("This cannot be empty"))]) + name = StringField("Full name", validators=[DataRequired(message=_l("This cannot be empty"))]) mobile_number = InternationalPhoneNumber( _l("Mobile number"), @@ -454,8 +442,7 @@ def __init__(self, all_template_folders=None, *args, **kwargs): (item["id"], item["name"]) for item in ([{"name": _l("Templates"), "id": None}] + all_template_folders) ] - folder_permissions = NestedCheckboxesField( - _l("Folders this team member can see")) + folder_permissions = NestedCheckboxesField(_l("Folders this team member can see")) login_authentication = RadioField( _l("Sign in using"), @@ -491,8 +478,7 @@ def __init__(self, invalid_email_address, *args, **kwargs): def validate_email_address(self, field): if field.data.lower() == self.invalid_email_address: - raise ValidationError( - _l("You cannot send an invitation to yourself")) + raise ValidationError(_l("You cannot send an invitation to yourself")) class InviteOrgUserForm(StripWhitespaceForm): @@ -504,8 +490,7 @@ def __init__(self, invalid_email_address, *args, **kwargs): def validate_email_address(self, field): if field.data.lower() == self.invalid_email_address: - raise ValidationError( - _l("You cannot send an invitation to yourself")) + raise ValidationError(_l("You cannot send an invitation to yourself")) class TwoFactorForm(StripWhitespaceForm): @@ -668,8 +653,7 @@ class CreateServiceStepCombinedOrganisationForm(StripWhitespaceForm): class CreateServiceStepOtherOrganisationForm(StripWhitespaceForm): other_organisation_name = StringField( _l("Enter name of your group"), - validators=[DataRequired(message=_l( - "Enter name to continue")), Length(max=500)], + validators=[DataRequired(message=_l("Enter name to continue")), Length(max=500)], ) @@ -762,8 +746,7 @@ class EmailMessageLimit(StripWhitespaceForm): class SMSMessageLimit(StripWhitespaceForm): message_limit = IntegerField( _l("Daily text message limit"), - validators=[DataRequired(message=_l( - "This cannot be empty")), validators.NumberRange(min=1)], + validators=[DataRequired(message=_l("This cannot be empty")), validators.NumberRange(min=1)], ) @@ -830,8 +813,7 @@ def validate_template_content(self, field): # TODO: Remove this class when FF_TEMPLATE_CATEGORY is removed class EmailTemplateForm(BaseTemplateForm): - subject = TextAreaField(_l("Subject line of the email"), validators=[ - DataRequired(message=_l("This cannot be empty"))]) + subject = TextAreaField(_l("Subject line of the email"), validators=[DataRequired(message=_l("This cannot be empty"))]) template_content = TextAreaField( _l("Email message"), @@ -844,8 +826,7 @@ class EmailTemplateForm(BaseTemplateForm): # TODO: Remove this class when FF_TEMPLATE_CATEGORY is removed class LetterTemplateForm(EmailTemplateForm): - subject = TextAreaField("Main heading", validators=[ - DataRequired(message="This cannot be empty")]) + subject = TextAreaField("Main heading", validators=[DataRequired(message="This cannot be empty")]) template_content = TextAreaField( "Body", @@ -868,15 +849,13 @@ def __init__(self, other_field_name, other_field_value, *args, **kwargs): def __call__(self, form, field): other_field = form._fields.get(self.other_field_name) if other_field is None: - raise Exception('no field named "%s" in form' % - self.other_field_name) + raise Exception('no field named "%s" in form' % self.other_field_name) if bool(other_field.data and other_field.data == self.other_field_value): super(RequiredIf, self).__call__(form, field) class BaseTemplateFormWithCategory(BaseTemplateForm): - template_category_id = RadioField(_l("Select category"), validators=[ - DataRequired(message=_l("This cannot be empty"))]) + template_category_id = RadioField(_l("Select category"), validators=[DataRequired(message=_l("This cannot be empty"))]) template_category_other = StringField( _l("Describe category"), validators=[RequiredIf("template_category_id", DefaultTemplateCategories.LOW.value)] @@ -897,8 +876,7 @@ def validate_template_content(self, field): class EmailTemplateFormWithCategory(BaseTemplateFormWithCategory): - subject = TextAreaField(_l("Subject line of the email"), validators=[ - DataRequired(message=_l("This cannot be empty"))]) + subject = TextAreaField(_l("Subject line of the email"), validators=[DataRequired(message=_l("This cannot be empty"))]) template_content = TextAreaField( _l("Email message"), @@ -910,8 +888,7 @@ class EmailTemplateFormWithCategory(BaseTemplateFormWithCategory): class LetterTemplateFormWithCategory(EmailTemplateFormWithCategory): - subject = TextAreaField("Main heading", validators=[ - DataRequired(message="This cannot be empty")]) + subject = TextAreaField("Main heading", validators=[DataRequired(message="This cannot be empty")]) template_content = TextAreaField( "Body", @@ -956,8 +933,7 @@ def validate_old_password(self, field): class CsvUploadForm(StripWhitespaceForm): file = FileField( _l("Choose a file"), - validators=[DataRequired( - message="Please pick a file"), CsvFileValidator()], + validators=[DataRequired(message="Please pick a file"), CsvFileValidator()], ) @@ -992,8 +968,7 @@ def __init__(self, *args, **kwargs): self.scheduled_for.choices = [("", "Now")] + [ get_time_value_and_label(hour) for hour in get_next_hours_until(get_furthest_possible_scheduled_time()) ] - self.scheduled_for.categories = get_next_days_until( - get_furthest_possible_scheduled_time()) + self.scheduled_for.categories = get_next_days_until(get_furthest_possible_scheduled_time()) scheduled_for = RadioField( _l("When should we send these messages?"), @@ -1003,8 +978,7 @@ def __init__(self, *args, **kwargs): class CreateKeyForm(StripWhitespaceForm): def __init__(self, existing_keys, *args, **kwargs): - self.existing_key_names = [ - key["name"].lower() for key in existing_keys if not key["expiry_date"]] + self.existing_key_names = [key["name"].lower() for key in existing_keys if not key["expiry_date"]] super().__init__(*args, **kwargs) key_type = RadioField( @@ -1013,8 +987,7 @@ def __init__(self, existing_keys, *args, **kwargs): key_name = StringField( "Description of key", - validators=[DataRequired(message=_l( - "You need to give the key a name")), Length(max=255)], + validators=[DataRequired(message=_l("You need to give the key a name")), Length(max=255)], ) def validate_key_name(self, key_name): @@ -1039,8 +1012,7 @@ def validate_number(self, number): class ContactNotify(StripWhitespaceForm): not_empty = _l("Enter your name") - name = StringField(_l("Your name"), validators=[ - DataRequired(message=not_empty), Length(max=255)]) + name = StringField(_l("Your name"), validators=[DataRequired(message=not_empty), Length(max=255)]) support_type = RadioField( _l("How can we help?"), choices=[ @@ -1056,8 +1028,7 @@ class ContactNotify(StripWhitespaceForm): class ContactMessageStep(ContactNotify): message = TextAreaField( _l("Message"), - validators=[DataRequired(message=_l( - "You need to enter something if you want to contact us")), Length(max=2000)], + validators=[DataRequired(message=_l("You need to enter something if you want to contact us")), Length(max=2000)], ) @@ -1071,8 +1042,7 @@ def __init__(self, *args, **kwargs): branding_style = SelectField() file = FileField_wtf( _l("Upload logo"), - validators=[FileAllowed(["png"], _l( - "Your logo must be an image in PNG format"))], + validators=[FileAllowed(["png"], _l("Your logo must be an image in PNG format"))], ) @@ -1089,8 +1059,7 @@ class Triage(StripWhitespaceForm): class ProviderForm(StripWhitespaceForm): priority = IntegerField( "Priority", - [validators.NumberRange( - min=1, max=100, message="Must be between 1 and 100")], + [validators.NumberRange(min=1, max=100, message="Must be between 1 and 100")], ) @@ -1110,8 +1079,7 @@ class ServiceContactDetailsForm(StripWhitespaceForm): def validate(self, extra_validators=None): if self.contact_details_type.data == "url": - self.url.validators = [DataRequired(), URL( - message="Must be a valid URL")] + self.url.validators = [DataRequired(), URL(message="Must be a valid URL")] elif self.contact_details_type.data == "email_address": self.email_address.validators = [ @@ -1183,8 +1151,7 @@ class ServiceLetterContactBlockForm(StripWhitespaceForm): def validate_letter_contact_block(self, field): line_count = field.data.strip().count("\n") if line_count >= 10: - raise ValidationError( - "Contains {} lines, maximum is 10".format(line_count + 1)) + raise ValidationError("Contains {} lines, maximum is 10".format(line_count + 1)) class OnOffField(RadioField): @@ -1202,8 +1169,7 @@ def __init__(self, label, *args, **kwargs): def process_formdata(self, valuelist): if valuelist: value = valuelist[0] - self.data = (value == "True") if value in [ - "True", "False"] else value + self.data = (value == "True") if value in ["True", "False"] else value class ServiceOnOffSettingForm(StripWhitespaceForm): @@ -1365,16 +1331,14 @@ def populate(self, email_addresses, phone_numbers): form_field[index].data = value email_addresses = FieldList( - EmailFieldInSafelist( - "", validators=[Optional(), ValidEmail()], default=""), + EmailFieldInSafelist("", validators=[Optional(), ValidEmail()], default=""), min_entries=5, max_entries=5, label=_l("Email safelist"), ) phone_numbers = FieldList( - InternationalPhoneNumberInSafelist( - "", validators=[Optional()], default=""), + InternationalPhoneNumberInSafelist("", validators=[Optional()], default=""), min_entries=5, max_entries=5, label=_l("Phone safelist"), @@ -1384,8 +1348,7 @@ def populate(self, email_addresses, phone_numbers): class DateFilterForm(StripWhitespaceForm): start_date = DateField(_l("Start Date"), [validators.optional()]) end_date = DateField(_l("End Date"), [validators.optional()]) - include_from_test_key = BooleanField( - _l("Include test keys"), default="checked", false_values={"N"}) + include_from_test_key = BooleanField(_l("Include test keys"), default="checked", false_values={"N"}) class RequiredDateFilterForm(StripWhitespaceForm): @@ -1396,16 +1359,14 @@ class RequiredDateFilterForm(StripWhitespaceForm): class SearchByNameForm(StripWhitespaceForm): search = SearchField( _l("Search by name"), - validators=[DataRequired( - "You need to enter full or partial name to search by.")], + validators=[DataRequired("You need to enter full or partial name to search by.")], ) class SearchUsersByEmailForm(StripWhitespaceForm): search = SearchField( _l("Search by name or email address"), - validators=[DataRequired( - _l("You need to enter full or partial email address to search by."))], + validators=[DataRequired(_l("You need to enter full or partial email address to search by."))], ) @@ -1457,8 +1418,7 @@ class ServiceReceiveMessagesCallbackForm(CallbackForm): "URL", validators=[ DataRequired(message=_l("This cannot be empty")), - Regexp(regex="^https.*", - message=_l("Enter a URL that starts with https://")), + Regexp(regex="^https.*", message=_l("Enter a URL that starts with https://")), ValidCallbackUrl(), ], ) @@ -1476,8 +1436,7 @@ class ServiceDeliveryStatusCallbackForm(CallbackForm): "URL", validators=[ DataRequired(message=_l("This cannot be empty")), - Regexp(regex="^https.*", - message=_l("Enter a URL that starts with https://")), + Regexp(regex="^https.*", message=_l("Enter a URL that starts with https://")), ValidCallbackUrl(), ], ) @@ -1527,12 +1486,10 @@ def get_placeholder_form_instance( else: field = uk_mobile_number(label=placeholder_name) elif optional_placeholder: - field = StringField( - _l("What is the custom content in (({})) ?").format(placeholder_name)) + field = StringField(_l("What is the custom content in (({})) ?").format(placeholder_name)) elif is_conditional: field = RadioField( - _l("Do you want to include the content in (({})) ?").format( - placeholder_name), + _l("Do you want to include the content in (({})) ?").format(placeholder_name), choices=[ ("yes", _l("Yes")), ("no", _l("No")), @@ -1604,16 +1561,14 @@ class ServiceDataRetentionForm(StripWhitespaceForm): ) days_of_retention = IntegerField( label="Days of retention", - validators=[validators.NumberRange( - min=3, max=7, message="Must be between 3 and 7")], + validators=[validators.NumberRange(min=3, max=7, message="Must be between 3 and 7")], ) class ServiceDataRetentionEditForm(StripWhitespaceForm): days_of_retention = IntegerField( label="Days of retention", - validators=[validators.NumberRange( - min=3, max=7, message="Must be between 3 and 7")], + validators=[validators.NumberRange(min=3, max=7, message="Must be between 3 and 7")], ) @@ -1631,13 +1586,10 @@ def __init__(self, all_service_users=None, *args, **kwargs): super().__init__(*args, **kwargs) if all_service_users is not None: self.users_with_permission.all_service_users = all_service_users - self.users_with_permission.choices = [ - (item.id, item.name) for item in all_service_users] + self.users_with_permission.choices = [(item.id, item.name) for item in all_service_users] - users_with_permission = MultiCheckboxField( - _l("Team members who can see this folder")) - name = StringField(_l("Folder name"), validators=[ - DataRequired(message=_l("This cannot be empty"))]) + users_with_permission = MultiCheckboxField(_l("Team members who can see this folder")) + name = StringField(_l("Folder name"), validators=[DataRequired(message=_l("This cannot be empty"))]) def required_for_ops(*operations): @@ -1664,23 +1616,19 @@ def __init__(self, *args, **kwargs): class AddEmailRecipientsForm(Form): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - self.what_type.choices = [("many_recipients", _l( - "Many recipients")), ("one_recipient", _l("One recipient"))] + self.what_type.choices = [("many_recipients", _l("Many recipients")), ("one_recipient", _l("One recipient"))] what_type = RadioField("") - placeholder_value = email_address( - _l("Email address of recipient"), gov_user=False) + placeholder_value = email_address(_l("Email address of recipient"), gov_user=False) class AddSMSRecipientsForm(Form): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - self.what_type.choices = [("many_recipients", _l( - "Many recipients")), ("one_recipient", _l("One recipient"))] + self.what_type.choices = [("many_recipients", _l("Many recipients")), ("one_recipient", _l("One recipient"))] what_type = RadioField("") - placeholder_value = international_phone_number( - _l("Phone number of recipient")) + placeholder_value = international_phone_number(_l("Phone number of recipient")) class TemplateAndFoldersSelectionForm(Form): @@ -1722,8 +1670,7 @@ def __init__( self.is_move_op = self.is_add_folder_op = False self.move_to.all_template_folders = all_template_folders - self.move_to.choices = [(item["id"], item["name"]) for item in ( - [self.ALL_TEMPLATES_FOLDER] + all_template_folders)] + self.move_to.choices = [(item["id"], item["name"]) for item in ([self.ALL_TEMPLATES_FOLDER] + all_template_folders)] def is_selected(self, template_folder_id): return template_folder_id in (self.templates_and_folders.data or []) @@ -1731,10 +1678,8 @@ def is_selected(self, template_folder_id): def validate(self, extra_validators=None): self.op = request.form.get("operation") - self.is_move_op = self.op in { - "move-to-existing-folder", "move-to-new-folder"} - self.is_add_folder_op = self.op in { - "add-new-folder", "move-to-new-folder"} + self.is_move_op = self.op in {"move-to-existing-folder", "move-to-new-folder"} + self.is_add_folder_op = self.op in {"add-new-folder", "move-to-new-folder"} if not (self.is_add_folder_op or self.is_move_op): return False @@ -1750,8 +1695,7 @@ def get_folder_name(self): templates_and_folders = MultiCheckboxField( "Choose templates or folders", - validators=[required_for_ops( - "move-to-new-folder", "move-to-existing-folder")], + validators=[required_for_ops("move-to-new-folder", "move-to-existing-folder")], ) # if no default set, it is set to None, which process_data transforms to '__NONE__' # this means '__NONE__' (self.ALL_TEMPLATES option) is selected when no form data has been submitted @@ -1761,10 +1705,8 @@ def get_folder_name(self): default="", validators=[required_for_ops("move-to-existing-folder"), Optional()], ) - add_new_folder_name = StringField(_l("Folder name"), validators=[ - required_for_ops("add-new-folder")]) - move_to_new_folder_name = StringField(_l("Folder name"), validators=[ - required_for_ops("move-to-new-folder")]) + add_new_folder_name = StringField(_l("Folder name"), validators=[required_for_ops("add-new-folder")]) + move_to_new_folder_name = StringField(_l("Folder name"), validators=[required_for_ops("move-to-new-folder")]) class ClearCacheForm(StripWhitespaceForm): @@ -1797,8 +1739,7 @@ def from_organisation(cls, org): on_behalf_of_email=org.agreement_signed_on_behalf_of_email_address, ) - version = StringField( - "Which version of the agreement do you want to accept?") + version = StringField("Which version of the agreement do you want to accept?") who = RadioField( "Who are you accepting the agreement for?", @@ -1933,8 +1874,7 @@ class BrandingGOCForm(StripWhitespaceForm): (FieldWithLanguageOptions.ENGLISH_OPTION_VALUE, _l("English-first")), (FieldWithLanguageOptions.FRENCH_OPTION_VALUE, _l("French-first")), ], - validators=[DataRequired(message=_l( - "You must select an option to continue"))], + validators=[DataRequired(message=_l("You must select an option to continue"))], ) def __init__(self, *args, **kwargs): @@ -1953,8 +1893,7 @@ class BrandingPoolForm(StripWhitespaceForm): _l("Select alternate logo"), # Choices by default, override to get more refined options. choices=[], - validators=[DataRequired(message=_l( - "You must select an option to continue"))], + validators=[DataRequired(message=_l("You must select an option to continue"))], ) def __init__(self, *args, **kwargs): @@ -1970,12 +1909,9 @@ class BrandingRequestForm(StripWhitespaceForm): file (FileField_wtf): Field for uploading the logo file. """ - name = StringField(label=_l("Name of logo"), validators=[ - DataRequired(message=_l("Enter the name of the logo"))]) - alt_text_en = StringField(label=_l("English"), validators=[ - DataRequired(message=_l("This cannot be empty"))]) - alt_text_fr = StringField(label=_l("French"), validators=[ - DataRequired(message=_l("This cannot be empty"))]) + name = StringField(label=_l("Name of logo"), validators=[DataRequired(message=_l("Enter the name of the logo"))]) + alt_text_en = StringField(label=_l("English"), validators=[DataRequired(message=_l("This cannot be empty"))]) + alt_text_fr = StringField(label=_l("French"), validators=[DataRequired(message=_l("This cannot be empty"))]) file = FileField_wtf( label=_l("Prepare your logo"), validators=[ @@ -1985,14 +1921,11 @@ class BrandingRequestForm(StripWhitespaceForm): class TemplateCategoryForm(StripWhitespaceForm): - name_en = StringField( - "EN", validators=[DataRequired(message=_l("This cannot be empty"))]) - name_fr = StringField( - "FR", validators=[DataRequired(message=_l("This cannot be empty"))]) + name_en = StringField("EN", validators=[DataRequired(message=_l("This cannot be empty"))]) + name_fr = StringField("FR", validators=[DataRequired(message=_l("This cannot be empty"))]) description_en = StringField("EN") description_fr = StringField("FR") - hidden = RadioField(_l("Hide category"), choices=[ - ("True", _l("Hide")), ("False", _l("Show"))]) + hidden = RadioField(_l("Hide category"), choices=[("True", _l("Hide")), ("False", _l("Show"))]) sms_sending_vehicle = RadioField( _l("Sending method for text messages"), choices=[("long_code", _l("Long code")), ("short_code", _l("Short code"))] ) diff --git a/app/main/validators.py b/app/main/validators.py index 5e9e5eef93..77ad8365b4 100644 --- a/app/main/validators.py +++ b/app/main/validators.py @@ -21,8 +21,7 @@ class Blocklist: def __init__(self, message=None): if not message: - message = _( - "This password is not allowed. Try a different password.") + message = _("This password is not allowed. Try a different password.") self.message = message def __call__(self, form, field): @@ -110,8 +109,7 @@ def __call__(self, form, field): class OnlySMSCharacters: def __call__(self, form, field): - non_sms_characters = sorted( - list(SanitiseSMS.get_non_compatible_characters(field.data))) + non_sms_characters = sorted(list(SanitiseSMS.get_non_compatible_characters(field.data))) if non_sms_characters: raise ValidationError( "You can’t use {} in text messages. {} won’t show up properly on everyone’s phones.".format( @@ -178,8 +176,7 @@ def validate_callback_url(service_callback_url, bearer_token): url=service_callback_url, allow_redirects=True, data={"health_check": "true"}, - headers={"Content-Type": "application/json", - "Authorization": f"Bearer {bearer_token}"}, + headers={"Content-Type": "application/json", "Authorization": f"Bearer {bearer_token}"}, timeout=2, ) @@ -189,15 +186,13 @@ def validate_callback_url(service_callback_url, bearer_token): current_app.logger.warning( f"Unable to create callback for service: {current_service.id} Error: Callback URL not reachable URL: {service_callback_url}" ) - raise ValidationError( - _l("Check your service is running and not using a proxy we cannot access")) + raise ValidationError(_l("Check your service is running and not using a proxy we cannot access")) except requests.RequestException as e: current_app.logger.warning( f"Unable to create callback for service: {current_service.id} Error: Callback URL not reachable URL: {service_callback_url} Exception: {e}" ) - raise ValidationError( - _l("Check your service is running and not using a proxy we cannot access")) + raise ValidationError(_l("Check your service is running and not using a proxy we cannot access")) def validate_email_from(form, field): @@ -207,23 +202,18 @@ def validate_email_from(form, field): if len(field.data) > 64: raise ValidationError(_l("This cannot exceed 64 characters in length")) # this filler is used because service id is not available when validating a new service to be created - service_id = getattr(form, "service_id", - current_app.config["NOTIFY_BAD_FILLER_UUID"]) - unique_name = service_api_client.is_service_email_from_unique( - service_id, field.data) + service_id = getattr(form, "service_id", current_app.config["NOTIFY_BAD_FILLER_UUID"]) + unique_name = service_api_client.is_service_email_from_unique(service_id, field.data) if not unique_name: raise ValidationError(_l("This email address is already in use")) def validate_service_name(form, field): if len(field.data) > 255: - raise ValidationError( - _l("This cannot exceed 255 characters in length")) + raise ValidationError(_l("This cannot exceed 255 characters in length")) if field.data != email_safe_name(field.data): - raise ValidationError( - _l("Make sure we formatted your email address correctly.")) - service_id = getattr(form, "service_id", - current_app.config["NOTIFY_BAD_FILLER_UUID"]) + raise ValidationError(_l("Make sure we formatted your email address correctly.")) + service_id = getattr(form, "service_id", current_app.config["NOTIFY_BAD_FILLER_UUID"]) unique_name = service_api_client.is_service_name_unique( service_id, field.data, From f3f3857f9b7676765c8bcce53ccb9de7e24de453 Mon Sep 17 00:00:00 2001 From: wbanks Date: Wed, 11 Sep 2024 16:11:53 -0400 Subject: [PATCH 11/22] Fix tests - Added placeholder FR translations - Removed validation that allowed the form to be submitted while empty, this was how callbacks were deleted previously - Added additional check to the make format task - Updated EN translations - Formatting --- Makefile | 1 + app/main/forms.py | 2 +- app/main/views/api_keys.py | 98 +- app/translations/csv/fr.csv | 7 +- application.py | 2 +- poetry.lock | 2 +- tests/app/main/views/test_api_integration.py | 1358 ++++++++--------- tests/app/main/views/test_service_settings.py | 2 - tests/conftest.py | 5 +- 9 files changed, 695 insertions(+), 782 deletions(-) diff --git a/Makefile b/Makefile index bebfe42280..8833fd26e4 100644 --- a/Makefile +++ b/Makefile @@ -58,6 +58,7 @@ run-dev: .PHONY: format format: ruff check --select I --fix . + ruff check ruff format . mypy ./ npx prettier --write app/assets/javascripts app/assets/stylesheets tests_cypress/cypress/e2e diff --git a/app/main/forms.py b/app/main/forms.py index 3786a77a17..809a1f3b67 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -1410,7 +1410,7 @@ def __init__(self, *args, **kwargs): class CallbackForm(StripWhitespaceForm): def validate(self, extra_validators=None): - return super().validate(extra_validators) or self.url.data == "" + return super().validate(extra_validators) class ServiceReceiveMessagesCallbackForm(CallbackForm): diff --git a/app/main/views/api_keys.py b/app/main/views/api_keys.py index ca020ecd79..cb66c165f5 100644 --- a/app/main/views/api_keys.py +++ b/app/main/views/api_keys.py @@ -28,13 +28,11 @@ @main.route("/services//api") @user_has_permissions("manage_api_keys") def api_integration(service_id): - callbacks_link = ".api_callbacks" if current_service.has_permission( - "inbound_sms") else ".delivery_status_callback" + callbacks_link = ".api_callbacks" if current_service.has_permission("inbound_sms") else ".delivery_status_callback" return render_template( "views/api/index.html", callbacks_link=callbacks_link, - api_notifications=notification_api_client.get_api_notifications_for_service( - service_id), + api_notifications=notification_api_client.get_api_notifications_for_service(service_id), ) @@ -89,8 +87,7 @@ def create_api_key(service_id): disabled_options, option_hints = [], {} if current_service.trial_mode: disabled_options = [KEY_TYPE_NORMAL] - option_hints[KEY_TYPE_NORMAL] = Markup( - _l("Not available because your service is in trial mode.")) + option_hints[KEY_TYPE_NORMAL] = Markup(_l("Not available because your service is in trial mode.")) if current_service.has_permission("letter"): option_hints[KEY_TYPE_TEAM] = "" if form.validate_on_submit(): @@ -122,8 +119,7 @@ def revoke_api_key(service_id, key_id): if request.method == "GET": flash( [ - "{} ‘{}’?".format( - _l("Are you sure you want to revoke"), key_name), + "{} ‘{}’?".format(_l("Are you sure you want to revoke"), key_name), _l("You will not be able to use this API key to connect to GC Notify"), ], "revoke this API key", @@ -141,11 +137,9 @@ def get_apis(): callback_api = None inbound_api = None if current_service.service_callback_api: - callback_api = service_api_client.get_service_callback_api( - current_service.id, current_service.service_callback_api[0]) + callback_api = service_api_client.get_service_callback_api(current_service.id, current_service.service_callback_api[0]) if current_service.inbound_api: - inbound_api = service_api_client.get_service_inbound_api( - current_service.id, current_service.inbound_api[0]) + inbound_api = service_api_client.get_service_inbound_api(current_service.id, current_service.inbound_api[0]) return (callback_api, inbound_api) @@ -167,8 +161,7 @@ def api_callbacks(service_id): return render_template( "views/api/callbacks.html", - received_text_messages_callback=received_text_messages_callback[ - "url"] if received_text_messages_callback else None, + received_text_messages_callback=received_text_messages_callback["url"] if received_text_messages_callback else None, delivery_status_callback=delivery_status_callback["url"] if delivery_status_callback else None, ) @@ -182,8 +175,7 @@ def get_delivery_status_callback_details(): @user_has_permissions("manage_api_keys") def delete_delivery_status_callback(service_id): delivery_status_callback = get_delivery_status_callback_details() - back_link = ".api_callbacks" if current_service.has_permission( - "inbound_sms") else ".api_integration" + back_link = ".api_callbacks" if current_service.has_permission("inbound_sms") else ".api_integration" url_hint_txt = "Must start with https://" if request.method == "POST": @@ -199,15 +191,13 @@ def delete_delivery_status_callback(service_id): flash(_l("Are you sure you want to delete this callback?"), "delete") form = ServiceDeliveryStatusCallbackForm( - url=delivery_status_callback.get( - "url") if delivery_status_callback else "", + url=delivery_status_callback.get("url") if delivery_status_callback else "", bearer_token=dummy_bearer_token if delivery_status_callback else "", ) return render_template( "views/api/callbacks/delivery-status-callback.html", - back_link=".api_callbacks" if current_service.has_permission( - "inbound_sms") else ".delivery_status_callback", + back_link=".api_callbacks" if current_service.has_permission("inbound_sms") else ".delivery_status_callback", hint_text=url_hint_txt, is_deleting=True, form=form, @@ -221,21 +211,18 @@ def delete_delivery_status_callback(service_id): @user_has_permissions("manage_api_keys") def delivery_status_callback(service_id): delivery_status_callback = get_delivery_status_callback_details() - back_link = ".api_callbacks" if current_service.has_permission( - "inbound_sms") else ".api_integration" - url_hint_txt = "Must start with https://" + back_link = ".api_callbacks" if current_service.has_permission("inbound_sms") else ".api_integration" + url_hint_txt = _l("Must start with https://") form = ServiceDeliveryStatusCallbackForm( - url=delivery_status_callback.get( - "url") if delivery_status_callback else "", + url=delivery_status_callback.get("url") if delivery_status_callback else "", bearer_token=dummy_bearer_token if delivery_status_callback else "", ) if form.validate_on_submit(): # As part of the ValidCallbackUrl validation, we ping their callback URL to check if it's up and set the response time in g - response_time = "{:.2f} {}".format( - g.callback_response_time, "milliseconds" if g.callback_response_time < 1 else "seconds" - ) + response_time = "{:.2f} {}".format(g.callback_response_time, _l("seconds")) + url_hostname = form.url.data.split("https://")[1] # Update existing callback if delivery_status_callback and form.url.data: @@ -243,8 +230,7 @@ def delivery_status_callback(service_id): service_api_client.update_service_callback_api( service_id, url=form.url.data, - bearer_token=check_token_against_dummy_bearer( - form.bearer_token.data), + bearer_token=check_token_against_dummy_bearer(form.bearer_token.data), user_id=current_user.id, callback_api_id=delivery_status_callback.get("id"), ) @@ -253,7 +239,7 @@ def delivery_status_callback(service_id): if request.form.get("button_pressed") == "test_response_time": flash( _l("Your service '{}' responded in {}").format( - form.url.data, + url_hostname, response_time, ), "default_with_tick", @@ -262,7 +248,7 @@ def delivery_status_callback(service_id): flash( _l("Callback to '{}' saved. Your service responded in {}").format( - form.url.data.split("https://")[1], + url_hostname, response_time, ), "default_with_tick", @@ -280,7 +266,7 @@ def delivery_status_callback(service_id): flash( _l("Callback to '{}' created. Your service responded in {}").format( - form.url.data.split("https://")[1], + url_hostname, response_time, ), "default_with_tick", @@ -296,7 +282,7 @@ def delivery_status_callback(service_id): if request.form.get("button_pressed") == "test_response_time": flash( _l("Your service '{}' responded in {}").format( - form.url.data, + url_hostname, response_time, ), "default_with_tick", @@ -305,15 +291,13 @@ def delivery_status_callback(service_id): flash( _l("Callback to '{}' saved. Your service responded in {}").format( - form.url.data.split("https://")[1], + url_hostname, response_time, ), "default_with_tick", ) return redirect(url_for(back_link, service_id=service_id)) - elif form.errors: - url_hint_txt = "Your service must be running and reachable from the internet." return render_template( "views/api/callbacks/delivery-status-callback.html", @@ -337,38 +321,35 @@ def get_received_text_messages_callback(): def received_text_messages_callback(service_id): if not current_service.has_permission("inbound_sms"): return redirect(url_for(".api_integration", service_id=service_id)) - back_link = ".api_callbacks" if current_service.has_permission( - "inbound_sms") else ".api_integration" + back_link = ".api_callbacks" if current_service.has_permission("inbound_sms") else ".api_integration" received_text_messages_callback = get_received_text_messages_callback() form = ServiceReceiveMessagesCallbackForm( - url=received_text_messages_callback.get( - "url") if received_text_messages_callback else "", + url=received_text_messages_callback.get("url") if received_text_messages_callback else "", bearer_token=dummy_bearer_token if received_text_messages_callback else "", ) - url_hint_txt = "Must start with https://" + url_hint_txt = _l("Must start with https://") if form.validate_on_submit(): # As part of the ValidCallbackUrl validation, we ping their callback URL to check if it's up and set the response time in g - response_time = "{:.2f} {}".format( - g.callback_response_time, "milliseconds" if g.callback_response_time < 1 else "seconds" - ) + response_time = "{:.2f} {}".format(g.callback_response_time, _l("seconds")) + url_hostname = form.url.data.split("https://")[1] + if received_text_messages_callback and form.url.data: if received_text_messages_callback.get("url") != form.url.data or form.bearer_token.data != dummy_bearer_token: service_api_client.update_service_inbound_api( service_id, url=form.url.data, - bearer_token=check_token_against_dummy_bearer( - form.bearer_token.data), + bearer_token=check_token_against_dummy_bearer(form.bearer_token.data), user_id=current_user.id, inbound_api_id=received_text_messages_callback.get("id"), ) - # If the user is just testing their URL, don't send them back to the API Integration page + # If the user is just testing their URL, don't send them back to the API Integration page if request.form.get("button_pressed") == "test_response_time": flash( _l("Your service '{}' responded in {}").format( - form.url.data, + url_hostname, response_time, ), "default_with_tick", @@ -377,7 +358,7 @@ def received_text_messages_callback(service_id): flash( _l("Callback to '{}' saved. Your service responded in {}").format( - form.url.data.split("https://")[1], + url_hostname, response_time, ), "default_with_tick", @@ -397,20 +378,18 @@ def received_text_messages_callback(service_id): ) flash( _l("Callback to '{}' created. Your service responded in {}").format( - form.url.data.split("https://")[1], + url_hostname, response_time, ), "default_with_tick", ) return redirect(url_for(back_link, service_id=service_id)) - elif form.errors: - url_hint_txt = "Your service must be running and reachable from the internet." if request.form.get("button_pressed") == "test_response_time": flash( _l("Your service '{}' responded in {}").format( - form.url.data, + url_hostname, response_time, ), "default_with_tick", @@ -419,7 +398,7 @@ def received_text_messages_callback(service_id): flash( _l("Callback to '{}' saved. Your service responded in {}").format( - form.url.data.split("https://")[1], + url_hostname, response_time, ), "default_with_tick", @@ -438,8 +417,7 @@ def received_text_messages_callback(service_id): @user_has_permissions("manage_api_keys") def delete_received_text_messages_callback(service_id): received_text_messages_callback = get_received_text_messages_callback() - back_link = ".api_callbacks" if current_service.has_permission( - "inbound_sms") else ".api_integration" + back_link = ".api_callbacks" if current_service.has_permission("inbound_sms") else ".api_integration" url_hint_txt = "Must start with https://" if request.method == "POST": @@ -455,15 +433,13 @@ def delete_received_text_messages_callback(service_id): flash(_l("Are you sure you want to delete this callback?"), "delete") form = ServiceReceiveMessagesCallbackForm( - url=received_text_messages_callback.get( - "url") if delivery_status_callback else "", + url=received_text_messages_callback.get("url") if delivery_status_callback else "", bearer_token=dummy_bearer_token if received_text_messages_callback else "", ) return render_template( "views/api/callbacks/delivery-status-callback.html", - back_link=".api_callbacks" if current_service.has_permission( - "inbound_sms") else ".delivery_status_callback", + back_link=".api_callbacks" if current_service.has_permission("inbound_sms") else ".delivery_status_callback", hint_text=url_hint_txt, is_deleting=True, form=form, diff --git a/app/translations/csv/fr.csv b/app/translations/csv/fr.csv index 20b646950e..e853f53414 100644 --- a/app/translations/csv/fr.csv +++ b/app/translations/csv/fr.csv @@ -1981,7 +1981,6 @@ "Review your activity","Examinez votre activité" "Set sensitive service","Définir un service sensible" "Suspend Callback","Suspension du rappel" -"Callback to '{}' created. Your service responded in {}","(FR) Callback to '{}' created. Your service responded in '{}'" -"Callback to '{}' saved. Your service responded in {}","(FR) Callback to '{}' saved. Your service responded in '{}'" -"Your service must be running and reachable from the internet","(FR) Your service must be running and reachable from the internet" -"We were unable to contact your callback service, please try again.","(FR)We were unable to contact your callback service, please try again." \ No newline at end of file +"We’ve set up your callback configuration. {} responded in {} seconds.","Nous avons configuré votre rappel. {} a répondu en {} secondes." +"We’ve saved your callback configuration. {} responded in {} seconds.","Nous avons enregistré votre configuration de rappel. {} a répondu en {} secondes." +"The service {} responded in {} seconds.","Le service {} a répondu en {} secondes." \ No newline at end of file diff --git a/application.py b/application.py index 8f82113d4f..39512e18b4 100644 --- a/application.py +++ b/application.py @@ -16,7 +16,7 @@ application = Flask("app") application.wsgi_app = ProxyFix(application.wsgi_app) # type: ignore -xray_recorder.configure(service='Notify-Admin') +xray_recorder.configure(service="Notify-Admin") XRayMiddleware(application, xray_recorder) create_app(application) diff --git a/poetry.lock b/poetry.lock index 6b1d0b11cb..693bc76827 100644 --- a/poetry.lock +++ b/poetry.lock @@ -2587,4 +2587,4 @@ testing = ["coverage (>=5.0.3)", "zope.event", "zope.testing"] [metadata] lock-version = "2.0" python-versions = "~3.10.9" -content-hash = "1757fcf62e61abf167c976adabb0dc20a5230cb7bd90cf1d6a8b33b4870bd429" +content-hash = "8b8a8f4feffd9dde38913c875e286034ff5dc1ff3f61e99eaa8ecb5552340a74" diff --git a/tests/app/main/views/test_api_integration.py b/tests/app/main/views/test_api_integration.py index 79101a1266..7d6c718727 100644 --- a/tests/app/main/views/test_api_integration.py +++ b/tests/app/main/views/test_api_integration.py @@ -1,6 +1,5 @@ import uuid from collections import OrderedDict -from datetime import timedelta from unittest.mock import Mock, call import pytest @@ -62,8 +61,7 @@ def test_should_show_api_page_with_no_notifications( service_id=SERVICE_ONE_ID, ) rows = page.find_all("div", {"class": "api-notifications-item"}) - assert "When you send messages via the API they’ll appear here." in rows[len( - rows) - 1].text.strip() + assert "When you send messages via the API they’ll appear here." in rows[len(rows) - 1].text.strip() @pytest.mark.parametrize( @@ -82,8 +80,7 @@ def test_letter_notifications_should_have_link_to_view_letter( link_text, ): notifications = create_notifications(template_type=template_type) - mocker.patch("app.notification_api_client.get_api_notifications_for_service", - return_value=notifications) + mocker.patch("app.notification_api_client.get_api_notifications_for_service", return_value=notifications) page = client_request.get( "main.api_integration", service_id=SERVICE_ONE_ID, @@ -97,8 +94,7 @@ def test_should_not_have_link_to_view_letter_for_precompiled_letters_in_virus_st client_request, fake_uuid, mock_has_permissions, mocker, status ): notifications = create_notifications(status=status) - mocker.patch("app.notification_api_client.get_api_notifications_for_service", - return_value=notifications) + mocker.patch("app.notification_api_client.get_api_notifications_for_service", return_value=notifications) page = client_request.get( "main.api_integration", @@ -124,8 +120,7 @@ def test_letter_notifications_should_show_client_reference( shows_ref, ): notifications = create_notifications(client_reference=client_reference) - mocker.patch("app.notification_api_client.get_api_notifications_for_service", - return_value=notifications) + mocker.patch("app.notification_api_client.get_api_notifications_for_service", return_value=notifications) page = client_request.get( "main.api_integration", @@ -163,763 +158,704 @@ def test_api_documentation_page_should_redirect( ) -def test_should_show_empty_api_keys_page( - client, - api_user_active, - mock_login, - mock_get_no_api_keys, - mock_get_service, - mock_has_permissions, -): - client.login(api_user_active) - service_id = str(uuid.uuid4()) - response = client.get(url_for("main.api_keys", service_id=service_id)) - - assert response.status_code == 200 - assert "You have not created any API keys yet" in response.get_data( - as_text=True) - assert "Create API key" in response.get_data(as_text=True) - mock_get_no_api_keys.assert_called_once_with(service_id) - - -def test_should_show_api_keys_page( - client_request, - mock_get_api_keys, - mock_get_api_key_statistics, -): - page = client_request.get("main.api_keys", service_id=SERVICE_ONE_ID) - rows = [normalize_spaces(row.text) for row in page.select("main tr")] - - assert rows[0] == "API keys Action" - assert "another key name 20 total sends in the last 7 days (20 email, 0 sms)" in rows[ - 1] - assert "Revoke API key some key name" in rows[2] - - mock_get_api_keys.assert_called_once_with(SERVICE_ONE_ID) - - -@pytest.mark.parametrize( - "restricted, can_send_letters, expected_options", - [ - ( - True, - False, - [ - ("Live – sends to anyone " "Not available because your service is in trial mode."), - "Team and safelist – limits who you can send to", - "Test – pretends to send messages", - ], - ), - ( - False, - False, - [ - "Live – sends to anyone", - "Team and safelist – limits who you can send to", - "Test – pretends to send messages", - ], - ), - ( - False, - True, - [ - "Live – sends to anyone", - ("Team and safelist – limits who you can send to" ""), - "Test – pretends to send messages", - ], - ), - ], -) -def test_should_show_create_api_key_page( - client_request, - mocker, - api_user_active, - mock_get_api_keys, - restricted, - can_send_letters, - expected_options, - service_one, -): - service_one["restricted"] = restricted - if can_send_letters: - service_one["permissions"].append("letter") - - mocker.patch("app.service_api_client.get_service", - return_value={"data": service_one}) - - page = client_request.get("main.create_api_key", service_id=SERVICE_ONE_ID) - - for index, option in enumerate(expected_options): - assert normalize_spaces(page.select( - ".block-label")[index].text) == option - - -def test_should_create_api_key_with_type_normal( - client_request, - api_user_active, - mock_login, - mock_get_api_keys, - mock_get_live_service, - mock_has_permissions, - fake_uuid, - mocker, -): - key_name_from_user = "Some default key name 1/2" - key_name_fixed = "some_default_key_name_12" - post = mocker.patch( - "app.notify_client.api_key_api_client.ApiKeyApiClient.post", - return_value={"data": {"key": fake_uuid, "key_name": key_name_fixed}}, - ) - - page = client_request.post( - "main.create_api_key", - service_id=SERVICE_ONE_ID, - _data={"key_name": key_name_from_user, "key_type": "normal"}, - _expected_status=200, - ) - - assert page.select_one("span.api-key-key").text.strip() == ( - "{}-{}-{}".format(key_name_fixed, SERVICE_ONE_ID, fake_uuid)) - - post.assert_called_once_with( - url="/service/{}/api-key".format(SERVICE_ONE_ID), - data={ - "name": key_name_from_user, - "key_type": "normal", - "created_by": api_user_active["id"], - }, - ) - - -def test_cant_create_normal_api_key_in_trial_mode( - client_request, - api_user_active, - mock_login, - mock_get_api_keys, - mock_get_service, - mock_has_permissions, - fake_uuid, - mocker, -): - mock_post = mocker.patch( - "app.notify_client.api_key_api_client.ApiKeyApiClient.post") - - client_request.post( - "main.create_api_key", - service_id=SERVICE_ONE_ID, - _data={"key_name": "some default key name", "key_type": "normal"}, - _expected_status=400, - ) - mock_post.assert_not_called() - - -def test_should_show_confirm_revoke_api_key( - client_request, - mock_get_api_keys, - fake_uuid, -): - page = client_request.get( - "main.revoke_api_key", - service_id=SERVICE_ONE_ID, - key_id=fake_uuid, - _test_page_title=False, - ) - assert normalize_spaces(page.select(".banner-dangerous")[0].text) == ( - "Are you sure you want to revoke ‘some key name’? " - "You will not be able to use this API key to connect to GC Notify " - "Yes, revoke this API key" - ) - assert mock_get_api_keys.call_args_list == [ - call("596364a0-858e-42c8-9062-a8fe822260eb"), - ] - - -def test_should_show_confirm_revoke_api_key_for_platform_admin( - platform_admin_client, - mock_get_api_keys, - fake_uuid, -): - url = url_for( - "main.revoke_api_key", - service_id=SERVICE_ONE_ID, - key_id=fake_uuid, - _test_page_title=False, - ) - response = platform_admin_client.get(url) - page = BeautifulSoup(response.data.decode("utf-8"), "html.parser") - assert normalize_spaces(page.select(".banner-dangerous")[0].text) == ( - "Are you sure you want to revoke ‘some key name’? " - "You will not be able to use this API key to connect to GC Notify " - "Yes, revoke this API key" - ) - assert mock_get_api_keys.call_args_list == [ - call("596364a0-858e-42c8-9062-a8fe822260eb"), - ] - - -def test_should_404_for_api_key_that_doesnt_exist( - client_request, - mock_get_api_keys, -): - client_request.get( - "main.revoke_api_key", - service_id=SERVICE_ONE_ID, - key_id="key-doesn’t-exist", - _expected_status=404, - ) - +class TestApiKeys: + def test_should_show_api_keys_page( + self, + client_request, + mock_get_api_keys, + mock_get_api_key_statistics, + ): + page = client_request.get("main.api_keys", service_id=SERVICE_ONE_ID) + rows = [normalize_spaces(row.text) for row in page.select("main tr")] + + assert rows[0] == "API keys Action" + assert "another key name 20 total sends in the last 7 days (20 email, 0 sms)" in rows[1] + assert "Revoke API key some key name" in rows[2] + + mock_get_api_keys.assert_called_once_with(SERVICE_ONE_ID) + + def test_should_show_empty_api_keys_page( + self, + client, + api_user_active, + mock_login, + mock_get_no_api_keys, + mock_get_service, + mock_has_permissions, + ): + client.login(api_user_active) + service_id = str(uuid.uuid4()) + response = client.get(url_for("main.api_keys", service_id=service_id)) + + assert response.status_code == 200 + assert "You have not created any API keys yet" in response.get_data(as_text=True) + assert "Create API key" in response.get_data(as_text=True) + mock_get_no_api_keys.assert_called_once_with(service_id) + + @pytest.mark.parametrize( + "restricted, can_send_letters, expected_options", + [ + ( + True, + False, + [ + ("Live – sends to anyone " "Not available because your service is in trial mode."), + "Team and safelist – limits who you can send to", + "Test – pretends to send messages", + ], + ), + ( + False, + False, + [ + "Live – sends to anyone", + "Team and safelist – limits who you can send to", + "Test – pretends to send messages", + ], + ), + ( + False, + True, + [ + "Live – sends to anyone", + ("Team and safelist – limits who you can send to" ""), + "Test – pretends to send messages", + ], + ), + ], + ) + def test_should_show_create_api_key_page( + self, + client_request, + mocker, + api_user_active, + mock_get_api_keys, + restricted, + can_send_letters, + expected_options, + service_one, + ): + service_one["restricted"] = restricted + if can_send_letters: + service_one["permissions"].append("letter") + + mocker.patch("app.service_api_client.get_service", return_value={"data": service_one}) + + page = client_request.get("main.create_api_key", service_id=SERVICE_ONE_ID) + + for index, option in enumerate(expected_options): + assert normalize_spaces(page.select(".block-label")[index].text) == option + + def test_should_create_api_key_with_type_normal( + self, + client_request, + api_user_active, + mock_login, + mock_get_api_keys, + mock_get_live_service, + mock_has_permissions, + fake_uuid, + mocker, + ): + key_name_from_user = "Some default key name 1/2" + key_name_fixed = "some_default_key_name_12" + post = mocker.patch( + "app.notify_client.api_key_api_client.ApiKeyApiClient.post", + return_value={"data": {"key": fake_uuid, "key_name": key_name_fixed}}, + ) -def test_should_redirect_after_revoking_api_key( - client_request, - api_user_active, - mock_login, - mock_revoke_api_key, - mock_get_api_keys, - mock_get_service, - mock_has_permissions, - fake_uuid, -): - client_request.post( - "main.revoke_api_key", - service_id=SERVICE_ONE_ID, - key_id=fake_uuid, - _expected_status=302, - _expected_redirect=url_for( - ".api_keys", + page = client_request.post( + "main.create_api_key", service_id=SERVICE_ONE_ID, - ), - ) - mock_revoke_api_key.assert_called_once_with( - service_id=SERVICE_ONE_ID, key_id=fake_uuid) - mock_get_api_keys.assert_called_once_with( - SERVICE_ONE_ID, - ) - - -@pytest.mark.parametrize("route", ["main.api_keys", "main.create_api_key", "main.revoke_api_key"]) -def test_route_permissions( - mocker, - app_, - fake_uuid, - api_user_active, - service_one, - mock_get_api_keys, - route, - mock_get_api_key_statistics, -): - with app_.test_request_context(): - validate_route_permission( - mocker, - app_, - "GET", - 200, - url_for(route, service_id=service_one["id"], key_id=fake_uuid), - ["manage_api_keys"], - api_user_active, - service_one, + _data={"key_name": key_name_from_user, "key_type": "normal"}, + _expected_status=200, ) + assert page.select_one("span.api-key-key").text.strip() == ("{}-{}-{}".format(key_name_fixed, SERVICE_ONE_ID, fake_uuid)) -@pytest.mark.parametrize("route", ["main.api_keys", "main.create_api_key", "main.revoke_api_key"]) -def test_route_invalid_permissions( - mocker, - app_, - fake_uuid, - api_user_active, - service_one, - mock_get_api_keys, - route, -): - with app_.test_request_context(): - validate_route_permission( - mocker, - app_, - "GET", - 403, - url_for(route, service_id=service_one["id"], key_id=fake_uuid), - ["view_activity"], - api_user_active, - service_one, + post.assert_called_once_with( + url="/service/{}/api-key".format(SERVICE_ONE_ID), + data={ + "name": key_name_from_user, + "key_type": "normal", + "created_by": api_user_active["id"], + }, ) - -def test_should_show_safelist_page( - client_request, - mock_login, - api_user_active, - mock_get_service, - mock_has_permissions, - mock_get_safelist, -): - page = client_request.get( - "main.safelist", - service_id=SERVICE_ONE_ID, - ) - textboxes = page.find_all( - "input", {"type": "email"}) + page.find_all("input", {"type": "tel"}) - for index, value in enumerate(["test@example.com"] + [""] * 4 + ["6502532222"] + [""] * 4): - assert textboxes[index]["value"] == value - - -def test_should_update_safelist( - client_request, - mock_update_safelist, -): - data = OrderedDict( - [ - ("email_addresses-1", "test@example.com"), - ("email_addresses-3", "test@example.com"), - ("phone_numbers-0", "6502532222"), - ("phone_numbers-2", "+4966921809"), + def test_cant_create_normal_api_key_in_trial_mode( + self, + client_request, + api_user_active, + mock_login, + mock_get_api_keys, + mock_get_service, + mock_has_permissions, + fake_uuid, + mocker, + ): + mock_post = mocker.patch("app.notify_client.api_key_api_client.ApiKeyApiClient.post") + + client_request.post( + "main.create_api_key", + service_id=SERVICE_ONE_ID, + _data={"key_name": "some default key name", "key_type": "normal"}, + _expected_status=400, + ) + mock_post.assert_not_called() + + def test_should_show_confirm_revoke_api_key( + self, + client_request, + mock_get_api_keys, + fake_uuid, + ): + page = client_request.get( + "main.revoke_api_key", + service_id=SERVICE_ONE_ID, + key_id=fake_uuid, + _test_page_title=False, + ) + assert normalize_spaces(page.select(".banner-dangerous")[0].text) == ( + "Are you sure you want to revoke ‘some key name’? " + "You will not be able to use this API key to connect to GC Notify " + "Yes, revoke this API key" + ) + assert mock_get_api_keys.call_args_list == [ + call("596364a0-858e-42c8-9062-a8fe822260eb"), ] - ) - - client_request.post( - "main.safelist", - service_id=SERVICE_ONE_ID, - _data=data, - ) - - mock_update_safelist.assert_called_once_with( - SERVICE_ONE_ID, - { - "email_addresses": ["test@example.com", "test@example.com"], - "phone_numbers": ["6502532222", "+4966921809"], - }, - ) + def test_should_show_confirm_revoke_api_key_for_platform_admin( + self, + platform_admin_client, + mock_get_api_keys, + fake_uuid, + ): + url = url_for( + "main.revoke_api_key", + service_id=SERVICE_ONE_ID, + key_id=fake_uuid, + _test_page_title=False, + ) + response = platform_admin_client.get(url) + page = BeautifulSoup(response.data.decode("utf-8"), "html.parser") + assert normalize_spaces(page.select(".banner-dangerous")[0].text) == ( + "Are you sure you want to revoke ‘some key name’? " + "You will not be able to use this API key to connect to GC Notify " + "Yes, revoke this API key" + ) + assert mock_get_api_keys.call_args_list == [ + call("596364a0-858e-42c8-9062-a8fe822260eb"), + ] -def test_should_validate_safelist_items( - client_request, - mock_update_safelist, -): - page = client_request.post( - "main.safelist", - service_id=SERVICE_ONE_ID, - _data=OrderedDict([("email_addresses-1", "abc"), - ("phone_numbers-0", "123")]), - _expected_status=200, - ) - - assert page.select_one( - ".banner-title").string.strip() == "There was a problem with your safelist" - jump_links = page.select(".banner-dangerous a") + def test_should_404_for_api_key_that_doesnt_exist( + self, + client_request, + mock_get_api_keys, + ): + client_request.get( + "main.revoke_api_key", + service_id=SERVICE_ONE_ID, + key_id="key-doesn’t-exist", + _expected_status=404, + ) - assert jump_links[0].string.strip() == "Enter valid email addresses" - assert jump_links[0]["href"] == "#email_addresses" + def test_should_redirect_after_revoking_api_key( + self, + client_request, + api_user_active, + mock_login, + mock_revoke_api_key, + mock_get_api_keys, + mock_get_service, + mock_has_permissions, + fake_uuid, + ): + client_request.post( + "main.revoke_api_key", + service_id=SERVICE_ONE_ID, + key_id=fake_uuid, + _expected_status=302, + _expected_redirect=url_for( + ".api_keys", + service_id=SERVICE_ONE_ID, + ), + ) + mock_revoke_api_key.assert_called_once_with(service_id=SERVICE_ONE_ID, key_id=fake_uuid) + mock_get_api_keys.assert_called_once_with( + SERVICE_ONE_ID, + ) - assert jump_links[1].string.strip() == "Enter valid phone numbers" - assert jump_links[1]["href"] == "#phone_numbers" + @pytest.mark.parametrize("route", ["main.api_keys", "main.create_api_key", "main.revoke_api_key"]) + def test_route_permissions( + self, + mocker, + app_, + fake_uuid, + api_user_active, + service_one, + mock_get_api_keys, + route, + mock_get_api_key_statistics, + ): + with app_.test_request_context(): + validate_route_permission( + mocker, + app_, + "GET", + 200, + url_for(route, service_id=service_one["id"], key_id=fake_uuid), + ["manage_api_keys"], + api_user_active, + service_one, + ) + + @pytest.mark.parametrize("route", ["main.api_keys", "main.create_api_key", "main.revoke_api_key"]) + def test_route_invalid_permissions( + self, + mocker, + app_, + fake_uuid, + api_user_active, + service_one, + mock_get_api_keys, + route, + ): + with app_.test_request_context(): + validate_route_permission( + mocker, + app_, + "GET", + 403, + url_for(route, service_id=service_one["id"], key_id=fake_uuid), + ["view_activity"], + api_user_active, + service_one, + ) + + +class TestSafelist: + def test_should_update_safelist( + self, + client_request, + mock_update_safelist, + ): + data = OrderedDict( + [ + ("email_addresses-1", "test@example.com"), + ("email_addresses-3", "test@example.com"), + ("phone_numbers-0", "6502532222"), + ("phone_numbers-2", "+4966921809"), + ] + ) - assert mock_update_safelist.called is False + client_request.post( + "main.safelist", + service_id=SERVICE_ONE_ID, + _data=data, + ) + mock_update_safelist.assert_called_once_with( + SERVICE_ONE_ID, + { + "email_addresses": ["test@example.com", "test@example.com"], + "phone_numbers": ["6502532222", "+4966921809"], + }, + ) -@pytest.mark.parametrize( - "endpoint", - [ - ("main.delivery_status_callback"), - ("main.received_text_messages_callback"), - ], -) -@pytest.mark.parametrize( - "url, bearer_token, response, expected_errors", - [ - ("https://example.com", "", None, "This cannot be empty"), - ("http://not_https.com", "1234567890", None, - "Enter a URL that starts with https://"), - ( - "https://test.com", - "123456789", - {"content": "a", "status_code": 500, "headers": {"a": "a"}}, - "Must be at least 10 characters", - ), - ( - "https://test.ee", - "1234567890", - {"content": "a", "status_code": 404, "headers": {"a": "a"}}, - "Check your service is running and not using a proxy we cannot access", - ), - ], -) -def test_callback_forms_validation( - client_request, - service_one, - mock_get_valid_service_callback_api, - endpoint, - url, - bearer_token, - response, - expected_errors, - mocker, -): - if endpoint == "main.received_text_messages_callback": - service_one["permissions"] = ["inbound_sms"] + def test_should_show_safelist_page( + self, + client_request, + mock_login, + api_user_active, + mock_get_service, + mock_has_permissions, + mock_get_safelist, + ): + page = client_request.get( + "main.safelist", + service_id=SERVICE_ONE_ID, + ) + textboxes = page.find_all("input", {"type": "email"}) + page.find_all("input", {"type": "tel"}) + for index, value in enumerate(["test@example.com"] + [""] * 4 + ["6502532222"] + [""] * 4): + assert textboxes[index]["value"] == value + + def test_should_validate_safelist_items( + self, + client_request, + mock_update_safelist, + ): + page = client_request.post( + "main.safelist", + service_id=SERVICE_ONE_ID, + _data=OrderedDict([("email_addresses-1", "abc"), ("phone_numbers-0", "123")]), + _expected_status=200, + ) - data = { - "url": url, - "bearer_token": bearer_token, - } - if response: - resp = Mock( - content=response["content"], status_code=response["status_code"], headers=response["headers"]) - mocker.patch("app.main.validators.requests.post", return_value=resp) + assert page.select_one(".banner-title").string.strip() == "There was a problem with your safelist" + jump_links = page.select(".banner-dangerous a") - response = client_request.post( - endpoint, service_id=service_one["id"], _data=data, _expected_status=200) - error_msgs = " ".join(msg.text.strip() - for msg in response.select(".error-message")) + assert jump_links[0].string.strip() == "Enter valid email addresses" + assert jump_links[0]["href"] == "#email_addresses" - assert error_msgs == expected_errors + assert jump_links[1].string.strip() == "Enter valid phone numbers" + assert jump_links[1]["href"] == "#phone_numbers" + assert mock_update_safelist.called is False -@pytest.mark.parametrize( - "endpoint, expected_delete_url", - [ - ( - "main.delete_delivery_status_callback", - "/service/{}/delivery-receipt-api/{}", - ), - ( - "main.received_text_messages_callback", - "/service/{}/inbound-api/{}", - ), - ], -) -def test_delete_callback( - client_request, - service_one, - endpoint, - expected_delete_url, - mocker, - fake_uuid, - mock_get_valid_service_callback_api, - mock_get_valid_service_inbound_api, -): - service_one["service_callback_api"] = [fake_uuid] - service_one["inbound_api"] = [fake_uuid] - service_one["permissions"] = ["inbound_sms"] - mocked_delete = mocker.patch("app.service_api_client.delete") - page = client_request.post( - endpoint, - service_id=service_one["id"], - _follow_redirects=True, +class TestApiCallbacks: + @pytest.mark.parametrize( + "endpoint", + [ + ("main.delivery_status_callback"), + ("main.received_text_messages_callback"), + ], ) - - assert not page.select(".error-message") - mocked_delete.assert_called_once_with( - expected_delete_url.format(service_one["id"], fake_uuid)) - - -@pytest.mark.parametrize("bearer_token", ["", "some-bearer-token"]) -@pytest.mark.parametrize( - "endpoint, expected_delete_url", - [ - ( - "main.delivery_status_callback", - "/service/{}/delivery-receipt-api/{}", - ), - ( - "main.received_text_messages_callback", - "/service/{}/inbound-api/{}", - ), - ], -) -def test_callback_forms_can_be_cleared_when_callback_and_inbound_apis_are_empty( - client_request, - service_one, - endpoint, - expected_delete_url, - bearer_token, - mocker, - mock_get_empty_service_callback_api, - mock_get_empty_service_inbound_api, -): - service_one["permissions"] = ["inbound_sms"] - mocked_delete = mocker.patch("app.service_api_client.delete") - - page = client_request.post( + @pytest.mark.parametrize( + "url, bearer_token, response, expected_errors", + [ + ("https://example.com", "", None, "This cannot be empty"), + ("http://not_https.com", "1234567890", None, "Enter a URL that starts with https://"), + ( + "https://test.com", + "123456789", + {"content": "a", "status_code": 500, "headers": {"a": "a"}}, + "Must be at least 10 characters", + ), + ( + "https://test.ee", + "1234567890", + {"content": "a", "status_code": 404, "headers": {"a": "a"}}, + "Check your service is running and not using a proxy we cannot access", + ), + ], + ) + def test_callback_forms_validation( + self, + client_request, + service_one, + mock_get_valid_service_callback_api, endpoint, - service_id=service_one["id"], - _data={ - "url": "", + url, + bearer_token, + response, + expected_errors, + mocker, + ): + if endpoint == "main.received_text_messages_callback": + service_one["permissions"] = ["inbound_sms"] + + data = { + "url": url, "bearer_token": bearer_token, - }, - _expected_redirect=url_for( - "main.api_callbacks", - service_id=service_one["id"], - ), - ) + } + if response: + resp = Mock(content=response["content"], status_code=response["status_code"], headers=response["headers"]) + mocker.patch("app.main.validators.requests.post", return_value=resp) - assert not page.select(".error-message") - assert mocked_delete.call_args_list == [] + response = client_request.post(endpoint, service_id=service_one["id"], _data=data, _expected_status=200) + error_msgs = " ".join(msg.text.strip() for msg in response.select(".error-message")) + assert error_msgs == expected_errors -@pytest.mark.parametrize( - "has_inbound_sms, expected_link", - [ - (True, "main.api_callbacks"), - (False, "main.delivery_status_callback"), - ], -) -def test_callbacks_button_links_straight_to_delivery_status_if_service_has_no_inbound_sms( - client_request, - service_one, - mocker, - mock_get_notifications, - has_inbound_sms, - expected_link, -): - if has_inbound_sms: + @pytest.mark.parametrize( + "endpoint, expected_delete_url", + [ + ( + "main.delete_delivery_status_callback", + "/service/{}/delivery-receipt-api/{}", + ), + ( + "main.delete_received_text_messages_callback", + "/service/{}/inbound-api/{}", + ), + ], + ) + def test_delete_delivery_status_and_receive_text_message_callbacks( + self, + client_request, + service_one, + endpoint, + expected_delete_url, + mocker, + fake_uuid, + mock_get_valid_service_callback_api, + mock_get_valid_service_inbound_api, + ): + service_one["service_callback_api"] = [fake_uuid] + service_one["inbound_api"] = [fake_uuid] service_one["permissions"] = ["inbound_sms"] + mocked_delete = mocker.patch("app.service_api_client.delete") - page = client_request.get( - "main.api_integration", - service_id=service_one["id"], - ) - - assert page.select( - ".api-header-links")[2]["href"] == url_for(expected_link, service_id=service_one["id"]) + page = client_request.post( + endpoint, + service_id=service_one["id"], + _follow_redirects=True, + ) + assert not page.select(".error-message") + mocked_delete.assert_called_once_with(expected_delete_url.format(service_one["id"], fake_uuid)) -def test_callbacks_page_redirects_to_delivery_status_if_service_has_no_inbound_sms( - client_request, - service_one, - mocker, - mock_get_valid_service_callback_api, -): - page = client_request.get( - "main.api_callbacks", - service_id=service_one["id"], - _follow_redirects=True, - ) + @pytest.mark.parametrize( + "has_inbound_sms, expected_link", + [ + (True, "main.api_callbacks"), + (False, "main.delivery_status_callback"), + ], + ) + def test_callbacks_button_links_straight_to_delivery_status_if_service_has_no_inbound_sms( + self, + client_request, + service_one, + mocker, + mock_get_notifications, + has_inbound_sms, + expected_link, + ): + if has_inbound_sms: + service_one["permissions"] = ["inbound_sms"] + + page = client_request.get( + "main.api_integration", + service_id=service_one["id"], + ) - assert normalize_spaces(page.select_one( - "h1").text) == "Callbacks for delivery receipts" + assert page.select(".api-header-links")[2]["href"] == url_for(expected_link, service_id=service_one["id"]) + def test_callbacks_page_redirects_to_delivery_status_if_service_has_no_inbound_sms( + self, + client_request, + service_one, + mocker, + mock_get_valid_service_callback_api, + ): + page = client_request.get( + "main.api_callbacks", + service_id=service_one["id"], + _follow_redirects=True, + ) -@pytest.mark.parametrize( - "has_inbound_sms, expected_link", - [ - (True, "main.api_callbacks"), - (False, "main.api_integration"), - ], -) -def test_back_link_directs_to_api_integration_from_delivery_callback_if_no_inbound_sms( - client_request, service_one, mocker, has_inbound_sms, expected_link -): - if has_inbound_sms: - service_one["permissions"] = ["inbound_sms"] + assert normalize_spaces(page.select_one("h1").text) == "Callbacks for delivery receipts" - page = client_request.get( - "main.delivery_status_callback", - service_id=service_one["id"], - _follow_redirects=True, + @pytest.mark.parametrize( + "has_inbound_sms, expected_link", + [ + (True, "main.api_callbacks"), + (False, "main.api_integration"), + ], ) + def test_back_link_directs_to_api_integration_from_delivery_callback_if_no_inbound_sms( + self, client_request, service_one, mocker, has_inbound_sms, expected_link + ): + if has_inbound_sms: + service_one["permissions"] = ["inbound_sms"] - assert page.select_one( - ".back-link")["href"] == url_for(expected_link, service_id=service_one["id"]) - - -@pytest.mark.parametrize( - "endpoint", - [ - ("main.delivery_status_callback"), - ("main.received_text_messages_callback"), - ], -) -def test_create_delivery_status_and_receive_text_message_callbacks( - client_request, - service_one, - mocker, - mock_get_notifications, - mock_create_service_inbound_api, - mock_create_service_callback_api, - endpoint, - fake_uuid, - mock_validate_callback_url, -): - if endpoint == "main.received_text_messages_callback": - service_one["permissions"] = ["inbound_sms"] + page = client_request.get( + "main.delivery_status_callback", + service_id=service_one["id"], + _follow_redirects=True, + ) - data = { - "url": "https://test.url.com/", - "bearer_token": "1234567890", - "user_id": fake_uuid, - } + assert page.select_one(".back-link")["href"] == url_for(expected_link, service_id=service_one["id"]) - client_request.post( + @pytest.mark.parametrize( + "endpoint", + [ + ("main.delivery_status_callback"), + ("main.received_text_messages_callback"), + ], + ) + def test_create_delivery_status_and_receive_text_message_callbacks( + self, + client_request, + service_one, + mocker, + mock_get_notifications, + mock_create_service_inbound_api, + mock_create_service_callback_api, endpoint, - service_id=service_one["id"], - _data=data, - ) + fake_uuid, + mock_validate_callback_url, + ): + if endpoint == "main.received_text_messages_callback": + service_one["permissions"] = ["inbound_sms"] + + data = { + "url": "https://test.url.com/", + "bearer_token": "1234567890", + "user_id": fake_uuid, + } + + client_request.post( + endpoint, + service_id=service_one["id"], + _data=data, + ) - if endpoint == "main.received_text_messages_callback": - mock_create_service_inbound_api.assert_called_once_with( - service_one["id"], - url="https://test.url.com/", - bearer_token="1234567890", - user_id=fake_uuid, + if endpoint == "main.received_text_messages_callback": + mock_create_service_inbound_api.assert_called_once_with( + service_one["id"], + url="https://test.url.com/", + bearer_token="1234567890", + user_id=fake_uuid, + ) + else: + mock_create_service_callback_api.assert_called_once_with( + service_one["id"], + url="https://test.url.com/", + bearer_token="1234567890", + user_id=fake_uuid, + ) + + def test_update_delivery_status_callback_details( + self, + client_request, + service_one, + mock_update_service_callback_api, + mock_get_valid_service_callback_api, + fake_uuid, + mock_validate_callback_url, + mocker, + ): + service_one["service_callback_api"] = [fake_uuid] + + data = { + "url": "https://test.url.com/", + "bearer_token": "1234567890", + "user_id": fake_uuid, + } + + client_request.post( + "main.delivery_status_callback", + service_id=service_one["id"], + _data=data, ) - else: - mock_create_service_callback_api.assert_called_once_with( + + mock_update_service_callback_api.assert_called_once_with( service_one["id"], url="https://test.url.com/", bearer_token="1234567890", user_id=fake_uuid, + callback_api_id=fake_uuid, ) + def test_update_receive_text_message_callback_details( + self, + client_request, + service_one, + mock_update_service_inbound_api, + mock_get_valid_service_inbound_api, + fake_uuid, + mock_validate_callback_url, + ): + service_one["inbound_api"] = [fake_uuid] + service_one["permissions"] = ["inbound_sms"] -def test_update_delivery_status_callback_details( - client_request, - service_one, - mock_update_service_callback_api, - mock_get_valid_service_callback_api, - fake_uuid, - mock_validate_callback_url, - mocker, -): - service_one["service_callback_api"] = [fake_uuid] - - data = { - "url": "https://test.url.com/", - "bearer_token": "1234567890", - "user_id": fake_uuid, - } - - client_request.post( - "main.delivery_status_callback", - service_id=service_one["id"], - _data=data, - ) - - mock_update_service_callback_api.assert_called_once_with( - service_one["id"], url="https://test.url.com/", bearer_token="1234567890", user_id=fake_uuid, callback_api_id=fake_uuid - ) - - -def test_update_receive_text_message_callback_details( - client_request, - service_one, - mock_update_service_inbound_api, - mock_get_valid_service_inbound_api, - fake_uuid, - mock_validate_callback_url, -): - service_one["inbound_api"] = [fake_uuid] - service_one["permissions"] = ["inbound_sms"] - - data = {"url": "https://test.url.com/", - "bearer_token": "1234567890", "user_id": fake_uuid} - - client_request.post( - "main.received_text_messages_callback", - service_id=service_one["id"], - _data=data, - ) - - mock_update_service_inbound_api.assert_called_once_with( - service_one["id"], - url="https://test.url.com/", - bearer_token="1234567890", - user_id=fake_uuid, - inbound_api_id=fake_uuid, - ) - - -def test_update_delivery_status_callback_without_changes_does_not_update( - client_request, - service_one, - mock_update_service_callback_api, - fake_uuid, - mock_get_valid_service_callback_api, - mock_validate_callback_url, - mocker, -): - service_one["service_callback_api"] = [fake_uuid] - data = {"user_id": fake_uuid, "url": "https://hello2.canada.ca", - "bearer_token": "bearer_token_set"} - - client_request.post( - "main.delivery_status_callback", - service_id=service_one["id"], - _data=data, - ) - - assert mock_update_service_callback_api.called is False + data = {"url": "https://test.url.com/", "bearer_token": "1234567890", "user_id": fake_uuid} + client_request.post( + "main.received_text_messages_callback", + service_id=service_one["id"], + _data=data, + ) -def test_update_receive_text_message_callback_without_changes_does_not_update( - client_request, - service_one, - mock_update_service_inbound_api, - fake_uuid, - mock_get_valid_service_inbound_api, - mock_validate_callback_url, -): - service_one["inbound_api"] = [fake_uuid] - service_one["permissions"] = ["inbound_sms"] - data = {"user_id": fake_uuid, "url": "https://hello3.canada.ca", - "bearer_token": "bearer_token_set"} - - client_request.post( - "main.received_text_messages_callback", - service_id=service_one["id"], - _data=data, - ) + mock_update_service_inbound_api.assert_called_once_with( + service_one["id"], + url="https://test.url.com/", + bearer_token="1234567890", + user_id=fake_uuid, + inbound_api_id=fake_uuid, + ) - assert mock_update_service_inbound_api.called is False + def test_update_delivery_status_callback_without_changes_does_not_update( + self, + client_request, + service_one, + mock_update_service_callback_api, + fake_uuid, + mock_get_valid_service_callback_api, + mock_validate_callback_url, + mocker, + ): + service_one["service_callback_api"] = [fake_uuid] + data = {"user_id": fake_uuid, "url": "https://hello2.canada.ca", "bearer_token": "bearer_token_set"} + + client_request.post( + "main.delivery_status_callback", + service_id=service_one["id"], + _data=data, + ) + assert mock_update_service_callback_api.called is False + + def test_update_receive_text_message_callback_without_changes_does_not_update( + self, + client_request, + service_one, + mock_update_service_inbound_api, + fake_uuid, + mock_get_valid_service_inbound_api, + mock_validate_callback_url, + ): + service_one["inbound_api"] = [fake_uuid] + service_one["permissions"] = ["inbound_sms"] + data = {"user_id": fake_uuid, "url": "https://hello3.canada.ca", "bearer_token": "bearer_token_set"} -@pytest.mark.parametrize( - "service_callback_api, delivery_url, expected_1st_table_row", - [ - (None, {}, "Delivery receipts Not set Change"), - ( - sample_uuid(), - {"url": "https://delivery.receipts"}, - "Delivery receipts https://delivery.receipts Change", - ), - ], -) -@pytest.mark.parametrize( - "inbound_api, inbound_url, expected_2nd_table_row", - [ - (None, {}, "Received text messages Not set Change"), - ( - sample_uuid(), - {"url": "https://inbound.sms"}, - "Received text messages https://inbound.sms Change", - ), - ], -) -def test_callbacks_page_works_when_no_apis_set( - client_request, - service_one, - mocker, - service_callback_api, - delivery_url, - expected_1st_table_row, - inbound_api, - inbound_url, - expected_2nd_table_row, -): - service_one["permissions"] = ["inbound_sms"] - service_one["inbound_api"] = inbound_api - service_one["service_callback_api"] = service_callback_api + client_request.post( + "main.received_text_messages_callback", + service_id=service_one["id"], + _data=data, + ) - mocker.patch("app.service_api_client.get_service_callback_api", - return_value=delivery_url) - mocker.patch("app.service_api_client.get_service_inbound_api", - return_value=inbound_url) + assert mock_update_service_inbound_api.called is False - page = client_request.get( - "main.api_callbacks", service_id=service_one["id"], _follow_redirects=True) - expected_rows = [ + @pytest.mark.parametrize( + "service_callback_api, delivery_url, expected_1st_table_row", + [ + (None, {}, "Delivery receipts Not set Change"), + ( + sample_uuid(), + {"url": "https://delivery.receipts"}, + "Delivery receipts https://delivery.receipts Change", + ), + ], + ) + @pytest.mark.parametrize( + "inbound_api, inbound_url, expected_2nd_table_row", + [ + (None, {}, "Received text messages Not set Change"), + ( + sample_uuid(), + {"url": "https://inbound.sms"}, + "Received text messages https://inbound.sms Change", + ), + ], + ) + def test_callbacks_page_works_when_no_apis_set( + self, + client_request, + service_one, + mocker, + service_callback_api, + delivery_url, expected_1st_table_row, + inbound_api, + inbound_url, expected_2nd_table_row, - ] - rows = page.select("tbody tr") - assert len(rows) == 2 - for index, row in enumerate(expected_rows): - assert row == normalize_spaces(rows[index].text) + ): + service_one["permissions"] = ["inbound_sms"] + service_one["inbound_api"] = inbound_api + service_one["service_callback_api"] = service_callback_api + + mocker.patch("app.service_api_client.get_service_callback_api", return_value=delivery_url) + mocker.patch("app.service_api_client.get_service_inbound_api", return_value=inbound_url) + + page = client_request.get("main.api_callbacks", service_id=service_one["id"], _follow_redirects=True) + expected_rows = [ + expected_1st_table_row, + expected_2nd_table_row, + ] + rows = page.select("tbody tr") + assert len(rows) == 2 + for index, row in enumerate(expected_rows): + assert row == normalize_spaces(rows[index].text) diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 32a0ec6917..49c94a005e 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -4294,7 +4294,6 @@ def test_update_service_data_retention_populates_form( class TestSettingSensitiveService: - def test_should_redirect_after_change_service_name( self, client_request, @@ -4310,7 +4309,6 @@ def test_should_redirect_after_change_service_name( class TestSuspendingCallbackApi: def test_should_suspend_service_callback_api(self, client_request, platform_admin_user, mocker, service_one): - client_request.login(platform_admin_user, service_one) client_request.post( "main.suspend_callback", diff --git a/tests/conftest.py b/tests/conftest.py index 3913b28292..529587d870 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -3736,7 +3736,10 @@ def mock_get_empty_service_callback_api(mocker): @pytest.fixture(scope="function") def mock_validate_callback_url(mocker): - return mocker.patch("app.main.validators.requests.post", return_value=Mock(content="a", status_code=200, headers={"a": "a"}, elapsed=timedelta(seconds=0.3))) + return mocker.patch( + "app.main.validators.requests.post", + return_value=Mock(content="a", status_code=200, headers={"a": "a"}, elapsed=timedelta(seconds=0.3)), + ) @pytest.fixture(scope="function") From 104bfca4cf0609304f33b20976d5b374c07a7a49 Mon Sep 17 00:00:00 2001 From: wbanks Date: Wed, 11 Sep 2024 16:24:32 -0400 Subject: [PATCH 12/22] Fix updated translations in code --- app/main/views/api_keys.py | 32 ++++++++++++++++---------------- app/translations/csv/fr.csv | 5 +++-- 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/app/main/views/api_keys.py b/app/main/views/api_keys.py index cb66c165f5..a13709c235 100644 --- a/app/main/views/api_keys.py +++ b/app/main/views/api_keys.py @@ -185,10 +185,10 @@ def delete_delivery_status_callback(service_id): delivery_status_callback["id"], ) - flash(_l("Callback configuration deleted"), "default_with_tick") + flash(_l("Callback configuration deleted."), "default_with_tick") return redirect(url_for(back_link, service_id=service_id)) - flash(_l("Are you sure you want to delete this callback?"), "delete") + flash(_l("Are you sure you want to delete this callback configuration?"), "delete") form = ServiceDeliveryStatusCallbackForm( url=delivery_status_callback.get("url") if delivery_status_callback else "", @@ -221,7 +221,7 @@ def delivery_status_callback(service_id): if form.validate_on_submit(): # As part of the ValidCallbackUrl validation, we ping their callback URL to check if it's up and set the response time in g - response_time = "{:.2f} {}".format(g.callback_response_time, _l("seconds")) + response_time = "{:.2f}".format(g.callback_response_time) url_hostname = form.url.data.split("https://")[1] # Update existing callback @@ -238,7 +238,7 @@ def delivery_status_callback(service_id): # If the user is just testing their URL, don't send them back to the API Integration page if request.form.get("button_pressed") == "test_response_time": flash( - _l("Your service '{}' responded in {}").format( + _l("The service {} responded in {} seconds.").format( url_hostname, response_time, ), @@ -247,7 +247,7 @@ def delivery_status_callback(service_id): return redirect(url_for("main.delivery_status_callback", service_id=service_id)) flash( - _l("Callback to '{}' saved. Your service responded in {}").format( + _l("We’ve saved your callback configuration. {} responded in {} seconds.").format( url_hostname, response_time, ), @@ -265,7 +265,7 @@ def delivery_status_callback(service_id): ) flash( - _l("Callback to '{}' created. Your service responded in {}").format( + _l("We’ve set up your callback configuration. {} responded in {} seconds.").format( url_hostname, response_time, ), @@ -281,7 +281,7 @@ def delivery_status_callback(service_id): if request.form.get("button_pressed") == "test_response_time": flash( - _l("Your service '{}' responded in {}").format( + _l("The service {} responded in {} seconds.").format( url_hostname, response_time, ), @@ -290,7 +290,7 @@ def delivery_status_callback(service_id): return redirect(url_for("main.delivery_status_callback", service_id=service_id)) flash( - _l("Callback to '{}' saved. Your service responded in {}").format( + _l("We’ve saved your callback configuration. {} responded in {} seconds.").format( url_hostname, response_time, ), @@ -332,7 +332,7 @@ def received_text_messages_callback(service_id): if form.validate_on_submit(): # As part of the ValidCallbackUrl validation, we ping their callback URL to check if it's up and set the response time in g - response_time = "{:.2f} {}".format(g.callback_response_time, _l("seconds")) + response_time = "{:.2f}".format(g.callback_response_time) url_hostname = form.url.data.split("https://")[1] if received_text_messages_callback and form.url.data: @@ -348,7 +348,7 @@ def received_text_messages_callback(service_id): # If the user is just testing their URL, don't send them back to the API Integration page if request.form.get("button_pressed") == "test_response_time": flash( - _l("Your service '{}' responded in {}").format( + _l("The service {} responded in {} seconds.").format( url_hostname, response_time, ), @@ -357,7 +357,7 @@ def received_text_messages_callback(service_id): return redirect(url_for("main.received_text_messages_callback", service_id=service_id)) flash( - _l("Callback to '{}' saved. Your service responded in {}").format( + _l("We’ve saved your callback configuration. {} responded in {} seconds.").format( url_hostname, response_time, ), @@ -377,7 +377,7 @@ def received_text_messages_callback(service_id): user_id=current_user.id, ) flash( - _l("Callback to '{}' created. Your service responded in {}").format( + _l("We’ve set up your callback configuration. {} responded in {} seconds.").format( url_hostname, response_time, ), @@ -388,7 +388,7 @@ def received_text_messages_callback(service_id): if request.form.get("button_pressed") == "test_response_time": flash( - _l("Your service '{}' responded in {}").format( + _l("The service {} responded in {} seconds.").format( url_hostname, response_time, ), @@ -397,7 +397,7 @@ def received_text_messages_callback(service_id): return redirect(url_for("main.received_text_messages_callback", service_id=service_id)) flash( - _l("Callback to '{}' saved. Your service responded in {}").format( + _l("We’ve saved your callback configuration. {} responded in {} seconds.").format( url_hostname, response_time, ), @@ -427,10 +427,10 @@ def delete_received_text_messages_callback(service_id): received_text_messages_callback["id"], ) - flash(_l("Callback configuration deleted"), "default_with_tick") + flash(_l("Callback configuration deleted."), "default_with_tick") return redirect(url_for(back_link, service_id=service_id)) - flash(_l("Are you sure you want to delete this callback?"), "delete") + flash(_l("Are you sure you want to delete this callback configuration?"), "delete") form = ServiceReceiveMessagesCallbackForm( url=received_text_messages_callback.get("url") if delivery_status_callback else "", diff --git a/app/translations/csv/fr.csv b/app/translations/csv/fr.csv index e853f53414..23d18fd270 100644 --- a/app/translations/csv/fr.csv +++ b/app/translations/csv/fr.csv @@ -1910,7 +1910,6 @@ "Keep accurate contact information","Maintenir à jour les coordonnées" "Send information that’s unclassified or Protected A only","N’envoyer que des renseignements non classifiés ou de niveau Protégé A" "Practice continuous security","Pratiquer la sécurité continue" -"Getting started","Découvrez comment fonctionne Notification GC" "Read and agree to the terms of use","Lisez et acceptez les conditions d’utilisation" "Read and agree to continue","Lisez et acceptez les conditions d’utilisation" "Agree follows terms of use","Accepter suite aux conditions d'utilisation" @@ -1983,4 +1982,6 @@ "Suspend Callback","Suspension du rappel" "We’ve set up your callback configuration. {} responded in {} seconds.","Nous avons configuré votre rappel. {} a répondu en {} secondes." "We’ve saved your callback configuration. {} responded in {} seconds.","Nous avons enregistré votre configuration de rappel. {} a répondu en {} secondes." -"The service {} responded in {} seconds.","Le service {} a répondu en {} secondes." \ No newline at end of file +"The service {} responded in {} seconds.","Le service {} a répondu en {} secondes." +"Are you sure you want to delete this callback configuration?","Êtes-vous sûr de vouloir supprimer cette configuration de rappel ?" +"Callback configuration deleted.","Configuration de rappel supprimée." \ No newline at end of file From dd83ae9a19aa30f1fd7e594650cb94d457650d36 Mon Sep 17 00:00:00 2001 From: wbanks Date: Tue, 17 Sep 2024 15:05:20 -0400 Subject: [PATCH 13/22] Add & fix tests - Add new message content --- app/main/views/api_keys.py | 20 +++++++- app/translations/csv/fr.csv | 3 +- tests/app/main/views/test_api_integration.py | 50 ++++++++++++++++++++ 3 files changed, 70 insertions(+), 3 deletions(-) diff --git a/app/main/views/api_keys.py b/app/main/views/api_keys.py index a13709c235..9103e9fde8 100644 --- a/app/main/views/api_keys.py +++ b/app/main/views/api_keys.py @@ -279,7 +279,15 @@ def delivery_status_callback(service_id): # nothing for us to do here pass - if request.form.get("button_pressed") == "test_response_time": + if request.form.get("button_pressed") == "test_response_time" and g.callback_response_time >= 1: + flash( + _l("The service {} took longer than 1 second to respond.").format( + url_hostname, + ), + "error", + ) + return redirect(url_for("main.delivery_status_callback", service_id=service_id)) + else: flash( _l("The service {} responded in {} seconds.").format( url_hostname, @@ -386,7 +394,15 @@ def received_text_messages_callback(service_id): return redirect(url_for(back_link, service_id=service_id)) - if request.form.get("button_pressed") == "test_response_time": + if request.form.get("button_pressed") == "test_response_time" and g.callback_response_time >= 1: + flash( + _l("The service {} took longer than 1 second to respond.").format( + url_hostname, + ), + "error", + ) + return redirect(url_for("main.received_text_messages_callback", service_id=service_id)) + else: flash( _l("The service {} responded in {} seconds.").format( url_hostname, diff --git a/app/translations/csv/fr.csv b/app/translations/csv/fr.csv index 23d18fd270..81958a099d 100644 --- a/app/translations/csv/fr.csv +++ b/app/translations/csv/fr.csv @@ -1984,4 +1984,5 @@ "We’ve saved your callback configuration. {} responded in {} seconds.","Nous avons enregistré votre configuration de rappel. {} a répondu en {} secondes." "The service {} responded in {} seconds.","Le service {} a répondu en {} secondes." "Are you sure you want to delete this callback configuration?","Êtes-vous sûr de vouloir supprimer cette configuration de rappel ?" -"Callback configuration deleted.","Configuration de rappel supprimée." \ No newline at end of file +"Callback configuration deleted.","Configuration de rappel supprimée." +"The service {service host name} took longer than 1 second to respond.","" \ No newline at end of file diff --git a/tests/app/main/views/test_api_integration.py b/tests/app/main/views/test_api_integration.py index 7d6c718727..02d1e2ea93 100644 --- a/tests/app/main/views/test_api_integration.py +++ b/tests/app/main/views/test_api_integration.py @@ -1,3 +1,4 @@ +import datetime import uuid from collections import OrderedDict from unittest.mock import Mock, call @@ -535,6 +536,7 @@ def test_callback_forms_validation( client_request, service_one, mock_get_valid_service_callback_api, + mock_validate_callback_url, endpoint, url, bearer_token, @@ -558,6 +560,54 @@ def test_callback_forms_validation( assert error_msgs == expected_errors + @pytest.mark.parametrize( + "endpoint", + [ + ("main.delivery_status_callback"), + ("main.received_text_messages_callback"), + ], + ) + def test_callback_response_time_banner_shows_error_when_response_time_greater_than_one_second( + self, + endpoint, + fake_uuid, + client_request, + service_one, + mock_get_valid_service_callback_api, + mock_get_valid_service_inbound_api, + mocker, + ): + mocker.patch( + "app.main.validators.requests.post", return_value=Mock(elapsed=datetime.timedelta(seconds=1.1), status_code=200) + ) + + if endpoint == "main.received_text_messages_callback": + service_one["permissions"] = ["inbound_sms"] + service_one["inbound_api"] = [fake_uuid] + url = "https://hello3.canada.ca" + else: + service_one["service_callback_api"] = [fake_uuid] + url = "https://hello2.canada.ca" + + data = { + "url": url, + "bearer_token": "bearer_token_set", + "button_pressed": "test_response_time", + } + + response = client_request.post( + endpoint, + service_id=service_one["id"], + _data=data, + _follow_redirects=True, + ) + + expected_banner_msg = f"The service {url.split('https://')[1]} took longer than 1 second to respond." + page = BeautifulSoup(response.decode("utf-8"), "html.parser") + banner_msg = normalize_spaces(page.select(".banner-dangerous")[0].text) + + assert banner_msg == expected_banner_msg + @pytest.mark.parametrize( "endpoint, expected_delete_url", [ From 1468f2190ee882c58b94cdba72ca344792325b40 Mon Sep 17 00:00:00 2001 From: wbanks Date: Tue, 17 Sep 2024 16:33:32 -0400 Subject: [PATCH 14/22] Update delete message --- app/translations/csv/fr.csv | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/translations/csv/fr.csv b/app/translations/csv/fr.csv index 81958a099d..25e7c1445a 100644 --- a/app/translations/csv/fr.csv +++ b/app/translations/csv/fr.csv @@ -1984,5 +1984,5 @@ "We’ve saved your callback configuration. {} responded in {} seconds.","Nous avons enregistré votre configuration de rappel. {} a répondu en {} secondes." "The service {} responded in {} seconds.","Le service {} a répondu en {} secondes." "Are you sure you want to delete this callback configuration?","Êtes-vous sûr de vouloir supprimer cette configuration de rappel ?" -"Callback configuration deleted.","Configuration de rappel supprimée." +"Your Callback configuration has been deleted.","Configuration de rappel supprimée." "The service {service host name} took longer than 1 second to respond.","" \ No newline at end of file From 7164b9ef91f24cfe6c2c4d1ad5cb2e3c9eec45f8 Mon Sep 17 00:00:00 2001 From: wbanks Date: Wed, 18 Sep 2024 13:32:30 -0400 Subject: [PATCH 15/22] Update french translations --- app/translations/csv/fr.csv | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/translations/csv/fr.csv b/app/translations/csv/fr.csv index 25e7c1445a..93795f304f 100644 --- a/app/translations/csv/fr.csv +++ b/app/translations/csv/fr.csv @@ -1980,9 +1980,9 @@ "Review your activity","Examinez votre activité" "Set sensitive service","Définir un service sensible" "Suspend Callback","Suspension du rappel" -"We’ve set up your callback configuration. {} responded in {} seconds.","Nous avons configuré votre rappel. {} a répondu en {} secondes." +"We’ve set up your callback configuration. {} responded in {} seconds.","Nous avons mis en place votre configuration de rappel. {} a répondu en {} secondes." "We’ve saved your callback configuration. {} responded in {} seconds.","Nous avons enregistré votre configuration de rappel. {} a répondu en {} secondes." "The service {} responded in {} seconds.","Le service {} a répondu en {} secondes." -"Are you sure you want to delete this callback configuration?","Êtes-vous sûr de vouloir supprimer cette configuration de rappel ?" -"Your Callback configuration has been deleted.","Configuration de rappel supprimée." +"Are you sure you want to delete this callback configuration?","Voulez-vous vraiment supprimer cette configuration de rappel?" +"Your Callback configuration has been deleted.","La configuration de rappel est supprimée." "The service {service host name} took longer than 1 second to respond.","" \ No newline at end of file From 0d0604e8539f3f8253a1b26cff0d10161b549831 Mon Sep 17 00:00:00 2001 From: wbanks Date: Mon, 23 Sep 2024 10:36:12 -0400 Subject: [PATCH 16/22] Add UI tests for callbacks page --- app/templates/components/page-footer.html | 7 +- app/templates/views/api/callbacks.html | 4 +- .../callbacks/delivery-status-callback.html | 9 +- app/templates/views/api/index.html | 8 +- poetry.lock | 2 +- .../Notify/Admin/Pages/ApiIntegrationPage.js | 28 ++++++ .../Notify/Admin/Pages/CallbacksPage.js | 38 ++++++++ .../cypress/Notify/Admin/Pages/all.js | 4 + tests_cypress/cypress/Notify/NotifyAPI.js | 88 ++++++++++++++++++- .../e2e/admin/api_integration/callbacks.cy.js | 78 ++++++++++++++++ 10 files changed, 253 insertions(+), 13 deletions(-) create mode 100644 tests_cypress/cypress/Notify/Admin/Pages/ApiIntegrationPage.js create mode 100644 tests_cypress/cypress/Notify/Admin/Pages/CallbacksPage.js create mode 100644 tests_cypress/cypress/e2e/admin/api_integration/callbacks.cy.js diff --git a/app/templates/components/page-footer.html b/app/templates/components/page-footer.html index 144aaaccab..3cf2ba666a 100644 --- a/app/templates/components/page-footer.html +++ b/app/templates/components/page-footer.html @@ -84,6 +84,7 @@ class="button text-base outline-none focus:shadow-outline" name="button_pressed" value="{{ button1_value }}" + data-testid="{{ button1_text.lower().replace(" ", '-') }}" > {{- button1_text -}} @@ -95,13 +96,17 @@ class="button button-secondary text-base outline-none focus:shadow-outline" name="button_pressed" value="{{ button2_value }}" + data-testid="{{ button2_text.lower().replace(" ", '-') }}" > {{- button2_text -}} {% endif %} {% if delete_link %} - + {{ delete_link_text }} {% endif %} diff --git a/app/templates/views/api/callbacks.html b/app/templates/views/api/callbacks.html index 6831405e9b..9b5b503d50 100644 --- a/app/templates/views/api/callbacks.html +++ b/app/templates/views/api/callbacks.html @@ -24,13 +24,13 @@ {% call row() %} {{ text_field('Delivery receipts') }} {{ optional_text_field(delivery_status_callback, truncate=false) }} - {{ edit_field('Change', url_for('.delivery_status_callback', service_id=current_service.id)) }} + {{ edit_field('Change', url_for('.delivery_status_callback', service_id=current_service.id), attributes="data-testid='change-delivery-receipts'") }} {% endcall %} {% call row() %} {{ text_field('Received text messages') }} {{ optional_text_field(received_text_messages_callback, truncate=false) }} - {{ edit_field('Change', url_for('.received_text_messages_callback', service_id=current_service.id)) }} + {{ edit_field('Change', url_for('.received_text_messages_callback', service_id=current_service.id), attributes="data-testid='change-received-text-messages'") }} {% endcall %} {% endcall %} diff --git a/app/templates/views/api/callbacks/delivery-status-callback.html b/app/templates/views/api/callbacks/delivery-status-callback.html index 211ed01a39..988466b5c3 100644 --- a/app/templates/views/api/callbacks/delivery-status-callback.html +++ b/app/templates/views/api/callbacks/delivery-status-callback.html @@ -28,7 +28,8 @@

{{ textbox( form.url, width='w-full', - hint=hint_text + hint=hint_text, + testid='url', ) }}

{{ _('Add a secret value for authentication') }} @@ -38,10 +39,12 @@

form.bearer_token, width='w-full', hint=hint_txt, - autocomplete='new-password' + autocomplete='new-password', + testid='bearer-token', ) }} {% set test_response_txt = _('Test response time') if has_callback_config else None %} {% set test_response_value = _('test_response_time') if has_callback_config else None %} + {% set delete_link = url_for('.delete_delivery_status_callback', service_id=current_service.id) if has_callback_config else None %} {% set display_footer = is_deleting if is_deleting else False %} {% if not display_footer %} {{ sticky_page_footer_two_submit_buttons_and_delete_link( @@ -49,7 +52,7 @@

button1_value=_('save'), button2_text=test_response_txt, button2_value=test_response_value, - delete_link=url_for('.delete_delivery_status_callback', service_id=current_service.id), + delete_link=delete_link, delete_link_text=_('Delete') ) }} {% endif %} diff --git a/app/templates/views/api/index.html b/app/templates/views/api/index.html index 7d084f4ade..47918e5ce0 100644 --- a/app/templates/views/api/index.html +++ b/app/templates/views/api/index.html @@ -21,7 +21,7 @@