From 18d464d4f0b5ae1a89879ec526308c8695b7884d Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 6 Jul 2020 10:53:14 +0100 Subject: [PATCH 1/5] Add some views for selecting broadcast areas MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These are just so we have some pages to click through for now. They don’t use real templates, or any of the broadcast stuff from the database. But I think it’s useful to get some skeleton pages in first so that we can see the map etc working in production, then build on that, without having to do it all in one mega PR. For that reason there are two short term things I’ve done in this commit which should be revisited soon: - no tests for the endpoints - data about which areas are selected is stored in the session --- app/main/__init__.py | 1 + app/main/views/broadcast.py | 106 +++++++++++++++ app/navigation.py | 28 ++++ app/templates/views/broadcast/areas.html | 20 +++ app/templates/views/broadcast/libraries.html | 21 +++ .../views/broadcast/preview-areas.html | 47 +++++++ .../views/broadcast/preview-message.html | 41 ++++++ app/templates/views/templates/_template.html | 9 +- requirements-app.txt | 2 +- requirements.txt | 7 +- tests/app/main/views/test_broadcast.py | 121 ++++++++++++++++++ tests/app/main/views/test_templates.py | 4 + 12 files changed, 402 insertions(+), 5 deletions(-) create mode 100644 app/main/views/broadcast.py create mode 100644 app/templates/views/broadcast/areas.html create mode 100644 app/templates/views/broadcast/libraries.html create mode 100644 app/templates/views/broadcast/preview-areas.html create mode 100644 app/templates/views/broadcast/preview-message.html create mode 100644 tests/app/main/views/test_broadcast.py diff --git a/app/main/__init__.py b/app/main/__init__.py index 97aa9f228..29208e5cd 100644 --- a/app/main/__init__.py +++ b/app/main/__init__.py @@ -7,6 +7,7 @@ from app.main.views import ( # noqa isort:skip add_service, agreement, api_keys, + broadcast, choose_account, code_not_received, conversation, diff --git a/app/main/views/broadcast.py b/app/main/views/broadcast.py new file mode 100644 index 000000000..f5e90aeeb --- /dev/null +++ b/app/main/views/broadcast.py @@ -0,0 +1,106 @@ +from flask import redirect, render_template, request, session, url_for +from notifications_utils.broadcast_areas import broadcast_area_libraries +from orderedset import OrderedSet + +from app import current_service +from app.main import main +from app.utils import service_has_permission, user_has_permissions + + +@main.route('/services//broadcast') +@user_has_permissions('send_messages') +@service_has_permission('broadcast') +def broadcast(service_id): + if 'broadcast_areas' in session: + session.pop('broadcast_areas') + return redirect(url_for( + '.preview_broadcast_areas', + service_id=current_service.id, + )) + + +@main.route('/services//broadcast/areas') +@user_has_permissions('send_messages') +@service_has_permission('broadcast') +def preview_broadcast_areas(service_id): + selected_areas_ids = session.get('broadcast_areas', []) + return render_template( + 'views/broadcast/preview-areas.html', + selected=list(broadcast_area_libraries.get_areas( + *selected_areas_ids + )), + area_polygons=broadcast_area_libraries.get_polygons_for_areas_lat_long( + *selected_areas_ids + ) + ) + + +@main.route('/services//broadcast/libraries') +@user_has_permissions('send_messages') +@service_has_permission('broadcast') +def choose_broadcast_library(service_id): + return render_template( + 'views/broadcast/libraries.html', + libraries=broadcast_area_libraries, + selected=broadcast_area_libraries.get_areas( + *session.get('broadcast_areas', []) + ), + ) + + +@main.route('/services//broadcast/libraries/') +@user_has_permissions('send_messages') +@service_has_permission('broadcast') +def choose_broadcast_area(service_id, library_slug): + return render_template( + 'views/broadcast/areas.html', + areas=broadcast_area_libraries.get(library_slug), + ) + + +@main.route('/services//broadcast/add/') +@user_has_permissions('send_messages') +@service_has_permission('broadcast') +def add_broadcast_area(service_id, area_slug): + if not session.get('broadcast_areas'): + session['broadcast_areas'] = [] + + session['broadcast_areas'].append(area_slug) + + session['broadcast_areas'] = list(OrderedSet( + session['broadcast_areas'] + )) + return redirect(url_for( + '.preview_broadcast_areas', + service_id=current_service.id, + )) + + +@main.route('/services//broadcast/remove/') +@user_has_permissions('send_messages') +@service_has_permission('broadcast') +def remove_broadcast_area(service_id, area_slug): + session['broadcast_areas'] = list(filter( + lambda saved_area_id: saved_area_id != area_slug, + session.get('broadcast_areas', []), + )) + + return redirect(url_for( + '.preview_broadcast_areas', + service_id=current_service.id, + )) + + +@main.route('/services//broadcast/preview', methods=['GET', 'POST']) +@user_has_permissions('send_messages') +@service_has_permission('broadcast') +def preview_broadcast_message(service_id): + if request.method == 'POST': + return 'OK' + selected_areas = session.get('broadcast_areas', []) + return render_template( + 'views/broadcast/preview-message.html', + selected=list(broadcast_area_libraries.get_areas( + *selected_areas + )), + ) diff --git a/app/navigation.py b/app/navigation.py index 1aa043ef2..51717ef52 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -351,6 +351,13 @@ class HeaderNavigation(Navigation): 'old_guest_list', 'who_can_use_notify', 'who_its_for', + 'broadcast', + 'preview_broadcast_areas', + 'choose_broadcast_library', + 'choose_broadcast_area', + 'add_broadcast_area', + 'remove_broadcast_area', + 'preview_broadcast_message', } # header HTML now comes from GOVUK Frontend so requires a boolean, not an attribute @@ -399,6 +406,13 @@ class MainNavigation(Navigation): 'view_template', 'view_template_version', 'view_template_versions', + 'broadcast', + 'preview_broadcast_areas', + 'choose_broadcast_library', + 'choose_broadcast_area', + 'add_broadcast_area', + 'remove_broadcast_area', + 'preview_broadcast_message', }, 'uploads': { 'upload_contact_list', @@ -978,6 +992,13 @@ class CaseworkNavigation(Navigation): 'old_guest_list', 'who_can_use_notify', 'who_its_for', + 'broadcast', + 'preview_broadcast_areas', + 'choose_broadcast_library', + 'choose_broadcast_area', + 'add_broadcast_area', + 'remove_broadcast_area', + 'preview_broadcast_message', } @@ -1288,4 +1309,11 @@ class OrgNavigation(Navigation): 'old_guest_list', 'who_can_use_notify', 'who_its_for', + 'broadcast', + 'preview_broadcast_areas', + 'choose_broadcast_library', + 'choose_broadcast_area', + 'add_broadcast_area', + 'remove_broadcast_area', + 'preview_broadcast_message', } diff --git a/app/templates/views/broadcast/areas.html b/app/templates/views/broadcast/areas.html new file mode 100644 index 000000000..55db5a0e2 --- /dev/null +++ b/app/templates/views/broadcast/areas.html @@ -0,0 +1,20 @@ +{% from "components/page-header.html" import page_header %} + +{% extends "withnav_template.html" %} + +{% block service_page_title %} + {{ page_title }} +{% endblock %} + +{% block maincolumn_content %} + + {{ page_header( + regions.name, + back_link=url_for('.choose_broadcast_library', service_id=current_service.id), + )}} + + {% for area in areas %} +

{{ region.name }}

+ {% endfor %} + +{% endblock %} diff --git a/app/templates/views/broadcast/libraries.html b/app/templates/views/broadcast/libraries.html new file mode 100644 index 000000000..e029febd8 --- /dev/null +++ b/app/templates/views/broadcast/libraries.html @@ -0,0 +1,21 @@ +{% from "components/page-header.html" import page_header %} + +{% extends "withnav_template.html" %} + +{% block service_page_title %} + Choose where to broadcast to +{% endblock %} + +{% block maincolumn_content %} + + {{ page_header( + "Choose where to broadcast to", + back_link=url_for(".preview_broadcast_areas", service_id=current_service.id) + ) }} + + {% for library in libraries|sort %} + {{ library.name }} +

{{ library.examples }}

+ {% endfor %} + +{% endblock %} diff --git a/app/templates/views/broadcast/preview-areas.html b/app/templates/views/broadcast/preview-areas.html new file mode 100644 index 000000000..6b86fc3ba --- /dev/null +++ b/app/templates/views/broadcast/preview-areas.html @@ -0,0 +1,47 @@ +{% from "components/button/macro.njk" import govukButton %} +{% from "components/page-header.html" import page_header %} +{% from "components/page-footer.html" import sticky_page_footer %} + +{% extends "withnav_template.html" %} + +{% block service_page_title %} + Choose where to broadcast to +{% endblock %} + +{% block maincolumn_content %} + + {{ page_header("Choose where to broadcast to", back_link="#") }} + + {% for area in selected %} + {{ area.name }} (remove)  + {% if loop.last %} +

+ {{ govukButton({ + "element": "a", + "text": "Add another area", + "href": url_for('.choose_broadcast_library', service_id=current_service.id), + "classes": "govuk-button--secondary govuk-!-margin-top-2" + }) }} +

+ {% endif %} + {% else %} +

+ {{ govukButton({ + "element": "a", + "text": "Add broadcast areas", + "href": url_for('.choose_broadcast_library', service_id=current_service.id), + "classes": "govuk-button--secondary" + }) }} +

+ {% endfor %} + + {% if selected %} + {% for area in area_polygons %} + {{ area }}

+ {% endfor %} +
+ {{ sticky_page_footer('Continue to preview') }} +
+ {% endif %} + +{% endblock %} diff --git a/app/templates/views/broadcast/preview-message.html b/app/templates/views/broadcast/preview-message.html new file mode 100644 index 000000000..7a7ecf719 --- /dev/null +++ b/app/templates/views/broadcast/preview-message.html @@ -0,0 +1,41 @@ +{% from "components/button/macro.njk" import govukButton %} +{% from "components/form.html" import form_wrapper %} +{% from "components/page-header.html" import page_header %} +{% from "components/page-footer.html" import sticky_page_footer %} + +{% extends "withnav_template.html" %} + +{% block service_page_title %} + Preview message +{% endblock %} + +{% block maincolumn_content %} + + {{ page_header("Preview message", back_link=url_for('.preview_broadcast_areas', service_id=current_service.id)) }} + + {% for region in selected %} + {% if loop.first %} +
    + {% endif %} +
  • + {{ region.name }} +
    • + {% if loop.last %} +
+ {% endif %} + {% endfor %} + + {% if selected %} + +

+ {{ "" }} +

+ + {% call form_wrapper() %} + {{ sticky_page_footer('Start broadcast') }} + {% endcall %} + + {% endif %} + + +{% endblock %} diff --git a/app/templates/views/templates/_template.html b/app/templates/views/templates/_template.html index e5913d5d6..8f3bbf321 100644 --- a/app/templates/views/templates/_template.html +++ b/app/templates/views/templates/_template.html @@ -29,8 +29,15 @@ {% endif %} {% elif template.template_type == 'broadcast' %} + {% if current_user.has_permissions('send_messages') %} +
+ + Prepare broadcast + +
+ {% endif %} {% if current_user.has_permissions('manage_templates') %} -
+
Edit diff --git a/requirements-app.txt b/requirements-app.txt index 5a2afd14c..b6514ae12 100644 --- a/requirements-app.txt +++ b/requirements-app.txt @@ -23,7 +23,7 @@ notifications-python-client==5.6.0 awscli-cwlogs>=1.4,<1.5 itsdangerous==1.1.0 -git+https://github.com/alphagov/notifications-utils.git@40.2.1#egg=notifications-utils==40.2.1 +git+https://github.com/alphagov/notifications-utils.git@40.3.1#egg=notifications-utils==40.3.1 git+https://github.com/alphagov/govuk-frontend-jinja.git@v0.5.1-alpha#egg=govuk-frontend-jinja==0.5.1-alpha # gds-metrics requires prometheseus 0.2.0, override that requirement as later versions bring significant performance gains diff --git a/requirements.txt b/requirements.txt index 037366b83..29cf99318 100644 --- a/requirements.txt +++ b/requirements.txt @@ -25,7 +25,7 @@ notifications-python-client==5.6.0 awscli-cwlogs>=1.4,<1.5 itsdangerous==1.1.0 -git+https://github.com/alphagov/notifications-utils.git@40.2.1#egg=notifications-utils==40.2.1 +git+https://github.com/alphagov/notifications-utils.git@40.3.1#egg=notifications-utils==40.3.1 git+https://github.com/alphagov/govuk-frontend-jinja.git@v0.5.1-alpha#egg=govuk-frontend-jinja==0.5.1-alpha # gds-metrics requires prometheseus 0.2.0, override that requirement as later versions bring significant performance gains @@ -33,10 +33,10 @@ prometheus-client==0.8.0 gds-metrics==0.2.0 ## The following requirements were added by pip freeze: -awscli==1.18.93 +awscli==1.18.95 bleach==3.1.4 boto3==1.10.38 -botocore==1.17.16 +botocore==1.17.18 cachetools==4.1.0 certifi==2020.6.20 chardet==3.0.4 @@ -48,6 +48,7 @@ docutils==0.15.2 et-xmlfile==1.0.1 flask-redis==0.4.0 future==0.18.2 +geojson==2.5.0 greenlet==0.4.16 idna==2.10 jdcal==1.4.1 diff --git a/tests/app/main/views/test_broadcast.py b/tests/app/main/views/test_broadcast.py new file mode 100644 index 000000000..378054a51 --- /dev/null +++ b/tests/app/main/views/test_broadcast.py @@ -0,0 +1,121 @@ +import pytest +from flask import url_for + +from tests.conftest import SERVICE_ONE_ID + + +@pytest.mark.parametrize('endpoint, extra_args', ( + ('.broadcast', {}), + ('.preview_broadcast_areas', {}), + ('.choose_broadcast_library', {}), + ('.choose_broadcast_area', {'library_slug': 'countries'}), + ('.add_broadcast_area', {'area_slug': 'england'}), + ('.remove_broadcast_area', {'area_slug': 'england'}), + ('.preview_broadcast_message', {}), +)) +def test_broadcast_pages_403_without_permission( + client_request, + endpoint, + extra_args, +): + client_request.get( + endpoint, + service_id=SERVICE_ONE_ID, + _expected_status=403, + **extra_args + ) + + +def test_broadcast_page( + client_request, + service_one, +): + service_one['permissions'] += ['broadcast'] + client_request.get( + '.broadcast', + service_id=SERVICE_ONE_ID, + _expected_redirect=url_for( + '.preview_broadcast_areas', + service_id=SERVICE_ONE_ID, + _external=True, + ), + ), + + +def test_preview_broadcast_areas_page( + client_request, + service_one, +): + service_one['permissions'] += ['broadcast'] + client_request.get( + '.preview_broadcast_areas', + service_id=SERVICE_ONE_ID, + ) + + +def test_choose_broadcast_library_page( + client_request, + service_one, +): + service_one['permissions'] += ['broadcast'] + client_request.get( + '.choose_broadcast_library', + service_id=SERVICE_ONE_ID, + ) + + +def test_choose_broadcast_area_page( + client_request, + service_one, +): + service_one['permissions'] += ['broadcast'] + client_request.get( + '.choose_broadcast_area', + service_id=SERVICE_ONE_ID, + library_slug='countries', + ) + + +def test_add_broadcast_area_page( + client_request, + service_one, +): + service_one['permissions'] += ['broadcast'] + client_request.get( + '.add_broadcast_area', + service_id=SERVICE_ONE_ID, + area_slug='england', + _expected_redirect=url_for( + '.preview_broadcast_areas', + service_id=SERVICE_ONE_ID, + _external=True, + ), + ), + + +def test_remove_broadcast_area_page( + client_request, + service_one, +): + service_one['permissions'] += ['broadcast'] + client_request.get( + '.remove_broadcast_area', + service_id=SERVICE_ONE_ID, + area_slug='england', + _expected_redirect=url_for( + '.preview_broadcast_areas', + service_id=SERVICE_ONE_ID, + _external=True, + ), + ), + + +def test_preview_broadcast_message_page( + client_request, + service_one, +): + service_one['permissions'] += ['broadcast'] + client_request.get( + '.preview_broadcast_message', + service_id=SERVICE_ONE_ID, + ), diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py index bc07a0c49..d44a2794f 100644 --- a/tests/app/main/views/test_templates.py +++ b/tests/app/main/views/test_templates.py @@ -693,6 +693,10 @@ def test_view_broadcast_template( (link.text.strip(), link['href']) for link in page.select('.pill-separate-item') ] == [ + ('Prepare broadcast', url_for( + '.broadcast', + service_id=SERVICE_ONE_ID, + )), ('Edit', url_for( '.edit_service_template', service_id=SERVICE_ONE_ID, From d0d3fc6857df6097eae15f1fef2ad3b6be103615 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 6 Jul 2020 10:53:14 +0100 Subject: [PATCH 2/5] Add a map MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit So you can check you’ve chosen the right areas, and to give you a clear idea of where the boundaries of an area are. The Javascript and CSS for the map is only loaded on this page because it adds quite a few kb, and we don’t want to be sending assets to the majority of our users who will never see them. --- app/__init__.py | 3 +- app/templates/admin_template.html | 4 ++ .../views/broadcast/preview-areas.html | 45 +++++++++++++++++-- tests/app/main/views/test_headers.py | 8 ++-- 4 files changed, 52 insertions(+), 8 deletions(-) diff --git a/app/__init__.py b/app/__init__.py index b596bdfdd..61237c7dc 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -612,7 +612,8 @@ def useful_headers_after_request(response): "connect-src 'self' *.google-analytics.com;" "object-src 'self';" "font-src 'self' {asset_domain} data:;" - "img-src 'self' {asset_domain} *.google-analytics.com *.notifications.service.gov.uk {logo_domain} data:;" + "img-src 'self' {asset_domain} *.tile.openstreetmap.org *.google-analytics.com" + " *.notifications.service.gov.uk {logo_domain} data:;" "frame-src 'self' www.youtube-nocookie.com;".format( asset_domain=current_app.config['ASSET_DOMAIN'], logo_domain=get_logo_cdn_domain(), diff --git a/app/templates/admin_template.html b/app/templates/admin_template.html index ec40d460b..318f116af 100644 --- a/app/templates/admin_template.html +++ b/app/templates/admin_template.html @@ -14,6 +14,8 @@ {% block head %} + {% block extra_stylesheets %} + {% endblock %} @@ -251,6 +253,8 @@ {% endblock %} {% block bodyEnd %} + {% block extra_javascripts %} + {% endblock %} diff --git a/app/templates/views/broadcast/preview-areas.html b/app/templates/views/broadcast/preview-areas.html index 6b86fc3ba..2793bff16 100644 --- a/app/templates/views/broadcast/preview-areas.html +++ b/app/templates/views/broadcast/preview-areas.html @@ -8,9 +8,48 @@ Choose where to broadcast to {% endblock %} +{% block extra_stylesheets %} + + +{% endblock %} + +{% block extra_javascripts %} + + +{% endblock %} + {% block maincolumn_content %} - {{ page_header("Choose where to broadcast to", back_link="#") }} + {{ page_header("Choose where to broadcast to", back_link="{{ url_for('.broadcast', service_id=current_service.id) }}") }} {% for area in selected %} {{ area.name }} (remove)  @@ -36,9 +75,7 @@ {% endfor %} {% if selected %} - {% for area in area_polygons %} - {{ area }}

- {% endfor %} +
{{ sticky_page_footer('Continue to preview') }}
diff --git a/tests/app/main/views/test_headers.py b/tests/app/main/views/test_headers.py index be940833d..003a208c1 100644 --- a/tests/app/main/views/test_headers.py +++ b/tests/app/main/views/test_headers.py @@ -20,7 +20,8 @@ def test_owasp_useful_headers_set( "object-src 'self';" "font-src 'self' static.example.com data:;" "img-src " - "'self' static.example.com *.google-analytics.com *.notifications.service.gov.uk static-logos.test.com data:;" + "'self' static.example.com *.tile.openstreetmap.org *.google-analytics.com" + " *.notifications.service.gov.uk static-logos.test.com data:;" "frame-src 'self' www.youtube-nocookie.com;" ) @@ -41,7 +42,8 @@ def test_headers_non_ascii_characters_are_replaced( "connect-src 'self' *.google-analytics.com;" "object-src 'self';" "font-src 'self' static.example.com data:;" - "img-src " - "'self' static.example.com *.google-analytics.com *.notifications.service.gov.uk static-logos??.test.com data:;" + "img-src" + " 'self' static.example.com *.tile.openstreetmap.org *.google-analytics.com" + " *.notifications.service.gov.uk static-logos??.test.com data:;" "frame-src 'self' www.youtube-nocookie.com;" ) From 49444221e9dfc1f2bdd18629013b35458dcdb44a Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 6 Jul 2020 10:53:15 +0100 Subject: [PATCH 3/5] Style the area labels and polygons Gives them some colours, borders and stuff to make them visually consistent with the rest of Notify. The idea is the tags and polygons have a similar affordances (i.e. border thickness, colour) to visually link them and imply that they are two representations of the same thing. --- .../stylesheets/components/area-list.scss | 64 +++++++++++++++++++ app/assets/stylesheets/main.scss | 1 + .../views/broadcast/preview-areas.html | 35 ++++++---- .../views/broadcast/preview-message.html | 8 +-- 4 files changed, 93 insertions(+), 15 deletions(-) create mode 100644 app/assets/stylesheets/components/area-list.scss diff --git a/app/assets/stylesheets/components/area-list.scss b/app/assets/stylesheets/components/area-list.scss new file mode 100644 index 000000000..24046d611 --- /dev/null +++ b/app/assets/stylesheets/components/area-list.scss @@ -0,0 +1,64 @@ +.area-list { + + &-item { + + display: inline-block; + border: 2px solid $black; + padding: (govuk-spacing(1) + 1px) (govuk-spacing(2) + 35px) govuk-spacing(1) govuk-spacing(2); + margin: 0 govuk-spacing(1) govuk-spacing(2) 0; + position: relative; + + &-remove { + + font-size: 0; + + &:before { + content: "×"; + display: block; + position: absolute; + top: -2px; + right: -2px; + bottom: -2px; + width: 35px; + background: $govuk-blue; + color: $white; + box-shadow: -2px 0 0 0 $black, inset 1px 0 0 0 rgba($white, 0.1); + border: 2px solid $govuk-blue; + border-left: none; + font-size: 24px; + font-weight: normal; + line-height: 40px; + text-align: center; + text-decoration: none; + } + + &:hover, + &:focus { + + &:before { + color: $light-blue-25; + } + + } + + &:focus { + + outline: none; + + &:before { + background: $focus-colour; + color: $black; + border-color: $black; + } + + } + + } + + } + + .govuk-button { + margin-left: 3px; + } + +} diff --git a/app/assets/stylesheets/main.scss b/app/assets/stylesheets/main.scss index 2a9ae48ae..5f08c25fe 100644 --- a/app/assets/stylesheets/main.scss +++ b/app/assets/stylesheets/main.scss @@ -70,6 +70,7 @@ $path: '/static/images/'; @import 'components/preview-pane'; @import 'components/task-list'; @import 'components/loading-indicator'; +@import 'components/area-list'; @import 'views/dashboard'; @import 'views/users'; diff --git a/app/templates/views/broadcast/preview-areas.html b/app/templates/views/broadcast/preview-areas.html index 2793bff16..8599a7f67 100644 --- a/app/templates/views/broadcast/preview-areas.html +++ b/app/templates/views/broadcast/preview-areas.html @@ -14,6 +14,7 @@ #map { z-index: 50; margin-bottom: 30px; + border: 1px solid #b1b4b6; } {% endblock %} @@ -37,12 +38,20 @@ {% for area in area_polygons %} polygons.push( - L.polygon({{area}}) + L.polygon({{area}}, { + color: '#0b0b0c', // $black + fillColor: '#2B8CC4', // $light-blue + fillOpacity: 0.2, + weight: 2 + }) ); {% endfor %} var polygonGroup = L.featureGroup(polygons).addTo(mymap); - mymap.fitBounds(polygonGroup.getBounds()); + mymap.fitBounds( + polygonGroup.getBounds(), + {padding: [5, 5]} + ); {% endblock %} @@ -52,16 +61,20 @@ {{ page_header("Choose where to broadcast to", back_link="{{ url_for('.broadcast', service_id=current_service.id) }}") }} {% for area in selected %} - {{ area.name }} (remove)  + {% if loop.first %} +
    + {% endif %} +
  • + {{ area.name }} remove +
    • {% if loop.last %} -

    - {{ govukButton({ - "element": "a", - "text": "Add another area", - "href": url_for('.choose_broadcast_library', service_id=current_service.id), - "classes": "govuk-button--secondary govuk-!-margin-top-2" - }) }} -

    + {{ govukButton({ + "element": "a", + "text": "Add another area", + "href": url_for('.choose_broadcast_library', service_id=current_service.id), + "classes": "govuk-button--secondary govuk-!-margin-bottom-5" + }) }} +
{% endif %} {% else %}

diff --git a/app/templates/views/broadcast/preview-message.html b/app/templates/views/broadcast/preview-message.html index 7a7ecf719..fe595c850 100644 --- a/app/templates/views/broadcast/preview-message.html +++ b/app/templates/views/broadcast/preview-message.html @@ -13,12 +13,12 @@ {{ page_header("Preview message", back_link=url_for('.preview_broadcast_areas', service_id=current_service.id)) }} - {% for region in selected %} + {% for area in selected %} {% if loop.first %} -

    +
      {% endif %} -
    • - {{ region.name }} +
    • + {{ area.name }}
      • {% if loop.last %}
    From 29ad5cf510b9b27734efec263c7cb93da652bb0b Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 6 Jul 2020 10:53:16 +0100 Subject: [PATCH 4/5] Add a form for choosing areas MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Picking multiple areas at once definitely feels like a need, so let’s make them checkboxes. --- app/main/forms.py | 15 ++++++++++ app/main/views/broadcast.py | 37 +++++++++++------------- app/navigation.py | 4 --- app/templates/views/broadcast/areas.html | 14 ++++++--- tests/app/main/views/test_broadcast.py | 18 ------------ 5 files changed, 42 insertions(+), 46 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index 911b0531e..713e5e036 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -1809,3 +1809,18 @@ class AcceptAgreementForm(StripWhitespaceForm): float(field.data) except (TypeError, ValueError): raise ValidationError("Must be a number") + + +class BroadcastAreaForm(StripWhitespaceForm): + + areas = MultiCheckboxField('Choose areas to broadcast to') + + def __init__(self, choices, *args, **kwargs): + super().__init__(*args, **kwargs) + self.areas.choices = choices + + @classmethod + def from_library(cls, library): + return cls(choices=[ + (area.id, area.name) for area in sorted(library) + ]) diff --git a/app/main/views/broadcast.py b/app/main/views/broadcast.py index f5e90aeeb..927a1907c 100644 --- a/app/main/views/broadcast.py +++ b/app/main/views/broadcast.py @@ -4,6 +4,7 @@ from orderedset import OrderedSet from app import current_service from app.main import main +from app.main.forms import BroadcastAreaForm from app.utils import service_has_permission, user_has_permissions @@ -48,34 +49,30 @@ def choose_broadcast_library(service_id): ) -@main.route('/services//broadcast/libraries/') +@main.route('/services//broadcast/libraries/', methods=['GET', 'POST']) @user_has_permissions('send_messages') @service_has_permission('broadcast') def choose_broadcast_area(service_id, library_slug): + library = broadcast_area_libraries.get(library_slug) + form = BroadcastAreaForm.from_library(library) + if form.validate_on_submit(): + if not session.get('broadcast_areas'): + session['broadcast_areas'] = [] + session['broadcast_areas'] = session['broadcast_areas'] + form.areas.data + session['broadcast_areas'] = list(OrderedSet( + session['broadcast_areas'] + )) + return redirect(url_for( + '.preview_broadcast_areas', + service_id=current_service.id, + )) return render_template( 'views/broadcast/areas.html', - areas=broadcast_area_libraries.get(library_slug), + form=form, + page_title=library.name, ) -@main.route('/services//broadcast/add/') -@user_has_permissions('send_messages') -@service_has_permission('broadcast') -def add_broadcast_area(service_id, area_slug): - if not session.get('broadcast_areas'): - session['broadcast_areas'] = [] - - session['broadcast_areas'].append(area_slug) - - session['broadcast_areas'] = list(OrderedSet( - session['broadcast_areas'] - )) - return redirect(url_for( - '.preview_broadcast_areas', - service_id=current_service.id, - )) - - @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 51717ef52..07458080c 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -355,7 +355,6 @@ class HeaderNavigation(Navigation): 'preview_broadcast_areas', 'choose_broadcast_library', 'choose_broadcast_area', - 'add_broadcast_area', 'remove_broadcast_area', 'preview_broadcast_message', } @@ -410,7 +409,6 @@ class MainNavigation(Navigation): 'preview_broadcast_areas', 'choose_broadcast_library', 'choose_broadcast_area', - 'add_broadcast_area', 'remove_broadcast_area', 'preview_broadcast_message', }, @@ -996,7 +994,6 @@ class CaseworkNavigation(Navigation): 'preview_broadcast_areas', 'choose_broadcast_library', 'choose_broadcast_area', - 'add_broadcast_area', 'remove_broadcast_area', 'preview_broadcast_message', } @@ -1313,7 +1310,6 @@ class OrgNavigation(Navigation): 'preview_broadcast_areas', 'choose_broadcast_library', 'choose_broadcast_area', - 'add_broadcast_area', 'remove_broadcast_area', 'preview_broadcast_message', } diff --git a/app/templates/views/broadcast/areas.html b/app/templates/views/broadcast/areas.html index 55db5a0e2..c8fbdd7eb 100644 --- a/app/templates/views/broadcast/areas.html +++ b/app/templates/views/broadcast/areas.html @@ -1,4 +1,7 @@ {% from "components/page-header.html" import page_header %} +{% from "components/page-footer.html" import sticky_page_footer %} +{% from "components/checkbox.html" import checkboxes %} +{% from "components/form.html" import form_wrapper %} {% extends "withnav_template.html" %} @@ -9,12 +12,15 @@ {% block maincolumn_content %} {{ page_header( - regions.name, + page_title, back_link=url_for('.choose_broadcast_library', service_id=current_service.id), )}} - {% for area in areas %} -

    {{ region.name }}

    - {% endfor %} + {{ live_search(target_selector='.multiple-choice', show=show_search_form, form=search_form, label='Search by name') }} + + {% call form_wrapper() %} + {{ checkboxes(form.areas, hide_legend=True) }} + {{ 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 378054a51..0719ae6b8 100644 --- a/tests/app/main/views/test_broadcast.py +++ b/tests/app/main/views/test_broadcast.py @@ -9,7 +9,6 @@ from tests.conftest import SERVICE_ONE_ID ('.preview_broadcast_areas', {}), ('.choose_broadcast_library', {}), ('.choose_broadcast_area', {'library_slug': 'countries'}), - ('.add_broadcast_area', {'area_slug': 'england'}), ('.remove_broadcast_area', {'area_slug': 'england'}), ('.preview_broadcast_message', {}), )) @@ -76,23 +75,6 @@ def test_choose_broadcast_area_page( ) -def test_add_broadcast_area_page( - client_request, - service_one, -): - service_one['permissions'] += ['broadcast'] - client_request.get( - '.add_broadcast_area', - service_id=SERVICE_ONE_ID, - area_slug='england', - _expected_redirect=url_for( - '.preview_broadcast_areas', - service_id=SERVICE_ONE_ID, - _external=True, - ), - ), - - def test_remove_broadcast_area_page( client_request, service_one, From 773f0b9ce78a78d0f9a620cb6cd792bd2ce67ac8 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 6 Jul 2020 10:53:16 +0100 Subject: [PATCH 5/5] Add search form MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There can be lots of areas in a library, for example local councils. So when there is, let’s allow people to do the find-as-you-type thing we support in lots of other places. --- app/main/views/broadcast.py | 4 +++- app/templates/views/broadcast/areas.html | 3 +++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/app/main/views/broadcast.py b/app/main/views/broadcast.py index 927a1907c..c2401e49e 100644 --- a/app/main/views/broadcast.py +++ b/app/main/views/broadcast.py @@ -4,7 +4,7 @@ from orderedset import OrderedSet from app import current_service from app.main import main -from app.main.forms import BroadcastAreaForm +from app.main.forms import BroadcastAreaForm, SearchByNameForm from app.utils import service_has_permission, user_has_permissions @@ -69,6 +69,8 @@ def choose_broadcast_area(service_id, library_slug): return render_template( 'views/broadcast/areas.html', form=form, + search_form=SearchByNameForm(), + show_search_form=(len(form.areas.choices) > 7), page_title=library.name, ) diff --git a/app/templates/views/broadcast/areas.html b/app/templates/views/broadcast/areas.html index c8fbdd7eb..b3d979dbc 100644 --- a/app/templates/views/broadcast/areas.html +++ b/app/templates/views/broadcast/areas.html @@ -2,6 +2,7 @@ {% from "components/page-footer.html" import sticky_page_footer %} {% from "components/checkbox.html" import checkboxes %} {% from "components/form.html" import form_wrapper %} +{% from "components/live-search.html" import live_search %} {% extends "withnav_template.html" %} @@ -18,6 +19,8 @@ {{ live_search(target_selector='.multiple-choice', show=show_search_form, form=search_form, label='Search by name') }} + {{ live_search(target_selector='.multiple-choice', show=show_search_form, form=search_form, label='Search by name') }} + {% call form_wrapper() %} {{ checkboxes(form.areas, hide_legend=True) }} {{ sticky_page_footer('Add to broadcast') }}