Merge pull request #2276 from alphagov/email-branding-refinements

Put the currently selected email branding at the top of the list

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):
 

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',

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