Use email fields for feedback form

Otherwise we can end up collecting invalid email addresses…

This required some refactoring to allow our email fields to be optional
(but not by default).

app/main/forms.py

@@ -110,15 +110,18 @@ class MultiCheckboxField(SelectMultipleField):
     option_widget = CheckboxInput()
 
 
-def email_address(label='Email address', gov_user=True):
+def email_address(label='Email address', gov_user=True, required=True):
+
     validators = [
-        Length(min=5, max=255),
+        ValidEmail(),
-        DataRequired(message='Can’t be empty'),
-        ValidEmail()
     ]
 
     if gov_user:
         validators.append(ValidGovEmail())
+
+    if required:
+        validators.append(DataRequired(message='Can’t be empty'))
+
     return EmailField(label, validators)
 
 
@@ -561,7 +564,7 @@ class SupportType(StripWhitespaceForm):
 
 class Feedback(StripWhitespaceForm):
     name = StringField('Name')
-    email_address = StringField('Email address')
+    email_address = email_address(label='Email address', gov_user=False, required=False)
     feedback = TextAreaField('Your message', validators=[DataRequired(message="Can’t be empty")])
 
 

app/main/validators.py

@@ -38,6 +38,10 @@ class CsvFileValidator:
 class ValidGovEmail:
 
     def __call__(self, form, field):
+
+        if field.data == '':
+            return
+
         from flask import url_for
         message = (
             'Enter a government email address.'
@@ -53,10 +57,15 @@ class ValidEmail(Email):
         super().__init__('Enter a valid email address')
 
     def __call__(self, form, field):
+
+        if field.data == '':
+            return
+
         try:
             validate_email_address(field.data)
         except InvalidEmailError:
             raise ValidationError(self.message)
+
         return super().__call__(form, field)
 
 

tests/app/main/views/test_feedback.py

@@ -17,6 +17,7 @@ from tests.conftest import (
     mock_get_services,
     mock_get_services_with_no_services,
     mock_get_services_with_one_service,
+    normalize_spaces,
 )
 
 
@@ -224,6 +225,30 @@ def test_email_address_required_for_problems(
     assert isinstance(page.find('span', {'class': 'error-message'}), expected_error)
 
 
+@freeze_time('2016-12-12 12:00:00.000000')
+@pytest.mark.parametrize('ticket_type', (
+    PROBLEM_TICKET_TYPE, QUESTION_TICKET_TYPE
+))
+def test_email_address_must_be_valid_if_provided_to_support_form(
+    client,
+    mocker,
+    ticket_type,
+):
+    response = client.post(
+        url_for('main.feedback', ticket_type=ticket_type),
+        data={
+            'feedback': 'blah',
+            'email_address': 'not valid',
+        },
+    )
+    assert response.status_code == 200
+    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
+
+    assert normalize_spaces(page.select_one('span.error-message').text) == (
+        'Enter a valid email address'
+    )
+
+
 @pytest.mark.parametrize('ticket_type, severe, is_in_business_hours, is_urgent, is_p1', [
 
     # business hours, always urgent, never p1