From 20eb08a12220e10b954a991db19cdf942f16fff0 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 21 Jul 2016 13:47:18 +0100 Subject: [PATCH 1/3] move too many messages error to its own partial --- .../partials/check/too_many_messages.html | 27 +++++++++++++++++ app/templates/views/check.html | 30 +------------------ 2 files changed, 28 insertions(+), 29 deletions(-) create mode 100644 app/templates/partials/check/too_many_messages.html diff --git a/app/templates/partials/check/too_many_messages.html b/app/templates/partials/check/too_many_messages.html new file mode 100644 index 000000000..66ba82f06 --- /dev/null +++ b/app/templates/partials/check/too_many_messages.html @@ -0,0 +1,27 @@ +
+ {% call banner_wrapper(type='dangerous') %} +

+ Too many recipients +

+ {% if statistics.emails_requested or statistics.sms_requested %} +

+ You can only send {{ current_service.message_limit }} + messages per day + {%- if current_service.restricted %} + in trial mode + {%- endif -%} + . +

+ {% endif %} +

+ You can still send + {{ current_service.message_limit - statistics.get('emails_requested', 0) - statistics.get('sms_requested', 0) }} + messages today, but + ‘{{ original_file_name }}’ contains + {{ count_of_recipients }} {{ recipients.recipient_column_header }} + {%- if count_of_recipients != 1 -%} + {{ 'es' if 'email address' == recipients.recipient_column_header else 's' }} + {%- endif %} +

+ {% endcall %} +
diff --git a/app/templates/views/check.html b/app/templates/views/check.html index 5c5c4db5b..8fe0d01f7 100644 --- a/app/templates/views/check.html +++ b/app/templates/views/check.html @@ -104,35 +104,7 @@ {% elif count_of_recipients > (current_service.message_limit - statistics.get('emails_requested', 0) - statistics.get('sms_requested', 0)) %} - -
- {% call banner_wrapper(type='dangerous') %} -

- Too many recipients -

- {% if statistics.emails_requested or statistics.sms_requested %} -

- You can only send {{ current_service.message_limit }} - messages per day - {%- if current_service.restricted %} - in trial mode - {%- endif -%} - . -

- {% endif %} -

- You can still send - {{ current_service.message_limit - statistics.get('emails_requested', 0) - statistics.get('sms_requested', 0) }} - messages today, but - ‘{{ original_file_name }}’ contains - {{ count_of_recipients }} {{ recipients.recipient_column_header }} - {%- if count_of_recipients != 1 -%} - {{ 'es' if 'email address' == recipients.recipient_column_header else 's' }} - {%- endif %} -

- {% endcall %} -
- + {% include "partials/check/too_many_messages.html" %} {% else %}

From afce715a3d59b76392093b33ffeebe08817d42c4 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 21 Jul 2016 13:48:10 +0100 Subject: [PATCH 2/3] fix issues with too many messages partial not branching correctly branch for "has service sent anything today" was around the intial paragraph, rather than around the "you can still send..." bit - they should always see the first paragraph, especially the bit that points out if they're in trial mode. They don't need to see how many messages they have remaining today if it's the same amount as their daily limit. --- .../partials/check/too_many_messages.html | 27 +++++++++---------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/app/templates/partials/check/too_many_messages.html b/app/templates/partials/check/too_many_messages.html index 66ba82f06..457c7321e 100644 --- a/app/templates/partials/check/too_many_messages.html +++ b/app/templates/partials/check/too_many_messages.html @@ -3,25 +3,24 @@

Too many recipients

- {% if statistics.emails_requested or statistics.sms_requested %} -

- You can only send {{ current_service.message_limit }} - messages per day - {%- if current_service.restricted %} - in trial mode - {%- endif -%} - . -

- {% endif %}

- You can still send - {{ current_service.message_limit - statistics.get('emails_requested', 0) - statistics.get('sms_requested', 0) }} - messages today, but + You can only send {{ current_service.message_limit }} messages per day + {%- if current_service.restricted %} + in trial mode + {%- endif -%} + . +

+

+ {% if statistics.emails_requested or statistics.sms_requested %} + You can still send + {{ current_service.message_limit - statistics.get('emails_requested', 0) - statistics.get('sms_requested', 0) }} + messages today, but + {% endif %} ‘{{ original_file_name }}’ contains {{ count_of_recipients }} {{ recipients.recipient_column_header }} {%- if count_of_recipients != 1 -%} {{ 'es' if 'email address' == recipients.recipient_column_header else 's' }} - {%- endif %} + {%- endif %}.

{% endcall %} From d6e6d05893157582a4446a035bb9825879c83a32 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 21 Jul 2016 15:39:00 +0100 Subject: [PATCH 3/3] add tests for send check error messages --- tests/app/main/views/test_send.py | 51 +++++++++++++++++++++++++++++++ tests/conftest.py | 2 +- 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index 44062872e..026dc55cf 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -719,3 +719,54 @@ def test_go_to_dashboard_after_tour( assert resp.status_code == 302 assert resp.location == url_for("main.service_dashboard", service_id=fake_uuid, _external=True) mock_delete_service_template.assert_called_once_with(fake_uuid, fake_uuid) + + +@pytest.mark.parametrize('num_already_sent,msg', [ + (0, '‘valid.csv’ contains 100 phone numbers.'), + (1, 'You can still send 49 messages today, but ‘valid.csv’ contains 100 phone numbers.') +], ids=['none_sent', 'some_sent']) +def test_check_messages_shows_too_many_messages_errors( + mocker, + app_, + api_user_active, + mock_login, + mock_get_users_by_service, + mock_get_service, + mock_get_service_template, + mock_has_permissions, + fake_uuid, + num_already_sent, + msg +): + # csv with 100 phone numbers + mocker.patch('app.main.views.send.s3download', return_value=',\n'.join( + ['phone number'] + ([mock_get_users_by_service(None)[0]._mobile_number]*100) + )) + mocker.patch('app.statistics_api_client.get_statistics_for_service_for_day', return_value={ + 'day': datetime.today().date().strftime('%Y-%m-%d'), + 'sms_requested': num_already_sent, 'sms_delivered': num_already_sent, 'sms_failed': 0, + 'emails_requested': 0, 'emails_delivered': 0, 'emails_failed': 0, 'service': fake_uuid, 'id': fake_uuid + }) + + with app_.test_request_context(), app_.test_client() as client: + client.login(api_user_active) + with client.session_transaction() as session: + session['upload_data'] = {'original_file_name': 'valid.csv', + 'template_id': fake_uuid, + 'notification_count': 1, + 'valid': True} + response = client.get(url_for( + 'main.check_messages', + service_id=fake_uuid, + template_type='sms', + upload_id=fake_uuid + )) + assert response.status_code == 200 + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + assert page.find('h1').text.strip() == 'Too many recipients' + assert page.find('div', class_='banner-dangerous').find('a').text.strip() == 'trial mode' + + # remove excess whitespace from element + details = page.find('div', class_='banner-dangerous').findAll('p')[1] + details = ' '.join([line.strip() for line in details.text.split('\n') if line.strip() != '']) + assert details == msg diff --git a/tests/conftest.py b/tests/conftest.py index bc7432c8c..a66b50e43 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -57,7 +57,7 @@ def fake_uuid(): def mock_get_service(mocker, api_user_active): def _get(service_id): service = service_json( - service_id, "Test Service", [api_user_active.id], message_limit=1000, + service_id, "Test Service", [api_user_active.id], message_limit=50, active=False, restricted=True) return {'data': service}