mirror of
https://github.com/GSA/notifications-admin.git
synced 2026-09-05 02:38:25 -04:00
Handle the database errors on name and domain fields
Form errors were being shown (such as a domain not being a valid), but we weren't showing nicely formatted error messages if the database failed to save a new row.
This commit is contained in:
@@ -7,6 +7,7 @@ from flask import (
|
|||||||
url_for,
|
url_for,
|
||||||
)
|
)
|
||||||
from flask_login import login_required
|
from flask_login import login_required
|
||||||
|
from notifications_python_client.errors import HTTPError
|
||||||
from requests import get as requests_get
|
from requests import get as requests_get
|
||||||
|
|
||||||
from app import letter_branding_client
|
from app import letter_branding_client
|
||||||
@@ -53,30 +54,30 @@ def create_letter_branding(logo=None):
|
|||||||
if details_form_submitted and letter_branding_details_form.validate_on_submit():
|
if details_form_submitted and letter_branding_details_form.validate_on_submit():
|
||||||
if logo:
|
if logo:
|
||||||
db_filename = letter_filename_for_db(logo, session['user_id'])
|
db_filename = letter_filename_for_db(logo, session['user_id'])
|
||||||
|
|
||||||
letter_branding_client.create_letter_branding(
|
|
||||||
filename=db_filename,
|
|
||||||
name=letter_branding_details_form.name.data,
|
|
||||||
domain=letter_branding_details_form.domain.data,
|
|
||||||
)
|
|
||||||
|
|
||||||
png_file = get_png_file_from_svg(logo)
|
png_file = get_png_file_from_svg(logo)
|
||||||
|
|
||||||
persist_logo(logo, permanent_letter_logo_name(db_filename, 'svg'))
|
try:
|
||||||
|
letter_branding_client.create_letter_branding(
|
||||||
|
filename=db_filename,
|
||||||
|
name=letter_branding_details_form.name.data,
|
||||||
|
domain=letter_branding_details_form.domain.data,
|
||||||
|
)
|
||||||
|
|
||||||
upload_letter_png_logo(
|
upload_letter_logos(logo, db_filename, png_file, session['user_id'])
|
||||||
permanent_letter_logo_name(db_filename, 'png'),
|
|
||||||
png_file,
|
|
||||||
current_app.config['AWS_REGION'],
|
|
||||||
)
|
|
||||||
|
|
||||||
delete_letter_temp_files_created_by(session['user_id'])
|
# TODO: redirect to all letter branding page once it exists
|
||||||
|
return redirect(url_for('main.platform_admin'))
|
||||||
|
|
||||||
# TODO: redirect to all letter branding page once it exists
|
except HTTPError as e:
|
||||||
return redirect(url_for('main.platform_admin'))
|
if 'domain' in e.message:
|
||||||
|
letter_branding_details_form.domain.errors.append(e.message['domain'][0])
|
||||||
# Show error on upload form if trying to submit with no logo
|
elif 'name' in e.message:
|
||||||
file_upload_form.validate()
|
letter_branding_details_form.name.errors.append(e.message['name'][0])
|
||||||
|
else:
|
||||||
|
raise e
|
||||||
|
else:
|
||||||
|
# Show error on upload form if trying to submit with no logo
|
||||||
|
file_upload_form.validate()
|
||||||
|
|
||||||
return render_template(
|
return render_template(
|
||||||
'views/letter-branding/manage-letter-branding.html',
|
'views/letter-branding/manage-letter-branding.html',
|
||||||
@@ -101,3 +102,15 @@ def get_png_file_from_svg(filename):
|
|||||||
)
|
)
|
||||||
|
|
||||||
return response.content
|
return response.content
|
||||||
|
|
||||||
|
|
||||||
|
def upload_letter_logos(old_filename, new_filename, png_file, 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)
|
||||||
|
|||||||
@@ -1,8 +1,10 @@
|
|||||||
from io import BytesIO
|
from io import BytesIO
|
||||||
|
from unittest.mock import Mock
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
from bs4 import BeautifulSoup
|
from bs4 import BeautifulSoup
|
||||||
from flask import current_app, url_for
|
from flask import current_app, url_for
|
||||||
|
from notifications_python_client.errors import HTTPError
|
||||||
|
|
||||||
from app.main.views.letter_branding import get_png_file_from_svg
|
from app.main.views.letter_branding import get_png_file_from_svg
|
||||||
from app.s3_client.s3_logo_client import LETTER_TEMP_LOGO_LOCATION
|
from app.s3_client.s3_logo_client import LETTER_TEMP_LOGO_LOCATION
|
||||||
@@ -185,7 +187,7 @@ def test_create_letter_branding_persists_logo_when_all_data_is_valid(
|
|||||||
mock_delete_temp_files.assert_called_once_with(user_id)
|
mock_delete_temp_files.assert_called_once_with(user_id)
|
||||||
|
|
||||||
|
|
||||||
def test_create_letter_branding_shows_errors_on_name_and_domain_fields(
|
def test_create_letter_branding_shows_form_errors_on_name_and_domain_fields(
|
||||||
logged_in_platform_admin_client,
|
logged_in_platform_admin_client,
|
||||||
fake_uuid
|
fake_uuid
|
||||||
):
|
):
|
||||||
@@ -212,6 +214,50 @@ def test_create_letter_branding_shows_errors_on_name_and_domain_fields(
|
|||||||
assert error_messages[1].text.strip() == 'Not a known government domain (you might need to update domains.yml)'
|
assert error_messages[1].text.strip() == 'Not a known government domain (you might need to update domains.yml)'
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize('error_field', ['name', 'domain'])
|
||||||
|
def test_create_letter_branding_shows_database_errors_on_name_and_domain_fields(
|
||||||
|
mocker,
|
||||||
|
logged_in_platform_admin_client,
|
||||||
|
fake_uuid,
|
||||||
|
error_field
|
||||||
|
):
|
||||||
|
with logged_in_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,
|
||||||
|
json={
|
||||||
|
'result': 'error',
|
||||||
|
'message': {
|
||||||
|
error_field: {
|
||||||
|
'{} already in use'.format(error_field)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
),
|
||||||
|
message={error_field: ['{} already in use'.format(error_field)]}
|
||||||
|
))
|
||||||
|
|
||||||
|
temp_logo = LETTER_TEMP_LOGO_LOCATION.format(user_id=user_id, unique_id=fake_uuid, filename='test.svg')
|
||||||
|
|
||||||
|
response = logged_in_platform_admin_client.post(
|
||||||
|
url_for('.create_letter_branding', logo=temp_logo),
|
||||||
|
data={
|
||||||
|
'name': 'my brand',
|
||||||
|
'domain': None,
|
||||||
|
'operation': 'branding-details'
|
||||||
|
}
|
||||||
|
)
|
||||||
|
|
||||||
|
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
|
||||||
|
error_message = page.find('span', class_='error-message').text.strip()
|
||||||
|
|
||||||
|
assert page.find('h1').text == 'Add letter branding'
|
||||||
|
assert error_message == '{} already in use'.format(error_field)
|
||||||
|
|
||||||
|
|
||||||
def test_get_png_file_from_svg(client, mocker, fake_uuid):
|
def test_get_png_file_from_svg(client, mocker, fake_uuid):
|
||||||
mocker.patch.dict(
|
mocker.patch.dict(
|
||||||
'flask.current_app.config',
|
'flask.current_app.config',
|
||||||
|
|||||||
Reference in New Issue
Block a user