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.
This commit is contained in:
Ben Thorner
2022-04-05 11:35:33 +01:00
parent 3b705a780a
commit c7c5793da4
5 changed files with 32 additions and 36 deletions

View File

@@ -2,14 +2,12 @@ import iso8601
from flask import Blueprint, jsonify, request from flask import Blueprint, jsonify, request
from notifications_utils.template import BroadcastMessageTemplate from notifications_utils.template import BroadcastMessageTemplate
from app.broadcast_message import utils as broadcast_utils
from app.broadcast_message.broadcast_message_schema import ( from app.broadcast_message.broadcast_message_schema import (
create_broadcast_message_schema, create_broadcast_message_schema,
update_broadcast_message_schema, update_broadcast_message_schema,
update_broadcast_message_status_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 ( from app.dao.broadcast_message_dao import (
dao_get_broadcast_message_by_id_and_service_id, dao_get_broadcast_message_by_id_and_service_id,
dao_get_broadcast_messages_for_service, dao_get_broadcast_messages_for_service,
@@ -162,6 +160,6 @@ def update_broadcast_message_status(service_id, broadcast_message_id):
status_code=400 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 return jsonify(broadcast_message.serialize()), 200

View File

@@ -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) _validate_broadcast_update(broadcast_message, new_status, updating_user)
if new_status == BroadcastStatusType.BROADCASTING: if new_status == BroadcastStatusType.BROADCASTING:

View File

@@ -6,10 +6,8 @@ from notifications_utils.template import BroadcastMessageTemplate
from sqlalchemy.orm.exc import MultipleResultsFound from sqlalchemy.orm.exc import MultipleResultsFound
from app import api_user, authenticated_service, redis_store 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.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 ( from app.dao.broadcast_message_dao import (
dao_get_broadcast_message_by_references_and_service_id, 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 new_status = BroadcastStatusType.REJECTED
else: else:
new_status = BroadcastStatusType.CANCELLED new_status = BroadcastStatusType.CANCELLED
validate_and_update_broadcast_message_status( broadcast_utils.update_broadcast_message_status(
broadcast_message, broadcast_message,
new_status, new_status,
api_key_id=api_user.id api_key_id=api_user.id

View File

@@ -1,8 +1,6 @@
import pytest import pytest
from app.broadcast_message.utils import ( from app.broadcast_message.utils import update_broadcast_message_status
validate_and_update_broadcast_message_status,
)
from app.errors import InvalidRequest from app.errors import InvalidRequest
from app.models import ( from app.models import (
BROADCAST_TYPE, 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, sample_broadcast_service,
mocker mocker
): ):
@@ -34,7 +32,7 @@ def test_validate_and_update_broadcast_message_status_stores_approved_by_and_app
sample_broadcast_service.users.append(approver) sample_broadcast_service.users.append(approver)
mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async') 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 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"} 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, sample_broadcast_service,
mocker, 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') 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 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 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, sample_broadcast_service,
mocker, 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') 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 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 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, sample_broadcast_service,
mocker 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') 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 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 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, sample_broadcast_service,
mocker 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') 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 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.BROADCASTING),
(BroadcastStatusType.CANCELLED, BroadcastStatusType.COMPLETED), (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, sample_broadcast_service,
mocker, mocker,
current_status, 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') mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async')
with pytest.raises(expected_exception=InvalidRequest) as e: 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 mock_task.called is False
assert f'from {current_status} to {new_status}' in str(e.value) assert f'from {current_status} to {new_status}' in str(e.value)
@pytest.mark.parametrize('is_platform_admin', [True, False]) @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, sample_broadcast_service,
mocker, mocker,
is_platform_admin 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') mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async')
with pytest.raises(expected_exception=InvalidRequest) as e: 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 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) 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, admin_request,
sample_broadcast_service, sample_broadcast_service,
mocker 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') mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async')
with pytest.raises(expected_exception=InvalidRequest) as e: 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 mock_task.called is False
assert f'broadcast_message {broadcast.id} has no selected areas and so cannot be broadcasted.' in str(e.value) 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, notify_db,
sample_broadcast_service, sample_broadcast_service,
mocker 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 creator_and_approver = sample_broadcast_service.created_by
mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async') 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 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), (True, False),
(False, True), (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, admin_request,
sample_broadcast_service, sample_broadcast_service,
mocker, 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 sample_broadcast_service.restricted = service_restricted_before_approval
validate_and_update_broadcast_message_status( update_broadcast_message_status(
broadcast_message, BroadcastStatusType.BROADCASTING, approver broadcast_message, BroadcastStatusType.BROADCASTING, approver
) )
assert broadcast_message.status == BroadcastStatusType.BROADCASTING 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 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, admin_request,
sample_broadcast_service, sample_broadcast_service,
mocker mocker
@@ -350,7 +348,7 @@ def test_validate_and_update_broadcast_message_status_creates_event_with_correct
sample_broadcast_service.users.append(approver) sample_broadcast_service.users.append(approver)
mock_task = mocker.patch('app.celery.broadcast_message_tasks.send_broadcast_event.apply_async') 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 broadcast_message, BroadcastStatusType.BROADCASTING, approver
) )

View File

@@ -124,7 +124,7 @@ def test_valid_post_cap_xml_broadcast_returns_201(
[True, "cancelled"], [True, "cancelled"],
[False, "rejected"] [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, client,
sample_broadcast_service, sample_broadcast_service,
mocker, mocker,
@@ -153,7 +153,9 @@ def test_valid_cancel_broadcast_request_calls_validate_and_update_broadcast_mess
if is_approved: if is_approved:
broadcast_message.status = 'broadcasting' 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 # cancel broadcast
response_for_cancel = client.post( response_for_cancel = client.post(