From 959dd6ac388f0d93a94f1b02877eac395b1ada46 Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Mon, 8 Jul 2019 14:09:29 +0100
Subject: [PATCH] Make one method for comma-formatting numbers

We were doing this a few different ways in different places.
---
 app/__init__.py                    |  9 +++++++--
 app/main/forms.py                  |  7 ++++---
 app/main/views/jobs.py             |  5 +++--
 app/main/views/service_settings.py | 11 ++++-------
 4 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/app/__init__.py b/app/__init__.py
index 43c93f90f..2ff39d8e4 100644
--- a/app/__init__.py
+++ b/app/__init__.py
@@ -3,6 +3,7 @@ import os
 import urllib
 from datetime import datetime, timedelta, timezone
 from functools import partial
+from numbers import Number
 from time import monotonic
 
 import ago
@@ -345,8 +346,12 @@ def format_delta(date):
     )
 
 
-def format_thousands(number):
-    return "{:,.0f}".format(number)
+def format_thousands(value):
+    if isinstance(value, Number):
+        return '{:,.0f}'.format(value)
+    if value is None:
+        return ''
+    return value
 
 
 def valid_phone_number(phone_number):
diff --git a/app/main/forms.py b/app/main/forms.py
index 6ab4e1035..2017b6433 100644
--- a/app/main/forms.py
+++ b/app/main/forms.py
@@ -34,6 +34,7 @@ from wtforms.fields.html5 import EmailField, SearchField, TelField
 from wtforms.validators import URL, DataRequired, Length, Optional, Regexp
 from wtforms.widgets import CheckboxInput, ListWidget
 
+from app import format_thousands
 from app.main.validators import (
     Blacklist,
     CsvFileValidator,
@@ -209,9 +210,9 @@ class ForgivingIntegerField(StringField):
             error = None
             try:
                 if int(self.data) > self.POSTGRES_MAX_INT:
-                    error = 'Number of {} must be {:,.0f} or less'.format(
+                    error = 'Number of {} must be {} or less'.format(
                         self.things,
-                        self.POSTGRES_MAX_INT,
+                        format_thousands(self.POSTGRES_MAX_INT),
                     )
             except ValueError:
                 error = 'Enter the number of {} {}'.format(
@@ -234,7 +235,7 @@ class ForgivingIntegerField(StringField):
 
         try:
             value = int(self.data)
-            value = '{:,.0f}'.format(value)
+            value = format_thousands(value)
         except (ValueError, TypeError):
             value = self.data if self.data is not None else ''
 
diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py
index acb1a4346..8d1bc5b48 100644
--- a/app/main/views/jobs.py
+++ b/app/main/views/jobs.py
@@ -25,6 +25,7 @@ from notifications_utils.template import Template, WithSubjectTemplate
 from app import (
     current_service,
     format_datetime_short,
+    format_thousands,
     job_api_client,
     notification_api_client,
     service_api_client,
@@ -186,8 +187,8 @@ def cancel_letter_job(service_id, job_id):
         except HTTPError as e:
             flash(e.message, 'dangerous')
             return redirect(url_for('main.view_job', service_id=service_id, job_id=job_id))
-        flash("Cancelled {:,.0f} letters from {}".format(
-            number_of_letters, job['original_file_name']
+        flash("Cancelled {} letters from {}".format(
+            format_thousands(number_of_letters), job['original_file_name']
         ), 'default_with_tick')
         return redirect(url_for('main.service_dashboard', service_id=service_id))
 
diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py
index 9a50c48df..46db6b1c9 100644
--- a/app/main/views/service_settings.py
+++ b/app/main/views/service_settings.py
@@ -19,6 +19,7 @@ from app import (
     billing_api_client,
     current_service,
     email_branding_client,
+    format_thousands,
     inbound_number_client,
     letter_branding_client,
     notification_api_client,
@@ -212,9 +213,9 @@ def submit_request_to_go_live(service_id):
             service_dashboard=url_for('main.service_dashboard', service_id=current_service.id, _external=True),
             organisation_type=str(current_service.organisation_type).title(),
             agreement=current_service.organisation.as_human_readable(current_user.email_domain),
-            volume_email_formatted=format_if_number(current_service.volume_email),
-            volume_sms_formatted=format_if_number(current_service.volume_sms),
-            volume_letter_formatted=format_if_number(current_service.volume_letter),
+            volume_email_formatted=format_thousands(current_service.volume_email),
+            volume_sms_formatted=format_thousands(current_service.volume_sms),
+            volume_letter_formatted=format_thousands(current_service.volume_letter),
             research_consent='Yes' if current_service.consent_to_research else 'No',
             existing_live='Yes' if current_user.live_services else 'No',
             email_address=current_user.email_address,
@@ -1121,7 +1122,3 @@ def check_contact_details_type(contact_details):
         return 'email_address'
     else:
         return 'phone_number'
-
-
-def format_if_number(value):
-    return '{:,.0f}'.format(value) if isinstance(value, int) else ''