Merge pull request #2692 from alphagov/put-address-in-to-field-for-precompiled

Put address in to field for precompiled

app/celery/letters_pdf_tasks.py

@@ -255,7 +255,8 @@ def process_virus_scan_passed(self, filename):
         update_letter_pdf_status(
             reference=reference,
             status=NOTIFICATION_DELIVERED if is_test_key else NOTIFICATION_CREATED,
-            billable_units=billable_units
+            billable_units=billable_units,
+            recipient_address=sanitise_response.get("recipient_address")
         )
         scan_pdf_object.delete()
     except BotoClientError:
@@ -474,14 +475,14 @@ def process_virus_scan_error(filename):
     raise error
 
 
-def update_letter_pdf_status(reference, status, billable_units):
+def update_letter_pdf_status(reference, status, billable_units, recipient_address=None):
+
+    update_dict = {'status': status, 'billable_units': billable_units, 'updated_at': datetime.utcnow()}
+    if recipient_address:
+        update_dict['to'] = recipient_address
     return dao_update_notifications_by_reference(
         references=[reference],
-        update_dict={
+        update_dict=update_dict)[0]
-            'status': status,
-            'billable_units': billable_units,
-            'updated_at': datetime.utcnow()
-        })[0]
 
 
 def replay_letters_in_error(filename=None):

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/celery/test_letters_pdf_tasks.py

@@ -440,6 +440,7 @@ def test_process_letter_task_check_virus_scan_passed(
             endpoint,
             json={
                 "file": base64.b64encode(b"new_pdf").decode("utf-8"),
+                "recipient_address": "Bugs Bunny",
                 "validation_passed": True,
                 "message": "",
                 "invalid_pages": [],

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)}