From fe24501075e9449f1621a8e23e6967d3966c9b2d Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Fri, 2 Jun 2017 16:25:24 +0100 Subject: [PATCH] Add a platform admin button to service-settings page to turn the inbound_sms messaging on and off. If clicked you will be prompted to enter a sms sender number, when setting the permission on or off. Team members will always be able to see the number, but will only be able to change it if the inbound_sms permission is off. --- app/main/views/service_settings.py | 26 +++++++++--- app/notify_client/service_api_client.py | 1 + app/templates/views/service-settings.html | 14 ++++++- tests/__init__.py | 4 +- tests/app/main/views/test_service_settings.py | 40 +++++++++++++++++++ tests/conftest.py | 3 +- 6 files changed, 78 insertions(+), 10 deletions(-) diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index e8a32fc1b..97cf75fad 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -49,6 +49,7 @@ def service_settings(service_id): letter_branding=letter_branding_organisations.get( current_service.get('dvla_organisation', '001') ), + can_receive_inbound=('inbound_sms' in current_service['permissions']), letter_contact_block=Field(current_service['letter_contact_block'], html='escape') ) @@ -267,14 +268,27 @@ def service_set_reply_to_email(service_id): @user_has_permissions('manage_settings', admin_override=True) def service_set_sms_sender(service_id): form = ServiceSmsSender() + if form.validate_on_submit(): + set_inbound_sms = request.args.get('set_inbound_sms', False) + if set_inbound_sms == 'True': + permissions = current_service['permissions'] + if 'inbound_sms' in permissions: + permissions.remove('inbound_sms') + else: + permissions.append('inbound_sms') + service_api_client.update_service_with_properties( + current_service['id'], + {'permissions': permissions, + 'sms_sender': form.sms_sender.data or None} + ) + else: + service_api_client.update_service( + current_service['id'], + sms_sender=form.sms_sender.data or None + ) + return redirect(url_for('.service_settings', service_id=service_id)) if request.method == 'GET': form.sms_sender.data = current_service.get('sms_sender') - if form.validate_on_submit(): - service_api_client.update_service( - current_service['id'], - sms_sender=form.sms_sender.data or None - ) - return redirect(url_for('.service_settings', service_id=service_id)) return render_template( 'views/service-settings/set-sms-sender.html', form=form) diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index dc48d0043..e97919257 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -94,6 +94,7 @@ class ServiceAPIClient(NotifyAdminAPIClient): 'organisation', 'letter_contact_block', 'dvla_organisation', + 'permissions' } if disallowed_attributes: raise TypeError('Not allowed to update service attributes: {}'.format( diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html index 64874597b..34580721c 100644 --- a/app/templates/views/service-settings.html +++ b/app/templates/views/service-settings.html @@ -36,8 +36,13 @@ {% call row() %} {{ text_field('Text message sender') }} - {{ text_field(current_service.sms_sender) }} - {{ edit_field('Change', url_for('.service_set_sms_sender', service_id=current_service.id)) }} + {{ text_field(current_service.sms_sender}} + {% if current_user.has_permissions([], admin_override=True) or not can_receive_inbound %} + {{ edit_field('Change', url_for('.service_set_sms_sender', service_id=current_service.id, set_inbound_sms=False)) }} + {% else %} + {{ text_field('') }} + {% endif %} + {% endcall %} {% call row() %} @@ -162,6 +167,11 @@ {% endif %} +
  • + + {{ 'Stop inbound sms' if can_receive_inbound else 'Allow inbound sms' }} + +
    • {% endif %} diff --git a/tests/__init__.py b/tests/__init__.py index bb0311efe..9454fe727 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -54,7 +54,8 @@ def service_json( organisation=None, branding='govuk', created_at=None, - letter_contact_block=None + letter_contact_block=None, + permissions=[] ): if users is None: users = [] @@ -76,6 +77,7 @@ def service_json( 'created_at': created_at or str(datetime.utcnow()), 'letter_contact_block': letter_contact_block, 'dvla_organisation': '001', + 'permissions': permissions, } diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 6fc6ea56e..245460259 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -92,6 +92,22 @@ def test_if_cant_send_letters_then_cant_see_letter_contact_block( assert 'Letter contact block' not in response.get_data(as_text=True) +def test_if_can_receive_inbound_then_cant_change_sms_sender( + logged_in_client, + service_one, + mock_get_letter_organisations, +): + service_one['permissions'] = ['inbound_sms'] + service_one['sms_sender'] = 'SomeNumber' + response = logged_in_client.get(url_for( + 'main.service_settings', service_id=service_one['id'] + )) + assert 'Text message sender SomeNumber Change' not in response.get_data(as_text=True) + assert url_for('.service_set_sms_sender', service_id=service_one['id'], + set_inbound_sms=False) not in response.get_data(as_text=True) + assert 'SomeNumber' in response.get_data(as_text=True) + + def test_letter_contact_block_shows_none_if_not_set( logged_in_client, service_one, @@ -615,6 +631,30 @@ def test_set_text_message_sender( ) +def test_set_text_message_sender_and_inbound_sms( + logged_in_client, + service_one, + mock_get_letter_organisations, + mocker, +): + + update_service_mock = mocker.patch('app.service_api_client.update_service_with_properties', + return_value=service_one) + + data = {"sms_sender": "elevenchars"} + response = logged_in_client.post(url_for('main.service_set_sms_sender', service_id=service_one['id'], + set_inbound_sms=True), + data=data, + follow_redirects=True) + assert response.status_code == 200 + + update_service_mock.assert_called_with( + service_one['id'], + {'permissions': ['inbound_sms'], + 'sms_sender': "elevenchars"} + ) + + @pytest.mark.parametrize('content, expected_error', [ ("", "Can’t be empty"), ("twelvecharss", "Enter 11 characters or fewer"), diff --git a/tests/conftest.py b/tests/conftest.py index e9171cb8e..a59ce5382 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -167,7 +167,8 @@ def mock_update_service(mocker): 'restricted', 'email_from', 'reply_to_email_address', - 'sms_sender' + 'sms_sender', + 'permissions' ]} ) return {'data': service}