Added the random string reference to the letter

- uses the reference field on the notifications table to store a 16char random string used to cross reference DVLA letters back to the notification
- used as letter barcode does not have space for a UUID notification id

Depends on https://github.com/alphagov/notifications-utils/pull/149

Renamed the numeric_id to notification_reference in utils and changed validation rules to match this

Note also the persist_notification method set "reference" to be "client_reference" which is confusing and they are different things, so fixed this too.

app/__init__.py

@@ -1,4 +1,6 @@
 import os
+import random
+import string
 import uuid
 
 from flask import Flask, _request_ctx_stack
@@ -204,6 +206,10 @@ def create_uuid():
     return str(uuid.uuid4())
 
 
+def create_random_identifier():
+    return ''.join(random.choice(string.ascii_letters + string.digits) for _ in range(16))
+
+
 def process_user_agent(user_agent_string):
     if user_agent_string and user_agent_string.lower().startswith("notify"):
         components = user_agent_string.split("/")

app/celery/tasks.py

@@ -10,6 +10,7 @@ from notifications_utils.template import SMSMessageTemplate, WithSubjectTemplate
 from sqlalchemy.exc import SQLAlchemyError
 from app import (
     create_uuid,
+    create_random_identifier,
     DATETIME_FORMAT,
     notify_celery,
     encryption
@@ -262,7 +263,8 @@ def persist_letter(
             created_at=created_at,
             job_id=notification['job'],
             job_row_number=notification['row_number'],
-            notification_id=notification_id
+            notification_id=notification_id,
+            reference=create_random_identifier()
         )
 
         current_app.logger.info("Letter {} created at {}".format(saved_notification.id, created_at))
@@ -311,10 +313,8 @@ def create_dvla_file_contents(job_id):
         str(LetterDVLATemplate(
             notification.template.__dict__,
             notification.personalisation,
-            # This unique id is a 7 digits requested by DVLA, not known
+            notification_reference=notification.reference,
-            # if this number needs to be sequential.
+            contact_block=notification.service.letter_contact_block
-            numeric_id=random.randint(1, int('9' * 7)),
-            contact_block=notification.service.letter_contact_block,
         ))
         for notification in dao_get_all_notifications_for_job(job_id)
     )

app/notifications/process_notifications.py

@@ -36,6 +36,7 @@ def persist_notification(template_id,
                          job_id=None,
                          job_row_number=None,
                          reference=None,
+                         client_reference=None,
                          notification_id=None,
                          simulated=False):
     # if simulated create a Notification model to return but do not persist the Notification to the dB
@@ -53,7 +54,8 @@ def persist_notification(template_id,
         created_at=created_at or datetime.utcnow(),
         job_id=job_id,
         job_row_number=job_row_number,
-        client_reference=reference
+        client_reference=client_reference,
+        reference=reference
     )
     if not simulated:
         dao_create_notification(notification)

app/v2/notifications/post_notifications.py

@@ -47,7 +47,7 @@ def post_notification(notification_type):
                                         notification_type=notification_type,
                                         api_key_id=api_user.id,
                                         key_type=api_user.key_type,
-                                        reference=form.get('reference', None),
+                                        client_reference=form.get('reference', None),
                                         simulated=simulated)
     if not simulated:
         queue_name = 'priority' if template.process_type == PRIORITY else None

tests/app/celery/test_tasks.py

@@ -907,6 +907,9 @@ def test_send_sms_does_not_send_duplicate_and_does_not_put_in_retry_queue(sample
 
 
 def test_persist_letter_saves_letter_to_database(sample_letter_job, mocker):
+
+    mocker.patch('app.celery.tasks.create_random_identifier', return_value="this-is-random-in-real-life")
+
     personalisation = {
         'addressline1': 'Foo',
         'addressline2': 'Bar',
@@ -945,6 +948,7 @@ def test_persist_letter_saves_letter_to_database(sample_letter_job, mocker):
     assert notification_db.sent_at is None
     assert notification_db.sent_by is None
     assert notification_db.personalisation == personalisation
+    assert notification_db.reference == "this-is-random-in-real-life"
 
 
 def test_should_cancel_job_if_service_is_inactive(sample_service,
@@ -1013,24 +1017,26 @@ def test_build_dvla_file_retries_if_all_notifications_are_not_created(sample_let
 
 
 def test_create_dvla_file_contents(sample_letter_template, mocker):
-    mocker.patch("app.celery.tasks.random.randint", return_value=999)
     job = create_job(template=sample_letter_template, notification_count=2)
-    create_notification(template=job.template, job=job)
+    create_notification(template=job.template, job=job, reference=1)
-    create_notification(template=job.template, job=job)
+    create_notification(template=job.template, job=job, reference=2)
     mocked_letter_template = mocker.patch("app.celery.tasks.LetterDVLATemplate")
     mocked_letter_template_instance = mocked_letter_template.return_value
     mocked_letter_template_instance.__str__.return_value = "dvla|string"
+
     create_dvla_file_contents(job.id)
+    calls = mocked_letter_template.call_args_list
     # Template
-    assert mocked_letter_template.call_args[0][0]['subject'] == 'Template subject'
+    assert calls[0][0][0]['subject'] == 'Template subject'
-    assert mocked_letter_template.call_args[0][0]['content'] == 'Dear Sir/Madam, Hello. Yours Truly, The Government.'
+    assert calls[0][0][0]['content'] == 'Dear Sir/Madam, Hello. Yours Truly, The Government.'
 
     # Personalisation
-    assert mocked_letter_template.call_args[0][1] is None
+    assert not calls[0][0][1]
 
     # Named arguments
-    assert mocked_letter_template.call_args[1]['numeric_id'] == 999
+    assert calls[1][1]['contact_block'] == 'London,\nSW1A 1AA'
-    assert mocked_letter_template.call_args[1]['contact_block'] == 'London,\nSW1A 1AA'
+    assert calls[0][1]['notification_reference'] == '1'
+    assert calls[1][1]['notification_reference'] == '2'
 
 
 @freeze_time("2017-03-23 11:09:00.061258")

tests/app/notifications/test_process_notification.py

@@ -179,7 +179,7 @@ def test_persist_notification_with_optionals(sample_job, sample_api_key, mocker)
                          created_at=created_at,
                          job_id=sample_job.id,
                          job_row_number=10,
-                         reference="ref from client",
+                         client_reference="ref from client",
                          notification_id=n_id)
     assert Notification.query.count() == 1
     assert NotificationHistory.query.count() == 1