From 7de42a31e637e0fd94733ba237c194e700f018f0 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 17 May 2016 09:23:33 +0100 Subject: [PATCH] =?UTF-8?q?Make=20=E2=80=98Too=20many=20recipients?= =?UTF-8?q?=E2=80=99=20error=20lower=20priority?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit makes two main changes to what happens when a user is in trial mode and they upload some email addresses belonging to other people. 1. Add a specific banner error telling the user about trial mode 2. Make this error higher priority, eg it will show up before the error about having too many recipients in your file This means making some changes to the tests so that the example CSV files include the user’s phone number, then making them invalid by omitting data required by the templates. Depends on: https://github.com/alphagov/notifications-utils/pull/34 --- app/templates/views/check.html | 59 ++++++++++++++++++++----------- requirements.txt | 2 +- tests/app/main/views/test_send.py | 27 ++++++++------ 3 files changed, 55 insertions(+), 33 deletions(-) diff --git a/app/templates/views/check.html b/app/templates/views/check.html index 3761f2642..021fe348e 100644 --- a/app/templates/views/check.html +++ b/app/templates/views/check.html @@ -13,31 +13,20 @@ {% block maincolumn_content %} - {% if count_of_recipients > (current_service.message_limit - statistics.get('emails_requested', 0) - statistics.get('sms_requested', 0)) %} + {% if not recipients.allowed_to_send_to and not recipients.missing_column_headers %}
{% 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.emails_requested - statistics.sms_requested }} - messages today, but - ‘{{ original_file_name }}’ contains - {{ count_of_recipients }} {{ recipients.recipient_column_header }} + You can’t send to + {{ 'this' if count_of_recipients == 1 else 'these' }} + {{ recipients.recipient_column_header }} {%- if count_of_recipients != 1 -%} {{ 'es' if 'email address' == recipients.recipient_column_header else 's' }} - {%- endif -%}. + {%- endif %} + +

+ In trial mode you can only + send to yourself and members of your team

{% endcall %}
@@ -66,6 +55,34 @@ {% endif %} {% endcall %} + {% 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.emails_requested - statistics.sms_requested }} + 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 %} +
{% else %}

Check and confirm @@ -95,7 +112,7 @@ {% if ( errors or - count_of_recipients > (current_service.message_limit - statistics.get('emails_requested', 0) - statistics.get('sms_requested', 0)) + count_of_recipients > (current_service.message_limit - statistics.get('emails_requested', 0) - statistics.get('sms_requested', 0)) ) %} {% if request.args.from_test %} Back diff --git a/requirements.txt b/requirements.txt index 6ef9ed861..acc96b2f0 100644 --- a/requirements.txt +++ b/requirements.txt @@ -18,4 +18,4 @@ pytz==2016.4 git+https://github.com/alphagov/notifications-python-client.git@1.0.0#egg=notifications-python-client==1.0.0 -git+https://github.com/alphagov/notifications-utils.git@5.2.5#egg=notifications-utils==5.2.5 +git+https://github.com/alphagov/notifications-utils.git@5.3.0#egg=notifications-utils==5.3.0 diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index 61fad0881..0141a3803 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -72,7 +72,7 @@ def test_upload_csvfile_with_errors_shows_check_page_with_errors( mocker, mock_login, mock_get_service, - mock_get_service_template, + mock_get_service_template_with_placeholders, mock_s3_upload, mock_has_permissions, mock_get_users_by_service, @@ -80,21 +80,27 @@ def test_upload_csvfile_with_errors_shows_check_page_with_errors( fake_uuid ): - contents = u'phone number,name\n+44 123,test1\n+44 456,test2' - mocker.patch('app.main.views.send.s3download', return_value=contents) + mocker.patch( + 'app.main.views.send.s3download', + return_value=""" + phone number,name + +447700900986 + +447700900986 + """ + ) with app_.test_request_context(): with app_.test_client() as client: client.login(api_user_active) initial_upload = client.post( url_for('main.send_messages', service_id=fake_uuid, template_id=fake_uuid), - data={'file': (BytesIO(contents.encode('utf-8')), 'invalid.csv')}, + data={'file': (BytesIO(''.encode('utf-8')), 'invalid.csv')}, content_type='multipart/form-data', follow_redirects=True ) reupload = client.post( url_for('main.check_messages', service_id=fake_uuid, template_type='sms', upload_id='abc123'), - data={'file': (BytesIO(contents.encode('utf-8')), 'invalid.csv')}, + data={'file': (BytesIO(''.encode('utf-8')), 'invalid.csv')}, content_type='multipart/form-data', follow_redirects=True ) @@ -102,9 +108,8 @@ def test_upload_csvfile_with_errors_shows_check_page_with_errors( assert response.status_code == 200 content = response.get_data(as_text=True) assert 'There was a problem with invalid.csv' in content - assert '+44 123' in content - assert '+44 456' in content - assert 'Not a UK mobile number' in content + assert '+447700900986' in content + assert 'Missing' in content assert 'Re-upload your file' in content @@ -354,7 +359,7 @@ def test_check_messages_should_revalidate_file_when_uploading_file( active_user_with_permissions, job_data, mock_create_job, - mock_get_service_template, + mock_get_service_template_with_placeholders, mock_s3_upload, mocker, mock_has_permissions, @@ -368,8 +373,8 @@ def test_check_messages_should_revalidate_file_when_uploading_file( 'app.main.views.send.s3download', return_value=""" phone number,name,,, - 123,test1,,, - 123,test2,,, + +447700900986,,,, + +447700900986,,,, """ ) with app_.test_request_context():