diff --git a/app/main/forms.py b/app/main/forms.py
index b8d769a4a..f9897cf34 100644
--- a/app/main/forms.py
+++ b/app/main/forms.py
@@ -714,7 +714,6 @@ class ServicePreviewBranding(StripWhitespaceForm):
class ServiceUpdateEmailBranding(StripWhitespaceForm):
-
name = StringField('Name of brand')
text = StringField('Text')
domain = StringField('Domain')
@@ -724,19 +723,16 @@ class ServiceUpdateEmailBranding(StripWhitespaceForm):
Regexp(regex="^$|^#(?:[0-9a-fA-F]{3}){1,2}$", message='Must be a valid color hex code')
]
)
- banner_colour = StringField(
- 'Banner colour',
- 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',
- validators=[
- 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!')])
+ brand_type = RadioField(
+ "Brand type",
+ choices=[
+ ('govuk', 'GOV.UK only'),
+ ('both', 'GOV.UK and branding'),
+ ('org', 'Branding only'),
+ ('org_banner', 'Branding banner'),
+ ]
+ )
class CreateOrUpdateOrganisation(StripWhitespaceForm):
diff --git a/app/main/views/email_branding.py b/app/main/views/email_branding.py
index 819a61ac7..787b59f50 100644
--- a/app/main/views/email_branding.py
+++ b/app/main/views/email_branding.py
@@ -36,7 +36,13 @@ def email_branding():
def update_email_branding(branding_id, logo=None):
email_branding = email_branding_client.get_email_branding(branding_id)['email_branding']
- form = ServiceUpdateEmailBranding()
+ form = ServiceUpdateEmailBranding(
+ name=email_branding['name'],
+ text=email_branding['text'],
+ colour=email_branding['colour'],
+ domain=email_branding['domain'],
+ brand_type=email_branding['brand_type']
+ )
logo = logo if logo else email_branding.get('logo') if email_branding else None
@@ -65,20 +71,12 @@ def update_email_branding(branding_id, logo=None):
name=form.name.data,
text=form.text.data,
colour=form.colour.data,
- banner_colour=form.banner_colour.data,
- single_id_colour=form.single_id_colour.data,
domain=form.domain.data,
+ brand_type=form.brand_type.data,
)
return redirect(url_for('.email_branding', branding_id=branding_id))
- 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',
form=form,
@@ -93,7 +91,7 @@ def update_email_branding(branding_id, logo=None):
@login_required
@user_is_platform_admin
def create_email_branding(logo=None):
- form = ServiceUpdateEmailBranding()
+ form = ServiceUpdateEmailBranding(brand_type='govuk')
if form.validate_on_submit():
if form.file.data:
@@ -119,9 +117,8 @@ def create_email_branding(logo=None):
name=form.name.data,
text=form.text.data,
colour=form.colour.data,
- banner_colour=form.banner_colour.data,
- single_id_colour=form.single_id_colour.data,
- domain=form.domain.data
+ domain=form.domain.data,
+ brand_type=form.brand_type.data,
)
return redirect(url_for('.email_branding'))
diff --git a/app/main/views/index.py b/app/main/views/index.py
index 3b4232825..ed07c2a15 100644
--- a/app/main/views/index.py
+++ b/app/main/views/index.py
@@ -78,13 +78,6 @@ 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')
@@ -98,7 +91,7 @@ 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)
+ colour = email_branding['colour']
brand_name = email_branding['text']
brand_colour = colour
brand_logo = 'https://{}/{}'.format(get_cdn_domain(), email_branding['logo'])
diff --git a/app/notify_client/email_branding_client.py b/app/notify_client/email_branding_client.py
index 0349c3f84..f847f030c 100644
--- a/app/notify_client/email_branding_client.py
+++ b/app/notify_client/email_branding_client.py
@@ -18,26 +18,24 @@ class EmailBrandingClient(NotifyAdminAPIClient):
def get_letter_email_branding(self):
return self.get(url='/dvla_organisations')
- def create_email_branding(self, logo, name, text, colour, banner_colour, single_id_colour, domain):
+ def create_email_branding(self, logo, name, text, colour, domain, brand_type):
data = {
"logo": logo,
"name": name,
"text": text,
"colour": colour,
- "banner_colour": banner_colour,
- "single_id_colour": single_id_colour,
"domain": domain,
+ "brand_type": brand_type
}
return self.post(url="/email-branding", data=data)
- def update_email_branding(self, branding_id, logo, name, text, colour, banner_colour, single_id_colour, domain):
+ def update_email_branding(self, branding_id, logo, name, text, colour, domain, brand_type):
data = {
"logo": logo,
"name": name,
"text": text,
"colour": colour,
- "banner_colour": banner_colour,
- "single_id_colour": single_id_colour,
"domain": domain,
+ "brand_type": brand_type
}
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 e48c98d81..5b303cc01 100644
--- a/app/templates/views/email-branding/manage-branding.html
+++ b/app/templates/views/email-branding/manage-branding.html
@@ -2,6 +2,7 @@
{% from "components/file-upload.html" import file_upload %}
{% from "components/page-footer.html" import page_footer %}
{% from "components/textbox.html" import textbox %}
+{% from "components/radios.html" import radios %}
{% block service_page_title %}
{{ '{} email branding'.format('Update' if email_branding else 'Create')}}
@@ -23,9 +24,8 @@
{{textbox(form.name)}}
{{textbox(form.text)}}
{{ textbox(form.colour, width='1-4', colour_preview=True) }}
- {{ textbox(form.banner_colour, width='1-4', colour_preview=True) }}
- {{ textbox(form.single_id_colour, width='1-4', colour_preview=True) }}
{{textbox(form.domain)}}
+ {{ radios(form.brand_type) }}
{{ 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 d9c3f5fe1..889399428 100644
--- a/tests/app/main/views/test_email_branding.py
+++ b/tests/app/main/views/test_email_branding.py
@@ -58,8 +58,6 @@ 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'
@@ -79,8 +77,6 @@ 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') == ''
@@ -96,8 +92,7 @@ def test_create_new_email_branding_without_logo(
'text': 'new text',
'name': 'new name',
'domain': 'sample.com',
- 'banner_colour': '#FFFF00',
- 'single_id_colour': '#00FF00',
+ 'brand_type': 'govuk'
}
mock_persist = mocker.patch('app.main.views.email_branding.persist_logo')
@@ -115,9 +110,8 @@ def test_create_new_email_branding_without_logo(
name=data['name'],
text=data['text'],
colour=data['colour'],
- banner_colour=data['banner_colour'],
- single_id_colour=data['single_id_colour'],
- domain=data['domain']
+ domain=data['domain'],
+ brand_type=data['brand_type']
)
assert mock_persist.call_args_list == []
@@ -137,8 +131,7 @@ def test_create_new_email_branding_when_branding_saved(
'text': 'new text',
'name': 'new name',
'domain': 'sample.com',
- 'banner_colour': '#FFFF00',
- 'single_id_colour': '#00FF00',
+ 'brand_type': 'govuk'
}
temp_filename = LOGO_LOCATION_STRUCTURE.format(
@@ -159,8 +152,7 @@ def test_create_new_email_branding_when_branding_saved(
'text': data['text'],
'cdn_url': 'https://static-logos.cdn.com',
'domain': data['domain'],
- 'banner_colour': data['banner_colour'],
- 'single_id_colour': data['single_id_colour'],
+ 'brand_type': data['brand_type']
}
)
@@ -170,9 +162,8 @@ def test_create_new_email_branding_when_branding_saved(
name=data['name'],
text=data['text'],
colour=data['colour'],
- banner_colour=data['banner_colour'],
- single_id_colour=data['single_id_colour'],
- domain=data['domain']
+ domain=data['domain'],
+ brand_type=data['brand_type']
)
@@ -238,9 +229,8 @@ def test_update_existing_branding(
'colour': '#0000ff',
'text': 'new text',
'name': 'new name',
- 'banner_colour': '#FFFF00',
- 'single_id_colour': '#00FF00',
'domain': 'sample.com',
+ 'brand_type': 'govuk'
}
temp_filename = LOGO_LOCATION_STRUCTURE.format(
@@ -257,8 +247,7 @@ def test_update_existing_branding(
content_type='multipart/form-data',
data={'colour': data['colour'], 'name': data['name'], 'text': data['text'],
'cdn_url': 'https://static-logos.cdn.com',
- 'banner_colour': data['banner_colour'], 'single_id_colour': data['single_id_colour'],
- 'domain': data['domain']
+ 'domain': data['domain'], 'brand_type': data['brand_type']
}
)
@@ -269,9 +258,8 @@ def test_update_existing_branding(
name=data['name'],
text=data['text'],
colour=data['colour'],
- banner_colour=data['banner_colour'],
- single_id_colour=data['single_id_colour'],
domain=data['domain'],
+ brand_type=data['brand_type']
)
@@ -326,7 +314,6 @@ def test_logo_persisted_when_organisation_saved(
url_for('.create_email_branding', logo=temp_filename),
content_type='multipart/form-data'
)
-
assert resp.status_code == 302
assert not mocked_upload_logo.called
@@ -355,8 +342,7 @@ def test_colour_regex_validation(
'text': 'new text',
'name': 'new name',
'domain': 'sample.com',
- 'banner_colour': '#FFFF00',
- 'single_id_colour': '#00FF00',
+ 'brand_type': 'govuk'
}
mocker.patch('app.main.views.email_branding.delete_temp_files_created_by')
@@ -366,5 +352,4 @@ def test_colour_regex_validation(
content_type='multipart/form-data',
data=data
)
-
assert response.status_code == expected_status_code
diff --git a/tests/app/main/views/test_email_preview.py b/tests/app/main/views/test_email_preview.py
index fd14de170..e1f819f52 100644
--- a/tests/app/main/views/test_email_preview.py
+++ b/tests/app/main/views/test_email_preview.py
@@ -4,8 +4,6 @@ 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", [
@@ -88,7 +86,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='#f11']") # banner colour is set
+ assert page.select("body > table > tr > td[bgcolor='#f00']") # banner colour is set
assert page.select("body > table table > tr > td > span")[0]\
.get_text().strip() == 'Organisation text' # brand text is set
@@ -106,28 +104,5 @@ 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='#f11']") # banner colour is set
+ assert page.select("body > table > tr > td[bgcolor='#f00']") # 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 a6dbb29f5..61a95683d 100644
--- a/tests/app/notify_client/test_email_branding_client.py
+++ b/tests/app/notify_client/test_email_branding_client.py
@@ -27,13 +27,12 @@ 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',
- 'banner_colour': 'blue', 'single_id_colour': 'yellow', 'domain': 'sample.com'}
+ 'domain': 'sample.com', 'brand_type': 'org'}
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'],
- banner_colour=org_data['banner_colour'], single_id_colour=org_data['single_id_colour'],
- domain=org_data['domain']
+ domain=org_data['domain'], brand_type='org'
)
mock_post.assert_called_once_with(
@@ -44,13 +43,12 @@ 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',
- 'banner_colour': 'blue', 'single_id_colour': 'yellow', 'domain': 'sample.com'}
+ 'domain': 'sample.com', 'brand_type': 'org'}
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'], banner_colour=org_data['banner_colour'],
- single_id_colour=org_data['single_id_colour'], domain=org_data['domain'])
+ colour=org_data['colour'], domain=org_data['domain'], brand_type='org')
mock_post.assert_called_once_with(
url='/email-branding/{}'.format(fake_uuid),
diff --git a/tests/conftest.py b/tests/conftest.py
index 4a33813cb..5ed2681da 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -2445,6 +2445,7 @@ def create_email_brandings(number_of_brandings, non_standard_values={}, shuffle=
'text': 'org {}'.format(idx),
'colour': None,
'logo': 'logo{}.png'.format(idx),
+ 'brand_type': 'org',
} for idx in range(1, number_of_brandings + 1)]
for idx, row in enumerate(non_standard_values):
@@ -2528,9 +2529,8 @@ def mock_get_email_branding(mocker, fake_uuid):
'text': 'Organisation text',
'id': fake_uuid,
'colour': '#f00',
- 'banner_colour': '#f11',
- 'single_id_colour': '#f22',
'domain': 'sample.com',
+ 'brand_type': 'org',
}
}
@@ -2549,8 +2549,7 @@ def mock_get_email_branding_without_brand_text(mocker, fake_uuid):
'text': '',
'id': fake_uuid,
'colour': '#f00',
- 'banner_colour': '#f11',
- 'single_id_colour': '#f22'
+ 'brand_type': 'org_banner'
}
}
@@ -2562,7 +2561,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, banner_colour, single_id_colour, domain):
+ def _create_email_branding(logo, name, text, colour, domain, brand_type):
return
return mocker.patch(
@@ -2572,7 +2571,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, banner_colour, single_id_colour, domain):
+ def _update_email_branding(branding_id, logo, name, text, colour, domain, brand_type):
return
return mocker.patch(