From c270688fe47d5e4e9c974ea5a85bff77707eb9c0 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 17 Jul 2020 08:07:40 +0100 Subject: [PATCH 1/6] Show pending broadcasts on dashboard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When we have an approval flow, `pending-approval` will be the state a broadcast is in between being a draft and broadcasting. This means it is the earliest stage at which a broadcast can appear on the dashboard, so this commit adds a new section at the top of the dashboard to display these broadcasts (since the dashboard is in a reverse chronological order). Rather than displaying the scheduled time, the extra information shown is the person who drafted the broadcast, since I reckon you’ll be coming to this page because they’ve asked you to approve their broadcast. --- app/main/views/broadcast.py | 7 ++++- app/templates/views/broadcast/dashboard.html | 8 ++++++ .../broadcast/partials/dashboard-table.html | 6 ++++- tests/app/main/views/test_broadcast.py | 26 +++++++++++++++++-- tests/conftest.py | 3 +++ 5 files changed, 46 insertions(+), 4 deletions(-) diff --git a/app/main/views/broadcast.py b/app/main/views/broadcast.py index 0c30ef0df..1737dbb8f 100644 --- a/app/main/views/broadcast.py +++ b/app/main/views/broadcast.py @@ -13,7 +13,7 @@ from app.utils import service_has_permission, user_has_permissions def broadcast_dashboard(service_id): return render_template( 'views/broadcast/dashboard.html', - partials=get_broadcast_dashboard_partials(current_service.id) + partials=get_broadcast_dashboard_partials(current_service.id), ) @@ -27,6 +27,11 @@ def broadcast_dashboard_updates(service_id): def get_broadcast_dashboard_partials(service_id): broadcast_messages = BroadcastMessages(service_id) return dict( + pending_approval_broadcasts=render_template( + 'views/broadcast/partials/dashboard-table.html', + broadcasts=broadcast_messages.with_status('pending-approval'), + empty_message='You do not have any broadcasts waiting for approval', + ), live_broadcasts=render_template( 'views/broadcast/partials/dashboard-table.html', broadcasts=broadcast_messages.with_status('broadcasting'), diff --git a/app/templates/views/broadcast/dashboard.html b/app/templates/views/broadcast/dashboard.html index 3e35af6ec..c44257e62 100644 --- a/app/templates/views/broadcast/dashboard.html +++ b/app/templates/views/broadcast/dashboard.html @@ -10,6 +10,14 @@

Dashboard

+

Waiting for approval

+ + {{ ajax_block( + partials, + url_for('.broadcast_dashboard_updates', service_id=current_service.id), + 'pending_approval_broadcasts' + ) }} +

Live broadcasts

{{ ajax_block( diff --git a/app/templates/views/broadcast/partials/dashboard-table.html b/app/templates/views/broadcast/partials/dashboard-table.html index fd197201e..92cd40b5d 100644 --- a/app/templates/views/broadcast/partials/dashboard-table.html +++ b/app/templates/views/broadcast/partials/dashboard-table.html @@ -21,7 +21,11 @@ {% endcall %} {% call field(align='right') %} - {% if item.status == 'broadcasting' %} + {% if item.status == 'pending-approval' %} +

+ Prepared by {{ item.created_by.name }} +

+ {% elif item.status == 'broadcasting' %}

Live until {{ item.finishes_at|format_datetime_relative }}

diff --git a/tests/app/main/views/test_broadcast.py b/tests/app/main/views/test_broadcast.py index f379717cf..d777b9002 100644 --- a/tests/app/main/views/test_broadcast.py +++ b/tests/app/main/views/test_broadcast.py @@ -62,6 +62,7 @@ def test_empty_broadcast_dashboard( assert [ normalize_spaces(row.text) for row in page.select('tbody tr .table-empty-message') ] == [ + 'You do not have any broadcasts waiting for approval', 'You do not have any live broadcasts at the moment', 'You do not have any previous broadcasts', ] @@ -78,13 +79,29 @@ def test_broadcast_dashboard( '.broadcast_dashboard', service_id=SERVICE_ONE_ID, ) + assert normalize_spaces(page.select('main h2')[0].text) == ( + 'Waiting for approval' + ) assert [ normalize_spaces(row.text) for row in page.select('table')[0].select('tbody tr') ] == [ - 'Example template To England and Scotland Live until tomorrow at 2:20am', + 'Example template To England and Scotland Prepared by Test User', ] + + assert normalize_spaces(page.select('main h2')[1].text) == ( + 'Live broadcasts' + ) assert [ normalize_spaces(row.text) for row in page.select('table')[1].select('tbody tr') + ] == [ + 'Example template To England and Scotland Live until tomorrow at 2:20am', + ] + + assert normalize_spaces(page.select('main h2')[2].text) == ( + 'Previous broadcasts' + ) + assert [ + normalize_spaces(row.text) for row in page.select('table')[2].select('tbody tr') ] == [ 'Example template To England and Scotland Stopped 10 February at 2:20am', 'Example template To England and Scotland Finished yesterday at 8:20pm', @@ -107,8 +124,13 @@ def test_broadcast_dashboard_json( json_response = json.loads(response.get_data(as_text=True)) - assert json_response.keys() == {'live_broadcasts', 'previous_broadcasts'} + assert json_response.keys() == { + 'pending_approval_broadcasts', + 'live_broadcasts', + 'previous_broadcasts', + } + assert 'Prepared by Test User' in json_response['pending_approval_broadcasts'] assert 'Live until tomorrow at 2:20am' in json_response['live_broadcasts'] assert 'Finished yesterday at 8:20pm' in json_response['previous_broadcasts'] diff --git a/tests/conftest.py b/tests/conftest.py index 88a96f8d4..b2ece7b0c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -4270,6 +4270,9 @@ def mock_get_broadcast_messages( partial_json( status='draft', ), + partial_json( + status='pending-approval', + ), partial_json( status='broadcasting', starts_at=( From 73c5aa9bb703b5c9c9f53fac140cd683cf27d7d6 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 17 Jul 2020 08:07:41 +0100 Subject: [PATCH 2/6] Add page for broadcast in pending state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since we’ll be linking to pending broadcasts from the dashboard, the page needs to be ready to display them. Pending broadcasts lack a few bits of information that live or previous broadcasts have (like the start date for example). So this commit hides the code that displays those bits of information. --- .../views/broadcast/view-message.html | 45 +++++++++++-------- 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/app/templates/views/broadcast/view-message.html b/app/templates/views/broadcast/view-message.html index 625190db9..9d534473e 100644 --- a/app/templates/views/broadcast/view-message.html +++ b/app/templates/views/broadcast/view-message.html @@ -16,26 +16,35 @@ back_link=url_for('.broadcast_dashboard', service_id=current_service.id) ) }} -

- Created by {{ broadcast_message.created_by.name }} and approved by - {{ broadcast_message.approved_by.name }}. -

+ {% if broadcast_message.status == 'pending-approval' %} +

+ {{ broadcast_message.created_by.name }} wants to broadcast this + message until {{ broadcast_message.finishes_at|format_datetime_relative }}. +

+ {% else %} +

+ Created by {{ broadcast_message.created_by.name }} and approved by + {{ broadcast_message.approved_by.name }}. +

-

- Started broadcasting - {{ broadcast_message.starts_at|format_datetime_human }}. -

+

+ Started broadcasting + {{ broadcast_message.starts_at|format_datetime_human }}. +

-

- {% if broadcast_message.status == 'broadcasting' %} - Live until {{ broadcast_message.finishes_at|format_datetime_relative }} Stop broadcast early - {% elif broadcast_message.status == 'cancelled' %} - Stopped by {{ broadcast_message.cancelled_by.name }} - {{ broadcast_message.cancelled_at|format_datetime_human }}. - {% else %} - Finished broadcasting {{ broadcast_message.finishes_at|format_datetime_human }}. - {% endif %} -

+

+ {% if broadcast_message.status == 'pending-approval' %} + Will broadcast until {{ broadcast_message.finishes_at|format_datetime_relative }}. + {% elif broadcast_message.status == 'broadcasting' %} + Live until {{ broadcast_message.finishes_at|format_datetime_relative }} Stop broadcast early + {% elif broadcast_message.status == 'cancelled' %} + Stopped by {{ broadcast_message.cancelled_by.name }} + {{ broadcast_message.cancelled_at|format_datetime_human }}. + {% else %} + Finished broadcasting {{ broadcast_message.finishes_at|format_datetime_human }}. + {% endif %} +

+ {% endif %} {% for area in broadcast_message.areas %} {% if loop.first %} From 4b1e3ec504bead2aa797358a07c1e854f3377b3b Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 17 Jul 2020 08:07:42 +0100 Subject: [PATCH 3/6] Refactor broadcast message model MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There was a lot of duplicate code here for updating the status and other properties of a broadcast. This commit abstracts them into reusable methods. They’re named with an underscore to suggest that they shouldn’t be used externally. --- app/models/broadcast_message.py | 54 ++++++++++++++------------------- 1 file changed, 22 insertions(+), 32 deletions(-) diff --git a/app/models/broadcast_message.py b/app/models/broadcast_message.py index 45ab2fb76..189e1d9e1 100644 --- a/app/models/broadcast_message.py +++ b/app/models/broadcast_message.py @@ -108,49 +108,39 @@ class BroadcastMessage(JSONModel): return User.from_id(self.cancelled_by_id) def add_areas(self, *new_areas): - broadcast_message_api_client.update_broadcast_message( - broadcast_message_id=self.id, - service_id=self.service_id, - data={ - 'areas': list(OrderedSet( - self._dict['areas'] + list(new_areas) - )) - }, - ) + self._update(areas=list(OrderedSet( + self._dict['areas'] + list(new_areas) + ))) def remove_area(self, area_to_remove): + self._update(areas=[ + area for area in self._dict['areas'] + if area != area_to_remove + ]) + + def _set_status_to(self, status): + broadcast_message_api_client.update_broadcast_message_status( + status, + broadcast_message_id=self.id, + service_id=self.service_id, + ) + + def _update(self, **kwargs): broadcast_message_api_client.update_broadcast_message( broadcast_message_id=self.id, service_id=self.service_id, - data={ - 'areas': [ - area for area in self._dict['areas'] - if area != area_to_remove - ] - }, + data=kwargs, ) def start_broadcast(self): - broadcast_message_api_client.update_broadcast_message( - broadcast_message_id=self.id, - service_id=self.service_id, - data={ - 'starts_at': datetime.utcnow().isoformat(), - 'finishes_at': (datetime.utcnow() + self.DEFAULT_TTL).isoformat(), - }, - ) - broadcast_message_api_client.update_broadcast_message_status( - 'broadcasting', - broadcast_message_id=self.id, - service_id=self.service_id, + self._update( + starts_at=datetime.utcnow().isoformat(), + finishes_at=(datetime.utcnow() + self.DEFAULT_TTL).isoformat(), ) + self._set_status_to('broadcasting') def cancel_broadcast(self): - broadcast_message_api_client.update_broadcast_message_status( - 'cancelled', - broadcast_message_id=self.id, - service_id=self.service_id, - ) + self._set_status_to('cancelled') class BroadcastMessages(ModelList): From 5b83db97688b09e0f84d3b17f357b51185d09e37 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 17 Jul 2020 08:07:43 +0100 Subject: [PATCH 4/6] =?UTF-8?q?Don=E2=80=99t=20start=20broadcasts=20immedi?= =?UTF-8?q?ately?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We don’t want one person going full yolo and start broadcasting without any oversight. This commit changes the flow so that the button on the ‘preview’ page puts the broadcast into `pending-approval`, rather than directly into `broadcasting`. --- app/main/views/broadcast.py | 2 +- app/models/broadcast_message.py | 5 ++--- app/templates/views/broadcast/preview-message.html | 2 +- tests/app/main/views/test_broadcast.py | 3 +-- 4 files changed, 5 insertions(+), 7 deletions(-) diff --git a/app/main/views/broadcast.py b/app/main/views/broadcast.py index 1737dbb8f..956db3b5b 100644 --- a/app/main/views/broadcast.py +++ b/app/main/views/broadcast.py @@ -142,7 +142,7 @@ def preview_broadcast_message(service_id, broadcast_message_id): service_id=current_service.id, ) if request.method == 'POST': - broadcast_message.start_broadcast() + broadcast_message.request_approval() return redirect(url_for( '.broadcast_dashboard', service_id=current_service.id, diff --git a/app/models/broadcast_message.py b/app/models/broadcast_message.py index 189e1d9e1..27898ee92 100644 --- a/app/models/broadcast_message.py +++ b/app/models/broadcast_message.py @@ -132,12 +132,11 @@ class BroadcastMessage(JSONModel): data=kwargs, ) - def start_broadcast(self): + def request_approval(self): self._update( - starts_at=datetime.utcnow().isoformat(), finishes_at=(datetime.utcnow() + self.DEFAULT_TTL).isoformat(), ) - self._set_status_to('broadcasting') + self._set_status_to('pending-approval') def cancel_broadcast(self): self._set_status_to('cancelled') diff --git a/app/templates/views/broadcast/preview-message.html b/app/templates/views/broadcast/preview-message.html index f37537b1f..bed3cae07 100644 --- a/app/templates/views/broadcast/preview-message.html +++ b/app/templates/views/broadcast/preview-message.html @@ -28,7 +28,7 @@ {{ broadcast_message.template|string }} {% call form_wrapper() %} - {{ sticky_page_footer('Start broadcast') }} + {{ sticky_page_footer('Submit for approval') }} {% endcall %} {% endblock %} diff --git a/tests/app/main/views/test_broadcast.py b/tests/app/main/views/test_broadcast.py index d777b9002..e306a30be 100644 --- a/tests/app/main/views/test_broadcast.py +++ b/tests/app/main/views/test_broadcast.py @@ -313,12 +313,11 @@ def test_start_broadcasting( service_id=SERVICE_ONE_ID, broadcast_message_id=fake_uuid, data={ - 'starts_at': '2020-02-02T02:02:02.222222', 'finishes_at': '2020-02-05T02:02:02.222222', }, ) mock_update_broadcast_message_status.assert_called_once_with( - 'broadcasting', + 'pending-approval', service_id=SERVICE_ONE_ID, broadcast_message_id=fake_uuid, ) From a99b40304b472c79c50469a7c753a164f7cd23dc Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 17 Jul 2020 08:07:44 +0100 Subject: [PATCH 5/6] Add button to approve broadcast MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since new broadcasts will go into `pending-approval`, we now need a way of approving them. This commit adds a button to this page to start (or approve) the broadcast. This button is wrapped in a bordered box, to emphasise that it’s something consequential. --- app/assets/stylesheets/components/banner.scss | 4 + app/main/views/broadcast.py | 26 +++++ app/models/broadcast_message.py | 6 ++ app/navigation.py | 4 + .../views/broadcast/view-message.html | 15 ++- tests/app/main/views/test_broadcast.py | 102 ++++++++++++++++++ 6 files changed, 152 insertions(+), 5 deletions(-) diff --git a/app/assets/stylesheets/components/banner.scss b/app/assets/stylesheets/components/banner.scss index 8203dcfd2..faaee067a 100644 --- a/app/assets/stylesheets/components/banner.scss +++ b/app/assets/stylesheets/components/banner.scss @@ -24,6 +24,10 @@ @include copy-19; } + .page-footer { + margin-bottom: govuk-spacing(1); + } + } %banner-with-tick, diff --git a/app/main/views/broadcast.py b/app/main/views/broadcast.py index 956db3b5b..f45414200 100644 --- a/app/main/views/broadcast.py +++ b/app/main/views/broadcast.py @@ -169,6 +169,32 @@ def view_broadcast_message(service_id, broadcast_message_id): ) +@main.route('/services//broadcast/', methods=['POST']) +@user_has_permissions('send_messages') +@service_has_permission('broadcast') +def approve_broadcast_message(service_id, broadcast_message_id): + + broadcast_message = BroadcastMessage.from_id( + broadcast_message_id, + service_id=current_service.id, + ) + + if broadcast_message.status != 'pending-approval': + return redirect(url_for( + '.view_broadcast_message', + service_id=current_service.id, + broadcast_message_id=broadcast_message.id, + )) + + broadcast_message.approve_broadcast() + + return redirect(url_for( + '.view_broadcast_message', + service_id=current_service.id, + broadcast_message_id=broadcast_message.id, + )) + + @main.route('/services//broadcast//cancel') @user_has_permissions('send_messages') @service_has_permission('broadcast') diff --git a/app/models/broadcast_message.py b/app/models/broadcast_message.py index 27898ee92..bf6772979 100644 --- a/app/models/broadcast_message.py +++ b/app/models/broadcast_message.py @@ -138,6 +138,12 @@ class BroadcastMessage(JSONModel): ) self._set_status_to('pending-approval') + def approve_broadcast(self): + self._update( + starts_at=datetime.utcnow().isoformat(), + ) + self._set_status_to('broadcasting') + def cancel_broadcast(self): self._set_status_to('cancelled') diff --git a/app/navigation.py b/app/navigation.py index 4442829f6..7d1af6f4b 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -361,6 +361,7 @@ class HeaderNavigation(Navigation): 'remove_broadcast_area', 'preview_broadcast_message', 'view_broadcast_message', + 'approve_broadcast_message', 'cancel_broadcast_message', } @@ -419,6 +420,7 @@ class MainNavigation(Navigation): 'remove_broadcast_area', 'preview_broadcast_message', 'view_broadcast_message', + 'approve_broadcast_message', 'cancel_broadcast_message', }, 'uploads': { @@ -1012,6 +1014,7 @@ class CaseworkNavigation(Navigation): 'remove_broadcast_area', 'preview_broadcast_message', 'view_broadcast_message', + 'approve_broadcast_message', 'cancel_broadcast_message', } @@ -1333,5 +1336,6 @@ class OrgNavigation(Navigation): 'remove_broadcast_area', 'preview_broadcast_message', 'view_broadcast_message', + 'approve_broadcast_message', 'cancel_broadcast_message', } diff --git a/app/templates/views/broadcast/view-message.html b/app/templates/views/broadcast/view-message.html index 9d534473e..d38584460 100644 --- a/app/templates/views/broadcast/view-message.html +++ b/app/templates/views/broadcast/view-message.html @@ -1,7 +1,7 @@ {% 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 %} +{% from "components/page-footer.html" import page_footer %} {% extends "withnav_template.html" %} @@ -17,10 +17,15 @@ ) }} {% if broadcast_message.status == 'pending-approval' %} -

- {{ broadcast_message.created_by.name }} wants to broadcast this - message until {{ broadcast_message.finishes_at|format_datetime_relative }}. -

+ {% call form_wrapper(class="banner govuk-!-margin-bottom-6") %} +

+ {{ broadcast_message.created_by.name }} wants to broadcast this + message until {{ broadcast_message.finishes_at|format_datetime_relative }}. +

+ {{ page_footer( + "Start broadcasting now" + ) }} + {% endcall %} {% else %}

Created by {{ broadcast_message.created_by.name }} and approved by diff --git a/tests/app/main/views/test_broadcast.py b/tests/app/main/views/test_broadcast.py index e306a30be..ec8826be9 100644 --- a/tests/app/main/views/test_broadcast.py +++ b/tests/app/main/views/test_broadcast.py @@ -400,6 +400,108 @@ def test_view_broadcast_message_page( ] == expected_paragraphs +@freeze_time('2020-02-22T22:22:22.000000') +def test_view_pending_broadcast( + mocker, + client_request, + service_one, + mock_get_broadcast_template, + fake_uuid, +): + mocker.patch( + 'app.broadcast_message_api_client.get_broadcast_message', + return_value=broadcast_message_json( + id_=fake_uuid, + service_id=SERVICE_ONE_ID, + template_id=fake_uuid, + created_by_id=fake_uuid, + finishes_at='2020-02-23T23:23:23.000000', + status='pending-approval', + ), + ) + service_one['permissions'] += ['broadcast'] + + page = client_request.get( + '.view_broadcast_message', + service_id=SERVICE_ONE_ID, + broadcast_message_id=fake_uuid, + ) + + assert ( + normalize_spaces(page.select_one('.banner').text) + ) == ( + 'Test User wants to broadcast this message until tomorrow at 11:23pm. ' + 'Start broadcasting now' + ) + + form = page.select_one('form.banner') + assert form['method'] == 'post' + assert 'action' not in form + assert form.select_one('button[type=submit]') + + +@pytest.mark.parametrize('initial_status, expected_approval', ( + ('draft', False,), + ('pending-approval', True), + ('rejected', False), + ('broadcasting', False), + ('cancelled', False), +)) +@freeze_time('2020-02-22T22:22:22.000000') +def test_approve_broadcast( + mocker, + client_request, + service_one, + mock_get_broadcast_template, + fake_uuid, + mock_update_broadcast_message, + mock_update_broadcast_message_status, + initial_status, + expected_approval, +): + mocker.patch( + 'app.broadcast_message_api_client.get_broadcast_message', + return_value=broadcast_message_json( + id_=fake_uuid, + service_id=SERVICE_ONE_ID, + template_id=fake_uuid, + created_by_id=fake_uuid, + finishes_at='2020-02-23T23:23:23.000000', + status=initial_status, + ), + ) + service_one['permissions'] += ['broadcast'] + + client_request.post( + '.view_broadcast_message', + service_id=SERVICE_ONE_ID, + broadcast_message_id=fake_uuid, + _expected_redirect=url_for( + '.view_broadcast_message', + service_id=SERVICE_ONE_ID, + broadcast_message_id=fake_uuid, + _external=True, + ) + ) + + if expected_approval: + mock_update_broadcast_message.assert_called_once_with( + service_id=SERVICE_ONE_ID, + broadcast_message_id=fake_uuid, + data={ + 'starts_at': '2020-02-22T22:22:22', + }, + ) + mock_update_broadcast_message_status.assert_called_once_with( + 'broadcasting', + service_id=SERVICE_ONE_ID, + broadcast_message_id=fake_uuid, + ) + else: + assert mock_update_broadcast_message.called is False + assert mock_update_broadcast_message_status.called is False + + def test_no_view_page_for_draft( client_request, service_one, From 03b4aabf5f2a88beeb6446d0c84754dfefb42002 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 17 Jul 2020 08:07:44 +0100 Subject: [PATCH 6/6] Add a link to reject a broadcast MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If a broadcast definitely shouldn’t go out (for example because it has a spelling mistake or is going to the wrong areas) then we should have a way of removing it. Once it’s removed no-one else can approve it, and it isn’t cluttering up the dashboard. This is a link (because it’s a secondary action) and red (because it’s destructive, in that it’s throwing away someone’s work). --- .../stylesheets/components/page-footer.scss | 1 + app/main/views/broadcast.py | 25 +++++ app/models/broadcast_message.py | 3 + app/navigation.py | 4 + .../views/broadcast/view-message.html | 4 +- tests/app/main/views/test_broadcast.py | 99 ++++++++++++++++++- 6 files changed, 134 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/components/page-footer.scss b/app/assets/stylesheets/components/page-footer.scss index 19209a24c..61b04299b 100644 --- a/app/assets/stylesheets/components/page-footer.scss +++ b/app/assets/stylesheets/components/page-footer.scss @@ -7,6 +7,7 @@ line-height: 40px; padding: 1px 0 0 15px; + font-weight: normal; } diff --git a/app/main/views/broadcast.py b/app/main/views/broadcast.py index f45414200..7dda048e8 100644 --- a/app/main/views/broadcast.py +++ b/app/main/views/broadcast.py @@ -195,6 +195,31 @@ def approve_broadcast_message(service_id, broadcast_message_id): )) +@main.route('/services//broadcast//reject') +@user_has_permissions('send_messages') +@service_has_permission('broadcast') +def reject_broadcast_message(service_id, broadcast_message_id): + + broadcast_message = BroadcastMessage.from_id( + broadcast_message_id, + service_id=current_service.id, + ) + + if broadcast_message.status != 'pending-approval': + return redirect(url_for( + '.view_broadcast_message', + service_id=current_service.id, + broadcast_message_id=broadcast_message.id, + )) + + broadcast_message.reject_broadcast() + + return redirect(url_for( + '.broadcast_dashboard', + service_id=current_service.id, + )) + + @main.route('/services//broadcast//cancel') @user_has_permissions('send_messages') @service_has_permission('broadcast') diff --git a/app/models/broadcast_message.py b/app/models/broadcast_message.py index bf6772979..8e7deb224 100644 --- a/app/models/broadcast_message.py +++ b/app/models/broadcast_message.py @@ -144,6 +144,9 @@ class BroadcastMessage(JSONModel): ) self._set_status_to('broadcasting') + def reject_broadcast(self): + self._set_status_to('rejected') + def cancel_broadcast(self): self._set_status_to('cancelled') diff --git a/app/navigation.py b/app/navigation.py index 7d1af6f4b..34d1639ca 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -362,6 +362,7 @@ class HeaderNavigation(Navigation): 'preview_broadcast_message', 'view_broadcast_message', 'approve_broadcast_message', + 'reject_broadcast_message', 'cancel_broadcast_message', } @@ -421,6 +422,7 @@ class MainNavigation(Navigation): 'preview_broadcast_message', 'view_broadcast_message', 'approve_broadcast_message', + 'reject_broadcast_message', 'cancel_broadcast_message', }, 'uploads': { @@ -1015,6 +1017,7 @@ class CaseworkNavigation(Navigation): 'preview_broadcast_message', 'view_broadcast_message', 'approve_broadcast_message', + 'reject_broadcast_message', 'cancel_broadcast_message', } @@ -1337,5 +1340,6 @@ class OrgNavigation(Navigation): 'preview_broadcast_message', 'view_broadcast_message', 'approve_broadcast_message', + 'reject_broadcast_message', 'cancel_broadcast_message', } diff --git a/app/templates/views/broadcast/view-message.html b/app/templates/views/broadcast/view-message.html index d38584460..9258592ef 100644 --- a/app/templates/views/broadcast/view-message.html +++ b/app/templates/views/broadcast/view-message.html @@ -23,7 +23,9 @@ message until {{ broadcast_message.finishes_at|format_datetime_relative }}.

{{ page_footer( - "Start broadcasting now" + "Start broadcasting now", + delete_link=url_for('main.reject_broadcast_message', service_id=current_service.id, broadcast_message_id=broadcast_message.id), + delete_link_text='Reject this broadcast' ) }} {% endcall %} {% else %} diff --git a/tests/app/main/views/test_broadcast.py b/tests/app/main/views/test_broadcast.py index ec8826be9..edad43dd8 100644 --- a/tests/app/main/views/test_broadcast.py +++ b/tests/app/main/views/test_broadcast.py @@ -431,7 +431,7 @@ def test_view_pending_broadcast( normalize_spaces(page.select_one('.banner').text) ) == ( 'Test User wants to broadcast this message until tomorrow at 11:23pm. ' - 'Start broadcasting now' + 'Start broadcasting now Reject this broadcast' ) form = page.select_one('form.banner') @@ -439,6 +439,14 @@ def test_view_pending_broadcast( assert 'action' not in form assert form.select_one('button[type=submit]') + link = form.select_one('a.govuk-link.govuk-link--destructive') + assert link.text == 'Reject this broadcast' + assert link['href'] == url_for( + '.reject_broadcast_message', + service_id=SERVICE_ONE_ID, + broadcast_message_id=fake_uuid, + ) + @pytest.mark.parametrize('initial_status, expected_approval', ( ('draft', False,), @@ -502,6 +510,95 @@ def test_approve_broadcast( assert mock_update_broadcast_message_status.called is False +@freeze_time('2020-02-22T22:22:22.000000') +def test_reject_broadcast( + mocker, + client_request, + service_one, + mock_get_broadcast_template, + fake_uuid, + mock_update_broadcast_message, + mock_update_broadcast_message_status, +): + mocker.patch( + 'app.broadcast_message_api_client.get_broadcast_message', + return_value=broadcast_message_json( + id_=fake_uuid, + service_id=SERVICE_ONE_ID, + template_id=fake_uuid, + created_by_id=fake_uuid, + finishes_at='2020-02-23T23:23:23.000000', + status='pending-approval', + ), + ) + service_one['permissions'] += ['broadcast'] + + client_request.get( + '.reject_broadcast_message', + service_id=SERVICE_ONE_ID, + broadcast_message_id=fake_uuid, + _expected_redirect=url_for( + '.broadcast_dashboard', + service_id=SERVICE_ONE_ID, + _external=True, + ) + ) + + assert mock_update_broadcast_message.called is False + + mock_update_broadcast_message_status.assert_called_once_with( + 'rejected', + service_id=SERVICE_ONE_ID, + broadcast_message_id=fake_uuid, + ) + + +@pytest.mark.parametrize('initial_status', ( + 'draft', + 'rejected', + 'broadcasting', + 'cancelled', +)) +@freeze_time('2020-02-22T22:22:22.000000') +def test_cant_reject_broadcast_in_wrong_state( + mocker, + client_request, + service_one, + mock_get_broadcast_template, + fake_uuid, + mock_update_broadcast_message, + mock_update_broadcast_message_status, + initial_status, +): + mocker.patch( + 'app.broadcast_message_api_client.get_broadcast_message', + return_value=broadcast_message_json( + id_=fake_uuid, + service_id=SERVICE_ONE_ID, + template_id=fake_uuid, + created_by_id=fake_uuid, + finishes_at='2020-02-23T23:23:23.000000', + status=initial_status, + ), + ) + service_one['permissions'] += ['broadcast'] + + client_request.get( + '.reject_broadcast_message', + service_id=SERVICE_ONE_ID, + broadcast_message_id=fake_uuid, + _expected_redirect=url_for( + '.view_broadcast_message', + service_id=SERVICE_ONE_ID, + broadcast_message_id=fake_uuid, + _external=True, + ) + ) + + assert mock_update_broadcast_message.called is False + assert mock_update_broadcast_message_status.called is False + + def test_no_view_page_for_draft( client_request, service_one,