Show recipient when about to send uploaded letter

The recipient of the letter now displays at the bottom of the page when
previewing a valid letter. The template preview `/precompiled/sanitise`
endpoint returns the address, but we format it to display on a single
line with commas between each line. We also need to convert the
recipient address to ASCII so that it can be stored as S3 metadata.
This commit is contained in:
Katie Smith
2019-11-08 09:56:59 +00:00
parent 21a59598b2
commit a542047581
5 changed files with 75 additions and 10 deletions

View File

@@ -92,14 +92,18 @@ def upload_letter(service_id):
else:
raise ex
else:
response = response.json()
recipient = response['recipient_address']
status = 'valid'
file_contents = base64.b64decode(response.json()['file'].encode())
file_contents = base64.b64decode(response['file'].encode())
upload_letter_to_s3(
file_contents,
file_location=file_location,
status=status,
page_count=page_count,
filename=original_filename)
filename=original_filename,
recipient=recipient)
return redirect(
url_for(
@@ -147,6 +151,7 @@ def uploaded_letter_preview(service_id, file_id):
status = metadata.get('status')
error_shortcode = metadata.get('message')
invalid_pages = metadata.get('invalid_pages')
recipient = metadata.get('recipient')
if invalid_pages:
invalid_pages = json.loads(invalid_pages)
@@ -180,6 +185,7 @@ def uploaded_letter_preview(service_id, file_id):
message=error_message,
error_code=error_shortcode,
form=form,
recipient=recipient,
)

View File

@@ -3,13 +3,24 @@ import json
from boto3 import resource
from flask import current_app
from notifications_utils.s3 import s3upload as utils_s3upload
from notifications_utils.sanitise_text import SanitiseASCII
def get_transient_letter_file_location(service_id, upload_id):
return 'service-{}/{}.pdf'.format(service_id, upload_id)
def upload_letter_to_s3(data, *, file_location, status, page_count, filename, message=None, invalid_pages=None):
def upload_letter_to_s3(
data,
*,
file_location,
status,
page_count,
filename,
message=None,
invalid_pages=None,
recipient=None
):
metadata = {
'status': status,
'page_count': str(page_count),
@@ -19,6 +30,8 @@ def upload_letter_to_s3(data, *, file_location, status, page_count, filename, me
metadata['message'] = message
if invalid_pages:
metadata['invalid_pages'] = json.dumps(invalid_pages)
if recipient:
metadata['recipient'] = format_recipient(recipient)
utils_s3upload(
filedata=data,
@@ -46,3 +59,20 @@ def get_letter_metadata(service_id, file_id):
s3_object = s3.Object(current_app.config['TRANSIENT_UPLOADED_LETTERS'], file_location).get()
return s3_object['Metadata']
def format_recipient(address):
'''
To format the recipient we need to:
- remove new line characters
- remove whitespace around the lines
- join the address lines, separated by a comma
- convert the string to ASCII (S3 metadata must be stored as ASCII)
'''
stripped_address_lines_no_trailing_commas = [
line.lstrip().rstrip(' ,')
for line in address.splitlines() if line
]
one_line_address = ', '.join(stripped_address_lines_no_trailing_commas)
return SanitiseASCII.encode(one_line_address)

View File

@@ -32,6 +32,10 @@
{% if status == 'valid' and current_service.live %}
<div class="js-stick-at-bottom-when-scrolling">
<p>
Recipient: {{ recipient }}
</p>
<form method="post" enctype="multipart/form-data" action="{{url_for(
'main.send_uploaded_letter',
service_id=current_service.id,

View File

@@ -55,11 +55,14 @@ def test_post_upload_letter_redirects_for_valid_file(mocker, active_user_with_pe
antivirus_mock = mocker.patch('app.main.views.uploads.antivirus_client.scan', return_value=True)
mocker.patch(
'app.main.views.uploads.sanitise_letter',
return_value=Mock(content='The sanitised content', json=lambda: {'file': 'VGhlIHNhbml0aXNlZCBjb250ZW50'})
return_value=Mock(
content='The sanitised content',
json=lambda: {'file': 'VGhlIHNhbml0aXNlZCBjb250ZW50', 'recipient_address': 'The Queen'}
)
)
mock_s3 = mocker.patch('app.main.views.uploads.upload_letter_to_s3')
mocker.patch('app.main.views.uploads.get_letter_metadata', return_value={
'filename': 'tests/test_pdf_files/one_page_pdf.pdf', 'page_count': '1', 'status': 'valid'})
'filename': 'tests/test_pdf_files/one_page_pdf.pdf', 'page_count': '1', 'status': 'valid', 'recipient': 'The Queen'})
mocker.patch('app.main.views.uploads.service_api_client.get_precompiled_template')
service_one['restricted'] = False
@@ -79,9 +82,11 @@ def test_post_upload_letter_redirects_for_valid_file(mocker, active_user_with_pe
file_location='service-{}/fake-uuid.pdf'.format(SERVICE_ONE_ID),
status='valid',
page_count=1,
filename='tests/test_pdf_files/one_page_pdf.pdf'
filename='tests/test_pdf_files/one_page_pdf.pdf',
recipient='The Queen',
)
assert 'The Queen' in page.find('div', class_='js-stick-at-bottom-when-scrolling').text
assert page.find('h1').text == 'tests/test_pdf_files/one_page_pdf.pdf'
assert not page.find(id='validation-error-message')
@@ -105,12 +110,15 @@ def test_post_upload_letter_shows_letter_preview_for_valid_file(
mocker.patch('app.main.views.uploads.antivirus_client.scan', return_value=True)
mocker.patch(
'app.main.views.uploads.sanitise_letter',
return_value=Mock(content='The sanitised content', json=lambda: {'file': 'VGhlIHNhbml0aXNlZCBjb250ZW50'})
return_value=Mock(
content='The sanitised content',
json=lambda: {'file': 'VGhlIHNhbml0aXNlZCBjb250ZW50', 'recipient_address': 'The Queen'}
)
)
mocker.patch('app.main.views.uploads.upload_letter_to_s3')
mocker.patch('app.main.views.uploads.pdf_page_count', return_value=3)
mocker.patch('app.main.views.uploads.get_letter_metadata', return_value={
'filename': 'tests/test_pdf_files/one_page_pdf.pdf', 'page_count': '3', 'status': 'valid'})
'filename': 'tests/test_pdf_files/one_page_pdf.pdf', 'page_count': '3', 'status': 'valid', 'recipient': 'The Queen'})
mocker.patch('app.main.views.uploads.service_api_client.get_precompiled_template', return_value=letter_template)
service_one['restricted'] = False
@@ -265,7 +273,7 @@ def test_post_upload_letter_shows_letter_preview_for_invalid_file(mocker, client
mocker.patch('app.main.views.uploads.upload_letter_to_s3')
mock_sanitise_response = Mock()
mock_sanitise_response.raise_for_status.side_effect = RequestException(response=Mock(status_code=400))
mock_sanitise_response.json = lambda: {"message": "template preview error"}
mock_sanitise_response.json = lambda: {"message": "template preview error", "recipient_address": "The Queen"}
mocker.patch('app.main.views.uploads.sanitise_letter', return_value=mock_sanitise_response)
mocker.patch('app.main.views.uploads.service_api_client.get_precompiled_template', return_value=letter_template)
mocker.patch('app.main.views.uploads.get_letter_metadata', return_value={
@@ -280,6 +288,7 @@ def test_post_upload_letter_shows_letter_preview_for_invalid_file(mocker, client
_follow_redirects=True,
)
assert 'The Queen' not in page.text
assert len(page.select('.letter-postage')) == 0
letter_images = page.select('main img')

View File

@@ -1,6 +1,10 @@
import pytest
from flask import current_app
from app.s3_client.s3_letter_upload_client import upload_letter_to_s3
from app.s3_client.s3_letter_upload_client import (
format_recipient,
upload_letter_to_s3,
)
def test_upload_letter_to_s3(mocker):
@@ -47,3 +51,15 @@ def test_upload_letter_to_s3_with_message_and_invalid_pages(mocker):
},
region=current_app.config['AWS_REGION']
)
@pytest.mark.parametrize('original_address,expected_address', [
('The Queen, Buckingham Palace, SW1 1AA', 'The Queen, Buckingham Palace, SW1 1AA'),
('The Queen Buckingham Palace SW1 1AA', 'The Queen Buckingham Palace SW1 1AA'),
('The Queen,\nBuckingham Palace,\r\nSW1 1AA', 'The Queen, Buckingham Palace, SW1 1AA'),
('The Queen ,,\nBuckingham Palace,\rSW1 1AA,', 'The Queen, Buckingham Palace, SW1 1AA'),
(' The Queen\n Buckingham Palace\n SW1 1AA', 'The Queen, Buckingham Palace, SW1 1AA'),
("The Queen\n Buckingham Palace\n SW1 1AA", "The 'Queen, Buckingham Palace, SW1 1AA"),
])
def test_format_recipient(original_address, expected_address):
assert format_recipient(original_address) == expected_address