From 857090173105a5cc6008ac71e6228b38aca1e2b9 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 31 Jul 2020 16:20:37 +0100 Subject: [PATCH] Let users select electoral wards of local authorities If a library has groups, we should show a link instead of selecting the group directly. Then we can give the user the choice of selecting the whole of that group, or specific areas within the group. For now the only libraries we have with groups are local authorities, which group electoral wards. --- app/main/forms.py | 20 +++ app/main/views/broadcast.py | 46 ++++++ app/navigation.py | 4 + .../views/broadcast/areas-with-sub-areas.html | 25 +++ app/templates/views/broadcast/sub-areas.html | 33 ++++ tests/app/main/views/test_broadcast.py | 146 +++++++++++++++++- 6 files changed, 273 insertions(+), 1 deletion(-) create mode 100644 app/templates/views/broadcast/areas-with-sub-areas.html create mode 100644 app/templates/views/broadcast/sub-areas.html diff --git a/app/main/forms.py b/app/main/forms.py index 8a4f09e2c..77c6d7cfd 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -2161,3 +2161,23 @@ class BroadcastAreaForm(StripWhitespaceForm): return cls(choices=[ (area.id, area.name) for area in sorted(library) ]) + + +class BroadcastAreaFormWithSelectAll(BroadcastAreaForm): + + select_all = govukCheckboxField('Select all') + + @classmethod + def from_library(cls, library, select_all_choice): + instance = super().from_library(library) + ( + instance.select_all.area_slug, + instance.select_all.label.text, + ) = select_all_choice + return instance + + @property + def selected_areas(self): + if self.select_all.data: + return [self.select_all.area_slug] + return self.areas.data diff --git a/app/main/views/broadcast.py b/app/main/views/broadcast.py index ff35b505d..180536ba8 100644 --- a/app/main/views/broadcast.py +++ b/app/main/views/broadcast.py @@ -12,6 +12,7 @@ from app import current_service from app.main import main from app.main.forms import ( BroadcastAreaForm, + BroadcastAreaFormWithSelectAll, ChooseBroadcastDurationForm, SearchByNameForm, ) @@ -118,6 +119,16 @@ def choose_broadcast_area(service_id, broadcast_message_id, library_slug): service_id=current_service.id, ) library = BroadcastMessage.libraries.get(library_slug) + + if library.is_group: + return render_template( + 'views/broadcast/areas-with-sub-areas.html', + search_form=SearchByNameForm(), + show_search_form=(len(library) > 7), + library=library, + broadcast_message=broadcast_message, + ) + form = BroadcastAreaForm.from_library(library) if form.validate_on_submit(): broadcast_message.add_areas(*form.areas.data) @@ -136,6 +147,41 @@ def choose_broadcast_area(service_id, broadcast_message_id, library_slug): ) +@main.route( + '/services//broadcast//libraries//', + methods=['GET', 'POST'], +) +@user_has_permissions('send_messages') +@service_has_permission('broadcast') +def choose_broadcast_sub_area(service_id, broadcast_message_id, library_slug, area_slug): + broadcast_message = BroadcastMessage.from_id( + broadcast_message_id, + service_id=current_service.id, + ) + area = BroadcastMessage.libraries.get_areas(area_slug)[0] + + form = BroadcastAreaFormWithSelectAll.from_library( + area.sub_areas, + select_all_choice=(area.id, f'All of {area.name}'), + ) + if form.validate_on_submit(): + broadcast_message.add_areas(*form.selected_areas) + return redirect(url_for( + '.preview_broadcast_areas', + service_id=current_service.id, + broadcast_message_id=broadcast_message.id, + )) + return render_template( + 'views/broadcast/sub-areas.html', + form=form, + search_form=SearchByNameForm(), + show_search_form=(len(form.areas.choices) > 7), + library_slug=library_slug, + page_title=f'Choose an area of {area.name}', + broadcast_message=broadcast_message, + ) + + @main.route('/services//broadcast//remove/') @user_has_permissions('send_messages') @service_has_permission('broadcast') diff --git a/app/navigation.py b/app/navigation.py index f5ddc5323..204c50f0b 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -359,6 +359,7 @@ class HeaderNavigation(Navigation): 'preview_broadcast_areas', 'choose_broadcast_library', 'choose_broadcast_area', + 'choose_broadcast_sub_area', 'remove_broadcast_area', 'preview_broadcast_message', 'view_broadcast_message', @@ -420,6 +421,7 @@ class MainNavigation(Navigation): 'preview_broadcast_areas', 'choose_broadcast_library', 'choose_broadcast_area', + 'choose_broadcast_sub_area', 'remove_broadcast_area', 'preview_broadcast_message', 'view_broadcast_message', @@ -1016,6 +1018,7 @@ class CaseworkNavigation(Navigation): 'preview_broadcast_areas', 'choose_broadcast_library', 'choose_broadcast_area', + 'choose_broadcast_sub_area', 'remove_broadcast_area', 'preview_broadcast_message', 'view_broadcast_message', @@ -1340,6 +1343,7 @@ class OrgNavigation(Navigation): 'preview_broadcast_areas', 'choose_broadcast_library', 'choose_broadcast_area', + 'choose_broadcast_sub_area', 'remove_broadcast_area', 'preview_broadcast_message', 'view_broadcast_message', diff --git a/app/templates/views/broadcast/areas-with-sub-areas.html b/app/templates/views/broadcast/areas-with-sub-areas.html new file mode 100644 index 000000000..c62e09ab4 --- /dev/null +++ b/app/templates/views/broadcast/areas-with-sub-areas.html @@ -0,0 +1,25 @@ +{% from "components/page-header.html" import page_header %} +{% from "components/live-search.html" import live_search %} + +{% extends "withnav_template.html" %} + +{% block service_page_title %} + {{ library.name }} +{% endblock %} + +{% block maincolumn_content %} + + {{ page_header( + library.name, + back_link=url_for('.choose_broadcast_library', service_id=current_service.id, broadcast_message_id=broadcast_message.id), + )}} + + {{ live_search(target_selector='.file-list-item', show=show_search_form, form=search_form, label='Search by name') }} + + {% for area in library|sort %} +
+ {{ area.name }} +
+ {% endfor %} + +{% endblock %} diff --git a/app/templates/views/broadcast/sub-areas.html b/app/templates/views/broadcast/sub-areas.html new file mode 100644 index 000000000..6a9f6dfd9 --- /dev/null +++ b/app/templates/views/broadcast/sub-areas.html @@ -0,0 +1,33 @@ +{% from "components/page-header.html" import page_header %} +{% from "components/page-footer.html" import sticky_page_footer %} +{% from "components/form.html" import form_wrapper %} +{% from "components/live-search.html" import live_search %} + +{% extends "withnav_template.html" %} + +{% block service_page_title %} + {{ page_title }} +{% endblock %} + +{% block maincolumn_content %} + + {{ page_header( + page_title, + back_link=url_for('.choose_broadcast_library', service_id=current_service.id, broadcast_message_id=broadcast_message.id), + )}} + + {% call form_wrapper() %} + +
+ {{ form.select_all }} +
+ + {{ live_search(target_selector='.multiple-choice', show=show_search_form, form=search_form, label='Or by electoral ward') }} + + {{ form.areas }} + + {{ sticky_page_footer('Add to broadcast') }} + + {% endcall %} + +{% endblock %} diff --git a/tests/app/main/views/test_broadcast.py b/tests/app/main/views/test_broadcast.py index 0570e1963..a223b6982 100644 --- a/tests/app/main/views/test_broadcast.py +++ b/tests/app/main/views/test_broadcast.py @@ -1,5 +1,6 @@ import json import uuid +from functools import partial import pytest from flask import url_for @@ -257,12 +258,104 @@ def test_choose_broadcast_area_page( fake_uuid, ): service_one['permissions'] += ['broadcast'] - client_request.get( + page = client_request.get( '.choose_broadcast_area', service_id=SERVICE_ONE_ID, broadcast_message_id=fake_uuid, library_slug='countries', ) + assert [ + ( + choice.select_one('input')['value'], + normalize_spaces(choice.select_one('label').text), + ) + for choice in page.select('form[method=post] .govuk-checkboxes__item') + ] == [ + ('countries-E92000001', 'England'), + ('countries-N92000002', 'Northern Ireland'), + ('countries-S92000003', 'Scotland'), + ('countries-W92000004', 'Wales'), + ] + + +def test_choose_broadcast_area_page_for_area_with_sub_areas( + client_request, + service_one, + mock_get_draft_broadcast_message, + fake_uuid, +): + service_one['permissions'] += ['broadcast'] + page = client_request.get( + '.choose_broadcast_area', + service_id=SERVICE_ONE_ID, + broadcast_message_id=fake_uuid, + library_slug='electoral-wards-of-the-united-kingdom', + ) + partial_url_for = partial( + url_for, + 'main.choose_broadcast_sub_area', + service_id=SERVICE_ONE_ID, + broadcast_message_id=fake_uuid, + library_slug='electoral-wards-of-the-united-kingdom', + ) + choices = [ + ( + choice.select_one('a.file-list-filename-large')['href'], + normalize_spaces(choice.text), + ) + for choice in page.select('.file-list-item') + ] + assert len(choices) == 379 + assert choices[:2] == [ + ( + partial_url_for(area_slug='electoral-wards-of-the-united-kingdom-S12000033'), + 'Aberdeen City', + ), + ( + partial_url_for(area_slug='electoral-wards-of-the-united-kingdom-S12000034'), + 'Aberdeenshire', + ), + ] + assert choices[-1:] == [ + ( + partial_url_for(area_slug='electoral-wards-of-the-united-kingdom-E06000014'), + 'York', + ), + ] + + +def test_choose_broadcast_sub_area_page( + client_request, + service_one, + mock_get_draft_broadcast_message, + fake_uuid, +): + service_one['permissions'] += ['broadcast'] + page = client_request.get( + 'main.choose_broadcast_sub_area', + service_id=SERVICE_ONE_ID, + broadcast_message_id=fake_uuid, + library_slug='electoral-wards-of-the-united-kingdom', + area_slug='electoral-wards-of-the-united-kingdom-S12000033', + ) + assert normalize_spaces(page.select_one('h1').text) == ( + 'Choose an area of Aberdeen City' + ) + choices = [ + ( + choice.select_one('input')['value'], + normalize_spaces(choice.select_one('label').text), + ) + for choice in page.select('form[method=post] .govuk-checkboxes__item') + ] + assert choices[:3] == [ + ('y', 'All of Aberdeen City'), + ('electoral-wards-of-the-united-kingdom-S13002845', 'Airyhall/Broomhill/Garthdee'), + ('electoral-wards-of-the-united-kingdom-S13002836', 'Bridge of Don'), + ] + assert choices[-1:] == [ + ('electoral-wards-of-the-united-kingdom-S13002846', 'Torry/Ferryhill'), + ] def test_add_broadcast_area( @@ -291,6 +384,57 @@ def test_add_broadcast_area( ) +@pytest.mark.parametrize('post_data, expected_selected', ( + ({ + 'select_all': 'y', + 'areas': [ + 'electoral-wards-of-the-united-kingdom-S13002845', + ] + }, [ + 'electoral-wards-of-the-united-kingdom-S12000033', + # S13002845 is ignored because the user chose ‘Select all…’ + ]), + ({ + 'areas': [ + 'electoral-wards-of-the-united-kingdom-S13002845', + 'electoral-wards-of-the-united-kingdom-S13002836', + ] + }, [ + 'electoral-wards-of-the-united-kingdom-S13002845', + 'electoral-wards-of-the-united-kingdom-S13002836', + ]), +)) +def test_add_broadcast_sub_area( + client_request, + service_one, + mock_get_draft_broadcast_message, + mock_update_broadcast_message, + fake_uuid, + post_data, + expected_selected, +): + service_one['permissions'] += ['broadcast'] + client_request.post( + '.choose_broadcast_sub_area', + service_id=SERVICE_ONE_ID, + broadcast_message_id=fake_uuid, + library_slug='countries', + area_slug='electoral-wards-of-the-united-kingdom-S12000033', + _data=post_data, + ) + mock_update_broadcast_message.assert_called_once_with( + service_id=SERVICE_ONE_ID, + broadcast_message_id=fake_uuid, + data={ + 'areas': [ + # These two areas are on the broadcast already + 'countries-E92000001', + 'countries-S92000003', + ] + expected_selected + }, + ) + + def test_remove_broadcast_area_page( client_request, service_one,