Merge pull request #126 from alphagov/better-bucket-name

Better bucket name pending possible change to uuid for service primary

app/main/uploader.py

@@ -6,7 +6,7 @@ from boto3 import resource
 def s3upload(service_id, filedata):
     upload_id = str(uuid.uuid4())
     s3 = resource('s3')
-    bucket_name = 'service-{}-notify'.format(service_id)
+    bucket_name = 'service-{}-{}-notify'.format(service_id, upload_id)
     s3.create_bucket(Bucket=bucket_name)
     contents = '\n'.join(filedata['data'])
     key = s3.Object(bucket_name, upload_id)
@@ -16,7 +16,7 @@ def s3upload(service_id, filedata):
 
 def s3download(service_id, upload_id):
     s3 = resource('s3')
-    bucket_name = 'service-{}-notify'.format(service_id)
+    bucket_name = 'service-{}-{}-notify'.format(service_id, upload_id)
     key = s3.Object(bucket_name, upload_id)
     contents = key.get()['Body'].read().decode('utf-8')
     return contents

app/main/views/sms.py

@@ -64,6 +64,7 @@ def send_sms(service_id):
             methods=['GET', 'POST'])
 @login_required
 def check_sms(service_id, upload_id):
+
     if request.method == 'GET':
         contents = s3download(service_id, upload_id)
         upload_result = _get_numbers(contents)
@@ -78,9 +79,10 @@ def check_sms(service_id, upload_id):
             service_id=service_id
         )
     elif request.method == 'POST':
-        file_name = request.form['original_file_name']
 
-        # TODO need a real template id picked from form
+        file_name = request.form['file_name']
+        # TODO - template id should come from form but is not wired in yet.
+        # that will be done in another story
         template_id = 1
 
         job_api_client.create_job(service_id, template_id, file_name)

app/notify_client/job_api_client.py

@@ -21,7 +21,7 @@ class JobApiClient(BaseAPIClient):
             "service": service_id,
             "template": template_id,
             "original_file_name": file_name,
-            "bucket_name": "service-{}-notify".format(service_id),
+            "bucket_name": "service-{}-{}-notify".format(service_id, job_id),
             "file_name": "{}.csv".format(job_id)
         }
 

tests/app/main/views/test_sms.py

@@ -126,16 +126,20 @@ def test_create_job_should_call_api(app_,
                                     job_data,
                                     mock_create_job):
 
-    service_id = job_data['service']
-    template_id = job_data['template']
-    upload_id = job_data['id']
-    file_name = job_data['original_file_name']
+    service_id = service_one['id']
+    job_id = job_data['id']
+    file_name = job_data['file_name']
+
+    # TODO - template id should come from form but is not wired in yet.
+    # that will be done in another story
+    template_id = 1
 
     with app_.test_request_context():
         with app_.test_client() as client:
             client.login(api_user_active)
-            url = url_for('main.check_sms', service_id=service_id, upload_id=upload_id, file_name=file_name)
+            url = url_for('main.check_sms', service_id=service_one['id'], upload_id=job_id, file_name=file_name)
             response = client.post(url, data=job_data, follow_redirects=True)
 
         assert response.status_code == 200
         mock_create_job.assert_called_with(service_id, template_id, file_name)
+        assert job_data['bucket_name'] == "service-{}-{}-notify".format(service_id, job_id)

tests/conftest.py

@@ -425,15 +425,9 @@ def mock_check_verify_code_code_expired(mocker):
 def job_data(mocker):
     import uuid
     job_id = uuid.uuid4()
-    original_file_name = 'thisisatest.csv'
-    bucket_name = 'service-2-notify'
-    file_name = '{}.csv'.format(job_id)
+    file_name = 'thisisatest.csv'
     data = {
         'id': str(job_id),
-        'service': 1,
-        'template': 1,
-        'original_file_name': original_file_name,
-        'bucket_name': bucket_name,
         'file_name': file_name,
     }
     return data
@@ -442,5 +436,10 @@ def job_data(mocker):
 @pytest.fixture(scope='function')
 def mock_create_job(mocker, job_data):
     def _create(service_id, template_id, file_name):
+        job_data['service'] = service_id
+        job_data['template'] = template_id
+        job_data['bucket_name'] = 'service-{}-{}-notify'.format(service_id, job_data['id'])
+        job_data['original_file_name'] = file_name
+        job_data['file_name'] = '{}.csv'.format(job_data['id'])
         return job_data
     return mocker.patch('app.job_api_client.create_job', side_effect=_create)