Merge pull request #3742 from alphagov/fix-for-file-input-in-safari

Fix for file input bug in safari

app/__init__.py

@@ -528,6 +528,15 @@ def format_number_in_pounds_as_currency(number):
     return f"{number * 100:.0f}p"
 
 
+def format_list_items(items, format_string, *args, **kwargs):
+    """
+    Apply formatting to each item in an iterable. Returns a list.
+    Each item is made available in the format_string as the 'item' keyword argument.
+    example usage: ['png','svg','pdf']|format_list_items('{0}. {item}', [1,2,3]) -> ['1. png', '2. svg', '3. pdf']
+    """
+    return [format_string.format(*args, item=item, **kwargs) for item in items]
+
+
 @login_manager.user_loader
 def load_user(user_id):
     return User.from_id(user_id)
@@ -818,6 +827,7 @@ def add_template_filters(application):
         format_thousands,
         id_safe,
         convert_to_boolean,
+        format_list_items,
     ]:
         application.add_template_filter(fn)
 

app/main/views/send.py

@@ -193,7 +193,8 @@ def send_messages(service_id, template_id):
         template=template,
         column_headings=list(ascii_uppercase[:len(column_headings)]),
         example=[column_headings, get_example_csv_rows(template)],
-        form=form
+        form=form,
+        allowed_file_extensions=Spreadsheet.ALLOWED_FILE_EXTENSIONS
     )
 
 
@@ -709,6 +710,7 @@ def _check_messages(service_id, template_id, upload_id, preview_row, letters_as_
 def check_messages(service_id, template_id, upload_id, row_index=2):
 
     data = _check_messages(service_id, template_id, upload_id, row_index)
+    data['allowed_file_extensions'] = Spreadsheet.ALLOWED_FILE_EXTENSIONS
 
     if (
         data['recipients'].too_many_rows

app/main/views/uploads.py

@@ -313,6 +313,7 @@ def uploaded_letter_preview(service_id, file_id):
         message=error_message,
         error_code=error_shortcode,
         form=form,
+        allowed_file_extensions=Spreadsheet.ALLOWED_FILE_EXTENSIONS,
         postal_address=postal_address,
         re_upload_form=re_upload_form
     )
@@ -413,6 +414,7 @@ def upload_contact_list(service_id):
     return render_template(
         'views/uploads/contact-list/upload.html',
         form=form,
+        allowed_file_extensions=Spreadsheet.ALLOWED_FILE_EXTENSIONS,
     )
 
 
@@ -455,6 +457,7 @@ def check_contact_list(service_id, upload_id):
             original_file_name=original_file_name,
             template_type=template_type,
             form=form,
+            allowed_file_extensions=Spreadsheet.ALLOWED_FILE_EXTENSIONS
         )
 
     if recipients.too_many_rows or not len(recipients):
@@ -463,6 +466,7 @@ def check_contact_list(service_id, upload_id):
             recipients=recipients,
             original_file_name=original_file_name,
             form=form,
+            allowed_file_extensions=Spreadsheet.ALLOWED_FILE_EXTENSIONS
         )
 
     row_errors = get_errors_for_csv(recipients, template_type)
@@ -473,6 +477,7 @@ def check_contact_list(service_id, upload_id):
             original_file_name=original_file_name,
             row_errors=row_errors,
             form=form,
+            allowed_file_extensions=Spreadsheet.ALLOWED_FILE_EXTENSIONS
         )
 
     if recipients.has_errors:
@@ -481,6 +486,7 @@ def check_contact_list(service_id, upload_id):
             recipients=recipients,
             original_file_name=original_file_name,
             form=form,
+            allowed_file_extensions=Spreadsheet.ALLOWED_FILE_EXTENSIONS
         )
 
     metadata_kwargs = {

app/templates/components/file-upload.html

@@ -2,6 +2,7 @@
 
 {% macro file_upload(
   field,
+  allowed_file_extensions,
   action=None,
   button_text="Choose file",
   alternate_link=None,
@@ -25,7 +26,8 @@
       {% endif %}
     </label>
     {{ field(**{
-      'class': 'file-upload-field'
+      'class': 'file-upload-field',
+      'accept': allowed_file_extensions|format_list_items('.{item}')|join(',')|e
     }) }}
     <label class="file-upload-button" for="{{ field.name }}">
       {{ button_text }}

app/templates/views/check/column-errors.html

@@ -137,6 +137,7 @@
       {% if not request.args.from_test %}
         {{ file_upload(
           form.file,
+          allowed_file_extensions=allowed_file_extensions,
           action=url_for('.send_messages', service_id=current_service.id, template_id=template.id),
           button_text='Upload your file again'
         ) }}

app/templates/views/check/row-errors.html

@@ -43,6 +43,7 @@
     <div class="form-group">
       {{ file_upload(
         form.file,
+        allowed_file_extensions=allowed_file_extensions,
         action=url_for('.send_messages', service_id=current_service.id, template_id=template.id),
         button_text='Upload your file again'
       ) }}

app/templates/views/email-branding/manage-branding.html

@@ -24,7 +24,7 @@
       <p class="govuk-body">
         Logos should be PNG files, 108px high
       </p>
-      {{ file_upload(form.file, button_text='{} logo'.format('Update' if email_branding else 'Upload')) }}
+      {{ file_upload(form.file, allowed_file_extensions=['png'], button_text='{} logo'.format('Update' if email_branding else 'Upload')) }}
       {% call form_wrapper() %}
         <div class="form-group">
           <div style='margin-top:15px;'>{{form.name}}</div>

app/templates/views/letter-branding/manage-letter-branding.html

@@ -24,7 +24,7 @@
       <p class="govuk-body">
         Logos should be SVG files, cropped to artwork bounds and with all fonts outlined.
       </p>
-      {{ file_upload(file_upload_form.file, button_text='{} logo'.format('Update' if is_update else 'Upload')) }}
+      {{ file_upload(file_upload_form.file, allowed_file_extensions=['svg'], button_text='{} logo'.format('Update' if is_update else 'Upload')) }}
       {% call form_wrapper() %}
         <div class="form-group">
           {{ letter_branding_details_form.name(param_extensions={

app/templates/views/send.html

@@ -19,6 +19,7 @@
   <div class="page-footer bottom-gutter">
     {{file_upload(
       form.file,
+      allowed_file_extensions=allowed_file_extensions,
       button_text='Choose a file'
     )}}
   </div>

app/templates/views/uploads/choose-file.html

@@ -29,6 +29,7 @@
   <p class="govuk-body">
     {{ file_upload(
       form.file,
+      allowed_file_extensions=['pdf'],
       action=url_for('main.upload_letter', service_id=current_service.id),
       button_text='Upload your file again' if error else 'Choose file',
       show_errors=False

app/templates/views/uploads/contact-list/column-errors.html

@@ -49,6 +49,7 @@
     <div class="form-group">
       {{ file_upload(
         form.file,
+        allowed_file_extensions=allowed_file_extensions,
         action=url_for('.upload_contact_list', service_id=current_service.id),
         button_text='Upload your file again'
       ) }}

app/templates/views/uploads/contact-list/row-errors.html

@@ -44,6 +44,7 @@
     <div class="form-group">
       {{ file_upload(
         form.file,
+        allowed_file_extensions=allowed_file_extensions,
         action=url_for('.upload_contact_list', service_id=current_service.id),
         button_text='Upload your file again'
       ) }}

app/templates/views/uploads/contact-list/too-many-columns.html

@@ -58,6 +58,7 @@
     <div class="form-group">
       {{ file_upload(
         form.file,
+        allowed_file_extensions=allowed_file_extensions,
         action=url_for('.upload_contact_list', service_id=current_service.id),
         button_text='Upload your file again'
       ) }}

app/templates/views/uploads/contact-list/upload.html

@@ -35,6 +35,7 @@
 <div class="bottom-gutter">
   {{ file_upload(
     form.file,
+    allowed_file_extensions=allowed_file_extensions,
     button_text='Upload your file again' if error else 'Choose file',
     show_errors=False
   )}}

app/templates/views/uploads/preview.html

@@ -19,6 +19,7 @@
         <div class="form-group">
           {{ file_upload(
             re_upload_form.file,
+            allowed_file_extensions=['pdf'],
             action=url_for('main.upload_letter', service_id=current_service.id),
             button_text='Upload your file again'
           ) }}

app/utils.py

@@ -291,7 +291,7 @@ def id_safe(string):
 
 class Spreadsheet():
 
-    allowed_file_extensions = ['csv', 'xlsx', 'xls', 'ods', 'xlsm', 'tsv']
+    ALLOWED_FILE_EXTENSIONS = ('csv', 'xlsx', 'xls', 'ods', 'xlsm', 'tsv')
 
     def __init__(self, csv_data=None, rows=None, filename=''):
 
@@ -322,7 +322,7 @@ class Spreadsheet():
 
     @classmethod
     def can_handle(cls, filename):
-        return cls.get_extension(filename) in cls.allowed_file_extensions
+        return cls.get_extension(filename) in cls.ALLOWED_FILE_EXTENSIONS
 
     @staticmethod
     def get_extension(filename):

tests/app/main/views/test_email_branding.py

@@ -61,6 +61,7 @@ def test_edit_email_branding_shows_the_correct_branding_info(
 
     assert page.select_one('#logo-img > img')['src'].endswith('/example.png')
     assert page.select_one('#name').attrs.get('value') == 'Organisation name'
+    assert page.select_one('#file').attrs.get('accept') == '.png'
     assert page.select_one('#text').attrs.get('value') == 'Organisation text'
     assert page.select_one('#colour').attrs.get('value') == '#f00'
 
@@ -79,6 +80,7 @@ def test_create_email_branding_does_not_show_any_branding_info(
 
     assert page.select_one('#logo-img > img') is None
     assert page.select_one('#name').attrs.get('value') is None
+    assert page.select_one('#file').attrs.get('accept') == '.png'
     assert page.select_one('#text').attrs.get('value') is None
     assert page.select_one('#colour').attrs.get('value') is None
 

tests/app/main/views/test_letter_branding.py

@@ -62,6 +62,7 @@ def test_update_letter_branding_shows_the_current_letter_brand(
     assert page.find('h1').text == 'Update letter branding'
     assert page.select_one('#logo-img > img')['src'].endswith('/hm-government.svg')
     assert page.select_one('#name').attrs.get('value') == 'HM Government'
+    assert page.select_one('#file').attrs.get('accept') == '.svg'
 
 
 def test_update_letter_branding_with_new_valid_file(
@@ -312,6 +313,7 @@ def test_create_letter_branding_does_not_show_branding_info(platform_admin_clien
 
     assert page.select_one('#logo-img > img') is None
     assert page.select_one('#name').attrs.get('value') is None
+    assert page.select_one('#file').attrs.get('accept') == '.svg'
 
 
 def test_create_letter_branding_when_uploading_valid_file(

tests/app/main/views/test_send.py

@@ -308,6 +308,8 @@ def test_example_spreadsheet(
     ) == (
         '1 phone number name date'
     )
+    assert page.select_one('input[type=file]').has_attr('accept')
+    assert page.select_one('input[type=file]')['accept'] == '.csv,.xlsx,.xls,.ods,.xlsm,.tsv'
 
 
 def test_example_spreadsheet_for_letters(
@@ -505,6 +507,11 @@ def test_upload_csv_file_with_errors_shows_check_page_with_errors(
         assert 'file_uploads' not in session
 
     assert response.status_code == 200
+
+    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
+    assert page.select_one('input[type=file]').has_attr('accept')
+    assert page.select_one('input[type=file]')['accept'] == '.csv,.xlsx,.xls,.ods,.xlsm,.tsv'
+
     content = response.get_data(as_text=True)
     assert 'There’s a problem with example.csv' in content
     assert '+447700900986' in content
@@ -896,6 +903,8 @@ def test_upload_csv_file_with_missing_columns_shows_error(
     with client_request.session_transaction() as session:
         assert 'file_uploads' not in session
 
+    assert page.select_one('input[type=file]').has_attr('accept')
+    assert page.select_one('input[type=file]')['accept'] == '.csv,.xlsx,.xls,.ods,.xlsm,.tsv'
     assert normalize_spaces(page.select('.banner-dangerous')[0].text) == expected_error
 
 

tests/app/main/views/uploads/test_upload_contact_list.py

@@ -18,6 +18,7 @@ def test_upload_contact_list_page(client_request):
     assert 'action' not in page.select_one('form')
     assert page.select_one('form input')['name'] == 'file'
     assert page.select_one('form input')['type'] == 'file'
+    assert page.select_one('form input')['accept'] == '.csv,.xlsx,.xls,.ods,.xlsm,.tsv'
 
     assert normalize_spaces(page.select('.spreadsheet')[0].text) == (
         'Example A '
@@ -234,6 +235,7 @@ def test_upload_csv_file_shows_error_banner(
         service_id=SERVICE_ONE_ID,
     )
     assert page.select_one('form input')['type'] == 'file'
+    assert page.select_one('form input')['accept'] == '.csv,.xlsx,.xls,.ods,.xlsm,.tsv'
 
     assert normalize_spaces(page.select_one('thead').text) == expected_thead
     assert normalize_spaces(page.select_one('tbody').text) == expected_tbody

tests/app/main/views/uploads/test_upload_letter.py

@@ -15,6 +15,7 @@ def test_get_upload_letter(client_request):
 
     assert page.find('h1').text == 'Upload a letter'
     assert page.find('input', class_='file-upload-field')
+    assert page.find('input', class_='file-upload-field')['accept'] == '.pdf'
     assert page.select('main button[type=submit]')
     assert normalize_spaces(page.find('label', class_='file-upload-button').text) == 'Choose file'
 
@@ -213,6 +214,7 @@ def test_post_upload_letter_shows_error_when_file_is_not_a_pdf(client_request):
     assert page.find('h1').text == 'Wrong file type'
     assert page.find('div', class_='banner-dangerous').find('p').text == 'Save your letter as a PDF and try again.'
     assert normalize_spaces(page.find('label', class_='file-upload-button').text) == 'Upload your file again'
+    assert page.find('input', type='file')['accept'] == '.pdf'
 
 
 def test_post_upload_letter_shows_error_when_no_file_uploaded(client_request):
@@ -346,6 +348,7 @@ def test_post_upload_letter_shows_letter_preview_for_invalid_file(mocker, client
 
     assert page.find("a", {"class": "govuk-back-link"})["href"] == "/services/{}/upload-letter".format(SERVICE_ONE_ID)
     assert page.find("label", {"class": "file-upload-button"})
+    assert page.find("input", {"type": "file"})["accept"] == '.pdf'
 
     letter_images = page.select('main img')
     assert len(letter_images) == 1