add explanation to ft_billing model

i found myself constantly trying to work out what each column means, so
i wrote it down

app/celery/letters_pdf_tasks.py

@@ -31,6 +31,7 @@ from app.letters.utils import (
     generate_letter_pdf_filename,
     get_billable_units_for_letter_page_count,
     get_file_names_from_error_bucket,
+    get_folder_name,
     get_reference_from_filename,
     move_error_pdf_to_scan_bucket,
     move_failed_pdf,
@@ -524,3 +525,37 @@ def replay_letters_in_error(filename=None):
                     [filename],
                     queue=QueueNames.LETTERS
                 )
+
+
+@notify_celery.task(name='resanitise-pdf')
+def resanitise_pdf(notification_id):
+    """
+    `notification_id` is the notification id for a PDF letter which was either uploaded or sent using the API.
+
+    This task calls the `recreate_pdf_for_precompiled_letter` template preview task which recreates the
+    PDF for a letter which is already sanitised and in the letters-pdf bucket. The new file that is generated
+    will then overwrite the existing letter in the letters-pdf bucket.
+    """
+    notification = get_notification_by_id(notification_id)
+
+    # folder_name is the folder that the letter is in the letters-pdf bucket e.g. '2021-10-10/'
+    folder_name = get_folder_name(notification.created_at)
+
+    filename = generate_letter_pdf_filename(
+            reference=notification.reference,
+            created_at=notification.created_at,
+            ignore_folder=True,
+            postage=notification.postage
+        )
+
+    notify_celery.send_task(
+        name=TaskNames.RECREATE_PDF_FOR_PRECOMPILED_LETTER,
+        kwargs={
+            'notification_id': str(notification.id),
+            'file_location': f'{folder_name}{filename}',
+            'allow_international_letters': notification.service.has_permission(
+                INTERNATIONAL_LETTERS
+            ),
+        },
+        queue=QueueNames.SANITISE_LETTERS,
+    )

app/celery/nightly_tasks.py

@@ -130,25 +130,35 @@ def delete_letter_notifications_older_than_retention():
 @notify_celery.task(name='timeout-sending-notifications')
 @cronitor('timeout-sending-notifications')
 def timeout_notifications():
-    technical_failure_notifications, temporary_failure_notifications = \
-        dao_timeout_notifications(current_app.config.get('SENDING_NOTIFICATIONS_TIMEOUT_PERIOD'))
+    # TEMPORARY: re-run the following code over small batches of notifications
+    # so that we can cope with a high volume that need processing. We've changed
+    # dao_timeout_notifications to return up to 100K notifications, so this task
+    # will operate on up to 500K - normally we only get around 20K.
+    for _ in range(0, 5):
+        technical_failure_notifications, temporary_failure_notifications = \
+            dao_timeout_notifications(current_app.config.get('SENDING_NOTIFICATIONS_TIMEOUT_PERIOD'))
 
-    notifications = technical_failure_notifications + temporary_failure_notifications
-    for notification in notifications:
-        # queue callback task only if the service_callback_api exists
-        service_callback_api = get_service_delivery_status_callback_api_for_service(service_id=notification.service_id)
-        if service_callback_api:
-            encrypted_notification = create_delivery_status_callback_data(notification, service_callback_api)
-            send_delivery_status_to_service.apply_async([str(notification.id), encrypted_notification],
-                                                        queue=QueueNames.CALLBACKS)
+        notifications = technical_failure_notifications + temporary_failure_notifications
+        for notification in notifications:
+            # queue callback task only if the service_callback_api exists
+            service_callback_api = get_service_delivery_status_callback_api_for_service(service_id=notification.service_id)  # noqa: E501
+            if service_callback_api:
+                encrypted_notification = create_delivery_status_callback_data(notification, service_callback_api)
+                send_delivery_status_to_service.apply_async([str(notification.id), encrypted_notification],
+                                                            queue=QueueNames.CALLBACKS)
 
-    current_app.logger.info(
-        "Timeout period reached for {} notifications, status has been updated.".format(len(notifications)))
-    if technical_failure_notifications:
-        message = "{} notifications have been updated to technical-failure because they " \
-                  "have timed out and are still in created.Notification ids: {}".format(
-                      len(technical_failure_notifications), [str(x.id) for x in technical_failure_notifications])
-        raise NotificationTechnicalFailureException(message)
+        current_app.logger.info(
+            "Timeout period reached for {} notifications, status has been updated.".format(len(notifications)))
+        if technical_failure_notifications:
+            message = "{} notifications have been updated to technical-failure because they " \
+                      "have timed out and are still in created.Notification ids: {}".format(
+                          len(technical_failure_notifications), [str(x.id) for x in technical_failure_notifications])
+            raise NotificationTechnicalFailureException(message)
+
+        if len(notifications) < 100000:
+            return
+
+    raise RuntimeError("Some notifications may still be in sending.")
 
 
 @notify_celery.task(name="delete-inbound-sms")

app/commands.py

@@ -19,7 +19,10 @@ from sqlalchemy.orm.exc import NoResultFound
 
 from app import db
 from app.aws import s3
-from app.celery.letters_pdf_tasks import get_pdf_for_templated_letter
+from app.celery.letters_pdf_tasks import (
+    get_pdf_for_templated_letter,
+    resanitise_pdf,
+)
 from app.celery.reporting_tasks import (
     create_nightly_notification_status_for_day,
 )
@@ -271,14 +274,22 @@ def insert_inbound_numbers_from_file(file_name):
     file.close()
 
 
-@notify_command(name='replay-create-pdf-letters')
+@notify_command(name='replay-create-pdf-for-templated-letter')
 @click.option('-n', '--notification_id', type=click.UUID, required=True,
               help="Notification id of the letter that needs the get_pdf_for_templated_letter task replayed")
-def replay_create_pdf_letters(notification_id):
+def replay_create_pdf_for_templated_letter(notification_id):
     print("Create task to get_pdf_for_templated_letter for notification: {}".format(notification_id))
     get_pdf_for_templated_letter.apply_async([str(notification_id)], queue=QueueNames.CREATE_LETTERS_PDF)
 
 
+@notify_command(name='recreate-pdf-for-precompiled-or-uploaded-letter')
+@click.option('-n', '--notification_id', type=click.UUID, required=True,
+              help="Notification ID of the precompiled or uploaded letter")
+def recreate_pdf_for_precompiled_or_uploaded_letter(notification_id):
+    print(f"Call resanitise_pdf task for notification: {notification_id}")
+    resanitise_pdf.apply_async([str(notification_id)], queue=QueueNames.LETTERS)
+
+
 @notify_command(name='replay-service-callbacks')
 @click.option('-f', '--file_name', required=True,
               help="""Full path of the file to upload, file is a contains client references of

app/config.py

@@ -77,6 +77,7 @@ class TaskNames(object):
     SANITISE_LETTER = 'sanitise-and-upload-letter'
     CREATE_PDF_FOR_TEMPLATED_LETTER = 'create-pdf-for-templated-letter'
     PUBLISH_GOVUK_ALERTS = 'publish-govuk-alerts'
+    RECREATE_PDF_FOR_PRECOMPILED_LETTER = 'recreate-pdf-for-precompiled-letter'
 
 
 class Config(object):

app/dao/notifications_dao.py

@@ -467,15 +467,21 @@ def dao_delete_notifications_by_id(notification_id):
 
 
 def _timeout_notifications(current_statuses, new_status, timeout_start, updated_at):
+    # TEMPORARY: limit the notifications to 100K as otherwise we
+    # see an issues where the task vanishes after it starts executing
+    # - we believe this is a OOM error but there are no logs. From
+    # experimentation we've found we can safely process up to 100K.
     notifications = Notification.query.filter(
         Notification.created_at < timeout_start,
         Notification.status.in_(current_statuses),
-        Notification.notification_type != LETTER_TYPE
-    ).all()
+        Notification.notification_type.in_([SMS_TYPE, EMAIL_TYPE])
+    ).limit(100000).all()
+
     Notification.query.filter(
         Notification.created_at < timeout_start,
         Notification.status.in_(current_statuses),
-        Notification.notification_type != LETTER_TYPE
+        Notification.notification_type.in_([SMS_TYPE, EMAIL_TYPE]),
+        Notification.id.in_([n.id for n in notifications]),
     ).update(
         {'status': new_status, 'updated_at': updated_at},
         synchronize_session=False

app/models.py

@@ -2069,6 +2069,35 @@ class DailySortedLetter(db.Model):
 
 
 class FactBilling(db.Model):
+    """
+    These are grouped by date, template, notificaiton type, provider, rate_multiplier, international, rate, and postage.
+
+    For SMS:
+
+    rate = cost per billable unit. Will only change when we change our pricing.
+    rate_multiplier = international rate multiplier as defined in notifications_utils/international_billing_rates.yml
+
+    billable_units = sum of billable units (fragments) for that grouping
+    notifications_sent = sum of notifications sent for that grouping
+
+    For letters:
+
+    rate = the cost to send a single notification, which will vary based on postage and page count.
+    rate_multiplier = always 1
+
+    billable_units = sum of sheets of paper sent for that grouping
+    notifications_sent = sum of notifications sent for that grouping
+
+    handy letter calculations:
+
+    number_of_sheets_per_letter = billable_units / notifications_sent
+    letter_cost = notifications_sent * ft_billing.rate
+
+    For emails:
+
+    rate_multiplier = 0
+    everything's free!
+    """
     __tablename__ = "ft_billing"
 
     bst_date = db.Column(db.Date, nullable=False, primary_key=True, index=True)

app/v2/notifications/post_notifications.py

@@ -232,10 +232,9 @@ def process_sms_or_email_notification(
                 reply_to_text=reply_to_text
             )
             return resp
-        except (botocore.exceptions.ClientError, botocore.parsers.ResponseParserError):
-            # If SQS cannot put the task on the queue, it's probably because the notification body was too long and it
-            # went over SQS's 256kb message limit. If the body is very large, it may exceed the HTTP max content length;
-            # the exception we get here isn't handled correctly by botocore - we get a ResponseParserError instead.
+        except botocore.exceptions.ClientError:
+            # if SQS cannot put the task on the queue, it's probably because the notification body was too long and it
+            # went over SQS's 256kb message limit. If so, we
             current_app.logger.info(
                 f'Notification {notification_id} failed to save to high volume queue. Using normal flow instead'
             )

tests/app/celery/test_letters_pdf_tasks.py

@@ -23,6 +23,7 @@ from app.celery.letters_pdf_tasks import (
     process_virus_scan_error,
     process_virus_scan_failed,
     replay_letters_in_error,
+    resanitise_pdf,
     sanitise_letter,
     send_letters_volume_email_to_dvla,
     update_billable_units_for_letter,
@@ -1098,3 +1099,33 @@ def test_replay_letters_in_error_for_one_file(notify_api, mocker):
     replay_letters_in_error("file_name")
     mock_move.assert_called_once_with('file_name')
     mock_celery.assert_called_once_with(name='scan-file', kwargs={'filename': 'file_name'}, queue='antivirus-tasks')
+
+
+@pytest.mark.parametrize('permissions, expected_international_letters_allowed', (
+    ([LETTER_TYPE], False),
+    ([LETTER_TYPE, INTERNATIONAL_LETTERS], True),
+))
+def test_resanitise_pdf_calls_template_preview_with_letter_details(
+    mocker,
+    sample_letter_notification,
+    permissions,
+    expected_international_letters_allowed,
+):
+    mock_celery = mocker.patch('app.celery.letters_pdf_tasks.notify_celery.send_task')
+
+    sample_letter_notification.created_at = datetime(2021, 2, 7, 12)
+    sample_letter_notification.service = create_service(
+        service_permissions=permissions
+    )
+
+    resanitise_pdf(sample_letter_notification.id)
+
+    mock_celery.assert_called_once_with(
+        name=TaskNames.RECREATE_PDF_FOR_PRECOMPILED_LETTER,
+        kwargs={
+            'notification_id': str(sample_letter_notification.id),
+            'file_location': '2021-02-07/NOTIFY.FOO.D.2.C.20210207120000.PDF',
+            'allow_international_letters': expected_international_letters_allowed,
+        },
+        queue=QueueNames.SANITISE_LETTERS,
+    )

tests/app/v2/notifications/test_post_notifications.py

@@ -1067,21 +1067,14 @@ def test_post_notifications_saves_email_or_sms_to_queue(client, notify_db_sessio
         assert not mock_send_task.called
         assert len(Notification.query.all()) == 0
 
-@pytest.mark.parametrize("exception", [
-    botocore.exceptions.ClientError({'some': 'json'}, 'some opname'),
-    botocore.parsers.ResponseParserError('exceeded max HTTP body length'),
-])
+
 @pytest.mark.parametrize("notification_type", ("email", "sms"))
 def test_post_notifications_saves_email_or_sms_normally_if_saving_to_queue_fails(
-    client,
-    notify_db_session,
-    mocker,
-    notification_type,
-    exception
+    client, notify_db_session, mocker, notification_type
 ):
     save_task = mocker.patch(
         f"app.celery.tasks.save_api_{notification_type}.apply_async",
-        side_effect=exception,
+        side_effect=botocore.exceptions.ClientError({'some': 'json'}, 'some opname')
     )
     mock_send_task = mocker.patch(f'app.celery.provider_tasks.deliver_{notification_type}.apply_async')