From 4bae4eec6cd85f10c1a22bd2d7645ad8e02b71e9 Mon Sep 17 00:00:00 2001
From: Ben Thorner <ben.thorner@digital.cabinet-office.gov.uk>
Date: Fri, 27 Aug 2021 14:58:45 +0100
Subject: [PATCH] Start sending aggregate area names to API

The aggregate names don't need to be sorted, but it helps to do
this for the tests, since the implementation of sets may not be
stable between machines and lead to sporadic test failures.

I did also toy with sorting granular area names so they have a
similar ordering, but this would take them out-of-order with IDs
and really the important thing is that the ordering is stable.
---
 app/broadcast_areas/utils.py            |  2 +-
 app/models/broadcast_message.py         |  2 ++
 tests/app/broadcast_areas/test_utils.py |  8 ++++----
 tests/app/main/views/test_broadcast.py  | 11 +++++++++--
 4 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/app/broadcast_areas/utils.py b/app/broadcast_areas/utils.py
index cd4ec378d..48766073d 100644
--- a/app/broadcast_areas/utils.py
+++ b/app/broadcast_areas/utils.py
@@ -7,7 +7,7 @@ def aggregate_areas(areas):
     areas = _convert_custom_areas_to_wards(areas)
     areas = _aggregate_wards_by_local_authority(areas)
     areas = _aggregate_lower_tier_authorities(areas)
-    return areas
+    return sorted(areas)
 
 
 def _convert_custom_areas_to_wards(areas):
diff --git a/app/models/broadcast_message.py b/app/models/broadcast_message.py
index 8b9878a8c..5e1a5deb4 100644
--- a/app/models/broadcast_message.py
+++ b/app/models/broadcast_message.py
@@ -11,6 +11,7 @@ from app.broadcast_areas.models import (
     CustomBroadcastAreas,
     broadcast_area_libraries,
 )
+from app.broadcast_areas.utils import aggregate_areas
 from app.formatters import round_to_significant_figures
 from app.models import JSONModel, ModelList
 from app.models.user import User
@@ -244,6 +245,7 @@ class BroadcastMessage(JSONModel):
         areas = {
             'ids': self.area_ids,
             'names': [area.name for area in self.areas],
+            'aggregate_names': [area.name for area in aggregate_areas(self.areas)],
             'simple_polygons': self.simple_polygons.as_coordinate_pairs_lat_long
         }
 
diff --git a/tests/app/broadcast_areas/test_utils.py b/tests/app/broadcast_areas/test_utils.py
index c95b75eff..8ed121995 100644
--- a/tests/app/broadcast_areas/test_utils.py
+++ b/tests/app/broadcast_areas/test_utils.py
@@ -99,9 +99,9 @@ def test_aggregate_areas(
         broadcast_message_json(area_ids=area_ids)
     )
 
-    assert sorted(
+    assert [
         area.name for area in aggregate_areas(broadcast_message.areas)
-    ) == expected_area_names
+    ] == expected_area_names
 
 
 @pytest.mark.parametrize(('simple_polygons', 'expected_area_names'), [
@@ -180,6 +180,6 @@ def test_aggregate_areas_for_custom_polygons(
         )
     )
 
-    assert sorted(
+    assert [
         area.name for area in aggregate_areas(broadcast_message.areas)
-    ) == expected_area_names
+    ] == expected_area_names
diff --git a/tests/app/main/views/test_broadcast.py b/tests/app/main/views/test_broadcast.py
index 414b1502c..e50b0a858 100644
--- a/tests/app/main/views/test_broadcast.py
+++ b/tests/app/main/views/test_broadcast.py
@@ -1398,6 +1398,7 @@ def test_add_broadcast_area(
             'areas': {
                 'ids': ['ctry19-E92000001', 'ctry19-S92000003', 'ctry19-W92000004'],
                 'names': ['England', 'Scotland', 'Wales'],
+                'aggregate_names': ['England', 'Scotland', 'Wales'],
                 'simple_polygons': coordinates
             }
         },
@@ -1411,7 +1412,9 @@ def test_add_broadcast_area(
         },
         {
             # wd20-S13002845 is ignored because the user chose ‘Select all…’
-            'ids': ['lad20-S12000033'], 'names': ['Aberdeen City']
+            'ids': ['lad20-S12000033'],
+            'names': ['Aberdeen City'],
+            'aggregate_names': ['Aberdeen City']
         }
     ),
     (
@@ -1421,6 +1424,7 @@ def test_add_broadcast_area(
         {
             'ids': ['wd20-S13002845', 'wd20-S13002836'],
             'names': ['Bridge of Don', 'Airyhall/Broomhill/Garthdee'],
+            'aggregate_names': ['Aberdeen City'],
         }
     ),
 ))
@@ -1454,6 +1458,7 @@ def test_add_broadcast_sub_area_district_view(
     # These two areas are on the broadcast already
     expected_data['ids'] = ['ctry19-E92000001', 'ctry19-S92000003'] + expected_data['ids']
     expected_data['names'] = ['England', 'Scotland'] + expected_data['names']
+    expected_data['aggregate_names'] = sorted(['England', 'Scotland'] + expected_data['aggregate_names'])
 
     mock_update_broadcast_message.assert_called_once_with(
         service_id=SERVICE_ONE_ID,
@@ -1504,7 +1509,8 @@ def test_add_broadcast_sub_area_county_view(
                 ] + [
                     'ctyua19-E10000016'
                 ],
-                'names': ['England', 'Scotland', 'Kent']
+                'names': ['England', 'Scotland', 'Kent'],
+                'aggregate_names': ['England', 'Kent', 'Scotland']
             }
         },
     )
@@ -1545,6 +1551,7 @@ def test_remove_broadcast_area_page(
             'areas': {
                 'simple_polygons': coordinates,
                 'names': ['Scotland'],
+                'aggregate_names': ['Scotland'],
                 'ids': ['ctry19-S92000003']
             },
         },