diff --git a/app/celery/letters_pdf_tasks.py b/app/celery/letters_pdf_tasks.py index c269ec5a0..3248082bb 100644 --- a/app/celery/letters_pdf_tasks.py +++ b/app/celery/letters_pdf_tasks.py @@ -306,7 +306,23 @@ def process_sanitised_letter(self, sanitise_data): billable_units=billable_units, recipient_address=letter_details['address'] ) - move_sanitised_letter_to_test_or_live_pdf_bucket(filename, is_test_key, notification.created_at) + + # The original filename could be wrong because we didn't know the postage. + # Now we know if the letter is international, we can check what the filename should be. + upload_file_name = get_letter_pdf_filename( + reference=notification.reference, + crown=notification.service.crown, + sending_date=notification.created_at, + dont_use_sending_date=True, + postage=notification.postage + ) + + move_sanitised_letter_to_test_or_live_pdf_bucket( + filename, + is_test_key, + notification.created_at, + upload_file_name, + ) # We've moved the sanitised PDF from the sanitise bucket, but still need to delete the original file: original_pdf_object.delete() diff --git a/app/letters/utils.py b/app/letters/utils.py index e2c2cbe2f..a04f9ffa9 100644 --- a/app/letters/utils.py +++ b/app/letters/utils.py @@ -148,10 +148,10 @@ def move_uploaded_pdf_to_letters_bucket(source_filename, upload_filename): ) -def move_sanitised_letter_to_test_or_live_pdf_bucket(filename, is_test_letter, created_at): +def move_sanitised_letter_to_test_or_live_pdf_bucket(filename, is_test_letter, created_at, new_filename): target_bucket_config = 'TEST_LETTERS_BUCKET_NAME' if is_test_letter else 'LETTERS_PDF_BUCKET_NAME' target_bucket_name = current_app.config[target_bucket_config] - target_filename = get_folder_name(created_at, dont_use_sending_date=is_test_letter) + filename + target_filename = get_folder_name(created_at, dont_use_sending_date=is_test_letter) + new_filename _move_s3_object( source_bucket=current_app.config['LETTER_SANITISE_BUCKET_NAME'], diff --git a/tests/app/celery/test_letters_pdf_tasks.py b/tests/app/celery/test_letters_pdf_tasks.py index 02e48980d..d1eadc9e3 100644 --- a/tests/app/celery/test_letters_pdf_tasks.py +++ b/tests/app/celery/test_letters_pdf_tasks.py @@ -592,18 +592,60 @@ def test_sanitise_letter_puts_letter_into_technical_failure_if_max_retries_excee @mock_s3 -@pytest.mark.parametrize('key_type, destination_bucket, expected_status, destination_filename', [ - (KEY_TYPE_NORMAL, 'LETTERS_PDF_BUCKET_NAME', NOTIFICATION_CREATED, '2018-07-01/NOTIFY.foo'), - (KEY_TYPE_TEST, 'TEST_LETTERS_BUCKET_NAME', NOTIFICATION_DELIVERED, 'NOTIFY.foo'), +@pytest.mark.parametrize('key_type, destination_bucket, expected_status, postage, destination_filename', [ + ( + KEY_TYPE_NORMAL, + 'LETTERS_PDF_BUCKET_NAME', + NOTIFICATION_CREATED, + 'first', + '2018-07-01/NOTIFY.FOO.D.1.C.C.20180701120000.PDF' + ), + ( + KEY_TYPE_NORMAL, + 'LETTERS_PDF_BUCKET_NAME', + NOTIFICATION_CREATED, + 'second', + '2018-07-01/NOTIFY.FOO.D.2.C.C.20180701120000.PDF' + ), + ( + KEY_TYPE_NORMAL, + 'LETTERS_PDF_BUCKET_NAME', + NOTIFICATION_CREATED, + 'europe', + '2018-07-01/NOTIFY.FOO.D.E.C.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' + ), + ( + KEY_TYPE_TEST, + 'TEST_LETTERS_BUCKET_NAME', + NOTIFICATION_DELIVERED, + 'second', + 'NOTIFY.FOO.D.2.C.C.20180701120000.PDF', + ), + ( + KEY_TYPE_TEST, + 'TEST_LETTERS_BUCKET_NAME', + NOTIFICATION_DELIVERED, + 'first', + 'NOTIFY.FOO.D.1.C.C.20180701120000.PDF', + ), ]) def test_process_sanitised_letter_with_valid_letter( sample_letter_notification, key_type, destination_bucket, expected_status, + postage, destination_filename, ): - filename = 'NOTIFY.{}'.format(sample_letter_notification.reference) + # 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' scan_bucket_name = current_app.config['LETTERS_SCAN_BUCKET_NAME'] template_preview_bucket_name = current_app.config['LETTER_SANITISE_BUCKET_NAME'] @@ -622,6 +664,7 @@ def test_process_sanitised_letter_with_valid_letter( sample_letter_notification.key_type = key_type sample_letter_notification.billable_units = 1 sample_letter_notification.created_at = datetime(2018, 7, 1, 12) + sample_letter_notification.postage = postage encrypted_data = encryption.encrypt({ 'page_count': 2, diff --git a/tests/app/letters/test_letter_utils.py b/tests/app/letters/test_letter_utils.py index d654f83bd..d9e471dec 100644 --- a/tests/app/letters/test_letter_utils.py +++ b/tests/app/letters/test_letter_utils.py @@ -336,7 +336,8 @@ def test_move_sanitised_letter_to_live_pdf_bucket(notify_api, mocker): move_sanitised_letter_to_test_or_live_pdf_bucket( filename=filename, is_test_letter=False, - created_at=datetime.utcnow() + created_at=datetime.utcnow(), + new_filename=filename ) assert not [x for x in source_bucket.objects.all()] @@ -359,7 +360,8 @@ def test_move_sanitised_letter_to_test_pdf_bucket(notify_api, mocker): move_sanitised_letter_to_test_or_live_pdf_bucket( filename=filename, is_test_letter=True, - created_at=datetime.utcnow() + created_at=datetime.utcnow(), + new_filename=filename ) assert not [x for x in source_bucket.objects.all()]