Merge pull request #3629 from alphagov/fix-broadcast-back-links

fix back links for broadcast libraries
This commit is contained in:
Leo Hemsted
2020-09-21 11:37:21 +01:00
committed by GitHub
4 changed files with 81 additions and 21 deletions

View File

@@ -147,6 +147,25 @@ def choose_broadcast_area(service_id, broadcast_message_id, library_slug):
) )
def _get_broadcast_sub_area_back_link(service_id, broadcast_message_id, library_slug):
prev_area_slug = request.args.get('prev_area_slug')
if prev_area_slug:
return url_for(
'.choose_broadcast_sub_area',
service_id=service_id,
broadcast_message_id=broadcast_message_id,
library_slug=library_slug,
area_slug=prev_area_slug,
)
else:
return url_for(
'.choose_broadcast_area',
service_id=service_id,
broadcast_message_id=broadcast_message_id,
library_slug=library_slug,
)
@main.route( @main.route(
'/services/<uuid:service_id>/broadcast/<uuid:broadcast_message_id>/libraries/<library_slug>/<area_slug>', '/services/<uuid:service_id>/broadcast/<uuid:broadcast_message_id>/libraries/<library_slug>/<area_slug>',
methods=['GET', 'POST'], methods=['GET', 'POST'],
@@ -160,6 +179,8 @@ def choose_broadcast_sub_area(service_id, broadcast_message_id, library_slug, ar
) )
area = BroadcastMessage.libraries.get_areas(area_slug)[0] area = BroadcastMessage.libraries.get_areas(area_slug)[0]
back_link = _get_broadcast_sub_area_back_link(service_id, broadcast_message_id, library_slug)
is_county = any(sub_area.sub_areas for sub_area in area.sub_areas) is_county = any(sub_area.sub_areas for sub_area in area.sub_areas)
form = BroadcastAreaFormWithSelectAll.from_library( form = BroadcastAreaFormWithSelectAll.from_library(
@@ -185,6 +206,7 @@ def choose_broadcast_sub_area(service_id, broadcast_message_id, library_slug, ar
page_title=f'Choose an area of {area.name}', page_title=f'Choose an area of {area.name}',
broadcast_message=broadcast_message, broadcast_message=broadcast_message,
county=area, county=area,
back_link=back_link,
) )
return render_template( return render_template(
@@ -195,6 +217,7 @@ def choose_broadcast_sub_area(service_id, broadcast_message_id, library_slug, ar
library_slug=library_slug, library_slug=library_slug,
page_title=f'Choose an area of {area.name}', page_title=f'Choose an area of {area.name}',
broadcast_message=broadcast_message, broadcast_message=broadcast_message,
back_link=back_link,
) )

View File

@@ -15,7 +15,7 @@
{{ page_header( {{ page_header(
page_title, page_title,
back_link=url_for('.choose_broadcast_library', service_id=current_service.id, broadcast_message_id=broadcast_message.id), back_link=back_link,
)}} )}}
{% call form_wrapper() %} {% call form_wrapper() %}
@@ -30,7 +30,7 @@
{# these are districts within a county#} {# these are districts within a county#}
<a <a
class="file-list-filename-large govuk-link govuk-link--no-visited-state" class="file-list-filename-large 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=library_slug, area_slug=area.id) }}" href="{{ url_for('.choose_broadcast_sub_area', service_id=current_service.id, broadcast_message_id=broadcast_message.id, library_slug=library_slug, area_slug=area.id, prev_area_slug=county.id) }}"
>{{ area.name }}</a> >{{ area.name }}</a>
</div> </div>
{% endfor %} {% endfor %}

View File

@@ -13,7 +13,7 @@
{{ page_header( {{ page_header(
page_title, page_title,
back_link=url_for('.choose_broadcast_library', service_id=current_service.id, broadcast_message_id=broadcast_message.id), back_link=back_link,
)}} )}}
{% call form_wrapper() %} {% call form_wrapper() %}

View File

@@ -453,22 +453,12 @@ def test_choose_broadcast_area_page_for_area_with_sub_areas(
for choice in page.select('.file-list-item') for choice in page.select('.file-list-item')
] ]
assert len(choices) == 394 assert len(choices) == 394
assert choices[:2] == [
( assert choices[0] == (partial_url_for(area_slug='lad20-S12000033'), 'Aberdeen City',)
partial_url_for(area_slug='lad20-S12000033'), # note: we don't populate prev_area_slug query param, so the back link will come here rather than to a county page,
'Aberdeen City', # even though ashford belongs to kent
), assert choices[12] == (partial_url_for(area_slug='lad20-E07000105'), 'Ashford',)
( assert choices[-1] == (partial_url_for(area_slug='lad20-E06000014'), 'York',)
partial_url_for(area_slug='lad20-S12000034'),
'Aberdeenshire',
),
]
assert choices[-1:] == [
(
partial_url_for(area_slug='lad20-E06000014'),
'York',
),
]
def test_choose_broadcast_sub_area_page_for_district_shows_checkboxes_for_wards( def test_choose_broadcast_sub_area_page_for_district_shows_checkboxes_for_wards(
@@ -520,6 +510,51 @@ def test_choose_broadcast_sub_area_page_for_district_shows_checkboxes_for_wards(
] ]
@pytest.mark.parametrize('prev_area_slug, expected_back_link_url, expected_back_link_extra_kwargs', [
(
'ctyua19-E10000016',
'main.choose_broadcast_sub_area',
{
'area_slug': 'ctyua19-E10000016' # Kent
}
),
(
None,
'.choose_broadcast_area',
{}
)
])
def test_choose_broadcast_sub_area_page_for_district_has_back_link(
client_request,
service_one,
mock_get_draft_broadcast_message,
prev_area_slug,
expected_back_link_url,
expected_back_link_extra_kwargs
):
service_one['permissions'] += ['broadcast']
page = client_request.get(
'main.choose_broadcast_sub_area',
service_id=SERVICE_ONE_ID,
broadcast_message_id=str(uuid.UUID(int=0)),
library_slug='wd20-lad20-ctyua19',
area_slug='lad20-E07000105', # Ashford
prev_area_slug=prev_area_slug,
)
assert normalize_spaces(page.select_one('h1').text) == (
'Choose an area of Ashford'
)
back_link = page.select_one('.govuk-back-link')
assert back_link['href'] == url_for(
expected_back_link_url,
service_id=SERVICE_ONE_ID,
broadcast_message_id=str(uuid.UUID(int=0)),
library_slug='wd20-lad20-ctyua19',
**expected_back_link_extra_kwargs
)
def test_choose_broadcast_sub_area_page_for_county_shows_links_for_districts( def test_choose_broadcast_sub_area_page_for_county_shows_links_for_districts(
client_request, client_request,
service_one, service_one,
@@ -563,7 +598,8 @@ def test_choose_broadcast_sub_area_page_for_county_shows_links_for_districts(
service_id=SERVICE_ONE_ID, service_id=SERVICE_ONE_ID,
broadcast_message_id=fake_uuid, broadcast_message_id=fake_uuid,
library_slug='wd20-lad20-ctyua19', library_slug='wd20-lad20-ctyua19',
area_slug='lad20-E07000105' area_slug='lad20-E07000105',
prev_area_slug='ctyua19-E10000016', # Kent
) )
assert districts[0][1] == 'Ashford' assert districts[0][1] == 'Ashford'
assert districts[-1][0] == url_for( assert districts[-1][0] == url_for(
@@ -571,7 +607,8 @@ def test_choose_broadcast_sub_area_page_for_county_shows_links_for_districts(
service_id=SERVICE_ONE_ID, service_id=SERVICE_ONE_ID,
broadcast_message_id=fake_uuid, broadcast_message_id=fake_uuid,
library_slug='wd20-lad20-ctyua19', library_slug='wd20-lad20-ctyua19',
area_slug='lad20-E07000116' area_slug='lad20-E07000116',
prev_area_slug='ctyua19-E10000016', # Kent
) )
assert districts[-1][1] == 'Tunbridge Wells' assert districts[-1][1] == 'Tunbridge Wells'