From b00308d1223e0e7f4e40c7a290a0a260550d45c8 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 20 Sep 2018 14:47:24 +0100 Subject: [PATCH] Removed an update statement to notifications. It's a small change, but we should remove any db operations that are not necessary. --- .../process_letter_notifications.py | 5 +++-- app/notifications/process_notifications.py | 4 +++- app/v2/notifications/post_notifications.py | 11 +++++------ .../test_process_letter_notifications.py | 15 +++++++++++++++ .../test_process_notification.py | 19 +++++++++++++++++++ 5 files changed, 45 insertions(+), 9 deletions(-) diff --git a/app/notifications/process_letter_notifications.py b/app/notifications/process_letter_notifications.py index 4c3efb752..984ad257e 100644 --- a/app/notifications/process_letter_notifications.py +++ b/app/notifications/process_letter_notifications.py @@ -3,7 +3,7 @@ from app.models import LETTER_TYPE from app.notifications.process_notifications import persist_notification -def create_letter_notification(letter_data, template, api_key, status, reply_to_text=None): +def create_letter_notification(letter_data, template, api_key, status, reply_to_text=None, billable_units=None): notification = persist_notification( template_id=template.id, template_version=template.version, @@ -19,6 +19,7 @@ def create_letter_notification(letter_data, template, api_key, status, reply_to_ reference=create_random_identifier(), client_reference=letter_data.get('reference'), status=status, - reply_to_text=reply_to_text + reply_to_text=reply_to_text, + billable_units=billable_units ) return notification diff --git a/app/notifications/process_notifications.py b/app/notifications/process_notifications.py index 100790f83..62de55140 100644 --- a/app/notifications/process_notifications.py +++ b/app/notifications/process_notifications.py @@ -70,7 +70,8 @@ def persist_notification( simulated=False, created_by_id=None, status=NOTIFICATION_CREATED, - reply_to_text=None + reply_to_text=None, + billable_units=None ): notification_created_at = created_at or datetime.utcnow() if not notification_id: @@ -94,6 +95,7 @@ def persist_notification( created_by_id=created_by_id, status=status, reply_to_text=reply_to_text, + billable_units=billable_units ) if notification_type == SMS_TYPE: diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index 8ab4cd76b..bbaef8385 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -11,7 +11,7 @@ from notifications_utils.recipients import try_validate_and_format_phone_number from app import api_user, authenticated_service, notify_celery, document_download_client from app.clients.document_download import DocumentDownloadError from app.config import QueueNames, TaskNames -from app.dao.notifications_dao import dao_update_notification, update_notification_status_by_reference +from app.dao.notifications_dao import update_notification_status_by_reference from app.dao.templates_dao import dao_create_template from app.dao.users_dao import get_user_by_id from app.letters.utils import upload_letter_pdf @@ -300,17 +300,16 @@ def process_precompiled_letter_notifications(*, letter_data, api_key, template, current_app.logger.exception(msg='Invalid PDF received') raise BadRequestError(message='Letter content is not a valid PDF', status_code=400) + pages_per_sheet = 2 + billable_units = math.ceil(pages / pages_per_sheet) notification = create_letter_notification(letter_data=letter_data, template=template, api_key=api_key, status=status, - reply_to_text=reply_to_text) + reply_to_text=reply_to_text, + billable_units=billable_units) 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)) diff --git a/tests/app/notifications/test_process_letter_notifications.py b/tests/app/notifications/test_process_letter_notifications.py index a607bf68b..af82d78d7 100644 --- a/tests/app/notifications/test_process_letter_notifications.py +++ b/tests/app/notifications/test_process_letter_notifications.py @@ -40,3 +40,18 @@ def test_create_letter_notification_sets_reference(sample_letter_template, sampl notification = create_letter_notification(data, sample_letter_template, sample_api_key, NOTIFICATION_CREATED) assert notification.client_reference == 'foo' + + +def test_create_letter_notification_sets_billable_units(sample_letter_template, sample_api_key): + data = { + 'personalisation': { + 'address_line_1': 'The Queen', + 'address_line_2': 'Buckingham Palace', + 'postcode': 'SW1 1AA', + }, + } + + notification = create_letter_notification(data, sample_letter_template, sample_api_key, NOTIFICATION_CREATED, + billable_units=3) + + assert notification.billable_units == 3 diff --git a/tests/app/notifications/test_process_notification.py b/tests/app/notifications/test_process_notification.py index cc3df6226..51ca2e94c 100644 --- a/tests/app/notifications/test_process_notification.py +++ b/tests/app/notifications/test_process_notification.py @@ -500,3 +500,22 @@ def test_persist_notification_increments_and_expires_redis_template_usage( 'service-{}-template-usage-{}'.format(str(sample_template.service_id), day_in_key), current_app.config['EXPIRE_CACHE_EIGHT_DAYS'] ) + + +def test_persist_notification_with_billable_units_stores_correct_info( + sample_template, +): + persist_notification( + template_id=sample_template.id, + template_version=sample_template.version, + recipient="123 Main Street", + service=sample_template.service, + personalisation=None, + notification_type='letter', + api_key_id=None, + key_type="normal", + billable_units=3 + ) + persisted_notification = Notification.query.all()[0] + + assert persisted_notification.billable_units == 3