From be7d4891ae7d127d79bbe08b6c3421ce50d01067 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Mon, 3 Aug 2020 15:10:02 +0100 Subject: [PATCH] Consolidate callback forms and convert them to gov uk frontend We had two identical callback form classes. One for delivery callbacks and one for inbound sms callbacks. Since they did not differ I consolidated them into one CallbackForm class that they both inherited from previously. I also substituted field classes for this form with new fields that cooperate with gov uk frontend. --- app/main/forms.py | 41 +++++-------------- app/main/views/api_keys.py | 11 ++--- .../callbacks/delivery-status-callback.html | 22 +++++----- .../received-text-messages-callback.html | 21 +++++----- tests/app/main/views/test_api_integration.py | 4 +- 5 files changed, 36 insertions(+), 63 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index bbb5fd494..d7a8f2638 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -34,7 +34,6 @@ from wtforms import ( TextAreaField, ValidationError, validators, - widgets, ) from wtforms.fields.html5 import EmailField, SearchField, TelField from wtforms.validators import URL, DataRequired, Length, Optional, Regexp @@ -1804,10 +1803,6 @@ class PlaceholderForm(StripWhitespaceForm): pass -class PasswordFieldShowHasContent(StringField): - widget = widgets.PasswordInput(hide_value=False) - - class ServiceInboundNumberForm(StripWhitespaceForm): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) @@ -1820,37 +1815,21 @@ class ServiceInboundNumberForm(StripWhitespaceForm): class CallbackForm(StripWhitespaceForm): + url = GovukTextInputField( + "URL", + validators=[DataRequired(message='Cannot be empty'), + Regexp(regex="^https.*", message='Must be a valid https URL')] + ) + bearer_token = GovukPasswordField( + "Bearer token", + validators=[DataRequired(message='Cannot be empty'), + Length(min=10, message='Must be at least 10 characters')] + ) def validate(self): return super().validate() or self.url.data == '' -class ServiceReceiveMessagesCallbackForm(CallbackForm): - url = StringField( - "URL", - validators=[DataRequired(message='Cannot be empty'), - Regexp(regex="^https.*", message='Must be a valid https URL')] - ) - bearer_token = PasswordFieldShowHasContent( - "Bearer token", - validators=[DataRequired(message='Cannot be empty'), - Length(min=10, message='Must be at least 10 characters')] - ) - - -class ServiceDeliveryStatusCallbackForm(CallbackForm): - url = StringField( - "URL", - validators=[DataRequired(message='Cannot be empty'), - Regexp(regex="^https.*", message='Must be a valid https URL')] - ) - bearer_token = PasswordFieldShowHasContent( - "Bearer token", - validators=[DataRequired(message='Cannot be empty'), - Length(min=10, message='Must be at least 10 characters')] - ) - - class InternationalSMSForm(StripWhitespaceForm): enabled = RadioField( 'Send text messages to international phone numbers', diff --git a/app/main/views/api_keys.py b/app/main/views/api_keys.py index 8fe5e1c88..5c599d84f 100644 --- a/app/main/views/api_keys.py +++ b/app/main/views/api_keys.py @@ -16,12 +16,7 @@ from app import ( service_api_client, ) from app.main import main -from app.main.forms import ( - CreateKeyForm, - GuestList, - ServiceDeliveryStatusCallbackForm, - ServiceReceiveMessagesCallbackForm, -) +from app.main.forms import CallbackForm, CreateKeyForm, GuestList from app.notify_client.api_key_api_client import ( KEY_TYPE_NORMAL, KEY_TYPE_TEAM, @@ -195,7 +190,7 @@ def delivery_status_callback(service_id): else '.api_integration' ) - form = ServiceDeliveryStatusCallbackForm( + form = CallbackForm( url=delivery_status_callback.get('url') if delivery_status_callback else '', bearer_token=dummy_bearer_token if delivery_status_callback else '' ) @@ -255,7 +250,7 @@ def received_text_messages_callback(service_id): return redirect(url_for('.api_integration', service_id=service_id)) received_text_messages_callback = get_received_text_messages_callback() - form = ServiceReceiveMessagesCallbackForm( + form = CallbackForm( url=received_text_messages_callback.get('url') if received_text_messages_callback else '', bearer_token=dummy_bearer_token if received_text_messages_callback else '' ) diff --git a/app/templates/views/api/callbacks/delivery-status-callback.html b/app/templates/views/api/callbacks/delivery-status-callback.html index ee0841b08..8ad50ce5a 100644 --- a/app/templates/views/api/callbacks/delivery-status-callback.html +++ b/app/templates/views/api/callbacks/delivery-status-callback.html @@ -23,17 +23,17 @@

{% call form_wrapper() %} - {{ textbox( - form.url, - width='1-1', - hint='Must start with https://' - ) }} - {{ textbox( - form.bearer_token, - width='1-1', - hint='At least 10 characters', - autocomplete='new-password' - ) }} + {{ form.url(param_extensions={ + "classes": "govuk-!-width-full", + "hint": {"text": "Must start with https://"} + }) }} + + {{ form.bearer_token(param_extensions={ + "classes": "govuk-!-width-full", + "hint": {"text": "At least 10 characters"}, + "autocomplete": "new-password" + }) }} + {{ page_footer('Save') }} {% endcall %} 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 1d02b146b..16db770f3 100644 --- a/app/templates/views/api/callbacks/received-text-messages-callback.html +++ b/app/templates/views/api/callbacks/received-text-messages-callback.html @@ -21,17 +21,16 @@

{% call form_wrapper() %} - {{ textbox( - form.url, - width='1-1', - hint='Must start with https://' - ) }} - {{ textbox( - form.bearer_token, - width='1-1', - hint='At least 10 characters', - autocomplete='new-password' - ) }} + {{ form.url(param_extensions={ + "classes": "govuk-!-width-full", + "hint": {"text": "Must start with https://"} + }) }} + + {{ form.bearer_token(param_extensions={ + "classes": "govuk-!-width-full", + "hint": {"text": "At least 10 characters"}, + "autocomplete": "new-password" + }) }} {{ page_footer('Save') }} {% endcall %} diff --git a/tests/app/main/views/test_api_integration.py b/tests/app/main/views/test_api_integration.py index 17a9a361d..1204dc42b 100644 --- a/tests/app/main/views/test_api_integration.py +++ b/tests/app/main/views/test_api_integration.py @@ -512,9 +512,9 @@ def test_callback_forms_validation( _data=data, _expected_status=200 ) - error_msgs = ' '.join(msg.text.strip() for msg in response.select(".error-message")) + error_msgs = ' '.join(msg.text.strip() for msg in response.select(".govuk-error-message")) - assert error_msgs == expected_errors + assert expected_errors in error_msgs @pytest.mark.parametrize('bearer_token', ['', 'some-bearer-token'])