From eb4a7907a45011adcf012a4b12e4c22c9614c474 Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Thu, 24 Sep 2020 15:52:18 +0100
Subject: [PATCH] Make estimated phone count clearer
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

We’ve had some feedback from user research that difference between
‘will get alert’ and ‘likely to get alert’ is not clear, and it’s hard
to tell if the latter is inclusive of the former. This leads people to
question the validity of these numbers, which is important, because an
the estimate should give you some idea of the impact of what you’re
about to do.

This commit reformats the number as a range, for example 1,000 to 2,000
phones.

If the range is small, eg 40,000,000 to 40,800,000 then this suggests
a false level of accuracy. So instead we just give one number and say
it’s an estimate, eg ‘40,000,000 phones estimated’
---
 app/models/broadcast_message.py               |  4 +-
 .../views/broadcast/macros/area-map.html      | 10 +++-
 tests/app/main/views/test_broadcast.py        | 56 ++++++++++++++++---
 3 files changed, 57 insertions(+), 13 deletions(-)

diff --git a/app/models/broadcast_message.py b/app/models/broadcast_message.py
index 87b431b37..c5951b152 100644
--- a/app/models/broadcast_message.py
+++ b/app/models/broadcast_message.py
@@ -128,7 +128,7 @@ class BroadcastMessage(JSONModel):
     def count_of_phones(self):
         return round_to_significant_figures(
             sum(area.count_of_phones for area in self.areas),
-            2
+            1
         )
 
     @property
@@ -136,7 +136,7 @@ class BroadcastMessage(JSONModel):
         area_estimate = self.simple_polygons.estimated_area
         bleed_area_estimate = self.simple_polygons.bleed.estimated_area - area_estimate
         return round_to_significant_figures(
-            self.count_of_phones * bleed_area_estimate / area_estimate,
+            self.count_of_phones + (self.count_of_phones * bleed_area_estimate / area_estimate),
             1
         )
 
diff --git a/app/templates/views/broadcast/macros/area-map.html b/app/templates/views/broadcast/macros/area-map.html
index 53a991dc9..32b0ce8ee 100644
--- a/app/templates/views/broadcast/macros/area-map.html
+++ b/app/templates/views/broadcast/macros/area-map.html
@@ -7,7 +7,7 @@
       </span>
       Will get
       <span class="visually-hidden">the</span>
-      alert ({{ broadcast_message.count_of_phones|format_thousands }} phones)
+      alert
     </li>
     <li class="area-key area-key--likely">
       <span class="visually-hidden">
@@ -18,7 +18,13 @@
         the
       </span>
       alert
-      ({{ broadcast_message.count_of_phones_likely|format_thousands }} phones)
+    </li>
+    <li class="area-key area-key--phone-estimate">
+      {% if broadcast_message.count_of_phones == broadcast_message.count_of_phones_likely %}
+        {{ broadcast_message.count_of_phones|format_thousands }} phones estimated
+      {% else %}
+        {{ broadcast_message.count_of_phones|format_thousands }} to {{ broadcast_message.count_of_phones_likely|format_thousands }} phones
+      {% endif %}
     </li>
   </ul>
 {% endmacro %}
diff --git a/tests/app/main/views/test_broadcast.py b/tests/app/main/views/test_broadcast.py
index b17f5c93e..35963b642 100644
--- a/tests/app/main/views/test_broadcast.py
+++ b/tests/app/main/views/test_broadcast.py
@@ -368,13 +368,57 @@ def test_broadcast_page(
     ),
 
 
+@pytest.mark.parametrize('areas_selected, areas_listed, estimates', (
+    ([
+        'ctry19-E92000001',
+        'ctry19-S92000003',
+    ], [
+        'England remove',
+        'Scotland remove',
+    ], [
+        'An area of 176,714.9 square miles Will get the alert',
+        'An extra area of 3,052.8 square miles is Likely to get the alert',
+        '40,000,000 phones estimated',
+    ]),
+    ([
+        'wd20-E05003224',
+        'wd20-E05003225',
+        'wd20-E05003227',
+        'wd20-E05003228',
+        'wd20-E05003229',
+    ], [
+        'Penrith Carleton remove',
+        'Penrith East remove',
+        'Penrith Pategill remove',
+        'Penrith South remove',
+        'Penrith West remove',
+    ], [
+        'An area of 6.3 square miles Will get the alert',
+        'An extra area of 14.4 square miles is Likely to get the alert',
+        '9,000 to 30,000 phones',
+    ]),
+))
 def test_preview_broadcast_areas_page(
+    mocker,
     client_request,
     service_one,
     fake_uuid,
-    mock_get_draft_broadcast_message,
+    areas_selected,
+    areas_listed,
+    estimates,
 ):
     service_one['permissions'] += ['broadcast']
+    mocker.patch(
+        'app.broadcast_message_api_client.get_broadcast_message',
+        return_value=broadcast_message_json(
+            id_=fake_uuid,
+            template_id=fake_uuid,
+            created_by_id=fake_uuid,
+            service_id=SERVICE_ONE_ID,
+            status='draft',
+            areas=areas_selected,
+        ),
+    )
     page = client_request.get(
         '.preview_broadcast_areas',
         service_id=SERVICE_ONE_ID,
@@ -384,20 +428,14 @@ def test_preview_broadcast_areas_page(
     assert [
         normalize_spaces(item.text)
         for item in page.select('ul.area-list li.area-list-item')
-    ] == [
-        'England remove',
-        'Scotland remove',
-    ]
+    ] == areas_listed
 
     assert len(page.select('#map')) == 1
 
     assert [
         normalize_spaces(item.text)
         for item in page.select('ul li.area-key')
-    ] == [
-        'An area of 176,714.9 square miles Will get the alert (44,000,000 phones)',
-        'An extra area of 3,052.8 square miles is Likely to get the alert (800,000 phones)',
-    ]
+    ] == estimates
 
 
 @pytest.mark.parametrize('areas, expected_list', (