Move recipient decoding into LetterMetadata class

This is for consistency with how we do it for filenames in the previous
commit and moves the decoding into the `LetterMetadata` class for
abstracting this behaviour.

Small refactor of the LetterMetadata class needed to handle None case as
recipient can be None.

app/main/views/uploads.py

@@ -1,7 +1,6 @@
 import base64
 import itertools
 import json
-import urllib
 import uuid
 from io import BytesIO
 from zipfile import BadZipFile
@@ -192,14 +191,12 @@ def _get_error_from_upload_form(form_errors):
 def format_recipient(address):
     '''
     To format the recipient we need to:
-        - decode, address is url encoded
         - remove new line characters
         - remove whitespace around the lines
         - join the address lines, separated by a comma
     '''
     if not address:
         return address
-    address = urllib.parse.unquote(address)
     stripped_address_lines_no_trailing_commas = [
         line.lstrip().rstrip(' ,')
         for line in address.splitlines() if line

app/s3_client/s3_letter_upload_client.py

@@ -45,15 +45,16 @@ def upload_letter_to_s3(
 
 
 class LetterMetadata:
-    KEYS_TO_DECODE = ["filename"]
+    KEYS_TO_DECODE = ["filename", "recipient"]
 
     def __init__(self, metadata):
         self._metadata = metadata
 
     def get(self, key, default=None):
-        if key in self.KEYS_TO_DECODE:
+        value = self._metadata.get(key, default)
-            return urllib.parse.unquote(self._metadata.get(key, default))
+        if value and key in self.KEYS_TO_DECODE:
-        return self._metadata.get(key, default)
+            value = urllib.parse.unquote(value)
+        return value
 
 
 def get_letter_pdf_and_metadata(service_id, file_id):

tests/app/main/views/test_uploads.py

@@ -1,5 +1,4 @@
 import re
-import urllib
 import uuid
 from io import BytesIO
 from unittest.mock import ANY, Mock
@@ -455,12 +454,11 @@ def test_uploaded_letter_preview(
     fake_uuid,
 ):
     mocker.patch('app.main.views.uploads.service_api_client')
-    recipient = 'Bugs Bunny\n123 Big Hole\rLooney Town'
     mocker.patch('app.main.views.uploads.get_letter_metadata', return_value=LetterMetadata({
         'filename': 'my_encoded_filename%C2%A3.pdf',
         'page_count': '1',
         'status': 'valid',
-        'recipient': urllib.parse.quote(recipient)
+        'recipient': 'Bugs Bunny%0A123 Big Hole%0DLooney Town'  # 'Bugs Bunny%0A123 Big Hole\rLooney Town' url encoded
     }))
 
     service_one['restricted'] = False
@@ -655,8 +653,12 @@ def test_send_uploaded_letter_when_metadata_states_pdf_is_invalid(mocker, servic
     mock_send = mocker.patch('app.main.views.uploads.notification_api_client.send_precompiled_letter')
     mocker.patch(
         'app.main.views.uploads.get_letter_metadata',
-        return_value=LetterMetadata({'filename': 'my_file.pdf', 'page_count': '3', 'status': 'invalid',
+        return_value=LetterMetadata(
-                      'message': 'error', 'invalid_pages': '[1]'})
+            {
+                'filename': 'my_file.pdf', 'page_count': '3', 'status': 'invalid',
+                'message': 'error', 'invalid_pages': '[1]'
+            }
+        )
     )
 
     service_one['permissions'] = ['letter', 'upload_letters']
@@ -680,7 +682,7 @@ def test_send_uploaded_letter_when_metadata_states_pdf_is_invalid(mocker, servic
     ('', ''),
 ])
 def test_format_recipient(original_address, expected_address):
-    assert format_recipient(urllib.parse.quote(original_address)) == expected_address
+    assert format_recipient(original_address) == expected_address
 
 
 @pytest.mark.parametrize('user', (