From 4dcb180da19dcf6fd0704a2431f53c7ee78fa4f9 Mon Sep 17 00:00:00 2001 From: Adam Shimali Date: Wed, 13 Jan 2016 17:32:40 +0000 Subject: [PATCH] Changed page flow to first save file and then redirect to check. On check numbers in file are validated. Posting to check then uploads file to s3 --- app/main/uploader.py | 15 +++++ app/main/views/jobs.py | 14 ++++- app/main/views/sms.py | 90 ++++++++++++++++++++---------- app/templates/views/check-sms.html | 2 + config.py | 1 + tests/app/main/views/test_jobs.py | 9 ++- tests/app/main/views/test_sms.py | 13 ++++- 7 files changed, 106 insertions(+), 38 deletions(-) create mode 100644 app/main/uploader.py 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..f8fb73270 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, @@ -12,9 +15,11 @@ from flask import ( ) 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 = [ @@ -39,26 +44,27 @@ message_templates = [ @login_required 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 + if form.validate_on_submit(): 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,59 @@ 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') + 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('.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 _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']}) + with open(csv_file) as f: + reader = csv.DictReader( + f, + 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} 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/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..6dd5f08ba 100644 --- a/tests/app/main/views/test_sms.py +++ b/tests/app/main/views/test_sms.py @@ -2,15 +2,22 @@ from io import BytesIO 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): 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)