Allow elaboration when ‘something else’ is chosen

Letting people input a bit of free text should reduce the amount of back
and forth we have to do over support tickets when setting up someone’s
branding.

If something else is the only option then we don’t show the radio button
at all and have just the free text input on the page (not behind a
progressive disclosure).
This commit is contained in:
Chris Hill-Scott
2019-09-16 11:18:22 +01:00
parent 6d0d10e8de
commit afcdedf598
4 changed files with 95 additions and 42 deletions
+22 -7
View File
@@ -1359,16 +1359,17 @@ class LinkOrganisationsForm(StripWhitespaceForm):
class BrandingOptionsEmail(StripWhitespaceForm): class BrandingOptionsEmail(StripWhitespaceForm):
options = RadioField( FALLBACK_OPTION_VALUE = 'something_else'
'Choose your new email branding', FALLBACK_OPTION = (FALLBACK_OPTION_VALUE, 'Something else')
validators=[
DataRequired() options = RadioField('Choose your new email branding')
], something_else = TextAreaField('Describe the branding you want')
)
def __init__(self, service, *args, **kwargs): def __init__(self, service, *args, **kwargs):
super().__init__(*args, **kwargs) super().__init__(*args, **kwargs)
self.options.choices = tuple(self.get_available_choices(service)) self.options.choices = tuple(self.get_available_choices(service))
if not self.something_else_is_only_option:
self.options.validators.append(DataRequired())
@staticmethod @staticmethod
def get_available_choices(service): def get_available_choices(service):
@@ -1404,7 +1405,21 @@ class BrandingOptionsEmail(StripWhitespaceForm):
): ):
yield ('organisation', service.organisation.name) 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('Cant be empty')
if self.options.data != self.FALLBACK_OPTION_VALUE:
field.data = ''
class ServiceDataRetentionForm(StripWhitespaceForm): class ServiceDataRetentionForm(StripWhitespaceForm):
+5
View File
@@ -1047,12 +1047,17 @@ def branding_request(service_id):
'\n---' '\n---'
'\nCurrent branding: {current_branding}' '\nCurrent branding: {current_branding}'
'\nBranding requested: {branding_requested}' '\nBranding requested: {branding_requested}'
'{new_paragraph}'
'{detail}'
'\n'
).format( ).format(
organisation=current_service.organisation.as_info_for_branding_request(current_user.email_domain), organisation=current_service.organisation.as_info_for_branding_request(current_user.email_domain),
service_name=current_service.name, service_name=current_service.name,
dashboard_url=url_for('main.service_dashboard', service_id=current_service.id, _external=True), dashboard_url=url_for('main.service_dashboard', service_id=current_service.id, _external=True),
current_branding=current_service.email_branding_name, current_branding=current_service.email_branding_name,
branding_requested=dict(form.options.choices)[form.options.data], 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, ticket_type=zendesk_client.TYPE_QUESTION,
user_email=current_user.email_address, user_email=current_user.email_address,
@@ -1,5 +1,6 @@
{% extends "withnav_template.html" %} {% 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/textbox.html" import textbox %}
{% from "components/page-header.html" import page_header %} {% from "components/page-header.html" import page_header %}
{% from "components/page-footer.html" import page_footer %} {% from "components/page-footer.html" import page_footer %}
@@ -17,7 +18,26 @@
) }} ) }}
{% call form_wrapper() %} {% 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 %}
<p class="form-group"> <p class="form-group">
Well email you once your brandings ready to use, or if we need any Well email you once your brandings ready to use, or if we need any
more information. more information.
+46 -33
View File
@@ -4378,12 +4378,8 @@ def test_update_service_organisation_does_not_update_if_same_value(
@pytest.mark.parametrize('organisation_type, expected_options', ( @pytest.mark.parametrize('organisation_type, expected_options', (
('central', [ ('central', None),
('something_else', 'Something else'), ('local', None),
]),
('local', [
('something_else', 'Something else'),
]),
('nhs_central', [ ('nhs_central', [
('nhs', 'NHS'), ('nhs', 'NHS'),
('something_else', 'Something else'), ('something_else', 'Something else'),
@@ -4396,12 +4392,8 @@ def test_update_service_organisation_does_not_update_if_same_value(
('nhs', 'NHS'), ('nhs', 'NHS'),
('something_else', 'Something else'), ('something_else', 'Something else'),
]), ]),
('emergency_service', [ ('emergency_service', None),
('something_else', 'Something else'), ('other', None),
]),
('other', [
('something_else', 'Something else'),
]),
)) ))
def test_show_email_branding_request_page_when_no_email_branding_is_set( def test_show_email_branding_request_page_when_no_email_branding_is_set(
mocker, 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() mock_get_email_branding.assert_not_called()
assert [ if expected_options:
( assert [
radio['value'], (
page.select_one('label[for={}]'.format(radio['id'])).text.strip() 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]') else:
] == expected_options assert page.select_one(
'textarea'
)['name'] == (
'something_else'
)
assert not page.select('.conditional-radios-panel')
@pytest.mark.parametrize('organisation_type, expected_options', ( @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 '.branding_request', service_id=SERVICE_ONE_ID
) )
assert [ # Central government organisations who have their own default
( # branding will do so because theyre exempt from GOV.UK.
radio['value'], # We also dont show their organisations branding because they
page.select_one('label[for={}]'.format(radio['id'])).text.strip() # have it already. So Something else is the only option.
) assert not page.select('input[type=radio]')
for radio in page.select('input[type=radio]') assert page.select_one('textarea')['name'] == 'something_else'
] == [
# Central government organisations who have their own default
# branding will do so because theyre exempt from GOV.UK.
# We also dont show their organisations branding because they
# have it already.
('something_else', 'Something else'),
]
@pytest.mark.parametrize('data, requested_branding', ( @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', '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', 'options': 'something_else',
}, },
'Something else' '[Missing details]',
marks=pytest.mark.xfail(raises=AssertionError),
), ),
pytest.param( pytest.param(
{'options': 'foo'}, {'options': 'foo'},
@@ -4618,7 +4631,7 @@ def test_submit_email_branding_request(
'', '',
'---', '---',
'Current branding: Organisation name', 'Current branding: Organisation name',
'Branding requested: {}', 'Branding requested: {}\n',
]).format(expected_organisation, requested_branding), ]).format(expected_organisation, requested_branding),
subject='Email branding request - service one', subject='Email branding request - service one',
ticket_type='question', ticket_type='question',