diff --git a/app/main/uploader.py b/app/main/uploader.py new file mode 100644 index 000000000..bedf4ba6d --- /dev/null +++ b/app/main/uploader.py @@ -0,0 +1,15 @@ +import os +import uuid + +from boto3 import resource + + +# TODO add service name to bucket name as well +def s3upload(filepath): + filename = filepath.split(os.path.sep)[-1] + upload_id = str(uuid.uuid4()) + s3 = resource('s3') + s3.create_bucket(Bucket=upload_id) + key = s3.Object(upload_id, filename) + key.put(Body=open(filepath, 'rb'), ServerSideEncryption='AES256') + return upload_id diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index 29755f3e1..b5675ead4 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -1,11 +1,14 @@ # -*- coding: utf-8 -*- import time -from flask import render_template + +from flask import ( + render_template, + session +) from flask_login import login_required from app.main import main - from ._jobs import jobs now = time.strftime('%H:%M') @@ -55,6 +58,11 @@ def showjobs(service_id): @main.route("/services//jobs/") @login_required def showjob(service_id, job_id): + + # TODO the uploaded file name could be part of job definition + # so won't need to be passed on from last view via session + uploaded_file_name = session.get(job_id) + return render_template( 'views/job.html', messages=messages, @@ -68,7 +76,7 @@ def showjob(service_id, job_id): ]) }, cost=u'£0.00', - uploaded_file_name='dispatch_20151114.csv', + uploaded_file_name=uploaded_file_name, uploaded_file_time=now, template_used='Test message 1', flash_message=u'We’ve started sending your messages', diff --git a/app/main/views/sms.py b/app/main/views/sms.py index 0ef985558..d5ca29981 100644 --- a/app/main/views/sms.py +++ b/app/main/views/sms.py @@ -1,5 +1,8 @@ import csv import re +import os + +from datetime import date from flask import ( request, @@ -8,13 +11,16 @@ from flask import ( url_for, session, flash, - current_app + current_app, + abort ) from flask_login import login_required +from werkzeug import secure_filename from app.main import main from app.main.forms import CsvUploadForm +from app.main.uploader import s3upload # TODO move this to the templates directory message_templates = [ @@ -40,25 +46,25 @@ message_templates = [ def sendsms(service_id): form = CsvUploadForm() if form.validate_on_submit(): - csv_file = form.file.data - - # in memory handing is temporary until next story to save csv file try: - results = _build_upload_result(csv_file) - except Exception: - message = 'There was a problem with the file: {}'.format( + csv_file = form.file.data + filename = _format_filename(csv_file.filename) + filepath = os.path.join(current_app.config['UPLOAD_FOLDER'], + filename) + csv_file.save(filepath) + _check_file(csv_file.filename, filepath) + return redirect(url_for('.checksms', + service_id=service_id, + recipients=filename)) + except (IOError, ValueError) as e: + message = 'There was a problem uploading: {}'.format( csv_file.filename) flash(message) + if isinstance(e, ValueError): + flash(str(e)) + os.remove(filepath) return redirect(url_for('.sendsms', service_id=service_id)) - if not results['valid'] and not results['rejects']: - message = "The file {} contained no data".format(csv_file.filename) - flash(message, 'error') - return redirect(url_for('.sendsms', service_id=service_id)) - - session[csv_file.filename] = results - return redirect(url_for('.checksms', service_id=service_id, recipients=csv_file.filename)) - return render_template('views/send-sms.html', message_templates=message_templates, form=form, @@ -69,33 +75,68 @@ def sendsms(service_id): @login_required def checksms(service_id): if request.method == 'GET': - - recipients_filename = request.args.get('recipients') - # upload results in session until file is persisted in next story - upload_result = session.get(recipients_filename) + filename = request.args.get('recipients') + if not filename: + abort(400) + filepath = os.path.join(current_app.config['UPLOAD_FOLDER'], + filename) + upload_result = _build_upload_result(filepath) if upload_result.get('rejects'): flash('There was a problem with some of the numbers') return render_template( 'views/check-sms.html', upload_result=upload_result, + filename=filename, message_template=message_templates[0]['body'], service_id=service_id ) elif request.method == 'POST': - return redirect(url_for('.showjob', service_id=service_id, job_id=456)) + filename = request.form['recipients'] + filepath = os.path.join(current_app.config['UPLOAD_FOLDER'], + filename) + try: + upload_id = s3upload(filepath) + # 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('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)) + + +def _check_file(filename, filepath): + if os.stat(filepath).st_size == 0: + message = 'The file {} contained no data'.format(filename) + raise ValueError(message) + + +def _format_filename(filename): + d = date.today() + basename, extenstion = filename.split('.') + formatted_name = '{}_{}.csv'.format(basename, d.strftime('%Y%m%d')) + return secure_filename(formatted_name) + + +def _open(file): + return open(file, 'r') def _build_upload_result(csv_file): - pattern = re.compile(r'^\+44\s?\d{4}\s?\d{6}$') - reader = csv.DictReader( - csv_file.read().decode('utf-8').splitlines(), - lineterminator='\n', - quoting=csv.QUOTE_NONE) - valid, rejects = [], [] - for i, row in enumerate(reader): - if pattern.match(row['phone']): - valid.append(row) - else: - rejects.append({"line_number": i+2, "phone": row['phone']}) - return {"valid": valid, "rejects": rejects} + try: + file = _open(csv_file, 'r') + pattern = re.compile(r'^\+44\s?\d{4}\s?\d{6}$') + reader = csv.DictReader( + file.read().splitlines(), + lineterminator='\n', + quoting=csv.QUOTE_NONE) + valid, rejects = [], [] + for i, row in enumerate(reader): + if pattern.match(row['phone']): + valid.append(row) + else: + rejects.append({"line_number": i+2, "phone": row['phone']}) + return {"valid": valid, "rejects": rejects} + finally: + file.close() diff --git a/app/templates/views/check-sms.html b/app/templates/views/check-sms.html index b7a564b11..79d924f08 100644 --- a/app/templates/views/check-sms.html +++ b/app/templates/views/check-sms.html @@ -59,6 +59,8 @@ back_link = url_for(".sendsms", service_id=service_id) )}} + + {% endif %} {% endblock %} diff --git a/config.py b/config.py index 80825d4cd..b0f596963 100644 --- a/config.py +++ b/config.py @@ -32,6 +32,7 @@ class Config(object): TOKEN_MAX_AGE_SECONDS = 3600 MAX_CONTENT_LENGTH = 10 * 1024 * 1024 # 10mb + UPLOAD_FOLDER = '/tmp' class Development(Config): diff --git a/requirements.txt b/requirements.txt index f2124af1e..3e87a8d90 100644 --- a/requirements.txt +++ b/requirements.txt @@ -9,6 +9,7 @@ Flask-WTF==0.11 Flask-Login==0.2.11 Flask-Bcrypt==0.6.2 credstash==1.8.0 +boto3==1.2.3 git+https://github.com/alphagov/notify-api-client.git@0.1.4#egg=notify-api-client==0.1.4 diff --git a/tests/app/main/views/test_jobs.py b/tests/app/main/views/test_jobs.py index cb03cf7ee..bb21c5a4b 100644 --- a/tests/app/main/views/test_jobs.py +++ b/tests/app/main/views/test_jobs.py @@ -16,9 +16,12 @@ def test_should_return_list_of_all_jobs(notifications_admin, notifications_admin def test_should_show_page_for_one_job(notifications_admin, notifications_admin_db, notify_db_session): with notifications_admin.test_request_context(): with notifications_admin.test_client() as client: - user = create_test_user('active') - client.login(user) - response = client.get('/services/123/jobs/456') + # TODO filename will be part of job metadata not in session + with client.session_transaction() as s: + s[456] = 'dispatch_20151114.csv' + user = create_test_user('active') + client.login(user) + response = client.get('/services/123/jobs/456') assert response.status_code == 200 assert 'dispatch_20151114.csv' in response.get_data(as_text=True) diff --git a/tests/app/main/views/test_sms.py b/tests/app/main/views/test_sms.py index c74356f5f..e2d84b701 100644 --- a/tests/app/main/views/test_sms.py +++ b/tests/app/main/views/test_sms.py @@ -1,16 +1,22 @@ from io import BytesIO +from unittest import mock +from unittest.mock import mock_open + from tests.app.main import create_test_user -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') client.login(user) - upload_data = {'file': (BytesIO(''.encode('utf-8')), 'emtpy.csv')} - response = client.post('/services/123/sms/send', data=upload_data, - follow_redirects=True) + 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) @@ -18,19 +24,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 @@ -39,17 +52,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) @@ -69,18 +89,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) @@ -96,13 +122,33 @@ 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