diff --git a/app/celery/provider_tasks.py b/app/celery/provider_tasks.py index 50cfd8506..8efcc4d92 100644 --- a/app/celery/provider_tasks.py +++ b/app/celery/provider_tasks.py @@ -9,6 +9,8 @@ from app.config import QueueNames from app.dao import notifications_dao from app.dao.notifications_dao import update_notification_status_by_id from app.delivery import send_to_providers +from app.exceptions import NotificationTechnicalFailureException +from app.models import NOTIFICATION_TECHNICAL_FAILURE @worker_process_shutdown.connect @@ -31,10 +33,10 @@ def deliver_sms(self, notification_id): ) self.retry(queue=QueueNames.RETRY) except self.MaxRetriesExceededError: - current_app.logger.exception( - "RETRY FAILED: task send_sms_to_provider failed for notification {}".format(notification_id), - ) - update_notification_status_by_id(notification_id, 'technical-failure') + message = "RETRY FAILED: Max retries reached. The task send_sms_to_provider failed for notification {}. " \ + "Notification has been updated to technical-failure".format(notification_id) + update_notification_status_by_id(notification_id, NOTIFICATION_TECHNICAL_FAILURE) + raise NotificationTechnicalFailureException(message) @notify_celery.task(bind=True, name="deliver_email", max_retries=48, default_retry_delay=300) @@ -55,7 +57,8 @@ def deliver_email(self, notification_id): ) self.retry(queue=QueueNames.RETRY) except self.MaxRetriesExceededError: - current_app.logger.error( - "RETRY FAILED: task send_email_to_provider failed for notification {}".format(notification_id) - ) - update_notification_status_by_id(notification_id, 'technical-failure') + message = "RETRY FAILED: Max retries reached. " \ + "The task send_email_to_provider failed for notification {}. " \ + "Notification has been updated to technical-failure".format(notification_id) + update_notification_status_by_id(notification_id, NOTIFICATION_TECHNICAL_FAILURE) + raise NotificationTechnicalFailureException(message) diff --git a/app/celery/scheduled_tasks.py b/app/celery/scheduled_tasks.py index 58c887c85..d863dda19 100644 --- a/app/celery/scheduled_tasks.py +++ b/app/celery/scheduled_tasks.py @@ -16,6 +16,7 @@ from app.dao.services_dao import ( dao_fetch_monthly_historical_stats_by_template ) from app.dao.stats_template_usage_by_month_dao import insert_or_update_stats_for_template +from app.exceptions import NotificationTechnicalFailureException from app.performance_platform import total_sent_notifications, processing_time from app import performance_platform_client, deskpro_client from app.dao.date_util import get_month_start_and_end_date_in_utc @@ -59,10 +60,15 @@ from app.celery.tasks import ( process_job ) from app.config import QueueNames, TaskNames -from app.utils import convert_utc_to_bst +from app.utils import ( + convert_utc_to_bst +) from app.v2.errors import JobIncompleteError from app.dao.service_callback_api_dao import get_service_callback_api_for_service -from app.celery.service_callback_tasks import send_delivery_status_to_service +from app.celery.service_callback_tasks import ( + send_delivery_status_to_service, + create_encrypted_callback_data, +) import pytz @@ -196,17 +202,25 @@ def delete_invitations(): @notify_celery.task(name='timeout-sending-notifications') @statsd(namespace="tasks") def timeout_notifications(): - notifications = dao_timeout_notifications(current_app.config.get('SENDING_NOTIFICATIONS_TIMEOUT_PERIOD')) + technical_failure_notifications, temporary_failure_notifications = \ + dao_timeout_notifications(current_app.config.get('SENDING_NOTIFICATIONS_TIMEOUT_PERIOD')) + notifications = technical_failure_notifications + temporary_failure_notifications for notification in notifications: # queue callback task only if the service_callback_api exists service_callback_api = get_service_callback_api_for_service(service_id=notification.service_id) - if service_callback_api: - send_delivery_status_to_service.apply_async([str(notification.id)], queue=QueueNames.CALLBACKS) + encrypted_notification = create_encrypted_callback_data(notification, service_callback_api) + send_delivery_status_to_service.apply_async([str(notification.id), encrypted_notification], + queue=QueueNames.CALLBACKS) current_app.logger.info( "Timeout period reached for {} notifications, status has been updated.".format(len(notifications))) + if technical_failure_notifications: + message = "{} notifications have been updated to technical-failure because they " \ + "have timed out and are still in created.Notification ids: {}".format( + len(technical_failure_notifications), [str(x.id) for x in technical_failure_notifications]) + raise NotificationTechnicalFailureException(message) @notify_celery.task(name='send-daily-performance-platform-stats') diff --git a/app/celery/service_callback_tasks.py b/app/celery/service_callback_tasks.py index 21e87266e..2da41869c 100644 --- a/app/celery/service_callback_tasks.py +++ b/app/celery/service_callback_tasks.py @@ -62,7 +62,7 @@ def send_delivery_status_to_service(self, notification_id, response.raise_for_status() except RequestException as e: current_app.logger.warning( - "send_delivery_status_to_service request failed for service_id: {} and url: {}. exc: {}".format( + "send_delivery_status_to_service request failed for notification_id: {} and url: {}. exc: {}".format( notification_id, status_update['service_callback_api_url'], e @@ -119,7 +119,7 @@ def process_update_with_notification_id(self, notification_id): response.raise_for_status() except RequestException as e: current_app.logger.warning( - "send_delivery_status_to_service request failed for service_id: {} and url: {}. exc: {}".format( + "send_delivery_status_to_service request failed for notification_id: {} and url: {}. exc: {}".format( notification_id, service_callback_api.url, e @@ -141,3 +141,21 @@ def process_update_with_notification_id(self, notification_id): """Retry: send_delivery_status_to_service has retried the max num of times for notification: {}""".format(notification_id) ) + + +def create_encrypted_callback_data(notification, service_callback_api): + from app import DATETIME_FORMAT, encryption + data = { + "notification_id": str(notification.id), + "notification_client_reference": notification.client_reference, + "notification_to": notification.to, + "notification_status": notification.status, + "notification_created_at": notification.created_at.strftime(DATETIME_FORMAT), + "notification_updated_at": + notification.updated_at.strftime(DATETIME_FORMAT) if notification.updated_at else None, + "notification_sent_at": notification.sent_at.strftime(DATETIME_FORMAT) if notification.sent_at else None, + "notification_type": notification.notification_type, + "service_callback_api_url": service_callback_api.url, + "service_callback_api_bearer_token": service_callback_api.bearer_token, + } + return encryption.encrypt(data) diff --git a/app/celery/tasks.py b/app/celery/tasks.py index d69e95ade..de626a733 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -50,7 +50,7 @@ from app.dao.provider_details_dao import get_current_provider from app.dao.service_inbound_api_dao import get_service_inbound_api_for_service from app.dao.services_dao import dao_fetch_service_by_id, fetch_todays_total_message_count from app.dao.templates_dao import dao_get_template_by_id -from app.exceptions import DVLAException +from app.exceptions import DVLAException, NotificationTechnicalFailureException from app.models import ( DVLA_RESPONSE_STATUS_SENT, EMAIL_TYPE, @@ -371,8 +371,10 @@ def update_letter_notifications_to_error(self, notification_references): 'updated_at': datetime.utcnow() } ) - - current_app.logger.debug("Updated {} letter notifications to technical-failure".format(updated_count)) + message = "Updated {} letter notifications to technical-failure with references {}".format( + updated_count, notification_references + ) + raise NotificationTechnicalFailureException(message=message) def handle_exception(task, notification, notification_id, exc): diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 37c7dddfb..2e0a77011 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -375,14 +375,15 @@ def dao_timeout_notifications(timeout_period_in_seconds): timeout = functools.partial(_timeout_notifications, timeout_start=timeout_start, updated_at=updated_at) # Notifications still in created status are marked with a technical-failure: - updated_ids = timeout([NOTIFICATION_CREATED], NOTIFICATION_TECHNICAL_FAILURE) + technical_failure_notifications = timeout([NOTIFICATION_CREATED], NOTIFICATION_TECHNICAL_FAILURE) # Notifications still in sending or pending status are marked with a temporary-failure: - updated_ids += timeout([NOTIFICATION_SENDING, NOTIFICATION_PENDING], NOTIFICATION_TEMPORARY_FAILURE) + temporary_failure_notifications = timeout([NOTIFICATION_SENDING, NOTIFICATION_PENDING], + NOTIFICATION_TEMPORARY_FAILURE) db.session.commit() - return updated_ids + return technical_failure_notifications, temporary_failure_notifications def get_total_sent_notifications_in_date_range(start_date, end_date, notification_type): diff --git a/app/delivery/send_to_providers.py b/app/delivery/send_to_providers.py index 4c1a3600b..7167202db 100644 --- a/app/delivery/send_to_providers.py +++ b/app/delivery/send_to_providers.py @@ -19,6 +19,7 @@ from app.dao.provider_details_dao import ( ) 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.exceptions import NotificationTechnicalFailureException from app.models import ( SMS_TYPE, KEY_TYPE_TEST, @@ -211,7 +212,7 @@ def get_html_email_options(service): def technical_failure(notification): notification.status = NOTIFICATION_TECHNICAL_FAILURE dao_update_notification(notification) - current_app.logger.warn( + raise NotificationTechnicalFailureException( "Send {} for notification id {} to provider is not allowed: service {} is inactive".format( notification.notification_type, notification.id, diff --git a/app/exceptions.py b/app/exceptions.py index 0fe1b7fe2..6f43f10ea 100644 --- a/app/exceptions.py +++ b/app/exceptions.py @@ -1,3 +1,8 @@ class DVLAException(Exception): def __init__(self, message): self.message = message + + +class NotificationTechnicalFailureException(Exception): + def __init__(self, message): + self.message = message diff --git a/app/notifications/notifications_ses_callback.py b/app/notifications/notifications_ses_callback.py index 99909272a..c5f65e0bd 100644 --- a/app/notifications/notifications_ses_callback.py +++ b/app/notifications/notifications_ses_callback.py @@ -12,7 +12,10 @@ from app.dao import ( ) from app.dao.service_callback_api_dao import get_service_callback_api_for_service from app.notifications.process_client_response import validate_callback_data -from app.celery.service_callback_tasks import send_delivery_status_to_service +from app.celery.service_callback_tasks import ( + send_delivery_status_to_service, + create_encrypted_callback_data, +) from app.config import QueueNames @@ -76,7 +79,7 @@ def process_ses_response(ses_request): notification.sent_at ) - _check_and_queue_callback_task(notification.id, notification.service_id) + _check_and_queue_callback_task(notification) return except KeyError: @@ -93,8 +96,10 @@ def remove_emails_from_bounce(bounce_dict): recip.pop('emailAddress') -def _check_and_queue_callback_task(notification_id, service_id): +def _check_and_queue_callback_task(notification): # queue callback task only if the service_callback_api exists - service_callback_api = get_service_callback_api_for_service(service_id=service_id) + service_callback_api = get_service_callback_api_for_service(service_id=notification.service_id) if service_callback_api: - send_delivery_status_to_service.apply_async([str(notification_id)], queue=QueueNames.CALLBACKS) + encrypted_notification = create_encrypted_callback_data(notification, service_callback_api) + send_delivery_status_to_service.apply_async([str(notification.id), encrypted_notification], + queue=QueueNames.CALLBACKS) diff --git a/app/notifications/process_client_response.py b/app/notifications/process_client_response.py index 55380b0cc..4a5cc0434 100644 --- a/app/notifications/process_client_response.py +++ b/app/notifications/process_client_response.py @@ -8,11 +8,13 @@ from app.clients import ClientException from app.dao import notifications_dao from app.clients.sms.firetext import get_firetext_responses from app.clients.sms.mmg import get_mmg_responses -from app.celery.service_callback_tasks import send_delivery_status_to_service +from app.celery.service_callback_tasks import ( + send_delivery_status_to_service, + create_encrypted_callback_data, +) from app.config import QueueNames from app.dao.service_callback_api_dao import get_service_callback_api_for_service - sms_response_mapper = { 'MMG': get_mmg_responses, 'Firetext': get_firetext_responses @@ -83,7 +85,9 @@ def _process_for_status(notification_status, client_name, reference): service_callback_api = get_service_callback_api_for_service(service_id=notification.service_id) if service_callback_api: - send_delivery_status_to_service.apply_async([str(notification.id)], queue=QueueNames.CALLBACKS) + encrypted_notification = create_encrypted_callback_data(notification, service_callback_api) + send_delivery_status_to_service.apply_async([str(notification.id), encrypted_notification], + queue=QueueNames.CALLBACKS) success = "{} callback succeeded. reference {} updated".format(client_name, reference) return success diff --git a/tests/app/celery/test_ftp_update_tasks.py b/tests/app/celery/test_ftp_update_tasks.py index 67c7d994e..de118f5dd 100644 --- a/tests/app/celery/test_ftp_update_tasks.py +++ b/tests/app/celery/test_ftp_update_tasks.py @@ -5,7 +5,7 @@ import pytest from freezegun import freeze_time from flask import current_app -from app.exceptions import DVLAException +from app.exceptions import DVLAException, NotificationTechnicalFailureException from app.models import ( Job, Notification, @@ -276,7 +276,9 @@ def test_update_letter_notifications_to_error_updates_based_on_notification_refe create_service_callback_api(service=sample_letter_template.service, url="https://original_url.com") dt = datetime.utcnow() with freeze_time(dt): - update_letter_notifications_to_error([first.reference]) + with pytest.raises(NotificationTechnicalFailureException) as e: + update_letter_notifications_to_error([first.reference]) + assert first.reference in e.value.message assert first.status == NOTIFICATION_TECHNICAL_FAILURE assert first.sent_by is None diff --git a/tests/app/celery/test_provider_tasks.py b/tests/app/celery/test_provider_tasks.py index 5b640a7ec..79cf03021 100644 --- a/tests/app/celery/test_provider_tasks.py +++ b/tests/app/celery/test_provider_tasks.py @@ -1,9 +1,11 @@ +import pytest from celery.exceptions import MaxRetriesExceededError from notifications_utils.recipients import InvalidEmailError import app from app.celery import provider_tasks from app.celery.provider_tasks import deliver_sms, deliver_email +from app.exceptions import NotificationTechnicalFailureException def test_should_have_decorated_tasks_functions(): @@ -59,21 +61,25 @@ def test_should_go_into_technical_error_if_exceeds_retries_on_deliver_sms_task(s mocker.patch('app.delivery.send_to_providers.send_sms_to_provider', side_effect=Exception("EXPECTED")) mocker.patch('app.celery.provider_tasks.deliver_sms.retry', side_effect=MaxRetriesExceededError()) - deliver_sms(sample_notification.id) + with pytest.raises(NotificationTechnicalFailureException) as e: + deliver_sms(sample_notification.id) provider_tasks.deliver_sms.retry.assert_called_with(queue="retry-tasks") assert sample_notification.status == 'technical-failure' + assert str(sample_notification.id) in e.value.message def test_should_go_into_technical_error_if_exceeds_retries_on_deliver_email_task(sample_notification, mocker): mocker.patch('app.delivery.send_to_providers.send_email_to_provider', side_effect=Exception("EXPECTED")) mocker.patch('app.celery.provider_tasks.deliver_email.retry', side_effect=MaxRetriesExceededError()) - deliver_email(sample_notification.id) + with pytest.raises(NotificationTechnicalFailureException) as e: + deliver_email(sample_notification.id) provider_tasks.deliver_email.retry.assert_called_with(queue="retry-tasks") assert sample_notification.status == 'technical-failure' + assert str(sample_notification.id) in e.value.message def test_should_technical_error_and_not_retry_if_invalid_email(sample_notification, mocker): diff --git a/tests/app/celery/test_scheduled_tasks.py b/tests/app/celery/test_scheduled_tasks.py index c7ef8ca0c..008a4bcf7 100644 --- a/tests/app/celery/test_scheduled_tasks.py +++ b/tests/app/celery/test_scheduled_tasks.py @@ -44,6 +44,7 @@ from app.dao.provider_details_dao import ( dao_update_provider_details, get_current_provider ) +from app.exceptions import NotificationTechnicalFailureException from app.models import ( MonthlyBilling, NotificationHistory, @@ -57,6 +58,7 @@ from app.models import ( SMS_TYPE ) from app.utils import get_london_midnight_in_utc +from app.celery.service_callback_tasks import create_encrypted_callback_data from app.v2.errors import JobIncompleteError from tests.app.db import ( create_notification, create_service, create_template, create_job, create_rate, @@ -180,8 +182,9 @@ def test_update_status_of_notifications_after_timeout(notify_api, sample_templat status='pending', created_at=datetime.utcnow() - timedelta( seconds=current_app.config.get('SENDING_NOTIFICATIONS_TIMEOUT_PERIOD') + 10)) - timeout_notifications() - + with pytest.raises(NotificationTechnicalFailureException) as e: + timeout_notifications() + assert str(not2.id) in e.value.message assert not1.status == 'temporary-failure' assert not2.status == 'technical-failure' assert not3.status == 'temporary-failure' @@ -210,7 +213,7 @@ def test_should_not_update_status_of_letter_notifications(client, sample_letter_ def test_timeout_notifications_sends_status_update_to_service(client, sample_template, mocker): - create_service_callback_api(service=sample_template.service) + callback_api = create_service_callback_api(service=sample_template.service) mocked = mocker.patch('app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async') notification = create_notification( template=sample_template, @@ -218,7 +221,9 @@ def test_timeout_notifications_sends_status_update_to_service(client, sample_tem created_at=datetime.utcnow() - timedelta( seconds=current_app.config.get('SENDING_NOTIFICATIONS_TIMEOUT_PERIOD') + 10)) timeout_notifications() - mocked.assert_called_once_with([str(notification.id)], queue=QueueNames.CALLBACKS) + + encrypted_data = create_encrypted_callback_data(notification, callback_api) + mocked.assert_called_once_with([str(notification.id), encrypted_data], queue=QueueNames.CALLBACKS) def test_should_update_scheduled_jobs_and_put_on_queue(notify_db, notify_db_session, mocker): diff --git a/tests/app/celery/test_service_callback_tasks.py b/tests/app/celery/test_service_callback_tasks.py index 11abf5f0f..3816d65ae 100644 --- a/tests/app/celery/test_service_callback_tasks.py +++ b/tests/app/celery/test_service_callback_tasks.py @@ -110,6 +110,29 @@ def test_send_delivery_status_to_service_does_not_retries_if_request_returns_404 assert mocked.call_count == 0 +def test_send_delivery_status_to_service_succeeds_if_sent_at_is_none( + notify_db_session, + mocker +): + callback_api, template = _set_up_test_data('email') + datestr = datetime(2017, 6, 20) + notification = create_notification(template=template, + created_at=datestr, + updated_at=datestr, + sent_at=None, + status='technical-failure' + ) + encrypted_data = _set_up_encrypted_data(callback_api, notification) + mocked = mocker.patch('app.celery.service_callback_tasks.send_delivery_status_to_service.retry') + with requests_mock.Mocker() as request_mock: + request_mock.post(callback_api.url, + json={}, + status_code=404) + send_delivery_status_to_service(notification.id, encrypted_status_update=encrypted_data) + + assert mocked.call_count == 0 + + def _set_up_test_data(notification_type): service = create_service(restricted=True) template = create_template(service=service, template_type=notification_type, subject='Hello') @@ -125,8 +148,9 @@ def _set_up_encrypted_data(callback_api, notification): "notification_to": notification.to, "notification_status": notification.status, "notification_created_at": notification.created_at.strftime(DATETIME_FORMAT), - "notification_updated_at": notification.updated_at.strftime(DATETIME_FORMAT), - "notification_sent_at": notification.sent_at.strftime(DATETIME_FORMAT), + "notification_updated_at": notification.updated_at.strftime( + DATETIME_FORMAT) if notification.updated_at else None, + "notification_sent_at": notification.sent_at.strftime(DATETIME_FORMAT) if notification.sent_at else None, "notification_type": notification.notification_type, "service_callback_api_url": callback_api.url, "service_callback_api_bearer_token": callback_api.bearer_token, diff --git a/tests/app/dao/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py index 24dffbdfa..1d6d63755 100644 --- a/tests/app/dao/notification_dao/test_notification_dao.py +++ b/tests/app/dao/notification_dao/test_notification_dao.py @@ -1222,7 +1222,7 @@ def test_dao_timeout_notifications(sample_template): assert Notification.query.get(sending.id).status == 'sending' assert Notification.query.get(pending.id).status == 'pending' assert Notification.query.get(delivered.id).status == 'delivered' - updated_ids = dao_timeout_notifications(1) + technical_failure_notifications, temporary_failure_notifications = dao_timeout_notifications(1) assert Notification.query.get(created.id).status == 'technical-failure' assert Notification.query.get(sending.id).status == 'temporary-failure' assert Notification.query.get(pending.id).status == 'temporary-failure' @@ -1231,7 +1231,7 @@ def test_dao_timeout_notifications(sample_template): assert NotificationHistory.query.get(sending.id).status == 'temporary-failure' assert NotificationHistory.query.get(pending.id).status == 'temporary-failure' assert NotificationHistory.query.get(delivered.id).status == 'delivered' - assert len(updated_ids) == 3 + assert len(technical_failure_notifications + temporary_failure_notifications) == 3 def test_dao_timeout_notifications_only_updates_for_older_notifications(sample_template): @@ -1245,12 +1245,12 @@ def test_dao_timeout_notifications_only_updates_for_older_notifications(sample_t assert Notification.query.get(sending.id).status == 'sending' assert Notification.query.get(pending.id).status == 'pending' assert Notification.query.get(delivered.id).status == 'delivered' - updated_ids = dao_timeout_notifications(1) + technical_failure_notifications, temporary_failure_notifications = dao_timeout_notifications(1) assert NotificationHistory.query.get(created.id).status == 'created' assert NotificationHistory.query.get(sending.id).status == 'sending' assert NotificationHistory.query.get(pending.id).status == 'pending' assert NotificationHistory.query.get(delivered.id).status == 'delivered' - assert len(updated_ids) == 0 + assert len(technical_failure_notifications + temporary_failure_notifications) == 0 def test_dao_timeout_notifications_doesnt_affect_letters(sample_letter_template): @@ -1265,13 +1265,13 @@ def test_dao_timeout_notifications_doesnt_affect_letters(sample_letter_template) assert Notification.query.get(pending.id).status == 'pending' assert Notification.query.get(delivered.id).status == 'delivered' - updated_ids = dao_timeout_notifications(1) + technical_failure_notifications, temporary_failure_notifications = dao_timeout_notifications(1) assert NotificationHistory.query.get(created.id).status == 'created' assert NotificationHistory.query.get(sending.id).status == 'sending' assert NotificationHistory.query.get(pending.id).status == 'pending' assert NotificationHistory.query.get(delivered.id).status == 'delivered' - assert len(updated_ids) == 0 + assert len(technical_failure_notifications + temporary_failure_notifications) == 0 def test_should_return_notifications_excluding_jobs_by_default(sample_template, sample_job, sample_api_key): diff --git a/tests/app/delivery/test_send_to_providers.py b/tests/app/delivery/test_send_to_providers.py index 627768220..4befcf7c1 100644 --- a/tests/app/delivery/test_send_to_providers.py +++ b/tests/app/delivery/test_send_to_providers.py @@ -13,6 +13,7 @@ from app import mmg_client, firetext_client from app.dao import (provider_details_dao, notifications_dao) from app.dao.provider_details_dao import dao_switch_sms_provider_to_provider_with_identifier from app.delivery import send_to_providers +from app.exceptions import NotificationTechnicalFailureException from app.models import ( Notification, EmailBranding, @@ -137,9 +138,11 @@ def test_should_not_send_email_message_when_service_is_inactive_notifcation_is_i sample_service.active = False send_mock = mocker.patch("app.aws_ses_client.send_email", return_value='reference') - send_to_providers.send_email_to_provider(sample_notification) + with pytest.raises(NotificationTechnicalFailureException) as e: + send_to_providers.send_email_to_provider(sample_notification) send_mock.assert_not_called() assert Notification.query.get(sample_notification.id).status == 'technical-failure' + assert str(sample_notification.id) in e.value.message @pytest.mark.parametrize("client_send", ["app.mmg_client.send_sms", "app.firetext_client.send_sms"]) @@ -148,9 +151,11 @@ def test_should_not_send_sms_message_when_service_is_inactive_notifcation_is_in_ sample_service.active = False send_mock = mocker.patch(client_send, return_value='reference') - send_to_providers.send_sms_to_provider(sample_notification) + with pytest.raises(NotificationTechnicalFailureException) as e: + send_to_providers.send_sms_to_provider(sample_notification) send_mock.assert_not_called() assert Notification.query.get(sample_notification.id).status == 'technical-failure' + assert str(sample_notification.id) in e.value.message def test_send_sms_should_use_template_version_from_notification_not_latest( diff --git a/tests/app/notifications/test_notifications_ses_callback.py b/tests/app/notifications/test_notifications_ses_callback.py index 515254538..2002cee82 100644 --- a/tests/app/notifications/test_notifications_ses_callback.py +++ b/tests/app/notifications/test_notifications_ses_callback.py @@ -5,8 +5,10 @@ from freezegun import freeze_time from app import statsd_client from app.dao.notifications_dao import get_notification_by_id +from app.models import Notification from app.notifications.notifications_ses_callback import process_ses_response, remove_emails_from_bounce from app.celery.research_mode_tasks import ses_hard_bounce_callback, ses_soft_bounce_callback, ses_notification_callback +from app.celery.service_callback_tasks import create_encrypted_callback_data from tests.app.conftest import sample_notification as create_sample_notification from tests.app.db import create_service_callback_api @@ -32,7 +34,8 @@ def test_ses_callback_should_update_notification_status( status='sending', sent_at=datetime.utcnow() ) - create_service_callback_api(service=sample_email_template.service, url="https://original_url.com") + callback_api = create_service_callback_api(service=sample_email_template.service, + url="https://original_url.com") assert get_notification_by_id(notification.id).status == 'sending' errors = process_ses_response(ses_notification_callback(reference='ref')) @@ -42,7 +45,9 @@ def test_ses_callback_should_update_notification_status( "callback.ses.elapsed-time", datetime.utcnow(), notification.sent_at ) statsd_client.incr.assert_any_call("callback.ses.delivered") - send_mock.assert_called_once_with([str(notification.id)], queue="service-callbacks") + updated_notification = Notification.query.get(notification.id) + encrypted_data = create_encrypted_callback_data(updated_notification, callback_api) + send_mock.assert_called_once_with([str(notification.id), encrypted_data], queue="service-callbacks") def test_ses_callback_does_not_call_send_delivery_status_if_no_db_entry( diff --git a/tests/app/notifications/test_process_client_response.py b/tests/app/notifications/test_process_client_response.py index 2ad460b3d..eb2e82cd0 100644 --- a/tests/app/notifications/test_process_client_response.py +++ b/tests/app/notifications/test_process_client_response.py @@ -7,6 +7,7 @@ from app.notifications.process_client_response import ( validate_callback_data, process_sms_client_response ) +from app.celery.service_callback_tasks import create_encrypted_callback_data from tests.app.db import create_service_callback_api @@ -57,13 +58,15 @@ def test_outcome_statistics_called_for_successful_callback(sample_notification, send_mock = mocker.patch( 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' ) - create_service_callback_api(service=sample_notification.service, url="https://original_url.com") + callback_api = create_service_callback_api(service=sample_notification.service, url="https://original_url.com") reference = str(uuid.uuid4()) success, error = process_sms_client_response(status='3', reference=reference, client_name='MMG') assert success == "MMG callback succeeded. reference {} updated".format(str(reference)) assert error is None - send_mock.assert_called_once_with([str(sample_notification.id)], queue="service-callbacks") + encrypted_data = create_encrypted_callback_data(sample_notification, callback_api) + send_mock.assert_called_once_with([str(sample_notification.id), encrypted_data], + queue="service-callbacks") def test_sms_resonse_does_not_call_send_callback_if_no_db_entry(sample_notification, mocker):