diff --git a/app/celery/tasks.py b/app/celery/tasks.py index d7f3d7a15..f9c68d874 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -420,22 +420,23 @@ def update_letter_notifications_statuses(self, filename): update_letter_notification(filename, temporary_failures, update) sorted_letter_counts[update.cost_threshold] += 1 - if temporary_failures: - # This will alert Notify that DVLA was unable to deliver the letters, we need to investigate - message = "DVLA response file: {filename} has failed letters with notification.reference {failures}".format( - filename=filename, failures=temporary_failures) - raise DVLAException(message) + try: + if sorted_letter_counts.keys() - {'Unsorted', 'Sorted'}: + unknown_status = sorted_letter_counts.keys() - {'Unsorted', 'Sorted'} - if sorted_letter_counts.keys() - {'Unsorted', 'Sorted'}: - unknown_status = sorted_letter_counts.keys() - {'Unsorted', 'Sorted'} + message = 'DVLA response file: {} contains unknown Sorted status {}'.format( + filename, unknown_status + ) + raise DVLAException(message) - message = 'DVLA response file: {} contains unknown Sorted status {}'.format( - filename, unknown_status - ) - raise DVLAException(message) - - billing_date = get_billing_date_in_bst_from_filename(filename) - persist_daily_sorted_letter_counts(billing_date, sorted_letter_counts) + billing_date = get_billing_date_in_bst_from_filename(filename) + persist_daily_sorted_letter_counts(billing_date, sorted_letter_counts) + finally: + if temporary_failures: + # This will alert Notify that DVLA was unable to deliver the letters, we need to investigate + message = "DVLA response file: {filename} has failed letters with notification.reference {failures}" \ + .format(filename=filename, failures=temporary_failures) + raise DVLAException(message) def get_billing_date_in_bst_from_filename(filename): diff --git a/tests/app/celery/test_ftp_update_tasks.py b/tests/app/celery/test_ftp_update_tasks.py index 435ccab93..5dc714046 100644 --- a/tests/app/celery/test_ftp_update_tasks.py +++ b/tests/app/celery/test_ftp_update_tasks.py @@ -113,6 +113,25 @@ def test_update_letter_notifications_statuses_raises_error_for_unknown_sorted_st ) in str(e) +def test_update_letter_notifications_statuses_still_raises_temp_failure_error_with_unknown_sorted_status( + notify_api, + mocker, + sample_letter_template +): + valid_file = 'ref-foo|Failed|1|unknown' + mocker.patch('app.celery.tasks.s3.get_s3_file', return_value=valid_file) + create_notification(sample_letter_template, reference='ref-foo', status=NOTIFICATION_SENDING, + billable_units=0) + + with pytest.raises(DVLAException) as e: + update_letter_notifications_statuses(filename="failed.txt") + + failed = ["ref-foo"] + assert "DVLA response file: {filename} has failed letters with notification.reference {failures}".format( + filename="failed.txt", failures=failed + ) in str(e) + + def test_update_letter_notifications_statuses_calls_with_correct_bucket_location(notify_api, mocker): s3_mock = mocker.patch('app.celery.tasks.s3.get_s3_object')