diff --git a/app/config.py b/app/config.py index 7b8984287..85a72ccb9 100644 --- a/app/config.py +++ b/app/config.py @@ -68,6 +68,7 @@ class Config(object): WTF_CSRF_ENABLED = True WTF_CSRF_TIME_LIMIT = None CSV_UPLOAD_BUCKET_NAME = 'local-notifications-csv-upload' + CONTACT_LIST_UPLOAD_BUCKET_NAME = 'local-contact-list' ACTIVITY_STATS_LIMIT_DAYS = 7 TEST_MESSAGE_FILENAME = 'Report' @@ -98,6 +99,7 @@ class Development(Config): SESSION_PROTECTION = None STATSD_ENABLED = False CSV_UPLOAD_BUCKET_NAME = 'development-notifications-csv-upload' + CONTACT_LIST_UPLOAD_BUCKET_NAME = 'development-contact-list' LOGO_UPLOAD_BUCKET_NAME = 'public-logos-tools' MOU_BUCKET_NAME = 'notify.tools-mou' TRANSIENT_UPLOADED_LETTERS = 'development-transient-uploaded-letters' @@ -121,6 +123,7 @@ class Test(Development): STATSD_ENABLED = False WTF_CSRF_ENABLED = False CSV_UPLOAD_BUCKET_NAME = 'test-notifications-csv-upload' + CONTACT_LIST_UPLOAD_BUCKET_NAME = 'test-contact-list' LOGO_UPLOAD_BUCKET_NAME = 'public-logos-test' MOU_BUCKET_NAME = 'test-mou' TRANSIENT_UPLOADED_LETTERS = 'test-transient-uploaded-letters' @@ -140,6 +143,7 @@ class Preview(Config): HEADER_COLOUR = '#F499BE' # $baby-pink STATSD_ENABLED = True CSV_UPLOAD_BUCKET_NAME = 'preview-notifications-csv-upload' + CONTACT_LIST_UPLOAD_BUCKET_NAME = 'preview-contact-list' LOGO_UPLOAD_BUCKET_NAME = 'public-logos-preview' MOU_BUCKET_NAME = 'notify.works-mou' TRANSIENT_UPLOADED_LETTERS = 'preview-transient-uploaded-letters' @@ -158,6 +162,7 @@ class Staging(Config): HEADER_COLOUR = '#6F72AF' # $mauve STATSD_ENABLED = True CSV_UPLOAD_BUCKET_NAME = 'staging-notifications-csv-upload' + CONTACT_LIST_UPLOAD_BUCKET_NAME = 'staging-contact-list' LOGO_UPLOAD_BUCKET_NAME = 'public-logos-staging' MOU_BUCKET_NAME = 'staging-notify.works-mou' TRANSIENT_UPLOADED_LETTERS = 'staging-transient-uploaded-letters' @@ -173,6 +178,7 @@ class Live(Config): HTTP_PROTOCOL = 'https' STATSD_ENABLED = True CSV_UPLOAD_BUCKET_NAME = 'live-notifications-csv-upload' + CONTACT_LIST_UPLOAD_BUCKET_NAME = 'live-contact-list' LOGO_UPLOAD_BUCKET_NAME = 'public-logos-production' MOU_BUCKET_NAME = 'notifications.service.gov.uk-mou' TRANSIENT_UPLOADED_LETTERS = 'production-transient-uploaded-letters' diff --git a/app/main/views/send.py b/app/main/views/send.py index dbfb3421b..f8833723b 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -151,7 +151,7 @@ def send_messages(service_id, template_id): try: upload_id = s3upload( service_id, - Spreadsheet.from_file(form.file.data, filename=form.file.data.filename).as_dict, + Spreadsheet.from_file_form(form).as_dict, current_app.config['AWS_REGION'] ) return redirect(url_for( diff --git a/app/main/views/uploads.py b/app/main/views/uploads.py index ea9fc5770..feedb58f5 100644 --- a/app/main/views/uploads.py +++ b/app/main/views/uploads.py @@ -28,11 +28,7 @@ from app import current_service, notification_api_client, service_api_client from app.extensions import antivirus_client from app.main import main from app.main.forms import CsvUploadForm, LetterUploadPostageForm, PDFUploadForm -from app.s3_client.s3_csv_client import ( - s3download, - s3upload, - set_metadata_on_csv_upload, -) +from app.models.contact_list import ContactList from app.s3_client.s3_letter_upload_client import ( get_letter_metadata, get_letter_pdf_and_metadata, @@ -307,10 +303,9 @@ def upload_contact_list(service_id): if form.validate_on_submit(): try: - upload_id = s3upload( - service_id, - Spreadsheet.from_file(form.file.data, filename=form.file.data.filename).as_dict, - current_app.config['AWS_REGION'], + upload_id = ContactList.upload( + current_service.id, + Spreadsheet.from_file_form(form).as_dict, ) return redirect(url_for( '.check_contact_list', @@ -345,7 +340,7 @@ def check_contact_list(service_id, upload_id): form = CsvUploadForm() - contents = s3download(service_id, upload_id).strip() + contents = ContactList.download(service_id, upload_id) first_row = contents.splitlines()[0].strip().rstrip(',') if contents else '' template_type = { @@ -414,7 +409,7 @@ def check_contact_list(service_id, upload_id): 'template_type': template_type } - set_metadata_on_csv_upload(service_id, upload_id, **metadata_kwargs) + ContactList.set_metadata(service_id, upload_id, **metadata_kwargs) return render_template( 'views/uploads/contact-list/ok.html', @@ -427,7 +422,7 @@ def check_contact_list(service_id, upload_id): @main.route("/services//save-contact-list/", methods=['POST']) @user_has_permissions('send_messages') def save_contact_list(service_id, upload_id): - current_service.save_contact_list(upload_id) + ContactList.create(current_service.id, upload_id) return redirect(url_for( '.contact_list', service_id=current_service.id, diff --git a/app/models/contact_list.py b/app/models/contact_list.py index 67cabc6d9..fb32a4327 100644 --- a/app/models/contact_list.py +++ b/app/models/contact_list.py @@ -1,16 +1,60 @@ -from flask import abort +from flask import abort, current_app +from notifications_utils.formatters import strip_whitespace from app.models import JSONModel from app.notify_client.contact_list_api_client import contact_list_api_client -from app.s3_client.s3_csv_client import get_csv_metadata +from app.s3_client.s3_csv_client import ( + get_csv_metadata, + s3download, + s3upload, + set_metadata_on_csv_upload, +) class ContactList(JSONModel): + @staticmethod + def get_bucket_name(): + return current_app.config['CONTACT_LIST_UPLOAD_BUCKET_NAME'] + + @staticmethod + def upload(service_id, file_dict): + return s3upload( + service_id, + file_dict, + current_app.config['AWS_REGION'], + bucket=ContactList.get_bucket_name(), + ) + + @staticmethod + def download(service_id, upload_id): + return strip_whitespace(s3download( + service_id, + upload_id, + bucket=ContactList.get_bucket_name(), + )) + + @staticmethod + def set_metadata(service_id, upload_id, **kwargs): + return set_metadata_on_csv_upload( + service_id, + upload_id, + bucket=ContactList.get_bucket_name(), + **kwargs, + ) + + @staticmethod + def get_metadata(service_id, upload_id): + return get_csv_metadata( + service_id, + upload_id, + bucket=ContactList.get_bucket_name(), + ) + @classmethod def create(cls, service_id, upload_id): - metadata = get_csv_metadata(service_id, upload_id) + metadata = cls.get_metadata(service_id, upload_id) if not metadata.get('valid'): abort(403) diff --git a/app/models/service.py b/app/models/service.py index 6b883724b..132ac157c 100644 --- a/app/models/service.py +++ b/app/models/service.py @@ -6,7 +6,6 @@ from notifications_utils.timezones import local_timezone from werkzeug.utils import cached_property from app.models import JSONModel -from app.models.contact_list import ContactList from app.models.job import ( ImmediateJobs, PaginatedJobs, @@ -133,9 +132,6 @@ class Service(JSONModel): return [] return ScheduledJobs(self.id) - def save_contact_list(self, upload_id): - return ContactList.create(self.id, upload_id) - @cached_property def invited_users(self): return InvitedUsers(self.id) diff --git a/app/s3_client/s3_csv_client.py b/app/s3_client/s3_csv_client.py index 7999426e1..8ddfb1a57 100644 --- a/app/s3_client/s3_csv_client.py +++ b/app/s3_client/s3_csv_client.py @@ -9,20 +9,20 @@ from app.s3_client.s3_logo_client import get_s3_object FILE_LOCATION_STRUCTURE = 'service-{}-notify/{}.csv' -def get_csv_location(service_id, upload_id): +def get_csv_location(service_id, upload_id, bucket=None): return ( - current_app.config['CSV_UPLOAD_BUCKET_NAME'], + bucket or current_app.config['CSV_UPLOAD_BUCKET_NAME'], FILE_LOCATION_STRUCTURE.format(service_id, upload_id), ) -def get_csv_upload(service_id, upload_id): - return get_s3_object(*get_csv_location(service_id, upload_id)) +def get_csv_upload(service_id, upload_id, bucket=None): + return get_s3_object(*get_csv_location(service_id, upload_id, bucket)) -def s3upload(service_id, filedata, region): +def s3upload(service_id, filedata, region, bucket=None): upload_id = str(uuid.uuid4()) - bucket_name, file_location = get_csv_location(service_id, upload_id) + bucket_name, file_location = get_csv_location(service_id, upload_id, bucket) utils_s3upload( filedata=filedata['data'], region=region, @@ -32,10 +32,10 @@ def s3upload(service_id, filedata, region): return upload_id -def s3download(service_id, upload_id): +def s3download(service_id, upload_id, bucket=None): contents = '' try: - key = get_csv_upload(service_id, upload_id) + key = get_csv_upload(service_id, upload_id, bucket) contents = key.get()['Body'].read().decode('utf-8') except botocore.exceptions.ClientError as e: current_app.logger.error("Unable to download s3 file {}".format( @@ -44,11 +44,11 @@ def s3download(service_id, upload_id): return contents -def set_metadata_on_csv_upload(service_id, upload_id, **kwargs): +def set_metadata_on_csv_upload(service_id, upload_id, bucket=None, **kwargs): get_csv_upload( - service_id, upload_id + service_id, upload_id, bucket=bucket ).copy_from( - CopySource='{}/{}'.format(*get_csv_location(service_id, upload_id)), + CopySource='{}/{}'.format(*get_csv_location(service_id, upload_id, bucket=bucket)), ServerSideEncryption='AES256', Metadata={ key: str(value) for key, value in kwargs.items() @@ -57,9 +57,18 @@ def set_metadata_on_csv_upload(service_id, upload_id, **kwargs): ) -def get_csv_metadata(service_id, upload_id): +def set_metadata_on_contact_list(service_id, upload_id, **kwargs): + return set_metadata_on_csv_upload( + service_id, + upload_id, + bucket=current_app.config['CONTACT_LIST_UPLOAD_BUCKET_NAME'], + **kwargs, + ) + + +def get_csv_metadata(service_id, upload_id, bucket=None): try: - key = get_csv_upload(service_id, upload_id) + key = get_csv_upload(service_id, upload_id, bucket) return key.get()['Metadata'] except botocore.exceptions.ClientError as e: current_app.logger.error("Unable to download s3 file {}".format( diff --git a/app/utils.py b/app/utils.py index 07d7b78c2..0fae647f0 100644 --- a/app/utils.py +++ b/app/utils.py @@ -323,6 +323,13 @@ class Spreadsheet(): pyexcel.free_resources() return instance + @classmethod + def from_file_form(cls, form): + return cls.from_file( + form.file.data, + filename=form.file.data.filename, + ) + @property def as_rows(self): if not self._rows: diff --git a/tests/app/main/views/test_uploads.py b/tests/app/main/views/test_uploads.py index fa9b12d9d..de794df60 100644 --- a/tests/app/main/views/test_uploads.py +++ b/tests/app/main/views/test_uploads.py @@ -885,9 +885,14 @@ def test_upload_csv_file_shows_error_banner( expected_thead, expected_tbody, ): - - mock_upload = mocker.patch('app.main.views.uploads.s3upload', return_value=fake_uuid) - mock_download = mocker.patch('app.main.views.uploads.s3download', return_value=file_contents) + mock_upload = mocker.patch( + 'app.models.contact_list.s3upload', + return_value=fake_uuid, + ) + mock_download = mocker.patch( + 'app.models.contact_list.s3download', + return_value=file_contents, + ) page = client_request.post( 'main.upload_contact_list', @@ -896,9 +901,16 @@ def test_upload_csv_file_shows_error_banner( _follow_redirects=True, ) mock_upload.assert_called_once_with( - SERVICE_ONE_ID, {'data': '', 'file_name': 'invalid.csv'}, ANY, + SERVICE_ONE_ID, + {'data': '', 'file_name': 'invalid.csv'}, + ANY, + bucket='test-contact-list', + ) + mock_download.assert_called_once_with( + SERVICE_ONE_ID, + fake_uuid, + bucket='test-contact-list', ) - mock_download.assert_called_once_with(SERVICE_ONE_ID, fake_uuid) assert normalize_spaces(page.select_one('.banner-dangerous').text) == expected_error @@ -920,9 +932,8 @@ def test_upload_csv_file_shows_error_banner_for_too_many_rows( mock_get_users_by_service, fake_uuid, ): - - mocker.patch('app.main.views.uploads.s3upload', return_value=fake_uuid) - mocker.patch('app.main.views.uploads.s3download', return_value='\n'.join( + mocker.patch('app.models.contact_list.s3upload', return_value=fake_uuid) + mocker.patch('app.models.contact_list.s3download', return_value='\n'.join( ['phone number'] + (['07700900986'] * 50001) )) @@ -952,8 +963,8 @@ def test_upload_csv_shows_trial_mode_error( fake_uuid, mocker ): - mocker.patch('app.main.views.uploads.s3upload', return_value=fake_uuid) - mocker.patch('app.main.views.uploads.s3download', return_value=( + mocker.patch('app.models.contact_list.s3upload', return_value=fake_uuid) + mocker.patch('app.models.contact_list.s3download', return_value=( 'phone number\n' '07900900321' # Not in team )) @@ -983,10 +994,10 @@ def test_upload_csv_shows_ok_page( fake_uuid, mocker ): - mocker.patch('app.main.views.uploads.s3download', return_value='\n'.join( + mocker.patch('app.models.contact_list.s3download', return_value='\n'.join( ['email address'] + ['test@example.com'] * 51 )) - mock_metadata_set = mocker.patch('app.main.views.uploads.set_metadata_on_csv_upload') + mock_metadata_set = mocker.patch('app.models.contact_list.set_metadata_on_csv_upload') page = client_request.get( 'main.check_contact_list', @@ -999,6 +1010,7 @@ def test_upload_csv_shows_ok_page( mock_metadata_set.assert_called_once_with( SERVICE_ONE_ID, fake_uuid, + bucket='test-contact-list', row_count=51, original_file_name='good times.xlsx', template_type='email', @@ -1038,7 +1050,7 @@ def test_save_contact_list( fake_uuid, mock_create_contact_list, ): - mocker.patch('app.models.contact_list.get_csv_metadata', return_value={ + mock_get_metadata = mocker.patch('app.models.contact_list.get_csv_metadata', return_value={ 'row_count': 999, 'valid': True, 'original_file_name': 'example.csv', @@ -1056,6 +1068,11 @@ def test_save_contact_list( _external=True, ) ) + mock_get_metadata.assert_called_once_with( + SERVICE_ONE_ID, + fake_uuid, + bucket='test-contact-list', + ) mock_create_contact_list.assert_called_once_with( service_id=SERVICE_ONE_ID, upload_id=fake_uuid, diff --git a/tests/app/s3_client/test_s3_csv_client.py b/tests/app/s3_client/test_s3_csv_client.py index f4014065f..c8e07092e 100644 --- a/tests/app/s3_client/test_s3_csv_client.py +++ b/tests/app/s3_client/test_s3_csv_client.py @@ -12,7 +12,7 @@ def test_sets_metadata(client, mocker): set_metadata_on_csv_upload('1234', '5678', foo='bar', baz=True) - mocked_get_s3_object.assert_called_once_with('1234', '5678') + mocked_get_s3_object.assert_called_once_with('1234', '5678', bucket=None) mocked_s3_object.copy_from.assert_called_once_with( CopySource='test-notifications-csv-upload/service-1234-notify/5678.csv', Metadata={'baz': 'True', 'foo': 'bar'},