From 2a04148ea1c00d92385c5f48370efd0125748203 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Wed, 2 Dec 2020 15:59:41 +0000 Subject: [PATCH 01/12] Add sequential numbering for broadcast messages We need that to send broadcast messages using proprietary IBAG format that Vodafone currently uses. --- app/models.py | 19 ++++++++ .../versions/0334_broadcast_message_number.py | 48 +++++++++++++++++++ 2 files changed, 67 insertions(+) create mode 100644 migrations/versions/0334_broadcast_message_number.py diff --git a/app/models.py b/app/models.py index 4edf8fac5..494842b6b 100644 --- a/app/models.py +++ b/app/models.py @@ -6,6 +6,7 @@ from flask import url_for, current_app from sqlalchemy.ext.declarative import declared_attr from sqlalchemy.ext.associationproxy import association_proxy from sqlalchemy.ext.hybrid import hybrid_property +from sqlalchemy.schema import Sequence from sqlalchemy.dialects.postgresql import ( UUID, JSON, @@ -2483,6 +2484,24 @@ class BroadcastProviderMessage(db.Model): UniqueConstraint(broadcast_event_id, provider) +class BroadcastProviderMessageNumber(db.Model): + """ + To send IBAG messages via the CBC proxy to Nokia CBC appliances, Notify must generate and store a numeric + message_number alongside the message ID (GUID). + Subsequent messages (Update, Cancel) in IBAG format must reference the original message_number & message_id. + This model relates broadcast_provider_message_id to that numeric message_number. + """ + __tablename__ = 'broadcast_provider_message_number' + + sequence = Sequence('broadcast_provider_message_number_seq') + broadcast_provider_message_number = db.Column( + db.Integer, sequence, server_default=sequence.next_value(), primary_key=True + ) + broadcast_provider_message_id = db.Column( + UUID(as_uuid=True), db.ForeignKey('broadcast_provider_message.id'), nullable=False + ) + + class ServiceBroadcastProviderRestriction(db.Model): """ Most services don't send broadcasts. Of those that do, most send to all broadcast providers. diff --git a/migrations/versions/0334_broadcast_message_number.py b/migrations/versions/0334_broadcast_message_number.py new file mode 100644 index 000000000..0e1fd04fb --- /dev/null +++ b/migrations/versions/0334_broadcast_message_number.py @@ -0,0 +1,48 @@ +""" + +Revision ID: 0334_broadcast_message_number +Revises: 0333_service_broadcast_provider +Create Date: 2020-12-04 15:06:22.544803 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +revision = '0334_broadcast_message_number' +down_revision = '0333_service_broadcast_provider' + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.execute("create sequence broadcast_provider_message_number_seq") + op.create_table( + 'broadcast_provider_message_number', + sa.Column( + 'broadcast_provider_message_number', + sa.Integer(), + server_default=sa.text("nextval('broadcast_provider_message_number_seq')"), + nullable=False + ), + sa.Column('broadcast_provider_message_id', postgresql.UUID(as_uuid=True), nullable=False), + sa.ForeignKeyConstraint(['broadcast_provider_message_id'], ['broadcast_provider_message.id'], ), + sa.PrimaryKeyConstraint('broadcast_provider_message_number') + ) + op.execute( + """ + INSERT INTO + broadcast_provider_message_number (broadcast_provider_message_id) + SELECT + id + FROM + broadcast_provider_message + """ + ) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_table('broadcast_provider_message_number') + op.execute("drop sequence broadcast_provider_message_number_seq") + # ### end Alembic commands ### From e6824dc3ff9b4d6b0866ca7e2f3b19cc6ec1520d Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Wed, 2 Dec 2020 18:34:17 +0000 Subject: [PATCH 02/12] Broadcast provider message created with a sequential number This is for the IBAG format (similar to CAP format, but proprietary) used in the XMLs that we exchange with broadcast providers (specifically Vodafone). --- app/dao/broadcast_message_dao.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/app/dao/broadcast_message_dao.py b/app/dao/broadcast_message_dao.py index 3c05f2ab9..281e5b09a 100644 --- a/app/dao/broadcast_message_dao.py +++ b/app/dao/broadcast_message_dao.py @@ -1,6 +1,14 @@ +import uuid + from app import db from app.dao.dao_utils import transactional -from app.models import BroadcastMessage, BroadcastEvent, BroadcastProviderMessage, BroadcastProviderMessageStatus +from app.models import ( + BroadcastMessage, + BroadcastEvent, + BroadcastProviderMessage, + BroadcastProviderMessageNumber, + BroadcastProviderMessageStatus +) def dao_get_broadcast_message_by_id_and_service_id(broadcast_message_id, service_id): @@ -36,10 +44,18 @@ def get_earlier_events_for_broadcast_event(broadcast_event_id): @transactional def create_broadcast_provider_message(broadcast_event, provider): + broadcast_provider_message_id = uuid.uuid4() provider_message = BroadcastProviderMessage( + id=broadcast_provider_message_id, broadcast_event=broadcast_event, provider=provider, status=BroadcastProviderMessageStatus.SENDING, ) db.session.add(provider_message) + db.session.commit() + + provider_message_number = BroadcastProviderMessageNumber( + broadcast_provider_message_id=broadcast_provider_message_id) + db.session.add(provider_message_number) + db.session.commit() return provider_message From b34bffaae63fb966e7805975425cb97356a7612d Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Fri, 4 Dec 2020 15:49:50 +0000 Subject: [PATCH 03/12] Sends sequential number to Vodafone as link test --- app/celery/broadcast_message_tasks.py | 11 +++++++--- app/clients/cbc_proxy.py | 20 +++++++++++++++++++ .../celery/test_broadcast_message_tasks.py | 16 ++++++++++++--- tests/app/db.py | 13 ++++++++++-- 4 files changed, 52 insertions(+), 8 deletions(-) diff --git a/app/celery/broadcast_message_tasks.py b/app/celery/broadcast_message_tasks.py index 9def33129..213b671ef 100644 --- a/app/celery/broadcast_message_tasks.py +++ b/app/celery/broadcast_message_tasks.py @@ -2,10 +2,11 @@ import uuid from flask import current_app from notifications_utils.statsd_decorators import statsd +from sqlalchemy.schema import Sequence -from app import cbc_proxy_client, notify_celery +from app import cbc_proxy_client, db, notify_celery from app.config import QueueNames -from app.models import BroadcastEventMessageType +from app.models import BroadcastEventMessageType, BroadcastProvider from app.dao.broadcast_message_dao import dao_get_broadcast_event_by_id, create_broadcast_provider_message @@ -78,6 +79,10 @@ def send_broadcast_provider_message(broadcast_event_id, provider): @notify_celery.task(name='trigger-link-test') def trigger_link_test(provider): identifier = str(uuid.uuid4()) + sequential_number = None + if provider == BroadcastProvider.VODAFONE: + sequence = Sequence('broadcast_provider_message_number_seq') + sequential_number = db.session.connection().execute(sequence) message = f"Sending a link test to CBC proxy for provider {provider} with ID {identifier}" current_app.logger.info(message) - cbc_proxy_client.get_proxy(provider).send_link_test(identifier) + cbc_proxy_client.get_proxy(provider).send_link_test(identifier, sequential_number) diff --git a/app/clients/cbc_proxy.py b/app/clients/cbc_proxy.py index 81d1a34d0..6a730301d 100644 --- a/app/clients/cbc_proxy.py +++ b/app/clients/cbc_proxy.py @@ -42,6 +42,7 @@ class CBCProxyClient: proxy_classes = { 'canary': CBCProxyCanary, BroadcastProvider.EE: CBCProxyEE, + BroadcastProvider.VODAFONE: CBCProxyVodafone, } return proxy_classes[provider](self._lambda_client) @@ -61,6 +62,7 @@ class CBCProxyClientBase: def send_link_test( self, identifier, + sequential_number=None ): pass @@ -131,6 +133,7 @@ class CBCProxyEE(CBCProxyClientBase): def send_link_test( self, identifier, + sequential_number=None ): """ link test - open up a connection to a specific provider, and send them an xml payload with a of @@ -155,3 +158,20 @@ class CBCProxyEE(CBCProxyClientBase): 'expires': expires, } self._invoke_lambda(payload=payload) + + +class CBCProxyVodafone(CBCProxyClientBase): + lambda_name = 'bt-ee-1-proxy' + + def send_link_test( + self, + identifier, + sequential_number + ): + """ + link test - open up a connection to a specific provider, and send them an xml payload with a of + test. + """ + payload = {'message_type': 'test', 'identifier': identifier, 'message_number': sequential_number} + + self._invoke_lambda(payload=payload) diff --git a/tests/app/celery/test_broadcast_message_tasks.py b/tests/app/celery/test_broadcast_message_tasks.py index c6afb6998..1b0ccecd8 100644 --- a/tests/app/celery/test_broadcast_message_tasks.py +++ b/tests/app/celery/test_broadcast_message_tasks.py @@ -279,14 +279,18 @@ def test_send_broadcast_provider_message_errors(mocker, sample_service): ) +@pytest.mark.parametrize("provider,provider_capitalised", [ + ["ee", "EE"], + ["vodafone", "Vodafone"] +]) def test_trigger_link_tests_invokes_cbc_proxy_client( - mocker, + mocker, provider, provider_capitalised ): mock_send_link_test = mocker.patch( - 'app.clients.cbc_proxy.CBCProxyEE.send_link_test', + f'app.clients.cbc_proxy.CBCProxy{provider_capitalised}.send_link_test', ) - trigger_link_test('ee') + trigger_link_test(provider) assert mock_send_link_test.called # the 0th argument of the call to send_link_test @@ -296,3 +300,9 @@ def test_trigger_link_tests_invokes_cbc_proxy_client( uuid.UUID(identifier) except BaseException: pytest.fail(f"{identifier} is not a valid uuid") + + # testing sequential number: + if provider == 'vodafone': + assert type(mock_send_link_test.mock_calls[0][1][1]) is int + else: + assert not mock_send_link_test.mock_calls[0][1][1] diff --git a/tests/app/db.py b/tests/app/db.py index 30f00e7b6..a0354e894 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -63,7 +63,8 @@ from app.models import ( BroadcastMessage, BroadcastStatusType, BroadcastEvent, - BroadcastProviderMessage + BroadcastProviderMessage, + BroadcastProviderMessageNumber ) @@ -1058,11 +1059,19 @@ def create_broadcast_provider_message( provider, status='sending' ): + broadcast_provider_message_id = uuid.uuid4() provider_message = BroadcastProviderMessage( + id=broadcast_provider_message_id, broadcast_event=broadcast_event, provider=provider, - status=status + status=status, ) db.session.add(provider_message) + db.session.commit() + + provider_message_number = BroadcastProviderMessageNumber( + broadcast_provider_message_id=broadcast_provider_message_id) + db.session.add(provider_message_number) + db.session.commit() return provider_message From a186d2d2969c20e0fe268697bc3de429082532b5 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Fri, 4 Dec 2020 16:00:20 +0000 Subject: [PATCH 04/12] Format sequential number into an 8 char long hex As per Vodafone spec for ibag format message number --- app/celery/broadcast_message_tasks.py | 7 +++++-- app/utils.py | 4 ++++ tests/app/celery/test_broadcast_message_tasks.py | 3 ++- tests/app/test_utils.py | 7 ++++++- 4 files changed, 17 insertions(+), 4 deletions(-) diff --git a/app/celery/broadcast_message_tasks.py b/app/celery/broadcast_message_tasks.py index 213b671ef..a02c6004b 100644 --- a/app/celery/broadcast_message_tasks.py +++ b/app/celery/broadcast_message_tasks.py @@ -9,6 +9,8 @@ from app.config import QueueNames from app.models import BroadcastEventMessageType, BroadcastProvider from app.dao.broadcast_message_dao import dao_get_broadcast_event_by_id, create_broadcast_provider_message +from app.utils import format_sequential_number + @notify_celery.task(name="send-broadcast-event") @statsd(namespace="tasks") @@ -79,10 +81,11 @@ def send_broadcast_provider_message(broadcast_event_id, provider): @notify_celery.task(name='trigger-link-test') def trigger_link_test(provider): identifier = str(uuid.uuid4()) - sequential_number = None + formatted_seq_number = None if provider == BroadcastProvider.VODAFONE: sequence = Sequence('broadcast_provider_message_number_seq') sequential_number = db.session.connection().execute(sequence) + formatted_seq_number = format_sequential_number(sequential_number) message = f"Sending a link test to CBC proxy for provider {provider} with ID {identifier}" current_app.logger.info(message) - cbc_proxy_client.get_proxy(provider).send_link_test(identifier, sequential_number) + cbc_proxy_client.get_proxy(provider).send_link_test(identifier, formatted_seq_number) diff --git a/app/utils.py b/app/utils.py index 719958399..b0f0aa8e2 100644 --- a/app/utils.py +++ b/app/utils.py @@ -146,3 +146,7 @@ def get_archived_db_column_value(column): def get_dt_string_or_none(val): return val.strftime(DATETIME_FORMAT) if val else None + + +def format_sequential_number(sequential_number): + return format(sequential_number, "x").zfill(8) diff --git a/tests/app/celery/test_broadcast_message_tasks.py b/tests/app/celery/test_broadcast_message_tasks.py index 1b0ccecd8..61aedcdd7 100644 --- a/tests/app/celery/test_broadcast_message_tasks.py +++ b/tests/app/celery/test_broadcast_message_tasks.py @@ -303,6 +303,7 @@ def test_trigger_link_tests_invokes_cbc_proxy_client( # testing sequential number: if provider == 'vodafone': - assert type(mock_send_link_test.mock_calls[0][1][1]) is int + assert type(mock_send_link_test.mock_calls[0][1][1]) is str + assert len(mock_send_link_test.mock_calls[0][1][1]) == 8 else: assert not mock_send_link_test.mock_calls[0][1][1] diff --git a/tests/app/test_utils.py b/tests/app/test_utils.py index 70c16e08d..d8e117eab 100644 --- a/tests/app/test_utils.py +++ b/tests/app/test_utils.py @@ -4,10 +4,11 @@ import pytest from freezegun import freeze_time from app.utils import ( + format_sequential_number, get_london_midnight_in_utc, get_midnight_for_day_before, - midnight_n_days_ago, get_notification_table_to_use, + midnight_n_days_ago ) from app.models import Notification, NotificationHistory @@ -99,3 +100,7 @@ def test_get_notification_table_to_use_checks_service_data_retention(sample_serv # falls back to 7 days if not specified assert get_notification_table_to_use(sample_service, 'sms', date(2019, 1, 1), False) == NotificationHistory assert get_notification_table_to_use(sample_service, 'sms', date(2019, 1, 2), False) == Notification + + +def test_format_sequential_number(): + assert format_sequential_number(123) == '0000007b' From e95dc9450ec16f42d492658beb5e9ec48f73761e Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Fri, 4 Dec 2020 17:07:08 +0000 Subject: [PATCH 05/12] Include message number in send_broadcast_provider_message --- app/celery/broadcast_message_tasks.py | 6 +++++- app/dao/broadcast_message_dao.py | 2 +- tests/app/celery/test_broadcast_message_tasks.py | 4 ++++ tests/app/dao/test_broadcast_message_dao.py | 2 +- tests/app/db.py | 2 +- 5 files changed, 12 insertions(+), 4 deletions(-) diff --git a/app/celery/broadcast_message_tasks.py b/app/celery/broadcast_message_tasks.py index a02c6004b..0cdc2800a 100644 --- a/app/celery/broadcast_message_tasks.py +++ b/app/celery/broadcast_message_tasks.py @@ -32,7 +32,8 @@ def send_broadcast_event(broadcast_event_id): def send_broadcast_provider_message(broadcast_event_id, provider): broadcast_event = dao_get_broadcast_event_by_id(broadcast_event_id) - broadcast_provider_message = create_broadcast_provider_message(broadcast_event, provider) + broadcast_provider_message, message_number = create_broadcast_provider_message(broadcast_event, provider) + formatted_message_number = format_sequential_number(message_number.broadcast_provider_message_number) current_app.logger.info( f'invoking cbc proxy to send ' @@ -50,6 +51,7 @@ def send_broadcast_provider_message(broadcast_event_id, provider): if broadcast_event.message_type == BroadcastEventMessageType.ALERT: cbc_proxy_provider_client.create_and_send_broadcast( identifier=str(broadcast_provider_message.id), + message_number=formatted_message_number, headline="GOV.UK Notify Broadcast", description=broadcast_event.transmitted_content['body'], areas=areas, @@ -59,6 +61,7 @@ def send_broadcast_provider_message(broadcast_event_id, provider): elif broadcast_event.message_type == BroadcastEventMessageType.UPDATE: cbc_proxy_provider_client.update_and_send_broadcast( identifier=str(broadcast_provider_message.id), + message_number=formatted_message_number, headline="GOV.UK Notify Broadcast", description=broadcast_event.transmitted_content['body'], areas=areas, @@ -69,6 +72,7 @@ def send_broadcast_provider_message(broadcast_event_id, provider): elif broadcast_event.message_type == BroadcastEventMessageType.CANCEL: cbc_proxy_provider_client.cancel_broadcast( identifier=str(broadcast_provider_message.id), + message_number=formatted_message_number, headline="GOV.UK Notify Broadcast", description=broadcast_event.transmitted_content['body'], areas=areas, diff --git a/app/dao/broadcast_message_dao.py b/app/dao/broadcast_message_dao.py index 281e5b09a..e79f0a8f8 100644 --- a/app/dao/broadcast_message_dao.py +++ b/app/dao/broadcast_message_dao.py @@ -58,4 +58,4 @@ def create_broadcast_provider_message(broadcast_event, provider): broadcast_provider_message_id=broadcast_provider_message_id) db.session.add(provider_message_number) db.session.commit() - return provider_message + return provider_message, provider_message_number diff --git a/tests/app/celery/test_broadcast_message_tasks.py b/tests/app/celery/test_broadcast_message_tasks.py index 61aedcdd7..e49db47e7 100644 --- a/tests/app/celery/test_broadcast_message_tasks.py +++ b/tests/app/celery/test_broadcast_message_tasks.py @@ -133,6 +133,7 @@ def test_send_broadcast_provider_message_sends_data_correctly(mocker, sample_ser mock_create_broadcast.assert_called_once_with( identifier=str(broadcast_provider_message.id), + message_number=mocker.ANY, headline='GOV.UK Notify Broadcast', description='this is an emergency broadcast message', areas=[{ @@ -178,6 +179,7 @@ def test_send_broadcast_provider_message_sends_update_with_references(mocker, sa mock_update_broadcast.assert_called_once_with( identifier=str(broadcast_provider_message.id), + message_number=mocker.ANY, headline="GOV.UK Notify Broadcast", description='this is an emergency broadcast message', areas=[{ @@ -223,6 +225,7 @@ def test_send_broadcast_provider_message_sends_cancel_with_references(mocker, sa mock_cancel_broadcast.assert_called_once_with( identifier=str(broadcast_provider_message.id), + message_number=mocker.ANY, headline="GOV.UK Notify Broadcast", description='this is an emergency broadcast message', areas=[{ @@ -265,6 +268,7 @@ def test_send_broadcast_provider_message_errors(mocker, sample_service): mock_create_broadcast.assert_called_once_with( identifier=ANY, + message_number=mocker.ANY, headline="GOV.UK Notify Broadcast", description='this is an emergency broadcast message', areas=[{ diff --git a/tests/app/dao/test_broadcast_message_dao.py b/tests/app/dao/test_broadcast_message_dao.py index a8fb160b5..dd5a69ac4 100644 --- a/tests/app/dao/test_broadcast_message_dao.py +++ b/tests/app/dao/test_broadcast_message_dao.py @@ -53,7 +53,7 @@ def test_create_broadcast_provider_message_creates_in_correct_state(sample_broad transmitted_content={'body': 'Initial content'} ) - broadcast_provider_message = create_broadcast_provider_message(broadcast_event, 'fake-provider') + broadcast_provider_message, message_number = create_broadcast_provider_message(broadcast_event, 'fake-provider') assert broadcast_provider_message.status == 'sending' assert broadcast_provider_message.broadcast_event_id == broadcast_event.id diff --git a/tests/app/db.py b/tests/app/db.py index a0354e894..4dd2ec9d1 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -1074,4 +1074,4 @@ def create_broadcast_provider_message( db.session.add(provider_message_number) db.session.commit() - return provider_message + return provider_message, provider_message_number From 932a09fe5bf6d4b6e0fd440ed564f1970edf1030 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Fri, 4 Dec 2020 17:29:12 +0000 Subject: [PATCH 06/12] Pass message_number to proxy clients --- app/clients/cbc_proxy.py | 70 +++++++++-------------------- tests/app/clients/test_cbc_proxy.py | 6 +++ 2 files changed, 27 insertions(+), 49 deletions(-) diff --git a/app/clients/cbc_proxy.py b/app/clients/cbc_proxy.py index 6a730301d..2b95d2763 100644 --- a/app/clients/cbc_proxy.py +++ b/app/clients/cbc_proxy.py @@ -62,16 +62,30 @@ class CBCProxyClientBase: def send_link_test( self, identifier, - sequential_number=None + sequential_number ): - pass + """ + link test - open up a connection to a specific provider, and send them an xml payload with a of + test. + """ + payload = {'message_type': 'test', 'identifier': identifier, 'message_number': sequential_number} + + self._invoke_lambda(payload=payload) def create_and_send_broadcast( - self, - identifier, headline, description, areas, - sent, expires, + self, identifier, message_number, headline, description, areas, sent, expires, ): - pass + payload = { + 'message_type': 'alert', + 'identifier': identifier, + 'message_number': message_number, + 'headline': headline, + 'description': description, + 'areas': areas, + 'sent': sent, + 'expires': expires, + } + self._invoke_lambda(payload=payload) # We have not implementated updating a broadcast def update_and_send_broadcast( @@ -130,48 +144,6 @@ class CBCProxyCanary(CBCProxyClientBase): class CBCProxyEE(CBCProxyClientBase): lambda_name = 'bt-ee-1-proxy' - def send_link_test( - self, - identifier, - sequential_number=None - ): - """ - link test - open up a connection to a specific provider, and send them an xml payload with a of - test. - """ - payload = {'message_type': 'test', 'identifier': identifier} - - self._invoke_lambda(payload=payload) - - def create_and_send_broadcast( - self, - identifier, headline, description, areas, - sent, expires, - ): - payload = { - 'message_type': 'alert', - 'identifier': identifier, - 'headline': headline, - 'description': description, - 'areas': areas, - 'sent': sent, - 'expires': expires, - } - self._invoke_lambda(payload=payload) - class CBCProxyVodafone(CBCProxyClientBase): - lambda_name = 'bt-ee-1-proxy' - - def send_link_test( - self, - identifier, - sequential_number - ): - """ - link test - open up a connection to a specific provider, and send them an xml payload with a of - test. - """ - payload = {'message_type': 'test', 'identifier': identifier, 'message_number': sequential_number} - - self._invoke_lambda(payload=payload) + lambda_name = 'vodafone-1-proxy' diff --git a/tests/app/clients/test_cbc_proxy.py b/tests/app/clients/test_cbc_proxy.py index ed3fad3fa..31398f391 100644 --- a/tests/app/clients/test_cbc_proxy.py +++ b/tests/app/clients/test_cbc_proxy.py @@ -83,6 +83,7 @@ def test_cbc_proxy_create_and_send_invokes_function(mocker, cbc_proxy_ee): cbc_proxy_ee.create_and_send_broadcast( identifier=identifier, + message_number='0000007b', headline=headline, description=description, areas=areas, @@ -100,6 +101,7 @@ def test_cbc_proxy_create_and_send_invokes_function(mocker, cbc_proxy_ee): payload = json.loads(payload_bytes) assert payload['identifier'] == identifier + assert payload['message_number'] == '0000007b' assert payload['message_type'] == 'alert' assert payload['headline'] == headline assert payload['description'] == description @@ -141,6 +143,7 @@ def test_cbc_proxy_create_and_send_handles_invoke_error(mocker, cbc_proxy_ee): with pytest.raises(CBCProxyException) as e: cbc_proxy_ee.create_and_send_broadcast( identifier=identifier, + message_number='0000007b', headline=headline, description=description, areas=areas, @@ -190,6 +193,7 @@ def test_cbc_proxy_create_and_send_handles_function_error(mocker, cbc_proxy_ee): with pytest.raises(CBCProxyException) as e: cbc_proxy_ee.create_and_send_broadcast( identifier=identifier, + message_number='0000007b', headline=headline, description=description, areas=areas, @@ -252,6 +256,7 @@ def test_cbc_proxy_send_link_test_invokes_function(mocker, cbc_proxy_ee): cbc_proxy_ee.send_link_test( identifier=identifier, + sequential_number='0000007b' ) ld_client_mock.invoke.assert_called_once_with( @@ -266,3 +271,4 @@ def test_cbc_proxy_send_link_test_invokes_function(mocker, cbc_proxy_ee): assert payload['identifier'] == identifier assert payload['message_type'] == 'test' + assert payload['message_number'] == '0000007b' From 2952b709309a23de5349f11cbdbf798adfb467bd Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Fri, 4 Dec 2020 17:45:23 +0000 Subject: [PATCH 07/12] Only create sequential numbers for Vodafone messages --- app/celery/broadcast_message_tasks.py | 4 +++- app/dao/broadcast_message_dao.py | 12 +++++++----- tests/app/db.py | 12 +++++++----- 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/app/celery/broadcast_message_tasks.py b/app/celery/broadcast_message_tasks.py index 0cdc2800a..ce80ea54f 100644 --- a/app/celery/broadcast_message_tasks.py +++ b/app/celery/broadcast_message_tasks.py @@ -33,7 +33,9 @@ def send_broadcast_provider_message(broadcast_event_id, provider): broadcast_event = dao_get_broadcast_event_by_id(broadcast_event_id) broadcast_provider_message, message_number = create_broadcast_provider_message(broadcast_event, provider) - formatted_message_number = format_sequential_number(message_number.broadcast_provider_message_number) + formatted_message_number = None + if provider == BroadcastProvider.VODAFONE: + formatted_message_number = format_sequential_number(message_number.broadcast_provider_message_number) current_app.logger.info( f'invoking cbc proxy to send ' diff --git a/app/dao/broadcast_message_dao.py b/app/dao/broadcast_message_dao.py index e79f0a8f8..5332b420d 100644 --- a/app/dao/broadcast_message_dao.py +++ b/app/dao/broadcast_message_dao.py @@ -5,6 +5,7 @@ from app.dao.dao_utils import transactional from app.models import ( BroadcastMessage, BroadcastEvent, + BroadcastProvider, BroadcastProviderMessage, BroadcastProviderMessageNumber, BroadcastProviderMessageStatus @@ -53,9 +54,10 @@ def create_broadcast_provider_message(broadcast_event, provider): ) db.session.add(provider_message) db.session.commit() - - provider_message_number = BroadcastProviderMessageNumber( - broadcast_provider_message_id=broadcast_provider_message_id) - db.session.add(provider_message_number) - db.session.commit() + provider_message_number = None + if provider == BroadcastProvider.VODAFONE: + provider_message_number = BroadcastProviderMessageNumber( + broadcast_provider_message_id=broadcast_provider_message_id) + db.session.add(provider_message_number) + db.session.commit() return provider_message, provider_message_number diff --git a/tests/app/db.py b/tests/app/db.py index 4dd2ec9d1..8208fee49 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -63,6 +63,7 @@ from app.models import ( BroadcastMessage, BroadcastStatusType, BroadcastEvent, + BroadcastProvider, BroadcastProviderMessage, BroadcastProviderMessageNumber ) @@ -1069,9 +1070,10 @@ def create_broadcast_provider_message( db.session.add(provider_message) db.session.commit() - provider_message_number = BroadcastProviderMessageNumber( - broadcast_provider_message_id=broadcast_provider_message_id) - db.session.add(provider_message_number) - - db.session.commit() + provider_message_number = None + if provider == BroadcastProvider.VODAFONE: + provider_message_number = BroadcastProviderMessageNumber( + broadcast_provider_message_id=broadcast_provider_message_id) + db.session.add(provider_message_number) + db.session.commit() return provider_message, provider_message_number From 9e4176ac50521ccd5a5453a634e6674832240a54 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Mon, 7 Dec 2020 14:56:56 +0000 Subject: [PATCH 08/12] Add Vodafone client to list of allowed CBCs --- app/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/config.py b/app/config.py index 056e4e4a6..f9750b90c 100644 --- a/app/config.py +++ b/app/config.py @@ -378,7 +378,7 @@ class Config(object): CBC_PROXY_ENABLED = bool(CBC_PROXY_AWS_ACCESS_KEY_ID) - ENABLED_CBCS = {BroadcastProvider.EE} + ENABLED_CBCS = {BroadcastProvider.EE, BroadcastProvider.VODAFONE} ###################### From 553565bc9186e863787edb0e97c38c527b89ae3b Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Tue, 8 Dec 2020 11:12:48 +0000 Subject: [PATCH 09/12] Send message format to CBC Either cap or ibag --- app/celery/broadcast_message_tasks.py | 7 ++- app/clients/cbc_proxy.py | 17 ++++-- app/models.py | 5 ++ .../celery/test_broadcast_message_tasks.py | 60 +++++++++++++------ tests/app/clients/test_cbc_proxy.py | 8 ++- 5 files changed, 71 insertions(+), 26 deletions(-) diff --git a/app/celery/broadcast_message_tasks.py b/app/celery/broadcast_message_tasks.py index ce80ea54f..e27edbe08 100644 --- a/app/celery/broadcast_message_tasks.py +++ b/app/celery/broadcast_message_tasks.py @@ -6,7 +6,7 @@ from sqlalchemy.schema import Sequence from app import cbc_proxy_client, db, notify_celery from app.config import QueueNames -from app.models import BroadcastEventMessageType, BroadcastProvider +from app.models import BroadcastEventMessageType, BroadcastProvider, BroadcastProviderMessageType from app.dao.broadcast_message_dao import dao_get_broadcast_event_by_id, create_broadcast_provider_message from app.utils import format_sequential_number @@ -34,8 +34,10 @@ def send_broadcast_provider_message(broadcast_event_id, provider): broadcast_provider_message, message_number = create_broadcast_provider_message(broadcast_event, provider) formatted_message_number = None + message_format = BroadcastProviderMessageType.CBC if provider == BroadcastProvider.VODAFONE: formatted_message_number = format_sequential_number(message_number.broadcast_provider_message_number) + message_format = BroadcastProviderMessageType.IBAG current_app.logger.info( f'invoking cbc proxy to send ' @@ -54,6 +56,7 @@ def send_broadcast_provider_message(broadcast_event_id, provider): cbc_proxy_provider_client.create_and_send_broadcast( identifier=str(broadcast_provider_message.id), message_number=formatted_message_number, + message_format=message_format, headline="GOV.UK Notify Broadcast", description=broadcast_event.transmitted_content['body'], areas=areas, @@ -64,6 +67,7 @@ def send_broadcast_provider_message(broadcast_event_id, provider): cbc_proxy_provider_client.update_and_send_broadcast( identifier=str(broadcast_provider_message.id), message_number=formatted_message_number, + message_format=message_format, headline="GOV.UK Notify Broadcast", description=broadcast_event.transmitted_content['body'], areas=areas, @@ -75,6 +79,7 @@ def send_broadcast_provider_message(broadcast_event_id, provider): cbc_proxy_provider_client.cancel_broadcast( identifier=str(broadcast_provider_message.id), message_number=formatted_message_number, + message_format=message_format, headline="GOV.UK Notify Broadcast", description=broadcast_event.transmitted_content['body'], areas=areas, diff --git a/app/clients/cbc_proxy.py b/app/clients/cbc_proxy.py index 2b95d2763..9cf9fb2bc 100644 --- a/app/clients/cbc_proxy.py +++ b/app/clients/cbc_proxy.py @@ -62,23 +62,30 @@ class CBCProxyClientBase: def send_link_test( self, identifier, - sequential_number + sequential_number, + message_format ): """ link test - open up a connection to a specific provider, and send them an xml payload with a of test. """ - payload = {'message_type': 'test', 'identifier': identifier, 'message_number': sequential_number} + payload = { + 'message_type': 'test', + 'identifier': identifier, + 'message_number': sequential_number, + 'message_format': message_format + } self._invoke_lambda(payload=payload) def create_and_send_broadcast( - self, identifier, message_number, headline, description, areas, sent, expires, + self, identifier, message_number, message_format, headline, description, areas, sent, expires, ): payload = { 'message_type': 'alert', 'identifier': identifier, 'message_number': message_number, + 'message_format': message_format, 'headline': headline, 'description': description, 'areas': areas, @@ -91,7 +98,7 @@ class CBCProxyClientBase: def update_and_send_broadcast( self, identifier, previous_provider_messages, headline, description, areas, - sent, expires, + sent, expires, message_number, message_format ): pass @@ -99,7 +106,7 @@ class CBCProxyClientBase: def cancel_broadcast( self, identifier, previous_provider_messages, headline, description, areas, - sent, expires, + sent, expires, message_number, message_format ): pass diff --git a/app/models.py b/app/models.py index 494842b6b..e88308f99 100644 --- a/app/models.py +++ b/app/models.py @@ -2453,6 +2453,11 @@ class BroadcastProvider: PROVIDERS = [EE, VODAFONE, THREE, O2] +class BroadcastProviderMessageType: + CBC = 'cbc' + IBAG = 'ibag' + + class BroadcastProviderMessageStatus: TECHNICAL_FAILURE = 'technical-failure' # Couldn’t send (cbc proxy 5xx/4xx) SENDING = 'sending' # Sent to cbc, awaiting response diff --git a/tests/app/celery/test_broadcast_message_tasks.py b/tests/app/celery/test_broadcast_message_tasks.py index e49db47e7..547c95c84 100644 --- a/tests/app/celery/test_broadcast_message_tasks.py +++ b/tests/app/celery/test_broadcast_message_tasks.py @@ -105,7 +105,13 @@ def test_send_broadcast_event_does_nothing_if_cbc_proxy_disabled(mocker, notify_ @freeze_time('2020-08-01 12:00') -def test_send_broadcast_provider_message_sends_data_correctly(mocker, sample_service): +@pytest.mark.parametrize('provider,provider_capitalised,message_format', [ + ['ee', 'EE', 'cbc'], + ['vodafone', 'Vodafone', 'ibag'], +]) +def test_send_broadcast_provider_message_sends_data_correctly( + mocker, sample_service, provider, provider_capitalised, message_format +): template = create_template(sample_service, BROADCAST_TYPE) broadcast_message = create_broadcast_message( template, @@ -121,19 +127,20 @@ def test_send_broadcast_provider_message_sends_data_correctly(mocker, sample_ser event = create_broadcast_event(broadcast_message) mock_create_broadcast = mocker.patch( - 'app.clients.cbc_proxy.CBCProxyEE.create_and_send_broadcast', + f'app.clients.cbc_proxy.CBCProxy{provider_capitalised}.create_and_send_broadcast', ) - assert event.get_provider_message('ee') is None + assert event.get_provider_message(provider) is None - send_broadcast_provider_message(provider='ee', broadcast_event_id=str(event.id)) + send_broadcast_provider_message(provider=provider, broadcast_event_id=str(event.id)) - broadcast_provider_message = event.get_provider_message('ee') + broadcast_provider_message = event.get_provider_message(provider) assert broadcast_provider_message.status == BroadcastProviderMessageStatus.SENDING mock_create_broadcast.assert_called_once_with( identifier=str(broadcast_provider_message.id), message_number=mocker.ANY, + message_format=message_format, headline='GOV.UK Notify Broadcast', description='this is an emergency broadcast message', areas=[{ @@ -150,7 +157,13 @@ def test_send_broadcast_provider_message_sends_data_correctly(mocker, sample_ser ) -def test_send_broadcast_provider_message_sends_update_with_references(mocker, sample_service): +@pytest.mark.parametrize('provider,provider_capitalised,message_format', [ + ['ee', 'EE', 'cbc'], + ['vodafone', 'Vodafone', 'ibag'], +]) +def test_send_broadcast_provider_message_sends_update_with_references( + mocker, sample_service, provider, provider_capitalised, message_format +): template = create_template(sample_service, BROADCAST_TYPE, content='content') broadcast_message = create_broadcast_message( @@ -165,35 +178,42 @@ 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') + create_broadcast_provider_message(alert_event, provider) update_event = create_broadcast_event(broadcast_message, message_type=BroadcastEventMessageType.UPDATE) mock_update_broadcast = mocker.patch( - 'app.clients.cbc_proxy.CBCProxyEE.update_and_send_broadcast', + f'app.clients.cbc_proxy.CBCProxy{provider_capitalised}.update_and_send_broadcast', ) - send_broadcast_provider_message(provider='ee', broadcast_event_id=str(update_event.id)) + send_broadcast_provider_message(provider=provider, broadcast_event_id=str(update_event.id)) - broadcast_provider_message = update_event.get_provider_message('ee') + broadcast_provider_message = update_event.get_provider_message(provider) assert broadcast_provider_message.status == BroadcastProviderMessageStatus.SENDING mock_update_broadcast.assert_called_once_with( identifier=str(broadcast_provider_message.id), message_number=mocker.ANY, + message_format=message_format, headline="GOV.UK Notify Broadcast", description='this is an emergency broadcast message', areas=[{ "polygon": [[50.12, 1.2], [50.13, 1.2], [50.14, 1.21]], }], previous_provider_messages=[ - alert_event.get_provider_message('ee') + alert_event.get_provider_message(provider) ], sent=update_event.sent_at_as_cap_datetime_string, expires=update_event.transmitted_finishes_at_as_cap_datetime_string, ) -def test_send_broadcast_provider_message_sends_cancel_with_references(mocker, sample_service): +@pytest.mark.parametrize('provider,provider_capitalised,message_format', [ + ['ee', 'EE', 'cbc'], + ['vodafone', 'Vodafone', 'ibag'], +]) +def test_send_broadcast_provider_message_sends_cancel_with_references( + mocker, sample_service, provider, provider_capitalised, message_format +): template = create_template(sample_service, BROADCAST_TYPE, content='content') broadcast_message = create_broadcast_message( @@ -211,29 +231,30 @@ 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') + create_broadcast_provider_message(alert_event, provider) + create_broadcast_provider_message(update_event, provider) mock_cancel_broadcast = mocker.patch( - 'app.clients.cbc_proxy.CBCProxyEE.cancel_broadcast', + f'app.clients.cbc_proxy.CBCProxy{provider_capitalised}.cancel_broadcast', ) - send_broadcast_provider_message(provider='ee', broadcast_event_id=str(cancel_event.id)) + send_broadcast_provider_message(provider=provider, broadcast_event_id=str(cancel_event.id)) - broadcast_provider_message = cancel_event.get_provider_message('ee') + broadcast_provider_message = cancel_event.get_provider_message(provider) assert broadcast_provider_message.status == BroadcastProviderMessageStatus.SENDING mock_cancel_broadcast.assert_called_once_with( identifier=str(broadcast_provider_message.id), message_number=mocker.ANY, + message_format=message_format, headline="GOV.UK Notify Broadcast", description='this is an emergency broadcast message', areas=[{ "polygon": [[50.12, 1.2], [50.13, 1.2], [50.14, 1.21]], }], previous_provider_messages=[ - alert_event.get_provider_message('ee'), - update_event.get_provider_message('ee') + alert_event.get_provider_message(provider), + update_event.get_provider_message(provider) ], sent=cancel_event.sent_at_as_cap_datetime_string, expires=cancel_event.transmitted_finishes_at_as_cap_datetime_string, @@ -269,6 +290,7 @@ def test_send_broadcast_provider_message_errors(mocker, sample_service): mock_create_broadcast.assert_called_once_with( identifier=ANY, message_number=mocker.ANY, + message_format='cbc', headline="GOV.UK Notify Broadcast", description='this is an emergency broadcast message', areas=[{ diff --git a/tests/app/clients/test_cbc_proxy.py b/tests/app/clients/test_cbc_proxy.py index 31398f391..eeb242727 100644 --- a/tests/app/clients/test_cbc_proxy.py +++ b/tests/app/clients/test_cbc_proxy.py @@ -84,6 +84,7 @@ def test_cbc_proxy_create_and_send_invokes_function(mocker, cbc_proxy_ee): cbc_proxy_ee.create_and_send_broadcast( identifier=identifier, message_number='0000007b', + message_format='cbc', headline=headline, description=description, areas=areas, @@ -102,6 +103,7 @@ def test_cbc_proxy_create_and_send_invokes_function(mocker, cbc_proxy_ee): assert payload['identifier'] == identifier assert payload['message_number'] == '0000007b' + assert payload['message_format'] == 'cbc' assert payload['message_type'] == 'alert' assert payload['headline'] == headline assert payload['description'] == description @@ -144,6 +146,7 @@ def test_cbc_proxy_create_and_send_handles_invoke_error(mocker, cbc_proxy_ee): cbc_proxy_ee.create_and_send_broadcast( identifier=identifier, message_number='0000007b', + message_format='cbc', headline=headline, description=description, areas=areas, @@ -194,6 +197,7 @@ def test_cbc_proxy_create_and_send_handles_function_error(mocker, cbc_proxy_ee): cbc_proxy_ee.create_and_send_broadcast( identifier=identifier, message_number='0000007b', + message_format='cbc', headline=headline, description=description, areas=areas, @@ -256,7 +260,8 @@ def test_cbc_proxy_send_link_test_invokes_function(mocker, cbc_proxy_ee): cbc_proxy_ee.send_link_test( identifier=identifier, - sequential_number='0000007b' + sequential_number='0000007b', + message_format='cbc' ) ld_client_mock.invoke.assert_called_once_with( @@ -272,3 +277,4 @@ def test_cbc_proxy_send_link_test_invokes_function(mocker, cbc_proxy_ee): assert payload['identifier'] == identifier assert payload['message_type'] == 'test' assert payload['message_number'] == '0000007b' + assert payload['message_format'] == 'cbc' From 8af4b27fd6924b5540a996e67f31cde05a0d9585 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Wed, 9 Dec 2020 11:13:50 +0000 Subject: [PATCH 10/12] Separate functions for cbc clients Also move message_format to the clients. --- app/celery/broadcast_message_tasks.py | 7 +- app/clients/cbc_proxy.py | 96 +++++++++++---- app/models.py | 5 - .../celery/test_broadcast_message_tasks.py | 28 ++--- tests/app/clients/test_cbc_proxy.py | 111 ++++++++++++++++-- 5 files changed, 188 insertions(+), 59 deletions(-) diff --git a/app/celery/broadcast_message_tasks.py b/app/celery/broadcast_message_tasks.py index e27edbe08..ce80ea54f 100644 --- a/app/celery/broadcast_message_tasks.py +++ b/app/celery/broadcast_message_tasks.py @@ -6,7 +6,7 @@ from sqlalchemy.schema import Sequence from app import cbc_proxy_client, db, notify_celery from app.config import QueueNames -from app.models import BroadcastEventMessageType, BroadcastProvider, BroadcastProviderMessageType +from app.models import BroadcastEventMessageType, BroadcastProvider from app.dao.broadcast_message_dao import dao_get_broadcast_event_by_id, create_broadcast_provider_message from app.utils import format_sequential_number @@ -34,10 +34,8 @@ def send_broadcast_provider_message(broadcast_event_id, provider): broadcast_provider_message, message_number = create_broadcast_provider_message(broadcast_event, provider) formatted_message_number = None - message_format = BroadcastProviderMessageType.CBC if provider == BroadcastProvider.VODAFONE: formatted_message_number = format_sequential_number(message_number.broadcast_provider_message_number) - message_format = BroadcastProviderMessageType.IBAG current_app.logger.info( f'invoking cbc proxy to send ' @@ -56,7 +54,6 @@ def send_broadcast_provider_message(broadcast_event_id, provider): cbc_proxy_provider_client.create_and_send_broadcast( identifier=str(broadcast_provider_message.id), message_number=formatted_message_number, - message_format=message_format, headline="GOV.UK Notify Broadcast", description=broadcast_event.transmitted_content['body'], areas=areas, @@ -67,7 +64,6 @@ def send_broadcast_provider_message(broadcast_event_id, provider): cbc_proxy_provider_client.update_and_send_broadcast( identifier=str(broadcast_provider_message.id), message_number=formatted_message_number, - message_format=message_format, headline="GOV.UK Notify Broadcast", description=broadcast_event.transmitted_content['body'], areas=areas, @@ -79,7 +75,6 @@ def send_broadcast_provider_message(broadcast_event_id, provider): cbc_proxy_provider_client.cancel_broadcast( identifier=str(broadcast_provider_message.id), message_number=formatted_message_number, - message_format=message_format, headline="GOV.UK Notify Broadcast", description=broadcast_event.transmitted_content['body'], areas=areas, diff --git a/app/clients/cbc_proxy.py b/app/clients/cbc_proxy.py index 9cf9fb2bc..7b80b107c 100644 --- a/app/clients/cbc_proxy.py +++ b/app/clients/cbc_proxy.py @@ -65,34 +65,12 @@ class CBCProxyClientBase: sequential_number, message_format ): - """ - link test - open up a connection to a specific provider, and send them an xml payload with a of - test. - """ - payload = { - 'message_type': 'test', - 'identifier': identifier, - 'message_number': sequential_number, - 'message_format': message_format - } - - self._invoke_lambda(payload=payload) + pass def create_and_send_broadcast( self, identifier, message_number, message_format, headline, description, areas, sent, expires, ): - payload = { - 'message_type': 'alert', - 'identifier': identifier, - 'message_number': message_number, - 'message_format': message_format, - 'headline': headline, - 'description': description, - 'areas': areas, - 'sent': sent, - 'expires': expires, - } - self._invoke_lambda(payload=payload) + pass # We have not implementated updating a broadcast def update_and_send_broadcast( @@ -151,6 +129,76 @@ class CBCProxyCanary(CBCProxyClientBase): class CBCProxyEE(CBCProxyClientBase): lambda_name = 'bt-ee-1-proxy' + def send_link_test( + self, + identifier, + sequential_number=None, + ): + pass + """ + link test - open up a connection to a specific provider, and send them an xml payload with a of + test. + """ + payload = { + 'message_type': 'test', + 'identifier': identifier, + 'message_format': 'cbc' + } + + self._invoke_lambda(payload=payload) + + def create_and_send_broadcast( + self, identifier, headline, description, areas, sent, expires, message_number=None + ): + pass + payload = { + 'message_type': 'alert', + 'identifier': identifier, + 'message_format': 'cbc', + 'headline': headline, + 'description': description, + 'areas': areas, + 'sent': sent, + 'expires': expires, + } + self._invoke_lambda(payload=payload) + class CBCProxyVodafone(CBCProxyClientBase): lambda_name = 'vodafone-1-proxy' + + def send_link_test( + self, + identifier, + sequential_number, + ): + pass + """ + link test - open up a connection to a specific provider, and send them an xml payload with a of + test. + """ + payload = { + 'message_type': 'test', + 'identifier': identifier, + 'message_number': sequential_number, + 'message_format': 'ibag' + } + + self._invoke_lambda(payload=payload) + + def create_and_send_broadcast( + self, identifier, message_number, headline, description, areas, sent, expires, + ): + pass + payload = { + 'message_type': 'alert', + 'identifier': identifier, + 'message_number': message_number, + 'message_format': 'ibag', + 'headline': headline, + 'description': description, + 'areas': areas, + 'sent': sent, + 'expires': expires, + } + self._invoke_lambda(payload=payload) diff --git a/app/models.py b/app/models.py index e88308f99..494842b6b 100644 --- a/app/models.py +++ b/app/models.py @@ -2453,11 +2453,6 @@ class BroadcastProvider: PROVIDERS = [EE, VODAFONE, THREE, O2] -class BroadcastProviderMessageType: - CBC = 'cbc' - IBAG = 'ibag' - - class BroadcastProviderMessageStatus: TECHNICAL_FAILURE = 'technical-failure' # Couldn’t send (cbc proxy 5xx/4xx) SENDING = 'sending' # Sent to cbc, awaiting response diff --git a/tests/app/celery/test_broadcast_message_tasks.py b/tests/app/celery/test_broadcast_message_tasks.py index 547c95c84..7bfb9e334 100644 --- a/tests/app/celery/test_broadcast_message_tasks.py +++ b/tests/app/celery/test_broadcast_message_tasks.py @@ -105,12 +105,12 @@ def test_send_broadcast_event_does_nothing_if_cbc_proxy_disabled(mocker, notify_ @freeze_time('2020-08-01 12:00') -@pytest.mark.parametrize('provider,provider_capitalised,message_format', [ - ['ee', 'EE', 'cbc'], - ['vodafone', 'Vodafone', 'ibag'], +@pytest.mark.parametrize('provider,provider_capitalised', [ + ['ee', 'EE'], + ['vodafone', 'Vodafone'], ]) def test_send_broadcast_provider_message_sends_data_correctly( - mocker, sample_service, provider, provider_capitalised, message_format + mocker, sample_service, provider, provider_capitalised ): template = create_template(sample_service, BROADCAST_TYPE) broadcast_message = create_broadcast_message( @@ -140,7 +140,6 @@ def test_send_broadcast_provider_message_sends_data_correctly( mock_create_broadcast.assert_called_once_with( identifier=str(broadcast_provider_message.id), message_number=mocker.ANY, - message_format=message_format, headline='GOV.UK Notify Broadcast', description='this is an emergency broadcast message', areas=[{ @@ -157,12 +156,12 @@ def test_send_broadcast_provider_message_sends_data_correctly( ) -@pytest.mark.parametrize('provider,provider_capitalised,message_format', [ - ['ee', 'EE', 'cbc'], - ['vodafone', 'Vodafone', 'ibag'], +@pytest.mark.parametrize('provider,provider_capitalised', [ + ['ee', 'EE'], + ['vodafone', 'Vodafone'], ]) def test_send_broadcast_provider_message_sends_update_with_references( - mocker, sample_service, provider, provider_capitalised, message_format + mocker, sample_service, provider, provider_capitalised ): template = create_template(sample_service, BROADCAST_TYPE, content='content') @@ -193,7 +192,6 @@ def test_send_broadcast_provider_message_sends_update_with_references( mock_update_broadcast.assert_called_once_with( identifier=str(broadcast_provider_message.id), message_number=mocker.ANY, - message_format=message_format, headline="GOV.UK Notify Broadcast", description='this is an emergency broadcast message', areas=[{ @@ -207,12 +205,12 @@ def test_send_broadcast_provider_message_sends_update_with_references( ) -@pytest.mark.parametrize('provider,provider_capitalised,message_format', [ - ['ee', 'EE', 'cbc'], - ['vodafone', 'Vodafone', 'ibag'], +@pytest.mark.parametrize('provider,provider_capitalised', [ + ['ee', 'EE'], + ['vodafone', 'Vodafone'], ]) def test_send_broadcast_provider_message_sends_cancel_with_references( - mocker, sample_service, provider, provider_capitalised, message_format + mocker, sample_service, provider, provider_capitalised ): template = create_template(sample_service, BROADCAST_TYPE, content='content') @@ -246,7 +244,6 @@ def test_send_broadcast_provider_message_sends_cancel_with_references( mock_cancel_broadcast.assert_called_once_with( identifier=str(broadcast_provider_message.id), message_number=mocker.ANY, - message_format=message_format, headline="GOV.UK Notify Broadcast", description='this is an emergency broadcast message', areas=[{ @@ -290,7 +287,6 @@ def test_send_broadcast_provider_message_errors(mocker, sample_service): mock_create_broadcast.assert_called_once_with( identifier=ANY, message_number=mocker.ANY, - message_format='cbc', headline="GOV.UK Notify Broadcast", description='this is an emergency broadcast message', areas=[{ diff --git a/tests/app/clients/test_cbc_proxy.py b/tests/app/clients/test_cbc_proxy.py index eeb242727..1f0f5efaa 100644 --- a/tests/app/clients/test_cbc_proxy.py +++ b/tests/app/clients/test_cbc_proxy.py @@ -24,6 +24,11 @@ def cbc_proxy_ee(cbc_proxy_client): return cbc_proxy_client.get_proxy('ee') +@pytest.fixture +def cbc_proxy_vodafone(cbc_proxy_client): + return cbc_proxy_client.get_proxy('vodafone') + + @pytest.mark.parametrize('provider_name, expected_provider_class', [ ('ee', CBCProxyEE), ('canary', CBCProxyCanary), @@ -51,7 +56,7 @@ def test_cbc_proxy_lambda_client_has_correct_keys(cbc_proxy_ee): assert secret == 'cbc-proxy-aws-secret-access-key' -def test_cbc_proxy_create_and_send_invokes_function(mocker, cbc_proxy_ee): +def test_cbc_proxy_ee_create_and_send_invokes_function(mocker, cbc_proxy_ee): identifier = 'my-identifier' headline = 'my-headline' description = 'my-description' @@ -84,7 +89,6 @@ def test_cbc_proxy_create_and_send_invokes_function(mocker, cbc_proxy_ee): cbc_proxy_ee.create_and_send_broadcast( identifier=identifier, message_number='0000007b', - message_format='cbc', headline=headline, description=description, areas=areas, @@ -102,7 +106,7 @@ def test_cbc_proxy_create_and_send_invokes_function(mocker, cbc_proxy_ee): payload = json.loads(payload_bytes) assert payload['identifier'] == identifier - assert payload['message_number'] == '0000007b' + assert 'message_number' not in payload assert payload['message_format'] == 'cbc' assert payload['message_type'] == 'alert' assert payload['headline'] == headline @@ -112,6 +116,66 @@ def test_cbc_proxy_create_and_send_invokes_function(mocker, cbc_proxy_ee): assert payload['expires'] == expires +def test_cbc_proxy_vodafone_create_and_send_invokes_function(mocker, cbc_proxy_vodafone): + identifier = 'my-identifier' + headline = 'my-headline' + description = 'my-description' + + sent = 'a-passed-through-sent-value' + expires = 'a-passed-through-expires-value' + + # a single area which is a square including london + areas = [{ + 'description': 'london', + 'polygon': [ + [51.12, -1.2], + [51.12, 1.2], + [51.74, 1.2], + [51.74, -1.2], + [51.12, -1.2], + ], + }] + + ld_client_mock = mocker.patch.object( + cbc_proxy_vodafone, + '_lambda_client', + create=True, + ) + + ld_client_mock.invoke.return_value = { + 'StatusCode': 200, + } + + cbc_proxy_vodafone.create_and_send_broadcast( + identifier=identifier, + message_number='0000007b', + headline=headline, + description=description, + areas=areas, + sent=sent, expires=expires, + ) + + ld_client_mock.invoke.assert_called_once_with( + FunctionName='vodafone-1-proxy', + InvocationType='RequestResponse', + Payload=mocker.ANY, + ) + + kwargs = ld_client_mock.invoke.mock_calls[0][-1] + payload_bytes = kwargs['Payload'] + payload = json.loads(payload_bytes) + + assert payload['identifier'] == identifier + assert payload['message_number'] == '0000007b' + assert payload['message_format'] == 'ibag' + assert payload['message_type'] == 'alert' + assert payload['headline'] == headline + assert payload['description'] == description + assert payload['areas'] == areas + assert payload['sent'] == sent + assert payload['expires'] == expires + + def test_cbc_proxy_create_and_send_handles_invoke_error(mocker, cbc_proxy_ee): identifier = 'my-identifier' headline = 'my-headline' @@ -146,7 +210,6 @@ def test_cbc_proxy_create_and_send_handles_invoke_error(mocker, cbc_proxy_ee): cbc_proxy_ee.create_and_send_broadcast( identifier=identifier, message_number='0000007b', - message_format='cbc', headline=headline, description=description, areas=areas, @@ -197,7 +260,6 @@ def test_cbc_proxy_create_and_send_handles_function_error(mocker, cbc_proxy_ee): cbc_proxy_ee.create_and_send_broadcast( identifier=identifier, message_number='0000007b', - message_format='cbc', headline=headline, description=description, areas=areas, @@ -245,7 +307,7 @@ def test_cbc_proxy_send_canary_invokes_function(mocker, cbc_proxy_client): assert payload['identifier'] == identifier -def test_cbc_proxy_send_link_test_invokes_function(mocker, cbc_proxy_ee): +def test_cbc_proxy_ee_send_link_test_invokes_function(mocker, cbc_proxy_ee): identifier = str(uuid.uuid4()) ld_client_mock = mocker.patch.object( @@ -261,7 +323,6 @@ def test_cbc_proxy_send_link_test_invokes_function(mocker, cbc_proxy_ee): cbc_proxy_ee.send_link_test( identifier=identifier, sequential_number='0000007b', - message_format='cbc' ) ld_client_mock.invoke.assert_called_once_with( @@ -276,5 +337,39 @@ def test_cbc_proxy_send_link_test_invokes_function(mocker, cbc_proxy_ee): assert payload['identifier'] == identifier assert payload['message_type'] == 'test' - assert payload['message_number'] == '0000007b' + assert 'message_number' not in payload assert payload['message_format'] == 'cbc' + + +def test_cbc_proxy_vodafone_send_link_test_invokes_function(mocker, cbc_proxy_vodafone): + identifier = str(uuid.uuid4()) + + ld_client_mock = mocker.patch.object( + cbc_proxy_vodafone, + '_lambda_client', + create=True, + ) + + ld_client_mock.invoke.return_value = { + 'StatusCode': 200, + } + + cbc_proxy_vodafone.send_link_test( + identifier=identifier, + sequential_number='0000007b', + ) + + ld_client_mock.invoke.assert_called_once_with( + FunctionName='vodafone-1-proxy', + InvocationType='RequestResponse', + Payload=mocker.ANY, + ) + + kwargs = ld_client_mock.invoke.mock_calls[0][-1] + payload_bytes = kwargs['Payload'] + payload = json.loads(payload_bytes) + + assert payload['identifier'] == identifier + assert payload['message_type'] == 'test' + assert payload['message_number'] == '0000007b' + assert payload['message_format'] == 'ibag' From def7a167659d8cada6c3b712afed915ee8e15ad7 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Wed, 9 Dec 2020 11:41:22 +0000 Subject: [PATCH 11/12] Establish relation between provider message and message number this is so we can access brodcast_provider_message_number from BroadcastProviderMessage object --- app/celery/broadcast_message_tasks.py | 4 ++-- app/dao/broadcast_message_dao.py | 2 +- app/models.py | 5 +++++ tests/app/dao/test_broadcast_message_dao.py | 2 +- 4 files changed, 9 insertions(+), 4 deletions(-) diff --git a/app/celery/broadcast_message_tasks.py b/app/celery/broadcast_message_tasks.py index ce80ea54f..b512e0739 100644 --- a/app/celery/broadcast_message_tasks.py +++ b/app/celery/broadcast_message_tasks.py @@ -32,10 +32,10 @@ def send_broadcast_event(broadcast_event_id): def send_broadcast_provider_message(broadcast_event_id, provider): broadcast_event = dao_get_broadcast_event_by_id(broadcast_event_id) - broadcast_provider_message, message_number = create_broadcast_provider_message(broadcast_event, provider) + broadcast_provider_message = create_broadcast_provider_message(broadcast_event, provider) formatted_message_number = None if provider == BroadcastProvider.VODAFONE: - formatted_message_number = format_sequential_number(message_number.broadcast_provider_message_number) + formatted_message_number = format_sequential_number(broadcast_provider_message.message_number) current_app.logger.info( f'invoking cbc proxy to send ' diff --git a/app/dao/broadcast_message_dao.py b/app/dao/broadcast_message_dao.py index 5332b420d..f7016fdcb 100644 --- a/app/dao/broadcast_message_dao.py +++ b/app/dao/broadcast_message_dao.py @@ -60,4 +60,4 @@ def create_broadcast_provider_message(broadcast_event, provider): broadcast_provider_message_id=broadcast_provider_message_id) db.session.add(provider_message_number) db.session.commit() - return provider_message, provider_message_number + return provider_message diff --git a/app/models.py b/app/models.py index 494842b6b..c14408784 100644 --- a/app/models.py +++ b/app/models.py @@ -2483,6 +2483,8 @@ class BroadcastProviderMessage(db.Model): UniqueConstraint(broadcast_event_id, provider) + message_number = association_proxy('broadcast_provider_message_number', 'broadcast_provider_message_number') + class BroadcastProviderMessageNumber(db.Model): """ @@ -2500,6 +2502,9 @@ class BroadcastProviderMessageNumber(db.Model): broadcast_provider_message_id = db.Column( UUID(as_uuid=True), db.ForeignKey('broadcast_provider_message.id'), nullable=False ) + broadcast_provider_message = db.relationship( + 'BroadcastProviderMessage', backref=db.backref("broadcast_provider_message_number", uselist=False) + ) class ServiceBroadcastProviderRestriction(db.Model): diff --git a/tests/app/dao/test_broadcast_message_dao.py b/tests/app/dao/test_broadcast_message_dao.py index dd5a69ac4..a8fb160b5 100644 --- a/tests/app/dao/test_broadcast_message_dao.py +++ b/tests/app/dao/test_broadcast_message_dao.py @@ -53,7 +53,7 @@ def test_create_broadcast_provider_message_creates_in_correct_state(sample_broad transmitted_content={'body': 'Initial content'} ) - broadcast_provider_message, message_number = create_broadcast_provider_message(broadcast_event, 'fake-provider') + broadcast_provider_message = create_broadcast_provider_message(broadcast_event, 'fake-provider') assert broadcast_provider_message.status == 'sending' assert broadcast_provider_message.broadcast_event_id == broadcast_event.id From ac3f56f4ed0826600b9adbbf8dfe3b99ce508ac6 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Wed, 9 Dec 2020 12:31:50 +0000 Subject: [PATCH 12/12] Delete unneeded code form migration --- migrations/versions/0334_broadcast_message_number.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/migrations/versions/0334_broadcast_message_number.py b/migrations/versions/0334_broadcast_message_number.py index 0e1fd04fb..db8360f98 100644 --- a/migrations/versions/0334_broadcast_message_number.py +++ b/migrations/versions/0334_broadcast_message_number.py @@ -28,16 +28,6 @@ def upgrade(): sa.ForeignKeyConstraint(['broadcast_provider_message_id'], ['broadcast_provider_message.id'], ), sa.PrimaryKeyConstraint('broadcast_provider_message_number') ) - op.execute( - """ - INSERT INTO - broadcast_provider_message_number (broadcast_provider_message_id) - SELECT - id - FROM - broadcast_provider_message - """ - ) # ### end Alembic commands ###