From 3dcb97c45af19a33ccfbb76921680612fac9854f Mon Sep 17 00:00:00 2001 From: David McDonald Date: Fri, 23 Oct 2020 09:58:28 +0100 Subject: [PATCH] Remove 'INSOLVENCY' from zip files for insolvency letters This is at request of DVLA. They would prefer to have zip files with the same number of arguments in the name. After being offered a few different options, such as including an org and service id for all zips, they chose to just remove the 'INSOLVENCY' tag. For more context see PR that added the tag https://github.com/alphagov/notifications-api/pull/3006 --- app/celery/letters_pdf_tasks.py | 6 +- tests/app/celery/test_letters_pdf_tasks.py | 110 +++------------------ 2 files changed, 12 insertions(+), 104 deletions(-) diff --git a/app/celery/letters_pdf_tasks.py b/app/celery/letters_pdf_tasks.py index a64dfb306..a2431fbd1 100644 --- a/app/celery/letters_pdf_tasks.py +++ b/app/celery/letters_pdf_tasks.py @@ -181,10 +181,6 @@ def get_key_and_size_of_letters_to_be_sent_to_print(print_run_deadline, postage) letter_pdfs = [] for letter in letters_awaiting_sending: try: - if str(letter.service.organisation_id) == 'f33fdfdd-7533-40cb-b5e8-cd78a1f5d21e': - letter_service_marking = str(letter.service.id) + ".INSOLVENCY" - else: - letter_service_marking = str(letter.service.id) letter_file_name = get_letter_pdf_filename( reference=letter.reference, crown=letter.service.crown, @@ -195,7 +191,7 @@ def get_key_and_size_of_letters_to_be_sent_to_print(print_run_deadline, postage) letter_pdfs.append({ "Key": letter_file_name, "Size": letter_head['ContentLength'], - "ServiceId": letter_service_marking + "ServiceId": str(letter.service.id) }) except BotoClientError as e: current_app.logger.exception( diff --git a/tests/app/celery/test_letters_pdf_tasks.py b/tests/app/celery/test_letters_pdf_tasks.py index 7ebccee7e..6e1664fef 100644 --- a/tests/app/celery/test_letters_pdf_tasks.py +++ b/tests/app/celery/test_letters_pdf_tasks.py @@ -45,7 +45,6 @@ from app.models import ( from tests.app.db import ( create_letter_branding, create_notification, - create_organisation, create_service, create_template ) @@ -256,68 +255,6 @@ def test_get_key_and_size_of_letters_to_be_sent_to_print(notify_api, mocker, sam ] -@freeze_time('2020-02-17 18:00:00') -def test_get_key_and_size_of_letters_to_be_sent_to_print_mark_insolvency_letters( - notify_api, notify_db_session, mocker -): - # random service - service_1 = create_service(service_name="service 1", service_id='f2fe37b0-1301-11eb-aba9-4c3275916899') - letter_template_1 = create_template(service_1, template_type=LETTER_TYPE) - create_notification( - template=letter_template_1, - status='created', - reference='ref0', - created_at=(datetime.now() - timedelta(hours=2)) - ) - # insolvency service - insolvency_org = create_organisation(organisation_id='f33fdfdd-7533-40cb-b5e8-cd78a1f5d21e', name="Insolvency") - insolvency_service = create_service( - service_name="insolvency service", - service_id='3a5cea08-29fd-4bb9-b582-8dedd928b149', - organisation=insolvency_org - ) - insolvency_letter_template = create_template(insolvency_service, template_type=LETTER_TYPE) - create_notification( - template=insolvency_letter_template, - status='created', - reference='insolvency', - created_at=(datetime.now() - timedelta(hours=3)) - ) - - mock_s3 = mocker.patch('app.celery.tasks.s3.head_s3_object', side_effect=[ - {'ContentLength': 1}, - {'ContentLength': 1}, - ]) - - results = get_key_and_size_of_letters_to_be_sent_to_print(datetime.now() - timedelta(minutes=30), postage='second') - - assert mock_s3.call_count == 2 - mock_s3.assert_has_calls( - [ - call( - current_app.config['LETTERS_PDF_BUCKET_NAME'], - '2020-02-17/NOTIFY.INSOLVENCY.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) == 2 - - assert results == [ - { - 'Key': '2020-02-17/NOTIFY.INSOLVENCY.D.2.C.C.20200217150000.PDF', - 'Size': 1, - 'ServiceId': str(insolvency_letter_template.service_id) + ".INSOLVENCY" - }, - { - 'Key': '2020-02-17/NOTIFY.REF0.D.2.C.C.20200217160000.PDF', - 'Size': 1, - 'ServiceId': str(letter_template_1.service_id) - }, - ] - - @freeze_time('2020-02-17 18:00:00') def test_get_key_and_size_of_letters_to_be_sent_to_print_catches_exception( notify_api, mocker, sample_letter_template @@ -429,21 +366,6 @@ def test_collate_letter_pdfs_to_be_sent( created_at=(datetime.now() - timedelta(hours=2)) ) - # insolvency service - insolvency_org = create_organisation(organisation_id='f33fdfdd-7533-40cb-b5e8-cd78a1f5d21e', name="Insolvency") - insolvency_service = create_service( - service_name="insolvency service", - service_id='0d3eb2b8-12fe-11eb-88be-4c3275916899', - organisation=insolvency_org - ) - insolvency_letter_template = create_template(insolvency_service, template_type=LETTER_TYPE) - create_notification( - template=insolvency_letter_template, - status='created', - reference='insolvency', - created_at=(datetime.now() - timedelta(hours=3)) - ) - mocker.patch('app.celery.tasks.s3.head_s3_object', side_effect=[ {'ContentLength': 1}, {'ContentLength': 1}, @@ -452,7 +374,6 @@ def test_collate_letter_pdfs_to_be_sent( {'ContentLength': 3}, {'ContentLength': 1}, {'ContentLength': 1}, - {'ContentLength': 1}, ]) mock_celery = mocker.patch('app.celery.letters_pdf_tasks.notify_celery.send_task') @@ -461,7 +382,7 @@ def test_collate_letter_pdfs_to_be_sent( with freeze_time(time_to_run_task): collate_letter_pdfs_to_be_sent() - assert len(mock_celery.call_args_list) == 7 + assert len(mock_celery.call_args_list) == 6 assert mock_celery.call_args_list[0] == call( name='zip-and-send-letter-pdfs', kwargs={ @@ -476,8 +397,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.INSOLVENCY.D.2.C.C.20200217150000.PDF'], - 'upload_filename': f'NOTIFY.2020-02-17.2.001.riA3Fz85m5DumlU7vaC0.{insolvency_service.id}.INSOLVENCY.ZIP' + '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' }, queue='process-ftp-tasks', compression='zlib' @@ -485,8 +406,11 @@ def test_collate_letter_pdfs_to_be_sent( assert mock_celery.call_args_list[2] == 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.002.MezXnKP3IvNZEoMsSlVo.{service_2.id}.ZIP' + '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' + ], + 'upload_filename': f'NOTIFY.2020-02-17.2.002.k3x_WqC5KhB6e2DWv9Ma.{letter_template_1.service_id}.ZIP' }, queue='process-ftp-tasks', compression='zlib' @@ -495,26 +419,14 @@ 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-17/NOTIFY.REF0.D.2.C.C.20200217160000.PDF' ], - 'upload_filename': f'NOTIFY.2020-02-17.2.003.k3x_WqC5KhB6e2DWv9Ma.{letter_template_1.service_id}.ZIP' + 'upload_filename': f'NOTIFY.2020-02-17.2.003.J85cUw-FWlKuAIOcwdLS.{letter_template_1.service_id}.ZIP' }, queue='process-ftp-tasks', compression='zlib' ) assert mock_celery.call_args_list[4] == call( - name='zip-and-send-letter-pdfs', - kwargs={ - 'filenames_to_zip': [ - '2020-02-17/NOTIFY.REF0.D.2.C.C.20200217160000.PDF' - ], - 'upload_filename': f'NOTIFY.2020-02-17.2.004.J85cUw-FWlKuAIOcwdLS.{letter_template_1.service_id}.ZIP' - }, - queue='process-ftp-tasks', - compression='zlib' - ) - assert mock_celery.call_args_list[5] == call( name='zip-and-send-letter-pdfs', kwargs={ 'filenames_to_zip': [ @@ -525,7 +437,7 @@ def test_collate_letter_pdfs_to_be_sent( queue='process-ftp-tasks', compression='zlib' ) - assert mock_celery.call_args_list[6] == call( + assert mock_celery.call_args_list[5] == call( name='zip-and-send-letter-pdfs', kwargs={ 'filenames_to_zip': [