diff --git a/app/celery/research_mode_tasks.py b/app/celery/research_mode_tasks.py index 0e0586466..ac2e1e8dd 100644 --- a/app/celery/research_mode_tasks.py +++ b/app/celery/research_mode_tasks.py @@ -123,7 +123,7 @@ def firetext_callback(notification_id, to): def create_fake_letter_response_file(self, reference): now = datetime.utcnow() dvla_response_data = '{}|Sent|0|Sorted'.format(reference) - upload_file_name = 'NOTIFY.{}.RSP.TXT'.format(now.strftime('%Y%m%d%H%M%S')) + upload_file_name = 'NOTIFY-{}-RSP.TXT'.format(now.strftime('%Y%m%d%H%M%S')) s3upload( filedata=dvla_response_data, diff --git a/app/celery/tasks.py b/app/celery/tasks.py index d7f3d7a15..167bcf58b 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -420,26 +420,27 @@ 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): - datetime_string = filename.split('.')[1] + datetime_string = filename.split('-')[1] datetime_obj = datetime.strptime(datetime_string, '%Y%m%d%H%M%S') return convert_utc_to_bst(datetime_obj).date() diff --git a/tests/app/celery/test_ftp_update_tasks.py b/tests/app/celery/test_ftp_update_tasks.py index 435ccab93..cffb22e28 100644 --- a/tests/app/celery/test_ftp_update_tasks.py +++ b/tests/app/celery/test_ftp_update_tasks.py @@ -59,8 +59,8 @@ def test_update_letter_notifications_statuses_raises_for_invalid_format(notify_a mocker.patch('app.celery.tasks.s3.get_s3_file', return_value=invalid_file) with pytest.raises(DVLAException) as e: - update_letter_notifications_statuses(filename='NOTIFY.20170823160812.RSP.TXT') - assert 'DVLA response file: {} has an invalid format'.format('NOTIFY.20170823160812.RSP.TXT') in str(e) + update_letter_notifications_statuses(filename='NOTIFY-20170823160812-RSP.TXT') + assert 'DVLA response file: {} has an invalid format'.format('NOTIFY-20170823160812-RSP.TXT') in str(e) def test_update_letter_notification_statuses_when_notification_does_not_exist_updates_notification_history( @@ -73,7 +73,7 @@ def test_update_letter_notification_statuses_when_notification_does_not_exist_up billable_units=1) Notification.query.filter_by(id=notification.id).delete() - update_letter_notifications_statuses(filename="NOTIFY.20170823160812.RSP.TXT") + update_letter_notifications_statuses(filename="NOTIFY-20170823160812-RSP.TXT") updated_history = NotificationHistory.query.filter_by(id=notification.id).one() assert updated_history.status == NOTIFICATION_DELIVERED @@ -106,10 +106,29 @@ def test_update_letter_notifications_statuses_raises_error_for_unknown_sorted_st mocker.patch('app.celery.tasks.s3.get_s3_file', return_value=valid_file) with pytest.raises(DVLAException) as e: - update_letter_notifications_statuses(filename='NOTIFY.20170823160812.RSP.TXT') + update_letter_notifications_statuses(filename='NOTIFY-20170823160812-RSP.TXT') assert "DVLA response file: {filename} contains unknown Sorted status {unknown_status}".format( - filename="NOTIFY.20170823160812.RSP.TXT", unknown_status="{'Error'}" + filename="NOTIFY-20170823160812-RSP.TXT", unknown_status="{'Error'}" + ) 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) @@ -117,10 +136,10 @@ def test_update_letter_notifications_statuses_calls_with_correct_bucket_location s3_mock = mocker.patch('app.celery.tasks.s3.get_s3_object') with set_config(notify_api, 'NOTIFY_EMAIL_DOMAIN', 'foo.bar'): - update_letter_notifications_statuses(filename='NOTIFY.20170823160812.RSP.TXT') + update_letter_notifications_statuses(filename='NOTIFY-20170823160812-RSP.TXT') s3_mock.assert_called_with('{}-ftp'.format( current_app.config['NOTIFY_EMAIL_DOMAIN']), - 'NOTIFY.20170823160812.RSP.TXT' + 'NOTIFY-20170823160812-RSP.TXT' ) @@ -129,7 +148,7 @@ def test_update_letter_notifications_statuses_builds_updates_from_content(notify mocker.patch('app.celery.tasks.s3.get_s3_file', return_value=valid_file) update_mock = mocker.patch('app.celery.tasks.process_updates_from_file') - update_letter_notifications_statuses(filename='NOTIFY.20170823160812.RSP.TXT') + update_letter_notifications_statuses(filename='NOTIFY-20170823160812-RSP.TXT') update_mock.assert_called_with('ref-foo|Sent|1|Unsorted\nref-bar|Sent|2|Sorted') @@ -162,7 +181,7 @@ def test_update_letter_notifications_statuses_persisted(notify_api, mocker, samp mocker.patch('app.celery.tasks.s3.get_s3_file', return_value=valid_file) with pytest.raises(expected_exception=DVLAException) as e: - update_letter_notifications_statuses(filename='NOTIFY.20170823160812.RSP.TXT') + update_letter_notifications_statuses(filename='NOTIFY-20170823160812-RSP.TXT') assert sent_letter.status == NOTIFICATION_DELIVERED assert sent_letter.billable_units == 1 @@ -171,7 +190,7 @@ def test_update_letter_notifications_statuses_persisted(notify_api, mocker, samp assert failed_letter.billable_units == 2 assert failed_letter.updated_at assert "DVLA response file: {filename} has failed letters with notification.reference {failures}".format( - filename="NOTIFY.20170823160812.RSP.TXT", failures=[format(failed_letter.reference)]) in str(e) + filename="NOTIFY-20170823160812-RSP.TXT", failures=[format(failed_letter.reference)]) in str(e) def test_update_letter_notifications_statuses_persists_daily_sorted_letter_count( @@ -187,7 +206,7 @@ def test_update_letter_notifications_statuses_persists_daily_sorted_letter_count mocker.patch('app.celery.tasks.s3.get_s3_file', return_value=valid_file) persist_letter_count_mock = mocker.patch('app.celery.tasks.persist_daily_sorted_letter_counts') - update_letter_notifications_statuses(filename='NOTIFY.20170823160812.RSP.TXT') + update_letter_notifications_statuses(filename='NOTIFY-20170823160812-RSP.TXT') persist_letter_count_mock.assert_called_once_with(date(2017, 8, 23), {'Unsorted': 1, 'Sorted': 1}) @@ -204,7 +223,7 @@ def test_update_letter_notifications_statuses_persists_daily_sorted_letter_count sent_letter_1.reference, sent_letter_2.reference) mocker.patch('app.celery.tasks.s3.get_s3_file', return_value=valid_file) - update_letter_notifications_statuses(filename='NOTIFY.20170823160812.RSP.TXT') + update_letter_notifications_statuses(filename='NOTIFY-20170823160812-RSP.TXT') daily_sorted_letter = dao_get_daily_sorted_letter_by_billing_day(date(2017, 8, 23)) @@ -223,7 +242,7 @@ def test_update_letter_notifications_does_not_call_send_callback_if_no_db_entry( 'app.celery.service_callback_tasks.send_delivery_status_to_service.apply_async' ) - update_letter_notifications_statuses(filename='NOTIFY.20170823160812.RSP.TXT') + update_letter_notifications_statuses(filename='NOTIFY-20170823160812-RSP.TXT') send_mock.assert_not_called() @@ -301,7 +320,7 @@ def test_check_billable_units_when_billable_units_does_not_match_page_count( ('20170120230000', date(2017, 1, 20)) ]) def test_get_billing_date_in_bst_from_filename(filename_date, billing_date): - filename = 'NOTIFY.{}.RSP.TXT'.format(filename_date) + filename = 'NOTIFY-{}-RSP.TXT'.format(filename_date) result = get_billing_date_in_bst_from_filename(filename) assert result == billing_date diff --git a/tests/app/celery/test_research_mode_tasks.py b/tests/app/celery/test_research_mode_tasks.py index 7c1b7d1f3..77656a62e 100644 --- a/tests/app/celery/test_research_mode_tasks.py +++ b/tests/app/celery/test_research_mode_tasks.py @@ -112,7 +112,7 @@ def test_failure_firetext_callback(phone_number): def test_create_fake_letter_response_file_uploads_response_file_s3( notify_api, mocker): mock_s3upload = mocker.patch('app.celery.research_mode_tasks.s3upload') - filename = 'NOTIFY.20180125140000.RSP.TXT' + filename = 'NOTIFY-20180125140000-RSP.TXT' with requests_mock.Mocker() as request_mock: request_mock.post( @@ -135,7 +135,7 @@ def test_create_fake_letter_response_file_uploads_response_file_s3( def test_create_fake_letter_response_file_calls_dvla_callback_on_development( notify_api, mocker): mocker.patch('app.celery.research_mode_tasks.s3upload') - filename = 'NOTIFY.20180125140000.RSP.TXT' + filename = 'NOTIFY-20180125140000-RSP.TXT' with set_config_values(notify_api, { 'NOTIFY_ENVIRONMENT': 'development' diff --git a/tests/app/notifications/rest/test_callbacks.py b/tests/app/notifications/rest/test_callbacks.py index 1654b3b68..9dc5bfa1c 100644 --- a/tests/app/notifications/rest/test_callbacks.py +++ b/tests/app/notifications/rest/test_callbacks.py @@ -96,12 +96,12 @@ def test_dvla_rs_txt_file_callback_calls_update_letter_notifications_task(client def test_dvla_rsp_txt_file_callback_calls_update_letter_notifications_task(client, mocker): update_task = \ mocker.patch('app.notifications.notifications_letter_callback.update_letter_notifications_statuses.apply_async') - data = _sample_sns_s3_callback('NOTIFY.20170823160812.RSP.TXT') + data = _sample_sns_s3_callback('NOTIFY-20170823160812-RSP.TXT') response = dvla_post(client, data) assert response.status_code == 200 assert update_task.called - update_task.assert_called_with(['NOTIFY.20170823160812.RSP.TXT'], queue='notify-internal-tasks') + update_task.assert_called_with(['NOTIFY-20170823160812-RSP.TXT'], queue='notify-internal-tasks') def test_dvla_ack_calls_does_not_call_letter_notifications_task(client, mocker):