From fce6a2d8dccb32f891942070735d2f7a653b3ab3 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Tue, 6 Apr 2021 15:55:56 +0100 Subject: [PATCH] Allow platform admins to cancel broadcasts. Also update error message for when someone does not have permissions. The message referenced approving broadcasts specifically, whereas people would also see it if they try to cancel or reject broadcast without permission. Why we allow platform admins to cancel broadcasts: we do this so they can react quickly if a broadcast was approved by accident. --- app/broadcast_message/rest.py | 10 ++++++---- tests/app/broadcast_message/test_rest.py | 15 +++++++++++---- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/app/broadcast_message/rest.py b/app/broadcast_message/rest.py index 9d8dffaa5..f1e6c619d 100644 --- a/app/broadcast_message/rest.py +++ b/app/broadcast_message/rest.py @@ -43,10 +43,12 @@ def _parse_nullable_datetime(dt): def _update_broadcast_message(broadcast_message, new_status, updating_user): if updating_user not in broadcast_message.service.users: - raise InvalidRequest( - f'User {updating_user.id} cannot approve broadcast_message {broadcast_message.id} from other service', - status_code=400 - ) + # we allow platform admins to cancel broadcasts + if not (new_status == BroadcastStatusType.CANCELLED and updating_user.platform_admin): + raise InvalidRequest( + f'User {updating_user.id} cannot update broadcast_message {broadcast_message.id} from other service', + status_code=400 + ) if new_status not in BroadcastStatusType.ALLOWED_STATUS_TRANSITIONS[broadcast_message.status]: raise InvalidRequest( diff --git a/tests/app/broadcast_message/test_rest.py b/tests/app/broadcast_message/test_rest.py index b067b8bc4..5de52f9e9 100644 --- a/tests/app/broadcast_message/test_rest.py +++ b/tests/app/broadcast_message/test_rest.py @@ -489,13 +489,20 @@ def test_update_broadcast_message_status_doesnt_let_you_update_other_things(admi }] -def test_update_broadcast_message_status_stores_cancelled_by_and_cancelled_at( - admin_request, sample_broadcast_service, mocker +@pytest.mark.parametrize('user_is_platform_admin', [True, False]) +def test_update_broadcast_message_allows_service_user_and_platform_admin_to_cancel( + admin_request, sample_broadcast_service, mocker, user_is_platform_admin ): + """ + Only platform admins and users belonging to that service should be able to cancel broadcasts. + """ t = create_template(sample_broadcast_service, BROADCAST_TYPE, content='emergency broadcast') bm = create_broadcast_message(t, status=BroadcastStatusType.BROADCASTING) canceller = create_user(email='canceller@gov.uk') - sample_broadcast_service.users.append(canceller) + if user_is_platform_admin: + canceller.platform_admin = True + else: + sample_broadcast_service.users.append(canceller) mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async') response = admin_request.post( @@ -736,7 +743,7 @@ def test_update_broadcast_message_status_rejects_approval_from_user_not_on_that_ ) assert mock_task.called is False - assert 'cannot approve broadcast' in response['message'] + assert 'cannot update broadcast' in response['message'] @pytest.mark.parametrize('current_status, new_status', [