From 5b77329e635b3de2dee4357ee454648f4c28b06e Mon Sep 17 00:00:00 2001
From: Katie Smith <katie.smith@digital.cabinet-office.gov.uk>
Date: Fri, 25 Jan 2019 16:54:24 +0000
Subject: [PATCH] Validate name field on ServiceUpdateEmailBranding form

This introduces a validator to validate that the name field is not empty
on the ServiceUpdateEmailBranding form, but only if the form details are
being submitted. If a file is being uploaded, the name is allowed to be
empty.
---
 app/main/forms.py                             |  5 ++
 .../views/email-branding/manage-branding.html |  2 +
 tests/app/main/views/test_email_branding.py   | 54 +++++++++++++++++++
 3 files changed, 61 insertions(+)

diff --git a/app/main/forms.py b/app/main/forms.py
index 5c66f3a6c..f6c27ec75 100644
--- a/app/main/forms.py
+++ b/app/main/forms.py
@@ -848,6 +848,11 @@ class ServiceUpdateEmailBranding(StripWhitespaceForm):
         ]
     )
 
+    def validate_name(form, name):
+        op = request.form.get('operation')
+        if op == 'email-branding-details' and not form.name.data:
+            raise ValidationError('This field is required')
+
 
 class PDFUploadForm(StripWhitespaceForm):
     file = FileField_wtf(
diff --git a/app/templates/views/email-branding/manage-branding.html b/app/templates/views/email-branding/manage-branding.html
index 06c587c37..544abb09f 100644
--- a/app/templates/views/email-branding/manage-branding.html
+++ b/app/templates/views/email-branding/manage-branding.html
@@ -29,6 +29,8 @@
            {{ radios(form.brand_type) }}
           {{ page_footer(
             'Save',
+            button_name='operation',
+            button_value='email-branding-details',
             back_link=url_for('.email_branding'),
             back_link_text='Back to email branding selection',
           ) }}
diff --git a/tests/app/main/views/test_email_branding.py b/tests/app/main/views/test_email_branding.py
index fdf872096..6b8d1f318 100644
--- a/tests/app/main/views/test_email_branding.py
+++ b/tests/app/main/views/test_email_branding.py
@@ -218,6 +218,60 @@ def test_rejects_non_canonical_domain_when_adding_email_branding(
     assert mock_create_email_branding.called is False
 
 
+def test_create_email_branding_requires_a_name_when_submitting_logo_details(
+    client_request,
+    mocker,
+    fake_uuid,
+    mock_create_email_branding,
+):
+    mocker.patch('app.main.views.email_branding.persist_logo')
+    mocker.patch('app.main.views.email_branding.delete_temp_files_created_by')
+    data = {
+        'operation': 'email-branding-details',
+        'logo': '',
+        'colour': '#ff0000',
+        'text': 'new text',
+        'name': '',
+        'domain': '',
+        'brand_type': 'org',
+    }
+    client_request.login(platform_admin_user(fake_uuid))
+    page = client_request.post(
+        '.create_email_branding',
+        content_type='multipart/form-data',
+        _data=data,
+        _expected_status=200,
+    )
+
+    assert page.select_one('.error-message').text.strip() == 'This field is required'
+    assert mock_create_email_branding.called is False
+
+
+def test_create_email_branding_does_not_require_a_name_when_uploading_a_file(
+    client_request,
+    mocker,
+    fake_uuid,
+):
+    mocker.patch('app.main.views.email_branding.upload_logo', return_value='temp_filename')
+    data = {
+        'file': (BytesIO(''.encode('utf-8')), 'test.png'),
+        'colour': '',
+        'text': '',
+        'name': '',
+        'domain': '',
+        'brand_type': 'org',
+    }
+    client_request.login(platform_admin_user(fake_uuid))
+    page = client_request.post(
+        '.create_email_branding',
+        content_type='multipart/form-data',
+        _data=data,
+        _follow_redirects=True
+    )
+
+    assert not page.find('.error-message')
+
+
 def test_create_new_email_branding_when_branding_saved(
     logged_in_platform_admin_client,
     mocker,