Merge pull request #2249 from alphagov/add-brand-type-to-email-branding

Add brand type to email branding.
This commit is contained in:
Rebecca Law
2018-08-24 13:50:36 +01:00
committed by GitHub
9 changed files with 49 additions and 108 deletions

View File

@@ -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):

View File

@@ -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'))

View File

@@ -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'])

View File

@@ -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)

View File

@@ -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 @@
<div style='margin-top:15px;'>{{textbox(form.name)}}</div>
<div style='margin-top:15px;'>{{textbox(form.text)}}</div>
{{ 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) }}
<div style='margin-top:15px;'>{{textbox(form.domain)}}</div>
{{ radios(form.brand_type) }}
{{ page_footer(
'Save',
back_link=url_for('.email_branding'),

View File

@@ -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

View File

@@ -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

View File

@@ -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),

View File

@@ -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(