From 27b3cece7d32dc3c7fff1f5996057d122c4af671 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Thu, 18 Mar 2021 15:04:23 +0000 Subject: [PATCH] Send template id and version with delivery status callback This adds the `template_id` and `template_version` fields to the data sent to services from the `send_delivery_status_to_service` task. We need to account for the task not being passed these fields at first since there might be tasks retrying which don't have that data. Once all tasks have been called with the new fields we can then update the code to assume they are always there. Since we only send delivery status callbacks for SMS and emails, I've removed the tests where we call that task with letters. --- app/celery/service_callback_tasks.py | 8 +++++++ app/commands.py | 23 ++++++------------- .../app/celery/test_service_callback_tasks.py | 15 ++++++------ 3 files changed, 23 insertions(+), 23 deletions(-) diff --git a/app/celery/service_callback_tasks.py b/app/celery/service_callback_tasks.py index e648a8dfa..e9a9dd0c7 100644 --- a/app/celery/service_callback_tasks.py +++ b/app/celery/service_callback_tasks.py @@ -26,6 +26,12 @@ def send_delivery_status_to_service( "sent_at": status_update['notification_sent_at'], "notification_type": status_update['notification_type'] } + # TODO: set the template_id and template_version keys when data dict is created once this change has + # been deployed long enough for all tasks to have those keys in status_update + if status_update.get("template_id"): + data["template_id"] = status_update['template_id'] + data["template_version"] = status_update['template_version'] + _send_data_to_service_callback_api( self, data, @@ -121,6 +127,8 @@ def create_delivery_status_callback_data(notification, service_callback_api): "notification_type": notification.notification_type, "service_callback_api_url": service_callback_api.url, "service_callback_api_bearer_token": service_callback_api.bearer_token, + "template_id": str(notification.template_id), + "template_version": notification.template_version, } return encryption.encrypt(data) diff --git a/app/commands.py b/app/commands.py index c47bc6173..22e28849f 100644 --- a/app/commands.py +++ b/app/commands.py @@ -15,13 +15,16 @@ from notifications_utils.template import SMSMessageTemplate from sqlalchemy.exc import IntegrityError from sqlalchemy.orm.exc import NoResultFound -from app import db, encryption +from app import db from app.aws import s3 from app.celery.letters_pdf_tasks import get_pdf_for_templated_letter from app.celery.reporting_tasks import ( create_nightly_notification_status_for_day, ) -from app.celery.service_callback_tasks import send_delivery_status_to_service +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 ( @@ -72,7 +75,7 @@ from app.models import ( Service, User, ) -from app.utils import DATETIME_FORMAT, get_london_midnight_in_utc +from app.utils import get_london_midnight_in_utc @click.group(name='command', help='Additional commands') @@ -322,19 +325,7 @@ def replay_service_callbacks(file_name, service_id): raise Exception("Some notifications for the given references were not found") for n in notifications: - data = { - "notification_id": str(n.id), - "notification_client_reference": n.client_reference, - "notification_to": n.to, - "notification_status": n.status, - "notification_created_at": n.created_at.strftime(DATETIME_FORMAT), - "notification_updated_at": n.updated_at.strftime(DATETIME_FORMAT), - "notification_sent_at": n.sent_at.strftime(DATETIME_FORMAT), - "notification_type": n.notification_type, - "service_callback_api_url": callback_api.url, - "service_callback_api_bearer_token": callback_api.bearer_token, - } - encrypted_status_update = encryption.encrypt(data) + 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) diff --git a/tests/app/celery/test_service_callback_tasks.py b/tests/app/celery/test_service_callback_tasks.py index 3306c9651..368b1480e 100644 --- a/tests/app/celery/test_service_callback_tasks.py +++ b/tests/app/celery/test_service_callback_tasks.py @@ -20,8 +20,7 @@ from tests.app.db import ( ) -@pytest.mark.parametrize("notification_type", - ["email", "letter", "sms"]) +@pytest.mark.parametrize("notification_type", ["email", "sms"]) def test_send_delivery_status_to_service_post_https_request_to_service_with_encrypted_data( notify_db_session, notification_type): @@ -49,7 +48,9 @@ def test_send_delivery_status_to_service_post_https_request_to_service_with_encr "created_at": datestr.strftime(DATETIME_FORMAT), "completed_at": datestr.strftime(DATETIME_FORMAT), "sent_at": datestr.strftime(DATETIME_FORMAT), - "notification_type": notification_type + "notification_type": notification_type, + "template_id": str(template.id), + "template_version": 1 } assert request_mock.call_count == 1 @@ -90,8 +91,7 @@ def test_send_complaint_to_service_posts_https_request_to_service_with_encrypted assert request_mock.request_history[0].headers["Authorization"] == "Bearer {}".format(callback_api.bearer_token) -@pytest.mark.parametrize("notification_type", - ["email", "letter", "sms"]) +@pytest.mark.parametrize("notification_type", ["email", "sms"]) def test__send_data_to_service_callback_api_retries_if_request_returns_500_with_encrypted_data( notify_db_session, mocker, notification_type ): @@ -115,8 +115,7 @@ def test__send_data_to_service_callback_api_retries_if_request_returns_500_with_ assert mocked.call_args[1]['queue'] == 'service-callbacks-retry' -@pytest.mark.parametrize("notification_type", - ["email", "letter", "sms"]) +@pytest.mark.parametrize("notification_type", ["email", "sms"]) def test__send_data_to_service_callback_api_does_not_retry_if_request_returns_404_with_encrypted_data( notify_db_session, mocker, @@ -185,6 +184,8 @@ def _set_up_data_for_status_update(callback_api, notification): "notification_type": notification.notification_type, "service_callback_api_url": callback_api.url, "service_callback_api_bearer_token": callback_api.bearer_token, + "template_id": str(notification.template_id), + "template_version": notification.template_version, } encrypted_status_update = encryption.encrypt(data) return encrypted_status_update