diff --git a/app/main/forms.py b/app/main/forms.py index 713badf07..e9bea2992 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -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): - branding_style = RadioField( + branding_style = BrandingStyle( 'Branding style', validators=[ 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): diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 589ced108..5cf9390db 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -887,19 +887,17 @@ def set_free_sms_allowance(service_id): def service_set_email_branding(service_id): email_branding = email_branding_client.get_all_email_branding() - form = ServiceSetBranding() - - # dynamically create org choices, including the null option - email_brandings = sorted(get_branding_as_value_and_label(email_branding), - key=lambda tup: tup[1].lower()) - form.branding_style.choices = [('None', 'GOV.UK')] + email_brandings + form = ServiceSetBranding( + all_email_brandings=get_branding_as_value_and_label(email_branding), + current_email_branding=current_service.email_branding, + ) if form.validate_on_submit(): - branding_style = None if form.branding_style.data == 'None' else form.branding_style.data - return redirect(url_for('.service_preview_email_branding', service_id=service_id, - branding_style=branding_style)) - - form.branding_style.data = current_service['email_branding'] or 'None' + return redirect(url_for( + '.service_preview_email_branding', + service_id=service_id, + branding_style=form.branding_style.data, + )) return render_template( 'views/service-settings/set-email-branding.html', diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 0a7924353..28a2792d3 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -1849,11 +1849,27 @@ def test_set_letter_branding_saves( 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( logged_in_platform_admin_client, service_one, 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( '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 branding_style_choices[0]['value'] == 'None' - assert branding_style_choices[1]['value'] == '1' - 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' + for index, expected_value in enumerate(expected_values): + assert branding_style_choices[index]['value'] == expected_value # 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' not in branding_style_choices[1].attrs