diff --git a/app/celery/letters_pdf_tasks.py b/app/celery/letters_pdf_tasks.py index 19e627c8a..6e9bc6210 100644 --- a/app/celery/letters_pdf_tasks.py +++ b/app/celery/letters_pdf_tasks.py @@ -116,6 +116,16 @@ def update_billable_units_for_letter(self, notification_id, page_count): ) +@notify_celery.task( + bind=True, name="update-validation-failed-for-templated-letter", max_retries=15, default_retry_delay=300 +) +def update_validation_failed_for_templated_letter(self, notification_id, page_count): + notification = get_notification_by_id(notification_id, _raise=True) + notification.status = NOTIFICATION_VALIDATION_FAILED + dao_update_notification(notification) + current_app.logger.info(f"Validation failed: letter is too long {page_count} for letter with id: {notification_id}") + + @notify_celery.task(name='collate-letter-pdfs-to-be-sent') @cronitor("collate-letter-pdfs-to-be-sent") def collate_letter_pdfs_to_be_sent(): diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 6b4c3baac..9f6826790 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -711,7 +711,8 @@ def dao_get_letters_to_be_printed(print_run_deadline, postage, query_limit=10000 Notification.notification_type == LETTER_TYPE, Notification.status == NOTIFICATION_CREATED, Notification.key_type == KEY_TYPE_NORMAL, - Notification.postage == postage + Notification.postage == postage, + Notification.billable_units > 0 ).order_by( Notification.service_id, Notification.created_at @@ -729,6 +730,7 @@ def dao_get_letters_and_sheets_volume_by_postage(print_run_deadline): Notification.notification_type == LETTER_TYPE, Notification.status == NOTIFICATION_CREATED, Notification.key_type == KEY_TYPE_NORMAL, + Notification.billable_units > 0 ).group_by( Notification.postage ).order_by( diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index c5ea8d2b3..f2c82d2a2 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -394,6 +394,7 @@ def process_letter_notification( (notification.reference,), queue=queue ) + resp = create_response_for_post_notification( notification_id=notification.id, client_reference=notification.client_reference, diff --git a/tests/app/celery/test_letters_pdf_tasks.py b/tests/app/celery/test_letters_pdf_tasks.py index 02aae3ce2..a2419508a 100644 --- a/tests/app/celery/test_letters_pdf_tasks.py +++ b/tests/app/celery/test_letters_pdf_tasks.py @@ -27,6 +27,7 @@ from app.celery.letters_pdf_tasks import ( sanitise_letter, send_letters_volume_email_to_dvla, update_billable_units_for_letter, + update_validation_failed_for_templated_letter, ) from app.config import QueueNames, TaskNames from app.dao.notifications_dao import get_notifications @@ -145,22 +146,26 @@ def test_get_pdf_for_templated_letter_sets_technical_failure_max_retries(mocker, mock_update_noti.assert_called_once_with(sample_letter_notification.id, 'technical-failure') -@pytest.mark.parametrize('number_of_pages, expected_billable_units', [(2, 1), (3, 2), (10, 5)]) -def test_update_billable_units_for_letter(mocker, sample_letter_notification, number_of_pages, expected_billable_units): +def test_update_billable_units_for_letter( + mocker, sample_letter_notification +): sample_letter_notification.billable_units = 0 mock_logger = mocker.patch('app.celery.tasks.current_app.logger.info') - update_billable_units_for_letter(sample_letter_notification.id, number_of_pages) + update_billable_units_for_letter(sample_letter_notification.id, 4) notification = Notification.query.filter(Notification.reference == sample_letter_notification.reference).one() - assert notification.billable_units == expected_billable_units + assert notification.billable_units == 2 + assert sample_letter_notification.status == NOTIFICATION_CREATED mock_logger.assert_called_once_with( f"Letter notification id: {sample_letter_notification.id} reference {sample_letter_notification.reference}:" - f" billable units set to {expected_billable_units}" + f" billable units set to 2" ) -def test_update_billable_units_for_letter_doesnt_update_if_sent_with_test_key(mocker, sample_letter_notification): +def test_update_billable_units_for_letter_doesnt_update_if_sent_with_test_key( + mocker, sample_letter_notification +): sample_letter_notification.billable_units = 0 sample_letter_notification.key_type = KEY_TYPE_TEST mock_logger = mocker.patch('app.celery.tasks.current_app.logger.info') @@ -172,6 +177,23 @@ def test_update_billable_units_for_letter_doesnt_update_if_sent_with_test_key(mo mock_logger.assert_not_called() +@pytest.mark.parametrize('key_type', ['test', 'normal', 'team']) +def test_update_validation_failed_for_templated_letter_with_too_many_pages( + mocker, sample_letter_notification, key_type +): + sample_letter_notification.billable_units = 0 + sample_letter_notification.key_type = key_type + mock_logger = mocker.patch('app.celery.tasks.current_app.logger.info') + + update_validation_failed_for_templated_letter(sample_letter_notification.id, 11) + + assert sample_letter_notification.billable_units == 0 + assert sample_letter_notification.status == NOTIFICATION_VALIDATION_FAILED + mock_logger.assert_called_once_with( + f"Validation failed: letter is too long 11 for letter with id: {sample_letter_notification.id}" + ) + + @mock_s3 @freeze_time('2020-02-17 18:00:00') def test_get_key_and_size_of_letters_to_be_sent_to_print( diff --git a/tests/app/dao/notification_dao/test_notification_dao.py b/tests/app/dao/notification_dao/test_notification_dao.py index bafc9861c..e4776135d 100644 --- a/tests/app/dao/notification_dao/test_notification_dao.py +++ b/tests/app/dao/notification_dao/test_notification_dao.py @@ -1698,20 +1698,35 @@ def test_letters_to_be_printed_sort_by_service(notify_db_session): assert [x.id for x in results] == [x.id for x in letters_ordered_by_service_then_time] +def test_letters_to_be_printed_does_not_include_letters_without_billable_units_set( + notify_db_session, sample_letter_template): + included_letter = create_notification( + template=sample_letter_template, created_at=datetime(2020, 12, 1, 9, 30), billable_units=3) + create_notification( + template=sample_letter_template, created_at=datetime(2020, 12, 1, 9, 31), billable_units=0) + + results = list( + dao_get_letters_to_be_printed(print_run_deadline=datetime(2020, 12, 1, 17, 30), postage='second', query_limit=4) + ) + assert len(results) == 1 + assert results[0].id == included_letter.id + + def test_dao_get_letters_and_sheets_volume_by_postage(notify_db_session): first_service = create_service(service_name='first service', service_id='3a5cea08-29fd-4bb9-b582-8dedd928b149') second_service = create_service(service_name='second service', service_id='642bf33b-54b5-45f2-8c13-942a46616704') first_template = create_template(service=first_service, template_type='letter', postage='second') second_template = create_template(service=second_service, template_type='letter', postage='second') - create_notification(template=first_template, created_at=datetime(2020, 12, 1, 9, 30), postage='first'), - create_notification(template=first_template, created_at=datetime(2020, 12, 1, 12, 30), postage='europe'), - create_notification(template=first_template, created_at=datetime(2020, 12, 1, 13, 30), postage='rest-of-world'), - create_notification(template=first_template, created_at=datetime(2020, 12, 1, 14, 30), billable_units=3), - create_notification(template=first_template, created_at=datetime(2020, 12, 1, 15, 30)), - create_notification(template=second_template, created_at=datetime(2020, 12, 1, 8, 30), postage='first'), - create_notification(template=second_template, created_at=datetime(2020, 12, 1, 8, 31), postage='first'), - create_notification(template=second_template, created_at=datetime(2020, 12, 1, 8, 32)), - create_notification(template=second_template, created_at=datetime(2020, 12, 1, 8, 33)), + create_notification(template=first_template, created_at=datetime(2020, 12, 1, 9, 30), postage='first') + create_notification(template=first_template, created_at=datetime(2020, 12, 1, 12, 30), postage='europe') + create_notification(template=first_template, created_at=datetime(2020, 12, 1, 13, 30), postage='rest-of-world') + create_notification(template=first_template, created_at=datetime(2020, 12, 1, 14, 30), billable_units=3) + create_notification(template=first_template, created_at=datetime(2020, 12, 1, 14, 30), billable_units=0) + create_notification(template=first_template, created_at=datetime(2020, 12, 1, 15, 30)) + create_notification(template=second_template, created_at=datetime(2020, 12, 1, 8, 30), postage='first') + create_notification(template=second_template, created_at=datetime(2020, 12, 1, 8, 31), postage='first') + create_notification(template=second_template, created_at=datetime(2020, 12, 1, 8, 32)) + create_notification(template=second_template, created_at=datetime(2020, 12, 1, 8, 33)) create_notification(template=second_template, created_at=datetime(2020, 12, 1, 8, 34)) results = dao_get_letters_and_sheets_volume_by_postage(print_run_deadline=datetime(2020, 12, 1, 17, 30)) diff --git a/tests/app/letters/test_letter_utils.py b/tests/app/letters/test_letter_utils.py index f4821a95d..ae9b188fa 100644 --- a/tests/app/letters/test_letter_utils.py +++ b/tests/app/letters/test_letter_utils.py @@ -12,6 +12,7 @@ from app.letters.utils import ( ScanErrorType, find_letter_pdf_in_s3, generate_letter_pdf_filename, + get_billable_units_for_letter_page_count, get_bucket_name_and_prefix_for_notification, get_folder_name, get_letter_pdf_and_metadata, @@ -433,3 +434,9 @@ def test_letter_print_day_returns_today_if_letter_was_printed_today(): @freeze_time('2017-07-07 16:30:00') def test_letter_print_day_returns_formatted_date_if_letter_printed_before_1730_yesterday(created_at, formatted_date): assert letter_print_day(created_at) == formatted_date + + +@pytest.mark.parametrize('number_of_pages, expected_billable_units', [(2, 1), (3, 2), (10, 5)]) +def test_get_billable_units_for_letter_page_count(number_of_pages, expected_billable_units): + result = get_billable_units_for_letter_page_count(number_of_pages) + assert result == expected_billable_units diff --git a/tests/app/v2/notifications/test_post_letter_notifications.py b/tests/app/v2/notifications/test_post_letter_notifications.py index c09b9f834..4687f5ae2 100644 --- a/tests/app/v2/notifications/test_post_letter_notifications.py +++ b/tests/app/v2/notifications/test_post_letter_notifications.py @@ -314,7 +314,6 @@ def test_post_letter_notification_with_test_key_creates_pdf_and_sets_status_to_s fake_create_letter_task = mocker.patch('app.celery.letters_pdf_tasks.get_pdf_for_templated_letter.apply_async') fake_create_dvla_response_task = mocker.patch( 'app.celery.research_mode_tasks.create_fake_letter_response_file.apply_async') - with set_config_values(notify_api, { 'NOTIFY_ENVIRONMENT': env }):