From f78b3bff81d8f77f3ef3841132398a4617edf152 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Thu, 7 Feb 2019 16:16:22 +0000 Subject: [PATCH] Add '/letter-branding' page to show all letter brands This is a platform admin page which lists all letter brands and their domain. Once the page to edit a letter brand has been created, each brand will link to its edit page, but for now this is just a list of brand names and domains. --- app/main/views/email_branding.py | 5 +-- app/main/views/letter_branding.py | 23 ++++++++-- app/navigation.py | 4 ++ .../views/email-branding/select-branding.html | 2 - .../manage-letter-branding.html | 2 +- .../select-letter-branding.html | 36 ++++++++++++++++ .../views/platform-admin/_base_template.html | 1 + tests/app/main/views/test_letter_branding.py | 42 ++++++++++++++++--- .../test_letter_branding_client.py | 3 ++ 9 files changed, 104 insertions(+), 14 deletions(-) create mode 100644 app/templates/views/letter-branding/select-letter-branding.html diff --git a/app/main/views/email_branding.py b/app/main/views/email_branding.py index 22b29325d..7fbb9aec8 100644 --- a/app/main/views/email_branding.py +++ b/app/main/views/email_branding.py @@ -12,7 +12,7 @@ from app.s3_client.s3_logo_client import ( persist_logo, upload_email_logo, ) -from app.utils import AgreementInfo, get_logo_cdn_domain, user_is_platform_admin +from app.utils import get_logo_cdn_domain, user_is_platform_admin @main.route("/email-branding", methods=['GET', 'POST']) @@ -24,8 +24,7 @@ def email_branding(): return render_template( 'views/email-branding/select-branding.html', email_brandings=brandings, - search_form=SearchByNameForm(), - agreement_info=AgreementInfo, + search_form=SearchByNameForm() ) diff --git a/app/main/views/letter_branding.py b/app/main/views/letter_branding.py index ec6630548..20810929d 100644 --- a/app/main/views/letter_branding.py +++ b/app/main/views/letter_branding.py @@ -12,7 +12,11 @@ from requests import get as requests_get from app import letter_branding_client from app.main import main -from app.main.forms import ServiceLetterBrandingDetails, SVGFileUpload +from app.main.forms import ( + SearchByNameForm, + ServiceLetterBrandingDetails, + SVGFileUpload, +) from app.s3_client.s3_logo_client import ( LETTER_TEMP_TAG, delete_letter_temp_file, @@ -27,6 +31,20 @@ from app.s3_client.s3_logo_client import ( from app.utils import get_logo_cdn_domain, user_is_platform_admin +@main.route("/letter-branding", methods=['GET']) +@login_required +@user_is_platform_admin +def letter_branding(): + + brandings = letter_branding_client.get_all_letter_branding() + + return render_template( + 'views/letter-branding/select-letter-branding.html', + letter_brandings=brandings, + search_form=SearchByNameForm() + ) + + @main.route("/letter-branding/create", methods=['GET', 'POST']) @main.route("/letter-branding/create/", methods=['GET', 'POST']) @login_required @@ -65,8 +83,7 @@ def create_letter_branding(logo=None): upload_letter_logos(logo, db_filename, png_file, session['user_id']) - # TODO: redirect to all letter branding page once it exists - return redirect(url_for('main.platform_admin')) + return redirect(url_for('main.letter_branding')) except HTTPError as e: if 'domain' in e.message: diff --git a/app/navigation.py b/app/navigation.py index 13355a3a3..6ba3117e1 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -78,6 +78,7 @@ class HeaderNavigation(Navigation): 'create_letter_branding', 'email_branding', 'find_users_by_email', + 'letter_branding', 'live_services', 'organisations', 'platform_admin', @@ -437,6 +438,7 @@ class MainNavigation(Navigation): 'information_security', 'integration_testing', 'invite_org_user', + 'letter_branding', 'live_services', 'manage_org_users', 'new_password', @@ -623,6 +625,7 @@ class CaseworkNavigation(Navigation): 'integration_testing', 'invite_org_user', 'invite_user', + 'letter_branding', 'link_service_to_organisation', 'live_services', 'manage_org_users', @@ -855,6 +858,7 @@ class OrgNavigation(Navigation): 'information_security', 'integration_testing', 'invite_user', + 'letter_branding', 'link_service_to_organisation', 'live_services', 'manage_template_folder', diff --git a/app/templates/views/email-branding/select-branding.html b/app/templates/views/email-branding/select-branding.html index 6007d7f98..2d9f703f7 100644 --- a/app/templates/views/email-branding/select-branding.html +++ b/app/templates/views/email-branding/select-branding.html @@ -1,6 +1,4 @@ {% extends "views/platform-admin/_base_template.html" %} -{% from "components/radios.html" import radios %} -{% from "components/page-footer.html" import page_footer %} {% from "components/live-search.html" import live_search %} {% block per_page_title %} diff --git a/app/templates/views/letter-branding/manage-letter-branding.html b/app/templates/views/letter-branding/manage-letter-branding.html index 060054208..1e76d9e3f 100644 --- a/app/templates/views/letter-branding/manage-letter-branding.html +++ b/app/templates/views/letter-branding/manage-letter-branding.html @@ -27,7 +27,7 @@ 'Save', button_name='operation', button_value='branding-details', - back_link=url_for('main.platform_admin'), + back_link=url_for('main.letter_branding'), back_link_text='Back to letter branding selection', ) }} diff --git a/app/templates/views/letter-branding/select-letter-branding.html b/app/templates/views/letter-branding/select-letter-branding.html new file mode 100644 index 000000000..fe845c260 --- /dev/null +++ b/app/templates/views/letter-branding/select-letter-branding.html @@ -0,0 +1,36 @@ +{% extends "views/platform-admin/_base_template.html" %} +{% from "components/live-search.html" import live_search %} + +{% block per_page_title %} + Letter branding +{% endblock %} + +{% block platform_admin_content %} + +
+
+

Letter branding

+
+ +
+ {{ live_search(target_selector='.letter-brand', show=True, form=search_form) }} + + +{% endblock %} diff --git a/app/templates/views/platform-admin/_base_template.html b/app/templates/views/platform-admin/_base_template.html index 68f41c504..71d617dfc 100644 --- a/app/templates/views/platform-admin/_base_template.html +++ b/app/templates/views/platform-admin/_base_template.html @@ -18,6 +18,7 @@ ('Organisations', url_for('main.organisations')), ('Providers', url_for('main.view_providers')), ('Email branding', url_for('main.email_branding')), + ('Letter branding', url_for('main.letter_branding')), ('Inbound SMS numbers', url_for('main.inbound_sms_admin')), ('Find users by email', url_for('main.find_users_by_email')), ('Email Complaints', url_for('main.platform_admin_list_complaints')), diff --git a/tests/app/main/views/test_letter_branding.py b/tests/app/main/views/test_letter_branding.py index 21eab361e..e73070f3a 100644 --- a/tests/app/main/views/test_letter_branding.py +++ b/tests/app/main/views/test_letter_branding.py @@ -8,6 +8,41 @@ 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 +from tests.conftest import normalize_spaces + + +def test_letter_branding_page_shows_full_branding_list( + mocker, + logged_in_platform_admin_client, +): + letter_brandings = [ + {'id': '1', 'filename': '1-test', 'domain': None, 'name': 'test brand'}, + {'id': '2', 'filename': '2-example', 'domain': 'cabinet-office.gov.uk', 'name': 'example brand'} + ] + mocker.patch('app.main.views.letter_branding.letter_branding_client.get_all_letter_branding', + return_value=letter_brandings) + + response = logged_in_platform_admin_client.get( + url_for('.letter_branding') + ) + + assert response.status_code == 200 + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + brand_names = [normalize_spaces(name.text) for name in page.select('.letter-brand .message-name')] + brand_hints = [normalize_spaces(hint.text) for hint in page.select('.letter-brand .message-type')] + + assert normalize_spaces( + page.select_one('h1').text + ) == "Letter branding" + + assert page.select_one('.column-three-quarters a')['href'] == url_for('main.create_letter_branding') + + assert list(zip( + brand_names, brand_hints + )) == [ + ('test brand', '–'), + ('example brand', 'Default for cabinet-office.gov.uk'), + ] def test_create_letter_branding_does_not_show_branding_info(logged_in_platform_admin_client): @@ -154,11 +189,6 @@ def test_create_letter_branding_persists_logo_when_all_data_is_valid( 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') - # TODO: remove platform admin page mocks once we no longer redirect there - mocker.patch('app.main.views.platform_admin.make_columns') - mocker.patch('app.main.views.platform_admin.platform_stats_api_client.get_aggregate_platform_stats') - mocker.patch('app.main.views.platform_admin.complaint_api_client.get_complaint_count') - response = logged_in_platform_admin_client.post( url_for('.create_letter_branding', logo=temp_logo), data={ @@ -168,8 +198,10 @@ def test_create_letter_branding_persists_logo_when_all_data_is_valid( }, follow_redirects=True ) + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert response.status_code == 200 + assert page.find('h1').text == 'Letter branding' mock_letter_client.create_letter_branding.assert_called_once_with( domain='bl.uk', filename='{}-test'.format(fake_uuid), name='Test brand' diff --git a/tests/app/notify_client/test_letter_branding_client.py b/tests/app/notify_client/test_letter_branding_client.py index 8e29510b6..454c1f3dc 100644 --- a/tests/app/notify_client/test_letter_branding_client.py +++ b/tests/app/notify_client/test_letter_branding_client.py @@ -40,6 +40,7 @@ def test_create_letter_branding(mocker): new_branding = {'filename': 'uuid-test', 'name': 'my letters', 'domain': 'example.com'} mock_post = mocker.patch('app.notify_client.letter_branding_client.LetterBrandingClient.post') + mock_redis_delete = mocker.patch('app.notify_client.RedisClient.delete') LetterBrandingClient().create_letter_branding( filename=new_branding['filename'], name=new_branding['name'], domain=new_branding['domain'] @@ -48,3 +49,5 @@ def test_create_letter_branding(mocker): url='/letter-branding', data=new_branding ) + + mock_redis_delete.assert_called_once_with('letter_branding')