From 84e25d6b98bf23e577791dfe6de0583fdadb313e Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Wed, 17 Jan 2018 09:52:13 +0000 Subject: [PATCH] Compare letter page count with billable units in DVLA response file We compare the page_count in the response file we receive from the DVLA with the billable_units of the letter. If these don't match, we log an error. --- app/celery/tasks.py | 13 ++++++ app/dao/notifications_dao.py | 7 +++ tests/app/celery/test_ftp_update_tasks.py | 45 +++++++++++++++++++ .../notification_dao/test_notification_dao.py | 21 +++++++++ 4 files changed, 86 insertions(+) diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 3b065f9fc..8908496cf 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -45,6 +45,7 @@ from app.dao.notifications_dao import ( dao_update_notifications_for_job_to_sent_to_dvla, dao_update_notifications_by_reference, dao_get_last_notification_added_for_job_id, + dao_get_notification_by_reference, ) from app.dao.provider_details_dao import get_current_provider from app.dao.service_inbound_api_dao import get_service_inbound_api_for_service @@ -476,6 +477,8 @@ def update_letter_notifications_statuses(self, filename): raise else: for update in notification_updates: + check_billable_units(update) + status = NOTIFICATION_DELIVERED if update.status == DVLA_RESPONSE_STATUS_SENT \ else NOTIFICATION_TEMPORARY_FAILURE updated_count = dao_update_notifications_by_reference( @@ -503,6 +506,16 @@ def process_updates_from_file(response_file): return notification_updates +def check_billable_units(notification_update): + notification = dao_get_notification_by_reference(notification_update.reference) + + if int(notification_update.page_count) != notification.billable_units: + msg = 'Notification with id {} had {} billable_units but a page count of {}'.format( + notification.id, notification.billable_units, notification_update.page_count) + + current_app.logger.error(msg) + + @notify_celery.task(bind=True, name="send-inbound-sms", max_retries=5, default_retry_delay=300) @statsd(namespace="tasks") def send_inbound_sms_to_service(self, inbound_sms_id, service_id): diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 0d1c6e72a..61977b1c9 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -471,6 +471,13 @@ def dao_get_notifications_by_to_field(service_id, search_term, statuses=None): return results +@statsd(namespace="dao") +def dao_get_notification_by_reference(reference): + return Notification.query.filter( + Notification.reference == reference + ).one() + + @statsd(namespace="dao") def dao_get_notifications_by_references(references): return Notification.query.filter( diff --git a/tests/app/celery/test_ftp_update_tasks.py b/tests/app/celery/test_ftp_update_tasks.py index 135f8e240..9fceade95 100644 --- a/tests/app/celery/test_ftp_update_tasks.py +++ b/tests/app/celery/test_ftp_update_tasks.py @@ -1,3 +1,4 @@ +from collections import namedtuple from datetime import datetime import pytest @@ -14,6 +15,7 @@ from app.models import ( NOTIFICATION_TECHNICAL_FAILURE ) from app.celery.tasks import ( + check_billable_units, process_updates_from_file, update_dvla_job_to_error, update_job_to_sent_to_dvla, @@ -26,6 +28,15 @@ from tests.app.db import create_notification, create_service_callback_api from tests.conftest import set_config +@pytest.fixture +def notification_update(): + """ + Returns a namedtuple to use as the argument for the check_billable_units function + """ + NotificationUpdate = namedtuple('NotificationUpdate', ['reference', 'status', 'page_count', 'cost_threshold']) + return NotificationUpdate('REFERENCE_ABC', 'sent', '1', 'cost') + + def test_update_job_to_sent_to_dvla(sample_letter_template, sample_letter_job): create_notification(template=sample_letter_template, job=sample_letter_job) create_notification(template=sample_letter_template, job=sample_letter_job) @@ -163,3 +174,37 @@ def test_update_letter_notifications_to_error_updates_based_on_notification_refe assert first.sent_at is None assert first.updated_at == dt assert second.status == NOTIFICATION_CREATED + + +def test_check_billable_units_when_billable_units_matches_page_count( + client, + sample_letter_template, + mocker, + notification_update +): + mock_logger = mocker.patch('app.celery.tasks.current_app.logger.error') + + notification = create_notification(sample_letter_template, reference='REFERENCE_ABC') + notification.billable_units = 1 + + check_billable_units(notification_update) + + mock_logger.assert_not_called() + + +def test_check_billable_units_when_billable_units_does_not_match_page_count( + client, + sample_letter_template, + mocker, + notification_update +): + mock_logger = mocker.patch('app.celery.tasks.current_app.logger.error') + + notification = create_notification(sample_letter_template, reference='REFERENCE_ABC') + notification.billable_units = 3 + + check_billable_units(notification_update) + + mock_logger.assert_called_once_with( + 'Notification with id {} had 3 billable_units but a page count of 1'.format(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 7d5083307..43b738d77 100644 --- a/tests/app/dao/notification_dao/test_notification_dao.py +++ b/tests/app/dao/notification_dao/test_notification_dao.py @@ -31,6 +31,7 @@ from app.dao.notifications_dao import ( set_scheduled_notification_to_processed, update_notification_status_by_id, update_notification_status_by_reference, + dao_get_notification_by_reference, dao_get_notifications_by_references ) from app.dao.services_dao import dao_update_service @@ -1994,6 +1995,26 @@ def test_dao_update_notifications_by_reference_returns_zero_when_no_notification assert updated_count == 0 +def test_dao_get_notification_by_reference_with_one_match_returns_notification(sample_letter_template, notify_db): + create_notification(template=sample_letter_template, reference='REF1') + notification = dao_get_notification_by_reference('REF1') + + assert notification.reference == 'REF1' + + +def test_dao_get_notification_by_reference_with_multiple_matches_raises_error(sample_letter_template, notify_db): + create_notification(template=sample_letter_template, reference='REF1') + create_notification(template=sample_letter_template, reference='REF1') + + with pytest.raises(SQLAlchemyError): + dao_get_notification_by_reference('REF1') + + +def test_dao_get_notification_by_reference_with_no_matches_raises_error(notify_db): + with pytest.raises(SQLAlchemyError): + dao_get_notification_by_reference('REF1') + + def test_dao_get_notifications_by_reference(sample_template): create_notification(template=sample_template, reference='noref') notification_1 = create_notification(template=sample_template, reference='ref')