diff --git a/app/broadcast_areas/models.py b/app/broadcast_areas/models.py index d7776c4ed..6084cd7c9 100644 --- a/app/broadcast_areas/models.py +++ b/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): diff --git a/app/commands.py b/app/commands.py index 026be777a..67a971f6c 100644 --- a/app/commands.py +++ b/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) diff --git a/app/models/broadcast_message.py b/app/models/broadcast_message.py index 112e234f7..c794b7461 100644 --- a/app/models/broadcast_message.py +++ b/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, diff --git a/tests/__init__.py b/tests/__init__.py index 9763c0eef..e296d0c1f 100644 --- a/tests/__init__.py +++ b/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, diff --git a/tests/app/main/views/test_broadcast.py b/tests/app/main/views/test_broadcast.py index 589bc9205..9d51c6725 100644 --- a/tests/app/main/views/test_broadcast.py +++ b/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'] + }, }, ) diff --git a/tests/app/models/test_broadcast_message.py b/tests/app/models/test_broadcast_message.py index f15a7c721..6eacc3f25 100644 --- a/tests/app/models/test_broadcast_message.py +++ b/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', ],