diff --git a/notifications_utils/broadcast_areas/__init__.py b/notifications_utils/broadcast_areas/__init__.py index 5ec0a1e2a..c42046cb5 100644 --- a/notifications_utils/broadcast_areas/__init__.py +++ b/notifications_utils/broadcast_areas/__init__.py @@ -7,7 +7,7 @@ from notifications_utils.safe_string import make_string_safe_for_id from .repo import BroadcastAreasRepository -class IdentifiableMixin: +class SortableMixin: def __repr__(self): return f'{self.__class__.__name__}(<{self.id}>)' @@ -15,14 +15,7 @@ class IdentifiableMixin: def __lt__(self, other): # Implementing __lt__ means any classes inheriting from this # method are sortable - return self.id < other.id - - -class IdFromNameMixin: - - @property - def id(self): - return make_string_safe_for_id(self.name) + return self.name < other.name class GetItemByIdMixin: @@ -33,7 +26,7 @@ class GetItemByIdMixin: raise KeyError(id) -class BroadcastArea(IdentifiableMixin): +class BroadcastArea(SortableMixin): def __init__(self, row): id, name, feature, simple_feature = row @@ -110,13 +103,23 @@ class BroadcastArea(IdentifiableMixin): return self._simple_geofeature + @property + def sub_areas(self): + return [ + BroadcastArea(row) + for row in BroadcastAreasRepository().get_all_areas_for_group(self.id) + ] -class BroadcastAreaLibrary(SerialisedModelCollection, IdentifiableMixin, IdFromNameMixin, GetItemByIdMixin): + +class BroadcastAreaLibrary(SerialisedModelCollection, SortableMixin, GetItemByIdMixin): model = BroadcastArea - def __init__(self, library_name): - self.name = library_name + def __init__(self, row): + id, name, is_group = row + self.id = id + self.name = name + self.is_group = bool(is_group) def get_examples(self): return BroadcastAreasRepository().get_library_description(self.id) diff --git a/notifications_utils/broadcast_areas/repo.py b/notifications_utils/broadcast_areas/repo.py index 9ac935ca0..4363ee024 100644 --- a/notifications_utils/broadcast_areas/repo.py +++ b/notifications_utils/broadcast_areas/repo.py @@ -1,3 +1,4 @@ +import geojson import os from pathlib import Path import sqlite3 @@ -20,7 +21,15 @@ class BroadcastAreasRepository(object): conn.execute(""" CREATE TABLE broadcast_area_libraries ( id TEXT PRIMARY KEY, - name TEXT NOT NULL + name TEXT NOT NULL, + is_group BOOLEAN NOT NULL + )""") + + conn.execute(""" + CREATE TABLE broadcast_area_library_groups ( + id TEXT PRIMARY KEY, + name TEXT NOT NULL, + broadcast_area_library_id TEXT NOT NULL )""") conn.execute(""" @@ -28,11 +37,15 @@ class BroadcastAreasRepository(object): id TEXT PRIMARY KEY, name TEXT NOT NULL, broadcast_area_library_id TEXT NOT NULL, + broadcast_area_library_group_id TEXT, feature_geojson TEXT NOT NULL, simple_feature_geojson TEXT NOT NULL, FOREIGN KEY (broadcast_area_library_id) - REFERENCES broadcast_area_libraries(id) + REFERENCES broadcast_area_libraries(id), + + FOREIGN KEY (broadcast_area_library_group_id) + REFERENCES broadcast_area_library_groups(id) )""") conn.execute(""" @@ -40,36 +53,49 @@ class BroadcastAreasRepository(object): ON broadcast_areas (broadcast_area_library_id); """) - def insert_broadcast_area_library(self, broadcast_area_name): - broadcast_area_id = make_string_safe_for_id(broadcast_area_name) + conn.execute(""" + CREATE INDEX broadcast_areas_broadcast_area_library_group_id + ON broadcast_areas (broadcast_area_library_group_id); + """) + + def insert_broadcast_area_library(self, id, name, is_group): q = """ - INSERT INTO broadcast_area_libraries (id, name) - VALUES (?, ?) + INSERT INTO broadcast_area_libraries (id, name, is_group) + VALUES (?, ?, ?) """ with self.conn() as conn: - conn.execute(q, (broadcast_area_id, broadcast_area_name)) + conn.execute(q, (id, name, is_group)) + + def insert_broadcast_area_library_group(self, id, name, library_id): + + q = """ + INSERT INTO broadcast_area_library_groups + (id, name, broadcast_area_library_id) + VALUES (?, ?, ?) + """ + + with self.conn() as conn: + conn.execute(q, (id, name, library_id)) def insert_broadcast_areas(self, areas): q = """ INSERT INTO broadcast_areas ( id, name, - broadcast_area_library_id, feature_geojson, simple_feature_geojson + broadcast_area_library_id, broadcast_area_library_group_id, + feature_geojson, simple_feature_geojson ) - VALUES (?, ?, ?, ?, ?) + VALUES (?, ?, ?, ?, ?, ?) """ with self.conn() as conn: - for name, area_name, feature, simple_feature in areas: - id = make_string_safe_for_id(name) - area_id = make_string_safe_for_id(area_name) - + for id, name, area_id, group, feature, simple_feature in areas: conn.execute(q, ( - id, name, area_id, - geojson.dumps(feature), - geojson.dumps(simple_feature), + id, name, + area_id, group, + geojson.dumps(feature), geojson.dumps(simple_feature), )) def query(self, sql, *args): @@ -79,9 +105,9 @@ class BroadcastAreasRepository(object): return cursor.fetchall() def get_libraries(self): - q = "SELECT id, name FROM broadcast_area_libraries" + q = "SELECT id, name, is_group FROM broadcast_area_libraries" results = self.query(q) - libraries = [row[1] for row in results] + libraries = [(row[0], row[1], row[2]) for row in results] return sorted(libraries) def get_library_description(self, library_id): @@ -139,6 +165,7 @@ class BroadcastAreasRepository(object): SELECT id, name, feature_geojson, simple_feature_geojson FROM broadcast_areas WHERE broadcast_area_library_id = ? + AND broadcast_area_library_group_id IS NULL """ results = self.query(q, library_id) @@ -149,3 +176,35 @@ class BroadcastAreasRepository(object): ] return areas + + def get_all_areas_for_group(self, group_id): + q = """ + SELECT id, name, feature_geojson, simple_feature_geojson + FROM broadcast_areas + WHERE broadcast_area_library_group_id = ? + """ + + results = self.query(q, group_id) + + areas = [ + (row[0], row[1], row[2], row[3]) + for row in results + ] + + return areas + + def get_all_groups_for_library(self, library_id): + q = """ + SELECT id, name + FROM broadcast_area_library_groups + WHERE broadcast_area_library_id = ? + """ + + results = self.query(q, library_id) + + areas = [ + (row[0], row[1]) + for row in results + ] + + return areas diff --git a/tests/test_broadcast_area.py b/tests/test_broadcast_area.py index 3a6787fbd..218b5312f 100644 --- a/tests/test_broadcast_area.py +++ b/tests/test_broadcast_area.py @@ -9,22 +9,27 @@ from notifications_utils.broadcast_areas import ( def test_loads_libraries(): assert [ - (library.id, library.name) for library in sorted(broadcast_area_libraries) + (library.id, library.name, library.is_group) for library in sorted(broadcast_area_libraries) ] == [ ( 'counties-and-unitary-authorities-in-england-and-wales', - 'Counties and Unitary Authorities in England and Wales'), + 'Counties and Unitary Authorities in England and Wales', + False, + ), ( 'countries', 'Countries', + False, ), ( 'electoral-wards-of-the-united-kingdom', 'Electoral Wards of the United Kingdom', + True, ), ( 'regions-of-england', 'Regions of England', + False, ), ] @@ -35,10 +40,10 @@ def test_loads_areas_from_library(): broadcast_area_libraries.get('countries') ) ] == [ - ('england', 'England'), - ('northern-ireland', 'Northern Ireland'), - ('scotland', 'Scotland'), - ('wales', 'Wales'), + ('countries-E92000001', 'England'), + ('countries-N92000002', 'Northern Ireland'), + ('countries-S92000003', 'Scotland'), + ('countries-W92000004', 'Wales'), ] @@ -54,10 +59,10 @@ def test_examples(): @pytest.mark.parametrize('id', ( - 'england', - 'northern-ireland', - 'scotland', - 'wales', + 'countries-E92000001', + 'countries-N92000002', + 'countries-S92000003', + 'countries-W92000004', pytest.param('mercia', marks=pytest.mark.xfail(raises=KeyError)), )) def test_loads_areas_from_libraries(id): @@ -70,7 +75,11 @@ def test_loads_areas_from_libraries(id): def test_get_names_of_areas(): areas = broadcast_area_libraries.get_areas( - 'wales', 'vale-of-glamorgan', 'england', 'essex', + 'countries-W92000004', + 'electoral-wards-of-the-united-kingdom-W06000014', + 'countries-E92000001', + 'counties-and-unitary-authorities-in-england-and-wales-E10000012', + ) assert [area.name for area in sorted(areas)] == [ 'England', 'Essex', 'Vale of Glamorgan', 'Wales', @@ -90,29 +99,34 @@ def test_get_areas_accepts_lists(): def test_has_polygons(): assert len( - broadcast_area_libraries.get_polygons_for_areas_long_lat('england') + broadcast_area_libraries.get_polygons_for_areas_long_lat('countries-E92000001') ) == 35 assert len( - broadcast_area_libraries.get_polygons_for_areas_long_lat('scotland') + broadcast_area_libraries.get_polygons_for_areas_long_lat('countries-S92000003') ) == 195 assert len( - broadcast_area_libraries.get_polygons_for_areas_long_lat('england', 'scotland') + broadcast_area_libraries.get_polygons_for_areas_long_lat( + 'countries-E92000001', + 'countries-S92000003', + ) ) == 35 + 195 == 230 assert len( - broadcast_area_libraries.get_polygons_for_areas_long_lat(['england', 'scotland']) + broadcast_area_libraries.get_polygons_for_areas_lat_long( + 'countries-E92000001', + 'countries-S92000003', + ) ) == 35 + 195 == 230 - assert broadcast_area_libraries.get_polygons_for_areas_lat_long('england')[0][0] == [ + assert broadcast_area_libraries.get_polygons_for_areas_lat_long('countries-E92000001')[0][0] == [ 55.811085, -2.034358 # https://goo.gl/maps/wsf2LUWzYinwydMk8 ] def test_polygons_are_enclosed_unless_asked_not_to_be(): - - england = broadcast_area_libraries.get('countries').get('england') + england = broadcast_area_libraries.get('countries').get('countries-E92000001') assert len(england.polygons) == len(england.unenclosed_polygons) @@ -128,8 +142,8 @@ def test_polygons_are_enclosed_unless_asked_not_to_be(): def test_lat_long_order(): - lat_long = broadcast_area_libraries.get_polygons_for_areas_lat_long('england') - long_lat = broadcast_area_libraries.get_polygons_for_areas_long_lat('england') + lat_long = broadcast_area_libraries.get_polygons_for_areas_lat_long('countries-E92000001') + long_lat = broadcast_area_libraries.get_polygons_for_areas_long_lat('countries-E92000001') assert len(lat_long[0]) == len(long_lat[0]) == 2082 # Coordinates in polygon assert len(lat_long[0][0]) == len(long_lat[0][0]) == 2 # Axes in coordinates assert lat_long[0][0] == list(reversed(long_lat[0][0])) @@ -137,7 +151,7 @@ def test_lat_long_order(): def test_includes_electoral_wards(): - areas = broadcast_area_libraries.get_areas(['city-of-london---aldgate']) + areas = broadcast_area_libraries.get_areas(['electoral-wards-of-the-united-kingdom-E05009289']) assert len(areas) == 1 @@ -151,4 +165,4 @@ def test_repository_has_all_libraries(): 'Countries', 'Electoral Wards of the United Kingdom', 'Regions of England', - ] == sorted(libraries) + ] == sorted([name for _, name, _is_group in libraries])