From a6b25b5991b7517d25cbe9210eca17a379ca2470 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Tue, 15 Sep 2020 13:53:38 +0100 Subject: [PATCH] add back links previously the back link went to choosing a library. Now, if you view a district from a county, go back to the county page. Otherwise, go back to the top level of the library. --- app/main/views/broadcast.py | 23 ++++++ app/templates/views/broadcast/counties.html | 4 +- app/templates/views/broadcast/sub-areas.html | 2 +- tests/app/main/views/test_broadcast.py | 73 +++++++++++++++----- 4 files changed, 81 insertions(+), 21 deletions(-) diff --git a/app/main/views/broadcast.py b/app/main/views/broadcast.py index 16b820f65..65ff1ae24 100644 --- a/app/main/views/broadcast.py +++ b/app/main/views/broadcast.py @@ -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( '/services//broadcast//libraries//', 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] + 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) 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}', broadcast_message=broadcast_message, county=area, + back_link=back_link, ) return render_template( @@ -195,6 +217,7 @@ def choose_broadcast_sub_area(service_id, broadcast_message_id, library_slug, ar library_slug=library_slug, page_title=f'Choose an area of {area.name}', broadcast_message=broadcast_message, + back_link=back_link, ) diff --git a/app/templates/views/broadcast/counties.html b/app/templates/views/broadcast/counties.html index 25a317c21..e50e091cb 100644 --- a/app/templates/views/broadcast/counties.html +++ b/app/templates/views/broadcast/counties.html @@ -15,7 +15,7 @@ {{ page_header( 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() %} @@ -30,7 +30,7 @@ {# these are districts within a county#} {{ area.name }} {% endfor %} diff --git a/app/templates/views/broadcast/sub-areas.html b/app/templates/views/broadcast/sub-areas.html index ca638b459..71b446996 100644 --- a/app/templates/views/broadcast/sub-areas.html +++ b/app/templates/views/broadcast/sub-areas.html @@ -13,7 +13,7 @@ {{ page_header( 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() %} diff --git a/tests/app/main/views/test_broadcast.py b/tests/app/main/views/test_broadcast.py index caad2f2bc..1b1f0018c 100644 --- a/tests/app/main/views/test_broadcast.py +++ b/tests/app/main/views/test_broadcast.py @@ -453,22 +453,12 @@ def test_choose_broadcast_area_page_for_area_with_sub_areas( for choice in page.select('.file-list-item') ] assert len(choices) == 394 - assert choices[:2] == [ - ( - partial_url_for(area_slug='lad20-S12000033'), - 'Aberdeen City', - ), - ( - partial_url_for(area_slug='lad20-S12000034'), - 'Aberdeenshire', - ), - ] - assert choices[-1:] == [ - ( - partial_url_for(area_slug='lad20-E06000014'), - 'York', - ), - ] + + assert choices[0] == (partial_url_for(area_slug='lad20-S12000033'), 'Aberdeen City',) + # note: we don't populate prev_area_slug query param, so the back link will come here rather than to a county page, + # 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',) 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( client_request, service_one, @@ -563,7 +598,8 @@ def test_choose_broadcast_sub_area_page_for_county_shows_links_for_districts( service_id=SERVICE_ONE_ID, broadcast_message_id=fake_uuid, 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[-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, broadcast_message_id=fake_uuid, 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'