From 0df88ea182ef380ae8a747fedbd4b6dd75ecc20d Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Wed, 29 Aug 2018 11:36:50 +0100 Subject: [PATCH 1/9] Remove branding_type from set_email_branding page Includes updates to the controller so branding_type is got from the email_branding model instead of from user input. Follows on from: https://github.com/alphagov/notifications-admin/pull/2249 --- app/assets/javascripts/emailPreviewPane.js | 15 +++++---------- app/main/forms.py | 18 ------------------ app/main/views/service_settings.py | 9 +++------ .../preview-email-branding.html | 2 +- .../service-settings/set-email-branding.html | 7 ++----- 5 files changed, 11 insertions(+), 40 deletions(-) diff --git a/app/assets/javascripts/emailPreviewPane.js b/app/assets/javascripts/emailPreviewPane.js index 7e42e99ba..5871118a3 100644 --- a/app/assets/javascripts/emailPreviewPane.js +++ b/app/assets/javascripts/emailPreviewPane.js @@ -5,18 +5,16 @@ const root = this, $ = this.jQuery; - let branding_type = $('.multiple-choice input[name="branding_type"]:checked'); let branding_style = $('.multiple-choice input[name="branding_style"]:checked'); - if (!branding_type.length || !branding_style.length) { return; } + if (!branding_style.length) { return; } - branding_type = branding_type.val(); branding_style = branding_style.val(); const $paneWrapper = $('
'); const $form = $('form'); - const $previewPane = $(''); function buildQueryString () { @@ -25,13 +23,10 @@ function setPreviewPane (e) { const $target = $(e.target); - if ($target.attr('name') == 'branding_type') { - branding_type = $target.val(); - } if ($target.attr('name') == 'branding_style') { branding_style = $target.val(); } - $previewPane.attr('src', '/_email?' + buildQueryString(['branding_type', branding_type], ['branding_style', branding_style])); + $previewPane.attr('src', '/_email?' + buildQueryString(['branding_style', branding_style])); } $paneWrapper.append($previewPane); @@ -39,5 +34,5 @@ $form.attr('action', location.pathname.replace(/set-email-branding$/, 'preview-email-branding')); $form.find('button[type="submit"]').text('Save'); - $('fieldset').on('change', 'input[name="branding_type"], input[name="branding_style"]', setPreviewPane); + $('fieldset').on('change', 'input[name="branding_style"]', setPreviewPane); })(); diff --git a/app/main/forms.py b/app/main/forms.py index f9897cf34..eea0a3110 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -682,23 +682,6 @@ class ServiceSwitchLettersForm(StripWhitespaceForm): class ServiceSetBranding(StripWhitespaceForm): - def __init__(self, email_branding=[], *args, **kwargs): - self.branding_style.choices = email_branding - super(ServiceSetBranding, self).__init__(*args, **kwargs) - - branding_type = RadioField( - 'Branding type', - choices=[ - ('govuk', 'GOV.UK only'), - ('both', 'GOV.UK and branding'), - ('org', 'Branding only'), - ('org_banner', 'Branding banner') - ], - validators=[ - DataRequired() - ] - ) - branding_style = RadioField( 'Branding style', validators=[ @@ -709,7 +692,6 @@ class ServiceSetBranding(StripWhitespaceForm): class ServicePreviewBranding(StripWhitespaceForm): - branding_type = HiddenField('branding_type') branding_style = HiddenField('branding_style') diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index e7c87f6a7..85a7b094c 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -868,9 +868,8 @@ def set_free_sms_allowance(service_id): @user_is_platform_admin def service_set_email_branding(service_id): email_branding = email_branding_client.get_all_email_branding() - branding_type = current_service.get('branding') - form = ServiceSetBranding(branding_type=branding_type) + form = ServiceSetBranding() # dynamically create org choices, including the null option email_brandings = sorted(get_branding_as_value_and_label(email_branding), @@ -880,7 +879,7 @@ def service_set_email_branding(service_id): if form.validate_on_submit(): branding_style = None if form.branding_style.data == 'None' else form.branding_style.data return redirect(url_for('.service_preview_email_branding', service_id=service_id, - branding_type=form.branding_type.data, branding_style=branding_style)) + branding_style=branding_style)) form.branding_style.data = current_service['email_branding'] or 'None' @@ -897,16 +896,14 @@ def service_set_email_branding(service_id): @login_required @user_is_platform_admin def service_preview_email_branding(service_id): - branding_type = request.args.get('branding_type', None) branding_style = request.args.get('branding_style', None) - form = ServicePreviewBranding(branding_type=branding_type, branding_style=branding_style) + form = ServicePreviewBranding(branding_style=branding_style) if form.validate_on_submit(): branding_style = None if form.branding_style.data == 'None' else form.branding_style.data service_api_client.update_service( service_id, - branding=form.branding_type.data, email_branding=branding_style ) return redirect(url_for('.service_settings', service_id=service_id)) diff --git a/app/templates/views/service-settings/preview-email-branding.html b/app/templates/views/service-settings/preview-email-branding.html index 51ecdb7e2..917a0bc44 100644 --- a/app/templates/views/service-settings/preview-email-branding.html +++ b/app/templates/views/service-settings/preview-email-branding.html @@ -9,7 +9,7 @@

Preview email branding

- +
{{ form.hidden_tag() }} diff --git a/app/templates/views/service-settings/set-email-branding.html b/app/templates/views/service-settings/set-email-branding.html index bbdd7d86c..ee8d8d73e 100644 --- a/app/templates/views/service-settings/set-email-branding.html +++ b/app/templates/views/service-settings/set-email-branding.html @@ -12,14 +12,11 @@

Set email branding

-
+
-
- {{ radios(form.branding_type) }} -
-
+
{{ live_search(target_selector='.brand_styles .multiple-choice', show=show_search_box, form=search_form, label='Search branding styles by name') }} {{ radios(form.branding_style) }}
From 151488fa784242f5de342aaff39800a96617ed37 Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Wed, 29 Aug 2018 11:38:40 +0100 Subject: [PATCH 2/9] Update email preview page No longer requires the branding_type GET param. Retains the 'govuk' default brand. --- app/assets/javascripts/emailPreviewPane.js | 2 +- app/main/views/index.py | 11 +++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/emailPreviewPane.js b/app/assets/javascripts/emailPreviewPane.js index 5871118a3..0509efa27 100644 --- a/app/assets/javascripts/emailPreviewPane.js +++ b/app/assets/javascripts/emailPreviewPane.js @@ -13,7 +13,7 @@ const $paneWrapper = $('
'); const $form = $('form'); - const $previewPane = $(''); diff --git a/app/main/views/index.py b/app/main/views/index.py index ed07c2a15..685cb77f9 100644 --- a/app/main/views/index.py +++ b/app/main/views/index.py @@ -80,17 +80,20 @@ def design_content(): @main.route('/_email') def email_template(): - branding_type = request.args.get('branding_type', 'govuk') - branding_style = request.args.get('branding_style', 'None') + branding_type = 'govuk' + branding_style = request.args.get('branding_style', None) - if branding_type == 'govuk' or branding_style == 'None': + if branding_style: + email_branding = email_branding_client.get_email_branding(branding_style)['email_branding'] + branding_type = email_branding['brand_type'] + + if branding_type == 'govuk': brand_name = None brand_colour = None brand_logo = None govuk_banner = True brand_banner = False else: - email_branding = email_branding_client.get_email_branding(branding_style)['email_branding'] colour = email_branding['colour'] brand_name = email_branding['text'] brand_colour = colour From 55f5cc8e1ea59a5b839aac11b021a8a3e90ed10f Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Wed, 29 Aug 2018 11:40:55 +0100 Subject: [PATCH 3/9] Update tests for changes Removes any checks for branding_type from tests for set_email_branding page. Updates tests for email preview page (usually iframed) so non-default brand_type is got from the email_branding model instead of a GET param. --- tests/app/main/views/test_email_preview.py | 25 +++---- tests/app/main/views/test_service_settings.py | 33 +-------- tests/conftest.py | 70 +++++++++++++------ 3 files changed, 64 insertions(+), 64 deletions(-) diff --git a/tests/app/main/views/test_email_preview.py b/tests/app/main/views/test_email_preview.py index e1f819f52..55421cb7f 100644 --- a/tests/app/main/views/test_email_preview.py +++ b/tests/app/main/views/test_email_preview.py @@ -32,9 +32,9 @@ def test_displays_govuk_branding_by_default(client): assert page.find("a", attrs={"href": "https://www.gov.uk"}) -def test_displays_govuk_branding(client): +def test_displays_govuk_branding(client, mock_get_email_branding_with_govuk_brand_type): - response = client.get(url_for('main.email_template', branding_type="govuk", branding_style="1")) + response = client.get(url_for('main.email_template', branding_style="1")) page = BeautifulSoup(response.data.decode("utf-8"), "html.parser") @@ -43,14 +43,14 @@ def test_displays_govuk_branding(client): assert page.find("a", attrs={"href": "https://www.gov.uk"}) -def test_displays_both_branding(client, mock_get_email_branding): +def test_displays_both_branding(client, mock_get_email_branding_with_both_brand_type): - response = client.get(url_for('main.email_template', branding_type="both", branding_style="1")) + response = client.get(url_for('main.email_template', branding_style="1")) page = BeautifulSoup(response.data.decode("utf-8"), "html.parser") assert response.status_code == 200 - mock_get_email_branding.assert_called_once_with('1') + mock_get_email_branding_with_both_brand_type.assert_called_once_with('1') assert page.find("a", attrs={"href": "https://www.gov.uk"}) assert page.find("img", attrs={"src": re.compile("example.png$")}) @@ -60,7 +60,8 @@ def test_displays_both_branding(client, mock_get_email_branding): def test_displays_org_branding(client, mock_get_email_branding): - response = client.get(url_for('main.email_template', branding_type="org", branding_style="1")) + # mock_get_email_branding has 'brand_type' of 'org' + response = client.get(url_for('main.email_template', branding_style="1")) page = BeautifulSoup(response.data.decode("utf-8"), "html.parser") @@ -74,15 +75,15 @@ def test_displays_org_branding(client, mock_get_email_branding): .get_text().strip() == 'Organisation text' # brand text is set -def test_displays_org_branding_with_banner(client, mock_get_email_branding): +def test_displays_org_branding_with_banner( + client, mock_get_email_branding_with_org_banner_brand_type): - response = client.get(url_for('main.email_template', branding_type="org_banner", - branding_style="1")) + response = client.get(url_for('main.email_template', branding_style="1")) page = BeautifulSoup(response.data.decode("utf-8"), "html.parser") assert response.status_code == 200 - mock_get_email_branding.assert_called_once_with('1') + mock_get_email_branding_with_org_banner_brand_type.assert_called_once_with('1') assert not page.find("a", attrs={"href": "https://www.gov.uk"}) assert page.find("img", attrs={"src": re.compile("example.png")}) @@ -94,8 +95,8 @@ def test_displays_org_branding_with_banner(client, mock_get_email_branding): def test_displays_org_branding_with_banner_without_brand_text( client, mock_get_email_branding_without_brand_text): - response = client.get(url_for('main.email_template', branding_type="org_banner", - branding_style="1")) + # mock_get_email_branding_without_brand_text has 'brand_type' of 'org_banner' + response = client.get(url_for('main.email_template', branding_style="1")) page = BeautifulSoup(response.data.decode("utf-8"), "html.parser") diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index b87f834bd..d3b85ca05 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -1718,31 +1718,6 @@ def test_set_letter_branding_saves( mock_update_service.assert_called_once_with(service_one['id'], dvla_organisation='500') -def test_should_show_branding_types( - logged_in_platform_admin_client, - service_one, - mock_get_all_email_branding, -): - response = logged_in_platform_admin_client.get(url_for( - 'main.service_set_email_branding', service_id=service_one['id'] - )) - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - - assert page.find('input', attrs={"id": "branding_type-0"})['value'] == 'govuk' - assert page.find('input', attrs={"id": "branding_type-1"})['value'] == 'both' - assert page.find('input', attrs={"id": "branding_type-2"})['value'] == 'org' - assert page.find('input', attrs={"id": "branding_type-3"})['value'] == 'org_banner' - - assert 'checked' in page.find('input', attrs={"id": "branding_type-0"}).attrs - assert 'checked' not in page.find('input', attrs={"id": "branding_type-1"}).attrs - assert 'checked' not in page.find('input', attrs={"id": "branding_type-2"}).attrs - assert 'checked' not in page.find('input', attrs={"id": "branding_type-3"}).attrs - - app.email_branding_client.get_all_email_branding.assert_called_once_with() - app.service_api_client.get_service.assert_called_once_with(service_one['id']) - - def test_should_show_branding_styles( logged_in_platform_admin_client, service_one, @@ -1827,8 +1802,8 @@ def test_should_send_branding_and_organisations_to_preview( ) assert response.status_code == 302 assert response.location == url_for('main.service_preview_email_branding', - service_id=service_one['id'], branding_type='org', - branding_style='1', _external=True) + service_id=service_one['id'], branding_style='1', + _external=True) mock_get_all_email_branding.assert_called_once_with() @@ -1847,10 +1822,8 @@ def test_should_preview_email_branding( iframeURLComponents = urlparse(iframe['src']) iframeQString = parse_qs(iframeURLComponents.query) - assert page.find('input', attrs={"id": "branding_type"})['value'] == 'org' assert page.find('input', attrs={"id": "branding_style"})['value'] == '1' assert iframeURLComponents.path == '/_email' - assert iframeQString['branding_type'] == ['org'] assert iframeQString['branding_style'] == ['1'] app.service_api_client.get_service.assert_called_once_with(service_one['id']) @@ -1866,7 +1839,6 @@ def test_should_set_branding_and_organisations( 'main.service_preview_email_branding', service_id=service_one['id'] ), data={ - 'branding_type': 'org', 'branding_style': '1' } ) @@ -1876,7 +1848,6 @@ def test_should_set_branding_and_organisations( mock_update_service.assert_called_once_with( service_one['id'], - branding='org', email_branding='1' ) diff --git a/tests/conftest.py b/tests/conftest.py index 5ed2681da..e00d303e2 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2519,20 +2519,57 @@ def mock_no_email_branding(mocker): ) +def create_email_branding(id, non_standard_values={}): + branding = { + 'logo': 'example.png', + 'name': 'Organisation name', + 'text': 'Organisation text', + 'id': id, + 'colour': '#f00', + 'domain': 'sample.com', + 'brand_type': 'org', + } + + if bool(non_standard_values): + branding.update(non_standard_values) + + return {'email_branding': branding} + + @pytest.fixture(scope='function') def mock_get_email_branding(mocker, fake_uuid): def _get_email_branding(id): - return { - 'email_branding': { - 'logo': 'example.png', - 'name': 'Organisation name', - 'text': 'Organisation text', - 'id': fake_uuid, - 'colour': '#f00', - 'domain': 'sample.com', - 'brand_type': 'org', - } - } + return create_email_branding(fake_uuid) + + return mocker.patch( + 'app.email_branding_client.get_email_branding', side_effect=_get_email_branding + ) + + +@pytest.fixture(scope='function') +def mock_get_email_branding_with_govuk_brand_type(mocker, fake_uuid): + def _get_email_branding(id): + return create_email_branding(fake_uuid, {'brand_type': 'govuk'}) + + return mocker.patch( + 'app.email_branding_client.get_email_branding', side_effect=_get_email_branding + ) + + +@pytest.fixture(scope='function') +def mock_get_email_branding_with_both_brand_type(mocker, fake_uuid): + def _get_email_branding(id): + return create_email_branding(fake_uuid, {'brand_type': 'both'}) + + return mocker.patch( + 'app.email_branding_client.get_email_branding', side_effect=_get_email_branding + ) + + +@pytest.fixture(scope='function') +def mock_get_email_branding_with_org_banner_brand_type(mocker, fake_uuid): + def _get_email_branding(id): + return create_email_branding(fake_uuid, {'brand_type': 'org_banner'}) return mocker.patch( 'app.email_branding_client.get_email_branding', side_effect=_get_email_branding @@ -2542,16 +2579,7 @@ def mock_get_email_branding(mocker, fake_uuid): @pytest.fixture(scope='function') def mock_get_email_branding_without_brand_text(mocker, fake_uuid): def _get_email_branding_without_brand_text(id): - return { - 'email_branding': { - 'logo': 'example.png', - 'name': 'Organisation name', - 'text': '', - 'id': fake_uuid, - 'colour': '#f00', - 'brand_type': 'org_banner' - } - } + return create_email_branding(fake_uuid, {'text': '', 'brand_type': 'org_banner'}) return mocker.patch( 'app.email_branding_client.get_email_branding', From 272049bb172ca28c2b8c23d57da37167ffec2985 Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Wed, 29 Aug 2018 15:20:46 +0100 Subject: [PATCH 4/9] Update branding request page It was getting the brand_type from the service model. This changes the controller to get it from the new email_branding model instead. --- app/main/views/service_settings.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 85a7b094c..b774f03fc 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -971,8 +971,11 @@ def link_service_to_organisation(service_id): @user_has_permissions('manage_service') def branding_request(service_id): + email_branding = email_branding_client.get_email_branding( + current_service.email_branding)['email_branding'] + form = BrandingOptionsEmail( - options=current_service.branding + options=email_branding['brand_type'] ) if form.validate_on_submit(): From 1d3b5fda629c581aa72fad6ad1cc0a3d83c0f539 Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Wed, 29 Aug 2018 15:22:20 +0100 Subject: [PATCH 5/9] Update brand request tests --- tests/app/main/views/test_service_settings.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index d3b85ca05..284fe148f 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -2731,11 +2731,14 @@ def test_update_service_organisation_does_not_update_if_same_value( def test_show_email_branding_request_page( client_request, + mock_get_email_branding ): page = client_request.get( '.branding_request', service_id=SERVICE_ONE_ID ) + mock_get_email_branding.called_once_with(None) + radios = page.select('input[type=radio]') for index, option in enumerate(( @@ -2767,6 +2770,11 @@ def test_submit_email_branding_request( single_sms_sender, ): + email_branding_client = mocker.patch( + 'app.email_branding_client.get_email_branding', + return_value={'email_branding': {'brand_type': choice}} + ) + zendesk = mocker.patch( 'app.main.views.service_settings.zendesk_client.create_ticket', autospec=True, @@ -2780,6 +2788,8 @@ def test_submit_email_branding_request( _follow_redirects=True, ) + email_branding_client.assert_called_once_with(None) + zendesk.assert_called_once_with( message='\n'.join([ 'Organisation: Can’t tell (domain is user.gov.uk)', From ad0ffcc3f36356850b0f540f41723c3919fb5836 Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Thu, 30 Aug 2018 11:38:47 +0100 Subject: [PATCH 6/9] Fix issue that happens with branding_style of None Setting the branding_style to 'None' causes the API to remove the email_branding field from a service model. The branding request page controller was depending on that value being to present get the brand type. The email preview page (used in an iframe on various pages) wasn't able to recognise a branding_style of 'None', causing a blank page to render. --- app/main/views/index.py | 4 ++-- app/main/views/service_settings.py | 10 +++++++--- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/app/main/views/index.py b/app/main/views/index.py index 685cb77f9..5e91a7982 100644 --- a/app/main/views/index.py +++ b/app/main/views/index.py @@ -81,9 +81,9 @@ def design_content(): @main.route('/_email') def email_template(): branding_type = 'govuk' - branding_style = request.args.get('branding_style', None) + branding_style = request.args.get('branding_style', 'None') - if branding_style: + if branding_style != 'None': email_branding = email_branding_client.get_email_branding(branding_style)['email_branding'] branding_type = email_branding['brand_type'] diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index b774f03fc..c3046abbe 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -971,11 +971,15 @@ def link_service_to_organisation(service_id): @user_has_permissions('manage_service') def branding_request(service_id): - email_branding = email_branding_client.get_email_branding( - current_service.email_branding)['email_branding'] + branding_type = 'govuk' + + if current_service.email_branding: + email_branding = email_branding_client.get_email_branding( + current_service.email_branding)['email_branding'] + branding_type = email_branding['brand_type'] form = BrandingOptionsEmail( - options=email_branding['brand_type'] + options=branding_type ) if form.validate_on_submit(): From c2075b6d716cb83798cb186e98bce122b77ee13a Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Thu, 30 Aug 2018 11:44:36 +0100 Subject: [PATCH 7/9] Make label for None branding_style read as 'GOV.UK' --- app/main/views/service_settings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index c3046abbe..a6b3ad532 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -874,7 +874,7 @@ def service_set_email_branding(service_id): # dynamically create org choices, including the null option email_brandings = sorted(get_branding_as_value_and_label(email_branding), key=lambda tup: tup[1].lower()) - form.branding_style.choices = [('None', 'None')] + email_brandings + form.branding_style.choices = [('None', 'GOV.UK')] + email_brandings if form.validate_on_submit(): branding_style = None if form.branding_style.data == 'None' else form.branding_style.data From 4f67502806ce411c197884ec9b1fa4aa72350e0e Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Thu, 30 Aug 2018 15:48:51 +0100 Subject: [PATCH 8/9] Add tests for changes to brand request page Tests the new code that gets the brand type from the email_branding model. Includes checks for a service without the email_branding field set. It also amends the test for a POST from that page, removing mocking of the email_branding client. This test runs against the default service which has its email_branding field set to None so no call is made to the client. It's testing the brand_type values selected so doesn't need the service to have an email_branding already set. --- tests/app/main/views/test_service_settings.py | 40 ++++++++++++++----- 1 file changed, 31 insertions(+), 9 deletions(-) diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 284fe148f..5360abd79 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -2729,7 +2729,7 @@ def test_update_service_organisation_does_not_update_if_same_value( mock_update_service_organisation.called is False -def test_show_email_branding_request_page( +def test_show_email_branding_request_page_when_no_email_branding_is_set( client_request, mock_get_email_branding ): @@ -2737,7 +2737,7 @@ def test_show_email_branding_request_page( '.branding_request', service_id=SERVICE_ONE_ID ) - mock_get_email_branding.called_once_with(None) + mock_get_email_branding.assert_not_called() radios = page.select('input[type=radio]') @@ -2751,6 +2751,35 @@ def test_show_email_branding_request_page( assert radios[index]['value'] == option +def test_show_email_branding_request_page_when_email_branding_is_set( + client_request, + mock_get_email_branding, + active_user_with_permissions, +): + + service_one = service_json(email_branding='1234') + client_request.login(active_user_with_permissions, service=service_one) + + page = client_request.get( + '.branding_request', service_id=SERVICE_ONE_ID + ) + + mock_get_email_branding.called_once_with('1234') + + radios = page.select('input[type=radio]') + + for index, option in enumerate(( + 'govuk', + 'both', + 'org', + 'org_banner', + )): + assert radios[index]['name'] == 'options' + assert radios[index]['value'] == option + if option == 'org': + assert 'checked' in radios[index].attrs + + @pytest.mark.parametrize('choice, requested_branding', ( ('govuk', 'GOV.UK only'), ('both', 'GOV.UK and logo'), @@ -2770,11 +2799,6 @@ def test_submit_email_branding_request( single_sms_sender, ): - email_branding_client = mocker.patch( - 'app.email_branding_client.get_email_branding', - return_value={'email_branding': {'brand_type': choice}} - ) - zendesk = mocker.patch( 'app.main.views.service_settings.zendesk_client.create_ticket', autospec=True, @@ -2788,8 +2812,6 @@ def test_submit_email_branding_request( _follow_redirects=True, ) - email_branding_client.assert_called_once_with(None) - zendesk.assert_called_once_with( message='\n'.join([ 'Organisation: Can’t tell (domain is user.gov.uk)', From d12fa00066c7c5ad0ed89bea4c628113238965fb Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Thu, 30 Aug 2018 16:03:20 +0100 Subject: [PATCH 9/9] Update tests to allow new 'GOV.UK' label --- tests/app/main/views/test_service_settings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 5360abd79..c41b18b46 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -1744,7 +1744,7 @@ def test_should_show_branding_styles( assert branding_style_choices[5]['value'] == '5' # radios should be in alphabetical order, based on their labels - assert radio_labels == ['None', 'org 1', 'org 2', 'org 3', 'org 4', 'org 5'] + assert radio_labels == ['GOV.UK', 'org 1', 'org 2', 'org 3', 'org 4', 'org 5'] assert 'checked' in branding_style_choices[0].attrs assert 'checked' not in branding_style_choices[1].attrs