From 61ac7fa0692e26c295ea4f4a832e1e747e2b07a1 Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Wed, 27 Feb 2019 12:05:28 +0000
Subject: [PATCH] =?UTF-8?q?Don=E2=80=99t=20reformat=20numbers=20if=20there?=
 =?UTF-8?q?=20are=20errors?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

It’s confusing to at the same time:
1. change what you’ve inputted
2. tell you it’s wrong

This commit makes it so that 1. only happens if 2. doesn’t.
---
 app/main/forms.py                             | 16 +++++++---
 tests/app/main/views/test_service_settings.py | 31 +++++++++++++++++++
 2 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/app/main/forms.py b/app/main/forms.py
index 546bab1a4..98a064fc2 100644
--- a/app/main/forms.py
+++ b/app/main/forms.py
@@ -181,17 +181,17 @@ class ForgivingIntegerField(StringField):
 
         if valuelist:
 
-            valuelist[0] = valuelist[0].replace(',', '').replace(' ', '')
+            value = valuelist[0].replace(',', '').replace(' ', '')
 
             try:
-                valuelist[0] = int(valuelist[0])
+                value = int(value)
             except ValueError:
                 pass
 
-            if valuelist[0] == '':
-                valuelist[0] = 0
+            if value == '':
+                value = 0
 
-        return super().process_formdata(valuelist)
+        return super().process_formdata([value])
 
     def pre_validate(self, form):
 
@@ -210,6 +210,12 @@ class ForgivingIntegerField(StringField):
 
     def __call__(self, **kwargs):
 
+        if self.get_form().is_submitted() and not self.get_form().validate():
+            return super().__call__(
+                value=(self.raw_data or [None])[0],
+                **kwargs
+            )
+
         try:
             value = int(self.data)
             value = '{:,.0f}'.format(value)
diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py
index 82c79827d..fce926ea1 100644
--- a/tests/app/main/views/test_service_settings.py
+++ b/tests/app/main/views/test_service_settings.py
@@ -968,6 +968,9 @@ def test_should_error_if_all_volumes_zero(
         },
         _expected_status=200,
     )
+    assert page.select('input[type=text]')[0]['value'] == ''
+    assert page.select('input[type=text]')[1]['value'] == '0'
+    assert page.select('input[type=text]')[2]['value'] == '0,00 0'
     assert normalize_spaces(page.select_one('.banner-dangerous').text) == (
         'no things supplied '
         'Tell us some things'
@@ -975,6 +978,34 @@ def test_should_error_if_all_volumes_zero(
     assert mock_update_service.called is False
 
 
+def test_should_not_default_to_zero_if_some_fields_dont_validate(
+    client_request,
+    mock_update_service,
+):
+    page = client_request.post(
+        'main.estimate_usage',
+        service_id=SERVICE_ONE_ID,
+        _data={
+            'volume_email': '1234',
+            'volume_sms': '',
+            'volume_letter': 'aaaaaaaaaaaaa',
+            'consent_to_research': 'yes',
+        },
+        _expected_status=200,
+    )
+    assert page.select('input[type=text]')[0]['value'] == '1234'
+    assert page.select('input[type=text]')[1]['value'] == ''
+    assert page.select('input[type=text]')[2]['value'] == 'aaaaaaaaaaaaa'
+    assert normalize_spaces(
+        page.select_one('label[for=volume_letter]').text
+    ) == (
+        'How many letters do you expect to send in the next year? '
+        'For example, 5,000 '
+        'Must be a whole number'
+    )
+    assert mock_update_service.called is False
+
+
 def test_non_gov_users_cant_request_to_go_live(
     client_request,
     api_nongov_user_active,