diff --git a/app/main/forms.py b/app/main/forms.py index 940f730e9..e9a0bb48f 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -1392,35 +1392,46 @@ class LinkOrganisationsForm(StripWhitespaceForm): ) -class BrandingOptionsEmail(StripWhitespaceForm): +class BrandingOptions(StripWhitespaceForm): FALLBACK_OPTION_VALUE = 'something_else' FALLBACK_OPTION = (FALLBACK_OPTION_VALUE, 'Something else') - options = RadioField('Choose your new email branding') + options = RadioField('Choose your new branding') something_else = TextAreaField('Describe the branding you want') - def __init__(self, service, *args, **kwargs): + def __init__(self, service, *args, branding_type="email", **kwargs): super().__init__(*args, **kwargs) - self.options.choices = tuple(self.get_available_choices(service)) + self.options.choices = tuple(self.get_available_choices(service, branding_type)) + self.options.label.text = 'Choose your new {} branding'.format(branding_type) if self.something_else_is_only_option: self.options.data = self.FALLBACK_OPTION_VALUE @staticmethod - def get_available_choices(service): + def get_available_choices(service, branding_type): + if branding_type == "email": + organisation_branding_id = service.organisation.email_branding_id if service.organisation else None + service_branding_id = service.email_branding_id + service_branding_name = service.email_branding_name + elif branding_type == "letter": + organisation_branding_id = service.organisation.letter_branding_id if service.organisation else None + service_branding_id = service.letter_branding_id + service_branding_name = service.letter_branding_name if ( - service.organisation_type == Organisation.TYPE_CENTRAL and - service.organisation.email_branding_id is None and - service.email_branding_id is not None + service.organisation_type == Organisation.TYPE_CENTRAL + and organisation_branding_id is None + and service_branding_id is not None + and branding_type == "email" ): yield ('govuk', 'GOV.UK') if ( - service.organisation_type == Organisation.TYPE_CENTRAL and - service.organisation and - service.organisation.email_branding_id is None and - service.email_branding_name.lower() != 'GOV.UK and {}'.format(service.organisation.name).lower() + service.organisation_type == Organisation.TYPE_CENTRAL + and service.organisation + and organisation_branding_id is None + and service_branding_name.lower() != 'GOV.UK and {}'.format(service.organisation.name).lower() + and branding_type == "email" ): yield ('govuk_and_org', 'GOV.UK and {}'.format(service.organisation.name)) @@ -1429,24 +1440,26 @@ class BrandingOptionsEmail(StripWhitespaceForm): Organisation.TYPE_NHS_CENTRAL, Organisation.TYPE_NHS_LOCAL, Organisation.TYPE_NHS_GP, - } and service.email_branding_name != 'NHS' + } + and service_branding_name != 'NHS' ): yield ('nhs', 'NHS') if ( - service.organisation and - service.organisation_type not in { + service.organisation + and service.organisation_type not in { Organisation.TYPE_NHS_LOCAL, Organisation.TYPE_NHS_CENTRAL, Organisation.TYPE_NHS_GP, - } and ( - service.email_branding_id is None or - service.email_branding_id != service.organisation.email_branding_id + } + and ( + service_branding_id is None + or service_branding_id != organisation_branding_id ) ): yield ('organisation', service.organisation.name) - yield BrandingOptionsEmail.FALLBACK_OPTION + yield BrandingOptions.FALLBACK_OPTION @property def something_else_is_only_option(self): @@ -1454,8 +1467,8 @@ class BrandingOptionsEmail(StripWhitespaceForm): def validate_something_else(self, field): if ( - self.something_else_is_only_option or - self.options.data == self.FALLBACK_OPTION_VALUE + self.something_else_is_only_option + or self.options.data == self.FALLBACK_OPTION_VALUE ) and not field.data: raise ValidationError('Cannot be empty') diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 79447ebe5..4408cd85d 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -30,7 +30,7 @@ from app import ( from app.extensions import zendesk_client from app.main import main from app.main.forms import ( - BrandingOptionsEmail, + BrandingOptions, ConfirmPasswordForm, EstimateUsageForm, FreeSMSAllowance, @@ -1001,15 +1001,6 @@ def service_preview_letter_branding(service_id): ) -@main.route("/services//service-settings/request-letter-branding", methods=['GET', 'POST']) -@user_has_permissions('manage_service', 'manage_templates') -def request_letter_branding(service_id): - return render_template( - 'views/service-settings/request-letter-branding.html', - from_template=request.args.get('from_template'), - ) - - @main.route("/services//service-settings/link-service-to-organisation", methods=['GET', 'POST']) @user_is_platform_admin def link_service_to_organisation(service_id): @@ -1038,15 +1029,18 @@ def link_service_to_organisation(service_id): ) -@main.route("/services//branding-request/email", methods=['GET', 'POST']) +@main.route("/services//branding-request/", methods=['GET', 'POST']) @user_has_permissions('manage_service') -def branding_request(service_id): - - form = BrandingOptionsEmail(current_service) - +def branding_request(service_id, branding_type): + form = BrandingOptions(current_service, branding_type=branding_type) + from_template = request.args.get('from_template') + if branding_type == "email": + branding_name = current_service.email_branding_name + elif branding_type == "letter": + branding_name = current_service.letter_branding_name if form.validate_on_submit(): zendesk_client.create_ticket( - subject='Email branding request - {}'.format(current_service.name), + subject='{} branding request - {}'.format(branding_type.capitalize(), current_service.name), message=( 'Organisation: {organisation}\n' 'Service: {service_name}\n' @@ -1061,7 +1055,7 @@ def branding_request(service_id): organisation=current_service.organisation.as_info_for_branding_request(current_user.email_domain), service_name=current_service.name, dashboard_url=url_for('main.service_dashboard', service_id=current_service.id, _external=True), - current_branding=current_service.email_branding_name, + current_branding=branding_name, branding_requested=dict(form.options.choices)[form.options.data], new_paragraph='\n\n' if form.something_else.data else '', detail=form.something_else.data or '' @@ -1071,16 +1065,20 @@ def branding_request(service_id): user_name=current_user.name, tags=['notify_action', 'notify_branding'], ) - flash(( 'Thanks for your branding request. We’ll get back to you ' 'within one working day.' ), 'default') - return redirect(url_for('.service_settings', service_id=service_id)) + return redirect(url_for( + '.view_template', service_id=current_service.id, template_id=from_template + ) if from_template else url_for('.service_settings', service_id=current_service.id)) return render_template( - 'views/service-settings/branding/email-options.html', + 'views/service-settings/branding/branding-options.html', form=form, + branding_type=branding_type, + branding_name=branding_name, + from_template=from_template ) diff --git a/app/models/service.py b/app/models/service.py index 4810624a7..9e4fad965 100644 --- a/app/models/service.py +++ b/app/models/service.py @@ -436,6 +436,12 @@ class Service(JSONModel): return 'GOV.UK' return self.email_branding['name'] + @cached_property + def letter_branding_name(self): + if self.letter_branding is None: + return 'no' + return self.letter_branding['name'] + @property def needs_to_change_email_branding(self): return self.email_branding_id is None and self.organisation_type != Organisation.TYPE_CENTRAL diff --git a/app/navigation.py b/app/navigation.py index 8cbd0ecab..3c4c21661 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -238,7 +238,6 @@ class HeaderNavigation(Navigation): 'registration_continue', 'remove_user_from_organisation', 'remove_user_from_service', - 'request_letter_branding', 'request_to_go_live', 'resend_email_link', 'resend_email_verification', @@ -405,7 +404,6 @@ class MainNavigation(Navigation): 'branding_request', 'estimate_usage', 'link_service_to_organisation', - 'request_letter_branding', 'request_to_go_live', 'service_add_email_reply_to', 'service_add_letter_contact', @@ -805,7 +803,6 @@ class CaseworkNavigation(Navigation): 'registration_continue', 'remove_user_from_organisation', 'remove_user_from_service', - 'request_letter_branding', 'request_to_go_live', 'resend_email_link', 'resend_email_verification', @@ -1085,7 +1082,6 @@ class OrgNavigation(Navigation): 'register_from_org_invite', 'registration_continue', 'remove_user_from_service', - 'request_letter_branding', 'request_to_go_live', 'resend_email_link', 'resend_email_verification', diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html index 52efd838d..a073efb1d 100644 --- a/app/templates/views/service-settings.html +++ b/app/templates/views/service-settings.html @@ -105,7 +105,7 @@ {{ text_field(current_service.email_branding_name) }} {{ edit_field( 'Change', - url_for('.branding_request', service_id=current_service.id), + url_for('.branding_request', service_id=current_service.id, branding_type="email"), permissions=['manage_service'], )}} {% endcall %} @@ -241,7 +241,7 @@ {{ optional_text_field(current_service.letter_branding.name) }} {{ edit_field( 'Change', - url_for('.request_letter_branding', service_id=current_service.id), + url_for('.branding_request', service_id=current_service.id, branding_type="letter"), permissions=['manage_service'] )}} {% endcall %} diff --git a/app/templates/views/service-settings/branding/email-options.html b/app/templates/views/service-settings/branding/branding-options.html similarity index 79% rename from app/templates/views/service-settings/branding/email-options.html rename to app/templates/views/service-settings/branding/branding-options.html index 2fd7563f4..d7b3cb68f 100644 --- a/app/templates/views/service-settings/branding/email-options.html +++ b/app/templates/views/service-settings/branding/branding-options.html @@ -7,21 +7,21 @@ {% from "components/form.html" import form_wrapper %} {% block service_page_title %} - Change email branding + Change {{ branding_type }} branding {% endblock %} {% block maincolumn_content %} {{ page_header( - 'Change email branding', - back_link=url_for('main.service_settings', service_id=current_service.id) + 'Change {} branding'.format(branding_type), + back_link=url_for('.view_template', service_id=current_service.id, template_id=from_template) if from_template else url_for('.service_settings', service_id=current_service.id) ) }}

- Your emails currently have {{ current_service.email_branding_name }} branding. + Your {{ branding_type }}s currently have {{ branding_name }} branding.

- {% if current_service.needs_to_change_email_branding %} + {% if current_service.needs_to_change_email_branding and branding_type == "email" %}

You should be using your own branding instead. We can help you to set this up.

diff --git a/app/templates/views/service-settings/request-letter-branding.html b/app/templates/views/service-settings/request-letter-branding.html deleted file mode 100644 index 508182001..000000000 --- a/app/templates/views/service-settings/request-letter-branding.html +++ /dev/null @@ -1,39 +0,0 @@ -{% extends "withnav_template.html" %} -{% from "components/radios.html" import radios %} -{% from "components/page-header.html" import page_header %} -{% from "components/page-footer.html" import page_footer %} - -{% block service_page_title %} - Letter branding -{% endblock %} - -{% block maincolumn_content %} - - {{ page_header( - 'Letter branding', - back_link=url_for('.view_template', service_id=current_service.id, template_id=from_template) if from_template else url_for('.service_settings', service_id=current_service.id) - ) }} - -
-
- {% if current_service.letter_branding_id %} -

- Your letters have the {{ current_service.letter_branding.name }} logo. -

-

- Contact us - if you want to use a different logo. -

- {% else %} -

- Your letters do not have a logo. -

-

- Contact us - if you want to add your organisation’s logo. -

- {% endif %} -
-
- -{% endblock %} diff --git a/app/templates/views/templates/_template.html b/app/templates/views/templates/_template.html index f236a5a1a..eecee2ece 100644 --- a/app/templates/views/templates/_template.html +++ b/app/templates/views/templates/_template.html @@ -51,7 +51,7 @@
{% if current_user.has_permissions('manage_templates') and template.template_type == 'letter' %} {% if not current_service.letter_branding_id %} - Add logo + Add logo {% endif %} Change Edit diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 2f38ebcb3..ddede014c 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -3037,58 +3037,6 @@ def test_set_letter_contact_block_has_max_10_lines( assert error_message == 'Contains 11 lines, maximum is 10' -@pytest.mark.parametrize('extra_args, expected_partial_url', ( - ( - {}, - partial(url_for, 'main.service_settings') - ), - ( - {'from_template': FAKE_TEMPLATE_ID}, - partial(url_for, 'main.view_template', template_id=FAKE_TEMPLATE_ID) - ), -)) -def test_request_letter_branding( - client_request, - mock_get_letter_branding_by_id, - extra_args, - expected_partial_url, -): - request_page = client_request.get( - 'main.request_letter_branding', - service_id=SERVICE_ONE_ID, - **extra_args - ) - assert request_page.select_one('main p').text.strip() == 'Your letters do not have a logo.' - back_link_href = request_page.select('main a')[0]['href'] - link_href = request_page.select('main a')[1]['href'] - assert link_href == url_for( - 'main.feedback', - ticket_type='ask-question-give-feedback', - body='letter-branding', - ) - feedback_page = client_request.get_url(link_href) - assert feedback_page.select_one('textarea').text.strip() == ( - 'I would like my own logo on my letter templates.' - ) - assert back_link_href == expected_partial_url(service_id=SERVICE_ONE_ID) - - -def test_request_letter_branding_if_already_have_branding( - client_request, - mock_get_letter_branding_by_id, - service_one, -): - service_one['letter_branding'] = uuid4() - - request_page = client_request.get( - 'main.request_letter_branding', - service_id=SERVICE_ONE_ID, - ) - - mock_get_letter_branding_by_id.assert_called_once_with(service_one['letter_branding']) - assert request_page.select_one('main p').text.strip() == 'Your letters have the HM Government logo.' - - def test_service_set_letter_branding_platform_admin_only( client_request, ): @@ -4452,6 +4400,7 @@ def test_update_service_organisation_does_not_update_if_same_value( mock_update_service_organisation.called is False +@pytest.mark.parametrize('branding_type', ['email', 'letter']) @pytest.mark.parametrize('organisation_type, expected_options', ( ('central', None), ('local', None), @@ -4470,15 +4419,17 @@ def test_update_service_organisation_does_not_update_if_same_value( ('emergency_service', None), ('other', None), )) -def test_show_email_branding_request_page_when_no_email_branding_is_set( +def test_show_branding_request_page_when_no_branding_is_set( mocker, service_one, client_request, mock_get_email_branding, + mock_get_letter_branding_by_id, organisation_type, expected_options, + branding_type ): - service_one['email_branding'] = None + service_one['{}_branding'.format(branding_type)] = None service_one['organisation_type'] = organisation_type mocker.patch( 'app.organisations_client.get_service_organisation', @@ -4486,10 +4437,11 @@ def test_show_email_branding_request_page_when_no_email_branding_is_set( ) page = client_request.get( - '.branding_request', service_id=SERVICE_ONE_ID + '.branding_request', service_id=SERVICE_ONE_ID, branding_type=branding_type ) mock_get_email_branding.assert_not_called() + mock_get_letter_branding_by_id.assert_not_called() if expected_options: assert [ @@ -4513,12 +4465,8 @@ def test_show_email_branding_request_page_when_no_email_branding_is_set( assert not page.select('.conditional-radios-panel') +@pytest.mark.parametrize('branding_type', ['email', 'letter']) @pytest.mark.parametrize('organisation_type, expected_options', ( - ('central', [ - ('govuk_and_org', 'GOV.UK and Test Organisation'), - ('organisation', 'Test Organisation'), - ('something_else', 'Something else'), - ]), ('local', [ ('organisation', 'Test Organisation'), ('something_else', 'Something else'), @@ -4544,15 +4492,17 @@ def test_show_email_branding_request_page_when_no_email_branding_is_set( ('something_else', 'Something else'), ]), )) -def test_show_email_branding_request_page_when_no_email_branding_is_set_but_organisation_exists( +def test_show_branding_request_page_when_no_branding_is_set_but_organisation_exists( mocker, service_one, client_request, mock_get_email_branding, + mock_get_letter_branding_by_id, organisation_type, expected_options, + branding_type ): - service_one['email_branding'] = None + service_one['{}_branding'.format(branding_type)] = None service_one['organisation_type'] = organisation_type mocker.patch( 'app.organisations_client.get_service_organisation', @@ -4560,10 +4510,55 @@ def test_show_email_branding_request_page_when_no_email_branding_is_set_but_orga ) page = client_request.get( - '.branding_request', service_id=SERVICE_ONE_ID + '.branding_request', service_id=SERVICE_ONE_ID, branding_type=branding_type ) mock_get_email_branding.assert_not_called() + mock_get_letter_branding_by_id.assert_not_called() + + assert [ + ( + radio['value'], + page.select_one('label[for={}]'.format(radio['id'])).text.strip() + ) + for radio in page.select('input[type=radio]') + ] == expected_options + + +@pytest.mark.parametrize('organisation_type, expected_options, branding_type', ( + ('central', [ + ('govuk_and_org', 'GOV.UK and Test Organisation'), + ('organisation', 'Test Organisation'), + ('something_else', 'Something else'), + ], 'email'), + ('central', [ + ('organisation', 'Test Organisation'), + ('something_else', 'Something else'), + ], 'letter'), +)) +def test_show_branding_request_page_when_no_branding_is_set_but_organisation_exists_central_org( + mocker, + service_one, + client_request, + mock_get_email_branding, + mock_get_letter_branding_by_id, + organisation_type, + expected_options, + branding_type +): + service_one['{}_branding'.format(branding_type)] = None + service_one['organisation_type'] = organisation_type + mocker.patch( + 'app.organisations_client.get_service_organisation', + return_value=organisation_json(organisation_type=organisation_type), + ) + + page = client_request.get( + '.branding_request', service_id=SERVICE_ONE_ID, branding_type=branding_type + ) + + mock_get_email_branding.assert_not_called() + mock_get_letter_branding_by_id.assert_not_called() assert [ ( @@ -4588,7 +4583,7 @@ def test_show_email_branding_request_page_when_email_branding_is_set( ) page = client_request.get( - '.branding_request', service_id=SERVICE_ONE_ID + '.branding_request', service_id=SERVICE_ONE_ID, branding_type="email" ) assert [ ( @@ -4604,21 +4599,91 @@ def test_show_email_branding_request_page_when_email_branding_is_set( ] -def test_show_email_branding_request_page_when_email_branding_is_same_as_org( +def test_show_letter_branding_request_page_when_letter_branding_is_set( + mocker, + service_one, + client_request, + mock_get_letter_branding_by_id, + active_user_with_permissions, +): + service_one['letter_branding'] = sample_uuid() + mocker.patch( + 'app.organisations_client.get_service_organisation', + return_value=organisation_json(), + ) + + page = client_request.get( + '.branding_request', service_id=SERVICE_ONE_ID, branding_type="letter" + ) + assert [ + ( + radio['value'], + page.select_one('label[for={}]'.format(radio['id'])).text.strip() + ) + for radio in page.select('input[type=radio]') + ] == [ + ('organisation', 'Test Organisation'), + ('something_else', 'Something else'), + ] + + +@pytest.mark.parametrize('branding_type', ['email', 'letter']) +@pytest.mark.parametrize('from_template,back_link_url', [ + (None, '/services/{}/service-settings'.format(SERVICE_ONE_ID),), + (TEMPLATE_ONE_ID, '/services/{}/templates/{}'.format(SERVICE_ONE_ID, TEMPLATE_ONE_ID),) +]) +def test_back_link_on_branding_request_page( mocker, service_one, client_request, mock_get_email_branding, + mock_get_letter_branding_by_id, active_user_with_permissions, + from_template, + back_link_url, + branding_type, ): - service_one['email_branding'] = sample_uuid() mocker.patch( 'app.organisations_client.get_service_organisation', - return_value=organisation_json(email_branding_id=service_one['email_branding']), + return_value=organisation_json(), ) + if from_template: + page = client_request.get( + '.branding_request', service_id=SERVICE_ONE_ID, branding_type=branding_type, from_template=from_template + ) + else: + page = client_request.get( + '.branding_request', service_id=SERVICE_ONE_ID, branding_type=branding_type + ) + + back_link = page.select('a[class=govuk-back-link]') + assert back_link[0].attrs['href'] == back_link_url + + +@pytest.mark.parametrize('branding_type', ['email', 'letter']) +def test_show_branding_request_page_when_branding_is_same_as_org( + mocker, + service_one, + client_request, + mock_get_email_branding, + mock_get_letter_branding_by_id, + active_user_with_permissions, + branding_type +): + service_one['{}_branding'.format(branding_type)] = sample_uuid() + if branding_type == 'email': + mocker.patch( + 'app.organisations_client.get_service_organisation', + return_value=organisation_json(email_branding_id=service_one['email_branding']), + ) + else: + mocker.patch( + 'app.organisations_client.get_service_organisation', + return_value=organisation_json(letter_branding_id=service_one['letter_branding']), + ) page = client_request.get( - '.branding_request', service_id=SERVICE_ONE_ID + '.branding_request', service_id=SERVICE_ONE_ID, branding_type=branding_type ) # Central government organisations who have their own default @@ -4682,6 +4747,7 @@ def test_submit_email_branding_request( expected_organisation, ): service_one['email_branding'] = sample_uuid() + mocker.patch( 'app.organisations_client.get_service_organisation', return_value=organisation_json(name=org_name) if org_name else None, @@ -4693,7 +4759,7 @@ def test_submit_email_branding_request( ) page = client_request.post( - '.branding_request', service_id=SERVICE_ONE_ID, + '.branding_request', service_id=SERVICE_ONE_ID, branding_type="email", _data=data, _follow_redirects=True, ) @@ -4720,12 +4786,135 @@ def test_submit_email_branding_request( ) -def test_submit_email_branding_when_something_else_is_only_option( +@pytest.mark.parametrize('data, requested_branding', ( + ( + { + 'options': 'something_else', + 'something_else': 'Homer Simpson' + }, + 'Something else\n\nHomer Simpson' + ), + pytest.param( + { + 'options': 'something_else', + }, + '[Missing details]', + marks=pytest.mark.xfail(raises=AssertionError), + ), + pytest.param( + {'options': 'foo'}, + 'Nope', + marks=pytest.mark.xfail(raises=AssertionError), + ), +)) +@pytest.mark.parametrize('org_name, expected_organisation', ( + (None, 'Can’t tell (domain is user.gov.uk)'), + ('Test Organisation', 'Test Organisation'), +)) +def test_submit_letter_branding_request( + client_request, + service_one, + mocker, + data, + requested_branding, + mock_get_service_settings_page_common, + mock_get_letter_branding_by_id, + no_reply_to_email_addresses, + no_letter_contact_blocks, + single_sms_sender, + org_name, + expected_organisation, +): + service_one['letter_branding'] = sample_uuid() + + mocker.patch( + 'app.organisations_client.get_service_organisation', + return_value=organisation_json(name=org_name) if org_name else None, + ) + + zendesk = mocker.patch( + 'app.main.views.service_settings.zendesk_client.create_ticket', + autospec=True, + ) + + page = client_request.post( + '.branding_request', service_id=SERVICE_ONE_ID, branding_type="letter", + _data=data, + _follow_redirects=True, + ) + + zendesk.assert_called_once_with( + message='\n'.join([ + 'Organisation: {}', + 'Service: service one', + 'http://localhost/services/596364a0-858e-42c8-9062-a8fe822260eb', + '', + '---', + 'Current branding: HM Government', + 'Branding requested: {}\n', + ]).format(expected_organisation, requested_branding), + subject='Letter branding request - service one', + ticket_type='question', + user_email='test@user.gov.uk', + user_name='Test User', + tags=['notify_action', 'notify_branding'], + ) + assert normalize_spaces(page.select_one('.banner-default').text) == ( + 'Thanks for your branding request. We’ll get back to you ' + 'within one working day.' + ) + + +@pytest.mark.parametrize('branding_type', ['email', 'letter']) +@pytest.mark.parametrize('from_template', [ + None, + TEMPLATE_ONE_ID +]) +def test_submit_letter_branding_request_redirects_if_from_template_is_set( + client_request, + service_one, + mocker, + mock_get_service_settings_page_common, + mock_get_letter_branding_by_id, + no_reply_to_email_addresses, + no_letter_contact_blocks, + single_sms_sender, + from_template, + branding_type, + +): + mocker.patch('app.organisations_client.get_service_organisation', return_value=None) + mocker.patch('app.main.views.service_settings.zendesk_client.create_ticket', autospec=True) + data = {'options': 'something_else', 'something_else': 'Homer Simpson'} + + if from_template: + client_request.post( + '.branding_request', service_id=SERVICE_ONE_ID, branding_type="letter", from_template=from_template, + _data=data, + _expected_redirect=url_for( + 'main.view_template', service_id=SERVICE_ONE_ID, template_id=from_template, _external=True + ) + ) + else: + client_request.post( + '.branding_request', service_id=SERVICE_ONE_ID, branding_type="letter", + _data=data, + _expected_redirect=url_for('main.service_settings', service_id=SERVICE_ONE_ID, _external=True) + ) + + +@pytest.mark.parametrize('branding_type,current_branding', [ + ('email', 'GOV.UK'), ('letter', 'no') +]) +def test_submit_branding_when_something_else_is_only_option( client_request, service_one, mocker, mock_get_service_settings_page_common, mock_get_email_branding, + mock_get_letter_branding_by_id, + branding_type, + current_branding, ): mocker.patch( 'app.organisations_client.get_service_organisation', @@ -4739,20 +4928,39 @@ def test_submit_email_branding_when_something_else_is_only_option( client_request.post( '.branding_request', - service_id=SERVICE_ONE_ID, + service_id=SERVICE_ONE_ID, branding_type=branding_type, _data={ 'something_else': 'Homer Simpson', }, ) assert ( - 'Current branding: GOV.UK\n' + 'Current branding: {}\n' 'Branding requested: Something else\n' '\n' - 'Homer Simpson' + 'Homer Simpson'.format(current_branding) ) in zendesk.call_args_list[0][1]['message'] +def test_service_settings_links_to_branding_request_page_for_letters( + mocker, + service_one, + client_request, + active_user_with_permissions, + no_reply_to_email_addresses, + no_letter_contact_blocks, + single_sms_sender, + mock_get_service_settings_page_common, + mock_get_service_organisation, +): + service_one["restricted"] is False + service_one['permissions'].append('letter') + page = client_request.get( + '.service_settings', service_id=SERVICE_ONE_ID + ) + assert len(page.findAll('a', attrs={'href': '/services/{}/branding-request/letter'.format(SERVICE_ONE_ID)})) == 1 + + def test_show_service_data_retention( platform_admin_client, service_one, diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py index b2262c213..b5ec10f42 100644 --- a/tests/app/main/views/test_templates.py +++ b/tests/app/main/views/test_templates.py @@ -437,7 +437,10 @@ def test_user_with_only_send_and_view_sees_letter_page( @pytest.mark.parametrize('letter_branding, expected_link, expected_link_text', ( ( None, - partial(url_for, 'main.request_letter_branding', from_template=TEMPLATE_ONE_ID), + partial( + url_for, 'main.branding_request', + service_id=SERVICE_ONE_ID, branding_type="letter", from_template=TEMPLATE_ONE_ID + ), 'Add logo', ), (