diff --git a/app/main/uploader.py b/app/main/uploader.py index a55eab1ea..cc1f708c3 100644 --- a/app/main/uploader.py +++ b/app/main/uploader.py @@ -1,3 +1,4 @@ +import uuid import botocore from boto3 import resource from flask import current_app @@ -5,7 +6,7 @@ from flask import current_app FILE_LOCATION_STRUCTURE = 'service-{}-notify/{}.csv' -def s3upload(upload_id, service_id, filedata, region): +def s3upload(service_id, filedata, region): s3 = resource('s3') bucket_name = current_app.config['CSV_UPLOAD_BUCKET_NAME'] contents = filedata['data'] @@ -27,10 +28,13 @@ def s3upload(upload_id, service_id, filedata, region): s3.create_bucket(Bucket=bucket_name, CreateBucketConfiguration={'LocationConstraint': region}) + upload_id = str(uuid.uuid4()) upload_file_name = FILE_LOCATION_STRUCTURE.format(service_id, upload_id) key = s3.Object(bucket_name, upload_file_name) key.put(Body=contents, ServerSideEncryption='AES256') + return upload_id + def s3download(service_id, upload_id): contents = '' diff --git a/app/main/views/send.py b/app/main/views/send.py index e015021a0..c19081295 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -110,11 +110,9 @@ def send_messages(service_id, template_id): form = CsvUploadForm() if form.validate_on_submit(): try: - upload_id = str(uuid.uuid4()) - s3upload( - upload_id, + upload_id = s3upload( service_id, - Spreadsheet.from_file(form.file.data.filename, form.file.data).as_dict, + Spreadsheet.from_file(form.file.data, filename=form.file.data.filename).as_dict, current_app.config['AWS_REGION'] ) session['upload_data'] = { @@ -144,16 +142,13 @@ def send_messages(service_id, template_id): @user_has_permissions('send_texts', 'send_emails', 'send_letters', 'manage_templates', any_=True) def get_example_csv(service_id, template_id): template = Template(service_api_client.get_service_template(service_id, template_id)['data']) - with io.StringIO() as output: - writer = csv.writer(output) - writer.writerows([ - [first_column_heading[template.template_type]] + list(template.placeholders), - get_example_csv_rows(template) - ]) - return output.getvalue(), 200, { - 'Content-Type': 'text/csv; charset=utf-8', - 'Content-Disposition': 'inline; filename="{}.csv"'.format(template.name) - } + return Spreadsheet.from_rows([ + [first_column_heading[template.template_type]] + list(template.placeholders), + get_example_csv_rows(template) + ]).as_csv_data, 200, { + 'Content-Type': 'text/csv; charset=utf-8', + 'Content-Disposition': 'inline; filename="{}.csv"'.format(template.name) + } @main.route("/services//send//test", methods=['GET', 'POST']) @@ -161,32 +156,36 @@ def get_example_csv(service_id, template_id): @user_has_permissions('send_texts', 'send_emails', 'send_letters') def send_test(service_id, template_id): + file_name = 'Test message' + template = Template( service_api_client.get_service_template(service_id, template_id)['data'], prefix=current_service['name'] ) if len(template.placeholders) == 0 or request.method == 'POST': - with io.StringIO() as output: - writer = csv.writer(output) - writer.writerows([ - [first_column_heading[template.template_type]] + list(template.placeholders), - get_example_csv_rows(template, use_example_as_example=False, submitted_fields=request.form) - ]) - filedata = { - 'file_name': 'Test message', - 'data': output.getvalue() - } - upload_id = str(uuid.uuid4()) - s3upload(upload_id, service_id, filedata, current_app.config['AWS_REGION']) - session['upload_data'] = {"template_id": template_id, "original_file_name": filedata['file_name']} - return redirect(url_for( - '.check_messages', - upload_id=upload_id, - service_id=service_id, - template_type=template.template_type, - from_test=True - )) + upload_id = s3upload( + service_id, + { + 'file_name': file_name, + 'data': Spreadsheet.from_rows([ + [first_column_heading[template.template_type]] + list(template.placeholders), + get_example_csv_rows(template, use_example_as_example=False, submitted_fields=request.form) + ]).as_csv_data + }, + current_app.config['AWS_REGION'] + ) + session['upload_data'] = { + "template_id": template_id, + "original_file_name": file_name + } + return redirect(url_for( + '.check_messages', + upload_id=upload_id, + service_id=service_id, + template_type=template.template_type, + from_test=True + )) return render_template( 'views/send-test.html', diff --git a/app/utils.py b/app/utils.py index 36289aeff..38ec97806 100644 --- a/app/utils.py +++ b/app/utils.py @@ -143,7 +143,7 @@ class Spreadsheet(): allowed_file_extensions = ['csv', 'xlsx', 'xls', 'ods', 'xlsm', 'tsv'] - def __init__(self, filename, csv_data): + def __init__(self, csv_data, filename=''): self.filename = filename self.as_csv_data = csv_data self.as_dict = { @@ -164,24 +164,28 @@ class Spreadsheet(): return '\r\n'.join(file_content.getvalue().decode('utf-8').splitlines()) @classmethod - def from_file(cls, filename, file_content): + def from_rows(cls, rows, filename=''): + + with StringIO() as converted: + output = csv.writer(converted) + + for row in rows: + output.writerow(row) + + return cls(converted.getvalue(), filename) + + @classmethod + def from_file(cls, file_content, filename=''): extension = cls.get_extension(filename) if extension == 'csv': - return cls(filename, Spreadsheet.normalise_newlines(file_content)) + return cls(Spreadsheet.normalise_newlines(file_content), filename) if extension == 'tsv': file_content = StringIO(Spreadsheet.normalise_newlines(file_content)) - with StringIO() as converted: - - output = csv.writer(converted) - - for row in pyexcel.get_sheet( - file_type=extension, - file_content=file_content.getvalue() - ).to_array(): - output.writerow(row) - - return cls(filename, converted.getvalue()) + return cls.from_rows(pyexcel.get_sheet( + file_type=extension, + file_content=file_content.getvalue() + ).to_array(), filename) diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index 444e32247..61fad0881 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -53,7 +53,7 @@ def test_upload_files_in_different_formats( ) if acceptable_file: - assert mock_s3_upload.call_args[0][2]['data'].strip() == ( + assert mock_s3_upload.call_args[0][1]['data'].strip() == ( "phone number,name,favourite colour,fruit\r\n" "07739 468 050,Pete,Coral,tomato\r\n" "07527 125 974,Not Pete,Magenta,Avacado\r\n" @@ -157,7 +157,7 @@ def test_send_test_sms_message( follow_redirects=True ) assert response.status_code == 200 - mock_s3_upload.assert_called_with(ANY, fake_uuid, expected_data, 'eu-west-1') + mock_s3_upload.assert_called_with(fake_uuid, expected_data, 'eu-west-1') def test_send_test_email_message( @@ -185,7 +185,7 @@ def test_send_test_email_message( follow_redirects=True ) assert response.status_code == 200 - mock_s3_upload.assert_called_with(ANY, fake_uuid, expected_data, 'eu-west-1') + mock_s3_upload.assert_called_with(fake_uuid, expected_data, 'eu-west-1') def test_send_test_sms_message_with_placeholders( @@ -221,7 +221,7 @@ def test_send_test_sms_message_with_placeholders( follow_redirects=True ) assert response.status_code == 200 - mock_s3_upload.assert_called_with(ANY, fake_uuid, expected_data, 'eu-west-1') + mock_s3_upload.assert_called_with(fake_uuid, expected_data, 'eu-west-1') def test_api_info_page( diff --git a/tests/conftest.py b/tests/conftest.py index 2acc3f57c..45fbfb008 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -862,8 +862,8 @@ def mock_get_users_by_service(mocker): @pytest.fixture(scope='function') def mock_s3_upload(mocker): - def _upload(upload_id, service_id, filedata, region): - pass + def _upload(service_id, filedata, region): + return fake_uuid() return mocker.patch('app.main.views.send.s3upload', side_effect=_upload)