Merge pull request #2571 from alphagov/valid-feedback-email

Use email fields for feedback form

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),
-        DataRequired(message='Can’t be empty'),
-        ValidEmail()
+        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