From 7cc83e04eb2b780aa03e81c1d6aa0a7231163925 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Mon, 16 Nov 2020 12:47:38 +0000 Subject: [PATCH] move BroadcastProvider from models.py to config.py It's not something that is tied to a database table, and was causing circular import issues --- app/celery/broadcast_message_tasks.py | 4 +- app/clients/cbc_proxy.py | 5 +- app/config.py | 12 +++- .../celery/test_broadcast_message_tasks.py | 57 ++++++++++++++++--- tests/app/celery/test_scheduled_tasks.py | 26 ++++----- 5 files changed, 74 insertions(+), 30 deletions(-) diff --git a/app/celery/broadcast_message_tasks.py b/app/celery/broadcast_message_tasks.py index 50eb7c982..448e6e183 100644 --- a/app/celery/broadcast_message_tasks.py +++ b/app/celery/broadcast_message_tasks.py @@ -5,14 +5,14 @@ from notifications_utils.statsd_decorators import statsd from app import cbc_proxy_client, notify_celery from app.config import QueueNames -from app.models import BroadcastEventMessageType, BroadcastProvider +from app.models import BroadcastEventMessageType from app.dao.broadcast_message_dao import dao_get_broadcast_event_by_id @notify_celery.task(name="send-broadcast-event") @statsd(namespace="tasks") def send_broadcast_event(broadcast_event_id): - for provider in BroadcastProvider.PROVIDERS: + for provider in current_app.config['ENABLED_CBCS']: # TODO: Decide whether to send to each provider based on platform admin, service level settings, broadcast # level settings, etc. send_broadcast_provider_message.apply_async( diff --git a/app/clients/cbc_proxy.py b/app/clients/cbc_proxy.py index 578ddcd66..c7c231888 100644 --- a/app/clients/cbc_proxy.py +++ b/app/clients/cbc_proxy.py @@ -2,7 +2,7 @@ import json import boto3 -from app.models import BroadcastProviders +from app.config import BroadcastProvider # The variable names in this file have specific meaning in a CAP message # @@ -70,7 +70,7 @@ class CBCProxyNoopClient: class CBCProxyClient: provider_function_name_map = { - BroadcastProviders.EE: 'bt-ee-1-proxy', + BroadcastProvider.EE: 'bt-ee-1-proxy', } def init_app(self, app): @@ -111,7 +111,6 @@ class CBCProxyClient: def send_link_test( self, identifier, - provider, ): """ link test - open up a connection to a specific provider, and send them an xml payload with a of diff --git a/app/config.py b/app/config.py index 793c2473d..c6871a6b5 100644 --- a/app/config.py +++ b/app/config.py @@ -56,6 +56,15 @@ class QueueNames(object): ] +class BroadcastProvider: + EE = 'ee' + VODAFONE = 'vodafone' + THREE = 'three' + O2 = 'o2' + + PROVIDERS = [EE, VODAFONE, THREE, O2] + + class TaskNames(object): PROCESS_INCOMPLETE_JOBS = 'process-incomplete-jobs' ZIP_AND_SEND_LETTER_PDFS = 'zip-and-send-letter-pdfs' @@ -367,8 +376,7 @@ class Config(object): CBC_PROXY_AWS_ACCESS_KEY_ID = os.environ.get('CBC_PROXY_AWS_ACCESS_KEY_ID', '') CBC_PROXY_AWS_SECRET_ACCESS_KEY = os.environ.get('CBC_PROXY_AWS_SECRET_ACCESS_KEY', '') - # matches up with the strings in models.py::BroadcastProvider - ENABLED_CBCS = ['ee'] + ENABLED_CBCS = {BroadcastProvider.EE} ###################### diff --git a/tests/app/celery/test_broadcast_message_tasks.py b/tests/app/celery/test_broadcast_message_tasks.py index d6b5cdb4e..4ae6034b7 100644 --- a/tests/app/celery/test_broadcast_message_tasks.py +++ b/tests/app/celery/test_broadcast_message_tasks.py @@ -1,13 +1,33 @@ +import uuid +from unittest.mock import call + from freezegun import freeze_time import pytest from app.models import BROADCAST_TYPE, BroadcastStatusType, BroadcastEventMessageType -from app.celery.broadcast_message_tasks import send_broadcast_event +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.conftest import set_config + + +def test_send_broadcast_event_queues_up_for_active_providers(mocker, notify_api): + mock_send_broadcast_provider_message = mocker.patch( + 'app.celery.broadcast_message_tasks.send_broadcast_provider_message', + ) + + event_id = uuid.uuid4() + with set_config(notify_api, 'ENABLED_CBCS', ['ee', 'vodafone']): + send_broadcast_event(event_id) + + assert mock_send_broadcast_provider_message.apply_async.call_args_list == [ + call(kwargs={'broadcast_event_id': event_id, 'provider': 'ee'}, queue='notify-internal-tasks'), + call(kwargs={'broadcast_event_id': event_id, 'provider': 'vodafone'}, queue='notify-internal-tasks') + ] @freeze_time('2020-08-01 12:00') -def test_create_broadcast_event_sends_data_correctly(mocker, sample_service): +def test_send_broadcast_provider_message_sends_data_correctly(mocker, sample_service): template = create_template(sample_service, BROADCAST_TYPE) broadcast_message = create_broadcast_message( template, @@ -26,7 +46,7 @@ def test_create_broadcast_event_sends_data_correctly(mocker, sample_service): 'app.cbc_proxy_client.create_and_send_broadcast', ) - send_broadcast_event(broadcast_event_id=str(event.id)) + send_broadcast_provider_message(provider='ee', broadcast_event_id=str(event.id)) mock_create_broadcast.assert_called_once_with( identifier=str(event.id), @@ -46,7 +66,7 @@ def test_create_broadcast_event_sends_data_correctly(mocker, sample_service): ) -def test_update_broadcast_event_sends_references(mocker, sample_service): +def test_send_broadcast_provider_message_sends_update_with_references(mocker, sample_service): template = create_template(sample_service, BROADCAST_TYPE, content='content') broadcast_message = create_broadcast_message( @@ -67,7 +87,7 @@ def test_update_broadcast_event_sends_references(mocker, sample_service): 'app.cbc_proxy_client.update_and_send_broadcast', ) - send_broadcast_event(broadcast_event_id=str(update_event.id)) + send_broadcast_provider_message(provider='ee', broadcast_event_id=str(update_event.id)) mock_update_broadcast.assert_called_once_with( identifier=str(update_event.id), @@ -82,7 +102,7 @@ def test_update_broadcast_event_sends_references(mocker, sample_service): ) -def test_cancel_broadcast_event_sends_references(mocker, sample_service): +def test_send_broadcast_provider_message_sends_cancel_with_references(mocker, sample_service): template = create_template(sample_service, BROADCAST_TYPE, content='content') broadcast_message = create_broadcast_message( @@ -104,7 +124,7 @@ def test_cancel_broadcast_event_sends_references(mocker, sample_service): 'app.cbc_proxy_client.cancel_broadcast', ) - send_broadcast_event(broadcast_event_id=str(cancel_event.id)) + send_broadcast_provider_message(provider='ee', broadcast_event_id=str(cancel_event.id)) mock_cancel_broadcast.assert_called_once_with( identifier=str(cancel_event.id), @@ -119,7 +139,7 @@ def test_cancel_broadcast_event_sends_references(mocker, sample_service): ) -def test_send_broadcast_event_errors(mocker, sample_service): +def test_send_broadcast_provider_message_errors(mocker, sample_service): template = create_template(sample_service, BROADCAST_TYPE) broadcast_message = create_broadcast_message( @@ -141,7 +161,7 @@ def test_send_broadcast_event_errors(mocker, sample_service): ) with pytest.raises(Exception) as ex: - send_broadcast_event(broadcast_event_id=str(event.id)) + send_broadcast_provider_message(provider='ee', broadcast_event_id=str(event.id)) assert ex.match('oh no') @@ -159,3 +179,22 @@ def test_send_broadcast_event_errors(mocker, sample_service): sent=event.sent_at_as_cap_datetime_string, expires=event.transmitted_finishes_at_as_cap_datetime_string, ) + + +def test_trigger_link_tests_invokes_cbc_proxy_client( + mocker, +): + mock_send_link_test = mocker.patch( + 'app.cbc_proxy_client.send_link_test', + ) + + trigger_link_test('some-provider') + + assert mock_send_link_test.called + # the 0th argument of the call to send_link_test + identifier = mock_send_link_test.mock_calls[0][1][0] + + try: + uuid.UUID(identifier) + except BaseException: + pytest.fail(f"{identifier} is not a valid uuid") diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index c76632c2e..a5b4d9cd3 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/tests/app/celery/test_scheduled_tasks.py @@ -19,6 +19,7 @@ from app.celery.scheduled_tasks import ( check_for_missing_rows_in_completed_jobs, check_for_services_with_high_failure_rates_or_sending_to_tv_numbers, switch_current_sms_provider_on_slow_delivery, + trigger_link_tests, ) from app.config import QueueNames, Config from app.dao.jobs_dao import dao_get_job_by_id @@ -30,8 +31,8 @@ from app.models import ( NOTIFICATION_DELIVERED, NOTIFICATION_PENDING_VIRUS_CHECK, ) +from tests.conftest import set_config from tests.app import load_example_csv - from tests.app.db import ( create_notification, create_template, @@ -577,20 +578,17 @@ def test_send_canary_to_cbc_proxy_invokes_cbc_proxy_client( pytest.fail(f"{identifier} is not a valid uuid") -def test_trigger_link_tests_invokes_cbc_proxy_client( - mocker, +def test_trigger_link_tests_calls_for_all_providers( + mocker, notify_api ): - mock_send_link_test = mocker.patch( - 'app.cbc_proxy_client.send_link_test', + mock_trigger_link_test = mocker.patch( + 'app.celery.scheduled_tasks.trigger_link_test', ) - scheduled_tasks.trigger_link_tests() + with set_config(notify_api, 'ENABLED_CBCS', ['ee', 'vodafone']): + trigger_link_tests() - mock_send_link_test.assert_called - # the 0th argument of the call to send_link_test - identifier = mock_send_link_test.mock_calls[0][1][0] - - try: - uuid.UUID(identifier) - except BaseException: - pytest.fail(f"{identifier} is not a valid uuid") + assert mock_trigger_link_test.apply_async.call_args_list == [ + call(kwargs={'provider': 'ee'}, queue='notify-internal-tasks'), + call(kwargs={'provider': 'vodafone'}, queue='notify-internal-tasks') + ]