diff --git a/app/celery/letters_pdf_tasks.py b/app/celery/letters_pdf_tasks.py index a4c7e2628..9307bc8ec 100644 --- a/app/celery/letters_pdf_tasks.py +++ b/app/celery/letters_pdf_tasks.py @@ -62,7 +62,6 @@ def get_pdf_for_templated_letter(self, notification_id): notification = get_notification_by_id(notification_id, _raise=True) letter_filename = generate_letter_pdf_filename( reference=notification.reference, - crown=notification.service.crown, created_at=notification.created_at, ignore_folder=notification.key_type == KEY_TYPE_TEST, postage=notification.postage @@ -373,7 +372,6 @@ def process_sanitised_letter(self, sanitise_data): # Now we know if the letter is international, we can check what the filename should be. upload_file_name = generate_letter_pdf_filename( reference=notification.reference, - crown=notification.service.crown, created_at=notification.created_at, ignore_folder=True, postage=notification.postage diff --git a/app/letters/utils.py b/app/letters/utils.py index f79a589af..4bdb85d2a 100644 --- a/app/letters/utils.py +++ b/app/letters/utils.py @@ -25,7 +25,7 @@ class ScanErrorType(Enum): LETTERS_PDF_FILE_LOCATION_STRUCTURE = \ - '{folder}NOTIFY.{reference}.{duplex}.{letter_class}.{colour}.{crown}.{date}.pdf' + '{folder}NOTIFY.{reference}.{duplex}.{letter_class}.{colour}.{date}.pdf' PRECOMPILED_BUCKET_PREFIX = '{folder}NOTIFY.{reference}' @@ -53,14 +53,13 @@ def find_letter_pdf_in_s3(notification): return item -def generate_letter_pdf_filename(reference, crown, created_at, ignore_folder=False, postage=SECOND_CLASS): +def generate_letter_pdf_filename(reference, created_at, ignore_folder=False, postage=SECOND_CLASS): upload_file_name = LETTERS_PDF_FILE_LOCATION_STRUCTURE.format( folder='' if ignore_folder else get_folder_name(created_at), reference=reference, duplex="D", letter_class=RESOLVE_POSTAGE_FOR_FILE_NAME[postage], colour="C", - crown="C" if crown else "N", date=created_at.strftime('%Y%m%d%H%M%S') ).upper() return upload_file_name @@ -85,7 +84,7 @@ def get_bucket_name_and_prefix_for_notification(notification): def get_reference_from_filename(filename): - # filename looks like '2018-01-13/NOTIFY.ABCDEF1234567890.D.2.C.C.20180113120000.PDF' + # filename looks like '2018-01-13/NOTIFY.ABCDEF1234567890.D.2.C.20180113120000.PDF' filename_parts = filename.split('.') return filename_parts[1] @@ -96,7 +95,6 @@ def upload_letter_pdf(notification, pdf_data, precompiled=False): upload_file_name = generate_letter_pdf_filename( reference=notification.reference, - crown=notification.service.crown, created_at=notification.created_at, ignore_folder=precompiled or notification.key_type == KEY_TYPE_TEST, postage=notification.postage diff --git a/app/service/send_notification.py b/app/service/send_notification.py index af32962bb..f6d7e5e5b 100644 --- a/app/service/send_notification.py +++ b/app/service/send_notification.py @@ -199,7 +199,6 @@ def send_pdf_letter_notification(service_id, post_data): upload_filename = generate_letter_pdf_filename( reference=notification.reference, - crown=notification.service.crown, created_at=notification.created_at, ignore_folder=False, postage=notification.postage diff --git a/tests/app/celery/test_letters_pdf_tasks.py b/tests/app/celery/test_letters_pdf_tasks.py index 540dc67cc..c9bac6535 100644 --- a/tests/app/celery/test_letters_pdf_tasks.py +++ b/tests/app/celery/test_letters_pdf_tasks.py @@ -98,7 +98,6 @@ def test_get_pdf_for_templated_letter_happy_path(mocker, sample_letter_notificat mock_generate_letter_pdf_filename.assert_called_once_with( reference=sample_letter_notification.reference, - crown=True, created_at=sample_letter_notification.created_at, ignore_folder=False, postage='second' @@ -176,9 +175,9 @@ def test_get_key_and_size_of_letters_to_be_sent_to_print(notify_api, mocker, sam pdf_bucket = current_app.config['LETTERS_PDF_BUCKET_NAME'] s3 = boto3.client('s3', region_name='eu-west-1') s3.create_bucket(Bucket=pdf_bucket, CreateBucketConfiguration={'LocationConstraint': 'eu-west-1'}) - s3.put_object(Bucket=pdf_bucket, Key='2020-02-17/NOTIFY.REF0.D.2.C.C.20200217160000.PDF', Body=b'1'), - s3.put_object(Bucket=pdf_bucket, Key='2020-02-17/NOTIFY.REF1.D.2.C.C.20200217150000.PDF', Body=b'22'), - s3.put_object(Bucket=pdf_bucket, Key='2020-02-16/NOTIFY.REF2.D.2.C.C.20200215180000.PDF', Body=b'333'), + s3.put_object(Bucket=pdf_bucket, Key='2020-02-17/NOTIFY.REF0.D.2.C.20200217160000.PDF', Body=b'1'), + s3.put_object(Bucket=pdf_bucket, Key='2020-02-17/NOTIFY.REF1.D.2.C.20200217150000.PDF', Body=b'22'), + s3.put_object(Bucket=pdf_bucket, Key='2020-02-16/NOTIFY.REF2.D.2.C.20200215180000.PDF', Body=b'333'), # second class create_notification( @@ -235,17 +234,17 @@ def test_get_key_and_size_of_letters_to_be_sent_to_print(notify_api, mocker, sam assert results == [ { - 'Key': '2020-02-16/NOTIFY.REF2.D.2.C.C.20200215180000.PDF', + 'Key': '2020-02-16/NOTIFY.REF2.D.2.C.20200215180000.PDF', 'Size': 3, 'ServiceId': str(sample_letter_template.service_id) }, { - 'Key': '2020-02-17/NOTIFY.REF1.D.2.C.C.20200217150000.PDF', + 'Key': '2020-02-17/NOTIFY.REF1.D.2.C.20200217150000.PDF', 'Size': 2, 'ServiceId': str(sample_letter_template.service_id) }, { - 'Key': '2020-02-17/NOTIFY.REF0.D.2.C.C.20200217160000.PDF', + 'Key': '2020-02-17/NOTIFY.REF0.D.2.C.20200217160000.PDF', 'Size': 1, 'ServiceId': str(sample_letter_template.service_id) }, @@ -260,7 +259,7 @@ def test_get_key_and_size_of_letters_to_be_sent_to_print_handles_file_not_found( pdf_bucket = current_app.config['LETTERS_PDF_BUCKET_NAME'] s3 = boto3.client('s3', region_name='eu-west-1') s3.create_bucket(Bucket=pdf_bucket, CreateBucketConfiguration={'LocationConstraint': 'eu-west-1'}) - s3.put_object(Bucket=pdf_bucket, Key='2020-02-17/NOTIFY.REF1.D.2.C.C.20200217150000.PDF', Body=b'12'), + s3.put_object(Bucket=pdf_bucket, Key='2020-02-17/NOTIFY.REF1.D.2.C.20200217150000.PDF', Body=b'12'), # no object for ref1 create_notification( @@ -282,7 +281,7 @@ def test_get_key_and_size_of_letters_to_be_sent_to_print_handles_file_not_found( ) assert results == [{ - 'Key': '2020-02-17/NOTIFY.REF1.D.2.C.C.20200217150000.PDF', + 'Key': '2020-02-17/NOTIFY.REF1.D.2.C.20200217150000.PDF', 'Size': 2, 'ServiceId': str(sample_letter_template.service_id)} ] @@ -362,13 +361,13 @@ def test_collate_letter_pdfs_to_be_sent( ) filenames = [ - '2020-02-17/NOTIFY.FIRST_CLASS.D.1.C.C.20200217140000.PDF', - '2020-02-16/NOTIFY.REF2.D.2.C.C.20200215180000.PDF', - '2020-02-17/NOTIFY.REF1.D.2.C.C.20200217150000.PDF', - '2020-02-17/NOTIFY.REF0.D.2.C.C.20200217160000.PDF', - '2020-02-15/NOTIFY.INTERNATIONAL.D.E.C.C.20200214180000.PDF', - '2020-02-14/NOTIFY.INTERNATIONAL.D.N.C.C.20200213180000.PDF', - '2020-02-17/NOTIFY.ANOTHER_SERVICE.D.2.C.C.20200217160000.PDF' + '2020-02-17/NOTIFY.FIRST_CLASS.D.1.C.20200217140000.PDF', + '2020-02-16/NOTIFY.REF2.D.2.C.20200215180000.PDF', + '2020-02-17/NOTIFY.REF1.D.2.C.20200217150000.PDF', + '2020-02-17/NOTIFY.REF0.D.2.C.20200217160000.PDF', + '2020-02-15/NOTIFY.INTERNATIONAL.D.E.C.20200214180000.PDF', + '2020-02-14/NOTIFY.INTERNATIONAL.D.N.C.20200213180000.PDF', + '2020-02-17/NOTIFY.ANOTHER_SERVICE.D.2.C.20200217160000.PDF' ] for filename in filenames: @@ -392,9 +391,9 @@ def test_collate_letter_pdfs_to_be_sent( name='zip-and-send-letter-pdfs', kwargs={ 'filenames_to_zip': [ - '2020-02-17/NOTIFY.FIRST_CLASS.D.1.C.C.20200217140000.PDF' + '2020-02-17/NOTIFY.FIRST_CLASS.D.1.C.20200217140000.PDF' ], - 'upload_filename': f'NOTIFY.2020-02-17.1.001.kHh01fdUxT9iEIYUt5Wx.{letter_template_1.service_id}.ZIP' + 'upload_filename': f'NOTIFY.2020-02-17.1.001.sO6RKzPyNrkxrR8OLonl.{letter_template_1.service_id}.ZIP' }, queue='process-ftp-tasks', compression='zlib' @@ -402,8 +401,8 @@ def test_collate_letter_pdfs_to_be_sent( assert mock_celery.call_args_list[1] == call( name='zip-and-send-letter-pdfs', kwargs={ - 'filenames_to_zip': ['2020-02-17/NOTIFY.ANOTHER_SERVICE.D.2.C.C.20200217160000.PDF'], - 'upload_filename': f'NOTIFY.2020-02-17.2.001.MezXnKP3IvNZEoMsSlVo.{service_2.id}.ZIP' + 'filenames_to_zip': ['2020-02-17/NOTIFY.ANOTHER_SERVICE.D.2.C.20200217160000.PDF'], + 'upload_filename': f'NOTIFY.2020-02-17.2.001.bGS-FKKV0QHcOUZgacEu.{service_2.id}.ZIP' }, queue='process-ftp-tasks', compression='zlib' @@ -412,10 +411,10 @@ def test_collate_letter_pdfs_to_be_sent( name='zip-and-send-letter-pdfs', kwargs={ 'filenames_to_zip': [ - '2020-02-16/NOTIFY.REF2.D.2.C.C.20200215180000.PDF', - '2020-02-17/NOTIFY.REF1.D.2.C.C.20200217150000.PDF' + '2020-02-16/NOTIFY.REF2.D.2.C.20200215180000.PDF', + '2020-02-17/NOTIFY.REF1.D.2.C.20200217150000.PDF' ], - 'upload_filename': f'NOTIFY.2020-02-17.2.002.k3x_WqC5KhB6e2DWv9Ma.{letter_template_1.service_id}.ZIP' + 'upload_filename': f'NOTIFY.2020-02-17.2.002.AmmswUYqPToXwlSZiFyK.{letter_template_1.service_id}.ZIP' }, queue='process-ftp-tasks', compression='zlib' @@ -424,9 +423,9 @@ def test_collate_letter_pdfs_to_be_sent( name='zip-and-send-letter-pdfs', kwargs={ 'filenames_to_zip': [ - '2020-02-17/NOTIFY.REF0.D.2.C.C.20200217160000.PDF' + '2020-02-17/NOTIFY.REF0.D.2.C.20200217160000.PDF' ], - 'upload_filename': f'NOTIFY.2020-02-17.2.003.J85cUw-FWlKuAIOcwdLS.{letter_template_1.service_id}.ZIP' + 'upload_filename': f'NOTIFY.2020-02-17.2.003.36PwhyI9lFKjzbPiWxwv.{letter_template_1.service_id}.ZIP' }, queue='process-ftp-tasks', compression='zlib' @@ -435,9 +434,9 @@ def test_collate_letter_pdfs_to_be_sent( name='zip-and-send-letter-pdfs', kwargs={ 'filenames_to_zip': [ - '2020-02-15/NOTIFY.INTERNATIONAL.D.E.C.C.20200214180000.PDF' + '2020-02-15/NOTIFY.INTERNATIONAL.D.E.C.20200214180000.PDF' ], - 'upload_filename': f'NOTIFY.2020-02-17.E.001.4YajCZzgzIl7zf8bjWK2.{letter_template_1.service_id}.ZIP' + 'upload_filename': f'NOTIFY.2020-02-17.E.001.lDBwqhnG__URJeGz3tH1.{letter_template_1.service_id}.ZIP' }, queue='process-ftp-tasks', compression='zlib' @@ -446,9 +445,9 @@ def test_collate_letter_pdfs_to_be_sent( name='zip-and-send-letter-pdfs', kwargs={ 'filenames_to_zip': [ - '2020-02-14/NOTIFY.INTERNATIONAL.D.N.C.C.20200213180000.PDF', + '2020-02-14/NOTIFY.INTERNATIONAL.D.N.C.20200213180000.PDF', ], - 'upload_filename': f'NOTIFY.2020-02-17.N.001.eSvP8Ph6EBKhh3k7BSA2.{letter_template_1.service_id}.ZIP' + 'upload_filename': f'NOTIFY.2020-02-17.N.001.ZE7k_jm7Bg5sYwLswkr4.{letter_template_1.service_id}.ZIP' }, queue='process-ftp-tasks', compression='zlib' @@ -727,42 +726,42 @@ def test_sanitise_letter_puts_letter_into_technical_failure_if_max_retries_excee 'LETTERS_PDF_BUCKET_NAME', NOTIFICATION_CREATED, 'first', - '2018-07-01/NOTIFY.FOO.D.1.C.C.20180701120000.PDF' + '2018-07-01/NOTIFY.FOO.D.1.C.20180701120000.PDF' ), ( KEY_TYPE_NORMAL, 'LETTERS_PDF_BUCKET_NAME', NOTIFICATION_CREATED, 'second', - '2018-07-01/NOTIFY.FOO.D.2.C.C.20180701120000.PDF' + '2018-07-01/NOTIFY.FOO.D.2.C.20180701120000.PDF' ), ( KEY_TYPE_NORMAL, 'LETTERS_PDF_BUCKET_NAME', NOTIFICATION_CREATED, 'europe', - '2018-07-01/NOTIFY.FOO.D.E.C.C.20180701120000.PDF' + '2018-07-01/NOTIFY.FOO.D.E.C.20180701120000.PDF' ), ( KEY_TYPE_NORMAL, 'LETTERS_PDF_BUCKET_NAME', NOTIFICATION_CREATED, 'rest-of-world', - '2018-07-01/NOTIFY.FOO.D.N.C.C.20180701120000.PDF' + '2018-07-01/NOTIFY.FOO.D.N.C.20180701120000.PDF' ), ( KEY_TYPE_TEST, 'TEST_LETTERS_BUCKET_NAME', NOTIFICATION_DELIVERED, 'second', - 'NOTIFY.FOO.D.2.C.C.20180701120000.PDF', + 'NOTIFY.FOO.D.2.C.20180701120000.PDF', ), ( KEY_TYPE_TEST, 'TEST_LETTERS_BUCKET_NAME', NOTIFICATION_DELIVERED, 'first', - 'NOTIFY.FOO.D.1.C.C.20180701120000.PDF', + 'NOTIFY.FOO.D.1.C.20180701120000.PDF', ), ]) def test_process_sanitised_letter_with_valid_letter( @@ -774,7 +773,7 @@ def test_process_sanitised_letter_with_valid_letter( destination_filename, ): # We save the letter as if it's 2nd class initially, and the task changes the filename to have the correct postage - filename = 'NOTIFY.FOO.D.2.C.C.20180701120000.PDF' + filename = 'NOTIFY.FOO.D.2.C.20180701120000.PDF' scan_bucket_name = current_app.config['LETTERS_SCAN_BUCKET_NAME'] template_preview_bucket_name = current_app.config['LETTER_SANITISE_BUCKET_NAME'] diff --git a/tests/app/dao/notification_dao/test_notification_dao_delete_notifications.py b/tests/app/dao/notification_dao/test_notification_dao_delete_notifications.py index 88c4f8717..808b15386 100644 --- a/tests/app/dao/notification_dao/test_notification_dao_delete_notifications.py +++ b/tests/app/dao/notification_dao/test_notification_dao_delete_notifications.py @@ -144,7 +144,7 @@ def test_delete_notifications_deletes_letters_from_s3(sample_letter_template, mo eight_days_ago = datetime.utcnow() - timedelta(days=8) create_notification(template=sample_letter_template, status='delivered', reference='LETTER_REF', created_at=eight_days_ago, sent_at=eight_days_ago) - filename = "{}/NOTIFY.LETTER_REF.D.2.C.C.{}.PDF".format( + filename = "{}/NOTIFY.LETTER_REF.D.2.C.{}.PDF".format( str(eight_days_ago.date()), eight_days_ago.strftime('%Y%m%d%H%M%S') ) @@ -268,7 +268,7 @@ def test_delete_notifications_deletes_letters_sent_and_in_final_state_from_table assert Notification.query.count() == 1 assert NotificationHistory.query.count() == 0 - filename = "{}/NOTIFY.LETTER_REF.D.2.C.C.{}.PDF".format( + filename = "{}/NOTIFY.LETTER_REF.D.2.C.{}.PDF".format( str(eight_days_ago.date()), eight_days_ago.strftime('%Y%m%d%H%M%S') ) diff --git a/tests/app/letters/test_letter_utils.py b/tests/app/letters/test_letter_utils.py index fb17feb84..f4821a95d 100644 --- a/tests/app/letters/test_letter_utils.py +++ b/tests/app/letters/test_letter_utils.py @@ -171,18 +171,6 @@ def test_get_bucket_name_and_prefix_for_notification_invalid_notification(): get_bucket_name_and_prefix_for_notification(None) -@pytest.mark.parametrize('crown_flag,expected_crown_text', [ - (True, 'C'), - (False, 'N'), -]) -def test_generate_letter_pdf_filename_returns_correct_filename( - notify_api, mocker, crown_flag, expected_crown_text): - created_at = datetime(2017, 12, 4, 17, 29) - filename = generate_letter_pdf_filename(reference='foo', crown=crown_flag, created_at=created_at) - - assert filename == '2017-12-04/NOTIFY.FOO.D.2.C.{}.20171204172900.PDF'.format(expected_crown_text) - - @pytest.mark.parametrize('postage,expected_postage', [ ('second', 2), ('first', 1), @@ -190,9 +178,9 @@ def test_generate_letter_pdf_filename_returns_correct_filename( def test_generate_letter_pdf_filename_returns_correct_postage_for_filename( notify_api, postage, expected_postage): created_at = datetime(2017, 12, 4, 17, 29) - filename = generate_letter_pdf_filename(reference='foo', crown=True, created_at=created_at, postage=postage) + filename = generate_letter_pdf_filename(reference='foo', created_at=created_at, postage=postage) - assert filename == '2017-12-04/NOTIFY.FOO.D.{}.C.C.20171204172900.PDF'.format(expected_postage) + assert filename == '2017-12-04/NOTIFY.FOO.D.{}.C.20171204172900.PDF'.format(expected_postage) def test_generate_letter_pdf_filename_returns_correct_filename_for_test_letters( @@ -200,25 +188,24 @@ def test_generate_letter_pdf_filename_returns_correct_filename_for_test_letters( created_at = datetime(2017, 12, 4, 17, 29) filename = generate_letter_pdf_filename( reference='foo', - crown='C', created_at=created_at, ignore_folder=True ) - assert filename == 'NOTIFY.FOO.D.2.C.C.20171204172900.PDF' + assert filename == 'NOTIFY.FOO.D.2.C.20171204172900.PDF' def test_generate_letter_pdf_filename_returns_tomorrows_filename(notify_api, mocker): created_at = datetime(2017, 12, 4, 17, 31) - filename = generate_letter_pdf_filename(reference='foo', crown=True, created_at=created_at) + filename = generate_letter_pdf_filename(reference='foo', created_at=created_at) - assert filename == '2017-12-05/NOTIFY.FOO.D.2.C.C.20171204173100.PDF' + assert filename == '2017-12-05/NOTIFY.FOO.D.2.C.20171204173100.PDF' @mock_s3 @pytest.mark.parametrize('bucket_config_name,filename_format', [ - ('TEST_LETTERS_BUCKET_NAME', 'NOTIFY.FOO.D.2.C.C.%Y%m%d%H%M%S.PDF'), - ('LETTERS_PDF_BUCKET_NAME', '%Y-%m-%d/NOTIFY.FOO.D.2.C.C.%Y%m%d%H%M%S.PDF') + ('TEST_LETTERS_BUCKET_NAME', 'NOTIFY.FOO.D.2.C.%Y%m%d%H%M%S.PDF'), + ('LETTERS_PDF_BUCKET_NAME', '%Y-%m-%d/NOTIFY.FOO.D.2.C.%Y%m%d%H%M%S.PDF') ]) @freeze_time(FROZEN_DATE_TIME) def test_get_letter_pdf_gets_pdf_from_correct_bucket( @@ -259,7 +246,6 @@ def test_upload_letter_pdf_to_correct_bucket( filename = generate_letter_pdf_filename( reference=sample_letter_notification.reference, - crown=sample_letter_notification.service.crown, created_at=sample_letter_notification.created_at, ignore_folder=is_precompiled_letter ) @@ -286,7 +272,6 @@ def test_upload_letter_pdf_uses_postage_from_notification( filename = generate_letter_pdf_filename( reference=letter_notification.reference, - crown=letter_notification.service.crown, created_at=letter_notification.created_at, ignore_folder=False, postage=letter_notification.postage diff --git a/tests/app/service/test_send_pdf_letter_notification.py b/tests/app/service/test_send_pdf_letter_notification.py index c374691bb..850db1f96 100644 --- a/tests/app/service/test_send_pdf_letter_notification.py +++ b/tests/app/service/test_send_pdf_letter_notification.py @@ -118,5 +118,5 @@ def test_send_pdf_letter_notification_creates_notification_and_moves_letter( s3_mock.assert_called_once_with( 'service-{}/{}.pdf'.format(sample_service_full_permissions.id, file_id), - '2019-08-02/NOTIFY.{}.D.2.C.C.20190802110000.PDF'.format(notification.reference) + '2019-08-02/NOTIFY.{}.D.2.C.20190802110000.PDF'.format(notification.reference) )