diff --git a/app/celery/nightly_tasks.py b/app/celery/nightly_tasks.py index 65b9a94d8..92d401f46 100644 --- a/app/celery/nightly_tasks.py +++ b/app/celery/nightly_tasks.py @@ -10,10 +10,6 @@ from sqlalchemy.exc import SQLAlchemyError from app import notify_celery, zendesk_client from app.aws import s3 -from app.celery.service_callback_tasks import ( - create_delivery_status_callback_data, - send_delivery_status_to_service, -) from app.config import QueueNames from app.cronitor import cronitor from app.dao.fact_processing_time_dao import insert_update_processing_time @@ -27,10 +23,6 @@ from app.dao.notifications_dao import ( dao_timeout_notifications, delete_notifications_older_than_retention_by_type, ) -from app.dao.service_callback_api_dao import ( - get_service_delivery_status_callback_api_for_service, -) -from app.exceptions import NotificationTechnicalFailureException from app.models import ( EMAIL_TYPE, KEY_TYPE_NORMAL, @@ -40,6 +32,9 @@ from app.models import ( FactProcessingTime, Notification, ) +from app.notifications.notifications_ses_callback import ( + check_and_queue_callback_task, +) from app.utils import get_london_midnight_in_utc @@ -123,25 +118,14 @@ def timeout_notifications(): # dao_timeout_notifications to return up to 100K notifications, so this task # will operate on up to 500K - normally we only get around 20K. for _ in range(0, 5): - technical_failure_notifications, temporary_failure_notifications = \ + 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_delivery_status_callback_api_for_service(service_id=notification.service_id) # noqa: E501 - if service_callback_api: - encrypted_notification = create_delivery_status_callback_data(notification, service_callback_api) - send_delivery_status_to_service.apply_async([str(notification.id), encrypted_notification], - queue=QueueNames.CALLBACKS) + check_and_queue_callback_task(notification) 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) if len(notifications) < 100000: return diff --git a/app/celery/process_ses_receipts_tasks.py b/app/celery/process_ses_receipts_tasks.py index fef0fb1f7..04a8d14f1 100644 --- a/app/celery/process_ses_receipts_tasks.py +++ b/app/celery/process_ses_receipts_tasks.py @@ -11,8 +11,8 @@ from app.config import QueueNames from app.dao import notifications_dao from app.models import NOTIFICATION_PENDING, NOTIFICATION_SENDING from app.notifications.notifications_ses_callback import ( - _check_and_queue_callback_task, _check_and_queue_complaint_callback_task, + check_and_queue_callback_task, determine_notification_bounce_type, handle_complaint, ) @@ -76,7 +76,7 @@ def process_ses_results(self, response): notification.sent_at ) - _check_and_queue_callback_task(notification) + check_and_queue_callback_task(notification) return True diff --git a/app/celery/process_sms_client_response_tasks.py b/app/celery/process_sms_client_response_tasks.py index 6cc04d7fb..e16175646 100644 --- a/app/celery/process_sms_client_response_tasks.py +++ b/app/celery/process_sms_client_response_tasks.py @@ -5,20 +5,15 @@ from flask import current_app from notifications_utils.template import SMSMessageTemplate from app import notify_celery, statsd_client -from app.celery.service_callback_tasks import ( - create_delivery_status_callback_data, - send_delivery_status_to_service, -) from app.clients import ClientException from app.clients.sms.firetext import get_firetext_responses from app.clients.sms.mmg import get_mmg_responses -from app.config import QueueNames from app.dao import notifications_dao -from app.dao.service_callback_api_dao import ( - get_service_delivery_status_callback_api_for_service, -) from app.dao.templates_dao import dao_get_template_by_id from app.models import NOTIFICATION_PENDING +from app.notifications.notifications_ses_callback import ( + check_and_queue_callback_task, +) sms_response_mapper = { 'MMG': get_mmg_responses, @@ -94,9 +89,4 @@ def _process_for_status(notification_status, client_name, provider_reference, de notifications_dao.dao_update_notification(notification) if notification_status != NOTIFICATION_PENDING: - service_callback_api = get_service_delivery_status_callback_api_for_service(service_id=notification.service_id) - # queue callback task only if the service_callback_api exists - if service_callback_api: - encrypted_notification = create_delivery_status_callback_data(notification, service_callback_api) - send_delivery_status_to_service.apply_async([str(notification.id), encrypted_notification], - queue=QueueNames.CALLBACKS) + check_and_queue_callback_task(notification) diff --git a/app/commands.py b/app/commands.py index 4edbce9d1..565b83bde 100644 --- a/app/commands.py +++ b/app/commands.py @@ -26,10 +26,6 @@ from app.celery.letters_pdf_tasks import ( from app.celery.reporting_tasks import ( create_nightly_notification_status_for_day, ) -from app.celery.service_callback_tasks import ( - create_delivery_status_callback_data, - send_delivery_status_to_service, -) from app.celery.tasks import process_row, record_daily_sorted_counts from app.config import QueueNames from app.dao.annual_billing_dao import ( @@ -52,9 +48,6 @@ from app.dao.permissions_dao import permission_dao from app.dao.provider_rates_dao import ( create_provider_rates as dao_create_provider_rates, ) -from app.dao.service_callback_api_dao import ( - get_service_delivery_status_callback_api_for_service, -) from app.dao.services_dao import ( dao_fetch_all_services_by_user, dao_fetch_all_services_created_by_user, @@ -284,44 +277,6 @@ def recreate_pdf_for_precompiled_or_uploaded_letter(notification_id): resanitise_pdf.apply_async([str(notification_id)], queue=QueueNames.LETTERS) -@notify_command(name='replay-service-callbacks') -@click.option('-f', '--file_name', required=True, - help="""Full path of the file to upload, file is a contains client references of - notifications that need the status to be sent to the service.""") -@click.option('-s', '--service_id', required=True, - help="""The service that the callbacks are for""") -def replay_service_callbacks(file_name, service_id): - print("Start send service callbacks for service: ", service_id) - callback_api = get_service_delivery_status_callback_api_for_service(service_id=service_id) - if not callback_api: - print("Callback api was not found for service: {}".format(service_id)) - return - - errors = [] - notifications = [] - file = open(file_name) - - for ref in file: - try: - notification = Notification.query.filter_by(client_reference=ref.strip()).one() - notifications.append(notification) - except NoResultFound: - errors.append("Reference: {} was not found in notifications.".format(ref)) - - for e in errors: - print(e) - if errors: - raise Exception("Some notifications for the given references were not found") - - for n in notifications: - encrypted_status_update = create_delivery_status_callback_data(n, callback_api) - send_delivery_status_to_service.apply_async([str(n.id), encrypted_status_update], - queue=QueueNames.CALLBACKS) - - print("Replay service status for service: {}. Sent {} notification status updates to the queue".format( - service_id, len(notifications))) - - def setup_commands(application): application.cli.add_command(command_group) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 4a9f028be..2ad80a226 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -1,4 +1,3 @@ -import functools from datetime import datetime, timedelta from itertools import groupby from operator import attrgetter @@ -40,7 +39,6 @@ from app.models import ( NOTIFICATION_SENDING, NOTIFICATION_SENT, NOTIFICATION_STATUS_TYPES_COMPLETED, - NOTIFICATION_TECHNICAL_FAILURE, NOTIFICATION_TEMPORARY_FAILURE, SMS_TYPE, FactNotificationStatus, @@ -491,7 +489,21 @@ def dao_delete_notifications_by_id(notification_id): ).delete(synchronize_session='fetch') -def _timeout_notifications(current_statuses, new_status, timeout_start, updated_at): +def dao_timeout_notifications(timeout_period_in_seconds): + """ + Timeout SMS and email notifications by the following rules: + + the notification was sent to the provider but there was not a delivery receipt + sending -> temporary-failure + pending -> temporary-failure + + Letter notifications are not timed out + """ + timeout_start = datetime.utcnow() - timedelta(seconds=timeout_period_in_seconds) + updated_at = datetime.utcnow() + current_statuses = [NOTIFICATION_SENDING, NOTIFICATION_PENDING] + new_status = NOTIFICATION_TEMPORARY_FAILURE + # TEMPORARY: limit the notifications to 100K as otherwise we # see an issues where the task vanishes after it starts executing # - we believe this is a OOM error but there are no logs. From @@ -511,36 +523,9 @@ def _timeout_notifications(current_statuses, new_status, timeout_start, updated_ {'status': new_status, 'updated_at': updated_at}, synchronize_session=False ) - return notifications - - -def dao_timeout_notifications(timeout_period_in_seconds): - """ - Timeout SMS and email notifications by the following rules: - - we never sent the notification to the provider for some reason - created -> technical-failure - - the notification was sent to the provider but there was not a delivery receipt - sending -> temporary-failure - pending -> temporary-failure - - Letter notifications are not timed out - """ - timeout_start = datetime.utcnow() - timedelta(seconds=timeout_period_in_seconds) - updated_at = datetime.utcnow() - timeout = functools.partial(_timeout_notifications, timeout_start=timeout_start, updated_at=updated_at) - - # Notifications still in created status are marked with a technical-failure: - technical_failure_notifications = timeout([NOTIFICATION_CREATED], NOTIFICATION_TECHNICAL_FAILURE) - - # Notifications still in sending or pending status are marked with a temporary-failure: - temporary_failure_notifications = timeout([NOTIFICATION_SENDING, NOTIFICATION_PENDING], - NOTIFICATION_TEMPORARY_FAILURE) db.session.commit() - - return technical_failure_notifications, temporary_failure_notifications + return notifications def is_delivery_slow_for_providers( diff --git a/app/notifications/notifications_ses_callback.py b/app/notifications/notifications_ses_callback.py index 484d78f30..c1fdceaa0 100644 --- a/app/notifications/notifications_ses_callback.py +++ b/app/notifications/notifications_ses_callback.py @@ -68,7 +68,7 @@ def remove_emails_from_complaint(complaint_dict): return complaint_dict['mail'].pop('destination') -def _check_and_queue_callback_task(notification): +def check_and_queue_callback_task(notification): # queue callback task only if the service_callback_api exists service_callback_api = get_service_delivery_status_callback_api_for_service(service_id=notification.service_id) if service_callback_api: diff --git a/tests/app/celery/test_nightly_tasks.py b/tests/app/celery/test_nightly_tasks.py index 8813b92fd..7a6167aa2 100644 --- a/tests/app/celery/test_nightly_tasks.py +++ b/tests/app/celery/test_nightly_tasks.py @@ -24,17 +24,11 @@ from app.celery.nightly_tasks import ( save_daily_notification_processing_time, timeout_notifications, ) -from app.celery.service_callback_tasks import ( - create_delivery_status_callback_data, -) -from app.config import QueueNames -from app.exceptions import NotificationTechnicalFailureException from app.models import EMAIL_TYPE, LETTER_TYPE, SMS_TYPE, FactProcessingTime from tests.app.db import ( create_job, create_notification, create_service, - create_service_callback_api, create_service_data_retention, create_template, ) @@ -148,87 +142,40 @@ def test_remove_csv_files_filters_by_type(mocker, sample_service): ] -def test_should_call_delete_sms_notifications_more_than_week_in_task(notify_api, mocker): +def test_delete_sms_notifications_older_than_retention_calls_child_task(notify_api, mocker): mocked = mocker.patch('app.celery.nightly_tasks.delete_notifications_older_than_retention_by_type') delete_sms_notifications_older_than_retention() mocked.assert_called_once_with('sms') -def test_should_call_delete_email_notifications_more_than_week_in_task(notify_api, mocker): +def test_delete_email_notifications_older_than_retentions_calls_child_task(notify_api, mocker): mocked_notifications = mocker.patch( 'app.celery.nightly_tasks.delete_notifications_older_than_retention_by_type') delete_email_notifications_older_than_retention() mocked_notifications.assert_called_once_with('email') -def test_should_call_delete_letter_notifications_more_than_week_in_task(notify_api, mocker): +def test_delete_letter_notifications_older_than_retention_calls_child_task(notify_api, mocker): mocked = mocker.patch('app.celery.nightly_tasks.delete_notifications_older_than_retention_by_type') delete_letter_notifications_older_than_retention() mocked.assert_called_once_with('letter') -def test_update_status_of_notifications_after_timeout(notify_api, sample_template): - with notify_api.test_request_context(): - not1 = create_notification( - template=sample_template, - status='sending', - created_at=datetime.utcnow() - timedelta( - seconds=current_app.config.get('SENDING_NOTIFICATIONS_TIMEOUT_PERIOD') + 10)) - not2 = create_notification( - template=sample_template, - status='created', - created_at=datetime.utcnow() - timedelta( - seconds=current_app.config.get('SENDING_NOTIFICATIONS_TIMEOUT_PERIOD') + 10)) - not3 = create_notification( - template=sample_template, - status='pending', - created_at=datetime.utcnow() - timedelta( - seconds=current_app.config.get('SENDING_NOTIFICATIONS_TIMEOUT_PERIOD') + 10)) - with pytest.raises(NotificationTechnicalFailureException) as e: - timeout_notifications() - assert str(not2.id) in str(e.value) - assert not1.status == 'temporary-failure' - assert not2.status == 'technical-failure' - assert not3.status == 'temporary-failure' - - -def test_not_update_status_of_notification_before_timeout(notify_api, sample_template): - with notify_api.test_request_context(): - not1 = create_notification( - template=sample_template, - status='sending', - created_at=datetime.utcnow() - timedelta( - seconds=current_app.config.get('SENDING_NOTIFICATIONS_TIMEOUT_PERIOD') - 10)) - timeout_notifications() - assert not1.status == 'sending' - - -def test_should_not_update_status_of_letter_notifications(client, sample_letter_template): - created_at = datetime.utcnow() - timedelta(days=5) - not1 = create_notification(template=sample_letter_template, status='sending', created_at=created_at) - not2 = create_notification(template=sample_letter_template, status='created', created_at=created_at) +def test_timeout_notifications(mocker, sample_notification): + mock_update = mocker.patch('app.celery.nightly_tasks.check_and_queue_callback_task') + mock_dao = mocker.patch('app.celery.nightly_tasks.dao_timeout_notifications') + mock_dao.return_value = [sample_notification] timeout_notifications() - assert not1.status == 'sending' - assert not2.status == 'created' + mock_dao.assert_called_once_with( + current_app.config.get('SENDING_NOTIFICATIONS_TIMEOUT_PERIOD') + ) + + mock_update.assert_called_once_with(sample_notification) -def test_timeout_notifications_sends_status_update_to_service(client, sample_template, mocker): - 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, - status='sending', - created_at=datetime.utcnow() - timedelta( - seconds=current_app.config.get('SENDING_NOTIFICATIONS_TIMEOUT_PERIOD') + 10)) - timeout_notifications() - - encrypted_data = create_delivery_status_callback_data(notification, callback_api) - mocked.assert_called_once_with([str(notification.id), encrypted_data], queue=QueueNames.CALLBACKS) - - -def test_should_call_delete_inbound_sms(notify_api, mocker): +def test_delete_inbound_sms_calls_child_task(notify_api, mocker): mocker.patch('app.celery.nightly_tasks.delete_inbound_sms_older_than_retention') delete_inbound_sms() assert nightly_tasks.delete_inbound_sms_older_than_retention.call_count == 1 diff --git a/tests/app/celery/test_process_ses_receipts_tasks.py b/tests/app/celery/test_process_ses_receipts_tasks.py index ee8c2081b..290b1029b 100644 --- a/tests/app/celery/test_process_ses_receipts_tasks.py +++ b/tests/app/celery/test_process_ses_receipts_tasks.py @@ -10,9 +10,6 @@ from app.celery.research_mode_tasks import ( ses_notification_callback, ses_soft_bounce_callback, ) -from app.celery.service_callback_tasks import ( - create_delivery_status_callback_data, -) from app.dao.notifications_dao import get_notification_by_id from app.models import Complaint, Notification from app.notifications.notifications_ses_callback import ( @@ -72,15 +69,13 @@ def test_ses_callback_should_update_notification_status( mocker.patch('app.statsd_client.incr') mocker.patch('app.statsd_client.timing_with_dates') send_mock = mocker.patch( - 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + 'app.celery.process_ses_receipts_tasks.check_and_queue_callback_task' ) notification = create_notification( template=sample_email_template, status='sending', reference='ref', ) - 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' assert process_ses_results(ses_notification_callback(reference='ref')) @@ -90,8 +85,7 @@ def test_ses_callback_should_update_notification_status( ) statsd_client.incr.assert_any_call("callback.ses.delivered") updated_notification = Notification.query.get(notification.id) - encrypted_data = create_delivery_status_callback_data(updated_notification, callback_api) - send_mock.assert_called_once_with([str(notification.id), encrypted_data], queue="service-callbacks") + send_mock.assert_called_once_with(updated_notification) def test_ses_callback_should_not_update_notification_status_if_already_delivered(sample_email_template, mocker): @@ -138,30 +132,6 @@ def test_ses_callback_should_not_retry_if_notification_is_old(client, notify_db, assert mock_retry.call_count == 0 -def test_ses_callback_does_not_call_send_delivery_status_if_no_db_entry( - client, - notify_db_session, - sample_email_template, - mocker): - with freeze_time('2001-01-01T12:00:00'): - - send_mock = mocker.patch( - 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' - ) - notification = create_notification( - template=sample_email_template, - status='sending', - reference='ref', - ) - - assert get_notification_by_id(notification.id).status == 'sending' - - assert process_ses_results(ses_notification_callback(reference='ref')) - assert get_notification_by_id(notification.id).status == 'delivered' - - send_mock.assert_not_called() - - def test_ses_callback_should_update_multiple_notification_status_sent( client, notify_db_session, @@ -169,7 +139,7 @@ def test_ses_callback_should_update_multiple_notification_status_sent( mocker): send_mock = mocker.patch( - 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + 'app.celery.process_ses_receipts_tasks.check_and_queue_callback_task' ) create_notification( template=sample_email_template, @@ -186,7 +156,6 @@ def test_ses_callback_should_update_multiple_notification_status_sent( status='sending', reference='ref3', ) - create_service_callback_api(service=sample_email_template.service, url="https://original_url.com") assert process_ses_results(ses_notification_callback(reference='ref1')) assert process_ses_results(ses_notification_callback(reference='ref2')) assert process_ses_results(ses_notification_callback(reference='ref3')) @@ -198,7 +167,7 @@ def test_ses_callback_should_set_status_to_temporary_failure(client, sample_email_template, mocker): send_mock = mocker.patch( - 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + 'app.celery.process_ses_receipts_tasks.check_and_queue_callback_task' ) mock_logger = mocker.patch('app.celery.process_ses_receipts_tasks.current_app.logger.info') notification = create_notification( @@ -206,7 +175,6 @@ def test_ses_callback_should_set_status_to_temporary_failure(client, status='sending', reference='ref', ) - create_service_callback_api(service=notification.service, url="https://original_url.com") assert get_notification_by_id(notification.id).status == 'sending' assert process_ses_results(ses_soft_bounce_callback(reference='ref')) assert get_notification_by_id(notification.id).status == 'temporary-failure' @@ -219,7 +187,7 @@ def test_ses_callback_should_set_status_to_permanent_failure(client, sample_email_template, mocker): send_mock = mocker.patch( - 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + 'app.celery.process_ses_receipts_tasks.check_and_queue_callback_task' ) mock_logger = mocker.patch('app.celery.process_ses_receipts_tasks.current_app.logger.info') notification = create_notification( @@ -227,7 +195,6 @@ def test_ses_callback_should_set_status_to_permanent_failure(client, status='sending', reference='ref', ) - create_service_callback_api(service=sample_email_template.service, url="https://original_url.com") assert get_notification_by_id(notification.id).status == 'sending' assert process_ses_results(ses_hard_bounce_callback(reference='ref')) diff --git a/tests/app/dao/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py index 66b54bbf2..f81899c21 100644 --- a/tests/app/dao/notification_dao/test_notification_dao.py +++ b/tests/app/dao/notification_dao/test_notification_dao.py @@ -675,12 +675,12 @@ 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' - technical_failure_notifications, temporary_failure_notifications = dao_timeout_notifications(1) - assert Notification.query.get(created.id).status == 'technical-failure' + temporary_failure_notifications = dao_timeout_notifications(1) + assert Notification.query.get(created.id).status == 'created' assert Notification.query.get(sending.id).status == 'temporary-failure' assert Notification.query.get(pending.id).status == 'temporary-failure' assert Notification.query.get(delivered.id).status == 'delivered' - assert len(technical_failure_notifications + temporary_failure_notifications) == 3 + assert len(temporary_failure_notifications) == 2 def test_dao_timeout_notifications_only_updates_for_older_notifications(sample_template): @@ -694,8 +694,8 @@ 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' - technical_failure_notifications, temporary_failure_notifications = dao_timeout_notifications(1) - assert len(technical_failure_notifications + temporary_failure_notifications) == 0 + temporary_failure_notifications = dao_timeout_notifications(1) + assert len(temporary_failure_notifications) == 0 def test_dao_timeout_notifications_doesnt_affect_letters(sample_letter_template): @@ -709,8 +709,8 @@ def test_dao_timeout_notifications_doesnt_affect_letters(sample_letter_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' - - technical_failure_notifications, temporary_failure_notifications = dao_timeout_notifications(1) + temporary_failure_notifications = dao_timeout_notifications(1) + assert len(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/notifications/test_notifications_ses_callback.py b/tests/app/notifications/test_notifications_ses_callback.py index 179aa5564..8e6409eeb 100644 --- a/tests/app/notifications/test_notifications_ses_callback.py +++ b/tests/app/notifications/test_notifications_ses_callback.py @@ -4,10 +4,14 @@ from sqlalchemy.exc import SQLAlchemyError from app.dao.notifications_dao import get_notification_by_id from app.models import Complaint -from app.notifications.notifications_ses_callback import handle_complaint +from app.notifications.notifications_ses_callback import ( + check_and_queue_callback_task, + handle_complaint, +) from tests.app.db import ( create_notification, create_notification_history, + create_service_callback_api, ses_complaint_callback, ses_complaint_callback_malformed_message_id, ses_complaint_callback_with_missing_complaint_type, @@ -64,3 +68,37 @@ def test_process_ses_results_in_complaint_save_complaint_with_null_complaint_typ assert len(complaints) == 1 assert complaints[0].notification_id == notification.id assert not complaints[0].complaint_type + + +def test_check_and_queue_callback_task(mocker, sample_notification): + mock_create = mocker.patch( + 'app.notifications.notifications_ses_callback.create_delivery_status_callback_data' + ) + + mock_send = mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) + + callback_api = create_service_callback_api(service=sample_notification.service) + mock_create.return_value = 'encrypted_status_update' + + check_and_queue_callback_task(sample_notification) + + # callback_api doesn't match by equality for some + # reason, so we need to take this approach instead + mock_create_args = mock_create.mock_calls[0][1] + assert mock_create_args[0] == sample_notification + assert mock_create_args[1].id == callback_api.id + + mock_send.assert_called_once_with( + [str(sample_notification.id), mock_create.return_value], queue="service-callbacks" + ) + + +def test_check_and_queue_callback_task_no_callback_api(mocker, sample_notification): + mock_send = mocker.patch( + 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' + ) + + check_and_queue_callback_task(sample_notification) + mock_send.assert_not_called() diff --git a/tests/app/notifications/test_process_client_response.py b/tests/app/notifications/test_process_client_response.py index f31a9c9ed..6005fbce3 100644 --- a/tests/app/notifications/test_process_client_response.py +++ b/tests/app/notifications/test_process_client_response.py @@ -8,12 +8,8 @@ from app import statsd_client from app.celery.process_sms_client_response_tasks import ( process_sms_client_response, ) -from app.celery.service_callback_tasks import ( - create_delivery_status_callback_data, -) from app.clients import ClientException from app.models import NOTIFICATION_TECHNICAL_FAILURE -from tests.app.db import create_service_callback_api def test_process_sms_client_response_raises_error_if_reference_is_not_a_valid_uuid(client): @@ -121,12 +117,7 @@ def test_process_sms_client_response_updates_notification_status_when_detailed_s def test_sms_response_does_not_send_callback_if_notification_is_not_in_the_db(sample_service, mocker): - mocker.patch( - 'app.celery.process_sms_client_response_tasks.get_service_delivery_status_callback_api_for_service', - return_value='mock-delivery-callback-for-service') - send_mock = mocker.patch( - 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' - ) + send_mock = mocker.patch('app.celery.process_sms_client_response_tasks.check_and_queue_callback_task') reference = str(uuid.uuid4()) process_sms_client_response(status='3', provider_reference=reference, client_name='MMG') send_mock.assert_not_called() @@ -156,26 +147,17 @@ def test_process_sms_updates_billable_units_if_zero(sample_notification): def test_process_sms_response_does_not_send_service_callback_for_pending_notifications(sample_notification, mocker): - mocker.patch( - 'app.celery.process_sms_client_response_tasks.get_service_delivery_status_callback_api_for_service', - return_value='fake-callback') - send_mock = mocker.patch('app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async') + send_mock = mocker.patch('app.celery.process_sms_client_response_tasks.check_and_queue_callback_task') process_sms_client_response('2', str(sample_notification.id), 'Firetext') send_mock.assert_not_called() def test_outcome_statistics_called_for_successful_callback(sample_notification, mocker): - send_mock = mocker.patch( - 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' - ) - callback_api = create_service_callback_api(service=sample_notification.service, url="https://original_url.com") + send_mock = mocker.patch('app.celery.process_sms_client_response_tasks.check_and_queue_callback_task') reference = str(sample_notification.id) process_sms_client_response('3', reference, 'MMG') - - encrypted_data = create_delivery_status_callback_data(sample_notification, callback_api) - send_mock.assert_called_once_with([reference, encrypted_data], - queue="service-callbacks") + send_mock.assert_called_once_with(sample_notification) def test_process_sms_updates_sent_by_with_client_name_if_not_in_noti(sample_notification):