diff --git a/app/main/views/send.py b/app/main/views/send.py index 8e62e4137..36a43b9c0 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -22,6 +22,7 @@ from notifications_utils.recipients import ( from orderedset import OrderedSet from werkzeug.routing import RequestRedirect from xlrd.biffh import XLRDError +from xlrd.xldate import XLDateError from app import ( current_service, @@ -137,6 +138,13 @@ def send_messages(service_id, template_id): flash('Couldn’t read {}. Try using a different file format.'.format( form.file.data.filename )) + except (XLDateError): + flash(( + '{} contains numbers or dates that Notify can’t understand. ' + 'Try formatting all columns as ‘text’ or export your file as CSV.' + ).format( + form.file.data.filename + )) column_headings = first_column_headings[template.template_type] + list(template.placeholders) diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index 4f94b3b32..3554a115d 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -5,6 +5,7 @@ from glob import glob from io import BytesIO from itertools import repeat from os import path +from zipfile import BadZipFile import pytest from bs4 import BeautifulSoup @@ -37,6 +38,13 @@ from tests.conftest import ( no_sms_senders, normalize_spaces, ) +from xlrd.biffh import XLRDError +from xlrd.xldate import ( + XLDateAmbiguous, + XLDateError, + XLDateNegative, + XLDateTooLarge, +) template_types = ['email', 'sms'] @@ -278,6 +286,7 @@ def test_upload_files_in_different_formats( data={'file': (BytesIO(uploaded.read()), filename)}, content_type='multipart/form-data' ) + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') if acceptable_file: assert mock_s3_upload.call_args[0][1]['data'].strip() == ( @@ -288,9 +297,64 @@ def test_upload_files_in_different_formats( ) else: assert not mock_s3_upload.called - assert ( + assert normalize_spaces(page.select_one('.banner-dangerous').text) == ( 'Couldn’t read {}. Try using a different file format.'.format(filename) - ) in response.get_data(as_text=True) + ) + + +@pytest.mark.parametrize('exception, expected_error_message', [ + (partial(UnicodeDecodeError, 'codec', b'', 1, 2, 'reason'), ( + 'Couldn’t read example.xlsx. Try using a different file format.' + )), + (BadZipFile, ( + 'Couldn’t read example.xlsx. Try using a different file format.' + )), + (XLRDError, ( + 'Couldn’t read example.xlsx. Try using a different file format.' + )), + (XLDateError, ( + 'example.xlsx contains numbers or dates that Notify can’t understand. ' + 'Try formatting all columns as ‘text’ or export your file as CSV.' + )), + (XLDateNegative, ( + 'example.xlsx contains numbers or dates that Notify can’t understand. ' + 'Try formatting all columns as ‘text’ or export your file as CSV.' + )), + (XLDateAmbiguous, ( + 'example.xlsx contains numbers or dates that Notify can’t understand. ' + 'Try formatting all columns as ‘text’ or export your file as CSV.' + )), + (XLDateTooLarge, ( + 'example.xlsx contains numbers or dates that Notify can’t understand. ' + 'Try formatting all columns as ‘text’ or export your file as CSV.' + )), +]) +def test_shows_error_if_parsing_exception( + logged_in_client, + mocker, + mock_get_service_template, + exception, + expected_error_message, +): + + def _raise_exception_or_partial_exception(file_content, filename): + raise exception() + + mocker.patch( + 'app.main.views.send.Spreadsheet.from_file', + side_effect=_raise_exception_or_partial_exception + ) + + response = logged_in_client.post( + url_for('main.send_messages', service_id=SERVICE_ONE_ID, template_id=fake_uuid), + data={'file': (BytesIO(b'example'), 'example.xlsx')}, + content_type='multipart/form-data' + ) + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + + assert normalize_spaces(page.select_one('.banner-dangerous').text) == ( + expected_error_message + ) def test_upload_csvfile_with_errors_shows_check_page_with_errors(