diff --git a/app/celery/broadcast_message_tasks.py b/app/celery/broadcast_message_tasks.py index 4a6a9648b..d13b76cd3 100644 --- a/app/celery/broadcast_message_tasks.py +++ b/app/celery/broadcast_message_tasks.py @@ -54,7 +54,7 @@ def send_broadcast_provider_message(broadcast_event_id, provider): headline="GOV.UK Notify Broadcast", description=broadcast_event.transmitted_content['body'], areas=areas, - references=broadcast_event.get_earlier_message_references(), + previous_provider_messages=broadcast_event.get_earlier_provider_messages(provider), sent=broadcast_event.sent_at_as_cap_datetime_string, expires=broadcast_event.transmitted_finishes_at_as_cap_datetime_string, ) @@ -64,7 +64,7 @@ def send_broadcast_provider_message(broadcast_event_id, provider): headline="GOV.UK Notify Broadcast", description=broadcast_event.transmitted_content['body'], areas=areas, - references=broadcast_event.get_earlier_message_references(), + previous_provider_messages=broadcast_event.get_earlier_provider_messages(provider), sent=broadcast_event.sent_at_as_cap_datetime_string, expires=broadcast_event.transmitted_finishes_at_as_cap_datetime_string, ) diff --git a/app/clients/cbc_proxy.py b/app/clients/cbc_proxy.py index c7c231888..841e6dad4 100644 --- a/app/clients/cbc_proxy.py +++ b/app/clients/cbc_proxy.py @@ -16,10 +16,10 @@ from app.config import BroadcastProvider # * description is a string which populates the areaDesc field # * polygon is a list of lat/long pairs # -# references is a whitespace separated list of message identifiers +# previous_provider_messages is a whitespace separated list of message identifiers # where each identifier is a previous sent message # ie a Cancel message would have a unique identifier but have the identifier of -# the preceeding Alert message in the references field +# the preceeding Alert message in the previous_provider_messages field class CBCProxyException(Exception): @@ -54,7 +54,7 @@ class CBCProxyNoopClient: # We have not implementated updating a broadcast def update_and_send_broadcast( self, - identifier, references, headline, description, areas, + identifier, previous_provider_messages, headline, description, areas, sent, expires, ): pass @@ -62,7 +62,7 @@ class CBCProxyNoopClient: # We have not implemented cancelling a broadcast def cancel_broadcast( self, - identifier, references, headline, description, areas, + identifier, previous_provider_messages, headline, description, areas, sent, expires, ): pass @@ -139,7 +139,7 @@ class CBCProxyClient: # We have not implementated updating a broadcast def update_and_send_broadcast( self, - identifier, references, headline, description, areas, + identifier, previous_provider_messages, headline, description, areas, sent, expires, ): pass @@ -147,7 +147,7 @@ class CBCProxyClient: # We have not implemented cancelling a broadcast def cancel_broadcast( self, - identifier, references, headline, description, areas, + identifier, previous_provider_messages, headline, description, areas, sent, expires, ): pass diff --git a/app/models.py b/app/models.py index 5bb8d8b50..1d5ea5174 100644 --- a/app/models.py +++ b/app/models.py @@ -2387,9 +2387,30 @@ class BroadcastEvent(db.Model): None ) - def get_earlier_message_references(self): + def get_earlier_provider_messages(self, provider): + """ + Get the previous message for a provider. These are differentper provider, as the identifiers are different. + Return the full provider_message object rather than just an identifier, since the different providers expect + reference to contain different things - let the cbc_proxy work out what information is relevant. + """ from app.dao.broadcast_message_dao import get_earlier_events_for_broadcast_event - return [event.reference for event in get_earlier_events_for_broadcast_event(self.id)] + earlier_events = [ + event for event in get_earlier_events_for_broadcast_event(self.id) + ] + ret = [] + for event in earlier_events: + provider_message = event.get_provider_message(provider) + if provider_message is None: + # TODO: We should figure out what to do if a previous message hasn't been sent out yet. + # We don't want to not cancel a message just because it's stuck in a queue somewhere. + # This exception should probably be named, and then should be caught further up and handled + # appropriately. + raise Exception( + f'Cannot get earlier message references for event {self.id}, previous event {event.id} has not ' + + f' been sent to provider "{provider}" yet' + ) + ret.append(provider_message) + return ret def serialize(self): return { @@ -2397,8 +2418,6 @@ class BroadcastEvent(db.Model): 'service_id': str(self.service_id), - 'previous_event_references': self.get_earlier_message_references(), - 'broadcast_message_id': str(self.broadcast_message_id), # sent_at is required by BroadcastMessageTemplate.from_broadcast_event 'sent_at': self.sent_at.strftime(DATETIME_FORMAT), diff --git a/tests/app/celery/test_broadcast_message_tasks.py b/tests/app/celery/test_broadcast_message_tasks.py index 9c77dedb1..92dff4db6 100644 --- a/tests/app/celery/test_broadcast_message_tasks.py +++ b/tests/app/celery/test_broadcast_message_tasks.py @@ -7,7 +7,12 @@ import pytest from app.models import BROADCAST_TYPE, BroadcastStatusType, BroadcastEventMessageType, BroadcastProviderMessageStatus from app.celery.broadcast_message_tasks import send_broadcast_event, send_broadcast_provider_message, trigger_link_test -from tests.app.db import create_template, create_broadcast_message, create_broadcast_event +from tests.app.db import ( + create_template, + create_broadcast_message, + create_broadcast_event, + create_broadcast_provider_message +) from tests.conftest import set_config @@ -86,6 +91,7 @@ def test_send_broadcast_provider_message_sends_update_with_references(mocker, sa ) alert_event = create_broadcast_event(broadcast_message, message_type=BroadcastEventMessageType.ALERT) + create_broadcast_provider_message(alert_event, 'ee') update_event = create_broadcast_event(broadcast_message, message_type=BroadcastEventMessageType.UPDATE) mock_update_broadcast = mocker.patch( @@ -95,7 +101,7 @@ def test_send_broadcast_provider_message_sends_update_with_references(mocker, sa send_broadcast_provider_message(provider='ee', broadcast_event_id=str(update_event.id)) broadcast_provider_message = update_event.get_provider_message('ee') - assert broadcast_provider_message.state == BroadcastProviderMessageStatus.SENDING + assert broadcast_provider_message.status == BroadcastProviderMessageStatus.SENDING mock_update_broadcast.assert_called_once_with( identifier=str(broadcast_provider_message.id), @@ -104,7 +110,9 @@ def test_send_broadcast_provider_message_sends_update_with_references(mocker, sa areas=[{ "polygon": [[50.12, 1.2], [50.13, 1.2], [50.14, 1.21]], }], - references=[alert_event.reference], + previous_provider_messages=[ + alert_event.get_provider_message('ee') + ], sent=update_event.sent_at_as_cap_datetime_string, expires=update_event.transmitted_finishes_at_as_cap_datetime_string, ) @@ -128,6 +136,9 @@ def test_send_broadcast_provider_message_sends_cancel_with_references(mocker, sa update_event = create_broadcast_event(broadcast_message, message_type=BroadcastEventMessageType.UPDATE) cancel_event = create_broadcast_event(broadcast_message, message_type=BroadcastEventMessageType.CANCEL) + create_broadcast_provider_message(alert_event, 'ee') + create_broadcast_provider_message(update_event, 'ee') + mock_cancel_broadcast = mocker.patch( 'app.cbc_proxy_client.cancel_broadcast', ) @@ -135,7 +146,7 @@ def test_send_broadcast_provider_message_sends_cancel_with_references(mocker, sa send_broadcast_provider_message(provider='ee', broadcast_event_id=str(cancel_event.id)) broadcast_provider_message = cancel_event.get_provider_message('ee') - assert broadcast_provider_message.state == BroadcastProviderMessageStatus.SENDING + assert broadcast_provider_message.status == BroadcastProviderMessageStatus.SENDING mock_cancel_broadcast.assert_called_once_with( identifier=str(broadcast_provider_message.id), @@ -144,7 +155,10 @@ def test_send_broadcast_provider_message_sends_cancel_with_references(mocker, sa areas=[{ "polygon": [[50.12, 1.2], [50.13, 1.2], [50.14, 1.21]], }], - references=[alert_event.reference, update_event.reference], + previous_provider_messages=[ + alert_event.get_provider_message('ee'), + update_event.get_provider_message('ee') + ], sent=cancel_event.sent_at_as_cap_datetime_string, expires=cancel_event.transmitted_finishes_at_as_cap_datetime_string, ) diff --git a/tests/app/dao/test_broadcast_message_dao.py b/tests/app/dao/test_broadcast_message_dao.py index 196a3457f..a8fb160b5 100644 --- a/tests/app/dao/test_broadcast_message_dao.py +++ b/tests/app/dao/test_broadcast_message_dao.py @@ -1,12 +1,11 @@ from datetime import datetime -from freezegun import freeze_time - -from app.models import BROADCAST_TYPE, BroadcastStatusType, BroadcastEventMessageType +from app.models import BROADCAST_TYPE, BroadcastEventMessageType from app.dao.broadcast_message_dao import get_earlier_events_for_broadcast_event, create_broadcast_provider_message from tests.app.db import create_broadcast_message, create_template, create_broadcast_event + def test_get_earlier_events_for_broadcast_event(sample_service): t = create_template(sample_service, BROADCAST_TYPE) bm = create_broadcast_message(t) @@ -44,10 +43,9 @@ def test_get_earlier_events_for_broadcast_event(sample_service): assert earlier_events == [events[0], events[1]] -@freeze_time('2020-02-03 04:05:06') def test_create_broadcast_provider_message_creates_in_correct_state(sample_broadcast_service): t = create_template(sample_broadcast_service, BROADCAST_TYPE) - broadcast_message = create_broadcast_message(t, status=BroadcastStatusType.APPROVED) + broadcast_message = create_broadcast_message(t) broadcast_event = create_broadcast_event( broadcast_message, sent_at=datetime(2020, 1, 1, 12, 0, 0), @@ -59,4 +57,5 @@ def test_create_broadcast_provider_message_creates_in_correct_state(sample_broad assert broadcast_provider_message.status == 'sending' assert broadcast_provider_message.broadcast_event_id == broadcast_event.id - assert broadcast_provider_message.created_at == datetime.utcnow() + assert broadcast_provider_message.created_at is not None + assert broadcast_provider_message.updated_at is None diff --git a/tests/app/db.py b/tests/app/db.py index 969a20e28..30f00e7b6 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -62,7 +62,8 @@ from app.models import ( ServiceContactList, BroadcastMessage, BroadcastStatusType, - BroadcastEvent + BroadcastEvent, + BroadcastProviderMessage ) @@ -1050,3 +1051,18 @@ def create_broadcast_event( db.session.add(b_e) db.session.commit() return b_e + + +def create_broadcast_provider_message( + broadcast_event, + provider, + status='sending' +): + provider_message = BroadcastProviderMessage( + broadcast_event=broadcast_event, + provider=provider, + status=status + ) + db.session.add(provider_message) + db.session.commit() + return provider_message