diff --git a/app/broadcast_areas/__init__.py b/app/broadcast_areas/__init__.py index b24e7ba66..5c10f6228 100644 --- a/app/broadcast_areas/__init__.py +++ b/app/broadcast_areas/__init__.py @@ -17,6 +17,12 @@ class SortableMixin: # method are sortable return self.name < other.name + def __eq__(self, other): + return self.id == other.id + + def __hash__(self): + return hash(self.id) + class GetItemByIdMixin: def get(self, id): @@ -29,10 +35,7 @@ class GetItemByIdMixin: class BroadcastArea(SortableMixin): def __init__(self, row): - self.id, self.name, self._count_of_phones = row - - def __eq__(self, other): - return self.id == other.id + self.id, self.name, self._count_of_phones, self.library_id = row @cached_property def polygons(self): @@ -65,6 +68,26 @@ class BroadcastArea(SortableMixin): # https://www.pivotaltracker.com/story/show/174837293 return self._count_of_phones or 0 + @cached_property + def parents(self): + return list(filter(None, self._parents_iterator)) + + @property + def _parents_iterator(self): + id = self.id + + while True: + parent = BroadcastAreasRepository().get_parent_for_area(id) + + if not parent: + return None + + parent_broadcast_area = BroadcastArea(parent) + + yield parent_broadcast_area + + id = parent_broadcast_area.id + class BroadcastAreaLibrary(SerialisedModelCollection, SortableMixin, GetItemByIdMixin): diff --git a/app/broadcast_areas/repo.py b/app/broadcast_areas/repo.py index fa8491097..d6f4ebcf7 100644 --- a/app/broadcast_areas/repo.py +++ b/app/broadcast_areas/repo.py @@ -123,7 +123,7 @@ class BroadcastAreasRepository(object): def get_areas(self, area_ids): q = """ - SELECT id, name, count_of_phones + SELECT id, name, count_of_phones, broadcast_area_library_id FROM broadcast_areas WHERE id IN ({}) """.format(("?," * len(area_ids))[:-1]) @@ -131,7 +131,7 @@ class BroadcastAreasRepository(object): results = self.query(q, *area_ids) areas = [ - (row[0], row[1], row[2]) + (row[0], row[1], row[2], row[3]) for row in results ] @@ -150,7 +150,7 @@ class BroadcastAreasRepository(object): if is_multi_tier_library: # only interested in areas with children - eg local authorities, counties, unitary authorities. not wards. q = """ - SELECT id, name, count_of_phones + SELECT id, name, count_of_phones, broadcast_area_library_id FROM broadcast_areas JOIN ( SELECT DISTINCT broadcast_area_library_group_id @@ -162,7 +162,7 @@ class BroadcastAreasRepository(object): else: # Countries don't have any children, so the above query wouldn't return anything. q = """ - SELECT id, name, count_of_phones + SELECT id, name, count_of_phones, broadcast_area_library_id FROM broadcast_areas WHERE broadcast_area_library_id = ? """ @@ -170,13 +170,13 @@ class BroadcastAreasRepository(object): results = self.query(q, library_id) return [ - (row[0], row[1], row[2]) + (row[0], row[1], row[2], row[3]) for row in results ] def get_all_areas_for_group(self, group_id): q = """ - SELECT id, name, count_of_phones + SELECT id, name, count_of_phones, broadcast_area_library_id FROM broadcast_areas WHERE broadcast_area_library_group_id = ? """ @@ -184,12 +184,30 @@ class BroadcastAreasRepository(object): results = self.query(q, group_id) areas = [ - (row[0], row[1], row[2]) + (row[0], row[1], row[2], row[3]) for row in results ] return areas + def get_parent_for_area(self, area_id): + q = """ + SELECT id, name, count_of_phones, broadcast_area_library_id + FROM broadcast_areas + WHERE id IN ( + SELECT broadcast_area_library_group_id + FROM broadcast_areas + WHERE id = ? + ) + """ + + results = self.query(q, area_id) + + if not results: + return None + + return (results[0][0], results[0][1], results[0][2], results[0][3]) + def get_polygons_for_area(self, area_id): q = """ SELECT polygons diff --git a/app/main/views/broadcast.py b/app/main/views/broadcast.py index 16b820f65..ffcebde2b 100644 --- a/app/main/views/broadcast.py +++ b/app/main/views/broadcast.py @@ -102,7 +102,10 @@ def choose_broadcast_library(service_id, broadcast_message_id): return render_template( 'views/broadcast/libraries.html', libraries=BroadcastMessage.libraries, - broadcast_message_id=broadcast_message_id, + broadcast_message=BroadcastMessage.from_id( + broadcast_message_id, + service_id=current_service.id, + ), ) diff --git a/app/models/broadcast_message.py b/app/models/broadcast_message.py index f5f88ec66..87b431b37 100644 --- a/app/models/broadcast_message.py +++ b/app/models/broadcast_message.py @@ -65,6 +65,16 @@ class BroadcastMessage(JSONModel): def areas(self): return self.get_areas(areas=self._dict['areas']) + @property + def parent_areas(self): + return sorted(set(self._parent_areas_iterator)) + + @property + def _parent_areas_iterator(self): + for area in self.areas: + for parent in area.parents: + yield parent + @property def initial_area_names(self): return [ diff --git a/app/templates/views/broadcast/libraries.html b/app/templates/views/broadcast/libraries.html index e5597d511..5b5981531 100644 --- a/app/templates/views/broadcast/libraries.html +++ b/app/templates/views/broadcast/libraries.html @@ -10,11 +10,18 @@ {{ page_header( "Choose where to broadcast to", - back_link=url_for(".preview_broadcast_areas", service_id=current_service.id, broadcast_message_id=broadcast_message_id) + back_link=url_for(".preview_broadcast_areas", service_id=current_service.id, broadcast_message_id=broadcast_message.id) ) }} + {% for area in broadcast_message.parent_areas %} + {{ area.name }} + {% if loop.last %} +
+ {% endif %} + {% endfor %} + {% for library in libraries|sort %} - {{ library.name }} + {{ library.name }}

{{ library.get_examples() }}

{% endfor %} diff --git a/tests/app/main/views/test_broadcast.py b/tests/app/main/views/test_broadcast.py index 0ee2a41ed..9ba3a1707 100644 --- a/tests/app/main/views/test_broadcast.py +++ b/tests/app/main/views/test_broadcast.py @@ -356,13 +356,65 @@ def test_preview_broadcast_areas_page( ] +@pytest.mark.parametrize('areas, expected_list', ( + ([], [ + 'Countries', + 'Local authorities', + ]), + ([ + # Countries have no parent areas + 'ctry19-E92000001', + 'ctry19-S92000003', + ], [ + 'Countries', + 'Local authorities', + ]), + ([ + # If you’ve chosen the whole of a county or unitary authority + # there’s no reason to also pick districts of it + 'ctyua19-E10000013', # Gloucestershire, a county + 'lad20-E06000052', # Cornwall, a unitary authority + ], [ + 'Countries', + 'Local authorities', + ]), + ([ + 'wd20-E05004299', # Pitville, in Cheltenham, in Gloucestershire + 'wd20-E05004290', # Benhall and the Reddings, in Cheltenham, in Gloucestershire + 'wd20-E05010951', # Abbeymead, in Gloucester, in Gloucestershire + 'wd20-S13002775', # Shetland Central, in Shetland Isles + 'lad20-E07000037', # High Peak, a district in Derbyshire + ], [ + 'Cheltenham', + 'Derbyshire', + 'Gloucester', + 'Gloucestershire', + 'Shetland Islands', + # --- + 'Countries', + 'Local authorities', + ]), +)) def test_choose_broadcast_library_page( + mocker, client_request, service_one, - mock_get_draft_broadcast_message, fake_uuid, + areas, + expected_list, ): service_one['permissions'] += ['broadcast'] + mocker.patch( + 'app.broadcast_message_api_client.get_broadcast_message', + return_value=broadcast_message_json( + id_=fake_uuid, + template_id=fake_uuid, + created_by_id=fake_uuid, + service_id=SERVICE_ONE_ID, + status='draft', + areas=areas, + ), + ) page = client_request.get( '.choose_broadcast_library', service_id=SERVICE_ONE_ID, @@ -371,11 +423,8 @@ def test_choose_broadcast_library_page( assert [ normalize_spaces(title.text) - for title in page.select('.file-list-filename-large') - ] == [ - 'Countries', - 'Local authorities', - ] + for title in page.select('main a.govuk-link') + ] == expected_list assert normalize_spaces(page.select('.file-list-hint-large')[0].text) == ( 'England, Northern Ireland, Scotland and Wales' @@ -389,6 +438,43 @@ def test_choose_broadcast_library_page( ) +def test_suggested_area_has_correct_link( + mocker, + client_request, + service_one, + fake_uuid, +): + service_one['permissions'] += ['broadcast'] + mocker.patch( + 'app.broadcast_message_api_client.get_broadcast_message', + return_value=broadcast_message_json( + id_=fake_uuid, + template_id=fake_uuid, + created_by_id=fake_uuid, + service_id=SERVICE_ONE_ID, + status='draft', + areas=[ + 'wd20-E05004299', # Pitville, a ward of Cheltenham + ], + ), + ) + page = client_request.get( + '.choose_broadcast_library', + service_id=SERVICE_ONE_ID, + broadcast_message_id=fake_uuid, + ) + link = page.select_one('main a.govuk-link') + + assert link.text == 'Cheltenham' + assert link['href'] == url_for( + 'main.choose_broadcast_sub_area', + service_id=SERVICE_ONE_ID, + broadcast_message_id=fake_uuid, + library_slug='wd20-lad20-ctyua19', + area_slug='lad20-E07000078', + ) + + def test_choose_broadcast_area_page( client_request, service_one,