From bd06b186843451d60691e38af332412d06294316 Mon Sep 17 00:00:00 2001 From: Martyn Inglis Date: Tue, 20 Sep 2016 17:24:28 +0100 Subject: [PATCH] Refactor of celery tasks. - Aim to move the code that contacts providers into it's own module. - Celery tasks now call this module to send to provider - No exceptions caught in the new module. Celery tasks now use any exception to trigger a retry. - tests moved about - new test directory for the new class, all tests from celery test module moved, excepting the retry logic. --- app/celery/provider_tasks.py | 191 ++------- app/delivery/__init__.py | 0 app/delivery/send_to_providers.py | 139 ++++++ tests/app/celery/test_provider_tasks.py | 423 +------------------ tests/app/delivery/test_send_to_providers.py | 415 ++++++++++++++++++ 5 files changed, 586 insertions(+), 582 deletions(-) create mode 100644 app/delivery/__init__.py create mode 100644 app/delivery/send_to_providers.py create mode 100644 tests/app/delivery/test_send_to_providers.py diff --git a/app/celery/provider_tasks.py b/app/celery/provider_tasks.py index 60ed740b5..ffc5787f9 100644 --- a/app/celery/provider_tasks.py +++ b/app/celery/provider_tasks.py @@ -1,33 +1,15 @@ -from datetime import datetime -from monotonic import monotonic -from urllib.parse import urljoin - from flask import current_app -from notifications_utils.recipients import ( - validate_and_format_phone_number -) -from notifications_utils.template import Template, get_sms_fragment_count -from notifications_utils.renderers import HTMLEmail, PlainTextEmail, SMSMessage -from app import notify_celery, statsd_client, clients, create_uuid -from app.clients.email import EmailClientException -from app.clients.sms import SmsClientException -from app.dao.notifications_dao import ( - get_notification_by_id, - dao_update_notification, - update_notification_status_by_id -) -from app.dao.provider_details_dao import get_provider_details_by_notification_type -from app.dao.services_dao import dao_fetch_service_by_id -from app.celery.research_mode_tasks import send_sms_response, send_email_response -from app.dao.templates_dao import dao_get_template_by_id - -from app.models import SMS_TYPE, EMAIL_TYPE, KEY_TYPE_TEST, BRANDING_ORG +from app import notify_celery +from app.dao.notifications_dao import update_notification_status_by_id from app.statsd_decorators import statsd +from app.delivery import send_to_providers + def retry_iteration_to_delay(retry=0): """ + :param retry times we have performed a retry Given current retry calculate some delay before retrying 0: 10 seconds 1: 60 seconds (1 minutes) @@ -52,149 +34,38 @@ def retry_iteration_to_delay(retry=0): @notify_celery.task(bind=True, name="send-sms-to-provider", max_retries=5, default_retry_delay=5) @statsd(namespace="tasks") def send_sms_to_provider(self, service_id, notification_id): - service = dao_fetch_service_by_id(service_id) - provider = provider_to_use(SMS_TYPE, notification_id) - notification = get_notification_by_id(notification_id) - if notification.status == 'created': - template_model = dao_get_template_by_id(notification.template_id, notification.template_version) - template = Template( - template_model.__dict__, - values={} if not notification.personalisation else notification.personalisation, - renderer=SMSMessage(prefix=service.name, sender=service.sms_sender) - ) + try: + send_to_providers.send_sms_to_provider(notification_id) + except Exception as e: try: - if service.research_mode or notification.key_type == KEY_TYPE_TEST: - send_sms_response.apply_async( - (provider.get_name(), str(notification_id), notification.to), queue='research-mode' - ) - notification.billable_units = 0 - else: - provider.send_sms( - to=validate_and_format_phone_number(notification.to), - content=template.replaced, - reference=str(notification_id), - sender=service.sms_sender - ) - notification.billable_units = get_sms_fragment_count(template.replaced_content_count) - - notification.sent_at = datetime.utcnow() - notification.sent_by = provider.get_name() - notification.status = 'sending' - dao_update_notification(notification) - except SmsClientException as e: - try: - current_app.logger.error( - "RETRY: SMS notification {} failed".format(notification_id) - ) - current_app.logger.exception(e) - self.retry(queue="retry", countdown=retry_iteration_to_delay(self.request.retries)) - except self.MaxRetriesExceededError: - current_app.logger.error( - "RETRY FAILED: task send_sms_to_provider failed for notification {}".format(notification.id), - e - ) - update_notification_status_by_id(notification.id, 'technical-failure') - - current_app.logger.info( - "SMS {} sent to provider at {}".format(notification_id, notification.sent_at) - ) - delta_milliseconds = (datetime.utcnow() - notification.created_at).total_seconds() * 1000 - statsd_client.timing("sms.total-time", delta_milliseconds) - - -def provider_to_use(notification_type, notification_id): - active_providers_in_order = [ - provider for provider in get_provider_details_by_notification_type(notification_type) if provider.active - ] - - if not active_providers_in_order: - current_app.logger.error( - "{} {} failed as no active providers".format(notification_type, notification_id) - ) - raise Exception("No active {} providers".format(notification_type)) - - return clients.get_client_by_name_and_type(active_providers_in_order[0].identifier, notification_type) + current_app.logger.error( + "RETRY: SMS notification {} failed".format(notification_id) + ) + current_app.logger.exception(e) + self.retry(queue="retry", countdown=retry_iteration_to_delay(self.request.retries)) + except self.MaxRetriesExceededError: + current_app.logger.error( + "RETRY FAILED: task send_sms_to_provider failed for notification {}".format(notification_id), + e + ) + update_notification_status_by_id(notification_id, 'technical-failure') @notify_celery.task(bind=True, name="send-email-to-provider", max_retries=5, default_retry_delay=5) @statsd(namespace="tasks") def send_email_to_provider(self, service_id, notification_id): - service = dao_fetch_service_by_id(service_id) - provider = provider_to_use(EMAIL_TYPE, notification_id) - notification = get_notification_by_id(notification_id) - if notification.status == 'created': + try: + send_to_providers.send_email_response(notification_id) + except Exception as e: try: - template_dict = dao_get_template_by_id(notification.template_id, notification.template_version).__dict__ - - html_email = Template( - template_dict, - values=notification.personalisation, - renderer=get_html_email_renderer(service) + current_app.logger.error( + "RETRY: Email notification {} failed".format(notification_id) ) - - plain_text_email = Template( - template_dict, - values=notification.personalisation, - renderer=PlainTextEmail() + current_app.logger.exception(e) + self.retry(queue="retry", countdown=retry_iteration_to_delay(self.request.retries)) + except self.MaxRetriesExceededError: + current_app.logger.error( + "RETRY FAILED: task send_email_to_provider failed for notification {}".format(notification_id), + e ) - - if service.research_mode or notification.key_type == KEY_TYPE_TEST: - reference = str(create_uuid()) - send_email_response.apply_async( - (provider.get_name(), reference, notification.to), queue='research-mode' - ) - else: - from_address = '"{}" <{}@{}>'.format(service.name, service.email_from, - current_app.config['NOTIFY_EMAIL_DOMAIN']) - reference = provider.send_email( - from_address, - notification.to, - plain_text_email.replaced_subject, - body=plain_text_email.replaced, - html_body=html_email.replaced, - reply_to_address=service.reply_to_email_address, - ) - - notification.reference = reference - notification.sent_at = datetime.utcnow() - notification.sent_by = provider.get_name(), - notification.status = 'sending' - dao_update_notification(notification) - except EmailClientException as e: - try: - current_app.logger.error( - "RETRY: Email notification {} failed".format(notification_id) - ) - current_app.logger.exception(e) - self.retry(queue="retry", countdown=retry_iteration_to_delay(self.request.retries)) - except self.MaxRetriesExceededError: - current_app.logger.error( - "RETRY FAILED: task send_email_to_provider failed for notification {}".format(notification.id), - e - ) - update_notification_status_by_id(notification.id, 'technical-failure') - - current_app.logger.info( - "Email {} sent to provider at {}".format(notification_id, notification.sent_at) - ) - delta_milliseconds = (datetime.utcnow() - notification.created_at).total_seconds() * 1000 - statsd_client.timing("email.total-time", delta_milliseconds) - - -def get_html_email_renderer(service): - govuk_banner = service.branding != BRANDING_ORG - if service.organisation: - logo = '{}{}{}'.format( - current_app.config['ADMIN_BASE_URL'], - current_app.config['BRANDING_PATH'], - service.organisation.logo - ) - branding = { - 'brand_colour': service.organisation.colour, - 'brand_logo': logo, - 'brand_name': service.organisation.name, - } - else: - branding = {} - - return HTMLEmail(govuk_banner=govuk_banner, **branding) + update_notification_status_by_id(notification_id, 'technical-failure') diff --git a/app/delivery/__init__.py b/app/delivery/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py new file mode 100644 index 000000000..73d7fc150 --- /dev/null +++ b/app/delivery/send_to_providers.py @@ -0,0 +1,139 @@ +from datetime import datetime + +from flask import current_app +from notifications_utils.recipients import ( + validate_and_format_phone_number +) +from notifications_utils.template import Template, get_sms_fragment_count +from notifications_utils.renderers import HTMLEmail, PlainTextEmail, SMSMessage + +from app import clients, statsd_client, create_uuid +from app.dao.notifications_dao import ( + get_notification_by_id, + dao_update_notification +) +from app.dao.provider_details_dao import get_provider_details_by_notification_type +from app.dao.services_dao import dao_fetch_service_by_id +from app.celery.research_mode_tasks import send_sms_response, send_email_response +from app.dao.templates_dao import dao_get_template_by_id + +from app.models import SMS_TYPE, KEY_TYPE_TEST, BRANDING_ORG, EMAIL_TYPE + + +def send_sms_to_provider(notification_id): + notification = get_notification_by_id(notification_id) + service = dao_fetch_service_by_id(notification.service_id) + provider = provider_to_use(SMS_TYPE, notification_id) + if notification.status == 'created': + template_model = dao_get_template_by_id(notification.template_id, notification.template_version) + template = Template( + template_model.__dict__, + values={} if not notification.personalisation else notification.personalisation, + renderer=SMSMessage(prefix=service.name, sender=service.sms_sender) + ) + if service.research_mode or notification.key_type == KEY_TYPE_TEST: + send_sms_response.apply_async( + (provider.get_name(), str(notification_id), notification.to), queue='research-mode' + ) + notification.billable_units = 0 + else: + provider.send_sms( + to=validate_and_format_phone_number(notification.to), + content=template.replaced, + reference=str(notification_id), + sender=service.sms_sender + ) + notification.billable_units = get_sms_fragment_count(template.replaced_content_count) + + notification.sent_at = datetime.utcnow() + notification.sent_by = provider.get_name() + notification.status = 'sending' + dao_update_notification(notification) + + current_app.logger.info( + "SMS {} sent to provider at {}".format(notification_id, notification.sent_at) + ) + delta_milliseconds = (datetime.utcnow() - notification.created_at).total_seconds() * 1000 + statsd_client.timing("sms.total-time", delta_milliseconds) + + +def send_email_to_provider(notification_id): + notification = get_notification_by_id(notification_id) + service = dao_fetch_service_by_id(notification.service_id) + provider = provider_to_use(EMAIL_TYPE, notification_id) + if notification.status == 'created': + template_dict = dao_get_template_by_id(notification.template_id, notification.template_version).__dict__ + + html_email = Template( + template_dict, + values=notification.personalisation, + renderer=get_html_email_renderer(service) + ) + + plain_text_email = Template( + template_dict, + values=notification.personalisation, + renderer=PlainTextEmail() + ) + + if service.research_mode or notification.key_type == KEY_TYPE_TEST: + reference = str(create_uuid()) + send_email_response.apply_async( + (provider.get_name(), reference, notification.to), queue='research-mode' + ) + else: + from_address = '"{}" <{}@{}>'.format(service.name, service.email_from, + current_app.config['NOTIFY_EMAIL_DOMAIN']) + reference = provider.send_email( + from_address, + notification.to, + plain_text_email.replaced_subject, + body=plain_text_email.replaced, + html_body=html_email.replaced, + reply_to_address=service.reply_to_email_address, + ) + + notification.reference = reference + notification.sent_at = datetime.utcnow() + notification.sent_by = provider.get_name(), + notification.status = 'sending' + dao_update_notification(notification) + + current_app.logger.info( + "Email {} sent to provider at {}".format(notification_id, notification.sent_at) + ) + delta_milliseconds = (datetime.utcnow() - notification.created_at).total_seconds() * 1000 + statsd_client.timing("email.total-time", delta_milliseconds) + + +def provider_to_use(notification_type, notification_id): + active_providers_in_order = [ + provider for provider in get_provider_details_by_notification_type(notification_type) if provider.active + ] + + if not active_providers_in_order: + current_app.logger.error( + "{} {} failed as no active providers".format(notification_type, notification_id) + ) + raise Exception("No active {} providers".format(notification_type)) + + return clients.get_client_by_name_and_type(active_providers_in_order[0].identifier, notification_type) + + +def get_html_email_renderer(service): + govuk_banner = service.branding != BRANDING_ORG + if service.organisation: + logo = '{}{}{}'.format( + current_app.config['ADMIN_BASE_URL'], + current_app.config['BRANDING_PATH'], + service.organisation.logo + ) + branding = { + 'brand_colour': service.organisation.colour, + 'brand_logo': logo, + 'brand_name': service.organisation.name, + } + else: + branding = {} + + return HTMLEmail(govuk_banner=govuk_banner, **branding) diff --git a/tests/app/celery/test_provider_tasks.py b/tests/app/celery/test_provider_tasks.py index 08a6026e9..310184629 100644 --- a/tests/app/celery/test_provider_tasks.py +++ b/tests/app/celery/test_provider_tasks.py @@ -62,231 +62,6 @@ def test_should_by_240_minute_delay_on_retry_two(): assert provider_tasks.retry_iteration_to_delay(4) == 14400 -def test_should_return_highest_priority_active_provider(notify_db, notify_db_session): - providers = provider_details_dao.get_provider_details_by_notification_type('sms') - - first = providers[0] - second = providers[1] - - assert provider_tasks.provider_to_use('sms', '1234').name == first.identifier - - first.priority = 20 - second.priority = 10 - - provider_details_dao.dao_update_provider_details(first) - provider_details_dao.dao_update_provider_details(second) - - assert provider_tasks.provider_to_use('sms', '1234').name == second.identifier - - first.priority = 10 - first.active = False - second.priority = 20 - - provider_details_dao.dao_update_provider_details(first) - provider_details_dao.dao_update_provider_details(second) - - assert provider_tasks.provider_to_use('sms', '1234').name == second.identifier - - first.active = True - provider_details_dao.dao_update_provider_details(first) - - assert provider_tasks.provider_to_use('sms', '1234').name == first.identifier - - -def test_should_send_personalised_template_to_correct_sms_provider_and_persist( - notify_db, - notify_db_session, - sample_template_with_placeholders, - mocker -): - db_notification = sample_notification(notify_db, notify_db_session, template=sample_template_with_placeholders, - to_field="+447234123123", personalisation={"name": "Jo"}, - status='created') - - mocker.patch('app.mmg_client.send_sms') - mocker.patch('app.mmg_client.get_name', return_value="mmg") - - send_sms_to_provider( - db_notification.service_id, - db_notification.id - ) - - mmg_client.send_sms.assert_called_once_with( - to=format_phone_number(validate_phone_number("+447234123123")), - content="Sample service: Hello Jo\nYour thing is due soon", - reference=str(db_notification.id), - sender=None - ) - notification = Notification.query.filter_by(id=db_notification.id).one() - - assert notification.status == 'sending' - assert notification.sent_at <= datetime.utcnow() - assert notification.sent_by == 'mmg' - assert notification.billable_units == 1 - assert notification.personalisation == {"name": "Jo"} - - -def test_should_send_personalised_template_to_correct_email_provider_and_persist( - notify_db, - notify_db_session, - sample_email_template_with_placeholders, - mocker -): - db_notification = sample_notification( - notify_db=notify_db, notify_db_session=notify_db_session, - template=sample_email_template_with_placeholders, - to_field="jo.smith@example.com", - personalisation={'name': 'Jo'} - ) - - mocker.patch('app.aws_ses_client.send_email', return_value='reference') - mocker.patch('app.aws_ses_client.get_name', return_value="ses") - - send_email_to_provider( - db_notification.service_id, - db_notification.id - ) - - app.aws_ses_client.send_email.assert_called_once_with( - '"Sample service" ', - 'jo.smith@example.com', - 'Jo', - body='Hello Jo\nThis is an email from GOV.\u200bUK', - html_body=ANY, - reply_to_address=None - ) - assert ' version_on_notification - - send_sms_to_provider( - db_notification.service_id, - db_notification.id - ) - - mmg_client.send_sms.assert_called_once_with( - to=format_phone_number(validate_phone_number("+447234123123")), - content="Sample service: This is a template:\nwith a newline", - reference=str(db_notification.id), - sender=None - ) - - persisted_notification = notifications_dao.get_notification_by_id(db_notification.id) - assert persisted_notification.to == db_notification.to - assert persisted_notification.template_id == sample_template.id - assert persisted_notification.template_version == version_on_notification - assert persisted_notification.template_version != sample_template.version - assert persisted_notification.status == 'sending' - assert not persisted_notification.personalisation - - -@pytest.mark.parametrize('research_mode,key_type', [ - (True, KEY_TYPE_NORMAL), - (False, KEY_TYPE_TEST) -]) -def test_should_call_send_sms_response_task_if_research_mode(notify_db, sample_service, sample_notification, mocker, - research_mode, key_type): - mocker.patch('app.mmg_client.send_sms') - mocker.patch('app.mmg_client.get_name', return_value="mmg") - mocker.patch('app.celery.research_mode_tasks.send_sms_response.apply_async') - - if research_mode: - sample_service.research_mode = True - notify_db.session.add(sample_service) - notify_db.session.commit() - - sample_notification.key_type = key_type - - send_sms_to_provider( - sample_notification.service_id, - sample_notification.id - ) - assert not mmg_client.send_sms.called - send_sms_response.apply_async.assert_called_once_with( - ('mmg', str(sample_notification.id), sample_notification.to), queue='research-mode' - ) - - persisted_notification = notifications_dao.get_notification_by_id(sample_notification.id) - assert persisted_notification.to == sample_notification.to - assert persisted_notification.template_id == sample_notification.template_id - assert persisted_notification.status == 'sending' - assert persisted_notification.sent_at <= datetime.utcnow() - assert persisted_notification.sent_by == 'mmg' - assert not persisted_notification.personalisation - - -@pytest.mark.parametrize('research_mode,key_type', [ - (True, KEY_TYPE_NORMAL), - (False, KEY_TYPE_TEST) -]) -def test_not_should_update_provider_stats_on_success_in_research_mode(notify_db, sample_service, sample_notification, - mocker, research_mode, key_type): - provider_stats = provider_statistics_dao.get_provider_statistics(sample_service).all() - assert len(provider_stats) == 0 - - mocker.patch('app.mmg_client.send_sms') - mocker.patch('app.mmg_client.get_name', return_value="mmg") - mocker.patch('app.celery.research_mode_tasks.send_sms_response.apply_async') - if research_mode: - sample_service.research_mode = True - notify_db.session.add(sample_service) - notify_db.session.commit() - sample_notification.key_type = key_type - - send_sms_to_provider( - sample_notification.service_id, - sample_notification.id - ) - - updated_provider_stats = provider_statistics_dao.get_provider_statistics(sample_service).all() - assert len(updated_provider_stats) == 0 - - -def test_should_not_send_to_provider_when_status_is_not_created(notify_db, notify_db_session, - sample_service, - mocker): - notification = sample_notification(notify_db=notify_db, notify_db_session=notify_db_session, - service=sample_service, - status='sending') - mocker.patch('app.mmg_client.send_sms') - mocker.patch('app.mmg_client.get_name', return_value="mmg") - mocker.patch('app.celery.research_mode_tasks.send_sms_response.apply_async') - - send_sms_to_provider( - notification.service_id, - notification.id - ) - - app.mmg_client.send_sms.assert_not_called() - app.celery.research_mode_tasks.send_sms_response.apply_async.assert_not_called() - - def test_should_go_into_technical_error_if_exceeds_retries( notify_db, notify_db_session, @@ -296,7 +71,7 @@ def test_should_go_into_technical_error_if_exceeds_retries( notification = sample_notification(notify_db=notify_db, notify_db_session=notify_db_session, service=sample_service, status='created') - mocker.patch('app.mmg_client.send_sms', side_effect=SmsClientException("EXPECTED")) + mocker.patch('app.delivery.send_to_providers.send_sms_to_provider', side_effect=Exception("EXPECTED")) mocker.patch('app.celery.provider_tasks.send_sms_to_provider.retry', side_effect=MaxRetriesExceededError()) send_sms_to_provider( @@ -310,90 +85,6 @@ def test_should_go_into_technical_error_if_exceeds_retries( assert db_notification.status == 'technical-failure' -def test_should_send_sms_sender_from_service_if_present( - notify_db, - notify_db_session, - sample_service, - sample_template, - mocker): - db_notification = sample_notification(notify_db, notify_db_session, template=sample_template, - to_field="+447234123123", - status='created') - - sample_service.sms_sender = 'elevenchars' - notify_db.session.add(sample_service) - notify_db.session.commit() - - mocker.patch('app.mmg_client.send_sms') - mocker.patch('app.mmg_client.get_name', return_value="mmg") - - send_sms_to_provider( - db_notification.service_id, - db_notification.id - ) - - mmg_client.send_sms.assert_called_once_with( - to=format_phone_number(validate_phone_number("+447234123123")), - content="This is a template:\nwith a newline", - reference=str(db_notification.id), - sender=sample_service.sms_sender - ) - - -@pytest.mark.parametrize('research_mode,key_type', [ - (True, KEY_TYPE_NORMAL), - (False, KEY_TYPE_TEST) -]) -def test_send_email_to_provider_should_call_research_mode_task_response_task_if_research_mode( - notify_db, - notify_db_session, - sample_service, - sample_email_template, - ses_provider, - mocker, - research_mode, - key_type): - notification = sample_notification(notify_db=notify_db, notify_db_session=notify_db_session, - template=sample_email_template, - to_field="john@smith.com", - key_type=key_type - ) - - reference = uuid.uuid4() - mocker.patch('app.uuid.uuid4', return_value=reference) - mocker.patch('app.aws_ses_client.send_email') - mocker.patch('app.aws_ses_client.get_name', return_value="ses") - mocker.patch('app.celery.research_mode_tasks.send_email_response.apply_async') - - if research_mode: - sample_service.research_mode = True - notify_db.session.add(sample_service) - notify_db.session.commit() - assert not get_provider_statistics( - sample_email_template.service, - providers=[ses_provider.identifier]).first() - send_email_to_provider( - sample_service.id, - notification.id - ) - assert not app.aws_ses_client.send_email.called - send_email_response.apply_async.assert_called_once_with( - ('ses', str(reference), 'john@smith.com'), queue="research-mode" - ) - assert not get_provider_statistics( - sample_email_template.service, - providers=[ses_provider.identifier]).first() - persisted_notification = Notification.query.filter_by(id=notification.id).one() - - assert persisted_notification.to == 'john@smith.com' - assert persisted_notification.template_id == sample_email_template.id - assert persisted_notification.status == 'sending' - assert persisted_notification.sent_at <= datetime.utcnow() - assert persisted_notification.created_at <= datetime.utcnow() - assert persisted_notification.sent_by == 'ses' - assert persisted_notification.reference == str(reference) - - def test_send_email_to_provider_should_go_into_technical_error_if_exceeds_retries( notify_db, notify_db_session, @@ -416,115 +107,3 @@ def test_send_email_to_provider_should_go_into_technical_error_if_exceeds_retrie db_notification = Notification.query.filter_by(id=notification.id).one() assert db_notification.status == 'technical-failure' - - -def test_send_email_to_provider_should_not_send_to_provider_when_status_is_not_created(notify_db, notify_db_session, - sample_service, - sample_email_template, - mocker): - notification = sample_notification(notify_db=notify_db, notify_db_session=notify_db_session, - template=sample_email_template, - service=sample_service, - status='sending') - mocker.patch('app.aws_ses_client.send_email') - mocker.patch('app.aws_ses_client.get_name', return_value="ses") - mocker.patch('app.celery.research_mode_tasks.send_email_response.apply_async') - - send_sms_to_provider( - notification.service_id, - notification.id - ) - - app.aws_ses_client.send_email.assert_not_called() - app.celery.research_mode_tasks.send_email_response.apply_async.assert_not_called() - - -def test_send_email_should_use_service_reply_to_email( - notify_db, notify_db_session, - sample_service, - sample_email_template, - mocker): - mocker.patch('app.aws_ses_client.send_email', return_value='reference') - mocker.patch('app.aws_ses_client.get_name', return_value="ses") - - db_notification = sample_notification(notify_db, notify_db_session, template=sample_email_template) - sample_service.reply_to_email_address = 'foo@bar.com' - - send_email_to_provider( - db_notification.service_id, - db_notification.id, - ) - - app.aws_ses_client.send_email.assert_called_once_with( - ANY, - ANY, - ANY, - body=ANY, - html_body=ANY, - reply_to_address=sample_service.reply_to_email_address - ) - - -def test_should_not_set_billable_units_if_research_mode(notify_db, sample_service, sample_notification, mocker): - mocker.patch('app.mmg_client.send_sms') - mocker.patch('app.mmg_client.get_name', return_value="mmg") - mocker.patch('app.celery.research_mode_tasks.send_sms_response.apply_async') - - sample_service.research_mode = True - notify_db.session.add(sample_service) - notify_db.session.commit() - - send_sms_to_provider( - sample_notification.service_id, - sample_notification.id - ) - - persisted_notification = notifications_dao.get_notification_by_id(sample_notification.id) - assert persisted_notification.billable_units == 0 - - -def test_get_html_email_renderer_should_return_for_normal_service(sample_service): - renderer = provider_tasks.get_html_email_renderer(sample_service) - assert renderer.govuk_banner - assert renderer.brand_colour is None - assert renderer.brand_logo is None - assert renderer.brand_name is None - - -@pytest.mark.parametrize('branding_type, govuk_banner', [ - (BRANDING_ORG, False), - (BRANDING_BOTH, True) -]) -def test_get_html_email_renderer_with_branding_details(branding_type, govuk_banner, notify_db, sample_service): - sample_service.branding = branding_type - org = Organisation(colour='#000000', logo='justice-league.png', name='Justice League') - sample_service.organisation = org - notify_db.session.add_all([sample_service, org]) - notify_db.session.commit() - - renderer = provider_tasks.get_html_email_renderer(sample_service) - - assert renderer.govuk_banner == govuk_banner - assert renderer.brand_colour == '000000' - assert renderer.brand_name == 'Justice League' - - -def test_get_html_email_renderer_prepends_logo_path(notify_db, sample_service): - sample_service.branding = BRANDING_ORG - org = Organisation(colour='#000000', logo='justice-league.png', name='Justice League') - sample_service.organisation = org - notify_db.session.add_all([sample_service, org]) - notify_db.session.commit() - - renderer = provider_tasks.get_html_email_renderer(sample_service) - - assert renderer.brand_logo == 'http://localhost:6012/static/images/email-template/crests/justice-league.png' - - -def _get_provider_statistics(service, **kwargs): - query = ProviderStatistics.query.filter_by(service=service) - if 'providers' in kwargs: - providers = ProviderDetails.query.filter(ProviderDetails.identifier.in_(kwargs['providers'])).all() - provider_ids = [provider.id for provider in providers] - query = query.filter(ProviderStatistics.provider_id.in_(provider_ids)) - return query diff --git a/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py new file mode 100644 index 000000000..755f33835 --- /dev/null +++ b/tests/app/delivery/test_send_to_providers.py @@ -0,0 +1,415 @@ +import uuid +from datetime import datetime + +import pytest +from mock import ANY + +import app +from app import mmg_client +from app.dao import (provider_details_dao, notifications_dao, provider_statistics_dao) +from app.dao.provider_statistics_dao import get_provider_statistics +from app.delivery import send_to_providers +from app.models import Notification, KEY_TYPE_NORMAL, KEY_TYPE_TEST, BRANDING_ORG, BRANDING_BOTH, Organisation +from tests.app.conftest import sample_notification + +from notifications_utils.recipients import validate_phone_number, format_phone_number + + +def test_should_return_highest_priority_active_provider(notify_db, notify_db_session): + providers = provider_details_dao.get_provider_details_by_notification_type('sms') + + first = providers[0] + second = providers[1] + + assert send_to_providers.provider_to_use('sms', '1234').name == first.identifier + + first.priority = 20 + second.priority = 10 + + provider_details_dao.dao_update_provider_details(first) + provider_details_dao.dao_update_provider_details(second) + + assert send_to_providers.provider_to_use('sms', '1234').name == second.identifier + + first.priority = 10 + first.active = False + second.priority = 20 + + provider_details_dao.dao_update_provider_details(first) + provider_details_dao.dao_update_provider_details(second) + + assert send_to_providers.provider_to_use('sms', '1234').name == second.identifier + + first.active = True + provider_details_dao.dao_update_provider_details(first) + + assert send_to_providers.provider_to_use('sms', '1234').name == first.identifier + + +def test_should_send_personalised_template_to_correct_sms_provider_and_persist( + notify_db, + notify_db_session, + sample_template_with_placeholders, + mocker +): + db_notification = sample_notification(notify_db, notify_db_session, template=sample_template_with_placeholders, + to_field="+447234123123", personalisation={"name": "Jo"}, + status='created') + + mocker.patch('app.mmg_client.send_sms') + mocker.patch('app.mmg_client.get_name', return_value="mmg") + + send_to_providers.send_sms_to_provider( + db_notification.id + ) + + mmg_client.send_sms.assert_called_once_with( + to=format_phone_number(validate_phone_number("+447234123123")), + content="Sample service: Hello Jo\nYour thing is due soon", + reference=str(db_notification.id), + sender=None + ) + notification = Notification.query.filter_by(id=db_notification.id).one() + + assert notification.status == 'sending' + assert notification.sent_at <= datetime.utcnow() + assert notification.sent_by == 'mmg' + assert notification.billable_units == 1 + assert notification.personalisation == {"name": "Jo"} + + +def test_should_send_personalised_template_to_correct_email_provider_and_persist( + notify_db, + notify_db_session, + sample_email_template_with_placeholders, + mocker +): + db_notification = sample_notification( + notify_db=notify_db, notify_db_session=notify_db_session, + template=sample_email_template_with_placeholders, + to_field="jo.smith@example.com", + personalisation={'name': 'Jo'} + ) + + mocker.patch('app.aws_ses_client.send_email', return_value='reference') + mocker.patch('app.aws_ses_client.get_name', return_value="ses") + + send_to_providers.send_email_to_provider( + db_notification.id + ) + + app.aws_ses_client.send_email.assert_called_once_with( + '"Sample service" ', + 'jo.smith@example.com', + 'Jo', + body='Hello Jo\nThis is an email from GOV.\u200bUK', + html_body=ANY, + reply_to_address=None + ) + assert ' version_on_notification + + send_to_providers.send_sms_to_provider( + db_notification.id + ) + + mmg_client.send_sms.assert_called_once_with( + to=format_phone_number(validate_phone_number("+447234123123")), + content="Sample service: This is a template:\nwith a newline", + reference=str(db_notification.id), + sender=None + ) + + persisted_notification = notifications_dao.get_notification_by_id(db_notification.id) + assert persisted_notification.to == db_notification.to + assert persisted_notification.template_id == sample_template.id + assert persisted_notification.template_version == version_on_notification + assert persisted_notification.template_version != sample_template.version + assert persisted_notification.status == 'sending' + assert not persisted_notification.personalisation + + +@pytest.mark.parametrize('research_mode,key_type', [ + (True, KEY_TYPE_NORMAL), + (False, KEY_TYPE_TEST) +]) +def test_should_call_send_sms_response_task_if_research_mode(notify_db, sample_service, sample_notification, mocker, + research_mode, key_type): + mocker.patch('app.mmg_client.send_sms') + mocker.patch('app.mmg_client.get_name', return_value="mmg") + mocker.patch('app.celery.research_mode_tasks.send_sms_response.apply_async') + + if research_mode: + sample_service.research_mode = True + notify_db.session.add(sample_service) + notify_db.session.commit() + + sample_notification.key_type = key_type + + send_to_providers.send_sms_to_provider( + sample_notification.id + ) + assert not mmg_client.send_sms.called + send_to_providers.send_sms_response.apply_async.assert_called_once_with( + ('mmg', str(sample_notification.id), sample_notification.to), queue='research-mode' + ) + + persisted_notification = notifications_dao.get_notification_by_id(sample_notification.id) + assert persisted_notification.to == sample_notification.to + assert persisted_notification.template_id == sample_notification.template_id + assert persisted_notification.status == 'sending' + assert persisted_notification.sent_at <= datetime.utcnow() + assert persisted_notification.sent_by == 'mmg' + assert not persisted_notification.personalisation + + +@pytest.mark.parametrize('research_mode,key_type', [ + (True, KEY_TYPE_NORMAL), + (False, KEY_TYPE_TEST) +]) +def test_should_set_billable_units_to_zero_in_research_mode_or_test_key( + notify_db, sample_service, sample_notification, mocker, research_mode, key_type): + provider_stats = provider_statistics_dao.get_provider_statistics(sample_service).all() + assert len(provider_stats) == 0 + + mocker.patch('app.mmg_client.send_sms') + mocker.patch('app.mmg_client.get_name', return_value="mmg") + mocker.patch('app.celery.research_mode_tasks.send_sms_response.apply_async') + if research_mode: + sample_service.research_mode = True + notify_db.session.add(sample_service) + notify_db.session.commit() + sample_notification.key_type = key_type + + send_to_providers.send_sms_to_provider( + sample_notification.id + ) + + assert notifications_dao.get_notification_by_id(sample_notification.id).billable_units == 0 + + +def test_should_not_send_to_provider_when_status_is_not_created(notify_db, notify_db_session, + sample_service, + mocker): + notification = sample_notification(notify_db=notify_db, notify_db_session=notify_db_session, + service=sample_service, + status='sending') + mocker.patch('app.mmg_client.send_sms') + mocker.patch('app.mmg_client.get_name', return_value="mmg") + mocker.patch('app.celery.research_mode_tasks.send_sms_response.apply_async') + + send_to_providers.send_sms_to_provider( + notification.id + ) + + app.mmg_client.send_sms.assert_not_called() + app.celery.research_mode_tasks.send_sms_response.apply_async.assert_not_called() + + +def test_should_send_sms_sender_from_service_if_present( + notify_db, + notify_db_session, + sample_service, + sample_template, + mocker): + db_notification = sample_notification(notify_db, notify_db_session, template=sample_template, + to_field="+447234123123", + status='created') + + sample_service.sms_sender = 'elevenchars' + notify_db.session.add(sample_service) + notify_db.session.commit() + + mocker.patch('app.mmg_client.send_sms') + mocker.patch('app.mmg_client.get_name', return_value="mmg") + + send_to_providers.send_sms_to_provider( + db_notification.id + ) + + mmg_client.send_sms.assert_called_once_with( + to=format_phone_number(validate_phone_number("+447234123123")), + content="This is a template:\nwith a newline", + reference=str(db_notification.id), + sender=sample_service.sms_sender + ) + + +@pytest.mark.parametrize('research_mode,key_type', [ + (True, KEY_TYPE_NORMAL), + (False, KEY_TYPE_TEST) +]) +def test_send_email_to_provider_should_call_research_mode_task_response_task_if_research_mode( + notify_db, + notify_db_session, + sample_service, + sample_email_template, + ses_provider, + mocker, + research_mode, + key_type): + notification = sample_notification(notify_db=notify_db, notify_db_session=notify_db_session, + template=sample_email_template, + to_field="john@smith.com", + key_type=key_type + ) + + reference = uuid.uuid4() + mocker.patch('app.uuid.uuid4', return_value=reference) + mocker.patch('app.aws_ses_client.send_email') + mocker.patch('app.aws_ses_client.get_name', return_value="ses") + mocker.patch('app.celery.research_mode_tasks.send_email_response.apply_async') + + if research_mode: + sample_service.research_mode = True + notify_db.session.add(sample_service) + notify_db.session.commit() + assert not get_provider_statistics( + sample_email_template.service, + providers=[ses_provider.identifier]).first() + send_to_providers.send_email_to_provider( + notification.id + ) + assert not app.aws_ses_client.send_email.called + send_to_providers.send_email_response.apply_async.assert_called_once_with( + ('ses', str(reference), 'john@smith.com'), queue="research-mode" + ) + assert not get_provider_statistics( + sample_email_template.service, + providers=[ses_provider.identifier]).first() + persisted_notification = Notification.query.filter_by(id=notification.id).one() + + assert persisted_notification.to == 'john@smith.com' + assert persisted_notification.template_id == sample_email_template.id + assert persisted_notification.status == 'sending' + assert persisted_notification.sent_at <= datetime.utcnow() + assert persisted_notification.created_at <= datetime.utcnow() + assert persisted_notification.sent_by == 'ses' + assert persisted_notification.reference == str(reference) + + +def test_send_email_to_provider_should_not_send_to_provider_when_status_is_not_created(notify_db, notify_db_session, + sample_service, + sample_email_template, + mocker): + notification = sample_notification(notify_db=notify_db, notify_db_session=notify_db_session, + template=sample_email_template, + service=sample_service, + status='sending') + mocker.patch('app.aws_ses_client.send_email') + mocker.patch('app.aws_ses_client.get_name', return_value="ses") + mocker.patch('app.celery.research_mode_tasks.send_email_response.apply_async') + + send_to_providers.send_sms_to_provider( + notification.id + ) + + app.aws_ses_client.send_email.assert_not_called() + app.celery.research_mode_tasks.send_email_response.apply_async.assert_not_called() + + +def test_send_email_should_use_service_reply_to_email( + notify_db, notify_db_session, + sample_service, + sample_email_template, + mocker): + mocker.patch('app.aws_ses_client.send_email', return_value='reference') + mocker.patch('app.aws_ses_client.get_name', return_value="ses") + + db_notification = sample_notification(notify_db, notify_db_session, template=sample_email_template) + sample_service.reply_to_email_address = 'foo@bar.com' + + send_to_providers.send_email_to_provider( + db_notification.id, + ) + + app.aws_ses_client.send_email.assert_called_once_with( + ANY, + ANY, + ANY, + body=ANY, + html_body=ANY, + reply_to_address=sample_service.reply_to_email_address + ) + + +def test_get_html_email_renderer_should_return_for_normal_service(sample_service): + renderer = send_to_providers.get_html_email_renderer(sample_service) + assert renderer.govuk_banner + assert renderer.brand_colour is None + assert renderer.brand_logo is None + assert renderer.brand_name is None + + +@pytest.mark.parametrize('branding_type, govuk_banner', [ + (BRANDING_ORG, False), + (BRANDING_BOTH, True) +]) +def test_get_html_email_renderer_with_branding_details(branding_type, govuk_banner, notify_db, sample_service): + sample_service.branding = branding_type + org = Organisation(colour='#000000', logo='justice-league.png', name='Justice League') + sample_service.organisation = org + notify_db.session.add_all([sample_service, org]) + notify_db.session.commit() + + renderer = send_to_providers.get_html_email_renderer(sample_service) + + assert renderer.govuk_banner == govuk_banner + assert renderer.brand_colour == '000000' + assert renderer.brand_name == 'Justice League' + + +def test_get_html_email_renderer_prepends_logo_path(notify_db, sample_service): + sample_service.branding = BRANDING_ORG + org = Organisation(colour='#000000', logo='justice-league.png', name='Justice League') + sample_service.organisation = org + notify_db.session.add_all([sample_service, org]) + notify_db.session.commit() + + renderer = send_to_providers.get_html_email_renderer(sample_service) + + assert renderer.brand_logo == 'http://localhost:6012/static/images/email-template/crests/justice-league.png' + + +def test_should_not_set_billable_units_if_research_mode(notify_db, sample_service, sample_notification, mocker): + mocker.patch('app.mmg_client.send_sms') + mocker.patch('app.mmg_client.get_name', return_value="mmg") + mocker.patch('app.celery.research_mode_tasks.send_sms_response.apply_async') + + sample_service.research_mode = True + notify_db.session.add(sample_service) + notify_db.session.commit() + + send_to_providers.send_sms_to_provider( + sample_notification.id + ) + + persisted_notification = notifications_dao.get_notification_by_id(sample_notification.id) + assert persisted_notification.billable_units == 0