Refactor and harmonise metadata for invalid letters with those sent from admin app

app/celery/letters_pdf_tasks.py

@@ -5,7 +5,6 @@ from uuid import UUID
 from hashlib import sha512
 from base64 import urlsafe_b64encode
 
-from PyPDF2.utils import PdfReadError
 from botocore.exceptions import ClientError as BotoClientError
 from flask import current_app
 from requests import (
@@ -30,6 +29,7 @@ from app.dao.notifications_dao import (
 from app.errors import VirusScanError
 from app.letters.utils import (
     copy_redaction_failed_pdf,
+    get_billable_units_for_letter_page_count,
     get_reference_from_filename,
     get_folder_name,
     upload_letter_pdf,
@@ -38,7 +38,6 @@ 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,
 )
 from app.models import (
     KEY_TYPE_TEST,
@@ -209,17 +208,8 @@ def process_virus_scan_passed(self, filename):
     scan_pdf_object = s3.get_s3_object(current_app.config['LETTERS_SCAN_BUCKET_NAME'], filename)
     old_pdf = scan_pdf_object.get()['Body'].read()
 
-    try:
-        billable_units = get_page_count(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=notification, filename=filename, scan_pdf_object=scan_pdf_object
-        )
-        return
-
-    sanitise_response = _sanitise_precompiled_pdf(self, notification, old_pdf)
-    if sanitise_response["message"]:  # is response without message attribute possible now? I think not?
+    sanitise_response, result = _sanitise_precompiled_pdf(self, notification, old_pdf)
+    if result == "validation_failed":
         new_pdf = None
     else:
         new_pdf = base64.b64decode(sanitise_response["file"].encode())
@@ -231,6 +221,8 @@ def process_virus_scan_passed(self, filename):
             )
             copy_redaction_failed_pdf(filename)
 
+    billable_units = get_billable_units_for_letter_page_count(sanitise_response.get("page_count"))
+
     # TODO: Remove this once CYSP update their template to not cross over the margins
     if notification.service_id == UUID('fe44178f-3b45-4625-9f85-2264a36dd9ec'):  # CYSP
         # Check your state pension submit letters with good addresses and notify tags, so just use their supplied pdf
@@ -322,19 +314,19 @@ def _sanitise_precompiled_pdf(self, notification, precompiled_pdf):
                      'Notification-ID': str(notification.id)}
         )
         response.raise_for_status()
-        return response.json()
+        return response.json(), "validation_passed"
     except RequestException as ex:
         if ex.response is not None and ex.response.status_code == 400:
             message = "sanitise_precompiled_pdf validation error for notification: {}. ".format(notification.id)
-            if "message" in response.json():
+            if response.json().get("message"):
                 message += response.json()["message"]
-            if "invalid_pages" in response.json():
-                message += (" on pages: " + ", ".join(map(str, response.json()["invalid_pages"])))
+                if response.json().get("invalid_pages"):
+                    message += (" on pages: " + ", ".join(map(str, response.json()["invalid_pages"])))
 
             current_app.logger.info(
                 message
             )
-            return response.json()
+            return response.json(), "validation_failed"
 
         try:
             current_app.logger.exception(

app/letters/utils.py

@@ -1,9 +1,13 @@
+import boto3
 import io
+import json
 import math
+
+from app.models import KEY_TYPE_TEST, SECOND_CLASS, RESOLVE_POSTAGE_FOR_FILE_NAME, NOTIFICATION_VALIDATION_FAILED
+
 from datetime import datetime, timedelta
 from enum import Enum
 
-import boto3
 from flask import current_app
 
 from notifications_utils.letter_timings import LETTER_PROCESSING_DEADLINE
@@ -11,8 +15,6 @@ from notifications_utils.pdf import pdf_page_count
 from notifications_utils.s3 import s3upload
 from notifications_utils.timezones import convert_utc_to_bst
 
-from app.models import KEY_TYPE_TEST, SECOND_CLASS, RESOLVE_POSTAGE_FOR_FILE_NAME, NOTIFICATION_VALIDATION_FAILED
-
 
 class ScanErrorType(Enum):
     ERROR = 1
@@ -130,9 +132,9 @@ def move_error_pdf_to_scan_bucket(source_filename):
 def move_scan_to_invalid_pdf_bucket(source_filename, message=None, invalid_pages=None, page_count=None):
     metadata = {}
     if message:
-        metadata["validation_failed_message"] = message
+        metadata["message"] = message
     if invalid_pages:
-        metadata["invalid_pages"] = "-".join(map(str, invalid_pages))
+        metadata["invalid_pages"] = json.dumps(invalid_pages)
     if page_count:
         metadata["page_count"] = str(page_count)
 
@@ -228,7 +230,12 @@ def letter_print_day(created_at):
 
 
 def get_page_count(pdf):
-    pages = pdf_page_count(io.BytesIO(pdf))
+    return pdf_page_count(io.BytesIO(pdf))
+
+
+def get_billable_units_for_letter_page_count(page_count):
+    if not page_count:
+        return 0
     pages_per_sheet = 2
-    billable_units = math.ceil(pages / pages_per_sheet)
+    billable_units = math.ceil(page_count / pages_per_sheet)
     return billable_units

app/service/send_notification.py

@@ -30,6 +30,7 @@ from app.dao.services_dao import dao_fetch_service_by_id
 from app.dao.templates_dao import dao_get_template_by_id_and_service_id, get_precompiled_letter_template
 from app.dao.users_dao import get_user_by_id
 from app.letters.utils import (
+    get_billable_units_for_letter_page_count,
     get_letter_pdf_filename,
     get_page_count,
     move_uploaded_pdf_to_letters_bucket,
@@ -152,7 +153,8 @@ 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())
+    page_count = get_page_count(letter.read())
+    billable_units = get_billable_units_for_letter_page_count(page_count)
 
     personalisation = {
         'address_line_1': post_data['filename']