diff --git a/app/main/forms.py b/app/main/forms.py index 15b9e5ee5..b3678f75d 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -731,6 +731,7 @@ class ServiceUpdateEmailBranding(StripWhitespaceForm): name = StringField('Name of brand') text = StringField('Text') + domain = StringField('Domain') colour = StringField( 'Colour', render_kw={'onchange': 'update_colour(this)'}, @@ -738,15 +739,15 @@ class ServiceUpdateEmailBranding(StripWhitespaceForm): Regexp(regex="^$|^#(?:[0-9a-fA-F]{3}){1,2}$", message='Must be a valid color hex code') ] ) - file = FileField_wtf('Upload a PNG logo', validators=[FileAllowed(['png'], 'PNG Images only!')]) - - -class ServiceCreateEmailBranding(StripWhitespaceForm): - - name = StringField('Name of brand') - text = StringField('Text') - colour = StringField( - 'Colour', + banner_colour = StringField( + 'Banner colour', + render_kw={'onchange': 'update_colour(this)'}, + validators=[ + Regexp(regex="^$|^#(?:[0-9a-fA-F]{3}){1,2}$", message='Must be a valid color hex code') + ] + ) + single_id_colour = StringField( + 'Single identity colour', render_kw={'onchange': 'update_colour(this)'}, validators=[ Regexp(regex="^$|^#(?:[0-9a-fA-F]{3}){1,2}$", message='Must be a valid color hex code') diff --git a/app/main/views/email_branding.py b/app/main/views/email_branding.py index 4f55b1e7f..fbe5595d4 100644 --- a/app/main/views/email_branding.py +++ b/app/main/views/email_branding.py @@ -4,7 +4,6 @@ from flask_login import login_required from app import email_branding_client from app.main import main from app.main.forms import ( - ServiceCreateEmailBranding, ServiceSelectEmailBranding, ServiceUpdateEmailBranding, ) @@ -80,7 +79,10 @@ def update_email_branding(branding_id, logo=None): logo=logo, name=form.name.data, text=form.text.data, - colour=form.colour.data + colour=form.colour.data, + banner_colour=form.banner_colour.data, + single_id_colour=form.single_id_colour.data, + domain=form.domain.data, ) return redirect(url_for('.email_branding', branding_id=branding_id)) @@ -88,6 +90,9 @@ def update_email_branding(branding_id, logo=None): form.name.data = email_branding['name'] form.text.data = email_branding['text'] form.colour.data = email_branding['colour'] + form.banner_colour.data = email_branding['banner_colour'] + form.single_id_colour.data = email_branding['single_id_colour'] + form.domain.data = email_branding['domain'] return render_template( 'views/email-branding/manage-branding.html', @@ -103,7 +108,7 @@ def update_email_branding(branding_id, logo=None): @login_required @user_is_platform_admin def create_email_branding(logo=None): - form = ServiceCreateEmailBranding() + form = ServiceUpdateEmailBranding() if form.validate_on_submit(): if form.file.data: @@ -128,7 +133,10 @@ def create_email_branding(logo=None): logo=logo, name=form.name.data, text=form.text.data, - colour=form.colour.data + colour=form.colour.data, + banner_colour=form.banner_colour.data, + single_id_colour=form.single_id_colour.data, + domain=form.domain.data ) return redirect(url_for('.email_branding')) diff --git a/app/main/views/index.py b/app/main/views/index.py index 5e46f95c9..3b4232825 100644 --- a/app/main/views/index.py +++ b/app/main/views/index.py @@ -78,6 +78,13 @@ def design_content(): return render_template('views/design-patterns-content-guidance.html') +def _set_colour(branding_style, email_branding): + if branding_style in ['both', 'org']: + return email_branding['single_id_colour'] or email_branding['colour'] + elif branding_style == 'org_banner': + return email_branding['banner_colour'] or email_branding['colour'] + + @main.route('/_email') def email_template(): branding_type = request.args.get('branding_type', 'govuk') @@ -91,8 +98,9 @@ def email_template(): brand_banner = False else: email_branding = email_branding_client.get_email_branding(branding_style)['email_branding'] + colour = _set_colour(branding_type, email_branding) brand_name = email_branding['text'] - brand_colour = email_branding['colour'] + brand_colour = colour brand_logo = 'https://{}/{}'.format(get_cdn_domain(), email_branding['logo']) govuk_banner = branding_type in ['govuk', 'both'] brand_banner = branding_type == 'org_banner' diff --git a/app/notify_client/email_branding_client.py b/app/notify_client/email_branding_client.py index ae2375144..0349c3f84 100644 --- a/app/notify_client/email_branding_client.py +++ b/app/notify_client/email_branding_client.py @@ -18,20 +18,26 @@ class EmailBrandingClient(NotifyAdminAPIClient): def get_letter_email_branding(self): return self.get(url='/dvla_organisations') - def create_email_branding(self, logo, name, text, colour): + def create_email_branding(self, logo, name, text, colour, banner_colour, single_id_colour, domain): data = { "logo": logo, "name": name, "text": text, - "colour": colour + "colour": colour, + "banner_colour": banner_colour, + "single_id_colour": single_id_colour, + "domain": domain, } return self.post(url="/email-branding", data=data) - def update_email_branding(self, branding_id, logo, name, text, colour): + def update_email_branding(self, branding_id, logo, name, text, colour, banner_colour, single_id_colour, domain): data = { "logo": logo, "name": name, "text": text, - "colour": colour + "colour": colour, + "banner_colour": banner_colour, + "single_id_colour": single_id_colour, + "domain": domain, } return self.post(url="/email-branding/{}".format(branding_id), data=data) diff --git a/app/templates/views/email-branding/manage-branding.html b/app/templates/views/email-branding/manage-branding.html index d8927ad65..8b628a9a4 100644 --- a/app/templates/views/email-branding/manage-branding.html +++ b/app/templates/views/email-branding/manage-branding.html @@ -12,7 +12,6 @@

{{ '{} email branding'.format('Update' if email_branding else 'Create')}}

- {% if logo %}
@@ -24,6 +23,9 @@
{{textbox(form.name)}}
{{textbox(form.text)}}
{{colour_textbox(form.colour, width='1-4', colour=email_branding.colour if email_branding)}} + {{colour_textbox(form.banner_colour, width='1-4', colour=email_branding.banner_colour if email_branding)}} + {{colour_textbox(form.single_id_colour, width='1-4', colour=email_branding.single_id_colour if email_branding)}} +
{{textbox(form.domain)}}
{{ page_footer( 'Save', back_link=url_for('.email_branding'), diff --git a/tests/app/main/views/test_email_branding.py b/tests/app/main/views/test_email_branding.py index a5b88bc80..9bef00beb 100644 --- a/tests/app/main/views/test_email_branding.py +++ b/tests/app/main/views/test_email_branding.py @@ -51,6 +51,9 @@ def test_edit_email_branding_shows_the_correct_branding_info( assert page.select_one('#name').attrs.get('value') == 'Organisation name' assert page.select_one('#text').attrs.get('value') == 'Organisation text' assert page.select_one('#colour').attrs.get('value') == '#f00' + assert page.select_one('#banner_colour').attrs.get('value') == '#f11' + assert page.select_one('#single_id_colour').attrs.get('value') == '#f22' + assert page.select_one('#domain').attrs.get('value') == 'sample.com' def test_create_email_branding_does_not_show_any_branding_info( @@ -69,6 +72,9 @@ def test_create_email_branding_does_not_show_any_branding_info( assert page.select_one('#name').attrs.get('value') == '' assert page.select_one('#text').attrs.get('value') == '' assert page.select_one('#colour').attrs.get('value') == '' + assert page.select_one('#banner_colour').attrs.get('value') == '' + assert page.select_one('#single_id_colour').attrs.get('value') == '' + assert page.select_one('#domain').attrs.get('value') == '' def test_create_new_email_branding_without_logo( @@ -82,6 +88,9 @@ def test_create_new_email_branding_without_logo( 'colour': '#ff0000', 'text': 'new text', 'name': 'new name', + 'domain': 'sample.com', + 'banner_colour': '#FFFF00', + 'single_id_colour': '#00FF00', } mock_persist = mocker.patch('app.main.views.email_branding.persist_logo') @@ -98,7 +107,10 @@ def test_create_new_email_branding_without_logo( logo=data['logo'], name=data['name'], text=data['text'], - colour=data['colour'] + colour=data['colour'], + banner_colour=data['banner_colour'], + single_id_colour=data['single_id_colour'], + domain=data['domain'] ) assert mock_persist.call_args_list == [] @@ -117,6 +129,9 @@ def test_create_new_email_branding_when_branding_saved( 'colour': '#ff0000', 'text': 'new text', 'name': 'new name', + 'domain': 'sample.com', + 'banner_colour': '#FFFF00', + 'single_id_colour': '#00FF00', } temp_filename = LOGO_LOCATION_STRUCTURE.format( @@ -135,7 +150,10 @@ def test_create_new_email_branding_when_branding_saved( 'colour': data['colour'], 'name': data['name'], 'text': data['text'], - 'cdn_url': 'https://static-logos.cdn.com' + 'cdn_url': 'https://static-logos.cdn.com', + 'domain': data['domain'], + 'banner_colour': data['banner_colour'], + 'single_id_colour': data['single_id_colour'], } ) @@ -144,7 +162,10 @@ def test_create_new_email_branding_when_branding_saved( logo=data['logo'], name=data['name'], text=data['text'], - colour=data['colour'] + colour=data['colour'], + banner_colour=data['banner_colour'], + single_id_colour=data['single_id_colour'], + domain=data['domain'] ) @@ -195,7 +216,7 @@ def test_deletes_previous_temp_logo_after_uploading_logo( assert mocked_delete_temp_file.call_args == call(temp_old_filename) -def test_update_exisiting_branding( +def test_update_existing_branding( logged_in_platform_admin_client, mocker, fake_uuid, @@ -209,7 +230,10 @@ def test_update_exisiting_branding( 'logo': 'test.png', 'colour': '#0000ff', 'text': 'new text', - 'name': 'new name' + 'name': 'new name', + 'banner_colour': '#FFFF00', + 'single_id_colour': '#00FF00', + 'domain': 'sample.com', } temp_filename = LOGO_LOCATION_STRUCTURE.format( @@ -225,7 +249,10 @@ def test_update_exisiting_branding( url_for('.update_email_branding', logo=temp_filename, branding_id=fake_uuid), content_type='multipart/form-data', data={'colour': data['colour'], 'name': data['name'], 'text': data['text'], - 'cdn_url': 'https://static-logos.cdn.com'} + 'cdn_url': 'https://static-logos.cdn.com', + 'banner_colour': data['banner_colour'], 'single_id_colour': data['single_id_colour'], + 'domain': data['domain'] + } ) assert mock_update_email_branding.called @@ -234,7 +261,10 @@ def test_update_exisiting_branding( logo=data['logo'], name=data['name'], text=data['text'], - colour=data['colour'] + colour=data['colour'], + banner_colour=data['banner_colour'], + single_id_colour=data['single_id_colour'], + domain=data['domain'], ) @@ -316,7 +346,10 @@ def test_colour_regex_validation( 'logo': None, 'colour': colour_hex, 'text': 'new text', - 'name': 'new name' + 'name': 'new name', + 'domain': 'sample.com', + 'banner_colour': '#FFFF00', + 'single_id_colour': '#00FF00', } mocker.patch('app.main.views.email_branding.delete_temp_files_created_by') diff --git a/tests/app/main/views/test_email_preview.py b/tests/app/main/views/test_email_preview.py index e1f819f52..fd14de170 100644 --- a/tests/app/main/views/test_email_preview.py +++ b/tests/app/main/views/test_email_preview.py @@ -4,6 +4,8 @@ import pytest from bs4 import BeautifulSoup from flask import url_for +from app.main.views.index import _set_colour + @pytest.mark.parametrize( "query_args, result", [ @@ -86,7 +88,7 @@ def test_displays_org_branding_with_banner(client, mock_get_email_branding): assert not page.find("a", attrs={"href": "https://www.gov.uk"}) assert page.find("img", attrs={"src": re.compile("example.png")}) - assert page.select("body > table > tr > td[bgcolor='#f00']") # banner colour is set + assert page.select("body > table > tr > td[bgcolor='#f11']") # banner colour is set assert page.select("body > table table > tr > td > span")[0]\ .get_text().strip() == 'Organisation text' # brand text is set @@ -104,5 +106,28 @@ def test_displays_org_branding_with_banner_without_brand_text( assert not page.find("a", attrs={"href": "https://www.gov.uk"}) assert page.find("img", attrs={"src": re.compile("example.png")}) - assert page.select("body > table > tr > td[bgcolor='#f00']") # banner colour is set + assert page.select("body > table > tr > td[bgcolor='#f11']") # banner colour is set assert not page.select("body > table table > tr > td > span") == 0 # brand text is not set + + +@pytest.mark.parametrize('colour, banner_colour, single_id_colour, branding_type, expected_colour', [ + ('black', 'yellow', 'red', 'org', 'red'), + ('black', 'yellow', None, 'org', 'black'), + ('black', 'yellow', 'red', 'org_banner', 'yellow'), + ('black', None, 'red', 'org_banner', 'black'), + ('black', 'yellow', 'red', 'govuk', None), + ('black', 'yellow', 'red', 'both', 'red'), + ('black', 'yellow', None, 'both', 'black'), +]) +def test_set_colour(colour, banner_colour, single_id_colour, branding_type, expected_colour): + email_branding = { + 'logo': None, + 'colour': colour, + 'text': 'new text', + 'name': 'new name', + 'domain': 'sample.com', + 'banner_colour': banner_colour, + 'single_id_colour': single_id_colour, + } + colour = _set_colour(branding_type, email_branding) + assert colour == expected_colour diff --git a/tests/app/notify_client/test_email_branding_client.py b/tests/app/notify_client/test_email_branding_client.py index 7a1fcd8bb..a6dbb29f5 100644 --- a/tests/app/notify_client/test_email_branding_client.py +++ b/tests/app/notify_client/test_email_branding_client.py @@ -26,11 +26,15 @@ def test_get_letter_email_branding(mocker): def test_create_email_branding(mocker): - org_data = {'logo': 'test.png', 'name': 'test name', 'text': 'test name', 'colour': 'red'} + org_data = {'logo': 'test.png', 'name': 'test name', 'text': 'test name', 'colour': 'red', + 'banner_colour': 'blue', 'single_id_colour': 'yellow', 'domain': 'sample.com'} mock_post = mocker.patch('app.notify_client.email_branding_client.EmailBrandingClient.post') EmailBrandingClient().create_email_branding( - logo=org_data['logo'], name=org_data['name'], text=org_data['text'], colour=org_data['colour']) + logo=org_data['logo'], name=org_data['name'], text=org_data['text'], colour=org_data['colour'], + banner_colour=org_data['banner_colour'], single_id_colour=org_data['single_id_colour'], + domain=org_data['domain'] + ) mock_post.assert_called_once_with( url='/email-branding', @@ -39,12 +43,14 @@ def test_create_email_branding(mocker): def test_update_email_branding(mocker, fake_uuid): - org_data = {'logo': 'test.png', 'name': 'test name', 'text': 'test name', 'colour': 'red'} + org_data = {'logo': 'test.png', 'name': 'test name', 'text': 'test name', 'colour': 'red', + 'banner_colour': 'blue', 'single_id_colour': 'yellow', 'domain': 'sample.com'} mock_post = mocker.patch('app.notify_client.email_branding_client.EmailBrandingClient.post') EmailBrandingClient().update_email_branding( branding_id=fake_uuid, logo=org_data['logo'], name=org_data['name'], text=org_data['text'], - colour=org_data['colour']) + colour=org_data['colour'], banner_colour=org_data['banner_colour'], + single_id_colour=org_data['single_id_colour'], domain=org_data['domain']) mock_post.assert_called_once_with( url='/email-branding/{}'.format(fake_uuid), diff --git a/tests/conftest.py b/tests/conftest.py index a056d43b2..492c5675d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2493,7 +2493,10 @@ def mock_get_email_branding(mocker, fake_uuid): 'name': 'Organisation name', 'text': 'Organisation text', 'id': fake_uuid, - 'colour': '#f00' + 'colour': '#f00', + 'banner_colour': '#f11', + 'single_id_colour': '#f22', + 'domain': 'sample.com', } } @@ -2511,7 +2514,9 @@ def mock_get_email_branding_without_brand_text(mocker, fake_uuid): 'name': 'Organisation name', 'text': '', 'id': fake_uuid, - 'colour': '#f00' + 'colour': '#f00', + 'banner_colour': '#f11', + 'single_id_colour': '#f22' } } @@ -2523,7 +2528,7 @@ def mock_get_email_branding_without_brand_text(mocker, fake_uuid): @pytest.fixture(scope='function') def mock_create_email_branding(mocker): - def _create_email_branding(logo, name, text, colour): + def _create_email_branding(logo, name, text, colour, banner_colour, single_id_colour, domain): return return mocker.patch( @@ -2533,7 +2538,7 @@ def mock_create_email_branding(mocker): @pytest.fixture(scope='function') def mock_update_email_branding(mocker): - def _update_email_branding(branding_id, logo, name, text, colour): + def _update_email_branding(branding_id, logo, name, text, colour, banner_colour, single_id_colour, domain): return return mocker.patch(