Suggest previously-used areas when adding new area

If you’re adding another area to your broadcast it’s likely to be close
to one of the areas you’ve already added.

But we make you start by choosing a library, then you have to find the
local authority again from the long list. This is clunky, and it
interrupts the task the user is trying to complete.

We thought about redirecting you somewhere deep into the hierarchy,
perhaps by sending you to either:
- the parent of the last area you’d chosen
- the common ancestor of all the areas you’d chosen

This approach would however mean you’d need a way to navigate back up
the hierarchy if we’d dropped you in the wrong place. And we don’t have
a pattern for that at the moment.

So instead this commit adds some ‘shortcuts’ to the chose library page,
giving you a choice of all the parents of the areas you’ve currently
selected. In most cases this will be one (unitary authority) or two
(county and district) choices, but it will scale to adding areas from
multiple different authorities.

It does mean an extra click compared to the redirect approach, but this
is still fewer, easier clicks compared to now.

This meant a couple of under-the-hood changes:
- making `BroadcastArea`s hashable so it’s possible to do
  `set([BroadcastArea(…), BroadcastArea(…), BroadcastArea(…)])`
- making `BroadcastArea`s aware of which library they live in, so we can
  link to the correct _Choose area_ page
This commit is contained in:
Chris Hill-Scott
2020-09-21 18:55:46 +01:00
parent 7040c7bdd0
commit f50ef84c0d
6 changed files with 167 additions and 20 deletions

View File

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