mirror of
https://github.com/GSA/notifications-api.git
synced 2026-09-07 16:18:25 -04:00
Refactor process_ses_receipt
It is possible a service has data rention that is smaller than the time it takes to get a delivery receipt. This PR refactors process_ses_receipt to update NotificationHistory if the Notifcation has already been purged.
This commit is contained in:
@@ -39,7 +39,7 @@ def process_ses_results(self, response):
|
|||||||
reference = ses_message['mail']['messageId']
|
reference = ses_message['mail']['messageId']
|
||||||
|
|
||||||
try:
|
try:
|
||||||
notification = notifications_dao.dao_get_notification_by_reference(reference)
|
notification = notifications_dao.dao_get_notification_history_by_reference(reference=reference)
|
||||||
except NoResultFound:
|
except NoResultFound:
|
||||||
message_time = iso8601.parse_date(ses_message['mail']['timestamp']).replace(tzinfo=None)
|
message_time = iso8601.parse_date(ses_message['mail']['timestamp']).replace(tzinfo=None)
|
||||||
if datetime.utcnow() - message_time < timedelta(minutes=5):
|
if datetime.utcnow() - message_time < timedelta(minutes=5):
|
||||||
@@ -50,22 +50,17 @@ def process_ses_results(self, response):
|
|||||||
)
|
)
|
||||||
return
|
return
|
||||||
|
|
||||||
if notification.status not in {NOTIFICATION_SENDING, NOTIFICATION_PENDING}:
|
if notification.status not in [NOTIFICATION_SENDING, NOTIFICATION_PENDING]:
|
||||||
notifications_dao._duplicate_update_warning(notification, notification_status)
|
notifications_dao._duplicate_update_warning(
|
||||||
return
|
notification=notification,
|
||||||
|
status=notification_status
|
||||||
notifications_dao._update_notification_status(notification=notification, status=notification_status)
|
|
||||||
|
|
||||||
if not aws_response_dict['success']:
|
|
||||||
current_app.logger.info(
|
|
||||||
"SES delivery failed: notification id {} and reference {} has error found. Status {}".format(
|
|
||||||
notification.id, reference, aws_response_dict['message']
|
|
||||||
)
|
|
||||||
)
|
)
|
||||||
|
return
|
||||||
else:
|
else:
|
||||||
current_app.logger.info('SES callback return status of {} for notification: {}'.format(
|
notifications_dao.dao_update_notifications_by_reference(
|
||||||
notification_status, notification.id
|
references=[reference],
|
||||||
))
|
update_dict={'status': notification_status}
|
||||||
|
)
|
||||||
|
|
||||||
statsd_client.incr('callback.ses.{}'.format(notification_status))
|
statsd_client.incr('callback.ses.{}'.format(notification_status))
|
||||||
|
|
||||||
|
|||||||
@@ -303,7 +303,6 @@ def save_api_email(self,
|
|||||||
service = dao_fetch_service_by_id(notification['service_id'])
|
service = dao_fetch_service_by_id(notification['service_id'])
|
||||||
|
|
||||||
try:
|
try:
|
||||||
current_app.logger.info(f"Persisting notification {notification['id']}")
|
|
||||||
|
|
||||||
persist_notification(
|
persist_notification(
|
||||||
notification_id=notification["id"],
|
notification_id=notification["id"],
|
||||||
@@ -327,7 +326,7 @@ def save_api_email(self,
|
|||||||
[notification['id']],
|
[notification['id']],
|
||||||
queue=q
|
queue=q
|
||||||
)
|
)
|
||||||
current_app.logger.info(f"Email {notification['id']} has been persisted.")
|
current_app.logger.info(f"Email {notification['id']} has been persisted and sent to delivery queue.")
|
||||||
except IntegrityError:
|
except IntegrityError:
|
||||||
current_app.logger.info(f"Email {notification['id']} already exists.")
|
current_app.logger.info(f"Email {notification['id']} already exists.")
|
||||||
|
|
||||||
|
|||||||
@@ -1,17 +1,23 @@
|
|||||||
import json
|
import json
|
||||||
from datetime import datetime
|
from datetime import datetime
|
||||||
|
|
||||||
|
|
||||||
from freezegun import freeze_time
|
from freezegun import freeze_time
|
||||||
|
|
||||||
|
|
||||||
from app import statsd_client, encryption
|
from app import statsd_client, encryption
|
||||||
from app.celery.process_ses_receipts_tasks import process_ses_results
|
from app.celery.process_ses_receipts_tasks import process_ses_results
|
||||||
from app.celery.research_mode_tasks import ses_hard_bounce_callback, ses_soft_bounce_callback, ses_notification_callback
|
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_delivery_status_callback_data
|
from app.celery.service_callback_tasks import create_delivery_status_callback_data
|
||||||
from app.dao.notifications_dao import get_notification_by_id
|
from app.dao.notifications_dao import get_notification_by_id
|
||||||
from app.models import Complaint, Notification
|
from app.models import Complaint, Notification
|
||||||
from app.notifications.notifications_ses_callback import remove_emails_from_complaint, remove_emails_from_bounce
|
from app.notifications.notifications_ses_callback import (
|
||||||
|
remove_emails_from_complaint,
|
||||||
|
remove_emails_from_bounce
|
||||||
|
)
|
||||||
|
|
||||||
from tests.app.db import (
|
from tests.app.db import (
|
||||||
create_notification,
|
create_notification,
|
||||||
@@ -29,7 +35,7 @@ def test_process_ses_results(sample_email_template):
|
|||||||
def test_process_ses_results_retry_called(sample_email_template, notify_db, mocker):
|
def test_process_ses_results_retry_called(sample_email_template, notify_db, mocker):
|
||||||
create_notification(sample_email_template, reference='ref1', sent_at=datetime.utcnow(), status='sending')
|
create_notification(sample_email_template, reference='ref1', sent_at=datetime.utcnow(), status='sending')
|
||||||
|
|
||||||
mocker.patch("app.dao.notifications_dao._update_notification_status", side_effect=Exception("EXPECTED"))
|
mocker.patch("app.dao.notifications_dao.dao_update_notifications_by_reference", side_effect=Exception("EXPECTED"))
|
||||||
mocked = mocker.patch('app.celery.process_ses_receipts_tasks.process_ses_results.retry')
|
mocked = mocker.patch('app.celery.process_ses_receipts_tasks.process_ses_results.retry')
|
||||||
process_ses_results(response=ses_notification_callback(reference='ref1'))
|
process_ses_results(response=ses_notification_callback(reference='ref1'))
|
||||||
assert mocked.call_count != 0
|
assert mocked.call_count != 0
|
||||||
@@ -90,13 +96,15 @@ def test_ses_callback_should_update_notification_status(
|
|||||||
|
|
||||||
def test_ses_callback_should_not_update_notification_status_if_already_delivered(sample_email_template, mocker):
|
def test_ses_callback_should_not_update_notification_status_if_already_delivered(sample_email_template, mocker):
|
||||||
mock_dup = mocker.patch('app.celery.process_ses_receipts_tasks.notifications_dao._duplicate_update_warning')
|
mock_dup = mocker.patch('app.celery.process_ses_receipts_tasks.notifications_dao._duplicate_update_warning')
|
||||||
mock_upd = mocker.patch('app.celery.process_ses_receipts_tasks.notifications_dao._update_notification_status')
|
mock_upd = mocker.patch(
|
||||||
|
'app.celery.process_ses_receipts_tasks.notifications_dao.dao_update_notifications_by_reference'
|
||||||
|
)
|
||||||
notification = create_notification(template=sample_email_template, reference='ref', status='delivered')
|
notification = create_notification(template=sample_email_template, reference='ref', status='delivered')
|
||||||
|
|
||||||
assert process_ses_results(ses_notification_callback(reference='ref')) is None
|
assert process_ses_results(ses_notification_callback(reference='ref')) is None
|
||||||
assert get_notification_by_id(notification.id).status == 'delivered'
|
assert get_notification_by_id(notification.id).status == 'delivered'
|
||||||
|
|
||||||
mock_dup.assert_called_once_with(notification, 'delivered')
|
mock_dup.assert_called_once_with(notification=notification, status='delivered')
|
||||||
assert mock_upd.call_count == 0
|
assert mock_upd.call_count == 0
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -1567,7 +1567,9 @@ def test_dao_update_notifications_by_reference_set_returned_letter_status(sample
|
|||||||
|
|
||||||
assert updated_count == 1
|
assert updated_count == 1
|
||||||
assert updated_history_count == 0
|
assert updated_history_count == 0
|
||||||
assert Notification.query.get(notification.id).status == 'returned-letter'
|
updated_notification = Notification.query.get(notification.id)
|
||||||
|
assert updated_notification.status == 'returned-letter'
|
||||||
|
assert updated_notification.updated_at <= datetime.utcnow()
|
||||||
|
|
||||||
|
|
||||||
def test_dao_update_notifications_by_reference_updates_history_when_one_of_two_notifications_exists(
|
def test_dao_update_notifications_by_reference_updates_history_when_one_of_two_notifications_exists(
|
||||||
|
|||||||
Reference in New Issue
Block a user