From c61bd9976f4078279aedec0476360d27aa826bfd Mon Sep 17 00:00:00 2001 From: David McDonald Date: Fri, 22 Jan 2021 16:56:05 +0000 Subject: [PATCH] Remove ability for platform admins to approve own broadcast This has been added in for speed of development but now we are getting close to integrating with production systems, we will be turning off these helpful hacks to reduce the risk of someone sending a real broadcast to citizens. Note, platforms are still able to approve broadcasts when their service is in training mode. --- app/broadcast_message/rest.py | 8 ++--- tests/app/broadcast_message/test_rest.py | 41 +++--------------------- 2 files changed, 7 insertions(+), 42 deletions(-) diff --git a/app/broadcast_message/rest.py b/app/broadcast_message/rest.py index abc47653d..111790be5 100644 --- a/app/broadcast_message/rest.py +++ b/app/broadcast_message/rest.py @@ -49,12 +49,8 @@ def _update_broadcast_message(broadcast_message, new_status, updating_user): ) if new_status == BroadcastStatusType.BROADCASTING: - # TODO: Remove this platform admin shortcut when the feature goes live - if updating_user == broadcast_message.created_by and not ( - # platform admins and trial mode services can approve their own broadcasts - updating_user.platform_admin or - broadcast_message.service.restricted - ): + # trial mode services can approve their own broadcasts + if updating_user == broadcast_message.created_by and not broadcast_message.service.restricted: raise InvalidRequest( f'User {updating_user.id} cannot approve their own broadcast_message {broadcast_message.id}', status_code=400 diff --git a/tests/app/broadcast_message/test_rest.py b/tests/app/broadcast_message/test_rest.py index e4032b882..2ec082267 100644 --- a/tests/app/broadcast_message/test_rest.py +++ b/tests/app/broadcast_message/test_rest.py @@ -579,14 +579,17 @@ def test_update_broadcast_message_status_creates_event_with_correct_content_if_b assert alert_event.transmitted_content == {"body": "tailor made emergency broadcast content"} - +@pytest.mark.parametrize('is_platform_admin', [True, False]) def test_update_broadcast_message_status_rejects_approval_from_creator( admin_request, sample_broadcast_service, - mocker + mocker, + is_platform_admin ): t = create_template(sample_broadcast_service, BROADCAST_TYPE) bm = create_broadcast_message(t, status=BroadcastStatusType.PENDING_APPROVAL) + user = sample_broadcast_service.created_by + user.platform_admin = is_platform_admin mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async') response = admin_request.post( @@ -626,40 +629,6 @@ def test_update_broadcast_message_status_rejects_approval_of_broadcast_with_no_a ] == f'broadcast_message {broadcast.id} has no selected areas and so cannot be broadcasted.' -def test_update_broadcast_message_status_allows_platform_admin_to_approve_own_message( - notify_db, - admin_request, - sample_broadcast_service, - mocker -): - user = sample_broadcast_service.created_by - user.platform_admin = True - t = create_template(sample_broadcast_service, BROADCAST_TYPE) - bm = create_broadcast_message( - t, - status=BroadcastStatusType.PENDING_APPROVAL, - areas={"areas": ["london"], "simple_polygons": [[[51.30, 0.7], [51.28, 0.8], [51.25, -0.7]]]} - ) - mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async') - - response = admin_request.post( - 'broadcast_message.update_broadcast_message_status', - _data={'status': BroadcastStatusType.BROADCASTING, 'created_by': str(user.id)}, - service_id=t.service_id, - broadcast_message_id=bm.id, - _expected_status=200 - ) - - assert response['status'] == BroadcastStatusType.BROADCASTING - assert response['approved_at'] is not None - assert response['created_by_id'] == str(user.id) - assert response['approved_by_id'] == str(user.id) - mock_task.assert_called_once_with( - kwargs={'broadcast_event_id': str(bm.events[0].id)}, - queue='broadcast-tasks' - ) - - def test_update_broadcast_message_status_allows_trial_mode_services_to_approve_own_message( notify_db, admin_request,