mirror of
https://github.com/GSA/notifications-admin.git
synced 2026-02-05 19:03:30 -05:00
don't convert letter logo svgs to pngs
https://github.com/alphagov/notifications-template-preview/pull/371 stops using png logos when rendering png previews of templated letters. when that is merged we no longer need to convert to pngs when we upload a new letter logo.
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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'}
|
||||
)
|
||||
|
||||
@@ -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')
|
||||
|
||||
Reference in New Issue
Block a user