diff --git a/app/celery/letters_pdf_tasks.py b/app/celery/letters_pdf_tasks.py index 42ced40d1..cd555f0e6 100644 --- a/app/celery/letters_pdf_tasks.py +++ b/app/celery/letters_pdf_tasks.py @@ -27,9 +27,10 @@ from app.errors import VirusScanError from app.exceptions import NotificationTechnicalFailureException from app.letters.utils import ( ScanErrorType, + find_letter_pdf_filename, + generate_letter_pdf_filename, get_billable_units_for_letter_page_count, get_file_names_from_error_bucket, - get_letter_pdf_filename, get_reference_from_filename, move_error_pdf_to_scan_bucket, move_failed_pdf, @@ -58,7 +59,7 @@ from app.models import ( def get_pdf_for_templated_letter(self, notification_id): try: notification = get_notification_by_id(notification_id, _raise=True) - letter_filename = get_letter_pdf_filename( + letter_filename = generate_letter_pdf_filename( reference=notification.reference, crown=notification.service.crown, created_at=notification.created_at, @@ -235,12 +236,7 @@ def get_key_and_size_of_letters_to_be_sent_to_print(print_run_deadline, postage) letters_awaiting_sending = dao_get_letters_to_be_printed(print_run_deadline, postage) for letter in letters_awaiting_sending: try: - letter_file_name = get_letter_pdf_filename( - reference=letter.reference, - crown=letter.crown, - created_at=letter.created_at, - postage=postage - ) + letter_file_name = find_letter_pdf_filename(letter) letter_head = s3.head_s3_object(current_app.config['LETTERS_PDF_BUCKET_NAME'], letter_file_name) yield { "Key": letter_file_name, @@ -375,7 +371,7 @@ def process_sanitised_letter(self, sanitise_data): # 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( + upload_file_name = generate_letter_pdf_filename( reference=notification.reference, crown=notification.service.crown, created_at=notification.created_at, diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index e873f5ad4..2a18a0b37 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -27,7 +27,7 @@ from app.clients.sms.firetext import ( get_message_status_and_reason_from_firetext_code, ) from app.dao.dao_utils import transactional -from app.letters.utils import get_letter_pdf_filename +from app.letters.utils import find_letter_pdf_filename from app.models import ( EMAIL_TYPE, KEY_TYPE_NORMAL, @@ -453,11 +453,7 @@ def _delete_letters_from_s3( Notification.status.in_(NOTIFICATION_STATUS_TYPES_COMPLETED) ).limit(query_limit).all() for letter in letters_to_delete_from_s3: - prefix = get_letter_pdf_filename(reference=letter.reference, - crown=letter.service.crown, - created_at=letter.created_at, - ignore_folder=letter.key_type == KEY_TYPE_TEST, - postage=letter.postage) + prefix = find_letter_pdf_filename(letter) s3_objects = get_s3_bucket_objects(bucket_name=bucket_name, subfolder=prefix) for s3_object in s3_objects: try: @@ -758,13 +754,7 @@ def dao_get_letters_to_be_printed(print_run_deadline, postage, query_limit=10000 https://docs.sqlalchemy.org/en/13/orm/query.html?highlight=yield_per#sqlalchemy.orm.query.Query.yield_per https://www.mail-archive.com/sqlalchemy@googlegroups.com/msg12443.html """ - notifications = db.session.query( - Notification.id, - Notification.created_at, - Notification.reference, - Notification.service_id, - Service.crown, - ).join( + notifications = Notification.query.join( Notification.service ).filter( Notification.created_at < convert_bst_to_utc(print_run_deadline), diff --git a/app/letters/utils.py b/app/letters/utils.py index 5487c8252..78503d4ba 100644 --- a/app/letters/utils.py +++ b/app/letters/utils.py @@ -37,7 +37,27 @@ def get_folder_name(created_at): return '{}/'.format(print_datetime.date()) -def get_letter_pdf_filename(reference, crown, created_at, ignore_folder=False, postage=SECOND_CLASS): +def find_letter_pdf_filename(notification): + """ + Retrieve the filename of a letter from s3 by searching for it based on a prefix. + + Use this when retrieving existing pdfs, so that we can be more resilient if the naming convention changes. + """ + bucket_name, prefix = get_bucket_name_and_prefix_for_notification(notification) + + s3 = boto3.resource('s3') + bucket = s3.Bucket(bucket_name) + item = next(x for x in bucket.objects.filter(Prefix=prefix)) + return item.key + + +def generate_letter_pdf_filename(reference, crown, created_at, ignore_folder=False, postage=SECOND_CLASS): + """ + Generate a filename for putting a letter into s3 or sending to dvla. + + We should only use this function when uploading data. If you need to get a letter or its metadata from s3 + then use `find_letter_pdf_filename` instead. + """ upload_file_name = LETTERS_PDF_FILE_LOCATION_STRUCTURE.format( folder='' if ignore_folder else get_folder_name(created_at), reference=reference, @@ -78,7 +98,7 @@ def upload_letter_pdf(notification, pdf_data, precompiled=False): current_app.logger.info("PDF Letter {} reference {} created at {}, {} bytes".format( notification.id, notification.reference, notification.created_at, len(pdf_data))) - upload_file_name = get_letter_pdf_filename( + upload_file_name = generate_letter_pdf_filename( reference=notification.reference, crown=notification.service.crown, created_at=notification.created_at, diff --git a/app/service/send_notification.py b/app/service/send_notification.py index 0d1a3b65d..af32962bb 100644 --- a/app/service/send_notification.py +++ b/app/service/send_notification.py @@ -17,8 +17,8 @@ from app.dao.templates_dao import ( ) from app.dao.users_dao import get_user_by_id from app.letters.utils import ( + generate_letter_pdf_filename, get_billable_units_for_letter_page_count, - get_letter_pdf_filename, get_page_count, move_uploaded_pdf_to_letters_bucket, ) @@ -197,7 +197,7 @@ def send_pdf_letter_notification(service_id, post_data): postage=post_data['postage'] or template.postage, ) - upload_filename = get_letter_pdf_filename( + upload_filename = generate_letter_pdf_filename( reference=notification.reference, crown=notification.service.crown, created_at=notification.created_at, diff --git a/tests/app/celery/test_letters_pdf_tasks.py b/tests/app/celery/test_letters_pdf_tasks.py index 6202e3d99..540dc67cc 100644 --- a/tests/app/celery/test_letters_pdf_tasks.py +++ b/tests/app/celery/test_letters_pdf_tasks.py @@ -68,8 +68,8 @@ def test_get_pdf_for_templated_letter_happy_path(mocker, sample_letter_notificat letter_branding = create_letter_branding(name=branding_name, filename=logo_filename) sample_letter_notification.service.letter_branding = letter_branding mock_celery = mocker.patch('app.celery.letters_pdf_tasks.notify_celery.send_task') - mock_get_letter_pdf_filename = mocker.patch( - 'app.celery.letters_pdf_tasks.get_letter_pdf_filename', + mock_generate_letter_pdf_filename = mocker.patch( + 'app.celery.letters_pdf_tasks.generate_letter_pdf_filename', return_value='LETTER.PDF' ) get_pdf_for_templated_letter(sample_letter_notification.id) @@ -96,7 +96,7 @@ def test_get_pdf_for_templated_letter_happy_path(mocker, sample_letter_notificat queue=QueueNames.SANITISE_LETTERS ) - mock_get_letter_pdf_filename.assert_called_once_with( + mock_generate_letter_pdf_filename.assert_called_once_with( reference=sample_letter_notification.reference, crown=True, created_at=sample_letter_notification.created_at, @@ -112,7 +112,7 @@ def test_get_pdf_for_templated_letter_non_existent_notification(notify_db_sessio def test_get_pdf_for_templated_letter_retries_upon_error(mocker, sample_letter_notification): mock_celery = mocker.patch('app.celery.letters_pdf_tasks.notify_celery.send_task', side_effect=Exception()) - mocker.patch('app.celery.letters_pdf_tasks.get_letter_pdf_filename', return_value='LETTER.PDF') + mocker.patch('app.celery.letters_pdf_tasks.generate_letter_pdf_filename', return_value='LETTER.PDF') mock_retry = mocker.patch('app.celery.letters_pdf_tasks.get_pdf_for_templated_letter.retry') mock_logger = mocker.patch('app.celery.tasks.current_app.logger.exception') @@ -127,7 +127,7 @@ def test_get_pdf_for_templated_letter_retries_upon_error(mocker, sample_letter_n def test_get_pdf_for_templated_letter_sets_technical_failure_max_retries(mocker, sample_letter_notification): mock_celery = mocker.patch('app.celery.letters_pdf_tasks.notify_celery.send_task', side_effect=Exception()) - mocker.patch('app.celery.letters_pdf_tasks.get_letter_pdf_filename', return_value='LETTER.PDF') + mocker.patch('app.celery.letters_pdf_tasks.generate_letter_pdf_filename', return_value='LETTER.PDF') mock_retry = mocker.patch( 'app.celery.letters_pdf_tasks.get_pdf_for_templated_letter.retry', side_effect=MaxRetriesExceededError) mock_update_noti = mocker.patch('app.celery.letters_pdf_tasks.update_notification_status_by_id') @@ -170,8 +170,16 @@ def test_update_billable_units_for_letter_doesnt_update_if_sent_with_test_key(mo mock_logger.assert_not_called() +@mock_s3 @freeze_time('2020-02-17 18:00:00') def test_get_key_and_size_of_letters_to_be_sent_to_print(notify_api, mocker, sample_letter_template): + 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'), + # second class create_notification( template=sample_letter_template, @@ -218,51 +226,43 @@ def test_get_key_and_size_of_letters_to_be_sent_to_print(notify_api, mocker, sam key_type=KEY_TYPE_TEST ) - mock_s3 = mocker.patch('app.celery.tasks.s3.head_s3_object', side_effect=[ - {'ContentLength': 2}, - {'ContentLength': 1}, - {'ContentLength': 3}, - ]) - results = list( get_key_and_size_of_letters_to_be_sent_to_print(datetime.now() - timedelta(minutes=30), postage='second') ) - assert mock_s3.call_count == 3 - mock_s3.assert_has_calls( - [ - call(current_app.config['LETTERS_PDF_BUCKET_NAME'], '2020-02-16/NOTIFY.REF2.D.2.C.C.20200215180000.PDF'), - call(current_app.config['LETTERS_PDF_BUCKET_NAME'], '2020-02-17/NOTIFY.REF1.D.2.C.C.20200217150000.PDF'), - call(current_app.config['LETTERS_PDF_BUCKET_NAME'], '2020-02-17/NOTIFY.REF0.D.2.C.C.20200217160000.PDF'), - ] - ) - assert len(results) == 3 assert results == [ { 'Key': '2020-02-16/NOTIFY.REF2.D.2.C.C.20200215180000.PDF', - 'Size': 2, + 'Size': 3, 'ServiceId': str(sample_letter_template.service_id) }, { 'Key': '2020-02-17/NOTIFY.REF1.D.2.C.C.20200217150000.PDF', - 'Size': 1, + 'Size': 2, 'ServiceId': str(sample_letter_template.service_id) }, { 'Key': '2020-02-17/NOTIFY.REF0.D.2.C.C.20200217160000.PDF', - 'Size': 3, + 'Size': 1, 'ServiceId': str(sample_letter_template.service_id) }, ] +@mock_s3 @freeze_time('2020-02-17 18:00:00') -def test_get_key_and_size_of_letters_to_be_sent_to_print_catches_exception( +def test_get_key_and_size_of_letters_to_be_sent_to_print_handles_file_not_found( notify_api, mocker, sample_letter_template ): + 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'), + # no object for ref1 + create_notification( template=sample_letter_template, status='created', @@ -276,30 +276,11 @@ def test_get_key_and_size_of_letters_to_be_sent_to_print_catches_exception( reference='ref1', created_at=(datetime.now() - timedelta(hours=3)) ) - error_response = { - 'Error': { - 'Code': 'FileNotFound', - 'Message': 'some error message from amazon', - 'Type': 'Sender' - } - } - mock_head_s3_object = mocker.patch('app.celery.tasks.s3.head_s3_object', side_effect=[ - {'ContentLength': 2}, - ClientError(error_response, "File not found") - ]) results = list( get_key_and_size_of_letters_to_be_sent_to_print(datetime.now() - timedelta(minutes=30), postage='second') ) - assert mock_head_s3_object.call_count == 2 - mock_head_s3_object.assert_has_calls( - [ - call(current_app.config['LETTERS_PDF_BUCKET_NAME'], '2020-02-17/NOTIFY.REF1.D.2.C.C.20200217150000.PDF'), - call(current_app.config['LETTERS_PDF_BUCKET_NAME'], '2020-02-17/NOTIFY.REF0.D.2.C.C.20200217160000.PDF'), - ] - ) - assert results == [{ 'Key': '2020-02-17/NOTIFY.REF1.D.2.C.C.20200217150000.PDF', 'Size': 2, @@ -307,6 +288,7 @@ def test_get_key_and_size_of_letters_to_be_sent_to_print_catches_exception( ] +@mock_s3 @pytest.mark.parametrize('time_to_run_task', [ "2020-02-17 18:00:00", # after 5:30pm "2020-02-18 02:00:00", # the next day after midnight, before 5:30pm we expect the same results @@ -372,15 +354,25 @@ def test_collate_letter_pdfs_to_be_sent( created_at=(datetime.now() - timedelta(hours=2)) ) - mocker.patch('app.celery.tasks.s3.head_s3_object', side_effect=[ - {'ContentLength': 1}, - {'ContentLength': 1}, - {'ContentLength': 2}, - {'ContentLength': 1}, - {'ContentLength': 3}, - {'ContentLength': 1}, - {'ContentLength': 1}, - ]) + bucket_name = current_app.config['LETTERS_PDF_BUCKET_NAME'] + s3 = boto3.client('s3', region_name='eu-west-1') + s3.create_bucket( + Bucket=bucket_name, + CreateBucketConfiguration={'LocationConstraint': 'eu-west-1'} + ) + + 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' + ] + + for filename in filenames: + s3.put_object(Bucket=bucket_name, Key=filename, Body=b'f') mock_celery = mocker.patch('app.celery.letters_pdf_tasks.notify_celery.send_task') mock_send_email_to_dvla = mocker.patch( 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 490d01e64..be636cda1 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 @@ -1,8 +1,10 @@ from datetime import date, datetime, timedelta +import boto3 import pytest from flask import current_app from freezegun import freeze_time +from moto import mock_s3 from app.dao.notifications_dao import ( delete_notifications_older_than_retention_by_type, @@ -70,6 +72,7 @@ def test_should_delete_notifications_by_type_after_seven_days( expected_email_count, expected_letter_count ): + mocker.patch("app.dao.notifications_dao.find_letter_pdf_filename") mocker.patch("app.dao.notifications_dao.get_s3_bucket_objects") email_template, letter_template, sms_template = _create_templates(sample_service) # create one notification a day between 1st and 10th from 11:00 to 19:00 of each type @@ -118,6 +121,7 @@ def test_should_not_delete_notification_history(sample_service, mocker): @pytest.mark.parametrize('notification_type', ['sms', 'email', 'letter']) def test_delete_notifications_for_days_of_retention(sample_service, notification_type, mocker): + mocker.patch('app.dao.notifications_dao.find_letter_pdf_filename') mock_get_s3 = mocker.patch("app.dao.notifications_dao.get_s3_bucket_objects") create_test_data(notification_type, sample_service) assert Notification.query.count() == 9 @@ -130,19 +134,29 @@ def test_delete_notifications_for_days_of_retention(sample_service, notification mock_get_s3.assert_not_called() +@mock_s3 @freeze_time('2019-09-01 04:30') def test_delete_notifications_deletes_letters_from_s3(sample_letter_template, mocker): - mock_get_s3 = mocker.patch("app.dao.notifications_dao.get_s3_bucket_objects") + s3 = boto3.client('s3', region_name='eu-west-1') + bucket_name = current_app.config['LETTERS_PDF_BUCKET_NAME'] + s3.create_bucket( + Bucket=bucket_name, + CreateBucketConfiguration={'LocationConstraint': 'eu-west-1'} + ) + 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 - ) + reference='LETTER_REF', created_at=eight_days_ago, sent_at=eight_days_ago) + filename = "{}/NOTIFY.LETTER_REF.D.2.C.C.{}.PDF".format( + str(eight_days_ago.date()), + eight_days_ago.strftime('%Y%m%d%H%M%S') + ) + s3.put_object(Bucket=bucket_name, Key=filename, Body=b'foo') + delete_notifications_older_than_retention_by_type(notification_type='letter') - mock_get_s3.assert_called_once_with(bucket_name=current_app.config['LETTERS_PDF_BUCKET_NAME'], - subfolder="{}/NOTIFY.LETTER_REF.D.2.C.C.{}.PDF".format( - str(eight_days_ago.date()), - eight_days_ago.strftime('%Y%m%d%H%M%S')) - ) + + with pytest.raises(s3.exceptions.NoSuchKey): + s3.get_object(Bucket=bucket_name, Key=filename) def test_delete_notifications_inserts_notification_history(sample_service): @@ -234,12 +248,19 @@ def test_delete_notifications_deletes_letters_not_sent_and_in_final_state_from_t mock_get_s3.assert_not_called() +@mock_s3 @freeze_time('2020-12-24 04:30') @pytest.mark.parametrize('notification_status', ['delivered', 'returned-letter', 'technical-failure']) def test_delete_notifications_deletes_letters_sent_and_in_final_state_from_table_and_s3( sample_service, mocker, notification_status ): - mock_get_s3 = mocker.patch("app.dao.notifications_dao.get_s3_bucket_objects") + bucket_name = current_app.config['LETTERS_PDF_BUCKET_NAME'] + s3 = boto3.client('s3', region_name='eu-west-1') + s3.create_bucket( + Bucket=bucket_name, + CreateBucketConfiguration={'LocationConstraint': 'eu-west-1'} + ) + letter_template = create_template(service=sample_service, template_type='letter') eight_days_ago = datetime.utcnow() - timedelta(days=8) create_notification( @@ -252,17 +273,19 @@ 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( + str(eight_days_ago.date()), + eight_days_ago.strftime('%Y%m%d%H%M%S') + ) + s3.put_object(Bucket=bucket_name, Key=filename, Body=b'foo') + delete_notifications_older_than_retention_by_type('letter') assert Notification.query.count() == 0 assert NotificationHistory.query.count() == 1 - mock_get_s3.assert_called_once_with( - bucket_name=current_app.config['LETTERS_PDF_BUCKET_NAME'], - subfolder="{}/NOTIFY.LETTER_REF.D.2.C.C.{}.PDF".format( - str(eight_days_ago.date()), - eight_days_ago.strftime('%Y%m%d%H%M%S') - ) - ) + + with pytest.raises(s3.exceptions.NoSuchKey): + s3.get_object(Bucket=bucket_name, Key=filename) @pytest.mark.parametrize('notification_status', ['pending-virus-check', 'created', 'sending']) diff --git a/tests/app/letters/test_letter_utils.py b/tests/app/letters/test_letter_utils.py index bca8ff5a6..2ae4d1031 100644 --- a/tests/app/letters/test_letter_utils.py +++ b/tests/app/letters/test_letter_utils.py @@ -9,10 +9,11 @@ from moto import mock_s3 from app.letters.utils import ( ScanErrorType, + find_letter_pdf_filename, + generate_letter_pdf_filename, get_bucket_name_and_prefix_for_notification, get_folder_name, get_letter_pdf_and_metadata, - get_letter_pdf_filename, letter_print_day, move_failed_pdf, move_sanitised_letter_to_test_or_live_pdf_bucket, @@ -46,6 +47,21 @@ def _sample_precompiled_letter_notification_using_test_key(sample_precompiled_le return sample_precompiled_letter_notification +@mock_s3 +def test_find_letter_pdf_filename_returns_filename(sample_notification): + bucket_name = current_app.config['LETTERS_PDF_BUCKET_NAME'] + s3 = boto3.client('s3', region_name='eu-west-1') + s3.create_bucket( + Bucket=bucket_name, + CreateBucketConfiguration={'LocationConstraint': 'eu-west-1'} + ) + + _, prefix = get_bucket_name_and_prefix_for_notification(sample_notification) + s3.put_object(Bucket=bucket_name, Key=f'{prefix}-and-then-some', Body=b'f') + + assert find_letter_pdf_filename(sample_notification) == f'{prefix}-and-then-some' + + @pytest.mark.parametrize('created_at,folder', [ (datetime(2017, 1, 1, 17, 29), '2017-01-01'), (datetime(2017, 1, 1, 17, 31), '2017-01-02'), @@ -145,10 +161,10 @@ def test_get_bucket_name_and_prefix_for_notification_invalid_notification(): (True, 'C'), (False, 'N'), ]) -def test_get_letter_pdf_filename_returns_correct_filename( +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 = get_letter_pdf_filename(reference='foo', crown=crown_flag, created_at=created_at) + 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) @@ -157,26 +173,30 @@ def test_get_letter_pdf_filename_returns_correct_filename( ('second', 2), ('first', 1), ]) -def test_get_letter_pdf_filename_returns_correct_postage_for_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 = get_letter_pdf_filename(reference='foo', crown=True, created_at=created_at, postage=postage) + filename = generate_letter_pdf_filename(reference='foo', crown=True, created_at=created_at, postage=postage) assert filename == '2017-12-04/NOTIFY.FOO.D.{}.C.C.20171204172900.PDF'.format(expected_postage) -def test_get_letter_pdf_filename_returns_correct_filename_for_test_letters( +def test_generate_letter_pdf_filename_returns_correct_filename_for_test_letters( notify_api, mocker): created_at = datetime(2017, 12, 4, 17, 29) - filename = get_letter_pdf_filename(reference='foo', crown='C', - created_at=created_at, ignore_folder=True) + 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' -def test_get_letter_pdf_filename_returns_tomorrows_filename(notify_api, mocker): +def test_generate_letter_pdf_filename_returns_tomorrows_filename(notify_api, mocker): created_at = datetime(2017, 12, 4, 17, 31) - filename = get_letter_pdf_filename(reference='foo', crown=True, created_at=created_at) + filename = generate_letter_pdf_filename(reference='foo', crown=True, created_at=created_at) assert filename == '2017-12-05/NOTIFY.FOO.D.2.C.C.20171204173100.PDF' @@ -223,7 +243,7 @@ def test_upload_letter_pdf_to_correct_bucket( mock_s3 = mocker.patch('app.letters.utils.s3upload') - filename = get_letter_pdf_filename( + filename = generate_letter_pdf_filename( reference=sample_letter_notification.reference, crown=sample_letter_notification.service.crown, created_at=sample_letter_notification.created_at, @@ -250,7 +270,7 @@ def test_upload_letter_pdf_uses_postage_from_notification( letter_notification = create_notification(template=sample_letter_template, postage=postage) mock_s3 = mocker.patch('app.letters.utils.s3upload') - filename = get_letter_pdf_filename( + filename = generate_letter_pdf_filename( reference=letter_notification.reference, crown=letter_notification.service.crown, created_at=letter_notification.created_at,