Refactor some of the logic into form

It’s messy having a lot of logic in the view methods. Handling the form
stuff can be better encapsulated inside the `Form` subclass itself.
This commit is contained in:
Chris Hill-Scott
2018-08-31 17:31:26 +01:00
parent 80801d827a
commit d80b735b2b
3 changed files with 50 additions and 23 deletions

View File

@@ -682,15 +682,37 @@ class ServiceSwitchLettersForm(StripWhitespaceForm):
) )
class BrandingStyle(RadioField):
def post_validate(self, form, validation_stopped):
if self.data == 'None':
self.data = None
class ServiceSetBranding(StripWhitespaceForm): class ServiceSetBranding(StripWhitespaceForm):
branding_style = RadioField( branding_style = BrandingStyle(
'Branding style', 'Branding style',
validators=[ validators=[
DataRequired() DataRequired()
] ]
) )
DEFAULT = ('None', 'GOV.UK')
def __init__(self, all_email_brandings, current_email_branding):
super().__init__(branding_style=current_email_branding)
self.branding_style.choices = sorted(
all_email_brandings + [self.DEFAULT],
key=lambda branding: (
branding[0] != current_email_branding,
branding[0] is not self.DEFAULT[0],
branding[1].lower(),
),
)
class ServicePreviewBranding(StripWhitespaceForm): class ServicePreviewBranding(StripWhitespaceForm):

View File

@@ -887,24 +887,17 @@ def set_free_sms_allowance(service_id):
def service_set_email_branding(service_id): def service_set_email_branding(service_id):
email_branding = email_branding_client.get_all_email_branding() email_branding = email_branding_client.get_all_email_branding()
form = ServiceSetBranding() form = ServiceSetBranding(
all_email_brandings=get_branding_as_value_and_label(email_branding),
# dynamically create org choices, including the null option current_email_branding=current_service.email_branding,
form.branding_style.choices = sorted(
get_branding_as_value_and_label(email_branding) + [('None', 'GOV.UK')],
key=lambda branding: (
branding[0] != current_service.email_branding,
branding[0] is not 'None',
branding[1].lower(),
),
) )
if form.validate_on_submit(): if form.validate_on_submit():
branding_style = None if form.branding_style.data == 'None' else form.branding_style.data return redirect(url_for(
return redirect(url_for('.service_preview_email_branding', service_id=service_id, '.service_preview_email_branding',
branding_style=branding_style)) service_id=service_id,
branding_style=form.branding_style.data,
form.branding_style.data = current_service['email_branding'] or 'None' ))
return render_template( return render_template(
'views/service-settings/set-email-branding.html', 'views/service-settings/set-email-branding.html',

View File

@@ -1849,11 +1849,27 @@ def test_set_letter_branding_saves(
mock_update_service.assert_called_once_with(service_one['id'], dvla_organisation='500') mock_update_service.assert_called_once_with(service_one['id'], dvla_organisation='500')
@pytest.mark.parametrize('current_branding, expected_values, expected_labels', [
(None, [
'None', '1', '2', '3', '4', '5',
], [
'GOV.UK', 'org 1', 'org 2', 'org 3', 'org 4', 'org 5'
]),
('5', [
'5', 'None', '1', '2', '3', '4',
], [
'org 5', 'GOV.UK', 'org 1', 'org 2', 'org 3', 'org 4',
]),
])
def test_should_show_branding_styles( def test_should_show_branding_styles(
logged_in_platform_admin_client, logged_in_platform_admin_client,
service_one, service_one,
mock_get_all_email_branding, mock_get_all_email_branding,
current_branding,
expected_values,
expected_labels,
): ):
service_one['email_branding'] = current_branding
response = logged_in_platform_admin_client.get(url_for( response = logged_in_platform_admin_client.get(url_for(
'main.service_set_email_branding', service_id=service_one['id'] 'main.service_set_email_branding', service_id=service_one['id']
)) ))
@@ -1867,15 +1883,11 @@ def test_should_show_branding_styles(
assert len(branding_style_choices) == 6 assert len(branding_style_choices) == 6
assert branding_style_choices[0]['value'] == 'None' for index, expected_value in enumerate(expected_values):
assert branding_style_choices[1]['value'] == '1' assert branding_style_choices[index]['value'] == expected_value
assert branding_style_choices[2]['value'] == '2'
assert branding_style_choices[3]['value'] == '3'
assert branding_style_choices[4]['value'] == '4'
assert branding_style_choices[5]['value'] == '5'
# radios should be in alphabetical order, based on their labels # radios should be in alphabetical order, based on their labels
assert radio_labels == ['GOV.UK', 'org 1', 'org 2', 'org 3', 'org 4', 'org 5'] assert radio_labels == expected_labels
assert 'checked' in branding_style_choices[0].attrs assert 'checked' in branding_style_choices[0].attrs
assert 'checked' not in branding_style_choices[1].attrs assert 'checked' not in branding_style_choices[1].attrs