Make answers to volume questions optional

It’s annoying and very ‘computer says no’ to make people type `0` in a
box. We can see from our analytics that this error is affecting about 7%
of users trying to go live.

This commit relaxes the validation to only require a number greater than
1 for at least one of the questions.

It also lets people enter their numbers comma-separated – like our
examples suggest – but normalises them to integers before sending them
over to the API.

app/main/forms.py

@@ -173,6 +173,52 @@ def __call__(self, **kwargs):
         return super().__call__(type='tel', pattern='[0-9]*', **kwargs)
 
 
+class ForgivingIntegerField(StringField):
+
+    POSTGRES_MAX_INT = 2147483647
+
+    def process_formdata(self, valuelist):
+
+        if valuelist:
+
+            valuelist[0] = valuelist[0].replace(',', '')
+
+            try:
+                valuelist[0] = int(valuelist[0])
+            except ValueError:
+                pass
+
+            if valuelist[0] == '':
+                valuelist[0] = 0
+
+        return super().process_formdata(valuelist)
+
+    def pre_validate(self, form):
+
+        if self.data:
+            error = None
+            try:
+                if int(self.data) > self.POSTGRES_MAX_INT:
+                    error = 'Must be less than {:,.0f}'.format(self.POSTGRES_MAX_INT)
+            except ValueError:
+                error = 'Must be a whole number'
+
+            if error:
+                raise ValidationError(error)
+
+        return super().pre_validate(form)
+
+    def __call__(self, **kwargs):
+
+        try:
+            value = int(self.data)
+            value = '{:,.0f}'.format(value)
+        except (ValueError, TypeError):
+            value = self.data if self.data is not None else ''
+
+        return super().__call__(value=value, **kwargs)
+
+
 def organisation_type():
     return RadioField(
         'Who runs this service?',
@@ -594,17 +640,15 @@ class Triage(StripWhitespaceForm):
 
 
 class EstimateUsageForm(StripWhitespaceForm):
-    volume_email = StringField(
+
+    volume_email = ForgivingIntegerField(
         'How many emails do you expect to send in the next year?',
-        validators=[DataRequired(message='Can’t be empty')]
     )
-    volume_sms = StringField(
+    volume_sms = ForgivingIntegerField(
         'How many text messages do you expect to send in the next year?',
-        validators=[DataRequired(message='Can’t be empty')]
     )
-    volume_letter = StringField(
+    volume_letter = ForgivingIntegerField(
         'How many letters do you expect to send in the next year?',
-        validators=[DataRequired(message='Can’t be empty')]
     )
     consent_to_research = RadioField(
         'Can we contact you when we’re doing user research?',
@@ -615,6 +659,16 @@ class EstimateUsageForm(StripWhitespaceForm):
         validators=[DataRequired()]
     )
 
+    at_least_one_volume_filled = True
+
+    def validate(self, *args, **kwargs):
+
+        if self.volume_email.data == self.volume_sms.data == self.volume_letter.data == 0:
+            self.at_least_one_volume_filled = False
+            return False
+
+        return super().validate(*args, **kwargs)
+
 
 class ProviderForm(StripWhitespaceForm):
     priority = IntegerField('Priority', [validators.NumberRange(min=1, max=100, message="Must be between 1 and 100")])

app/templates/views/service-settings/estimate-usage.html

@@ -1,4 +1,5 @@
 {% extends "withnav_template.html" %}
+{% from "components/banner.html" import banner_wrapper %}
 {% from "components/form.html" import form_wrapper %}
 {% from "components/page-footer.html" import page_footer %}
 {% from "components/radios.html" import radios %}
@@ -11,8 +12,17 @@
 {% block maincolumn_content %}
   <div class="grid-row">
     <div class="column-whole">
-      {% call form_wrapper() %}
+      {% if not form.at_least_one_volume_filled %}
+        {% call banner_wrapper(type='dangerous') %}
+          <h1 class='banner-title'>
+            no things supplied
+          </h1>
+          <p>Tell us some things</p>
+        {% endcall %}
+      {% else %}
         <h1 class="heading-large">Estimate usage</h1>
+      {% endif %}
+      {% call form_wrapper() %}
         <div class="form-group">
           {{ textbox(form.volume_email, width='1-2', hint='For example, 1,000,000') }}
           {{ textbox(form.volume_sms, width='1-2', hint='For example, 500,000') }}

tests/app/main/views/test_service_settings.py

@@ -823,7 +823,7 @@ def test_non_gov_user_is_told_they_cant_go_live(
     ),
     (
         (('email', 1234), ('sms', 0), ('letter', 999)),
-        ('1234', '0', '999'),
+        ('1,234', '0', '999'),
     ),
 ))
 def test_should_show_estimate_volumes(
@@ -880,9 +880,9 @@ def test_should_show_persist_estimated_volumes(
         'main.estimate_usage',
         service_id=SERVICE_ONE_ID,
         _data={
-            'volume_email': '1234',
-            'volume_sms': '0',
-            'volume_letter': '9876',
+            'volume_email': '1,234,567',
+            'volume_sms': '',
+            'volume_letter': '098',
             'consent_to_research': consent_to_research,
         },
         _expected_status=302,
@@ -894,25 +894,25 @@ def test_should_show_persist_estimated_volumes(
     )
     mock_update_service.assert_called_once_with(
         SERVICE_ONE_ID,
-        volume_email='1234',
-        volume_sms='0',
-        volume_letter='9876',
+        volume_email=1234567,
+        volume_sms=0,
+        volume_letter=98,
         consent_to_research=expected_persisted_consent_to_research,
     )
 
 
 @pytest.mark.parametrize('data, error_selector, expected_error_message', (
     (
         {
-            'volume_email': '',
-            'volume_sms': '0',
+            'volume_email': '1234',
+            'volume_sms': '2147483648',
             'volume_letter': '9876',
             'consent_to_research': 'yes',
         },
-        'label[for=volume_email]',
+        'label[for=volume_sms]',
         (
-            'How many emails do you expect to send in the next year? For example, 1,000,000 '
-            'Can’t be empty'
+            'How many text messages do you expect to send in the next year? For example, 500,000 '
+            'Must be less than 2,147,483,647'
         )
     ),
     (
@@ -939,8 +939,28 @@ def test_should_error_if_bad_estimations_given(
         _data=data,
         _expected_status=200,
     )
-    assert normalize_spaces(page.select_one(error_selector).text) == (
-        expected_error_message
+    assert normalize_spaces(page.select_one(error_selector).text) == expected_error_message
+    assert mock_update_service.called is False
+
+
+def test_should_error_if_all_volumes_zero(
+    client_request,
+    mock_update_service,
+):
+    page = client_request.post(
+        'main.estimate_usage',
+        service_id=SERVICE_ONE_ID,
+        _data={
+            'volume_email': '',
+            'volume_sms': '0',
+            'volume_letter': '0,000',
+            'consent_to_research': 'yes',
+        },
+        _expected_status=200,
+    )
+    assert normalize_spaces(page.select_one('.banner-dangerous').text) == (
+        'no things supplied '
+        'Tell us some things'
     )
     assert mock_update_service.called is False