Create a letter branding request flow to match email branding request

Test if service settings links to branding request page for letters

Parametrize all branding tests so they also work for letter branding
This commit is contained in:
Pea Tyczynska
2020-01-17 17:03:07 +00:00
parent 861a29ddd3
commit 02cb6c9c38
6 changed files with 123 additions and 52 deletions

View File

@@ -1392,35 +1392,43 @@ 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))
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
organisation_branding_id is None and
service_branding_id is not None
):
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()
organisation_branding_id is None and
service_branding_name.lower() != 'GOV.UK and {}'.format(service.organisation.name).lower()
):
yield ('govuk_and_org', 'GOV.UK and {}'.format(service.organisation.name))
@@ -1429,7 +1437,7 @@ 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')
@@ -1440,13 +1448,13 @@ class BrandingOptionsEmail(StripWhitespaceForm):
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
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):

View File

@@ -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,
@@ -1038,15 +1038,18 @@ def link_service_to_organisation(service_id):
)
@main.route("/services/<uuid:service_id>/branding-request/email", methods=['GET', 'POST'])
@main.route("/services/<uuid:service_id>/branding-request/<branding_type>", 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)
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 +1064,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 ''
@@ -1079,8 +1082,10 @@ def branding_request(service_id):
return redirect(url_for('.service_settings', service_id=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
)

View File

@@ -417,6 +417,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

View File

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

View File

@@ -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',
'Change {} branding'.format(branding_type),
back_link=url_for('main.service_settings', service_id=current_service.id)
) }}
<p>
Your emails currently have {{ current_service.email_branding_name }} branding.
Your {{ branding_type }}s currently have {{ branding_name }} branding.
</p>
{% if current_service.needs_to_change_email_branding %}
{% if current_service.needs_to_change_email_branding and branding_type == "email" %}
<p>
You should be using your own branding instead. We can help you to set this up.
</p>

View File

@@ -4439,6 +4439,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),
@@ -4457,15 +4458,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',
@@ -4473,10 +4476,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 [
@@ -4500,6 +4504,7 @@ 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'),
@@ -4531,15 +4536,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',
@@ -4547,10 +4554,11 @@ 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 [
(
@@ -4561,21 +4569,24 @@ def test_show_email_branding_request_page_when_no_email_branding_is_set_but_orga
] == expected_options
def test_show_email_branding_request_page_when_email_branding_is_set(
@pytest.mark.parametrize('branding_type', ['email', 'letter'])
def test_show_branding_request_page_when_branding_is_set(
mocker,
service_one,
client_request,
mock_get_email_branding,
mock_get_letter_branding_by_id,
active_user_with_permissions,
branding_type
):
service_one['email_branding'] = sample_uuid()
service_one['{}_branding'.format(branding_type)] = 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_request', service_id=SERVICE_ONE_ID, branding_type=branding_type
)
assert [
(
@@ -4591,21 +4602,30 @@ 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(
@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['email_branding'] = sample_uuid()
mocker.patch(
'app.organisations_client.get_service_organisation',
return_value=organisation_json(email_branding_id=service_one['email_branding']),
)
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
@@ -4616,6 +4636,9 @@ def test_show_email_branding_request_page_when_email_branding_is_same_as_org(
assert page.select_one('textarea')['name'] == 'something_else'
@pytest.mark.parametrize('branding_type,current_branding', [
('email', 'Organisation name'), ('letter', 'HM Government')
])
@pytest.mark.parametrize('data, requested_branding', (
(
{
@@ -4654,21 +4677,25 @@ def test_show_email_branding_request_page_when_email_branding_is_same_as_org(
(None, 'Cant tell (domain is user.gov.uk)'),
('Test Organisation', 'Test Organisation'),
))
def test_submit_email_branding_request(
def test_submit_branding_request(
client_request,
service_one,
mocker,
data,
requested_branding,
branding_type,
current_branding,
mock_get_service_settings_page_common,
mock_get_email_branding,
mock_get_letter_branding_by_id,
no_reply_to_email_addresses,
no_letter_contact_blocks,
single_sms_sender,
org_name,
expected_organisation,
):
service_one['email_branding'] = sample_uuid()
service_one['{}_branding'.format(branding_type)] = sample_uuid()
mocker.patch(
'app.organisations_client.get_service_organisation',
return_value=organisation_json(name=org_name) if org_name else None,
@@ -4680,7 +4707,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=branding_type,
_data=data,
_follow_redirects=True,
)
@@ -4692,10 +4719,10 @@ def test_submit_email_branding_request(
'http://localhost/services/596364a0-858e-42c8-9062-a8fe822260eb',
'',
'---',
'Current branding: Organisation name',
'Current branding: {}'.format(current_branding),
'Branding requested: {}\n',
]).format(expected_organisation, requested_branding),
subject='Email branding request - service one',
subject='{} branding request - service one'.format(branding_type.capitalize()),
ticket_type='question',
user_email='test@user.gov.uk',
user_name='Test User',
@@ -4707,12 +4734,18 @@ def test_submit_email_branding_request(
)
def test_submit_email_branding_when_something_else_is_only_option(
@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',
@@ -4726,20 +4759,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,