Make handling of None values in forms clearer

If the browser posts the value of `<input value='None'>` to the server
it does so as a string.

We want to post a value of `None` (actually JSON `null`) to the API. To
do this we:
- set the value in the form class to `'None'` (ie a string)
- convert to `None` (as a type) afterwards

However seeing `x = 'None'` in code looks a bit like a mistake. So to
make sure it looks deliberate and clear what is happening this commit:
- makes a reusable constant for `'None'`
- adds a comment explaining why it’s a string

app/main/forms.py

@@ -702,13 +702,25 @@ class ServicePostageForm(StripWhitespaceForm):
     )
 
 
-class RadioFieldWithNoneOption(RadioField):
+class FieldWithNoneOption():
+
+    # This needs to match the data the browser will post from
+    # <input value='None'>
+    NONE_OPTION_VALUE = 'None'
 
     def post_validate(self, form, validation_stopped):
-        if self.data == 'None':
+        if self.data == self.NONE_OPTION_VALUE:
             self.data = None
 
 
+class RadioFieldWithNoneOption(FieldWithNoneOption, RadioField):
+    pass
+
+
+class HiddenFieldWithNoneOption(FieldWithNoneOption, HiddenField):
+    pass
+
+
 class ServiceSetBranding(StripWhitespaceForm):
 
     branding_style = RadioFieldWithNoneOption(
@@ -718,7 +730,7 @@ class ServiceSetBranding(StripWhitespaceForm):
         ]
     )
 
-    DEFAULT = ('None', 'GOV.UK')
+    DEFAULT = (FieldWithNoneOption.NONE_OPTION_VALUE, 'GOV.UK')
 
     def __init__(self, all_email_brandings, current_email_branding):
 
@@ -736,7 +748,7 @@ class ServiceSetBranding(StripWhitespaceForm):
 
 class ServicePreviewBranding(StripWhitespaceForm):
 
-    branding_style = HiddenField('branding_style')
+    branding_style = HiddenFieldWithNoneOption('branding_style')
 
 
 class GovernmentDomainField(StringField):
@@ -1124,7 +1136,7 @@ class TemplateAndFoldersSelectionForm(Form):
 
     ALL_TEMPLATES_FOLDER = {
         'name': 'All templates',
-        'id': 'None',
+        'id': RadioFieldWithNoneOption.NONE_OPTION_VALUE,
     }
 
     def __init__(

app/main/views/service_settings.py

@@ -813,9 +813,8 @@ def service_preview_email_branding(service_id):
     form = ServicePreviewBranding(branding_style=branding_style)
 
     if form.validate_on_submit():
-        branding_style = None if form.branding_style.data == 'None' else form.branding_style.data
         current_service.update(
-            email_branding=branding_style
+            email_branding=form.branding_style.data
         )
         return redirect(url_for('.service_settings', service_id=service_id))
 

tests/app/main/views/test_service_settings.py

@@ -2304,17 +2304,23 @@ def test_should_preview_email_branding(
     app.service_api_client.get_service.assert_called_once_with(service_one['id'])
 
 
+@pytest.mark.parametrize('posted_value, submitted_value', (
+    ('1', '1'),
+    ('None', None),
+))
 def test_should_set_branding_and_organisations(
     logged_in_platform_admin_client,
     service_one,
     mock_update_service,
+    posted_value,
+    submitted_value,
 ):
     response = logged_in_platform_admin_client.post(
         url_for(
             'main.service_preview_email_branding', service_id=service_one['id']
         ),
         data={
-            'branding_style': '1'
+            'branding_style': posted_value
         }
     )
     assert response.status_code == 302
@@ -2323,7 +2329,7 @@ def test_should_set_branding_and_organisations(
 
     mock_update_service.assert_called_once_with(
         service_one['id'],
-        email_branding='1'
+        email_branding=submitted_value
     )