From d02cd67b0dde033dc7fbef70e372ae035bba373f Mon Sep 17 00:00:00 2001 From: chrisw Date: Mon, 30 Oct 2017 14:30:43 +0000 Subject: [PATCH] Fixed broken edit functionality --- app/main/forms.py | 6 ++++- app/main/views/service_settings.py | 27 +++++++++++-------- .../service-settings/sms-sender/edit.html | 4 +-- tests/app/main/test_validators.py | 4 +-- tests/app/main/views/test_service_settings.py | 12 ++++----- 5 files changed, 31 insertions(+), 22 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index 82c62288e..75d15fb16 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -497,7 +497,7 @@ class ServiceReplyToEmailForm(Form): is_default = BooleanField("Make this email address the default") -class ServiceSmsSender(Form): +class ServiceSmsSenderForm(Form): sms_sender = StringField( 'Text message sender', validators=[ @@ -512,6 +512,10 @@ class ServiceSmsSender(Form): raise ValidationError('Use letters and numbers only') +class ServiceEditInboundNumberForm(Form): + is_default = BooleanField("Make this text message sender the default") + + class ServiceLetterContactBlockForm(Form): letter_contact_block = TextAreaField( validators=[ diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 2bc91599a..4c2cef034 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -29,7 +29,7 @@ from app.main.forms import ( RequestToGoLiveForm, ServiceReplyToEmailForm, ServiceInboundNumberForm, - ServiceSmsSender, + ServiceSmsSenderForm, ServiceLetterContactBlockForm, ServiceBrandingOrg, LetterBranding, @@ -37,6 +37,7 @@ from app.main.forms import ( InternationalSMSForm, OrganisationTypeForm, FreeSMSAllowance, + ServiceEditInboundNumberForm, ) from app import user_api_client, current_service, organisations_client, inbound_number_client from notifications_utils.formatters import formatted_list @@ -428,7 +429,7 @@ def service_edit_email_reply_to(service_id, reply_to_email_id): @login_required @user_has_permissions('manage_settings', admin_override=True) def service_set_sms_sender(service_id): - form = ServiceSmsSender() + form = ServiceSmsSenderForm() if form.validate_on_submit(): if 'inbound_sms' in current_service['permissions']: abort(403) @@ -608,7 +609,7 @@ def service_sms_senders(service_id): @login_required @user_has_permissions('manage_settings', admin_override=True) def service_add_sms_sender(service_id): - form = ServiceSmsSender() + form = ServiceSmsSenderForm() sms_sender_count = len(service_api_client.get_sms_senders(service_id)) first_sms_sender = sms_sender_count == 0 if form.validate_on_submit(): @@ -629,24 +630,28 @@ def service_add_sms_sender(service_id): @user_has_permissions('manage_settings', admin_override=True) def service_edit_sms_sender(service_id, sms_sender_id): sms_sender = service_api_client.get_sms_sender(service_id, sms_sender_id) - form = ServiceSmsSender() - form.sms_sender.data = sms_sender['sms_sender'] - is_inbound_number = True if sms_sender['inbound_number_id'] else False - if request.method == 'GET': - form.is_default.data = sms_sender['is_default'] + is_inbound_number = sms_sender['inbound_number_id'] + if is_inbound_number: + form = ServiceEditInboundNumberForm(is_default=sms_sender['is_default']) + else: + form = ServiceSmsSenderForm(**sms_sender) + if form.validate_on_submit(): service_api_client.update_sms_sender( current_service['id'], sms_sender_id=sms_sender_id, - sms_sender=form.sms_sender.data.replace('\r', ''), + sms_sender=sms_sender['sms_sender'] if is_inbound_number else form.sms_sender.data.replace('\r', ''), is_default=True if sms_sender['is_default'] else form.is_default.data ) return redirect(url_for('.service_sms_senders', service_id=service_id)) + + form.is_default.data = sms_sender['is_default'] return render_template( 'views/service-settings/sms-sender/edit.html', form=form, - sms_sender_id=sms_sender['id'], - is_inbound_number=is_inbound_number) + sms_sender=sms_sender, + inbound_number=is_inbound_number + ) @main.route("/services//service-settings/set-letter-contact-block", methods=['GET', 'POST']) diff --git a/app/templates/views/service-settings/sms-sender/edit.html b/app/templates/views/service-settings/sms-sender/edit.html index 81857c2cd..cccc8ef12 100644 --- a/app/templates/views/service-settings/sms-sender/edit.html +++ b/app/templates/views/service-settings/sms-sender/edit.html @@ -13,9 +13,9 @@ Edit text message sender
- {% if is_inbound_number %} + {% if inbound_number %}

- {{ form.sms_sender.data }} + {{ sms_sender.sms_sender }} This phone number receives replies and can’t be changed

{% else %} diff --git a/tests/app/main/test_validators.py b/tests/app/main/test_validators.py index 7b70e74dd..663d7b438 100644 --- a/tests/app/main/test_validators.py +++ b/tests/app/main/test_validators.py @@ -1,5 +1,5 @@ import pytest -from app.main.forms import RegisterUserForm, ServiceSmsSender +from app.main.forms import RegisterUserForm, ServiceSmsSenderForm from app.main.validators import ValidGovEmail, NoCommasInPlaceHolders, OnlyGSMCharacters from wtforms import ValidationError from unittest.mock import Mock @@ -184,7 +184,7 @@ def test_sms_sender_form_validation( client, mock_get_user_by_email, ): - form = ServiceSmsSender() + form = ServiceSmsSenderForm() form.sms_sender.data = 'elevenchars' form.validate() diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 4ae91e081..3a7cc9884 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -1060,10 +1060,10 @@ def test_edit_letter_contact_block( @pytest.mark.parametrize('fixture, data, api_default_args', [ - (get_default_sms_sender, {"is_default": "y"}, True), - (get_default_sms_sender, {}, True), - (get_non_default_sms_sender, {}, False), - (get_non_default_sms_sender, {"is_default": "y"}, True) + (get_default_sms_sender, {"is_default": "y", "sms_sender": "test"}, True), + (get_default_sms_sender, {"sms_sender": "test"}, True), + (get_non_default_sms_sender, {"sms_sender": "test"}, False), + (get_non_default_sms_sender, {"is_default": "y", "sms_sender": "test"}, True) ]) def test_edit_sms_sender( fixture, @@ -1085,7 +1085,7 @@ def test_edit_sms_sender( mock_update_sms_sender.assert_called_once_with( SERVICE_ONE_ID, sms_sender_id=fake_uuid, - sms_sender="GOVUK", + sms_sender="test", is_default=api_default_args ) @@ -1184,7 +1184,7 @@ def test_inbound_sms_sender_is_not_editable( sms_sender_id=fixture_sender_id, ) - assert (page.select_one('main input[name="sms_sender"]') is None) == hide_textbox + assert bool(page.find('input', attrs={'name': "sms_sender"})) != hide_textbox if hide_textbox: assert normalize_spaces( page.select_one('form[method="post"] p').text