diff --git a/app/main/views/letter_branding.py b/app/main/views/letter_branding.py index 76edbc08a..76c694e30 100644 --- a/app/main/views/letter_branding.py +++ b/app/main/views/letter_branding.py @@ -8,7 +8,6 @@ from flask import ( url_for, ) from notifications_python_client.errors import HTTPError -from requests import get as requests_get from app import letter_branding_client from app.main import main @@ -21,11 +20,9 @@ from app.s3_client.s3_logo_client import ( LETTER_TEMP_TAG, delete_letter_temp_file, delete_letter_temp_files_created_by, - get_letter_filename_with_no_path_or_extension, letter_filename_for_db, permanent_letter_logo_name, persist_logo, - upload_letter_png_logo, upload_letter_temp_logo, ) from app.utils import get_logo_cdn_domain, user_is_platform_admin @@ -87,15 +84,13 @@ def update_letter_branding(branding_id, logo=None): return redirect(url_for('main.letter_branding')) else: - png_file = get_png_file_from_svg(logo) - letter_branding_client.update_letter_branding( branding_id=branding_id, filename=db_filename, name=letter_branding_details_form.name.data, ) - upload_letter_logos(logo, db_filename, png_file, session['user_id']) + upload_letter_svg_logo(logo, db_filename, session['user_id']) return redirect(url_for('main.letter_branding')) @@ -149,7 +144,6 @@ def create_letter_branding(logo=None): if details_form_submitted and letter_branding_details_form.validate_on_submit(): if logo: db_filename = letter_filename_for_db(logo, session['user_id']) - png_file = get_png_file_from_svg(logo) try: letter_branding_client.create_letter_branding( @@ -157,7 +151,7 @@ def create_letter_branding(logo=None): name=letter_branding_details_form.name.data, ) - upload_letter_logos(logo, db_filename, png_file, session['user_id']) + upload_letter_svg_logo(logo, db_filename, session['user_id']) return redirect(url_for('main.letter_branding')) @@ -179,29 +173,7 @@ def create_letter_branding(logo=None): ) -def get_png_file_from_svg(filename): - filename_for_template_preview = get_letter_filename_with_no_path_or_extension(filename) - - template_preview_svg_endpoint = '{}/{}.svg.png'.format( - current_app.config['TEMPLATE_PREVIEW_API_HOST'], - filename_for_template_preview - ) - - response = requests_get( - template_preview_svg_endpoint, - headers={'Authorization': 'Token {}'.format(current_app.config['TEMPLATE_PREVIEW_API_KEY'])} - ) - - return response.content - - -def upload_letter_logos(old_filename, new_filename, png_file, user_id): +def upload_letter_svg_logo(old_filename, new_filename, user_id): persist_logo(old_filename, permanent_letter_logo_name(new_filename, 'svg')) - upload_letter_png_logo( - permanent_letter_logo_name(new_filename, 'png'), - png_file, - current_app.config['AWS_REGION'], - ) - delete_letter_temp_files_created_by(user_id) diff --git a/app/s3_client/s3_logo_client.py b/app/s3_client/s3_logo_client.py index bb8626bae..3e280bc25 100644 --- a/app/s3_client/s3_logo_client.py +++ b/app/s3_client/s3_logo_client.py @@ -80,17 +80,6 @@ def upload_letter_temp_logo(filename, filedata, region, user_id): return upload_filename -def upload_letter_png_logo(filename, filedata, region): - bucket_name = current_app.config['LOGO_UPLOAD_BUCKET_NAME'] - utils_s3upload( - filedata=filedata, - region=region, - bucket_name=bucket_name, - file_location=filename, - content_type='image/png' - ) - - def permanent_email_logo_name(filename, user_id): if filename.startswith(TEMP_TAG.format(user_id=user_id)): return get_temp_truncated_filename(filename=filename, user_id=user_id) diff --git a/tests/app/main/views/test_letter_branding.py b/tests/app/main/views/test_letter_branding.py index 6e70b3057..65a691d67 100644 --- a/tests/app/main/views/test_letter_branding.py +++ b/tests/app/main/views/test_letter_branding.py @@ -4,10 +4,9 @@ from uuid import UUID from botocore.exceptions import ClientError as BotoClientError from bs4 import BeautifulSoup -from flask import current_app, url_for +from flask import url_for from notifications_python_client.errors import HTTPError -from app.main.views.letter_branding import get_png_file_from_svg from app.s3_client.s3_logo_client import ( LETTER_TEMP_LOGO_LOCATION, permanent_letter_logo_name, @@ -150,8 +149,7 @@ def test_update_letter_branding_with_original_file_and_new_details( fake_uuid ): mock_client_update = mocker.patch('app.main.views.letter_branding.letter_branding_client.update_letter_branding') - mock_template_preview = mocker.patch('app.main.views.letter_branding.get_png_file_from_svg') - mock_upload_logos = mocker.patch('app.main.views.letter_branding.upload_letter_logos') + mock_upload_logos = mocker.patch('app.main.views.letter_branding.upload_letter_svg_logo') response = platform_admin_client.post( url_for('.update_letter_branding', branding_id=fake_uuid), @@ -166,7 +164,6 @@ def test_update_letter_branding_with_original_file_and_new_details( assert page.find('h1').text == 'Letter branding' mock_upload_logos.assert_not_called() - mock_template_preview.assert_not_called() mock_client_update.assert_called_once_with( branding_id=fake_uuid, @@ -208,7 +205,6 @@ def test_update_letter_branding_shows_database_errors_on_name_field( mock_get_letter_branding_by_id, fake_uuid, ): - mocker.patch('app.main.views.letter_branding.get_png_file_from_svg') mocker.patch('app.main.views.letter_branding.letter_branding_client.update_letter_branding', side_effect=HTTPError( response=Mock( status_code=400, @@ -249,11 +245,7 @@ def test_update_letter_branding_with_new_file_and_new_details( temp_logo = LETTER_TEMP_LOGO_LOCATION.format(user_id=fake_uuid, unique_id=fake_uuid, filename='new_file.svg') mock_client_update = mocker.patch('app.main.views.letter_branding.letter_branding_client.update_letter_branding') - mock_template_preview = mocker.patch( - 'app.main.views.letter_branding.get_png_file_from_svg', - return_value='fake_png') mock_persist_logo = mocker.patch('app.main.views.letter_branding.persist_logo') - mock_upload_png = mocker.patch('app.main.views.letter_branding.upload_letter_png_logo') mock_delete_temp_files = mocker.patch('app.main.views.letter_branding.delete_letter_temp_files_created_by') response = platform_admin_client.post( @@ -268,7 +260,6 @@ def test_update_letter_branding_with_new_file_and_new_details( assert response.status_code == 200 assert page.find('h1').text == 'Letter branding' - assert mock_template_preview.called mock_client_update.assert_called_once_with( branding_id=fake_uuid, filename='{}-new_file'.format(fake_uuid), @@ -278,11 +269,6 @@ def test_update_letter_branding_with_new_file_and_new_details( temp_logo, 'letters/static/images/letter-template/{}-new_file.svg'.format(fake_uuid) ) - mock_upload_png.assert_called_once_with( - 'letters/static/images/letter-template/{}-new_file.png'.format(fake_uuid), - 'fake_png', - current_app.config['AWS_REGION'] - ) mock_delete_temp_files.assert_called_once_with(fake_uuid) @@ -292,9 +278,8 @@ def test_update_letter_branding_rolls_back_db_changes_and_shows_error_if_saving_ mock_get_letter_branding_by_id, fake_uuid ): - mocker.patch('app.main.views.letter_branding.get_png_file_from_svg',) mock_client_update = mocker.patch('app.main.views.letter_branding.letter_branding_client.update_letter_branding') - mocker.patch('app.main.views.letter_branding.upload_letter_logos', side_effect=BotoClientError({}, 'error')) + mocker.patch('app.main.views.letter_branding.upload_letter_svg_logo', side_effect=BotoClientError({}, 'error')) temp_logo = LETTER_TEMP_LOGO_LOCATION.format(user_id=fake_uuid, unique_id=fake_uuid, filename='new_file.svg') response = platform_admin_client.post( @@ -450,11 +435,7 @@ def test_create_letter_branding_persists_logo_when_all_data_is_valid( temp_logo = LETTER_TEMP_LOGO_LOCATION.format(user_id=user_id, unique_id=fake_uuid, filename='test.svg') mock_letter_client = mocker.patch('app.main.views.letter_branding.letter_branding_client') - mock_template_preview = mocker.patch( - 'app.main.views.letter_branding.get_png_file_from_svg', - return_value='fake_png') mock_persist_logo = mocker.patch('app.main.views.letter_branding.persist_logo') - mock_upload_png = mocker.patch('app.main.views.letter_branding.upload_letter_png_logo') mock_delete_temp_files = mocker.patch('app.main.views.letter_branding.delete_letter_temp_files_created_by') response = platform_admin_client.post( @@ -473,16 +454,10 @@ def test_create_letter_branding_persists_logo_when_all_data_is_valid( mock_letter_client.create_letter_branding.assert_called_once_with( filename='{}-test'.format(fake_uuid), name='Test brand' ) - assert mock_template_preview.called mock_persist_logo.assert_called_once_with( temp_logo, 'letters/static/images/letter-template/{}-test.svg'.format(fake_uuid) ) - mock_upload_png.assert_called_once_with( - 'letters/static/images/letter-template/{}-test.png'.format(fake_uuid), - 'fake_png', - current_app.config['AWS_REGION'] - ) mock_delete_temp_files.assert_called_once_with(user_id) @@ -519,7 +494,6 @@ def test_create_letter_branding_shows_database_errors_on_name_fields( with platform_admin_client.session_transaction() as session: user_id = session["user_id"] - mocker.patch('app.main.views.letter_branding.get_png_file_from_svg') mocker.patch('app.main.views.letter_branding.letter_branding_client.create_letter_branding', side_effect=HTTPError( response=Mock( status_code=400, @@ -550,19 +524,3 @@ def test_create_letter_branding_shows_database_errors_on_name_fields( assert page.find('h1').text == 'Add letter branding' assert error_message == 'name already in use' - - -def test_get_png_file_from_svg(client, mocker, fake_uuid): - mocker.patch.dict( - 'flask.current_app.config', - {'TEMPLATE_PREVIEW_API_HOST': 'localhost', 'TEMPLATE_PREVIEW_API_KEY': 'abc'} - ) - tp_mock = mocker.patch('app.main.views.letter_branding.requests_get') - filename = LETTER_TEMP_LOGO_LOCATION.format(user_id=fake_uuid, unique_id=fake_uuid, filename='test.svg') - - get_png_file_from_svg(filename) - - tp_mock.assert_called_once_with( - 'localhost/temp-{}_{}-test.svg.png'.format(fake_uuid, fake_uuid), - headers={'Authorization': 'Token abc'} - ) diff --git a/tests/app/s3_client/test_s3_logo_client.py b/tests/app/s3_client/test_s3_logo_client.py index 29cedd4a9..a36d73c97 100644 --- a/tests/app/s3_client/test_s3_logo_client.py +++ b/tests/app/s3_client/test_s3_logo_client.py @@ -16,7 +16,6 @@ from app.s3_client.s3_logo_client import ( permanent_email_logo_name, persist_logo, upload_email_logo, - upload_letter_png_logo, upload_letter_temp_logo, ) @@ -76,21 +75,6 @@ def test_upload_letter_temp_logo_calls_correct_args(mocker, fake_uuid, letter_up assert new_filename == 'letters/static/images/letter-template/temp-{}_test_uuid-test.svg'.format(fake_uuid) -def test_upload_letter_png_logo_calls_correct_args(mocker): - mocked_s3_upload = mocker.patch('app.s3_client.s3_logo_client.utils_s3upload') - mocker.patch.dict('flask.current_app.config', {'LOGO_UPLOAD_BUCKET_NAME': bucket}) - - upload_letter_png_logo(filename, data, region) - - mocked_s3_upload.assert_called_once_with( - filedata=data, - region=region, - bucket_name=bucket, - file_location=filename, - content_type='image/png' - ) - - def test_persist_logo(client, mocker, fake_uuid, upload_filename): mocker.patch.dict('flask.current_app.config', {'LOGO_UPLOAD_BUCKET_NAME': bucket}) mocked_get_s3_object = mocker.patch('app.s3_client.s3_logo_client.get_s3_object')