mirror of
https://github.com/GSA/notifications-api.git
synced 2025-12-22 00:11:16 -05:00
Merge pull request #3214 from alphagov/check-broadcast-suspended
Enforce service suspension for broadcasts
This commit is contained in:
@@ -192,6 +192,9 @@ def update_broadcast_message_status(service_id, broadcast_message_id):
|
||||
validate(data, update_broadcast_message_status_schema)
|
||||
broadcast_message = dao_get_broadcast_message_by_id_and_service_id(broadcast_message_id, service_id)
|
||||
|
||||
if not broadcast_message.service.active:
|
||||
raise InvalidRequest("Updating broadcast message is not allowed: service is inactive ", 403)
|
||||
|
||||
new_status = data['status']
|
||||
updating_user = get_user_by_id(data['created_by'])
|
||||
|
||||
|
||||
@@ -5,10 +5,7 @@ from flask import current_app
|
||||
from sqlalchemy.schema import Sequence
|
||||
|
||||
from app import cbc_proxy_client, db, notify_celery, zendesk_client
|
||||
from app.clients.cbc_proxy import (
|
||||
CBCProxyFatalException,
|
||||
CBCProxyRetryableException,
|
||||
)
|
||||
from app.clients.cbc_proxy import CBCProxyRetryableException
|
||||
from app.config import QueueNames
|
||||
from app.dao.broadcast_message_dao import (
|
||||
create_broadcast_provider_message,
|
||||
@@ -23,6 +20,10 @@ from app.models import (
|
||||
from app.utils import format_sequential_number
|
||||
|
||||
|
||||
class BroadcastIntegrityError(Exception):
|
||||
pass
|
||||
|
||||
|
||||
def get_retry_delay(retry_count):
|
||||
"""
|
||||
Given a count of retries so far, return a delay for the next one.
|
||||
@@ -36,7 +37,27 @@ def get_retry_delay(retry_count):
|
||||
return min(delay, 240)
|
||||
|
||||
|
||||
def check_provider_message_should_send(broadcast_event, provider):
|
||||
def check_event_is_authorised_to_be_sent(broadcast_event, provider):
|
||||
if not broadcast_event.service.active:
|
||||
raise BroadcastIntegrityError(
|
||||
f'Cannot send broadcast_event {broadcast_event.id} ' +
|
||||
f'to provider {provider}: the service is suspended'
|
||||
)
|
||||
|
||||
if broadcast_event.service.restricted:
|
||||
raise BroadcastIntegrityError(
|
||||
f'Cannot send broadcast_event {broadcast_event.id} ' +
|
||||
f'to provider {provider}: the service is not live'
|
||||
)
|
||||
|
||||
if broadcast_event.broadcast_message.stubbed:
|
||||
raise BroadcastIntegrityError(
|
||||
f'Cannot send broadcast_event {broadcast_event.id} ' +
|
||||
f'to provider {provider}: the broadcast message is stubbed'
|
||||
)
|
||||
|
||||
|
||||
def check_event_makes_sense_in_sequence(broadcast_event, provider):
|
||||
"""
|
||||
If any previous event hasn't sent yet for that provider, then we shouldn't send the current event. Instead, fail and
|
||||
raise a P1 - so that a notify team member can assess the state of the previous messages, and if necessary, can
|
||||
@@ -58,16 +79,14 @@ def check_provider_message_should_send(broadcast_event, provider):
|
||||
current_provider_message = broadcast_event.get_provider_message(provider)
|
||||
# if this is the first time a task is being executed, it won't have a provider message yet
|
||||
if current_provider_message and current_provider_message.status != BroadcastProviderMessageStatus.SENDING:
|
||||
raise CBCProxyFatalException(
|
||||
raise BroadcastIntegrityError(
|
||||
f'Cannot send broadcast_event {broadcast_event.id} ' +
|
||||
f'to provider {provider}: ' +
|
||||
f'It is in status {current_provider_message.status}'
|
||||
)
|
||||
|
||||
if broadcast_event.transmitted_finishes_at < datetime.utcnow():
|
||||
# TODO: This should be a different kind of exception to distinguish "We should know something went wrong, but
|
||||
# no immediate action" from "We need to fix this immediately"
|
||||
raise CBCProxyFatalException(
|
||||
raise BroadcastIntegrityError(
|
||||
f'Cannot send broadcast_event {broadcast_event.id} ' +
|
||||
f'to provider {provider}: ' +
|
||||
f'The expiry time of {broadcast_event.transmitted_finishes_at} has already passed'
|
||||
@@ -83,7 +102,7 @@ def check_provider_message_should_send(broadcast_event, provider):
|
||||
|
||||
# the previous message hasn't even got round to running `send_broadcast_provider_message` yet.
|
||||
if not prev_provider_message:
|
||||
raise CBCProxyFatalException(
|
||||
raise BroadcastIntegrityError(
|
||||
f'Cannot send {broadcast_event.id}. Previous event {prev_event.id} ' +
|
||||
f'(type {prev_event.message_type}) has no provider_message for provider {provider} yet.\n' +
|
||||
'You must ensure that the other event sends succesfully, then manually kick off this event ' +
|
||||
@@ -93,7 +112,7 @@ def check_provider_message_should_send(broadcast_event, provider):
|
||||
# if there's a previous message that has started but not finished sending (whether it fatally errored or is
|
||||
# currently retrying)
|
||||
if prev_provider_message.status != BroadcastProviderMessageStatus.ACK:
|
||||
raise CBCProxyFatalException(
|
||||
raise BroadcastIntegrityError(
|
||||
f'Cannot send {broadcast_event.id}. Previous event {prev_event.id} ' +
|
||||
f'(type {prev_event.message_type}) has not finished sending to provider {provider} yet.\n' +
|
||||
f'It is currently in status "{prev_provider_message.status}".\n' +
|
||||
@@ -151,7 +170,8 @@ def send_broadcast_provider_message(self, broadcast_event_id, provider):
|
||||
|
||||
broadcast_event = dao_get_broadcast_event_by_id(broadcast_event_id)
|
||||
|
||||
check_provider_message_should_send(broadcast_event, provider)
|
||||
check_event_is_authorised_to_be_sent(broadcast_event, provider)
|
||||
check_event_makes_sense_in_sequence(broadcast_event, provider)
|
||||
|
||||
# the broadcast_provider_message may already exist if we retried previously
|
||||
broadcast_provider_message = broadcast_event.get_provider_message(provider)
|
||||
|
||||
@@ -26,10 +26,6 @@ from app.utils import DATETIME_FORMAT, format_sequential_number
|
||||
# the preceeding Alert message in the previous_provider_messages field
|
||||
|
||||
|
||||
class CBCProxyFatalException(Exception):
|
||||
pass
|
||||
|
||||
|
||||
class CBCProxyRetryableException(Exception):
|
||||
pass
|
||||
|
||||
|
||||
@@ -645,6 +645,22 @@ def test_update_broadcast_message_status_updates_details_but_does_not_queue_task
|
||||
assert len(mock_task.mock_calls) == 0
|
||||
|
||||
|
||||
def test_update_broadcast_message_status_aborts_if_service_is_suspended(
|
||||
admin_request,
|
||||
sample_broadcast_service,
|
||||
):
|
||||
bm = create_broadcast_message(service=sample_broadcast_service, content='test')
|
||||
sample_broadcast_service.active = False
|
||||
|
||||
admin_request.post(
|
||||
'broadcast_message.update_broadcast_message_status',
|
||||
_data={'status': BroadcastStatusType.BROADCASTING, 'created_by': str(uuid.uuid4())},
|
||||
service_id=sample_broadcast_service.id,
|
||||
broadcast_message_id=bm.id,
|
||||
_expected_status=403
|
||||
)
|
||||
|
||||
|
||||
def test_update_broadcast_message_status_creates_event_with_correct_content_if_broadcast_has_no_template(
|
||||
admin_request,
|
||||
sample_broadcast_service,
|
||||
|
||||
@@ -7,16 +7,14 @@ from celery.exceptions import Retry
|
||||
from freezegun import freeze_time
|
||||
|
||||
from app.celery.broadcast_message_tasks import (
|
||||
check_provider_message_should_send,
|
||||
BroadcastIntegrityError,
|
||||
check_event_makes_sense_in_sequence,
|
||||
get_retry_delay,
|
||||
send_broadcast_event,
|
||||
send_broadcast_provider_message,
|
||||
trigger_link_test,
|
||||
)
|
||||
from app.clients.cbc_proxy import (
|
||||
CBCProxyFatalException,
|
||||
CBCProxyRetryableException,
|
||||
)
|
||||
from app.clients.cbc_proxy import CBCProxyRetryableException
|
||||
from app.models import (
|
||||
BROADCAST_TYPE,
|
||||
BroadcastEventMessageType,
|
||||
@@ -602,31 +600,31 @@ def test_get_retry_delay_has_capped_backoff(retry_count, expected_delay):
|
||||
|
||||
|
||||
@freeze_time('2021-01-01 12:00')
|
||||
def test_check_provider_message_should_send_doesnt_raise_if_event_hasnt_expired_yet(sample_template):
|
||||
def test_check_event_makes_sense_in_sequence_doesnt_raise_if_event_hasnt_expired_yet(sample_template):
|
||||
broadcast_message = create_broadcast_message(sample_template)
|
||||
current_event = create_broadcast_event(
|
||||
broadcast_message,
|
||||
transmitted_starts_at=datetime(2021, 1, 1, 0, 0),
|
||||
transmitted_finishes_at=datetime(2021, 1, 1, 12, 1),
|
||||
)
|
||||
check_provider_message_should_send(current_event, 'ee')
|
||||
check_event_makes_sense_in_sequence(current_event, 'ee')
|
||||
|
||||
|
||||
@freeze_time('2021-01-01 12:00')
|
||||
def test_check_provider_message_should_send_raises_if_event_has_expired(sample_template):
|
||||
def test_send_broadcast_provider_message_raises_if_event_has_expired(sample_template):
|
||||
broadcast_message = create_broadcast_message(sample_template)
|
||||
current_event = create_broadcast_event(
|
||||
broadcast_message,
|
||||
transmitted_starts_at=datetime(2021, 1, 1, 0, 0),
|
||||
transmitted_finishes_at=datetime(2021, 1, 1, 11, 59),
|
||||
)
|
||||
with pytest.raises(CBCProxyFatalException) as exc:
|
||||
check_provider_message_should_send(current_event, 'ee')
|
||||
with pytest.raises(BroadcastIntegrityError) as exc:
|
||||
send_broadcast_provider_message(current_event.id, 'ee')
|
||||
assert 'The expiry time of 2021-01-01 11:59:00 has already passed' in str(exc.value)
|
||||
|
||||
|
||||
@freeze_time('2021-01-01 12:00')
|
||||
def test_check_provider_message_should_send_raises_if_older_event_still_sending(sample_template):
|
||||
def test_send_broadcast_provider_message_raises_if_older_event_still_sending(sample_template):
|
||||
broadcast_message = create_broadcast_message(sample_template)
|
||||
# event approved at midnight
|
||||
past_succesful_event = create_broadcast_event(
|
||||
@@ -651,14 +649,14 @@ def test_check_provider_message_should_send_raises_if_older_event_still_sending(
|
||||
create_broadcast_provider_message(past_still_sending_event, provider='ee', status=BroadcastProviderMessageStatus.SENDING) # noqa
|
||||
|
||||
# we havent sent the previous update yet - it's still in sending - so don't try and send this one.
|
||||
with pytest.raises(CBCProxyFatalException) as exc:
|
||||
check_provider_message_should_send(current_event, 'ee')
|
||||
with pytest.raises(BroadcastIntegrityError) as exc:
|
||||
send_broadcast_provider_message(current_event.id, 'ee')
|
||||
|
||||
assert f'Previous event {past_still_sending_event.id} (type update) has not finished sending to provider ee' in str(exc.value) # noqa
|
||||
|
||||
|
||||
@freeze_time('2021-01-01 12:00')
|
||||
def test_check_provider_message_should_send_raises_if_older_event_hasnt_started_sending_yet(sample_template):
|
||||
def test_send_broadcast_provider_message_raises_if_older_event_hasnt_started_sending_yet(sample_template):
|
||||
broadcast_message = create_broadcast_message(sample_template)
|
||||
# event approved at midnight
|
||||
past_succesful_event = create_broadcast_event(
|
||||
@@ -683,14 +681,14 @@ def test_check_provider_message_should_send_raises_if_older_event_hasnt_started_
|
||||
create_broadcast_provider_message(past_succesful_event, provider='ee', status=BroadcastProviderMessageStatus.ACK)
|
||||
|
||||
# we shouldn't send the update now, because a previous event is still stuck in sending
|
||||
with pytest.raises(CBCProxyFatalException) as exc:
|
||||
check_provider_message_should_send(current_event, 'ee')
|
||||
with pytest.raises(BroadcastIntegrityError) as exc:
|
||||
send_broadcast_provider_message(current_event.id, 'ee')
|
||||
|
||||
assert f'Previous event {past_still_sending_event.id} (type update) has no provider_message for provider ee' in str(exc.value) # noqa
|
||||
|
||||
|
||||
@freeze_time('2021-01-01 12:00')
|
||||
def test_check_provider_message_should_send_doesnt_raise_if_newer_event_not_acked_yet(sample_template):
|
||||
def test_check_event_makes_sense_in_sequence_doesnt_raise_if_newer_event_not_acked_yet(sample_template):
|
||||
broadcast_message = create_broadcast_message(sample_template)
|
||||
# event approved at midnight
|
||||
current_event = create_broadcast_event(
|
||||
@@ -707,34 +705,64 @@ def test_check_provider_message_should_send_doesnt_raise_if_newer_event_not_acke
|
||||
|
||||
# this doesn't raise, because the alert event got an ack. The cancel doesn't have an event yet
|
||||
# but this task is only interested in the current task (the update) so doesn't worry about that
|
||||
check_provider_message_should_send(current_event, 'ee')
|
||||
check_event_makes_sense_in_sequence(current_event, 'ee')
|
||||
|
||||
|
||||
@pytest.mark.parametrize('existing_message_status', [
|
||||
BroadcastProviderMessageStatus.SENDING,
|
||||
pytest.param(
|
||||
BroadcastProviderMessageStatus.ACK,
|
||||
marks=pytest.mark.xfail(raises=CBCProxyFatalException)
|
||||
),
|
||||
pytest.param(
|
||||
BroadcastProviderMessageStatus.ERR,
|
||||
marks=pytest.mark.xfail(raises=CBCProxyFatalException)
|
||||
),
|
||||
pytest.param(
|
||||
BroadcastProviderMessageStatus.TECHNICAL_FAILURE,
|
||||
marks=pytest.mark.xfail(raises=CBCProxyFatalException)
|
||||
),
|
||||
])
|
||||
def test_check_provider_message_should_send_raises_if_current_event_already_has_provider_message_not_in_sending(
|
||||
def test_send_broadcast_provider_message_raises_if_current_event_already_has_provider_message_not_in_sending(
|
||||
sample_template,
|
||||
existing_message_status
|
||||
):
|
||||
broadcast_message = create_broadcast_message(sample_template)
|
||||
current_event = create_broadcast_event(broadcast_message, message_type='alert')
|
||||
|
||||
create_broadcast_provider_message(current_event, provider='ee', status=existing_message_status)
|
||||
|
||||
check_provider_message_should_send(current_event, 'ee')
|
||||
with pytest.raises(BroadcastIntegrityError) as exc:
|
||||
send_broadcast_provider_message(current_event.id, 'ee')
|
||||
|
||||
assert f'in status {existing_message_status}' in str(exc.value)
|
||||
|
||||
|
||||
def test_send_broadcast_provider_message_raises_if_service_is_suspended(
|
||||
sample_broadcast_service,
|
||||
):
|
||||
sample_broadcast_service.active = False
|
||||
broadcast_message = create_broadcast_message(service=sample_broadcast_service, content='test')
|
||||
current_event = create_broadcast_event(broadcast_message, message_type='alert')
|
||||
|
||||
with pytest.raises(BroadcastIntegrityError) as exc:
|
||||
send_broadcast_provider_message(current_event.id, 'ee')
|
||||
|
||||
assert 'service is suspended' in str(exc.value)
|
||||
|
||||
|
||||
def test_send_broadcast_provider_message_raises_if_service_is_not_live(
|
||||
sample_broadcast_service,
|
||||
):
|
||||
sample_broadcast_service.restricted = True
|
||||
broadcast_message = create_broadcast_message(service=sample_broadcast_service, content='test')
|
||||
current_event = create_broadcast_event(broadcast_message, message_type='alert')
|
||||
|
||||
with pytest.raises(BroadcastIntegrityError) as exc:
|
||||
send_broadcast_provider_message(current_event.id, 'ee')
|
||||
|
||||
assert 'service is not live' in str(exc.value)
|
||||
|
||||
|
||||
def test_send_broadcast_provider_message_raises_if_message_is_stubbed(
|
||||
sample_template,
|
||||
):
|
||||
broadcast_message = create_broadcast_message(sample_template, stubbed=True)
|
||||
current_event = create_broadcast_event(broadcast_message, message_type='alert')
|
||||
|
||||
with pytest.raises(BroadcastIntegrityError) as exc:
|
||||
send_broadcast_provider_message(current_event.id, 'ee')
|
||||
|
||||
assert 'message is stubbed' in str(exc.value)
|
||||
|
||||
|
||||
def test_send_broadcast_provider_message_does_nothing_if_cbc_proxy_disabled(mocker, notify_api, sample_template):
|
||||
|
||||
Reference in New Issue
Block a user