mirror of
https://github.com/GSA/notifications-admin.git
synced 2026-07-17 12:29:20 -04:00
Don’t 500 when a CSV is missing rows
> When the CSV is missing the header row, we get an error and the user > will see "Sorry, we are experiencing technical difficulties..." > > We should return a better error message for the user. – https://www.pivotaltracker.com/story/show/140668615 This was caused by an attempt to access the `first_recipient` variable before it was assigned. It would only be assigned when there was at least one row in the file. Fixing this means doing two things: - defaulting `first_recipient` to be `None` before looking in the file - adding an error message for when we can’t extract any rows out of the file (which is more nuanced than the file just being completely empty) (There’s a nasty `sort` in the Jinja template because when there are no rows in the file the order of the required column headers is not deterministic.)
This commit is contained in:
@@ -249,8 +249,6 @@ def _check_messages(service_id, template_type, upload_id, letters_as_pdf=False):
|
||||
remaining_messages = (current_service['message_limit'] - sum(stat['requested'] for stat in statistics.values()))
|
||||
|
||||
contents = s3download(service_id, upload_id)
|
||||
if not contents:
|
||||
flash('There was a problem reading your upload file')
|
||||
|
||||
template = get_template(
|
||||
service_api_client.get_service_template(
|
||||
@@ -295,6 +293,7 @@ def _check_messages(service_id, template_type, upload_id, letters_as_pdf=False):
|
||||
choose_time_form = ChooseTimeForm()
|
||||
|
||||
with suppress(StopIteration):
|
||||
first_recipient = None
|
||||
template.values = next(recipients.rows)
|
||||
first_recipient = template.values.get(
|
||||
Columns.make_key(recipients.recipient_column_headers[0]),
|
||||
|
||||
@@ -35,6 +35,23 @@
|
||||
{% endcall %}
|
||||
</div>
|
||||
|
||||
{% elif not count_of_recipients %}
|
||||
|
||||
<div class="bottom-gutter">
|
||||
{% call banner_wrapper(type='dangerous') %}
|
||||
<h1 class='banner-title'>
|
||||
Your file is missing some rows
|
||||
</h1>
|
||||
<p>
|
||||
It needs at least one row of data, and {{ recipients.missing_column_headers | sort() | formatted_list(
|
||||
prefix='a column called',
|
||||
prefix_plural='columns called'
|
||||
) }}.
|
||||
</p>
|
||||
{{ skip_to_file_contents() }}
|
||||
{% endcall %}
|
||||
</div>
|
||||
|
||||
{% elif not recipients.has_recipient_columns %}
|
||||
|
||||
<div class="bottom-gutter">
|
||||
|
||||
@@ -138,7 +138,23 @@ def test_upload_csvfile_with_errors_shows_check_page_with_errors(
|
||||
'It doesn’t have a column called ‘name’. '
|
||||
'Skip to file contents'
|
||||
)
|
||||
)
|
||||
),
|
||||
(
|
||||
"+447700900986",
|
||||
(
|
||||
'Your file is missing some rows '
|
||||
'It needs at least one row of data, and columns called ‘name’ and ‘phone number’. '
|
||||
'Skip to file contents'
|
||||
)
|
||||
),
|
||||
(
|
||||
"",
|
||||
(
|
||||
'Your file is missing some rows '
|
||||
'It needs at least one row of data, and columns called ‘name’ and ‘phone number’. '
|
||||
'Skip to file contents'
|
||||
)
|
||||
),
|
||||
])
|
||||
def test_upload_csvfile_with_missing_columns_shows_error(
|
||||
logged_in_client,
|
||||
|
||||
Reference in New Issue
Block a user