diff --git a/app/broadcast_areas/models.py b/app/broadcast_areas/models.py index 6084cd7c9..8d364e3c5 100644 --- a/app/broadcast_areas/models.py +++ b/app/broadcast_areas/models.py @@ -86,6 +86,14 @@ class BroadcastArea(BaseBroadcastArea, SortableMixin): def __init__(self, row): self.id, self.name, self._count_of_phones, self.library_id = row + @cached_property + def is_lower_tier_local_authority(self): + return self.id.startswith('lad20-') and self.parent + + @cached_property + def is_electoral_ward(self): + return self.id.startswith('wd20-') + @classmethod def from_row_with_simple_polygons(cls, row): instance = cls(row[:4]) @@ -127,6 +135,10 @@ class BroadcastArea(BaseBroadcastArea, SortableMixin): def ancestors(self): return list(self._ancestors_iterator) + @cached_property + def parent(self): + return next(iter(self.ancestors), None) + @property def _ancestors_iterator(self): id = self.id @@ -162,6 +174,13 @@ class CustomBroadcastArea(BaseBroadcastArea): simple_polygons = polygons + @cached_property + def overlapping_electoral_wards(self): + return [ + area for area in self.nearby_electoral_wards + if area.simple_polygons.intersects(self.polygons) + ] + @cached_property def nearby_electoral_wards(self): if not self.polygons: diff --git a/app/broadcast_areas/utils.py b/app/broadcast_areas/utils.py new file mode 100644 index 000000000..48766073d --- /dev/null +++ b/app/broadcast_areas/utils.py @@ -0,0 +1,65 @@ +from collections import defaultdict + +from app.broadcast_areas.models import CustomBroadcastArea + + +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 sorted(areas) + + +def _convert_custom_areas_to_wards(areas): + results = set() + + for area in areas: + if type(area) == CustomBroadcastArea: + results |= set(area.overlapping_electoral_wards) + else: + results |= {area} + + return results + + +def _aggregate_wards_by_local_authority(areas): + return { + area.parent if area.is_electoral_ward + else area for area in areas + } + + +def _aggregate_lower_tier_authorities(areas): + results = set() + clusters = _cluster_lower_tier_authorities(areas) + + for cluster in clusters: + # always show a single area cluster as itself (aggregation isn't helpful) + if len(cluster) == 1: + results |= set(cluster) + # aggregate a single cluster with lots of areas (too complex to show in full) + elif len(cluster) > 3: + results |= {cluster[0].parent} + # if cluster is 2 or 3 areas, and there are more than 1 cluster, aggregate the cluster + elif len(clusters) > 1: + area = cluster[0] + results |= {area.parent or area} + # else keep single 2-3 areas cluster in full (easy enough to understand) + else: + results |= set(cluster) + + return results + + +def _cluster_lower_tier_authorities(areas): + result = defaultdict(lambda: []) + + for area in areas: + # group lower tier authorities by "county" + if area.is_lower_tier_local_authority: + result[area.parent] += [area] + # leave countries, unitary authorities as-is + else: + result[area] = [area] + + return result.values() 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/__init__.py b/tests/__init__.py index 27785c65c..95bd17233 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -667,11 +667,11 @@ def find_element_by_tag_and_partial_text(page, tag, string): def broadcast_message_json( *, - id_, - service_id, - template_id, - status, - created_by_id, + id_=None, + service_id=None, + template_id=None, + status='draft', + created_by_id=None, starts_at=None, finishes_at=None, cancelled_at=None, diff --git a/tests/app/broadcast_areas/custom_polygons.py b/tests/app/broadcast_areas/custom_polygons.py index 6faea8675..e79360649 100644 --- a/tests/app/broadcast_areas/custom_polygons.py +++ b/tests/app/broadcast_areas/custom_polygons.py @@ -13,3 +13,43 @@ SKYE = [ [57.7334, -6.8280], [57.1004, -6.8280], ] + +SANTA_A = [ + [25.8890, 66.5500], + [25.8890, 66.551], + [25.8910, 66.551], + [25.8910, 66.5500], + [25.889, 66.55000], +] + +BURFORD = [ + [51.8925, -1.7495], + [51.7372, -1.7825], + [51.7159, -1.5257], + [51.8866, -1.5037], + [51.8925, -1.7495], +] + +CHELTENHAM = [ + [51.9324, -2.1265], + [51.8633, -2.1306], + [51.8637, -2.0242], + [51.9328, -2.0221], + [51.9324, -2.1265], +] + +CHELTENHAM_AND_GLOUCESTER = [ + [51.8820, -2.2920], + [51.8234, -2.2570], + [51.8883, -2.0262], + [51.9425, -2.0771], + [51.8820, -2.2920], +] + +SEVERN_ESTUARY = [ + [51.5719, -2.9388], + [51.4180, -2.7259], + [51.8493, -2.1958], + [51.8883, -2.3016], + [51.5719, -2.9388], +] diff --git a/tests/app/broadcast_areas/test_models.py b/tests/app/broadcast_areas/test_models.py new file mode 100644 index 000000000..23b9933d2 --- /dev/null +++ b/tests/app/broadcast_areas/test_models.py @@ -0,0 +1,20 @@ +import pytest + +from app.broadcast_areas.models import CustomBroadcastArea +from tests.app.broadcast_areas.custom_polygons import BRISTOL, SANTA_A, SKYE + + +@pytest.mark.parametrize(('simple_polygon', 'expected_wards_length'), [ + (SKYE, 2), + (BRISTOL, 12), + (SANTA_A, 0) # does not overlap with UK +]) +def test_custom_broadcast_area_overlapping_electoral_wards( + simple_polygon, + expected_wards_length, +): + custom_area = CustomBroadcastArea( + name='foo', polygons=[simple_polygon] + ) + + assert len(custom_area.overlapping_electoral_wards) == expected_wards_length diff --git a/tests/app/broadcast_areas/test_utils.py b/tests/app/broadcast_areas/test_utils.py new file mode 100644 index 000000000..8ed121995 --- /dev/null +++ b/tests/app/broadcast_areas/test_utils.py @@ -0,0 +1,185 @@ +import pytest + +from app.broadcast_areas.utils import aggregate_areas +from app.models.broadcast_message import BroadcastMessage +from tests import broadcast_message_json +from tests.app.broadcast_areas.custom_polygons import ( + BRISTOL, + BURFORD, + CHELTENHAM, + CHELTENHAM_AND_GLOUCESTER, + SANTA_A, + SEVERN_ESTUARY, + SKYE, +) + + +@pytest.mark.parametrize(('area_ids', 'expected_area_names'), [ + ( + [ + 'wd20-E05009336', # Whitechapel, Tower Hamlets (electoral ward) + 'wd20-E05009372', # Hackney Central, Hackney (electoral ward) + 'wd20-E05009374', # Hackney Wick, Hackney (electoral ward) + ], [ + 'Hackney', # in Greater London* (DB doesn't know this) + 'Tower Hamlets', # in Greater London* + ], + ), + ( + [ + 'wd20-E05004294', # Hester’s Way, Cheltenham (electoral ward) + 'wd20-E05010981', # Painswick & Upton, Stroud (electoral ward) + ], [ + 'Cheltenham', # in Gloucestershire (upper tier authority) + 'Stroud', # in Gloucestershire + ], + ), + ( + [ + 'wd20-E05004294', # Hester’s Way, Cheltenham (electoral ward) + 'wd20-E05009372', # Hackney Central (electoral ward) + ], [ + 'Cheltenham', # in Gloucestershire (upper tier authority) + 'Hackney', # in Greater London* (DB doesn't know this) + ], + ), + ( + [ + 'wd20-E05004294', # Hester’s Way, Cheltenham (electoral ward) + 'wd20-E05010981', # Painswick & Upton, Stroud (electoral ward) + 'wd20-E05009372', # Hackney Central, Hackney (electoral ward) + ], [ + 'Gloucestershire', # upper tier authority + 'Hackney', # in Greater London* (DB doesn't know this) + ], + ), + ( + [ + 'lad20-E07000037', # High Peak (lower tier authority) + ], + [ + 'High Peak', # in Derbyshire (upper tier authority) + ] + ), + ( + [ + 'lad20-E07000037', # High Peak (lower tier authority) + 'lad20-E07000035', # Derbyshire Dales (lower tier authority) + ], + [ + 'Derbyshire Dales', # in Derbyshire (upper tier authority) + 'High Peak', # in Derbyshire + ] + ), + ( + [ + 'lad20-E07000037', # High Peak (lower tier authority) + 'lad20-E07000035', # Derbyshire Dales (lower tier authority) + 'ctyua19-E10000028', # Staffordshire (upper tier authority) + ], + [ + 'Derbyshire', # upper tier authority + 'Staffordshire', + ] + ), + ( + [ + 'ctry19-E92000001', # England + ], + [ + 'England', + ] + ) +]) +def test_aggregate_areas( + area_ids, + expected_area_names, +): + broadcast_message = BroadcastMessage( + broadcast_message_json(area_ids=area_ids) + ) + + assert [ + area.name for area in aggregate_areas(broadcast_message.areas) + ] == expected_area_names + + +@pytest.mark.parametrize(('simple_polygons', 'expected_area_names'), [ + ( + [SKYE], [ + # Area covers a single unitary authority + 'Highland', + ] + ), + ( + [BRISTOL], [ + # Area covers a single unitary authority + 'Bristol, City of', + ] + ), + ( + [SEVERN_ESTUARY], [ + # Area covers various lower-tier authorities + # with more than three in Gloucestershire so + # we aggregate them into that + 'Bristol, City of', + 'Gloucestershire', + 'Monmouthshire', + 'Newport', + 'North Somerset', + 'South Gloucestershire', + ] + ), + ( + [CHELTENHAM], [ + # Area covers three lower-tier authorities + # in Gloucestershire (upper tier authority) + 'Cheltenham', + 'Cotswold', + 'Tewkesbury', + ] + ), + ( + [CHELTENHAM_AND_GLOUCESTER], [ + # Area covers more than 3 lower-tier authorities + # in Gloucestershire so we aggregate them + 'Gloucestershire', + ] + ), + ( + [BURFORD], [ + # Area covers one lower-tier authority in + # Gloucestershire and one in Oxfordshire (both + # upper tier authorities) + 'Cotswold', + 'West Oxfordshire', + ], + ), + ( + [CHELTENHAM_AND_GLOUCESTER, BURFORD], [ + # Area covers many lower-tier authorities + # in Gloucestershire but only one in Oxfordshire + 'Gloucestershire', + 'West Oxfordshire', + ], + ), + ( + [SANTA_A], [ + # Does not overlap with the UK + ], + ), +]) +def test_aggregate_areas_for_custom_polygons( + simple_polygons, + expected_area_names, +): + broadcast_message = BroadcastMessage( + broadcast_message_json( + area_ids=['derived from polygons'], + simple_polygons=simple_polygons + ) + ) + + assert [ + area.name for area in aggregate_areas(broadcast_message.areas) + ] == 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'] }, },