Make ‘Too many recipients’ error lower priority

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

@@ -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 %}
     <div class="bottom-gutter">
       {% call banner_wrapper(type='dangerous') %}
         <h1 class='banner-title'>
-          Too many recipients
+          You can’t send to
-        </h1>
+          {{ 'this' if count_of_recipients == 1 else 'these' }}
-        {% if statistics.emails_requested or statistics.sms_requested %}
+          {{ recipients.recipient_column_header }}
-          <p>
-            You can only send {{ current_service.message_limit }}
-            messages per day
-            {%- if current_service.restricted %}
-              in <a href="{{ url_for('.trial_mode')}}">trial mode</a>
-            {%- endif -%}
-            .
-          </p>
-        {% endif %}
-        <p>
-          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 -%}.
+          {%- endif %}
+        </h1>
+        <p>
+          In <a href="{{ url_for('.trial_mode') }}">trial mode</a> you can only
+          send to yourself and members of your team
         </p>
       {% endcall %}
     </div>
@@ -66,6 +55,34 @@
         {% endif %}
       {% endcall %}
     </div>
+  {% elif count_of_recipients > (current_service.message_limit - statistics.get('emails_requested', 0) - statistics.get('sms_requested', 0)) %}
+    <div class="bottom-gutter">
+      {% call banner_wrapper(type='dangerous') %}
+        <h1 class='banner-title'>
+          Too many recipients
+        </h1>
+        {% if statistics.emails_requested or statistics.sms_requested %}
+          <p>
+            You can only send {{ current_service.message_limit }}
+            messages per day
+            {%- if current_service.restricted %}
+              in <a href="{{ url_for('.trial_mode')}}">trial mode</a>
+            {%- endif -%}
+            .
+          </p>
+        {% endif %}
+        <p>
+          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 -%}.
+        </p>
+      {% endcall %}
+    </div>
   {% else %}
     <h1 class="heading-large">
       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 %}
       <a href="{{ back_link }}" class="page-footer-back-link">Back</a>

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

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(
-    mocker.patch('app.main.views.send.s3download', return_value=contents)
+        '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 '+447700900986' in content
-            assert '+44 456' in content
+            assert 'Missing' in content
-            assert 'Not a UK mobile number' 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,,,
+            +447700900986,,,,
-            123,test2,,,
+            +447700900986,,,,
         """
     )
     with app_.test_request_context():