diff --git a/app/celery/tasks.py b/app/celery/tasks.py index bece59f7c..a042d7f0a 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -36,6 +36,7 @@ from app.dao.jobs_dao import ( dao_update_job_status) from app.dao.notifications_dao import ( get_notification_by_id, + update_notification_status_by_reference, dao_update_notifications_for_job_to_sent_to_dvla, dao_update_notifications_by_reference, dao_get_last_notification_added_for_job_id) @@ -46,6 +47,7 @@ from app.dao.templates_dao import dao_get_template_by_id from app.models import ( Job, Notification, + DVLA_RESPONSE_STATUS_SENT, EMAIL_TYPE, JOB_STATUS_CANCELLED, JOB_STATUS_FINISHED, @@ -55,6 +57,7 @@ from app.models import ( JOB_STATUS_SENT_TO_DVLA, JOB_STATUS_ERROR, KEY_TYPE_NORMAL, LETTER_TYPE, + NOTIFICATION_DELIVERED, NOTIFICATION_SENDING, NOTIFICATION_TECHNICAL_FAILURE, SMS_TYPE, @@ -443,8 +446,22 @@ def update_letter_notifications_statuses(self, filename): raise else: for update in notification_updates: - current_app.logger.info('DVLA update: {}'.format(str(update))) - # TODO: Update notifications with desired status + status = NOTIFICATION_DELIVERED if update.status == DVLA_RESPONSE_STATUS_SENT \ + else NOTIFICATION_TECHNICAL_FAILURE + notification = update_notification_status_by_reference( + update.reference, + status + ) + + if not notification: + msg = "Update letter notification file {filename} failed: notification either not found " \ + "or already updated from delivered. Status {status} for notification reference {reference}".format( + filename=filename, status=status, reference=update.reference) + current_app.logger.error(msg) + else: + current_app.logger.info( + 'DVLA file: {filename}, notification updated to {status}: {reference}'.format( + filename=filename, status=status, reference=str(update.reference))) def process_updates_from_file(response_file): diff --git a/app/models.py b/app/models.py index 645b5490f..8df375279 100644 --- a/app/models.py +++ b/app/models.py @@ -873,7 +873,9 @@ NOTIFICATION_STATUS_TYPES_NON_BILLABLE = list(set(NOTIFICATION_STATUS_TYPES) - s NOTIFICATION_STATUS_TYPES_ENUM = db.Enum(*NOTIFICATION_STATUS_TYPES, name='notify_status_type') NOTIFICATION_STATUS_LETTER_ACCEPTED = 'accepted' -NOTIFICATION_STATUS_LETTER_ACCEPTED_PRETTY = 'Accepted' +NOTIFICATION_STATUS_LETTER_RECEIVED = 'received' + +DVLA_RESPONSE_STATUS_SENT = 'Sent' class NotificationStatusTypes(db.Model): @@ -984,6 +986,14 @@ class Notification(db.Model): ['technical-failure', 'temporary-failure', 'permanent-failure', 'created', 'sending'] + - + + > IN + 'delivered' + + < OUT + ['received'] + :param status_or_statuses: a single status or list of statuses :return: a single status or list with the current failure statuses substituted for 'failure' """ @@ -992,6 +1002,7 @@ class Notification(db.Model): return ( NOTIFICATION_STATUS_TYPES_FAILED if _status == NOTIFICATION_FAILED else [NOTIFICATION_CREATED, NOTIFICATION_SENDING] if _status == NOTIFICATION_STATUS_LETTER_ACCEPTED else + NOTIFICATION_DELIVERED if _status == NOTIFICATION_STATUS_LETTER_RECEIVED else [_status] ) @@ -1040,8 +1051,9 @@ class Notification(db.Model): }, 'letter': { 'technical-failure': 'Technical failure', - 'sending': NOTIFICATION_STATUS_LETTER_ACCEPTED_PRETTY, - 'created': NOTIFICATION_STATUS_LETTER_ACCEPTED_PRETTY, + 'sending': 'Accepted', + 'created': 'Accepted', + 'delivered': 'Received' } }[self.template.template_type].get(self.status, self.status) @@ -1056,8 +1068,10 @@ class Notification(db.Model): # get the two code flows mixed up at all assert self.notification_type == LETTER_TYPE - if self.status == NOTIFICATION_CREATED or NOTIFICATION_SENDING: + if self.status in [NOTIFICATION_CREATED, NOTIFICATION_SENDING]: return NOTIFICATION_STATUS_LETTER_ACCEPTED + elif self.status == NOTIFICATION_DELIVERED: + return NOTIFICATION_STATUS_LETTER_RECEIVED else: # Currently can only be technical-failure return self.status diff --git a/app/v2/notifications/notification_schemas.py b/app/v2/notifications/notification_schemas.py index 2eeddc532..1689a7319 100644 --- a/app/v2/notifications/notification_schemas.py +++ b/app/v2/notifications/notification_schemas.py @@ -1,4 +1,9 @@ -from app.models import NOTIFICATION_STATUS_TYPES, NOTIFICATION_STATUS_LETTER_ACCEPTED, TEMPLATE_TYPES +from app.models import ( + NOTIFICATION_STATUS_TYPES, + NOTIFICATION_STATUS_LETTER_ACCEPTED, + NOTIFICATION_STATUS_LETTER_RECEIVED, + TEMPLATE_TYPES +) from app.schema_validation.definitions import (uuid, personalisation, letter_personalisation) @@ -59,7 +64,8 @@ get_notifications_request = { "status": { "type": "array", "items": { - "enum": NOTIFICATION_STATUS_TYPES + [NOTIFICATION_STATUS_LETTER_ACCEPTED] + "enum": NOTIFICATION_STATUS_TYPES + + [NOTIFICATION_STATUS_LETTER_ACCEPTED + ', ' + NOTIFICATION_STATUS_LETTER_RECEIVED] } }, "template_type": { diff --git a/tests/app/celery/test_ftp_update_tasks.py b/tests/app/celery/test_ftp_update_tasks.py index dc96ac91d..eef0c8c36 100644 --- a/tests/app/celery/test_ftp_update_tasks.py +++ b/tests/app/celery/test_ftp_update_tasks.py @@ -7,17 +7,21 @@ from flask import current_app from app.models import ( Job, Notification, - NOTIFICATION_SENDING, NOTIFICATION_CREATED, + NOTIFICATION_DELIVERED, + NOTIFICATION_TECHNICAL_FAILURE, + NOTIFICATION_SENDING, + NOTIFICATION_STATUS_LETTER_RECEIVED, NOTIFICATION_TECHNICAL_FAILURE ) +from app.dao.notifications_dao import dao_update_notifications_by_reference from app.celery.tasks import ( - update_job_to_sent_to_dvla, + process_updates_from_file, update_dvla_job_to_error, + update_job_to_sent_to_dvla, update_letter_notifications_statuses, - update_letter_notifications_to_sent_to_dvla, update_letter_notifications_to_error, - process_updates_from_file + update_letter_notifications_to_sent_to_dvla ) from tests.app.db import create_notification @@ -91,6 +95,20 @@ def test_update_letter_notifications_statuses_builds_updates_list(notify_api, mo assert updates[1].cost_threshold == 'Sorted' +def test_update_letter_notifications_statuses_persisted(notify_api, mocker, sample_letter_template): + sent_letter = create_notification(sample_letter_template, reference='ref-foo', status=NOTIFICATION_SENDING) + failed_letter = create_notification(sample_letter_template, reference='ref-bar', status=NOTIFICATION_SENDING) + + valid_file = '{}|Sent|1|Unsorted\n{}|Failed|2|Sorted'.format( + sent_letter.reference, failed_letter.reference) + mocker.patch('app.celery.tasks.s3.get_s3_file', return_value=valid_file) + + update_letter_notifications_statuses(filename='foo.txt') + + assert sent_letter.status == NOTIFICATION_DELIVERED + assert failed_letter.status == NOTIFICATION_TECHNICAL_FAILURE + + def test_update_letter_notifications_to_sent_to_dvla_updates_based_on_notification_references( client, sample_letter_template diff --git a/tests/app/test_model.py b/tests/app/test_model.py index 5ae310a19..8809a51bb 100644 --- a/tests/app/test_model.py +++ b/tests/app/test_model.py @@ -10,12 +10,14 @@ from app.models import ( MOBILE_TYPE, EMAIL_TYPE, NOTIFICATION_CREATED, + NOTIFICATION_DELIVERED, NOTIFICATION_SENDING, NOTIFICATION_PENDING, NOTIFICATION_FAILED, - NOTIFICATION_TECHNICAL_FAILURE, + NOTIFICATION_STATUS_LETTER_ACCEPTED, + NOTIFICATION_STATUS_LETTER_RECEIVED, NOTIFICATION_STATUS_TYPES_FAILED, - NOTIFICATION_STATUS_LETTER_ACCEPTED + NOTIFICATION_TECHNICAL_FAILURE ) from tests.app.conftest import ( sample_template as create_sample_template, @@ -71,6 +73,7 @@ def test_should_not_build_service_whitelist_from_invalid_contact(recipient_type, ([NOTIFICATION_FAILED], NOTIFICATION_STATUS_TYPES_FAILED), ([NOTIFICATION_CREATED], [NOTIFICATION_CREATED]), ([NOTIFICATION_TECHNICAL_FAILURE], [NOTIFICATION_TECHNICAL_FAILURE]), + (NOTIFICATION_STATUS_LETTER_RECEIVED, NOTIFICATION_DELIVERED), # passing in lists containing multiple statuses ([NOTIFICATION_FAILED, NOTIFICATION_CREATED], NOTIFICATION_STATUS_TYPES_FAILED + [NOTIFICATION_CREATED]), ([NOTIFICATION_CREATED, NOTIFICATION_PENDING], [NOTIFICATION_CREATED, NOTIFICATION_PENDING]), @@ -132,7 +135,8 @@ def test_notification_for_csv_returns_correct_job_row_number(notify_db, notify_d ('sms', 'sent', 'Sent internationally'), ('letter', 'created', 'Accepted'), ('letter', 'sending', 'Accepted'), - ('letter', 'technical-failure', 'Technical failure') + ('letter', 'technical-failure', 'Technical failure'), + ('letter', 'delivered', 'Received') ]) def test_notification_for_csv_returns_formatted_status( notify_db, diff --git a/tests/app/v2/notifications/test_get_notifications.py b/tests/app/v2/notifications/test_get_notifications.py index 926c8c1d8..d4306ba93 100644 --- a/tests/app/v2/notifications/test_get_notifications.py +++ b/tests/app/v2/notifications/test_get_notifications.py @@ -393,7 +393,7 @@ def test_get_all_notifications_filter_by_status_invalid_status(client, sample_no assert json_response['status_code'] == 400 assert len(json_response['errors']) == 1 assert json_response['errors'][0]['message'] == "status elephant is not one of [created, sending, sent, " \ - "delivered, pending, failed, technical-failure, temporary-failure, permanent-failure, accepted]" + "delivered, pending, failed, technical-failure, temporary-failure, permanent-failure, accepted, received]" def test_get_all_notifications_filter_by_multiple_statuses(client, sample_template): @@ -583,14 +583,25 @@ def test_get_all_notifications_renames_letter_statuses( pytest.fail() -def test_get_notifications_renames_letter_statuses(client, sample_letter_notification): - auth_header = create_authorization_header(service_id=sample_letter_notification.service_id) +@pytest.mark.parametrize('db_status,expected_status', [ + ('created', 'accepted'), + ('sending', 'accepted'), + ('delivered', 'received'), + ('pending', 'pending'), + ('technical-failure', 'technical-failure') +]) +def test_get_notifications_renames_letter_statuses(client, sample_letter_template, db_status, expected_status): + letter_noti = create_notification( + sample_letter_template, + status=db_status, + personalisation={'address_line_1': 'Mr Foo', 'address_line_2': '1 Bar Street', 'postcode': 'N1'} + ) + auth_header = create_authorization_header(service_id=letter_noti.service_id) response = client.get( - path=url_for('v2_notifications.get_notification_by_id', id=sample_letter_notification.id), + path=url_for('v2_notifications.get_notification_by_id', id=letter_noti.id), headers=[('Content-Type', 'application/json'), auth_header] ) json_response = json.loads(response.get_data(as_text=True)) assert response.status_code == 200 - - assert json_response['status'] == 'accepted' + assert json_response['status'] == expected_status diff --git a/tests/app/v2/notifications/test_notification_schemas.py b/tests/app/v2/notifications/test_notification_schemas.py index c667def11..6b51b195d 100644 --- a/tests/app/v2/notifications/test_notification_schemas.py +++ b/tests/app/v2/notifications/test_notification_schemas.py @@ -26,7 +26,7 @@ def test_get_notifications_request_invalid_statuses( ): partial_error_status = "is not one of " \ "[created, sending, sent, delivered, pending, failed, " \ - "technical-failure, temporary-failure, permanent-failure, accepted]" + "technical-failure, temporary-failure, permanent-failure, accepted, received]" with pytest.raises(ValidationError) as e: validate({'status': invalid_statuses + valid_statuses}, get_notifications_request) @@ -73,7 +73,7 @@ def test_get_notifications_request_invalid_statuses_and_template_types(): error_messages = [error['message'] for error in errors] for invalid_status in ["elephant", "giraffe"]: assert "status {} is not one of [created, sending, sent, delivered, " \ - "pending, failed, technical-failure, temporary-failure, permanent-failure, accepted]".format( + "pending, failed, technical-failure, temporary-failure, permanent-failure, accepted, received]".format( invalid_status ) in error_messages