Merge pull request #3641 from alphagov/area-suggestions

Suggest previously-used areas when adding new area
This commit is contained in:
Chris Hill-Scott
2020-09-23 15:00:21 +01:00
committed by GitHub
6 changed files with 167 additions and 20 deletions

View File

@@ -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):

View File

@@ -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

View File

@@ -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,
),
)

View File

@@ -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 [

View File

@@ -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 %}
<a class="govuk-heading-m govuk-link govuk-link--no-visited-state" href="{{ url_for('.choose_broadcast_sub_area', service_id=current_service.id, broadcast_message_id=broadcast_message.id, library_slug=area.library_id, area_slug=area.id) }}">{{ area.name }}</a>
{% if loop.last %}
<div class="keyline-block"></div>
{% endif %}
{% endfor %}
{% for library in libraries|sort %}
<a class="file-list-filename-large govuk-link govuk-link--no-visited-state" href="{{ url_for('.choose_broadcast_area', service_id=current_service.id, broadcast_message_id=broadcast_message_id, library_slug=library.id) }}">{{ library.name }}</a>
<a class="file-list-filename-large govuk-link govuk-link--no-visited-state" href="{{ url_for('.choose_broadcast_area', service_id=current_service.id, broadcast_message_id=broadcast_message.id, library_slug=library.id) }}">{{ library.name }}</a>
<p class="file-list-hint-large">{{ library.get_examples() }}</p>
{% endfor %}

View File

@@ -358,13 +358,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 youve chosen the whole of a county or unitary authority
# theres 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,
@@ -373,11 +425,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'
@@ -391,6 +440,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,