From 92549fd2d6c1fcf87d3eed904570f018f0681a8e Mon Sep 17 00:00:00 2001 From: Ben Thorner Date: Fri, 3 Dec 2021 18:14:02 +0000 Subject: [PATCH] Show flash instead of inline upload errors This has several advantages: - It gives us more room to explain the error and actions. This will be useful for upcoming work we want to do, which will add yet more validations for CSV uploads. - We already use a flash to show certain kinds of errors on these pages (just above). This is more consistent. - It's potentially more accessible. Previously the error and the button text used to be read out as a single sentence. Now the page reloads and reads the flash error alone. In theory we should show an error in both places, but this can be confusing on pages where there's only a single form control, and especially if the error is long. --- app/main/views/send.py | 5 +++++ app/main/views/uploads.py | 5 +++++ app/templates/views/send.html | 3 ++- app/templates/views/uploads/contact-list/upload.html | 1 + tests/app/main/views/uploads/test_upload_contact_list.py | 3 +-- 5 files changed, 14 insertions(+), 3 deletions(-) diff --git a/app/main/views/send.py b/app/main/views/send.py index dcf707e8a..2bbc7947d 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -175,6 +175,11 @@ def send_messages(service_id, template_id): ).format( form.file.data.filename )) + elif form.errors: + # just show the first error, as we don't expect the form to have more + # than one, since it only has one field + first_field_errors = list(form.errors.values())[0] + flash(first_field_errors[0]) column_headings = get_spreadsheet_column_headings_from_template(template) diff --git a/app/main/views/uploads.py b/app/main/views/uploads.py index 87949e92f..f8f7fc955 100644 --- a/app/main/views/uploads.py +++ b/app/main/views/uploads.py @@ -417,6 +417,11 @@ def upload_contact_list(service_id): ).format( form.file.data.filename )) + elif form.errors: + # just show the first error, as we don't expect the form to have more + # than one, since it only has one field + first_field_errors = list(form.errors.values())[0] + flash(first_field_errors[0]) return render_template( 'views/uploads/contact-list/upload.html', diff --git a/app/templates/views/send.html b/app/templates/views/send.html index fa39b13f4..91a38674b 100644 --- a/app/templates/views/send.html +++ b/app/templates/views/send.html @@ -22,7 +22,8 @@ {{file_upload( form.file, allowed_file_extensions=allowed_file_extensions, - button_text='Choose a file' + button_text='Choose a file', + show_errors=False )}} diff --git a/app/templates/views/uploads/contact-list/upload.html b/app/templates/views/uploads/contact-list/upload.html index 4cf7064f2..ad145fa3a 100644 --- a/app/templates/views/uploads/contact-list/upload.html +++ b/app/templates/views/uploads/contact-list/upload.html @@ -41,6 +41,7 @@ form.file, allowed_file_extensions=allowed_file_extensions, button_text='Choose file', + show_errors=False, )}} diff --git a/tests/app/main/views/uploads/test_upload_contact_list.py b/tests/app/main/views/uploads/test_upload_contact_list.py index a105ff7bf..9d79f6fa5 100644 --- a/tests/app/main/views/uploads/test_upload_contact_list.py +++ b/tests/app/main/views/uploads/test_upload_contact_list.py @@ -279,7 +279,6 @@ def test_upload_csv_file_shows_error_banner_for_too_many_rows( def test_upload_csv_shows_error_with_invalid_extension( client_request, ): - page = client_request.post( 'main.upload_contact_list', service_id=SERVICE_ONE_ID, @@ -287,7 +286,7 @@ def test_upload_csv_shows_error_with_invalid_extension( _follow_redirects=True, ) - assert normalize_spaces(page.select_one('.file-upload-label .error-message').text) == ( + assert normalize_spaces(page.select_one('.banner-dangerous').text) == ( "invalid.txt is not a spreadsheet that Notify can read" )