Save recipient address in the "to" field of a notification

When a precompiled letter is sent via the admin app, we now pass in the address which can be set in the Notifications.to field.
Once a precompiled letters sent by the API has passed validation we can set the address in Notifications.to field.

The celery tasks to validate precompiled letters sent by the API will be done in another PR.

app/celery/letters_pdf_tasks.py

@@ -309,7 +309,6 @@ def process_sanitised_letter(
     validation_status,
     filename,
     notification_id,
-    recipient_address=None
 ):
     current_app.logger.info('Processing sanitised letter with id {}'.format(notification_id))
     notification = get_notification_by_id(notification_id, _raise=True)

app/service/send_notification.py

@@ -1,3 +1,5 @@
+import urllib
+
 from flask import current_app
 from notifications_utils.s3 import S3ObjectNotFound, s3download as utils_s3download
 from sqlalchemy.orm.exc import NoResultFound
@@ -141,7 +143,7 @@ def send_pdf_letter_notification(service_id, post_data):
     check_service_over_daily_message_limit(KEY_TYPE_NORMAL, service)
     validate_created_by(service, post_data['created_by'])
     validate_and_format_recipient(
-        send_to=post_data['filename'],
+        send_to=post_data['recipient_address'],
         key_type=KEY_TYPE_NORMAL,
         service=service,
         notification_type=LETTER_TYPE,
@@ -172,7 +174,7 @@ def send_pdf_letter_notification(service_id, post_data):
         template_id=template.id,
         template_version=template.version,
         template_postage=template.postage,
-        recipient=post_data['filename'],
+        recipient=urllib.parse.unquote(post_data['recipient_address']),
         service=service,
         personalisation=personalisation,
         notification_type=LETTER_TYPE,

app/service/send_pdf_letter_schema.py

@@ -8,6 +8,7 @@ send_pdf_letter_request = {
         "filename": {"type": "string"},
         "created_by": {"type": "string"},
         "file_id": {"type": "string"},
+        "recipient_address": {"type": "string"}
     },
-    "required": ["postage", "filename", "created_by", "file_id"]
+    "required": ["postage", "filename", "created_by", "file_id", "recipient_address"]
 }

app/v2/notifications/post_notifications.py

@@ -75,8 +75,10 @@ def post_precompiled_letter_notification():
 
     template = get_precompiled_letter_template(authenticated_service.id)
 
+    # For precompiled letters the to field will be set to Provided as PDF until the validation passes,
+    # then the address of the letter will be set as the to field
     form['personalisation'] = {
-        'address_line_1': form['reference']
+        'address_line_1': 'Provided as PDF'
     }
 
     reply_to = get_reply_to_text(LETTER_TYPE, form, template)

tests/app/service/test_rest.py

@@ -2282,7 +2282,8 @@ def test_create_pdf_letter(mocker, sample_service_full_permissions, client, fake
         'filename': 'valid.pdf',
         'created_by': str(user.id),
         'file_id': fake_uuid,
-        'postage': 'second'
+        'postage': 'second',
+        'recipient_address': 'Bugs%20Bunny%0A123%20Main%20Street%0ALooney%20Town'
     })
 
     response = client.post(
@@ -2303,11 +2304,13 @@ def test_create_pdf_letter(mocker, sample_service_full_permissions, client, fake
             {'error': 'ValidationError', 'message': 'postage is a required property'},
             {'error': 'ValidationError', 'message': 'filename is a required property'},
             {'error': 'ValidationError', 'message': 'created_by is a required property'},
-            {'error': 'ValidationError', 'message': 'file_id is a required property'}
+            {'error': 'ValidationError', 'message': 'file_id is a required property'},
+            {'error': 'ValidationError', 'message': 'recipient_address is a required property'}
         ]
     ),
     (
-        {"postage": "third", "filename": "string", "created_by": "string", "file_id": "string"},
+        {"postage": "third", "filename": "string", "created_by": "string", "file_id": "string",
+         "recipient_address": "Some Address"},
         [
             {'error': 'ValidationError', 'message': 'postage invalid. It must be either first or second.'}
         ]

tests/app/service/test_send_pdf_letter_notification.py

@@ -22,7 +22,8 @@ def test_send_pdf_letter_notification_raises_error_if_service_does_not_have_perm
     permissions,
 ):
     service = create_service(service_permissions=permissions)
-    post_data = {'filename': 'valid.pdf', 'created_by': fake_uuid, 'file_id': fake_uuid, 'postage': 'first'}
+    post_data = {'filename': 'valid.pdf', 'created_by': fake_uuid, 'file_id': fake_uuid, 'postage': 'first',
+                 'recipient_address': 'Bugs%20Bunny%0A123%20Main%20Street%0ALooney%20Town'}
 
     with pytest.raises(BadRequestError):
         send_pdf_letter_notification(service.id, post_data)
@@ -36,7 +37,8 @@ def test_send_pdf_letter_notification_raises_error_if_service_is_over_daily_mess
     mocker.patch(
         'app.service.send_notification.check_service_over_daily_message_limit',
         side_effect=TooManyRequestsError(10))
-    post_data = {'filename': 'valid.pdf', 'created_by': fake_uuid, 'file_id': fake_uuid, 'postage': 'first'}
+    post_data = {'filename': 'valid.pdf', 'created_by': fake_uuid, 'file_id': fake_uuid, 'postage': 'first',
+                 'recipient_address': 'Bugs%20Bunny%0A123%20Main%20Street%0ALooney%20Town'}
 
     with pytest.raises(TooManyRequestsError):
         send_pdf_letter_notification(sample_service_full_permissions.id, post_data)
@@ -45,7 +47,8 @@ def test_send_pdf_letter_notification_raises_error_if_service_is_over_daily_mess
 def test_send_pdf_letter_notification_validates_created_by(
     sample_service_full_permissions, fake_uuid, sample_user
 ):
-    post_data = {'filename': 'valid.pdf', 'created_by': sample_user.id, 'file_id': fake_uuid, 'postage': 'first'}
+    post_data = {'filename': 'valid.pdf', 'created_by': sample_user.id, 'file_id': fake_uuid, 'postage': 'first',
+                 'recipient_address': 'Bugs%20Bunny%0A123%20Main%20Street%0ALooney%20Town'}
 
     with pytest.raises(BadRequestError):
         send_pdf_letter_notification(sample_service_full_permissions.id, post_data)
@@ -58,7 +61,8 @@ def test_send_pdf_letter_notification_raises_error_if_service_in_trial_mode(
 ):
     sample_service_full_permissions.restricted = True
     user = sample_service_full_permissions.users[0]
-    post_data = {'filename': 'valid.pdf', 'created_by': user.id, 'file_id': fake_uuid}
+    post_data = {'filename': 'valid.pdf', 'created_by': user.id, 'file_id': fake_uuid,
+                 'recipient_address': 'Bugs%20Bunny%0A123%20Main%20Street%0ALooney%20Town'}
 
     with pytest.raises(BadRequestError) as e:
         send_pdf_letter_notification(sample_service_full_permissions.id, post_data)
@@ -72,7 +76,8 @@ def test_send_pdf_letter_notification_raises_error_when_pdf_is_not_in_transient_
     notify_user,
 ):
     user = sample_service_full_permissions.users[0]
-    post_data = {'filename': 'valid.pdf', 'created_by': user.id, 'file_id': fake_uuid, 'postage': 'first'}
+    post_data = {'filename': 'valid.pdf', 'created_by': user.id, 'file_id': fake_uuid, 'postage': 'first',
+                 'recipient_address': 'Bugs%20Bunny%0A123%20Main%20Street%0ALooney%20Town'}
     mocker.patch('app.service.send_notification.utils_s3download', side_effect=S3ObjectNotFound({}, ''))
 
     with pytest.raises(S3ObjectNotFound):
@@ -88,7 +93,8 @@ def test_send_pdf_letter_notification_creates_notification_and_moves_letter(
     user = sample_service_full_permissions.users[0]
     filename = 'valid.pdf'
     file_id = uuid.uuid4()
-    post_data = {'filename': filename, 'created_by': user.id, 'file_id': file_id, 'postage': 'second'}
+    post_data = {'filename': filename, 'created_by': user.id, 'file_id': file_id, 'postage': 'second',
+                 'recipient_address': 'Bugs%20Bunny%0A123%20Main%20Street%0ALooney%20Town'}
 
     mocker.patch('app.service.send_notification.utils_s3download')
     mocker.patch('app.service.send_notification.get_page_count', return_value=1)
@@ -105,7 +111,8 @@ def test_send_pdf_letter_notification_creates_notification_and_moves_letter(
     assert notification.postage == 'second'
     assert notification.notification_type == LETTER_TYPE
     assert notification.billable_units == 1
-    assert notification.to == filename
+    assert notification.to == "Bugs Bunny\n123 Main Street\nLooney Town"
+
     assert notification.service_id == sample_service_full_permissions.id
 
     assert result == {'id': str(notification.id)}