diff --git a/app/main/views/send.py b/app/main/views/send.py index ea55a50eb..b324f6b1e 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -543,6 +543,7 @@ def _check_messages(service_id, template_id, upload_id, preview_row, letters_as_ recipients = RecipientCSV( contents, template_type=template.template_type, + template=template, placeholders=template.placeholders, max_initial_rows_shown=50, max_errors_shown=50, diff --git a/app/templates/govuk_template.html b/app/templates/govuk_template.html new file mode 100644 index 000000000..c8ea57fe9 --- /dev/null +++ b/app/templates/govuk_template.html @@ -0,0 +1,109 @@ +{% block top_of_page %}{% endblock %} + + + + + + {% block page_title %}GOV.UK - The best place to find government services and information{% endblock %} + + + + + + + + + + + + + + + + + + + + + + + + {% block head %}{% endblock %} + + + + + + + + + {% block body_start %}{% endblock %} + + + + + + + + + + {% block after_header %}{% endblock %} + +
+ + {% block content %}{% endblock %} + + + + + + + + {% block body_end %}{% endblock %} + + + + + diff --git a/app/utils.py b/app/utils.py index 98254c291..930dc800d 100644 --- a/app/utils.py +++ b/app/utils.py @@ -131,6 +131,20 @@ def get_errors_for_csv(recipients, template_type): else: errors.append("enter missing data in {} rows".format(number_of_rows_with_missing_data)) + if any(recipients.rows_with_message_too_long): + number_of_rows_with_message_too_long = len(list(recipients.rows_with_message_too_long)) + if 1 == number_of_rows_with_message_too_long: + errors.append("shorten your message in 1 row") + else: + errors.append("shorten your message in {} rows".format(number_of_rows_with_missing_data)) + + if any(recipients.rows_with_empty_message): + number_of_rows_with_empty_message = len(list(recipients.rows_with_empty_message)) + if 1 == number_of_rows_with_empty_message: + errors.append("add content to empty message in 1 row") + else: + errors.append("add content to empty messages {} rows".format(number_of_rows_with_empty_message)) + return errors diff --git a/tests/app/main/test_errors_for_csv.py b/tests/app/main/test_errors_for_csv.py index 68f300905..67e6f6103 100644 --- a/tests/app/main/test_errors_for_csv.py +++ b/tests/app/main/test_errors_for_csv.py @@ -8,41 +8,44 @@ MockRecipients = namedtuple( 'RecipientCSV', [ 'rows_with_bad_recipients', - 'rows_with_missing_data' + 'rows_with_missing_data', + 'rows_with_message_too_long', + 'rows_with_empty_message' ] ) @pytest.mark.parametrize( - "rows_with_bad_recipients,rows_with_missing_data,template_type,expected_errors", + "rows_with_bad_recipients, rows_with_missing_data, " + "rows_with_message_too_long, rows_with_empty_message, template_type, expected_errors", [ ( - [], [], + [], [], [], [], 'sms', [] ), ( - {2}, [], + {2}, [], [], [], 'sms', ['fix 1 phone number'] ), ( - {2, 4, 6}, [], + {2, 4, 6}, [], [], [], 'sms', ['fix 3 phone numbers'] ), ( - {1}, [], + {1}, [], [], [], 'email', ['fix 1 email address'] ), ( - {2, 4, 6}, [], + {2, 4, 6}, [], [], [], 'email', ['fix 3 email addresses'] ), ( - {2}, {3}, + {2}, {3}, [], [], 'sms', [ 'fix 1 phone number', @@ -50,21 +53,35 @@ MockRecipients = namedtuple( ] ), ( - {2, 4, 6, 8}, {3, 6, 9, 12}, + {2, 4, 6, 8}, {3, 6, 9, 12}, [], [], 'sms', [ 'fix 4 phone numbers', 'enter missing data in 4 rows' ] - ) + ), + ( + {}, {}, {3}, [], + 'sms', + [ + 'shorten your message in 1 row' + ] + ), + ( + {}, {}, {}, {2}, + 'sms', + [ + 'add content to empty message in 1 row' + ] + ), ] ) def test_get_errors_for_csv( - rows_with_bad_recipients, rows_with_missing_data, + rows_with_bad_recipients, rows_with_missing_data, rows_with_message_too_long, rows_with_empty_message, template_type, expected_errors ): assert get_errors_for_csv( - MockRecipients(rows_with_bad_recipients, rows_with_missing_data), + MockRecipients(rows_with_bad_recipients, rows_with_missing_data, rows_with_message_too_long, rows_with_empty_message), template_type ) == expected_errors diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index 7dc257974..3f7a64172 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -420,6 +420,44 @@ def test_upload_csv_file_with_errors_shows_check_page_with_errors( assert 'Upload your file again' in content +def test_upload_csv_file_with_empty_message_shows_check_page_with_errors( + logged_in_client, + service_one, + mocker, + mock_get_empty_service_template_with_optional_placeholder, + mock_s3_upload, + mock_get_users_by_service, + mock_get_service_statistics, + mock_get_job_doesnt_exist, + mock_get_jobs, + fake_uuid, +): + + mocker.patch( + 'app.main.views.send.s3download', + return_value=""" + phone number, show_placeholder + +447700900986, yes + +447700900986, no + """ + ) + + response = logged_in_client.post( + url_for('main.send_messages', service_id=service_one['id'], template_id=fake_uuid), + data={'file': (BytesIO(''.encode('utf-8')), 'invalid.csv')}, + content_type='multipart/form-data', + follow_redirects=True + ) + + with logged_in_client.session_transaction() as session: + assert 'file_uploads' not in session + + assert response.status_code == 200 + content = response.get_data(as_text=True) + assert 'There is a problem with invalid.csv' in content + assert 'add content to empty message in 1 row' in content + + @pytest.mark.parametrize('file_contents, expected_error,', [ ( """ diff --git a/tests/conftest.py b/tests/conftest.py index b8937ca14..4c7eba310 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -850,6 +850,20 @@ def mock_get_service_template_with_placeholders(mocker): ) +@pytest.fixture(scope='function') +def mock_get_empty_service_template_with_optional_placeholder(mocker): + def _get(service_id, template_id, version=None): + template = template_json( + service_id, template_id, name="Optional content", content="((show_placeholder??Some content))" + ) + return {'data': template} + + return mocker.patch( + 'app.service_api_client.get_service_template', + side_effect=_get + ) + + @pytest.fixture(scope='function') def mock_get_service_template_with_multiple_placeholders(mocker): def _get(service_id, template_id, version=None):