From ed9b4362e731bdce1e386f9745c7ff68937e8169 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 24 Aug 2017 09:57:43 +0100 Subject: [PATCH 1/4] fix whitespace --- tests/app/main/views/test_service_settings.py | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 5c43eb6d5..07ebda338 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -1298,25 +1298,25 @@ def test_cant_resume_active_service( @pytest.mark.parametrize('endpoint, expected_p', [ ( - 'main.service_set_international_sms', - ( - 'Sending text messages to international phone numbers is ' - 'an invitation‑only feature.' - ) + 'main.service_set_international_sms', + ( + 'Sending text messages to international phone numbers is ' + 'an invitation‑only feature.' + ) ), ( - 'main.service_set_letters', - ( - 'Using GOV.UK Notify to send letters is an invitation‑only ' - 'feature.' - ) + 'main.service_set_letters', + ( + 'Using GOV.UK Notify to send letters is an invitation‑only ' + 'feature.' + ) ), ]) def test_invitation_pages( - logged_in_client, - service_one, - endpoint, - expected_p, + logged_in_client, + service_one, + endpoint, + expected_p, ): response = logged_in_client.get(url_for(endpoint, service_id=service_one['id'])) From 453ee44dfb839e3baa21882a6ff4b960690b9c6e Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 24 Aug 2017 10:02:02 +0100 Subject: [PATCH 2/4] Put the inbound number on the invite page Inbound number and SMS sender are divorced from each other now. This page was assuming that they were the same thing. --- app/main/views/service_settings.py | 1 + app/templates/views/service-settings/set-inbound-sms.html | 2 +- tests/app/main/views/test_service_settings.py | 8 ++++++++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 84b43ef1b..9bd587a93 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -394,6 +394,7 @@ def service_set_international_sms(service_id): def service_set_inbound_sms(service_id): return render_template( 'views/service-settings/set-inbound-sms.html', + inbound_number=inbound_number_client.get_inbound_sms_number_for_service(service_id)['data']['number'], ) diff --git a/app/templates/views/service-settings/set-inbound-sms.html b/app/templates/views/service-settings/set-inbound-sms.html index ffafb8847..6fef29a27 100644 --- a/app/templates/views/service-settings/set-inbound-sms.html +++ b/app/templates/views/service-settings/set-inbound-sms.html @@ -13,7 +13,7 @@

Receive text messages

{% if 'inbound_sms' in current_service.permissions %}

- Your service can receive text messages sent to {{ current_service.sms_sender }}. + Your service can receive text messages sent to {{ inbound_number }}.

If you want to turn this feature off, diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 07ebda338..8eda92c73 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -1311,13 +1311,21 @@ def test_cant_resume_active_service( 'feature.' ) ), + ( + 'main.service_set_inbound_sms', + ( + 'Your service can receive text messages sent to 0781239871.' + ) + ), ]) def test_invitation_pages( logged_in_client, service_one, endpoint, expected_p, + mock_get_inbound_number_for_service, ): + service_one['permissions'] = ['sms', 'email', 'inbound_sms'] response = logged_in_client.get(url_for(endpoint, service_id=service_one['id'])) assert response.status_code == 200 From 66e4129c54ff310ac91bfa39c958cfffc5745343 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 24 Aug 2017 10:13:14 +0100 Subject: [PATCH 3/4] Make tests more comprehensive --- tests/app/main/views/test_service_settings.py | 33 ++++++++++++++++--- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 8eda92c73..a278ad9cd 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -1296,23 +1296,47 @@ def test_cant_resume_active_service( assert 'Resume service' not in {a.text for a in page.find_all('a', class_='button')} -@pytest.mark.parametrize('endpoint, expected_p', [ +@pytest.mark.parametrize('endpoint, permissions, expected_p', [ ( 'main.service_set_international_sms', + ['sms'], ( 'Sending text messages to international phone numbers is ' 'an invitation‑only feature.' ) ), + ( + 'main.service_set_international_sms', + ['sms', 'international_sms'], + ( + 'Your service can send text messages to international phone numbers.' + ) + ), ( 'main.service_set_letters', + [], ( 'Using GOV.UK Notify to send letters is an invitation‑only ' 'feature.' ) ), + ( + 'main.service_set_letters', + ['letter'], + ( + 'Your service can send letters.' + ) + ), ( 'main.service_set_inbound_sms', + ['sms'], + ( + 'Receiving text messages from your users is an invitation‑only feature.' + ) + ), + ( + 'main.service_set_inbound_sms', + ['sms', 'inbound_sms'], ( 'Your service can receive text messages sent to 0781239871.' ) @@ -1321,11 +1345,12 @@ def test_cant_resume_active_service( def test_invitation_pages( logged_in_client, service_one, - endpoint, - expected_p, mock_get_inbound_number_for_service, + endpoint, + permissions, + expected_p, ): - service_one['permissions'] = ['sms', 'email', 'inbound_sms'] + service_one['permissions'] = permissions response = logged_in_client.get(url_for(endpoint, service_id=service_one['id'])) assert response.status_code == 200 From 7e097e887e70486fcfe72a69f54d0cd335214003 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 24 Aug 2017 13:57:12 +0100 Subject: [PATCH 4/4] Make sure empty inbox also show the correct number MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit …since it’s divorced from the SMS sender now. --- app/main/views/dashboard.py | 9 ++++++++- app/templates/views/dashboard/_inbox_messages.html | 2 +- tests/app/main/views/test_dashboard.py | 4 +++- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index 81c15790a..7f47b081a 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -18,7 +18,8 @@ from app import ( billing_api_client, job_api_client, service_api_client, - template_statistics_client + template_statistics_client, + inbound_number_client, ) from app.statistics_utils import get_formatted_percentage, add_rate_to_job from app.utils import ( @@ -175,11 +176,17 @@ def get_inbox_partials(service_id): }: messages_to_show.append(message) + if not inbound_messages: + inbound_number = inbound_number_client.get_inbound_sms_number_for_service(service_id)['data']['number'] + else: + inbound_number = None + return {'messages': render_template( 'views/dashboard/_inbox_messages.html', messages=messages_to_show, count_of_messages=len(inbound_messages), count_of_users=len(messages_to_show), + inbound_number=inbound_number, )} diff --git a/app/templates/views/dashboard/_inbox_messages.html b/app/templates/views/dashboard/_inbox_messages.html index 95af9ffab..b0ca019dc 100644 --- a/app/templates/views/dashboard/_inbox_messages.html +++ b/app/templates/views/dashboard/_inbox_messages.html @@ -6,7 +6,7 @@ messages, caption="Inbox", caption_visible=False, - empty_message='When users text your service’s phone number ({}) you’ll see the messages here'.format(current_service.sms_sender), + empty_message='When users text your service’s phone number ({}) you’ll see the messages here'.format(inbound_number), field_headings=[ 'From', 'First two lines of message' diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index 4a0074bd2..a9752e569 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -176,6 +176,7 @@ def test_empty_inbox( mock_get_template_statistics, mock_get_usage, mock_get_inbound_sms_with_no_messages, + mock_get_inbound_number_for_service, ): service_one['permissions'] = ['inbound_sms'] @@ -185,7 +186,7 @@ def test_empty_inbox( assert response.status_code == 200 assert normalize_spaces(page.select('tbody tr')) == ( - 'When users text your service’s phone number (GOVUK) you’ll see the messages here' + 'When users text your service’s phone number (0781239871) you’ll see the messages here' ) @@ -210,6 +211,7 @@ def test_anyone_can_see_inbox( service_one, mocker, mock_get_inbound_sms_with_no_messages, + mock_get_inbound_number_for_service, ): service_one['permissions'] = ['inbound_sms']