From c7c5793da4f4572e290179f14eb53a6c807476d6 Mon Sep 17 00:00:00 2001 From: Ben Thorner Date: Tue, 5 Apr 2022 11:35:33 +0100 Subject: [PATCH] Shorten name of broadcast utility function This is doing more than just validating and updating an is about to do even more. Saying "update" is broad enough to cover the others. --- app/broadcast_message/rest.py | 6 +-- app/broadcast_message/utils.py | 2 +- app/v2/broadcast/post_broadcast.py | 6 +-- tests/app/broadcast_message/test_utils.py | 48 +++++++++---------- tests/app/v2/broadcast/test_post_broadcast.py | 6 ++- 5 files changed, 32 insertions(+), 36 deletions(-) diff --git a/app/broadcast_message/rest.py b/app/broadcast_message/rest.py index 1388c2481..aca1c408f 100644 --- a/app/broadcast_message/rest.py +++ b/app/broadcast_message/rest.py @@ -2,14 +2,12 @@ import iso8601 from flask import Blueprint, jsonify, request from notifications_utils.template import BroadcastMessageTemplate +from app.broadcast_message import utils as broadcast_utils from app.broadcast_message.broadcast_message_schema import ( create_broadcast_message_schema, update_broadcast_message_schema, update_broadcast_message_status_schema, ) -from app.broadcast_message.utils import ( - validate_and_update_broadcast_message_status, -) from app.dao.broadcast_message_dao import ( dao_get_broadcast_message_by_id_and_service_id, dao_get_broadcast_messages_for_service, @@ -162,6 +160,6 @@ def update_broadcast_message_status(service_id, broadcast_message_id): status_code=400 ) - validate_and_update_broadcast_message_status(broadcast_message, new_status, updating_user) + broadcast_utils.update_broadcast_message_status(broadcast_message, new_status, updating_user) return jsonify(broadcast_message.serialize()), 200 diff --git a/app/broadcast_message/utils.py b/app/broadcast_message/utils.py index 91807228b..3694f8e0b 100644 --- a/app/broadcast_message/utils.py +++ b/app/broadcast_message/utils.py @@ -13,7 +13,7 @@ from app.models import ( ) -def validate_and_update_broadcast_message_status(broadcast_message, new_status, updating_user=None, api_key_id=None): +def update_broadcast_message_status(broadcast_message, new_status, updating_user=None, api_key_id=None): _validate_broadcast_update(broadcast_message, new_status, updating_user) if new_status == BroadcastStatusType.BROADCASTING: diff --git a/app/v2/broadcast/post_broadcast.py b/app/v2/broadcast/post_broadcast.py index c76c4dfde..c3062efe9 100644 --- a/app/v2/broadcast/post_broadcast.py +++ b/app/v2/broadcast/post_broadcast.py @@ -6,10 +6,8 @@ from notifications_utils.template import BroadcastMessageTemplate from sqlalchemy.orm.exc import MultipleResultsFound from app import api_user, authenticated_service, redis_store +from app.broadcast_message import utils as broadcast_utils from app.broadcast_message.translators import cap_xml_to_dict -from app.broadcast_message.utils import ( - validate_and_update_broadcast_message_status, -) from app.dao.broadcast_message_dao import ( dao_get_broadcast_message_by_references_and_service_id, ) @@ -121,7 +119,7 @@ def _cancel_or_reject_broadcast(references_to_original_broadcast, service_id): new_status = BroadcastStatusType.REJECTED else: new_status = BroadcastStatusType.CANCELLED - validate_and_update_broadcast_message_status( + broadcast_utils.update_broadcast_message_status( broadcast_message, new_status, api_key_id=api_user.id diff --git a/tests/app/broadcast_message/test_utils.py b/tests/app/broadcast_message/test_utils.py index 4e9e4ee25..d35654cb8 100644 --- a/tests/app/broadcast_message/test_utils.py +++ b/tests/app/broadcast_message/test_utils.py @@ -1,8 +1,6 @@ import pytest -from app.broadcast_message.utils import ( - validate_and_update_broadcast_message_status, -) +from app.broadcast_message.utils import update_broadcast_message_status from app.errors import InvalidRequest from app.models import ( BROADCAST_TYPE, @@ -17,7 +15,7 @@ from tests.app.db import ( ) -def test_validate_and_update_broadcast_message_status_stores_approved_by_and_approved_at_and_queues_task( +def test_update_broadcast_message_status_stores_approved_by_and_approved_at_and_queues_task( sample_broadcast_service, mocker ): @@ -34,7 +32,7 @@ def test_validate_and_update_broadcast_message_status_stores_approved_by_and_app sample_broadcast_service.users.append(approver) mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async') - validate_and_update_broadcast_message_status( + update_broadcast_message_status( broadcast_message, BroadcastStatusType.BROADCASTING, approver ) @@ -54,7 +52,7 @@ def test_validate_and_update_broadcast_message_status_stores_approved_by_and_app assert alert_event.transmitted_content == {"body": "emergency broadcast"} -def test_validate_and_update_broadcast_message_status_for_cancelling_broadcast_from_admin_interface( +def test_update_broadcast_message_status_for_cancelling_broadcast_from_admin_interface( sample_broadcast_service, mocker, ): @@ -71,7 +69,7 @@ def test_validate_and_update_broadcast_message_status_for_cancelling_broadcast_f mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async') - validate_and_update_broadcast_message_status( + update_broadcast_message_status( broadcast_message, BroadcastStatusType.CANCELLED, updating_user=canceller, api_key_id=None ) @@ -89,7 +87,7 @@ def test_validate_and_update_broadcast_message_status_for_cancelling_broadcast_f assert alert_event.message_type == BroadcastEventMessageType.CANCEL -def test_validate_and_update_broadcast_message_status_for_cancelling_broadcast_from_API_call( +def test_update_broadcast_message_status_for_cancelling_broadcast_from_API_call( sample_broadcast_service, mocker, ): @@ -105,7 +103,7 @@ def test_validate_and_update_broadcast_message_status_for_cancelling_broadcast_f ) mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async') - validate_and_update_broadcast_message_status( + update_broadcast_message_status( broadcast_message, BroadcastStatusType.CANCELLED, updating_user=None, api_key_id=api_key.id ) @@ -123,7 +121,7 @@ def test_validate_and_update_broadcast_message_status_for_cancelling_broadcast_f assert alert_event.message_type == BroadcastEventMessageType.CANCEL -def test_validate_and_update_broadcast_message_status_for_rejecting_broadcast_via_admin_interface( +def test_update_broadcast_message_status_for_rejecting_broadcast_via_admin_interface( sample_broadcast_service, mocker ): @@ -138,7 +136,7 @@ def test_validate_and_update_broadcast_message_status_for_rejecting_broadcast_vi ) mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async') - validate_and_update_broadcast_message_status( + update_broadcast_message_status( broadcast_message, BroadcastStatusType.REJECTED, updating_user=sample_broadcast_service.created_by ) @@ -151,7 +149,7 @@ def test_validate_and_update_broadcast_message_status_for_rejecting_broadcast_vi assert len(broadcast_message.events) == 0 -def test_validate_and_update_broadcast_message_status_for_rejecting_broadcast_from_API_call( +def test_update_broadcast_message_status_for_rejecting_broadcast_from_API_call( sample_broadcast_service, mocker ): @@ -167,7 +165,7 @@ def test_validate_and_update_broadcast_message_status_for_rejecting_broadcast_fr ) mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async') - validate_and_update_broadcast_message_status( + update_broadcast_message_status( broadcast_message, BroadcastStatusType.REJECTED, api_key_id=api_key.id ) @@ -209,7 +207,7 @@ def test_validate_and_update_broadcast_message_status_for_rejecting_broadcast_fr (BroadcastStatusType.CANCELLED, BroadcastStatusType.BROADCASTING), (BroadcastStatusType.CANCELLED, BroadcastStatusType.COMPLETED), ]) -def test_validate_and_update_broadcast_message_status_restricts_status_transitions_to_explicit_list( +def test_update_broadcast_message_status_restricts_status_transitions_to_explicit_list( sample_broadcast_service, mocker, current_status, @@ -222,14 +220,14 @@ def test_validate_and_update_broadcast_message_status_restricts_status_transitio mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async') with pytest.raises(expected_exception=InvalidRequest) as e: - validate_and_update_broadcast_message_status(broadcast_message, new_status, approver) + update_broadcast_message_status(broadcast_message, new_status, approver) assert mock_task.called is False assert f'from {current_status} to {new_status}' in str(e.value) @pytest.mark.parametrize('is_platform_admin', [True, False]) -def test_validate_and_update_broadcast_message_status_rejects_approval_from_creator( +def test_update_broadcast_message_status_rejects_approval_from_creator( sample_broadcast_service, mocker, is_platform_admin @@ -241,7 +239,7 @@ def test_validate_and_update_broadcast_message_status_rejects_approval_from_crea mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async') with pytest.raises(expected_exception=InvalidRequest) as e: - validate_and_update_broadcast_message_status( + update_broadcast_message_status( broadcast_message, BroadcastStatusType.BROADCASTING, creator_and_approver ) @@ -249,7 +247,7 @@ def test_validate_and_update_broadcast_message_status_rejects_approval_from_crea assert 'cannot approve their own broadcast' in str(e.value) -def test_validate_and_update_broadcast_message_status_rejects_approval_of_broadcast_with_no_areas( +def test_update_broadcast_message_status_rejects_approval_of_broadcast_with_no_areas( admin_request, sample_broadcast_service, mocker @@ -261,13 +259,13 @@ def test_validate_and_update_broadcast_message_status_rejects_approval_of_broadc mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async') with pytest.raises(expected_exception=InvalidRequest) as e: - validate_and_update_broadcast_message_status(broadcast, BroadcastStatusType.BROADCASTING, approver) + update_broadcast_message_status(broadcast, BroadcastStatusType.BROADCASTING, approver) assert mock_task.called is False assert f'broadcast_message {broadcast.id} has no selected areas and so cannot be broadcasted.' in str(e.value) -def test_validate_and_update_broadcast_message_status_allows_trial_mode_services_to_approve_own_message( +def test_update_broadcast_message_status_allows_trial_mode_services_to_approve_own_message( notify_db, sample_broadcast_service, mocker @@ -282,7 +280,7 @@ def test_validate_and_update_broadcast_message_status_allows_trial_mode_services creator_and_approver = sample_broadcast_service.created_by mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async') - validate_and_update_broadcast_message_status( + update_broadcast_message_status( broadcast_message, BroadcastStatusType.BROADCASTING, creator_and_approver ) @@ -298,7 +296,7 @@ def test_validate_and_update_broadcast_message_status_allows_trial_mode_services (True, False), (False, True), ]) -def test_validate_and_update_broadcast_message_status_when_broadcast_message_is_stubbed_or_service_not_live( +def test_update_broadcast_message_status_when_broadcast_message_is_stubbed_or_service_not_live( admin_request, sample_broadcast_service, mocker, @@ -319,7 +317,7 @@ def test_validate_and_update_broadcast_message_status_when_broadcast_message_is_ sample_broadcast_service.restricted = service_restricted_before_approval - validate_and_update_broadcast_message_status( + update_broadcast_message_status( broadcast_message, BroadcastStatusType.BROADCASTING, approver ) assert broadcast_message.status == BroadcastStatusType.BROADCASTING @@ -331,7 +329,7 @@ def test_validate_and_update_broadcast_message_status_when_broadcast_message_is_ assert len(mock_task.mock_calls) == 0 -def test_validate_and_update_broadcast_message_status_creates_event_with_correct_content_if_broadcast_has_no_template( +def test_update_broadcast_message_status_creates_event_with_correct_content_if_broadcast_has_no_template( admin_request, sample_broadcast_service, mocker @@ -350,7 +348,7 @@ def test_validate_and_update_broadcast_message_status_creates_event_with_correct sample_broadcast_service.users.append(approver) mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async') - validate_and_update_broadcast_message_status( + update_broadcast_message_status( broadcast_message, BroadcastStatusType.BROADCASTING, approver ) diff --git a/tests/app/v2/broadcast/test_post_broadcast.py b/tests/app/v2/broadcast/test_post_broadcast.py index 9b6c56d5a..6fc902a37 100644 --- a/tests/app/v2/broadcast/test_post_broadcast.py +++ b/tests/app/v2/broadcast/test_post_broadcast.py @@ -124,7 +124,7 @@ def test_valid_post_cap_xml_broadcast_returns_201( [True, "cancelled"], [False, "rejected"] ]) -def test_valid_cancel_broadcast_request_calls_validate_and_update_broadcast_message_status_and_returns_201( +def test_valid_cancel_broadcast_request_calls_update_broadcast_message_status_and_returns_201( client, sample_broadcast_service, mocker, @@ -153,7 +153,9 @@ def test_valid_cancel_broadcast_request_calls_validate_and_update_broadcast_mess if is_approved: broadcast_message.status = 'broadcasting' - mock_update = mocker.patch('app.v2.broadcast.post_broadcast.validate_and_update_broadcast_message_status') + mock_update = mocker.patch( + 'app.v2.broadcast.post_broadcast.broadcast_utils.update_broadcast_message_status' + ) # cancel broadcast response_for_cancel = client.post(