Skip to content

Commit

Permalink
Squash #2137 - Rename methods and update docstring original used to s…
Browse files Browse the repository at this point in the history
…end ONLY email notification status to VAProfile to reflect methods used for BOTH email and sms
  • Loading branch information
MackHalliday committed Dec 4, 2024
1 parent 7ad295b commit 49a3458
Show file tree
Hide file tree
Showing 7 changed files with 40 additions and 35 deletions.
4 changes: 2 additions & 2 deletions app/celery/process_delivery_status_result_tasks.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import datetime
from typing import Tuple

from app.celery.process_ses_receipts_tasks import check_and_queue_va_profile_email_status_callback
from app.celery.process_ses_receipts_tasks import check_and_queue_va_profile_notification_status_callback
from celery import Task
from flask import current_app
from notifications_utils.statsd_decorators import statsd
Expand Down Expand Up @@ -260,7 +260,7 @@ def sms_status_update(
# Only send if there was an update
if last_updated_at != notification.updated_at:
check_and_queue_callback_task(notification, sms_status.payload)
check_and_queue_va_profile_email_status_callback(notification)
check_and_queue_va_profile_notification_status_callback(notification)
statsd_client.incr(f'clients.sms.{sms_status.provider}.status_update.success')
except Exception:
current_app.logger.exception('Failed to check_and_queue_callback_task for notification: %s', notification.id)
Expand Down
6 changes: 3 additions & 3 deletions app/celery/process_ses_receipts_tasks.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from datetime import datetime, timedelta

from app.celery.send_va_profile_notification_status import check_and_queue_va_profile_email_status_callback
from app.celery.send_va_profile_notification_status import check_and_queue_va_profile_notification_status_callback
import iso8601
from app.celery.common import log_notification_total_time
from celery.exceptions import Retry
Expand Down Expand Up @@ -227,7 +227,7 @@ def process_ses_results( # noqa: C901 (too complex 14 > 10)

notifications_dao.dao_update_notification(notification)
check_and_queue_callback_task(notification)
check_and_queue_va_profile_email_status_callback(notification)
check_and_queue_va_profile_notification_status_callback(notification)

return
elif notification_status == NOTIFICATION_DELIVERED:
Expand Down Expand Up @@ -262,7 +262,7 @@ def process_ses_results( # noqa: C901 (too complex 14 > 10)
)

check_and_queue_callback_task(notification)
check_and_queue_va_profile_email_status_callback(notification)
check_and_queue_va_profile_notification_status_callback(notification)

return True

Expand Down
19 changes: 10 additions & 9 deletions app/celery/send_va_profile_notification_status.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,16 @@
from app.models import Notification


def check_and_queue_va_profile_email_status_callback(notification: Notification) -> None:
def check_and_queue_va_profile_notification_status_callback(notification: Notification) -> None:
"""
This checks the feature flag is enabled. If it is, queues the celery task and collects data from the notification.
Otherwise, it only logs a message.
:param notification: the email notification to collect data from
:param notification: the notification (email or sms) to collect data from
"""
current_app.logger.debug(
'Sending email status to VA Profile, checking feature flag... | notification %s', notification.id
'Sending email status to VA Profile, checking VA_PROFILE_SMS_STATUS_ENABLE feature flag... | notification %s',
notification.id,
)

if is_feature_enabled(FeatureFlag.VA_PROFILE_SMS_STATUS_ENABLED) or notification.notification_type == EMAIL_TYPE:
Expand All @@ -27,18 +28,18 @@ def check_and_queue_va_profile_email_status_callback(notification: Notification)
notification_data = {
'id': str(notification.id), # this is the notification id
'reference': notification.client_reference,
'to': notification.to, # this is the recipient's contact info (email)
'to': notification.to, # this is the recipient's contact info
'status': notification.status, # this will specify the delivery status of the notification
'status_reason': notification.status_reason, # populated if there's additional context on the delivery status
'created_at': notification.created_at.strftime(DATETIME_FORMAT), # noqa: F821
'completed_at': notification.updated_at.strftime(DATETIME_FORMAT) if notification.updated_at else None,
'sent_at': notification.sent_at.strftime(DATETIME_FORMAT) if notification.sent_at else None,
'notification_type': notification.notification_type, # this is the channel/type of notification (email)
'provider': notification.sent_by, # email provider
'notification_type': notification.notification_type, # this is the channel/type of notification (email or sms)
'provider': notification.sent_by,
}

# data passed to tasks must be JSON serializable
send_email_status_to_va_profile.delay(notification_data)
send_notification_status_to_va_profile.delay(notification_data)
else:
current_app.logger.debug(
'SMS status not sent to VA Profile, feature flag disabled | notification %s', notification.id
Expand All @@ -52,11 +53,11 @@ def check_and_queue_va_profile_email_status_callback(notification: Notification)
retry_backoff=True,
retry_backoff_max=3600,
)
def send_email_status_to_va_profile(notification_data: dict) -> None:
def send_notification_status_to_va_profile(notification_data: dict) -> None:
"""
This function calls the VAProfileClient method to send the information to VA Profile.
:param notification_data: the email notification data to send
:param notification_data: the email or sms notification data to send
"""

try:
Expand Down
4 changes: 3 additions & 1 deletion app/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,9 @@ class Config(object):
'app.celery.v3.notification_tasks.v3_send_email_notification': {'queue': QueueNames.SEND_EMAIL},
'app.celery.v3.notification_tasks.v3_send_sms_notification': {'queue': QueueNames.SEND_SMS},
'app.celery.process_ga4_measurement_tasks.post_to_ga4': {'queue': QueueNames.SEND_EMAIL},
'app.celery.process_ses_receipts_tasks.send_email_status_to_va_profile': {'queue': QueueNames.CALLBACKS},
'app.celery.process_ses_receipts_tasks.send_notification_status_to_va_profile': {
'queue': QueueNames.CALLBACKS
},
},
}

Expand Down
10 changes: 6 additions & 4 deletions app/va/va_profile/va_profile_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ def _handle_exceptions(self, va_profile_id_value: str, error: Exception):
def send_va_profile_email_status(self, notification_data: dict) -> None:
"""
This method sends notification status data to VA Profile. This is part of our integration to help VA Profile
provide better service by letting them know which emails are good, and which ones result in bounces.
provide better service by letting them know which emails and phone numbers are good, and which ones result in bounces.
:param notification_data: the data to include with the POST request to VA Profile
Expand All @@ -367,7 +367,9 @@ def send_va_profile_email_status(self, notification_data: dict) -> None:
url = f'{self.va_profile_url}/contact-information-vanotify/notify/status'

self.logger.debug(
'Sending email status to VA Profile with url: %s | notification: %s', url, notification_data.get('id')
'Sending notification status to VA Profile with url: %s | notification: %s',
url,
notification_data.get('id'),
)

# make POST request to VA Profile endpoint for notification statuses
Expand All @@ -376,13 +378,13 @@ def send_va_profile_email_status(self, notification_data: dict) -> None:
response = requests.post(url, json=notification_data, headers=headers, timeout=self.timeout)
except requests.Timeout:
self.logger.exception(
'Request timeout attempting to send email status to VA Profile for notification %s | retrying...',
'Request timeout attempting to send notification status to VA Profile for notification %s | retrying...',
notification_data.get('id'),
)
raise
except requests.RequestException:
self.logger.exception(
'Unexpected request exception. E-mail status NOT sent to VA Profile for notification %s.',
'Unexpected request exception. Notification status NOT sent to VA Profile for notification %s.',
notification_data.get('id'),
)
raise
Expand Down
26 changes: 13 additions & 13 deletions tests/app/celery/test_process_ses_receipts_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
ses_notification_callback,
ses_soft_bounce_callback,
)
from app.celery.send_va_profile_notification_status import send_email_status_to_va_profile
from app.celery.send_va_profile_notification_status import send_notification_status_to_va_profile
from app.celery.service_callback_tasks import create_delivery_status_callback_data
from app.constants import (
EMAIL_TYPE,
Expand Down Expand Up @@ -46,7 +46,7 @@ def test_process_ses_results(notify_db_session, sample_template, sample_notifica
ref = str(uuid4())

mock_send_email_status = mocker.patch(
'app.celery.send_va_profile_notification_status.send_email_status_to_va_profile.apply_async'
'app.celery.send_va_profile_notification_status.send_notification_status_to_va_profile.apply_async'
)
mock_send_email_status.return_value = None

Expand Down Expand Up @@ -110,7 +110,7 @@ def test_ses_callback_should_call_send_delivery_status_to_service(
):
send_mock = mocker.patch('app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async')
mock_send_email_status = mocker.patch(
'app.celery.send_va_profile_notification_status.send_email_status_to_va_profile.apply_async'
'app.celery.send_va_profile_notification_status.send_notification_status_to_va_profile.apply_async'
)
mock_send_email_status.return_value = None

Expand Down Expand Up @@ -141,7 +141,7 @@ def test_wt_ses_callback_should_log_total_time(
mock_log_total_time = mocker.patch('app.celery.common.log_notification_total_time')
mocker.patch('app.celery.service_callback_tasks.check_and_queue_callback_task')
mock_send_email_status = mocker.patch(
'app.celery.send_va_profile_notification_status.send_email_status_to_va_profile.apply_async'
'app.celery.send_va_profile_notification_status.send_notification_status_to_va_profile.apply_async'
)
mock_send_email_status.return_value = None

Expand Down Expand Up @@ -175,7 +175,7 @@ def test_it_ses_callback_should_send_email_status_to_va_profile_when_set_to_deli
mocker.patch('app.celery.process_ses_receipts_tasks.check_and_queue_callback_task')
mocker.patch('app.celery.send_va_profile_notification_status.is_feature_enabled', return_value=True)
mock_send_email_status = mocker.patch(
'app.celery.send_va_profile_notification_status.send_email_status_to_va_profile.apply_async'
'app.celery.send_va_profile_notification_status.send_notification_status_to_va_profile.apply_async'
)
ref = str(uuid4())

Expand Down Expand Up @@ -212,7 +212,7 @@ def test_it_ses_callback_should_send_email_status_to_va_profile_with_notificatio
mocker.patch('app.celery.process_ses_receipts_tasks.check_and_queue_callback_task')
mocker.patch('app.celery.send_va_profile_notification_status.is_feature_enabled', return_value=True)
mock_send_email_status = mocker.patch(
'app.celery.send_va_profile_notification_status.send_email_status_to_va_profile.apply_async'
'app.celery.send_va_profile_notification_status.send_notification_status_to_va_profile.apply_async'
)
ref = str(uuid4())

Expand Down Expand Up @@ -250,7 +250,7 @@ def test_it_ses_callback_should_send_email_status_to_va_profile_with_notificatio
mock_callback = mocker.patch('app.celery.process_ses_receipts_tasks.check_and_queue_callback_task')
mocker.patch('app.celery.send_va_profile_notification_status.is_feature_enabled', return_value=True)
mock_send_email_status = mocker.patch(
'app.celery.send_va_profile_notification_status.send_email_status_to_va_profile.apply_async'
'app.celery.send_va_profile_notification_status.send_notification_status_to_va_profile.apply_async'
)
ref = str(uuid4())

Expand Down Expand Up @@ -342,7 +342,7 @@ def test_ses_callback_does_not_call_send_delivery_status_if_no_db_entry(
ref = str(uuid4())
send_mock = mocker.patch('app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async')
mock_send_email_status = mocker.patch(
'app.celery.send_va_profile_notification_status.send_email_status_to_va_profile.apply_async'
'app.celery.send_va_profile_notification_status.send_notification_status_to_va_profile.apply_async'
)
mock_send_email_status.return_value = None
notification_id = sample_notification(template=template, status=NOTIFICATION_SENDING, reference=ref).id
Expand All @@ -363,7 +363,7 @@ def test_ses_callback_should_update_multiple_notification_status_sent(
send_mock = mocker.patch('app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async')

mock_send_email_status = mocker.patch(
'app.celery.send_va_profile_notification_status.send_email_status_to_va_profile.apply_async'
'app.celery.send_va_profile_notification_status.send_notification_status_to_va_profile.apply_async'
)
mock_send_email_status.return_value = None

Expand Down Expand Up @@ -402,7 +402,7 @@ def test_ses_callback_should_set_status_to_temporary_failure(
):
send_mock = mocker.patch('app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async')
mock_send_email_status = mocker.patch(
'app.celery.send_va_profile_notification_status.send_email_status_to_va_profile.apply_async'
'app.celery.send_va_profile_notification_status.send_notification_status_to_va_profile.apply_async'
)
mock_send_email_status.return_value = None

Expand Down Expand Up @@ -434,7 +434,7 @@ def test_ses_callback_should_set_status_to_permanent_failure(
):
send_mock = mocker.patch('app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async')
mock_send_email_status = mocker.patch(
'app.celery.send_va_profile_notification_status.send_email_status_to_va_profile.apply_async'
'app.celery.send_va_profile_notification_status.send_notification_status_to_va_profile.apply_async'
)
mock_send_email_status.return_value = None

Expand Down Expand Up @@ -648,7 +648,7 @@ def test_ut_send_email_status_to_va_profile(self, mocker):
'app.celery.send_va_profile_notification_status.va_profile_client.send_va_profile_email_status'
)

send_email_status_to_va_profile(self.mock_notification_data)
send_notification_status_to_va_profile(self.mock_notification_data)

mock_send_va_profile_email_status.assert_called_once_with(self.mock_notification_data)

Expand All @@ -659,7 +659,7 @@ def test_ut_send_email_status_to_va_profile_raises_auto_retry_exception(self, mo
)

with pytest.raises(AutoRetryException):
send_email_status_to_va_profile(self.mock_notification_data)
send_notification_status_to_va_profile(self.mock_notification_data)

mock_send_va_profile_email_status.assert_called_once()

Expand Down
6 changes: 3 additions & 3 deletions tests/app/celery/test_send_va_profile_notification_status.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from datetime import datetime
from unittest.mock import patch

from app.celery.send_va_profile_notification_status import check_and_queue_va_profile_email_status_callback
from app.celery.send_va_profile_notification_status import check_and_queue_va_profile_notification_status_callback
from app.constants import EMAIL_TYPE, SMS_TYPE
from app.models import Notification

Expand Down Expand Up @@ -40,7 +40,7 @@ def test_can_send_sms_notification_status_to_va_profile(self, mock_post, notify_

sms_notification = Notification(**self.mock_sms_notification_data)

check_and_queue_va_profile_email_status_callback(sms_notification)
check_and_queue_va_profile_notification_status_callback(sms_notification)

va_profile_url = os.getenv('VA_PROFILE_URL')
expected_url = f'{va_profile_url}/contact-information-vanotify/notify/status'
Expand Down Expand Up @@ -71,7 +71,7 @@ def test_can_send_email_notification_status_to_va_profile(self, mock_post, notif

email_notification = Notification(**self.mock_email_notification_data)

check_and_queue_va_profile_email_status_callback(email_notification)
check_and_queue_va_profile_notification_status_callback(email_notification)

va_profile_url = os.getenv('VA_PROFILE_URL')
expected_url = f'{va_profile_url}/contact-information-vanotify/notify/status'
Expand Down

0 comments on commit 49a3458

Please sign in to comment.