From 3a00c1939044ae4b62844b5a91e1ec4995061f2b Mon Sep 17 00:00:00 2001
From: Pea Tyczynska <pea.tyczynska@digital.cabinet-office.gov.uk>
Date: Tue, 5 May 2020 11:17:22 +0100
Subject: [PATCH] Polish and test the small task that updates billable units
 for letter

---
 app/celery/letters_pdf_tasks.py            |  9 ++++---
 app/config.py                              |  2 +-
 tests/app/celery/test_letters_pdf_tasks.py | 29 +++++++++++++++++++---
 3 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/app/celery/letters_pdf_tasks.py b/app/celery/letters_pdf_tasks.py
index 2a7f14fcc..37bb0c5d5 100644
--- a/app/celery/letters_pdf_tasks.py
+++ b/app/celery/letters_pdf_tasks.py
@@ -76,7 +76,7 @@ def create_letters_pdf(self, notification_id):
         encrypted_data = encryption.encrypt(letter_data)
 
         notify_celery.send_task(
-            name=TaskNames.CREATE_LETTER_PDF,
+            name=TaskNames.CREATE_PDF_FOR_TEMPLATED_LETTER,
             args=(encrypted_data,),
             queue=QueueNames.SANITISE_LETTERS
         )
@@ -105,9 +105,10 @@ def update_billable_units_for_letter(self, notification_id, page_count):
         notification.billable_units = billable_units
         dao_update_notification(notification)
 
-    current_app.logger.info(
-        'Letter notification reference {reference}: billable units set to {billable_units}'.format(
-            reference=str(notification.reference), billable_units=billable_units))
+        current_app.logger.info(
+            f"Letter notification id: {notification_id} reference {notification.reference}: "
+            f"billable units set to {billable_units}"
+        )
 
 
 @notify_celery.task(name='collate-letter-pdfs-to-be-sent')
diff --git a/app/config.py b/app/config.py
index 52de6b46c..eea49f4ff 100644
--- a/app/config.py
+++ b/app/config.py
@@ -59,7 +59,7 @@ class TaskNames(object):
     ZIP_AND_SEND_LETTER_PDFS = 'zip-and-send-letter-pdfs'
     SCAN_FILE = 'scan-file'
     SANITISE_LETTER = 'sanitise-and-upload-letter'
-    CREATE_LETTER_PDF = 'create-letter-pdf'
+    CREATE_PDF_FOR_TEMPLATED_LETTER = 'create-pdf-for-templated-letter'
 
 
 class Config(object):
diff --git a/tests/app/celery/test_letters_pdf_tasks.py b/tests/app/celery/test_letters_pdf_tasks.py
index e066f9e18..dd8d9cbdf 100644
--- a/tests/app/celery/test_letters_pdf_tasks.py
+++ b/tests/app/celery/test_letters_pdf_tasks.py
@@ -56,7 +56,7 @@ def test_should_have_decorated_tasks_functions():
     assert process_sanitised_letter.__wrapped__.__name__ == 'process_sanitised_letter'
 
 
-def test_create_letters_pdf(mocker, sample_letter_notification):
+def test_create_letters_pdf_happy_path(mocker, sample_letter_notification):
     mock_celery = mocker.patch('app.celery.letters_pdf_tasks.notify_celery.send_task')
     mocker.patch('app.celery.letters_pdf_tasks.get_letter_pdf_filename', return_value='LETTER.PDF')
     create_letters_pdf(sample_letter_notification.id)
@@ -148,10 +148,31 @@ def test_create_letters_gets_the_right_logo_when_service_has_letter_branding_log
     )
 
 
-def test_update_billable_units_for_letter(mocker, sample_letter_notification):
+@pytest.mark.parametrize('number_of_pages, expected_billable_units', [(2, 1), (3, 2), (10, 5)])
+def test_update_billable_units_for_letter(mocker, sample_letter_notification, number_of_pages, expected_billable_units):
+    sample_letter_notification.billable_units = 0
+    mock_logger = mocker.patch('app.celery.tasks.current_app.logger.info')
+
+    update_billable_units_for_letter(sample_letter_notification.id, number_of_pages)
+
+    notification = Notification.query.filter(Notification.reference == sample_letter_notification.reference).one()
+    assert notification.billable_units == expected_billable_units
+    mock_logger.assert_called_once_with(
+        f"Letter notification id: {sample_letter_notification.id} reference {sample_letter_notification.reference}:"
+        f" billable units set to {expected_billable_units}"
+    )
+
+
+def test_update_billable_units_for_letter_doesnt_update_if_sent_with_test_key(mocker, sample_letter_notification):
+    sample_letter_notification.billable_units = 0
+    sample_letter_notification.key_type = KEY_TYPE_TEST
+    mock_logger = mocker.patch('app.celery.tasks.current_app.logger.info')
+
     update_billable_units_for_letter(sample_letter_notification.id, 2)
-    noti = Notification.query.filter(Notification.reference == sample_letter_notification.reference).one()
-    assert noti.billable_units == 1
+
+    notification = Notification.query.filter(Notification.reference == sample_letter_notification.reference).one()
+    assert notification.billable_units == 0
+    mock_logger.assert_not_called()
 
 
 @freeze_time('2020-02-17 18:00:00')