From a89bf47f5a9458ed7bb7de779fdbad8c0aea2c65 Mon Sep 17 00:00:00 2001 From: Evan Parish <104009494+EvanParish@users.noreply.github.com> Date: Tue, 17 Sep 2024 16:03:04 -0500 Subject: [PATCH] #1977 Bug: Update to Delivered After Other Status is Assigned --- .../process_delivery_status_result_tasks.py | 37 +--- app/celery/process_pinpoint_receipt_tasks.py | 65 ++++--- app/dao/notifications_dao.py | 176 +++++++++--------- .../test_process_pinpoint_receipt_tasks.py | 41 +++- tests/app/celery/test_tasks.py | 6 +- .../notification_dao/test_notification_dao.py | 43 ++--- 6 files changed, 198 insertions(+), 170 deletions(-) diff --git a/app/celery/process_delivery_status_result_tasks.py b/app/celery/process_delivery_status_result_tasks.py index eed3b639a0..4e83a59a78 100644 --- a/app/celery/process_delivery_status_result_tasks.py +++ b/app/celery/process_delivery_status_result_tasks.py @@ -1,5 +1,4 @@ -import datetime -from app.models import DELIVERY_STATUS_CALLBACK_TYPE +from app.models import DELIVERY_STATUS_CALLBACK_TYPE, NOTIFICATION_DELIVERED from app.celery.common import log_notification_total_time from app.celery.service_callback_tasks import check_and_queue_callback_task from app.celery.process_pinpoint_inbound_sms import CeleryEvent @@ -8,7 +7,6 @@ dao_get_notification_by_reference, dao_update_notification_by_id, update_notification_delivery_status, - FINAL_STATUS_STATES, ) from typing import Tuple @@ -19,6 +17,7 @@ from app import notify_celery, statsd_client, clients from app.celery.exceptions import AutoRetryException from app.dao.service_callback_dao import dao_get_callback_include_payload_status +from app.dao.notifications_dao import duplicate_update_warning from app.models import Notification @@ -85,11 +84,11 @@ def process_delivery_status( try: # calculate pricing current_app.logger.info( - 'Notification ID (%s) - Calculate Pricing: %s and notification_status: %s with number_of_message_parts: %s', - notification.id, + 'Calculate Pricing: %s and update notification_status: %s with number_of_message_parts: %s for notification: %s', provider_name, notification_status, number_of_message_parts, + getattr(notification, 'id', 'unknown'), ) _calculate_pricing(price_in_millicents_usd, notification, notification_status, number_of_message_parts) @@ -157,23 +156,6 @@ def attempt_to_get_notification( return notification, should_exit -def log_notification_status_warning( - notification: Notification, - status: str, -) -> None: - time_diff = datetime.datetime.utcnow() - (notification.updated_at or notification.created_at) - current_app.logger.warning( - 'Invalid callback received. Notification id %s received a status update to %s ' - '%s after being set to %s. %s sent by %s', - notification.id, - status, - time_diff, - notification.status, - notification.notification_type, - notification.sent_by, - ) - - def check_notification_status( notification: Notification, notification_status: str, @@ -182,13 +164,13 @@ def check_notification_status( # Do not update if the status has not changed. if notification_status == notification.status: current_app.logger.info( - 'SQS callback received the same status of %s for notification %s)', notification_status, notification.id + 'SQS callback received the same status of %s for notification %s', notification_status, notification.id ) return True - # Do not update if notification status is in a final state. - if notification.status in FINAL_STATUS_STATES: - log_notification_status_warning(notification, notification_status) + # Do not update if notification status is already delivered + if notification.status == NOTIFICATION_DELIVERED: + duplicate_update_warning(notification, notification_status) return True return False @@ -216,14 +198,13 @@ def _calculate_pricing( price_in_millicents_usd: float, notification: Notification, notification_status: str, number_of_message_parts: int ): """Calculate pricing""" - current_app.logger.info('Calculate Pricing') + current_app.logger.debug('Calculate pricing and update notification %s', notification.id) if price_in_millicents_usd > 0.0: dao_update_notification_by_id( notification_id=notification.id, status=notification_status, segments_count=number_of_message_parts, cost_in_millicents=price_in_millicents_usd, - updated_at=datetime.utcnow(), ) else: update_notification_delivery_status(notification_id=notification.id, new_status=notification_status) diff --git a/app/celery/process_pinpoint_receipt_tasks.py b/app/celery/process_pinpoint_receipt_tasks.py index 11ff2531a4..3be4e80f12 100644 --- a/app/celery/process_pinpoint_receipt_tasks.py +++ b/app/celery/process_pinpoint_receipt_tasks.py @@ -12,7 +12,8 @@ from app.celery.exceptions import AutoRetryException from app.dao.notifications_dao import ( dao_get_notification_by_reference, - dao_update_notification, + dao_update_notification_by_id, + duplicate_update_warning, update_notification_status_by_id, FINAL_STATUS_STATES, ) @@ -134,10 +135,12 @@ def process_pinpoint_results( notification.status_reason = 'The veteran is opted-out at the Pinpoint level.' if price_in_millicents_usd > 0.0: - notification.status = notification_status - notification.segments_count = number_of_message_parts - notification.cost_in_millicents = price_in_millicents_usd - dao_update_notification(notification) + dao_update_notification_by_id( + notification_id=notification.id, + status=notification_status, + segments_count=number_of_message_parts, + cost_in_millicents=price_in_millicents_usd, + ) else: update_notification_status_by_id( notification_id=notification.id, status=notification_status, status_reason=notification.status_reason @@ -209,10 +212,21 @@ def attempt_to_get_notification( return notification, should_exit -def check_notification_status( - notification: Notification, - notification_status: str, -) -> bool: +def check_notification_status(notification: Notification, notification_status: str) -> bool: + """ + Check if the notification status should be updated. If the status has not changed, or if the status is in a final + state, do not update the notification. *Unless* the status is being updated to delivered from a different final + status, in which case, the notification should always be updated. + + Args: + notification (Notification): The notification to check. + notification_status (str): The status to update the notification to. + + Returns: + bool: False if the notification status should not be updated, True if the downstream task should exit. + """ + should_exit = False + # Do not update if the status has not changed. if notification_status == notification.status: current_app.logger.info( @@ -220,28 +234,13 @@ def check_notification_status( notification_status, notification_status, ) - return True - - # Do not update if notification status is in a final state. - if notification.status in FINAL_STATUS_STATES: - log_notification_status_warning(notification, notification_status) - return True + should_exit = True + elif notification_status == NOTIFICATION_DELIVERED: + # ensure the notification is updated to delivered whenever we receive a delivered status + should_exit = False + elif notification.status in FINAL_STATUS_STATES: + # Do not update if notification status is in a final state. + duplicate_update_warning(notification, notification_status) + should_exit = True - return False - - -def log_notification_status_warning( - notification, - status: str, -) -> None: - time_diff = datetime.datetime.utcnow() - (notification.updated_at or notification.created_at) - current_app.logger.warning( - 'Invalid callback received. Notification id %s received a status update to %s ' - '%s after being set to %s. %s sent by %s', - notification.id, - status, - time_diff, - notification.status, - notification.notification_type, - notification.sent_by, - ) + return should_exit diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 3255de390e..0323425096 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -1,5 +1,6 @@ import functools import string +from typing import Optional from uuid import UUID from datetime import ( datetime, @@ -16,7 +17,7 @@ ) from notifications_utils.statsd_decorators import statsd from notifications_utils.timezones import convert_local_timezone_to_utc, convert_utc_to_local_timezone -from sqlalchemy import asc, delete, desc, func, select, update, literal_column +from sqlalchemy import and_, asc, delete, desc, func, or_, select, update, literal_column from sqlalchemy.exc import ArgumentError from sqlalchemy.engine.row import Row from sqlalchemy.orm import joinedload @@ -112,7 +113,7 @@ def _decide_permanent_temporary_failure( current_status, status, ): - # Firetext will send pending, then send either succes or fail. + # Firetext will send pending, then send either success or fail. # If we go from pending to delivered we need to set failure type as temporary-failure if current_status == NOTIFICATION_PENDING and status == NOTIFICATION_PERMANENT_FAILURE: status = NOTIFICATION_TEMPORARY_FAILURE @@ -132,9 +133,15 @@ def _get_notification_status_update_statement( """ Generates an update statement for the given notification id and status. Preserves status order and ensures a transient status will not override a final status. - :param notification_id: String value of notification uuid to be updated - :param status: String value of the status the given notification will attempt to update to - :return: An update statement to be executed, or None + + Args: + notification_id (str): String value of notification uuid to be updated + incoming_status (str): String value of the status the given notification will attempt to update to + notification (Notification): The notification to update, or None + **kwargs: Additional key-value pairs to be updated + + Returns: + update_statement: An update statement to be executed, or None if the notification should not be updated """ notification = db.session.get(Notification, notification_id) if notification is None: @@ -143,20 +150,6 @@ def _get_notification_status_update_statement( ) return None - current_status = notification.status - incoming_status = _decide_permanent_temporary_failure(current_status=current_status, status=incoming_status) - - # do not update if the incoming status happens before the current status - if incoming_status in TRANSIENT_STATUSES and current_status in TRANSIENT_STATUSES: - if TRANSIENT_STATUSES.index(incoming_status) < TRANSIENT_STATUSES.index(current_status): - current_app.logger.warning( - 'An erroneous attempt was made to transition notification id %s from %s to %s', - notification_id, - current_status, - incoming_status, - ) - return None - # add status to values dict if it doesn't have anything if len(kwargs) < 1: kwargs['status'] = incoming_status @@ -165,29 +158,59 @@ def _get_notification_status_update_statement( kwargs['updated_at'] = datetime.utcnow() - # when updating make sure notification is not in final state via query - update_statement = ( - update(Notification) - .where(db.and_(Notification.id == notification_id, Notification.status.not_in(FINAL_STATUS_STATES))) - .values(kwargs) + # Define the allowed update conditions + conditions = and_( + Notification.id == notification_id, + or_( + # update to delivered, unless it's already set to delivered + and_(incoming_status == NOTIFICATION_DELIVERED, Notification.status != NOTIFICATION_DELIVERED), + # update to final status if the current status is not a final status + and_(incoming_status in FINAL_STATUS_STATES, Notification.status.not_in(FINAL_STATUS_STATES)), + # update to sent or temporary failure if the current status is a transient status + and_( + incoming_status in [NOTIFICATION_SENT, NOTIFICATION_TEMPORARY_FAILURE], + Notification.status.in_(TRANSIENT_STATUSES), + ), + # update to pending if the current status is created or sending + and_( + incoming_status == NOTIFICATION_PENDING, + Notification.status.in_([NOTIFICATION_CREATED, NOTIFICATION_SENDING]), + ), + # update to sending from created + and_(incoming_status == NOTIFICATION_SENDING, Notification.status == NOTIFICATION_CREATED), + ), ) - return update_statement + # create the update query with the above conditions + # added execution_options to prevent the session from being out of sync and avoid this error: + # "Could not evaluate current criteria in Python: Cannot evaluate ..." + # https://docs.sqlalchemy.org/en/14/orm/session_basics.html#update-and-delete-with-arbitrary-where-clause + stmt = update(Notification).where(conditions).values(kwargs).execution_options(synchronize_session='fetch') + + return stmt def _update_notification_status( - notification, - status, -): + notification: Notification, + status: str, +) -> Notification: """ Update the notification status if it should be updated. + + Args: + notification (Notification): The notification to update. + status (str): The status to update the notification to. + + Returns: + Notification: The updated notification, or the original notification if it should not be updated. """ + notification_updated = False + # verify notification status is valid if not (status in TRANSIENT_STATUSES or status in FINAL_STATUS_STATES): current_app.logger.error( 'Attempting to update notification %s to a status that does not exist %s', notification.id, status ) - return notification update_statement = _get_notification_status_update_statement(notification.id, status) @@ -201,20 +224,22 @@ def _update_notification_status( status, ) current_app.logger.exception(e) - return notification except ArgumentError as e: current_app.logger.warning('Cannot update notification %s to status %s', notification.id, status) current_app.logger.exception(e) - return notification - except: + except Exception: current_app.logger.exception( 'An error occured when attempting to update notification %s to status %s', notification.id, status, ) - return notification + else: + notification_updated = True + + if notification_updated: + return db.session.get(Notification, notification.id) - return db.session.get(Notification, notification.id) + return notification @statsd(namespace='dao') @@ -231,10 +256,6 @@ def update_notification_status_by_id( current_app.logger.info('notification not found for id %s (update to status %s)', notification_id, status) return None - if notification.status not in TRANSIENT_STATUSES: - duplicate_update_warning(notification, status) - return None - if notification.international and not country_records_delivery(notification.phone_prefix): return None @@ -263,44 +284,19 @@ def update_notification_delivery_status( Based on the desired update status, a query is constructed that cannot put the notification in an incorrect state. Arguments: - notification_id: Notification id, - new_status: Status to update the notification to, + notification_id (UUID): Notification id, + new_status (str): Status to update the notification to, """ - current_app.logger.info('Update notification: %s to status: %s', notification_id, new_status) - stmt = ( - update(Notification) - .where(Notification.id == notification_id) - .where(Notification.status != NOTIFICATION_DELIVERED) - .where(Notification.status != NOTIFICATION_PERMANENT_FAILURE) - ) - should_update = True - if new_status in (NOTIFICATION_CREATED, NOTIFICATION_SENDING, NOTIFICATION_PENDING): - # For these stauses, in addition to the stmt above, do not allow overriding sent/temp-failure - stmt = stmt.where(Notification.status != NOTIFICATION_SENT).where( - Notification.status != NOTIFICATION_TEMPORARY_FAILURE - ) - elif new_status not in ( - NOTIFICATION_DELIVERED, - NOTIFICATION_PERMANENT_FAILURE, - NOTIFICATION_SENT, - NOTIFICATION_TEMPORARY_FAILURE, - ): - # Do not run updates if it does not match the other cases - should_update = False - current_app.logger.warning('Did not find match for: %s - Not updating', new_status) - # Otherwise only use the base stmt that was set. - - if should_update: - try: - stmt = stmt.values(status=new_status, updated_at=datetime.utcnow()) - db.session.execute(stmt) - db.session.commit() - except Exception as e: - current_app.logger.critical('Updating notification %s to status "%s" failed.', notification_id, new_status) - current_app.logger.exception(e) - db.session.rollback() + try: + stmt = _get_notification_status_update_statement(notification_id=notification_id, incoming_status=new_status) + db.session.execute(stmt) + db.session.commit() + except Exception as e: + current_app.logger.critical('Updating notification %s to status "%s" failed.', notification_id, new_status) + current_app.logger.exception(e) + db.session.rollback() @statsd(namespace='dao') @@ -335,13 +331,17 @@ def update_notification_status_by_reference( def dao_update_notification_by_id( notification_id: str, **kwargs, -): +) -> Optional[Notification]: """ Update the notification by ID, ensure kwargs paramaters are named appropriately according to the notification model. - :param notification_id: The notification uuid in string form - :param kwargs: The notification key-value pairs to be updated - Note: Ensure keys are valid according to notification model - :return: Notification or None + + Args: + notification_id (str): The notification uuid in string form + kwargs: The notification key-value pairs to be updated. + **Note**: Ensure keys are valid according to notification model + + Returns: + Notification: The updated notification or None if the notification was not found """ kwargs['updated_at'] = datetime.utcnow() status = kwargs.get('status') @@ -365,7 +365,7 @@ def dao_update_notification_by_id( except ArgumentError: current_app.logger.exception('Cannot update notification %s', notification_id) return None - except: + except Exception: current_app.logger.exception( 'Unexpected exception thrown attempting to update notification %s, given parameters for updating ' 'notification may be incorrect', @@ -1006,15 +1006,21 @@ def guess_notification_type(search_term): def duplicate_update_warning( - notification, - status, -): - # This is a log, so this is not SQL injection + notification: Notification, + status: str, +) -> None: + """ + Log a warning when a notification status update is received after the notification has reached a final state. + + Args: + notification (Notification): The notification that will not be updated. + status (str): The status that the notification was attempting to be updated to. + """ time_diff = datetime.utcnow() - (notification.updated_at or notification.created_at) current_app.logger.info( - 'Duplicate callback received. Notification id %s received a status update to ' - '%s %s after being set to %s. %s sent by %s', + 'Duplicate callback received. Notification id %s received a status update to %s ' + '%s after being set to %s. %s sent by %s', notification.id, status, time_diff, diff --git a/tests/app/celery/test_process_pinpoint_receipt_tasks.py b/tests/app/celery/test_process_pinpoint_receipt_tasks.py index 4a489afb24..637b967078 100644 --- a/tests/app/celery/test_process_pinpoint_receipt_tasks.py +++ b/tests/app/celery/test_process_pinpoint_receipt_tasks.py @@ -126,7 +126,7 @@ def test_process_pinpoint_results_should_not_update_notification_status_if_uncha @pytest.mark.parametrize( 'status', [NOTIFICATION_DELIVERED, NOTIFICATION_PERMANENT_FAILURE, NOTIFICATION_TECHNICAL_FAILURE] ) -def test_process_pinpoint_results_should_not_update_notification_status_if_status_already_final( +def test_process_pinpoint_results_should_not_update_notification_status_to_sending_if_status_already_final( mocker, sample_template, status, @@ -153,6 +153,45 @@ def test_process_pinpoint_results_should_not_update_notification_status_if_statu mock_callback.assert_not_called() +@pytest.mark.parametrize( + 'status', [NOTIFICATION_DELIVERED, NOTIFICATION_PERMANENT_FAILURE, NOTIFICATION_TECHNICAL_FAILURE] +) +def test_process_pinpoint_results_should_update_notification_status_with_delivered( + mocker, + sample_template, + status, + sample_notification, +): + mocker.patch('app.celery.process_pinpoint_receipt_tasks.is_feature_enabled', return_value=True) + mock_callback = mocker.patch('app.celery.process_pinpoint_receipt_tasks.check_and_queue_callback_task') + update_notification_status = mocker.patch( + 'app.celery.process_pinpoint_receipt_tasks.update_notification_status_by_id' + ) + + update_notification_spy = mocker.spy(process_pinpoint_receipt_tasks, 'dao_update_notification_by_id') + + test_reference = f'{uuid4()}=sms-reference-1' + template = sample_template() + sample_notification(template=template, reference=test_reference, sent_at=datetime.datetime.utcnow(), status=status) + process_pinpoint_receipt_tasks.process_pinpoint_results( + response=pinpoint_notification_callback_record( + reference=test_reference, + event_type='_SMS.SUCCESS', + record_status='DELIVERED', + ) + ) + notification = notifications_dao.dao_get_notification_by_reference(test_reference) + assert notification.status == NOTIFICATION_DELIVERED + + update_notification_status.assert_not_called() + + if status == NOTIFICATION_DELIVERED: + mock_callback.assert_not_called() + else: + update_notification_spy.assert_called_once() + mock_callback.assert_called_once() + + def test_process_pinpoint_results_segments_and_price_buffered_first( mocker, sample_template, diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 4c024feef9..0a8315ff3c 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -902,6 +902,7 @@ def test_save_email_should_use_template_version_from_job_not_latest( ) +@pytest.mark.serial def test_should_use_email_template_subject_placeholders( notify_db_session, sample_template, @@ -914,15 +915,16 @@ def test_should_use_email_template_subject_placeholders( ) # Cleaned by sample_template - notification = _notification_json(template, 'my_email@my_email.com', {'name': 'Jo'}) + notification_data = _notification_json(template, 'my_email@my_email.com', {'name': 'Jo'}) mocker.patch('app.celery.provider_tasks.deliver_email.apply_async') notification_id = uuid4() now = datetime.utcnow() + # Intermittently makes the status 'technical-failure' save_email( template.service_id, notification_id, - encryption.encrypt(notification), + encryption.encrypt(notification_data), ) persisted_notification = notify_db_session.session.get(Notification, notification_id) diff --git a/tests/app/dao/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py index def3b858ae..c289a7adc0 100644 --- a/tests/app/dao/notification_dao/test_notification_dao.py +++ b/tests/app/dao/notification_dao/test_notification_dao.py @@ -80,45 +80,45 @@ def test_should_have_decorated_notifications_dao_functions(): assert dao_delete_notification_by_id.__wrapped__.__name__ == 'dao_delete_notification_by_id' # noqa -def test_should_by_able_to_update_status_by_reference(notify_db_session, sample_template): +def test_should_be_able_to_update_status_by_reference(notify_db_session, sample_template): template = sample_template(template_type=EMAIL_TYPE) - data = _notification_json(template, status='sending') + data = _notification_json(template, status=NOTIFICATION_SENDING) notification = Notification(**data) dao_create_notification(notification) db_notification = notify_db_session.session.get(Notification, notification.id) - assert db_notification.status == 'sending' + assert db_notification.status == NOTIFICATION_SENDING ref = str(uuid4()) notification.reference = ref dao_update_notification(notification) - updated = update_notification_status_by_reference(ref, 'delivered') - assert updated.status == 'delivered' - assert notify_db_session.session.get(Notification, notification.id).status == 'delivered' + updated = update_notification_status_by_reference(ref, NOTIFICATION_DELIVERED) + assert updated.status == NOTIFICATION_DELIVERED + assert notify_db_session.session.get(Notification, notification.id).status == NOTIFICATION_DELIVERED -def test_should_by_able_to_update_status_by_id(notify_db_session, sample_template, sample_job): +def test_should_be_able_to_update_status_by_id(notify_db_session, sample_template, sample_job): template = sample_template() job = sample_job(template) with freeze_time('2000-01-01 12:00:00'): - data = _notification_json(template, job_id=job.id, status='sending') + data = _notification_json(template, job_id=job.id, status=NOTIFICATION_SENDING) notification = Notification(**data) dao_create_notification(notification) try: - assert notification.status == 'sending' - assert notify_db_session.session.get(Notification, notification.id).status == 'sending' + assert notification.status == NOTIFICATION_SENDING + assert notify_db_session.session.get(Notification, notification.id).status == NOTIFICATION_SENDING with freeze_time('2000-01-02 12:00:00'): - updated = update_notification_status_by_id(notification.id, 'delivered') + updated = update_notification_status_by_id(notification.id, NOTIFICATION_DELIVERED) - assert updated.status == 'delivered' + assert updated.status == NOTIFICATION_DELIVERED assert updated.updated_at == datetime(2000, 1, 2, 12, 0, 0) - assert notify_db_session.session.get(Notification, notification.id).status == 'delivered' + assert notify_db_session.session.get(Notification, notification.id).status == NOTIFICATION_DELIVERED assert notification.updated_at == datetime(2000, 1, 2, 12, 0, 0) - assert notification.status == 'delivered' + assert notification.status == NOTIFICATION_DELIVERED finally: # Teardown notify_db_session.session.delete(notification) @@ -133,10 +133,10 @@ def test_should_not_update_status_by_id_if_not_sending_and_does_not_update_job( ): template = sample_template() job = sample_job(template) - notification = sample_notification(template=template, status='delivered', job=job) - assert notify_db_session.session.get(Notification, notification.id).status == 'delivered' - assert not update_notification_status_by_id(notification.id, 'failed') - assert notify_db_session.session.get(Notification, notification.id).status == 'delivered' + notification = sample_notification(template=template, status=NOTIFICATION_DELIVERED, job=job) + assert notify_db_session.session.get(Notification, notification.id).status == NOTIFICATION_DELIVERED + assert update_notification_status_by_id(notification.id, NOTIFICATION_PERMANENT_FAILURE) is not None + assert notify_db_session.session.get(Notification, notification.id).status == NOTIFICATION_DELIVERED assert job == notify_db_session.session.get(Job, notification.job_id) @@ -282,6 +282,7 @@ def test_should_by_able_to_update_status_by_id_from_pending_to_delivered( assert notify_db_session.session.get(Notification, notification.id).status == 'delivered' +@pytest.mark.skip(reason='firetext status update, not valid anymore') def test_should_by_able_to_update_status_by_id_from_pending_to_temporary_failure( notify_db_session, sample_template, @@ -1921,7 +1922,8 @@ def test_update_notification_status_by_id_cannot_exit_delivered_status_after_int update_notification_status_by_id(notification_id=notification.id, status=next_status) # record the last update value that is in the database - notification_last_updated = notification.updated_at + # or set to current time -1 second if no updated_at value + notification_last_updated = notification.updated_at or datetime.utcnow() - timedelta(seconds=1) # get the notification object and make sure it has the values you gave it # establish sending as the intermediate step @@ -2077,7 +2079,6 @@ def test_update_notification_status_by_id_can_update_status_in_order_when_given_ (NOTIFICATION_CREATED, NOTIFICATION_DELIVERED), (NOTIFICATION_CREATED, NOTIFICATION_TEMPORARY_FAILURE), (NOTIFICATION_CREATED, NOTIFICATION_PERMANENT_FAILURE), - (NOTIFICATION_PENDING, NOTIFICATION_SENDING), (NOTIFICATION_PENDING, NOTIFICATION_PENDING), (NOTIFICATION_PENDING, NOTIFICATION_SENT), (NOTIFICATION_PENDING, NOTIFICATION_DELIVERED), @@ -2097,6 +2098,7 @@ def test_update_notification_status_by_id_can_update_status_in_order_when_given_ (NOTIFICATION_SENT, NOTIFICATION_DELIVERED), (NOTIFICATION_SENT, NOTIFICATION_TEMPORARY_FAILURE), (NOTIFICATION_DELIVERED, NOTIFICATION_DELIVERED), + (NOTIFICATION_PERMANENT_FAILURE, NOTIFICATION_DELIVERED), ], ) def test_update_notification_delivery_status_valid_updates( @@ -2139,7 +2141,6 @@ def test_update_notification_delivery_status_valid_updates( (NOTIFICATION_PERMANENT_FAILURE, NOTIFICATION_PENDING), (NOTIFICATION_PERMANENT_FAILURE, NOTIFICATION_TEMPORARY_FAILURE), (NOTIFICATION_PERMANENT_FAILURE, NOTIFICATION_SENT), - (NOTIFICATION_PERMANENT_FAILURE, NOTIFICATION_DELIVERED), ], ) def test_update_notification_delivery_status_invalid_updates(