diff --git a/app/main/forms.py b/app/main/forms.py index 363e9d3be..270da37a0 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -1359,16 +1359,17 @@ class LinkOrganisationsForm(StripWhitespaceForm): class BrandingOptionsEmail(StripWhitespaceForm): - options = RadioField( - 'Choose your new email branding', - validators=[ - DataRequired() - ], - ) + FALLBACK_OPTION_VALUE = 'something_else' + FALLBACK_OPTION = (FALLBACK_OPTION_VALUE, 'Something else') + + options = RadioField('Choose your new email branding') + something_else = TextAreaField('Describe the branding you want') def __init__(self, service, *args, **kwargs): super().__init__(*args, **kwargs) self.options.choices = tuple(self.get_available_choices(service)) + if not self.something_else_is_only_option: + self.options.validators.append(DataRequired()) @staticmethod def get_available_choices(service): @@ -1404,7 +1405,21 @@ class BrandingOptionsEmail(StripWhitespaceForm): ): yield ('organisation', service.organisation.name) - yield ('something_else', 'Something else') + yield BrandingOptionsEmail.FALLBACK_OPTION + + @property + def something_else_is_only_option(self): + return self.options.choices == (self.FALLBACK_OPTION,) + + def validate_something_else(self, field): + if ( + self.something_else_is_only_option or + self.options.data == self.FALLBACK_OPTION_VALUE + ) and not field.data: + raise ValidationError('Can’t be empty') + + if self.options.data != self.FALLBACK_OPTION_VALUE: + field.data = '' class ServiceDataRetentionForm(StripWhitespaceForm): diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 4036f6d74..a1b165869 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -1047,12 +1047,17 @@ def branding_request(service_id): '\n---' '\nCurrent branding: {current_branding}' '\nBranding requested: {branding_requested}' + '{new_paragraph}' + '{detail}' + '\n' ).format( 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, 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 '' ), ticket_type=zendesk_client.TYPE_QUESTION, user_email=current_user.email_address, diff --git a/app/templates/views/service-settings/branding/email-options.html b/app/templates/views/service-settings/branding/email-options.html index 81b65c764..1e7957720 100644 --- a/app/templates/views/service-settings/branding/email-options.html +++ b/app/templates/views/service-settings/branding/email-options.html @@ -1,5 +1,6 @@ {% extends "withnav_template.html" %} -{% from "components/radios.html" import radios %} +{% from "components/radios.html" import radio, conditional_radio_panel %} +{% from "components/select-input.html" import select_wrapper %} {% from "components/textbox.html" import textbox %} {% from "components/page-header.html" import page_header %} {% from "components/page-footer.html" import page_footer %} @@ -17,7 +18,26 @@ ) }} {% call form_wrapper() %} - {{ radios(form.options) }} + {% if form.something_else_is_only_option %} + {{ textbox( + form.something_else, + hint='Include links to your brand guidelines or examples of how to use your branding', + width='1-1', + ) }} + {% else %} + {% call select_wrapper(form.options) %} + {% for option in form.options %} + {{ radio(option, data_target='panel-something-else' if option.data == form.FALLBACK_OPTION_VALUE else '') }} + {% endfor %} + {% endcall %} + {% call conditional_radio_panel('panel-something-else') %} + {{ textbox( + form.something_else, + hint='Include links to your brand guidelines or examples of how to use your branding', + width='1-1', + ) }} + {% endcall %} + {% endif %}

We’ll email you once your branding’s ready to use, or if we need any more information. diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 110826fe7..c31041e23 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -4378,12 +4378,8 @@ def test_update_service_organisation_does_not_update_if_same_value( @pytest.mark.parametrize('organisation_type, expected_options', ( - ('central', [ - ('something_else', 'Something else'), - ]), - ('local', [ - ('something_else', 'Something else'), - ]), + ('central', None), + ('local', None), ('nhs_central', [ ('nhs', 'NHS'), ('something_else', 'Something else'), @@ -4396,12 +4392,8 @@ def test_update_service_organisation_does_not_update_if_same_value( ('nhs', 'NHS'), ('something_else', 'Something else'), ]), - ('emergency_service', [ - ('something_else', 'Something else'), - ]), - ('other', [ - ('something_else', 'Something else'), - ]), + ('emergency_service', None), + ('other', None), )) def test_show_email_branding_request_page_when_no_email_branding_is_set( mocker, @@ -4424,13 +4416,26 @@ def test_show_email_branding_request_page_when_no_email_branding_is_set( mock_get_email_branding.assert_not_called() - assert [ - ( - radio['value'], - page.select_one('label[for={}]'.format(radio['id'])).text.strip() + if expected_options: + assert [ + ( + radio['value'], + page.select_one('label[for={}]'.format(radio['id'])).text.strip() + ) + for radio in page.select('input[type=radio]') + ] == expected_options + assert page.select_one( + '.conditional-radios-panel#panel-something-else textarea' + )['name'] == ( + 'something_else' ) - for radio in page.select('input[type=radio]') - ] == expected_options + else: + assert page.select_one( + 'textarea' + )['name'] == ( + 'something_else' + ) + assert not page.select('.conditional-radios-panel') @pytest.mark.parametrize('organisation_type, expected_options', ( @@ -4541,19 +4546,12 @@ def test_show_email_branding_request_page_when_email_branding_is_same_as_org( '.branding_request', service_id=SERVICE_ONE_ID ) - assert [ - ( - radio['value'], - page.select_one('label[for={}]'.format(radio['id'])).text.strip() - ) - for radio in page.select('input[type=radio]') - ] == [ - # Central government organisations who have their own default - # branding will do so because they’re exempt from GOV.UK. - # We also don’t show their organisation’s branding because they - # have it already. - ('something_else', 'Something else'), - ] + # Central government organisations who have their own default + # branding will do so because they’re exempt from GOV.UK. + # We also don’t show their organisation’s branding because they + # have it already. So ‘Something else’ is the only option. + assert not page.select('input[type=radio]') + assert page.select_one('textarea')['name'] == 'something_else' @pytest.mark.parametrize('data, requested_branding', ( @@ -4564,10 +4562,25 @@ def test_show_email_branding_request_page_when_email_branding_is_same_as_org( 'GOV.UK', ), ( + { + 'options': 'govuk', + 'something_else': 'ignored', + }, + 'GOV.UK', + ), + ( + { + 'options': 'something_else', + 'something_else': 'Homer Simpson' + }, + 'Something else\n\nHomer Simpson' + ), + pytest.param( { 'options': 'something_else', }, - 'Something else' + '[Missing details]', + marks=pytest.mark.xfail(raises=AssertionError), ), pytest.param( {'options': 'foo'}, @@ -4618,7 +4631,7 @@ def test_submit_email_branding_request( '', '---', 'Current branding: Organisation name', - 'Branding requested: {}', + 'Branding requested: {}\n', ]).format(expected_organisation, requested_branding), subject='Email branding request - service one', ticket_type='question',