diff --git a/app/letters/utils.py b/app/letters/utils.py index 4fa041491..17863a953 100644 --- a/app/letters/utils.py +++ b/app/letters/utils.py @@ -64,16 +64,16 @@ def get_reference_from_filename(filename): return filename_parts[1] -def upload_letter_pdf(notification, pdf_data): +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( notification.reference, notification.service.crown, - is_scan_letter=notification.template.is_precompiled_letter) + is_scan_letter=precompiled) - if notification.template.is_precompiled_letter: + if precompiled: bucket_name = current_app.config['LETTERS_SCAN_BUCKET_NAME'] else: bucket_name = current_app.config['LETTERS_PDF_BUCKET_NAME'] diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index 7e39ad116..ef1941d01 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -220,23 +220,17 @@ def process_letter_notification(*, letter_data, api_key, template, reply_to_text if not api_key.service.research_mode and api_key.service.restricted and api_key.key_type != KEY_TYPE_TEST: raise BadRequestError(message='Cannot send letters when service is in trial mode', status_code=403) - should_send = not (api_key.service.research_mode or api_key.key_type == KEY_TYPE_TEST) + if precompiled: + return process_precompiled_letter_notifications(letter_data=letter_data, + api_key=api_key, + template=template, + reply_to_text=reply_to_text) + + should_send = not (api_key.key_type == KEY_TYPE_TEST) # if we don't want to actually send the letter, then start it off in SENDING so we don't pick it up status = NOTIFICATION_CREATED if should_send else NOTIFICATION_SENDING - if precompiled: - try: - if should_send or (precompiled and api_key.key_type == KEY_TYPE_TEST): - status = NOTIFICATION_PENDING_VIRUS_CHECK - letter_content = base64.b64decode(letter_data['content']) - pages = pdf_page_count(io.BytesIO(letter_content)) - except ValueError: - raise BadRequestError(message='Cannot decode letter content (invalid base64 encoding)', status_code=400) - except PdfReadError: - current_app.logger.exception(msg='Invalid PDF received') - raise BadRequestError(message='Letter content is not a valid PDF', status_code=400) - notification = create_letter_notification(letter_data=letter_data, template=template, api_key=api_key, @@ -244,45 +238,52 @@ def process_letter_notification(*, letter_data, api_key, template, reply_to_text reply_to_text=reply_to_text) if should_send: - if precompiled: - filename = upload_letter_pdf(notification, letter_content) - pages_per_sheet = 2 - notification.billable_units = math.ceil(pages / pages_per_sheet) - dao_update_notification(notification) - - current_app.logger.info( - 'Calling task scan-file for {}'.format(filename) - ) - - # call task to add the filename to anti virus queue - notify_celery.send_task( - name=TaskNames.SCAN_FILE, - kwargs={'filename': filename}, - queue=QueueNames.ANTIVIRUS, - ) - else: - create_letters_pdf.apply_async( - [str(notification.id)], - queue=QueueNames.CREATE_LETTERS_PDF - ) - elif (api_key.service.research_mode and - current_app.config['NOTIFY_ENVIRONMENT'] in ['preview', 'development']): + create_letters_pdf.apply_async( + [str(notification.id)], + queue=QueueNames.CREATE_LETTERS_PDF + ) + elif current_app.config['NOTIFY_ENVIRONMENT'] in ['preview', 'development']: create_fake_letter_response_file.apply_async( (notification.reference,), queue=QueueNames.RESEARCH_MODE ) else: - if precompiled and api_key.key_type == KEY_TYPE_TEST: - filename = upload_letter_pdf(notification, letter_content) + update_notification_status_by_reference(notification.reference, NOTIFICATION_DELIVERED) - # call task to add the filename to anti virus queue - notify_celery.send_task( - name=TaskNames.SCAN_FILE, - kwargs={'filename': filename}, - queue=QueueNames.ANTIVIRUS, - ) - else: - update_notification_status_by_reference(notification.reference, NOTIFICATION_DELIVERED) + return notification + + +def process_precompiled_letter_notifications(*, letter_data, api_key, template, reply_to_text): + try: + status = NOTIFICATION_PENDING_VIRUS_CHECK + letter_content = base64.b64decode(letter_data['content']) + pages = pdf_page_count(io.BytesIO(letter_content)) + except ValueError: + raise BadRequestError(message='Cannot decode letter content (invalid base64 encoding)', status_code=400) + except PdfReadError: + current_app.logger.exception(msg='Invalid PDF received') + raise BadRequestError(message='Letter content is not a valid PDF', status_code=400) + + notification = create_letter_notification(letter_data=letter_data, + template=template, + api_key=api_key, + status=status, + reply_to_text=reply_to_text) + + filename = upload_letter_pdf(notification, letter_content, precompiled=True) + pages_per_sheet = 2 + notification.billable_units = math.ceil(pages / pages_per_sheet) + + dao_update_notification(notification) + + current_app.logger.info('Calling task scan-file for {}'.format(filename)) + + # call task to add the filename to anti virus queue + notify_celery.send_task( + name=TaskNames.SCAN_FILE, + kwargs={'filename': filename}, + queue=QueueNames.ANTIVIRUS, + ) return notification diff --git a/tests/app/letters/test_letter_utils.py b/tests/app/letters/test_letter_utils.py index 5a8be234d..b4cfe678f 100644 --- a/tests/app/letters/test_letter_utils.py +++ b/tests/app/letters/test_letter_utils.py @@ -128,7 +128,7 @@ def test_upload_letter_pdf_to_correct_bucket( is_scan_letter=is_precompiled_letter ) - upload_letter_pdf(sample_letter_notification, b'\x00\x01') + upload_letter_pdf(sample_letter_notification, b'\x00\x01', precompiled=is_precompiled_letter) mock_s3.assert_called_once_with( bucket_name=current_app.config[bucket_config_name], diff --git a/tests/app/v2/notifications/test_post_letter_notifications.py b/tests/app/v2/notifications/test_post_letter_notifications.py index 851cf2e3c..72afe5214 100644 --- a/tests/app/v2/notifications/test_post_letter_notifications.py +++ b/tests/app/v2/notifications/test_post_letter_notifications.py @@ -96,12 +96,11 @@ def test_post_letter_notification_returns_201(client, sample_letter_template, mo @pytest.mark.parametrize('env', [ - 'development', - 'preview', + 'staging', + 'live', ]) -def test_post_letter_notification_calls_create_fake_response_in_research_and_test_key_correct_env( +def test_post_letter_notification_with_test_key_set_status_to_delivered( notify_api, client, sample_letter_template, mocker, env): - sample_letter_template.service.research_mode = True data = { 'template_id': str(sample_letter_template.id), @@ -126,83 +125,44 @@ def test_post_letter_notification_calls_create_fake_response_in_research_and_tes notification = Notification.query.one() + assert not fake_create_letter_task.called + assert not fake_create_dvla_response_task.called + assert notification.status == NOTIFICATION_DELIVERED + + +@pytest.mark.parametrize('env', [ + 'development', + 'preview', +]) +def test_post_letter_notification_with_test_key_sets_status_to_sending_and_sends_fake_response_file( + notify_api, client, sample_letter_template, mocker, env): + + data = { + 'template_id': str(sample_letter_template.id), + 'personalisation': { + 'address_line_1': 'Her Royal Highness Queen Elizabeth II', + 'address_line_2': 'Buckingham Palace', + 'address_line_3': 'London', + 'postcode': 'SW1 1AA', + 'name': 'Lizzie' + }, + 'reference': 'foo' + } + + fake_create_letter_task = mocker.patch('app.celery.letters_pdf_tasks.create_letters_pdf.apply_async') + fake_create_dvla_response_task = mocker.patch( + 'app.celery.research_mode_tasks.create_fake_letter_response_file.apply_async') + + with set_config_values(notify_api, { + 'NOTIFY_ENVIRONMENT': env + }): + letter_request(client, data, service_id=sample_letter_template.service_id, key_type=KEY_TYPE_TEST) + + notification = Notification.query.one() + + assert not fake_create_letter_task.called + assert fake_create_dvla_response_task.called assert notification.status == NOTIFICATION_SENDING - assert not fake_create_letter_task.called - fake_create_dvla_response_task.assert_called_once_with((notification.reference,), queue=QueueNames.RESEARCH_MODE) - - -@pytest.mark.parametrize('env', [ - 'staging', - 'live', -]) -def test_post_letter_notification_sets_status_delivered_in_research_and_test_key_incorrect_env( - notify_api, client, sample_letter_template, mocker, env): - sample_letter_template.service.research_mode = True - - data = { - 'template_id': str(sample_letter_template.id), - 'personalisation': { - 'address_line_1': 'Her Royal Highness Queen Elizabeth II', - 'address_line_2': 'Buckingham Palace', - 'address_line_3': 'London', - 'postcode': 'SW1 1AA', - 'name': 'Lizzie' - }, - 'reference': 'foo' - } - - fake_create_letter_task = mocker.patch('app.celery.letters_pdf_tasks.create_letters_pdf.apply_async') - fake_create_dvla_response_task = mocker.patch( - 'app.celery.research_mode_tasks.create_fake_letter_response_file.apply_async') - - with set_config_values(notify_api, { - 'NOTIFY_ENVIRONMENT': env - }): - letter_request(client, data, service_id=sample_letter_template.service_id, key_type=KEY_TYPE_TEST) - - notification = Notification.query.one() - - assert not fake_create_letter_task.called - assert not fake_create_dvla_response_task.called - assert notification.status == NOTIFICATION_DELIVERED - - -@pytest.mark.parametrize('env', [ - 'development', - 'preview', - 'staging', - 'live', -]) -def test_post_letter_notification_sets_status_to_delivered_using_test_key_and_not_research_all_env( - notify_api, client, sample_letter_template, mocker, env): - sample_letter_template.service.research_mode = False - - data = { - 'template_id': str(sample_letter_template.id), - 'personalisation': { - 'address_line_1': 'Her Royal Highness Queen Elizabeth II', - 'address_line_2': 'Buckingham Palace', - 'address_line_3': 'London', - 'postcode': 'SW1 1AA', - 'name': 'Lizzie' - }, - 'reference': 'foo' - } - - fake_create_letter_task = mocker.patch('app.celery.letters_pdf_tasks.create_letters_pdf.apply_async') - fake_create_dvla_response_task = mocker.patch( - 'app.celery.research_mode_tasks.create_fake_letter_response_file.apply_async') - - with set_config_values(notify_api, { - 'NOTIFY_ENVIRONMENT': env - }): - letter_request(client, data, service_id=sample_letter_template.service_id, key_type=KEY_TYPE_TEST) - - notification = Notification.query.one() - - assert not fake_create_letter_task.called - assert not fake_create_dvla_response_task.called - assert notification.status == NOTIFICATION_DELIVERED def test_post_letter_notification_returns_400_and_missing_template( @@ -234,11 +194,11 @@ def test_post_letter_notification_returns_400_for_empty_personalisation( assert error_json['status_code'] == 400 assert all([e['error'] == 'ValidationError' for e in error_json['errors']]) - assert set([e['message'] for e in error_json['errors']]) == set([ + assert set([e['message'] for e in error_json['errors']]) == { 'personalisation address_line_1 is required', 'personalisation address_line_2 is required', 'personalisation postcode is required' - ]) + } def test_notification_returns_400_for_missing_template_field( @@ -415,7 +375,7 @@ def test_post_letter_notification_is_delivered_and_has_pdf_uploaded_to_test_lett notification = Notification.query.one() assert notification.status == NOTIFICATION_PENDING_VIRUS_CHECK - s3mock.assert_called_once_with(ANY, b'letter-content') + s3mock.assert_called_once_with(ANY, b'letter-content', precompiled=True) mock_celery.assert_called_once_with( name=TaskNames.SCAN_FILE, kwargs={'filename': 'test.pdf'}, @@ -498,7 +458,7 @@ def test_post_precompiled_letter_notification_returns_201(client, notify_user, m assert response.status_code == 201, response.get_data(as_text=True) - s3mock.assert_called_once_with(ANY, b'letter-content') + s3mock.assert_called_once_with(ANY, b'letter-content', precompiled=True) notification = Notification.query.first()