Merge pull request #4004 from alphagov/areas-refactor-178986763

Prepare to migrate to new format for "areas"

app/broadcast_areas/models.py

@@ -184,8 +184,8 @@ class CustomBroadcastArea(BaseBroadcastArea):
 class CustomBroadcastAreas(SerialisedModelCollection):
     model = CustomBroadcastArea
 
-    def __init__(self, *, areas, polygons):
-        self.items = areas
+    def __init__(self, *, area_ids, polygons):
+        self.items = area_ids
         self._polygons = polygons
 
     def __getitem__(self, index):

app/commands.py

@@ -1,11 +1,32 @@
+import click
 from flask import current_app
+from flask.cli import with_appcontext
 
 
+@click.command('list-routes')
+@with_appcontext
 def list_routes():
     """List URLs of all application routes."""
     for rule in sorted(current_app.url_map.iter_rules(), key=lambda r: r.rule):
         print("{:10} {}".format(", ".join(rule.methods - set(['OPTIONS', 'HEAD'])), rule.rule))  # noqa
 
 
+@click.command()
+@click.argument('csv_path')
+@with_appcontext
+def tmp_backfill_areas(csv_path, dry_run=True):
+    import csv
+
+    from app.models.broadcast_message import BroadcastMessage
+
+    for id, service_id in csv.reader(open(csv_path)):
+        message = BroadcastMessage.from_id(id, service_id=service_id)
+        print(f'Updating {message.id}')  # noqa
+
+        if not dry_run:
+            message._update_areas(force_override=True)
+
+
 def setup_commands(application):
-    application.cli.command('list-routes')(list_routes)
+    application.cli.add_command(list_routes)
+    application.cli.add_command(tmp_backfill_areas)

app/models/broadcast_message.py

@@ -88,21 +88,30 @@ class BroadcastMessage(JSONModel):
 
     @property
     def areas(self):
-        library_areas = self.get_areas(areas=self._dict['areas'])
+        polygons = self._dict['areas_2']['simple_polygons']
+        library_areas = self.get_areas(self.area_ids)
 
         if library_areas:
-            if len(library_areas) != len(self._dict['areas']):
+            if len(library_areas) != len(self.area_ids):
                 raise RuntimeError(
-                    f'BroadcastMessage has {len(self._dict["areas"])} areas '
+                    f'BroadcastMessage has {len(self.area_ids)} areas '
                     f'but {len(library_areas)} found in the library'
                 )
             return library_areas
 
         return CustomBroadcastAreas(
-            areas=self._dict['areas'],
-            polygons=self._dict['simple_polygons'],
+            area_ids=self.area_ids,
+            polygons=polygons,
         )
 
+    @property
+    def area_ids(self):
+        return self._dict['areas_2']['ids']
+
+    @area_ids.setter
+    def area_ids(self, value):
+        self._dict['areas_2']['ids'] = value
+
     @property
     def ancestor_areas(self):
         return sorted(set(self._ancestor_areas_iterator))
@@ -201,9 +210,9 @@ class BroadcastMessage(JSONModel):
 
         return round_to_significant_figures(count, 1)
 
-    def get_areas(self, areas):
+    def get_areas(self, area_ids):
         return broadcast_area_libraries.get_areas(
-            areas
+            area_ids
         )
 
     def get_simple_polygons(self, areas):
@@ -216,20 +225,13 @@ class BroadcastMessage(JSONModel):
         # combined shapes to keep the point count down
         return polygons.smooth.simplify if len(areas) > 1 else polygons
 
-    def add_areas(self, *new_areas):
-        areas = list(OrderedSet(
-            self._dict['areas'] + list(new_areas)
-        ))
-        simple_polygons = self.get_simple_polygons(areas=self.get_areas(areas=areas))
-        self._update(areas=areas, simple_polygons=simple_polygons.as_coordinate_pairs_lat_long)
+    def add_areas(self, *new_area_ids):
+        self.area_ids = list(OrderedSet(self.area_ids + list(new_area_ids)))
+        self._update_areas()
 
-    def remove_area(self, area_to_remove):
-        areas = [
-            area for area in self._dict['areas']
-            if area != area_to_remove
-        ]
-        simple_polygons = self.get_simple_polygons(areas=self.get_areas(areas=areas))
-        self._update(areas=areas, simple_polygons=simple_polygons.as_coordinate_pairs_lat_long)
+    def remove_area(self, area_id):
+        self.area_ids = list(set(self._dict['areas_2']['ids']) - {area_id})
+        self._update_areas()
 
     def _set_status_to(self, status):
         broadcast_message_api_client.update_broadcast_message_status(
@@ -238,6 +240,21 @@ class BroadcastMessage(JSONModel):
             service_id=self.service_id,
         )
 
+    def _update_areas(self, force_override=False):
+        areas_2 = {
+            'ids': self.area_ids,
+            'names': [area.name for area in self.areas],
+            'simple_polygons': self.simple_polygons.as_coordinate_pairs_lat_long
+        }
+
+        data = {'areas_2': areas_2}
+
+        # TEMPORARY: while we migrate to a new format for "areas"
+        if force_override:
+            data['force_override'] = True
+
+        self._update(**data)
+
     def _update(self, **kwargs):
         broadcast_message_api_client.update_broadcast_message(
             broadcast_message_id=self.id,

tests/__init__.py

@@ -678,7 +678,7 @@ def broadcast_message_json(
     updated_at=None,
     approved_by_id=None,
     cancelled_by_id=None,
-    areas=None,
+    area_ids=None,
     simple_polygons=None,
     content=None,
     reference=None,
@@ -696,10 +696,10 @@ def broadcast_message_json(
         'reference': reference,
 
         'personalisation': {},
-        'areas': areas or [
-            'ctry19-E92000001', 'ctry19-S92000003',
-        ],
-        'simple_polygons': simple_polygons or [],
+        'areas_2': {
+            'ids': area_ids or ['ctry19-E92000001', 'ctry19-S92000003'],
+            'simple_polygons': simple_polygons or [],
+        },
 
         'status': status,
 

tests/app/main/views/test_broadcast.py

@@ -904,7 +904,7 @@ def test_preview_broadcast_areas_page(
             created_by_id=fake_uuid,
             service_id=SERVICE_ONE_ID,
             status='draft',
-            areas=areas_selected,
+            area_ids=areas_selected,
         ),
     )
     client_request.login(active_user_create_broadcasts_permission)
@@ -974,7 +974,7 @@ def test_preview_broadcast_areas_page_with_custom_polygons(
             created_by_id=fake_uuid,
             service_id=SERVICE_ONE_ID,
             status='draft',
-            areas=['Area one', 'Area two', 'Area three'],
+            area_ids=['Area one', 'Area two', 'Area three'],
             simple_polygons=polygons,
         ),
     )
@@ -1000,7 +1000,7 @@ def test_preview_broadcast_areas_page_with_custom_polygons(
     ] == expected_list_items
 
 
-@pytest.mark.parametrize('areas, expected_list', (
+@pytest.mark.parametrize('area_ids, expected_list', (
     ([], [
         'Countries',
         'Demo areas',
@@ -1053,7 +1053,7 @@ def test_choose_broadcast_library_page(
     service_one,
     fake_uuid,
     active_user_create_broadcasts_permission,
-    areas,
+    area_ids,
     expected_list,
 ):
     service_one['permissions'] += ['broadcast']
@@ -1065,7 +1065,7 @@ def test_choose_broadcast_library_page(
             created_by_id=fake_uuid,
             service_id=SERVICE_ONE_ID,
             status='draft',
-            areas=areas,
+            area_ids=area_ids,
         ),
     )
     client_request.login(active_user_create_broadcasts_permission)
@@ -1108,7 +1108,7 @@ def test_suggested_area_has_correct_link(
             created_by_id=fake_uuid,
             service_id=SERVICE_ONE_ID,
             status='draft',
-            areas=[
+            area_ids=[
                 'wd20-E05004299',  # Pitville, a ward of Cheltenham
             ],
         ),
@@ -1395,30 +1395,34 @@ def test_add_broadcast_area(
         service_id=SERVICE_ONE_ID,
         broadcast_message_id=fake_uuid,
         data={
-            'areas': ['ctry19-E92000001', 'ctry19-S92000003', 'ctry19-W92000004'], 'simple_polygons': coordinates
+            'areas_2': {
+                'ids': ['ctry19-E92000001', 'ctry19-S92000003', 'ctry19-W92000004'],
+                'names': ['England', 'Scotland', 'Wales'],
+                'simple_polygons': coordinates
+            }
         },
     )
 
 
-@pytest.mark.parametrize('post_data, expected_selected', (
-    ({
-        'select_all': 'y',
-        'areas': [
-            'wd20-S13002845',
-        ]
-    }, [
-        'lad20-S12000033',
-        # wd20-S13002845 is ignored because the user chose ‘Select all…’
-    ]),
-    ({
-        'areas': [
-            'wd20-S13002845',
-            'wd20-S13002836',
-        ]
-    }, [
-        'wd20-S13002845',
-        'wd20-S13002836',
-    ]),
+@pytest.mark.parametrize('post_data, expected_data', (
+    (
+        {
+            'select_all': 'y', 'areas': ['wd20-S13002845']
+        },
+        {
+            # wd20-S13002845 is ignored because the user chose ‘Select all…’
+            'ids': ['lad20-S12000033'], 'names': ['Aberdeen City']
+        }
+    ),
+    (
+        {
+            'areas': ['wd20-S13002845', 'wd20-S13002836']
+        },
+        {
+            'ids': ['wd20-S13002845', 'wd20-S13002836'],
+            'names': ['Bridge of Don', 'Airyhall/Broomhill/Garthdee'],
+        }
+    ),
 ))
 def test_add_broadcast_sub_area_district_view(
     client_request,
@@ -1427,7 +1431,7 @@ def test_add_broadcast_sub_area_district_view(
     mock_update_broadcast_message,
     fake_uuid,
     post_data,
-    expected_selected,
+    expected_data,
     mocker,
     active_user_create_broadcasts_permission,
 ):
@@ -1446,16 +1450,19 @@ def test_add_broadcast_sub_area_district_view(
         area_slug='lad20-S12000033',
         _data=post_data,
     )
+
+    # 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']
+
     mock_update_broadcast_message.assert_called_once_with(
         service_id=SERVICE_ONE_ID,
         broadcast_message_id=fake_uuid,
         data={
-            'simple_polygons': coordinates,
-            'areas': [
-                # These two areas are on the broadcast already
-                'ctry19-E92000001',
-                'ctry19-S92000003',
-            ] + expected_selected
+            'areas_2': {
+                'simple_polygons': coordinates,
+                **expected_data,
+            }
         },
     )
 
@@ -1488,14 +1495,17 @@ def test_add_broadcast_sub_area_county_view(
         service_id=SERVICE_ONE_ID,
         broadcast_message_id=fake_uuid,
         data={
-            'simple_polygons': coordinates,
-            'areas': [
-                # These two areas are on the broadcast already
-                'ctry19-E92000001',
-                'ctry19-S92000003',
-            ] + [
-                'ctyua19-E10000016'
-            ]
+            'areas_2': {
+                'simple_polygons': coordinates,
+                'ids': [
+                    # These two areas are on the broadcast already
+                    'ctry19-E92000001',
+                    'ctry19-S92000003',
+                ] + [
+                    'ctyua19-E10000016'
+                ],
+                'names': ['England', 'Scotland', 'Kent']
+            }
         },
     )
 
@@ -1532,8 +1542,11 @@ def test_remove_broadcast_area_page(
         service_id=SERVICE_ONE_ID,
         broadcast_message_id=fake_uuid,
         data={
-            'simple_polygons': coordinates,
-            'areas': ['ctry19-S92000003']
+            'areas_2': {
+                'simple_polygons': coordinates,
+                'names': ['Scotland'],
+                'ids': ['ctry19-S92000003']
+            },
         },
     )
 

tests/app/models/test_broadcast_message.py

@@ -11,7 +11,7 @@ def test_simple_polygons(fake_uuid):
         template_id=fake_uuid,
         status='draft',
         created_by_id=fake_uuid,
-        areas=[
+        area_ids=[
             # Hackney Central
             'wd20-E05009372',
             # Hackney Wick
@@ -56,7 +56,7 @@ def test_raises_for_missing_areas(fake_uuid):
         template_id=fake_uuid,
         status='draft',
         created_by_id=fake_uuid,
-        areas=[
+        area_ids=[
             'wd20-E05009372',
             'something else',
         ],