Make s3upload function return the UUID

Generating the UUID can be can be contained within this function,
thus any other part of the code using it doesn’t have to do the
ID-generating stuff itself.

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 = ''

app/main/views/send.py

@@ -110,9 +110,7 @@ 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,
                 current_app.config['AWS_REGION']
@@ -161,6 +159,8 @@ 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']
@@ -173,13 +173,18 @@ def send_test(service_id, template_id):
                 [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 = s3upload(
+                service_id,
+                {
+                    'file_name': file_name,
+                    'data': output.getvalue()
+                },
+                current_app.config['AWS_REGION']
+            )
+            session['upload_data'] = {
+                "template_id": template_id,
+                "original_file_name": file_name
             }
-            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,

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(

tests/conftest.py

@@ -816,8 +816,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)