diff --git a/app/broadcast_areas/models.py b/app/broadcast_areas/models.py index 4af79ed54..b33ee1ee4 100644 --- a/app/broadcast_areas/models.py +++ b/app/broadcast_areas/models.py @@ -86,6 +86,12 @@ class BroadcastArea(BaseBroadcastArea, SortableMixin): def __init__(self, row): self.id, self.name, self._count_of_phones, self.library_id = row + @classmethod + def from_row_with_simple_polygons(cls, row): + instance = cls(row[:4]) + instance.simple_polygons = Polygons(row[4]) + return instance + @cached_property def polygons(self): return Polygons( @@ -158,11 +164,11 @@ class CustomBroadcastArea(BaseBroadcastArea): simple_polygons = polygons - @property + @cached_property def overlapping_areas(self): if not self.polygons: return [] - return broadcast_area_libraries.get_areas([ + return broadcast_area_libraries.get_areas_with_simple_polygons([ overlap.data for overlap in rtree_index.query( Rect(*self.polygons.bounds) ) @@ -171,7 +177,7 @@ class CustomBroadcastArea(BaseBroadcastArea): @cached_property def count_of_phones(self): return sum( - area.polygons.ratio_of_intersection_with(self.polygons) * area.count_of_phones + area.simple_polygons.ratio_of_intersection_with(self.polygons) * area.count_of_phones for area in self.overlapping_areas ) @@ -221,13 +227,13 @@ class BroadcastAreaLibraries(SerialisedModelCollection, GetItemByIdMixin): def __init__(self): self.items = BroadcastAreasRepository().get_libraries() - def get_areas(self, *area_ids): - # allow people to call `get_areas('a', 'b') or get_areas(['a', 'b'])` - if len(area_ids) == 1 and isinstance(area_ids[0], list): - area_ids = area_ids[0] - + def get_areas(self, area_ids): areas = BroadcastAreasRepository().get_areas(area_ids) return [BroadcastArea(area) for area in areas] + def get_areas_with_simple_polygons(self, area_ids): + areas = BroadcastAreasRepository().get_areas_with_simple_polygons(area_ids) + return [BroadcastArea.from_row_with_simple_polygons(area) for area in areas] + broadcast_area_libraries = BroadcastAreaLibraries() diff --git a/app/broadcast_areas/repo.py b/app/broadcast_areas/repo.py index 91014077a..b18a46456 100644 --- a/app/broadcast_areas/repo.py +++ b/app/broadcast_areas/repo.py @@ -130,7 +130,7 @@ class BroadcastAreasRepository(object): SELECT id, name, count_of_phones, broadcast_area_library_id FROM broadcast_areas WHERE id IN ({}) - """.format(("?," * len(area_ids))[:-1]) + """.format(",".join("?" * len(area_ids))) results = self.query(q, *area_ids) @@ -141,6 +141,23 @@ class BroadcastAreasRepository(object): return areas + def get_areas_with_simple_polygons(self, area_ids): + q = """ + SELECT broadcast_areas.id, name, count_of_phones, broadcast_area_library_id, simple_polygons + FROM broadcast_areas + JOIN broadcast_area_polygons on broadcast_area_polygons.id = broadcast_areas.id + WHERE broadcast_areas.id IN ({}) + """.format(",".join("?" * len(area_ids))) + + results = self.query(q, *area_ids) + + areas = [ + (row[0], row[1], row[2], row[3], json.loads(row[4])) + for row in results + ] + + return areas + def get_all_areas_for_library(self, library_id): is_multi_tier_library = self.query(""" SELECT exists( diff --git a/app/main/views/broadcast.py b/app/main/views/broadcast.py index 1123b5c5b..e11a8fb30 100644 --- a/app/main/views/broadcast.py +++ b/app/main/views/broadcast.py @@ -287,7 +287,7 @@ def choose_broadcast_sub_area(service_id, broadcast_message_id, library_slug, ar broadcast_message_id, service_id=current_service.id, ) - area = BroadcastMessage.libraries.get_areas(area_slug)[0] + area = BroadcastMessage.libraries.get_areas([area_slug])[0] back_link = _get_broadcast_sub_area_back_link(service_id, broadcast_message_id, library_slug) diff --git a/app/models/broadcast_message.py b/app/models/broadcast_message.py index b1c0a8ad5..a3ea36499 100644 --- a/app/models/broadcast_message.py +++ b/app/models/broadcast_message.py @@ -18,9 +18,9 @@ from app.notify_client.broadcast_message_api_client import ( broadcast_message_api_client, ) -ESTIMATED_AREA_OF_LARGEST_UK_COUNTY = broadcast_area_libraries.get_areas( +ESTIMATED_AREA_OF_LARGEST_UK_COUNTY = broadcast_area_libraries.get_areas([ 'ctyua19-E10000023' # North Yorkshire -)[0].polygons.estimated_area +])[0].polygons.estimated_area class BroadcastMessage(JSONModel): @@ -172,14 +172,14 @@ class BroadcastMessage(JSONModel): def cancelled_by(self): return User.from_id(self.cancelled_by_id) - @property + @cached_property def count_of_phones(self): return round_to_significant_figures( sum(area.count_of_phones for area in self.areas), 1 ) - @property + @cached_property def count_of_phones_likely(self): estimated_area = self.simple_polygons.estimated_area @@ -203,7 +203,7 @@ class BroadcastMessage(JSONModel): def get_areas(self, areas): return broadcast_area_libraries.get_areas( - *areas + areas ) def get_simple_polygons(self, areas): diff --git a/tests/app/broadcast_areas/test_broadcast_area.py b/tests/app/broadcast_areas/test_broadcast_area.py index 448b76f00..4c2af384b 100644 --- a/tests/app/broadcast_areas/test_broadcast_area.py +++ b/tests/app/broadcast_areas/test_broadcast_area.py @@ -77,40 +77,25 @@ def test_loads_areas_from_libraries(id): assert ( broadcast_area_libraries.get('ctry19').get(id) ) == ( - broadcast_area_libraries.get_areas(id)[0] + broadcast_area_libraries.get_areas([id])[0] ) def test_get_names_of_areas(): - areas = broadcast_area_libraries.get_areas( + areas = broadcast_area_libraries.get_areas([ 'ctry19-W92000004', 'lad20-W06000014', 'ctry19-E92000001', - ) + ]) assert [area.name for area in sorted(areas)] == [ 'England', 'Vale of Glamorgan', 'Wales', ] -def test_get_areas_accepts_lists(): - areas_from_list = broadcast_area_libraries.get_areas( - [ - 'ctry19-W92000004', - 'ctry19-E92000001', - ] - ) - areas_from_args = broadcast_area_libraries.get_areas( - 'ctry19-W92000004', - 'ctry19-E92000001', - ) - assert len(areas_from_args) == len(areas_from_list) == 2 - assert areas_from_args == areas_from_list - - def test_has_polygons(): - england = broadcast_area_libraries.get_areas('ctry19-E92000001')[0] - scotland = broadcast_area_libraries.get_areas('ctry19-S92000003')[0] + england = broadcast_area_libraries.get_areas(['ctry19-E92000001'])[0] + scotland = broadcast_area_libraries.get_areas(['ctry19-S92000003'])[0] assert len(england.polygons) == 35 assert len(scotland.polygons) == 195 @@ -130,7 +115,7 @@ def test_polygons_are_enclosed(): def test_lat_long_order(): - england = broadcast_area_libraries.get_areas('ctry19-E92000001')[0] + england = broadcast_area_libraries.get_areas(['ctry19-E92000001'])[0] lat_long = england.polygons.as_coordinate_pairs_lat_long long_lat = england.polygons.as_coordinate_pairs_long_lat @@ -212,13 +197,13 @@ def test_every_area_has_count_of_phones(library): )) def test_count_of_phones_for_all_levels(area_id, area_name, expected_count): - area = broadcast_area_libraries.get_areas(area_id)[0] + area = broadcast_area_libraries.get_areas([area_id])[0] assert area.name == area_name assert area.count_of_phones == expected_count def test_city_of_london_counts_are_not_derived_from_population(): - city_of_london = broadcast_area_libraries.get_areas('lad20-E09000001')[0] + city_of_london = broadcast_area_libraries.get_areas(['lad20-E09000001'])[0] assert city_of_london.name == 'City of London' assert len(city_of_london.sub_areas) == len(CITY_OF_LONDON.WARDS) == 25 @@ -315,7 +300,7 @@ def test_phone_density( area, expected_phones_per_square_mile, ): assert close_enough( - broadcast_area_libraries.get_areas(area)[0].phone_density, + broadcast_area_libraries.get_areas([area])[0].phone_density, expected_phones_per_square_mile, ) @@ -351,11 +336,11 @@ def test_estimated_bleed( area, expected_bleed_in_m, expected_bleed_in_degrees, ): assert close_enough( - broadcast_area_libraries.get_areas(area)[0].estimated_bleed_in_m, + broadcast_area_libraries.get_areas([area])[0].estimated_bleed_in_m, expected_bleed_in_m, ) assert close_enough( - broadcast_area_libraries.get_areas(area)[0].estimated_bleed_in_degrees, + broadcast_area_libraries.get_areas([area])[0].estimated_bleed_in_degrees, expected_bleed_in_degrees, ) @@ -377,7 +362,7 @@ def test_estimated_bleed( 'Stoke Bishop', 'Windmill Hill', ], - 73_496, + 73_119, ), ( SKYE, @@ -387,7 +372,7 @@ def test_estimated_bleed( 'Na Hearadh agus Ceann a Deas nan Loch', 'Wester Ross, Strathpeffer and Lochalsh', ], - 3_517, + 3_534, ), )) def test_count_of_phones_for_custom_area(