diff --git a/app/main/views/sms.py b/app/main/views/sms.py index f8fb73270..ac5e6cf67 100644 --- a/app/main/views/sms.py +++ b/app/main/views/sms.py @@ -44,7 +44,6 @@ message_templates = [ @login_required def sendsms(service_id): form = CsvUploadForm() - if form.validate_on_submit(): try: csv_file = form.file.data @@ -98,7 +97,7 @@ def checksms(service_id): # TODO when job is created record filename in job itself # so downstream pages can get the original filename that way session[upload_id] = filename - return redirect(url_for('.showjob', service_id=service_id, job_id=upload_id)) + return redirect(url_for('main.showjob', service_id=service_id, job_id=upload_id)) except: flash('There as a problem saving the file') return redirect(url_for('.checksms', recipients=filename)) @@ -118,10 +117,11 @@ def _format_filename(filename): def _build_upload_result(csv_file): - pattern = re.compile(r'^\+44\s?\d{4}\s?\d{6}$') - with open(csv_file) as f: + try: + file = open(csv_file, 'r') + pattern = re.compile(r'^\+44\s?\d{4}\s?\d{6}$') reader = csv.DictReader( - f, + file.read().splitlines(), lineterminator='\n', quoting=csv.QUOTE_NONE) valid, rejects = [], [] @@ -130,4 +130,6 @@ def _build_upload_result(csv_file): valid.append(row) else: rejects.append({"line_number": i+2, "phone": row['phone']}) - return {"valid": valid, "rejects": rejects} + return {"valid": valid, "rejects": rejects} + finally: + file.close() diff --git a/tests/app/main/views/test_sms.py b/tests/app/main/views/test_sms.py index 6dd5f08ba..916ac022e 100644 --- a/tests/app/main/views/test_sms.py +++ b/tests/app/main/views/test_sms.py @@ -1,16 +1,12 @@ from io import BytesIO +from unittest import mock +from unittest.mock import mock_open + from tests.app.main import create_test_user -def teardown_function(function): - import glob - import os - files = glob.glob("/tmp/*.csv") - for f in files: - os.remove(f) - - -def test_upload_empty_csvfile_returns_to_upload_page(notifications_admin, notifications_admin_db, notify_db_session): +def test_upload_empty_csvfile_returns_to_upload_page(notifications_admin, notifications_admin_db, notify_db_session, mocker): + _setup_mocker_for_empty_file(mocker) with notifications_admin.test_request_context(): with notifications_admin.test_client() as client: user = create_test_user('active') @@ -25,19 +21,26 @@ def test_upload_empty_csvfile_returns_to_upload_page(notifications_admin, notifi def test_upload_csvfile_with_invalid_phone_shows_check_page_with_errors( - notifications_admin, notifications_admin_db, notify_db_session): + notifications_admin, notifications_admin_db, notify_db_session, + mocker): + + contents = 'phone\n+44 123\n+44 456' + file_data = (BytesIO(contents.encode('utf-8')), 'invalid.csv') + m_open = mock_open(read_data=contents) + _setup_mocker_for_nonemtpy_file(mocker) with notifications_admin.test_request_context(): with notifications_admin.test_client() as client: user = create_test_user('active') client.login(user) - - file_contents = 'phone\n+44 123\n+44 456'.encode('utf-8') - upload_data = {'file': (BytesIO(file_contents), 'invalid.csv')} - response = client.post('/services/123/sms/send', data=upload_data, - follow_redirects=True) + upload_data = {'file': file_data} + with mock.patch('app.main.views.sms.open', m_open): + response = client.post('/services/123/sms/send', + data=upload_data, + follow_redirects=True) assert response.status_code == 200 content = response.get_data(as_text=True) + assert 'There was a problem with some of the numbers' in content assert 'The following numbers are invalid' in content assert '+44 123' in content @@ -46,17 +49,24 @@ def test_upload_csvfile_with_invalid_phone_shows_check_page_with_errors( def test_upload_csvfile_with_valid_phone_shows_first3_and_last3_numbers( - notifications_admin, notifications_admin_db, notify_db_session): + notifications_admin, notifications_admin_db, notify_db_session, + mocker): + + contents = 'phone\n+44 7700 900981\n+44 7700 900982\n+44 7700 900983\n+44 7700 900984\n+44 7700 900985\n+44 7700 900986\n+44 7700 900987\n+44 7700 900988\n+44 7700 900989' # noqa + + file_data = (BytesIO(contents.encode('utf-8')), 'valid.csv') + m_open = mock_open(read_data=contents) + _setup_mocker_for_nonemtpy_file(mocker) with notifications_admin.test_request_context(): with notifications_admin.test_client() as client: user = create_test_user('active') client.login(user) - file_contents = 'phone\n+44 7700 900981\n+44 7700 900982\n+44 7700 900983\n+44 7700 900984\n+44 7700 900985\n+44 7700 900986\n+44 7700 900987\n+44 7700 900988\n+44 7700 900989'.encode('utf-8') # noqa - - upload_data = {'file': (BytesIO(file_contents), 'valid.csv')} - response = client.post('/services/123/sms/send', data=upload_data, - follow_redirects=True) + upload_data = {'file': file_data} + with mock.patch('app.main.views.sms.open', m_open): + response = client.post('/services/123/sms/send', + data=upload_data, + follow_redirects=True) content = response.get_data(as_text=True) @@ -76,18 +86,24 @@ def test_upload_csvfile_with_valid_phone_shows_first3_and_last3_numbers( def test_upload_csvfile_with_valid_phone_shows_all_if_6_or_less_numbers( - notifications_admin, notifications_admin_db, notify_db_session): + notifications_admin, notifications_admin_db, notify_db_session, + mocker): + + contents = 'phone\n+44 7700 900981\n+44 7700 900982\n+44 7700 900983\n+44 7700 900984\n+44 7700 900985\n+44 7700 900986' # noqa + + file_data = (BytesIO(contents.encode('utf-8')), 'valid.csv') + m_open = mock_open(read_data=contents) + _setup_mocker_for_nonemtpy_file(mocker) with notifications_admin.test_request_context(): with notifications_admin.test_client() as client: user = create_test_user('active') client.login(user) - - file_contents = 'phone\n+44 7700 900981\n+44 7700 900982\n+44 7700 900983\n+44 7700 900984\n+44 7700 900985\n+44 7700 900986'.encode('utf-8') # noqa - - upload_data = {'file': (BytesIO(file_contents), 'valid.csv')} - response = client.post('/services/123/sms/send', data=upload_data, - follow_redirects=True) + upload_data = {'file': file_data} + with mock.patch('app.main.views.sms.open', m_open): + response = client.post('/services/123/sms/send', + data=upload_data, + follow_redirects=True) content = response.get_data(as_text=True) @@ -103,13 +119,34 @@ def test_upload_csvfile_with_valid_phone_shows_all_if_6_or_less_numbers( def test_should_redirect_to_job(notifications_admin, notifications_admin_db, - notify_db_session): + notify_db_session, mocker): + _setup_mocker_for_check(mocker) with notifications_admin.test_request_context(): with notifications_admin.test_client() as client: user = create_test_user('active') client.login(user) + with client.session_transaction() as s: + s[456] = 'test.csv' - response = client.post('/services/123/sms/check') + response = client.post('/services/123/sms/check', + data={'recipients': 'test.csv'}) assert response.status_code == 302 - assert response.location == 'http://localhost/services/123/jobs/456' + + +def _setup_mocker_for_empty_file(mocker): + mocker.patch('werkzeug.datastructures.FileStorage.save') + mocker.patch('os.remove') + ret = ValueError('The file emtpy.csv contained no data') + mocker.patch('app.main.views.sms._check_file', side_effect=ret) + + +def _setup_mocker_for_nonemtpy_file(mocker): + mocker.patch('werkzeug.datastructures.FileStorage.save') + mocker.patch('os.remove') + mocker.patch('app.main.views.sms._check_file') + + +def _setup_mocker_for_check(mocker): + mocker.patch('app.main.views.sms.s3upload').return_value = 456 +