From 69d86064f4e9e8ef30d53a1b96418a69a2085058 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Fri, 27 Sep 2019 11:12:12 +0100 Subject: [PATCH] WIP enforce letter page limit --- app/celery/letters_pdf_tasks.py | 4 ++-- app/letters/utils.py | 8 ++++++-- app/service/send_notification.py | 4 ++-- app/v2/notifications/post_notifications.py | 2 +- tests/app/celery/test_letters_pdf_tasks.py | 16 ++++++++-------- tests/app/service/test_rest.py | 2 +- .../service/test_send_pdf_letter_notification.py | 2 +- 7 files changed, 21 insertions(+), 17 deletions(-) diff --git a/app/celery/letters_pdf_tasks.py b/app/celery/letters_pdf_tasks.py index 434f2fdb7..08df83fa0 100644 --- a/app/celery/letters_pdf_tasks.py +++ b/app/celery/letters_pdf_tasks.py @@ -38,7 +38,7 @@ from app.letters.utils import ( move_scan_to_invalid_pdf_bucket, move_error_pdf_to_scan_bucket, get_file_names_from_error_bucket, - get_page_count, + get_billable_units_for_pdf, ) from app.models import ( KEY_TYPE_TEST, @@ -211,7 +211,7 @@ def process_virus_scan_passed(self, filename): old_pdf = scan_pdf_object.get()['Body'].read() try: - billable_units = get_page_count(old_pdf) + billable_units = get_billable_units_for_pdf(old_pdf) except PdfReadError: current_app.logger.exception(msg='Invalid PDF received for notification_id: {}'.format(notification.id)) _move_invalid_letter_and_update_status(notification, filename, scan_pdf_object) diff --git a/app/letters/utils.py b/app/letters/utils.py index 98e954fb5..0f05718a8 100644 --- a/app/letters/utils.py +++ b/app/letters/utils.py @@ -211,8 +211,12 @@ def letter_print_day(created_at): return 'on {}'.format(print_date) -def get_page_count(pdf): - pages = pdf_page_count(io.BytesIO(pdf)) +def get_page_count_for_pdf(pdf): + return pdf_page_count(io.BytesIO(pdf)) + + +def get_billable_units_for_pdf(pdf): + pages = get_page_count_for_pdf(pdf) pages_per_sheet = 2 billable_units = math.ceil(pages / pages_per_sheet) return billable_units diff --git a/app/service/send_notification.py b/app/service/send_notification.py index 00c78936b..adb3575e8 100644 --- a/app/service/send_notification.py +++ b/app/service/send_notification.py @@ -31,7 +31,7 @@ from app.dao.templates_dao import dao_get_template_by_id_and_service_id, get_pre from app.dao.users_dao import get_user_by_id from app.letters.utils import ( get_letter_pdf_filename, - get_page_count, + get_billable_units_for_pdf, move_uploaded_pdf_to_letters_bucket, ) from app.v2.errors import BadRequestError @@ -152,7 +152,7 @@ def send_pdf_letter_notification(service_id, post_data): raise e # Getting the page count won't raise an error since admin has already checked the PDF is valid - billable_units = get_page_count(letter.read()) + billable_units = get_billable_units_for_pdf(letter.read()) personalisation = { 'address_line_1': post_data['filename'] diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index 7a36ffbfd..517c3413a 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -12,7 +12,7 @@ from app.clients.document_download import DocumentDownloadError from app.config import QueueNames, TaskNames from app.dao.notifications_dao import update_notification_status_by_reference from app.dao.templates_dao import get_precompiled_letter_template -from app.letters.utils import upload_letter_pdf +from app.letters.utils import upload_letter_pdf, get_page_count_for_pdf from app.models import ( SMS_TYPE, EMAIL_TYPE, diff --git a/tests/app/celery/test_letters_pdf_tasks.py b/tests/app/celery/test_letters_pdf_tasks.py index 441e3bffc..fde2cc9af 100644 --- a/tests/app/celery/test_letters_pdf_tasks.py +++ b/tests/app/celery/test_letters_pdf_tasks.py @@ -426,7 +426,7 @@ def test_process_letter_task_check_virus_scan_passed( s3 = boto3.client('s3', region_name='eu-west-1') s3.put_object(Bucket=source_bucket_name, Key=filename, Body=b'old_pdf') - mock_get_page_count = mocker.patch('app.celery.letters_pdf_tasks.get_page_count', return_value=1) + mock_get_billable_units_for_pdf = mocker.patch('app.celery.letters_pdf_tasks.get_billable_units_for_pdf', return_value=1) mock_s3upload = mocker.patch('app.celery.letters_pdf_tasks.s3upload') endpoint = 'http://localhost:9999/precompiled/sanitise' with requests_mock.mock() as rmock: @@ -456,7 +456,7 @@ def test_process_letter_task_check_virus_scan_passed( file_location=destination_folder + filename, region='eu-west-1', ) - mock_get_page_count.assert_called_once_with(b'old_pdf') + mock_get_billable_units_for_pdf.assert_called_once_with(b'old_pdf') @freeze_time('2018-01-01 18:00') @@ -480,7 +480,7 @@ def test_process_letter_task_check_virus_scan_passed_when_sanitise_fails( sample_letter_notification.key_type = key_type mock_move_s3 = mocker.patch('app.letters.utils._move_s3_object') mock_sanitise = mocker.patch('app.celery.letters_pdf_tasks._sanitise_precompiled_pdf', return_value=None) - mock_get_page_count = mocker.patch('app.celery.letters_pdf_tasks.get_page_count', return_value=2) + mock_get_billable_units_for_pdf = mocker.patch('app.celery.letters_pdf_tasks.get_billable_units_for_pdf', return_value=2) process_virus_scan_passed(filename) @@ -496,7 +496,7 @@ def test_process_letter_task_check_virus_scan_passed_when_sanitise_fails( target_bucket_name, filename ) - mock_get_page_count.assert_called_once_with(b'pdf_content') + mock_get_billable_units_for_pdf.assert_called_once_with(b'pdf_content') @freeze_time('2018-01-01 18:00') @@ -522,7 +522,7 @@ def test_process_letter_task_check_virus_scan_passed_when_redaction_fails( sample_letter_notification.status = NOTIFICATION_PENDING_VIRUS_CHECK sample_letter_notification.key_type = key_type mock_copy_s3 = mocker.patch('app.letters.utils._copy_s3_object') - mocker.patch('app.celery.letters_pdf_tasks.get_page_count', return_value=2) + mocker.patch('app.celery.letters_pdf_tasks.get_billable_units_for_pdf', return_value=2) endpoint = 'http://localhost:9999/precompiled/sanitise' with requests_mock.mock() as rmock: @@ -574,13 +574,13 @@ def test_process_letter_task_check_virus_scan_passed_when_file_cannot_be_opened( sample_letter_notification.key_type = key_type mock_move_s3 = mocker.patch('app.letters.utils._move_s3_object') - mock_get_page_count = mocker.patch('app.celery.letters_pdf_tasks.get_page_count', side_effect=PdfReadError) + mock_get_billable_units_for_pdf = mocker.patch('app.celery.letters_pdf_tasks.get_billable_units_for_pdf', side_effect=PdfReadError) mock_sanitise = mocker.patch('app.celery.letters_pdf_tasks._sanitise_precompiled_pdf') process_virus_scan_passed(filename) mock_sanitise.assert_not_called() - mock_get_page_count.assert_called_once_with(b'pdf_content') + mock_get_billable_units_for_pdf.assert_called_once_with(b'pdf_content') mock_move_s3.assert_called_once_with( source_bucket_name, filename, target_bucket_name, filename @@ -606,7 +606,7 @@ def test_process_virus_scan_passed_logs_error_and_sets_tech_failure_if_s3_error_ s3 = boto3.client('s3', region_name='eu-west-1') s3.put_object(Bucket=source_bucket_name, Key=filename, Body=b'pdf_content') - mocker.patch('app.celery.letters_pdf_tasks.get_page_count', return_value=1) + mocker.patch('app.celery.letters_pdf_tasks.get_billable_units_for_pdf', return_value=1) error_response = { 'Error': { diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index b4dbcb068..bd0597398 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -2275,7 +2275,7 @@ def test_send_one_off_notification(sample_service, admin_request, mocker): def test_create_pdf_letter(mocker, sample_service_full_permissions, client, fake_uuid, notify_user): mocker.patch('app.service.send_notification.utils_s3download') - mocker.patch('app.service.send_notification.get_page_count', return_value=1) + mocker.patch('app.service.send_notification.get_billable_units_for_pdf', return_value=1) mocker.patch('app.service.send_notification.move_uploaded_pdf_to_letters_bucket') user = sample_service_full_permissions.users[0] diff --git a/tests/app/service/test_send_pdf_letter_notification.py b/tests/app/service/test_send_pdf_letter_notification.py index 71b57f51d..f03672c2c 100644 --- a/tests/app/service/test_send_pdf_letter_notification.py +++ b/tests/app/service/test_send_pdf_letter_notification.py @@ -77,7 +77,7 @@ def test_send_pdf_letter_notification_creates_notification_and_moves_letter( post_data = {'filename': filename, 'created_by': user.id, 'file_id': file_id} mocker.patch('app.service.send_notification.utils_s3download') - mocker.patch('app.service.send_notification.get_page_count', return_value=1) + mocker.patch('app.service.send_notification.get_billable_units_for_pdf', return_value=1) s3_mock = mocker.patch('app.service.send_notification.move_uploaded_pdf_to_letters_bucket') result = send_pdf_letter_notification(sample_service_full_permissions.id, post_data)