From 5c5eb8a96aff9f9b65f43be8df69de40392df90a Mon Sep 17 00:00:00 2001 From: David McDonald Date: Wed, 19 Feb 2020 12:54:06 +0000 Subject: [PATCH] Remove unneeded check that notification is in created state We instead rely on the fact that only files being passed into this function we already know are in the created state --- app/celery/letters_pdf_tasks.py | 23 +------------ tests/app/celery/test_letters_pdf_tasks.py | 40 ++-------------------- 2 files changed, 4 insertions(+), 59 deletions(-) diff --git a/app/celery/letters_pdf_tasks.py b/app/celery/letters_pdf_tasks.py index a4649ce6c..f76492849 100644 --- a/app/celery/letters_pdf_tasks.py +++ b/app/celery/letters_pdf_tasks.py @@ -25,7 +25,6 @@ from app.dao.notifications_dao import ( update_notification_status_by_id, dao_update_notification, dao_get_notification_by_reference, - dao_get_notifications_by_references, dao_update_notifications_by_reference, dao_get_letters_to_be_printed, ) @@ -180,10 +179,6 @@ def get_key_and_size_of_letters_to_be_sent_to_print(print_run_date): postage=letter.postage ) - current_app.logger.info( - f'Found notification {letter.id} to send to DVLA to print: {letter_file_name}' - ) - letter_head = s3.head_s3_object(current_app.config['LETTERS_PDF_BUCKET_NAME'], letter_file_name) letter_pdfs.append({"Key": letter_file_name, "Size": letter_head['ContentLength']}) @@ -199,7 +194,7 @@ def group_letters(letter_pdfs): running_filesize = 0 list_of_files = [] for letter in letter_pdfs: - if letter['Key'].lower().endswith('.pdf') and letter_in_created_state(letter['Key']): + if letter['Key'].lower().endswith('.pdf'): if ( running_filesize + letter['Size'] > current_app.config['MAX_LETTER_PDF_ZIP_FILESIZE'] or len(list_of_files) >= current_app.config['MAX_LETTER_PDF_COUNT_PER_ZIP'] @@ -215,22 +210,6 @@ def group_letters(letter_pdfs): yield list_of_files -def letter_in_created_state(filename): - # filename looks like '2018-01-13/NOTIFY.ABCDEF1234567890.D.2.C.C.20180113120000.PDF' - subfolder = filename.split('/')[0] - ref = get_reference_from_filename(filename) - notifications = dao_get_notifications_by_references([ref]) - if notifications: - if notifications[0].status == NOTIFICATION_CREATED: - return True - current_app.logger.info('Collating letters for {} but notification with reference {} already in {}'.format( - subfolder, - ref, - notifications[0].status - )) - return False - - @notify_celery.task(bind=True, name='process-virus-scan-passed', max_retries=15, default_retry_delay=300) def process_virus_scan_passed(self, filename): reference = get_reference_from_filename(filename) diff --git a/tests/app/celery/test_letters_pdf_tasks.py b/tests/app/celery/test_letters_pdf_tasks.py index efdab98db..f8c566e37 100644 --- a/tests/app/celery/test_letters_pdf_tasks.py +++ b/tests/app/celery/test_letters_pdf_tasks.py @@ -22,7 +22,6 @@ from app.celery.letters_pdf_tasks import ( collate_letter_pdfs_for_day, get_key_and_size_of_letters_to_be_sent_to_print, group_letters, - letter_in_created_state, process_sanitised_letter, process_virus_scan_passed, process_virus_scan_failed, @@ -41,7 +40,6 @@ from app.models import ( NOTIFICATION_CREATED, NOTIFICATION_DELIVERED, NOTIFICATION_PENDING_VIRUS_CHECK, - NOTIFICATION_SENDING, NOTIFICATION_TECHNICAL_FAILURE, NOTIFICATION_VALIDATION_FAILED, NOTIFICATION_VIRUS_SCAN_FAILED, @@ -423,8 +421,7 @@ def test_collate_letter_pdfs_for_day_when_run_after_midnight(notify_api, sample_ ) -def test_group_letters_splits_on_file_size(notify_api, mocker): - mocker.patch('app.celery.letters_pdf_tasks.letter_in_created_state', return_value=True) +def test_group_letters_splits_on_file_size(notify_api): letters = [ # ends under max but next one is too big {'Key': 'A.pdf', 'Size': 1}, {'Key': 'B.pdf', 'Size': 2}, @@ -450,8 +447,7 @@ def test_group_letters_splits_on_file_size(notify_api, mocker): assert next(x, None) is None -def test_group_letters_splits_on_file_count(notify_api, mocker): - mocker.patch('app.celery.letters_pdf_tasks.letter_in_created_state', return_value=True) +def test_group_letters_splits_on_file_count(notify_api): letters = [ {'Key': 'A.pdf', 'Size': 1}, {'Key': 'B.pdf', 'Size': 2}, @@ -474,8 +470,7 @@ def test_group_letters_splits_on_file_count(notify_api, mocker): assert next(x, None) is None -def test_group_letters_splits_on_file_size_and_file_count(notify_api, mocker): - mocker.patch('app.celery.letters_pdf_tasks.letter_in_created_state', return_value=True) +def test_group_letters_splits_on_file_size_and_file_count(notify_api): letters = [ # ends under max file size but next file is too big {'Key': 'A.pdf', 'Size': 1}, @@ -510,43 +505,14 @@ def test_group_letters_splits_on_file_size_and_file_count(notify_api, mocker): def test_group_letters_ignores_non_pdfs(notify_api, mocker): - mocker.patch('app.celery.letters_pdf_tasks.letter_in_created_state', return_value=True) letters = [{'Key': 'A.zip'}] assert list(group_letters(letters)) == [] -def test_group_letters_ignores_notifications_already_sent(notify_api, mocker): - mock = mocker.patch('app.celery.letters_pdf_tasks.letter_in_created_state', return_value=False) - letters = [{'Key': 'A.pdf'}] - assert list(group_letters(letters)) == [] - mock.assert_called_once_with('A.pdf') - - def test_group_letters_with_no_letters(notify_api, mocker): - mocker.patch('app.celery.letters_pdf_tasks.letter_in_created_state', return_value=True) assert list(group_letters([])) == [] -def test_letter_in_created_state(sample_notification): - sample_notification.reference = 'ABCDEF1234567890' - filename = '2018-01-13/NOTIFY.ABCDEF1234567890.D.2.C.C.20180113120000.PDF' - - assert letter_in_created_state(filename) is True - - -def test_letter_in_created_state_fails_if_notification_not_in_created(sample_notification): - sample_notification.reference = 'ABCDEF1234567890' - sample_notification.status = NOTIFICATION_SENDING - filename = '2018-01-13/NOTIFY.ABCDEF1234567890.D.2.C.C.20180113120000.PDF' - assert letter_in_created_state(filename) is False - - -def test_letter_in_created_state_fails_if_notification_doesnt_exist(sample_notification): - sample_notification.reference = 'QWERTY1234567890' - filename = '2018-01-13/NOTIFY.ABCDEF1234567890.D.2.C.C.20180113120000.PDF' - assert letter_in_created_state(filename) is False - - @freeze_time('2018-01-01 18:00') @mock_s3 @pytest.mark.parametrize('key_type,noti_status,bucket_config_name,destination_folder', [