diff --git a/app/celery/letters_pdf_tasks.py b/app/celery/letters_pdf_tasks.py index 913cce6a6..0f26c1fdc 100644 --- a/app/celery/letters_pdf_tasks.py +++ b/app/celery/letters_pdf_tasks.py @@ -4,7 +4,6 @@ from hashlib import sha512 from botocore.exceptions import ClientError as BotoClientError from flask import current_app -from notifications_utils import LETTER_MAX_PAGE_COUNT from notifications_utils.letter_timings import LETTER_PROCESSING_DEADLINE from notifications_utils.postal_address import PostalAddress from notifications_utils.timezones import convert_utc_to_bst @@ -102,27 +101,30 @@ def get_pdf_for_templated_letter(self, notification_id): @notify_celery.task(bind=True, name="update-billable-units-for-letter", max_retries=15, default_retry_delay=300) -def update_billable_units_or_validation_failed_for_templated_letter(self, notification_id, page_count, - create_fake_letter_response_file=None): +def update_billable_units_for_letter(self, notification_id, page_count): notification = get_notification_by_id(notification_id, _raise=True) - if page_count > LETTER_MAX_PAGE_COUNT: - notification.status = NOTIFICATION_VALIDATION_FAILED + + billable_units = get_billable_units_for_letter_page_count(page_count) + + if notification.key_type != KEY_TYPE_TEST: + notification.billable_units = billable_units dao_update_notification(notification) - current_app.logger.info(f"Letter notification id: {notification_id} reference {notification.reference}: " - f"validation failed: letter is too long {page_count}") - else: - if notification.key_type != KEY_TYPE_TEST: - notification.billable_units = get_billable_units_for_letter_page_count(page_count) - dao_update_notification(notification) - current_app.logger.info(f"Letter notification id: {notification_id} reference {notification.reference}: " - f"billable units set to {notification.billable_units}") - if notification.key_type == KEY_TYPE_TEST \ - and current_app.config['NOTIFY_ENVIRONMENT'] in ['preview', 'development']: - # For test letters send a fake letter response - create_fake_letter_response_file.apply_async( - (notification.reference,), - queue=QueueNames.RESEARCH_MODE - ) + + current_app.logger.info( + f"Letter notification id: {notification_id} reference {notification.reference}: " + f"billable units set to {billable_units}" + ) + + +@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"Letter notification id: {notification_id} reference {notification.reference}: " + f"validation failed: letter is too long {page_count}") @notify_celery.task(name='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 5aab5067b..f2c82d2a2 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -19,6 +19,7 @@ from app.celery.letters_pdf_tasks import ( get_pdf_for_templated_letter, sanitise_letter, ) +from app.celery.research_mode_tasks import create_fake_letter_response_file from app.celery.tasks import save_api_email, save_api_sms from app.clients.document_download import DocumentDownloadError from app.config import QueueNames, TaskNames @@ -388,6 +389,12 @@ def process_letter_notification( queue=queue ) + if test_key and current_app.config['NOTIFY_ENVIRONMENT'] in ['preview', 'development']: + create_fake_letter_response_file.apply_async( + (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 db33e398d..1de2d8379 100644 --- a/tests/app/celery/test_letters_pdf_tasks.py +++ b/tests/app/celery/test_letters_pdf_tasks.py @@ -26,7 +26,8 @@ from app.celery.letters_pdf_tasks import ( resanitise_pdf, sanitise_letter, send_letters_volume_email_to_dvla, - update_billable_units_or_validation_failed_for_templated_letter, + 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,32 +146,31 @@ 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_or_validation_failed_for_templated_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_or_validation_failed_for_templated_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_or_validation_failed_for_templated_letter_doesnt_update_if_sent_with_test_key( +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') - update_billable_units_or_validation_failed_for_templated_letter(sample_letter_notification.id, 2) + update_billable_units_for_letter(sample_letter_notification.id, 2) notification = Notification.query.filter(Notification.reference == sample_letter_notification.reference).one() assert notification.billable_units == 0 @@ -178,14 +178,14 @@ def test_update_billable_units_or_validation_failed_for_templated_letter_doesnt_ @pytest.mark.parametrize('key_type', ['test', 'normal', 'team']) -def test_update_billable_units_or_validation_failed_for_templated_letter_with_too_many_pages( +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_billable_units_or_validation_failed_for_templated_letter(sample_letter_notification.id, 11) + 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 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 1528391f5..4687f5ae2 100644 --- a/tests/app/v2/notifications/test_post_letter_notifications.py +++ b/tests/app/v2/notifications/test_post_letter_notifications.py @@ -296,7 +296,7 @@ def test_post_letter_notification_with_test_key_creates_pdf_and_sets_status_to_d 'development', 'preview', ]) -def test_post_letter_notification_with_test_key_creates_pdf( +def test_post_letter_notification_with_test_key_creates_pdf_and_sets_status_to_sending_and_sends_fake_response_file( notify_api, client, sample_letter_template, mocker, env): data = { @@ -312,7 +312,8 @@ def test_post_letter_notification_with_test_key_creates_pdf( } 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 }): @@ -321,6 +322,7 @@ def test_post_letter_notification_with_test_key_creates_pdf( notification = Notification.query.one() fake_create_letter_task.assert_called_once_with([str(notification.id)], queue='research-mode-tasks') + assert fake_create_dvla_response_task.called assert notification.status == NOTIFICATION_SENDING