Merge pull request #1925 from alphagov/refactored-csv-processing

Bring in refactored CSV processing
This commit is contained in:
Chris Hill-Scott
2018-03-09 15:16:37 +00:00
committed by GitHub
8 changed files with 79 additions and 53 deletions

View File

@@ -528,29 +528,23 @@ def _check_messages(service_id, template_type, upload_id, preview_row, letters_a
back_link = url_for('.send_messages', service_id=service_id, template_id=template.id) back_link = url_for('.send_messages', service_id=service_id, template_id=template.id)
choose_time_form = ChooseTimeForm() choose_time_form = ChooseTimeForm()
count_of_recipients = len(list(recipients.rows))
if preview_row < 2: if preview_row < 2:
abort(404) abort(404)
if preview_row < count_of_recipients + 2: if preview_row < len(recipients) + 2:
template.values = recipients[preview_row - 2] template.values = recipients[preview_row - 2].recipient_and_personalisation
elif preview_row > 2: elif preview_row > 2:
abort(404) abort(404)
session['upload_data']['notification_count'] = count_of_recipients session['upload_data']['notification_count'] = len(recipients)
session['upload_data']['valid'] = not recipients.has_errors session['upload_data']['valid'] = not recipients.has_errors
return dict( return dict(
recipients=recipients, recipients=recipients,
template=template, template=template,
errors=recipients.has_errors, errors=recipients.has_errors,
row_errors=get_errors_for_csv(recipients, template.template_type), row_errors=get_errors_for_csv(recipients, template.template_type),
count_of_recipients=count_of_recipients, count_of_recipients=len(recipients),
count_of_displayed_recipients=( count_of_displayed_recipients=len(list(recipients.displayed_rows)),
len(list(recipients.initial_annotated_rows_with_errors))
if any(recipients.rows_with_errors) and not recipients.missing_column_headers else
len(list(recipients.initial_annotated_rows))
),
original_file_name=session['upload_data'].get('original_file_name'), original_file_name=session['upload_data'].get('original_file_name'),
upload_id=upload_id, upload_id=upload_id,
form=CsvUploadForm(), form=CsvUploadForm(),

View File

@@ -83,7 +83,9 @@
{% if text is iterable and text is not string %} {% if text is iterable and text is not string %}
<ul class="list list-bullet"> <ul class="list list-bullet">
{% for item in text %} {% for item in text %}
<li>{{ item }}</li> {% if item %}
<li>{{ item }}</li>
{% endif %}
{% endfor %} {% endfor %}
</ul> </ul>
{% else %} {% else %}

View File

@@ -149,7 +149,7 @@
<div class="fullscreen-content" data-module="fullscreen-table"> <div class="fullscreen-content" data-module="fullscreen-table">
{% call(item, row_number) list_table( {% call(item, row_number) list_table(
recipients.initial_annotated_rows_with_errors if row_errors and not recipients.missing_column_headers else recipients.initial_annotated_rows, recipients.displayed_rows,
caption=original_file_name, caption=original_file_name,
caption_visible=False, caption_visible=False,
field_headings=[ field_headings=[
@@ -162,21 +162,21 @@
</span> </span>
{% endcall %} {% endcall %}
{% for column in column_headers %} {% for column in column_headers %}
{% if item['columns'][column].error and not recipients.missing_column_headers %} {% if item[column].error and not recipients.missing_column_headers %}
{% call field() %} {% call field() %}
<span> <span>
<span class="table-field-error-label">{{ item['columns'][column].error }}</span> <span class="table-field-error-label">{{ item[column].error }}</span>
{{ item['columns'][column].data if item['columns'][column].data != None }} {{ item[column].data if item[column].data != None }}
</span> </span>
{% endcall %} {% endcall %}
{% elif item['columns'][column].ignore %} {% elif item[column].ignore %}
{{ text_field(item['columns'][column].data or '', status='default') }} {{ text_field(item[column].data or '', status='default') }}
{% else %} {% else %}
{{ text_field(item['columns'][column].data or '') }} {{ text_field(item[column].data or '') }}
{% endif %} {% endif %}
{% endfor %} {% endfor %}
{% if item['columns'].get(None) %} {% if item[None].data %}
{% for column in item['columns'][None].data %} {% for column in item[None].data %}
{{ text_field(column, status='default') }} {{ text_field(column, status='default') }}
{% endfor %} {% endfor %}
{% endif %} {% endif %}

View File

@@ -51,7 +51,7 @@
<div class="fullscreen-content" data-module="fullscreen-table"> <div class="fullscreen-content" data-module="fullscreen-table">
{% call(item, row_number) list_table( {% call(item, row_number) list_table(
recipients.initial_annotated_rows_with_errors if row_errors and not recipients.missing_column_headers else recipients.initial_annotated_rows, recipients.displayed_rows,
caption=original_file_name, caption=original_file_name,
caption_visible=False, caption_visible=False,
field_headings=[ field_headings=[
@@ -68,14 +68,14 @@
</span> </span>
{% endcall %} {% endcall %}
{% for column in recipients.column_headers %} {% for column in recipients.column_headers %}
{% if item['columns'][column].ignore %} {% if item[column].ignore %}
{{ text_field(item['columns'][column].data or '', status='default') }} {{ text_field(item[column].data or '', status='default') }}
{% else %} {% else %}
{{ text_field(item['columns'][column].data or '') }} {{ text_field(item[column].data or '') }}
{% endif %} {% endif %}
{% endfor %} {% endfor %}
{% if item['columns'].get(None) %} {% if item[None].data %}
{% for column in item['columns'][None].data %} {% for column in item[None].data %}
{{ text_field(column, status='default') }} {{ text_field(column, status='default') }}
{% endfor %} {% endfor %}
{% endif %} {% endif %}

View File

@@ -54,7 +54,7 @@
<div class="fullscreen-content" data-module="fullscreen-table"> <div class="fullscreen-content" data-module="fullscreen-table">
{% call(item, row_number) list_table( {% call(item, row_number) list_table(
recipients.initial_annotated_rows_with_errors if row_errors and not recipients.missing_column_headers else recipients.initial_annotated_rows, recipients.displayed_rows,
caption=original_file_name, caption=original_file_name,
caption_visible=False, caption_visible=False,
field_headings=[ field_headings=[
@@ -62,26 +62,26 @@
] + recipients.column_headers ] + recipients.column_headers
) %} ) %}
{% call index_field() %} {% call index_field() %}
<span class="{% if item.index in recipients.rows_with_errors %}table-field-error{% endif %}"> <span class="{% if item.has_errors %}table-field-error{% endif %}">
{{ item.index + 2 }} {{ item.index + 2 }}
</span> </span>
{% endcall %} {% endcall %}
{% for column in recipients.column_headers %} {% for column in recipients.column_headers %}
{% if item['columns'][column].error and not recipients.missing_column_headers %} {% if item[column].error and not recipients.missing_column_headers %}
{% call field() %} {% call field() %}
<span> <span>
<span class="table-field-error-label">{{ item['columns'][column].error }}</span> <span class="table-field-error-label">{{ item[column].error }}</span>
{{ item['columns'][column].data if item['columns'][column].data != None }} {{ item[column].data if item[column].data != None }}
</span> </span>
{% endcall %} {% endcall %}
{% elif item['columns'][column].ignore %} {% elif item[column].ignore %}
{{ text_field(item['columns'][column].data or '', status='default') }} {{ text_field(item[column].data or '', status='default') }}
{% else %} {% else %}
{{ text_field(item['columns'][column].data or '') }} {{ text_field(item[column].data or '') }}
{% endif %} {% endif %}
{% endfor %} {% endfor %}
{% if item['columns'].get(None) %} {% if item[None].data %}
{% for column in item['columns'][None].data %} {% for column in item[None].data %}
{{ text_field(column, status='default') }} {{ text_field(column, status='default') }}
{% endfor %} {% endfor %}
{% endif %} {% endif %}

View File

@@ -102,7 +102,7 @@ def get_errors_for_csv(recipients, template_type):
errors = [] errors = []
if recipients.rows_with_bad_recipients: if any(recipients.rows_with_bad_recipients):
number_of_bad_recipients = len(list(recipients.rows_with_bad_recipients)) number_of_bad_recipients = len(list(recipients.rows_with_bad_recipients))
if 'sms' == template_type: if 'sms' == template_type:
if 1 == number_of_bad_recipients: if 1 == number_of_bad_recipients:
@@ -120,7 +120,7 @@ def get_errors_for_csv(recipients, template_type):
else: else:
errors.append("fix {} addresses".format(number_of_bad_recipients)) errors.append("fix {} addresses".format(number_of_bad_recipients))
if recipients.rows_with_missing_data: if any(recipients.rows_with_missing_data):
number_of_rows_with_missing_data = len(list(recipients.rows_with_missing_data)) number_of_rows_with_missing_data = len(list(recipients.rows_with_missing_data))
if 1 == number_of_rows_with_missing_data: if 1 == number_of_rows_with_missing_data:
errors.append("enter missing data in 1 row") errors.append("enter missing data in 1 row")
@@ -156,7 +156,7 @@ def generate_notifications_csv(**kwargs):
values = [ values = [
notification['row_number'], notification['row_number'],
] + [ ] + [
original_upload[notification['row_number'] - 1].get(header) original_upload[notification['row_number'] - 1].get(header).data
for header in original_column_headers for header in original_column_headers
] + [ ] + [
notification['template_name'], notification['template_name'],

View File

@@ -18,4 +18,4 @@ notifications-python-client==4.7.2
# PaaS # PaaS
awscli-cwlogs>=1.4,<1.5 awscli-cwlogs>=1.4,<1.5
git+https://github.com/alphagov/notifications-utils.git@24.0.0#egg=notifications-utils==24.0.0 git+https://github.com/alphagov/notifications-utils.git@25.0.0#egg=notifications-utils==25.0.0

View File

@@ -585,7 +585,7 @@ def test_upload_valid_csv_shows_preview_and_table(
phone number,name,thing,thing,thing phone number,name,thing,thing,thing
07700900001, A, foo, foo, foo 07700900001, A, foo, foo, foo
07700900002, B, foo, foo, foo 07700900002, B, foo, foo, foo
07700900003, C, foo, foo, foo 07700900003, C, foo, foo,
""") """)
page = client_request.get( page = client_request.get(
@@ -609,20 +609,50 @@ def test_upload_valid_csv_shows_preview_and_table(
else: else:
assert not page.select_one('.table-field-index').select_one('a') assert not page.select_one('.table-field-index').select_one('a')
for index, cell in enumerate([ for row_index, row in enumerate([
'<td class="table-field-center-aligned "> <div class=""> 07700900001 </div> </td>',
'<td class="table-field-center-aligned "> <div class=""> A </div> </td>',
( (
'<td class="table-field-center-aligned "> ' '<td class="table-field-center-aligned "> <div class=""> 07700900001 </div> </td>',
'<div class="table-field-status-default"> ' '<td class="table-field-center-aligned "> <div class=""> A </div> </td>',
'<ul class="list list-bullet"> ' (
'<li>foo</li> <li>foo</li> <li>foo</li> ' '<td class="table-field-center-aligned "> '
'</ul> ' '<div class="table-field-status-default"> '
'</div> ' '<ul class="list list-bullet"> '
'</td>' '<li>foo</li> <li>foo</li> <li>foo</li> '
'</ul> '
'</div> '
'</td>'
)
),
(
'<td class="table-field-center-aligned "> <div class=""> 07700900002 </div> </td>',
'<td class="table-field-center-aligned "> <div class=""> B </div> </td>',
(
'<td class="table-field-center-aligned "> '
'<div class="table-field-status-default"> '
'<ul class="list list-bullet"> '
'<li>foo</li> <li>foo</li> <li>foo</li> '
'</ul> '
'</div> '
'</td>'
)
),
(
'<td class="table-field-center-aligned "> <div class=""> 07700900003 </div> </td>',
'<td class="table-field-center-aligned "> <div class=""> C </div> </td>',
(
'<td class="table-field-center-aligned "> '
'<div class="table-field-status-default"> '
'<ul class="list list-bullet"> '
'<li>foo</li> <li>foo</li> '
'</ul> '
'</div> '
'</td>'
)
), ),
]): ]):
assert normalize_spaces(str(page.select('table tbody td')[index + 1])) == cell for index, cell in enumerate(row):
row = page.select('table tbody tr')[row_index]
assert normalize_spaces(str(row.select('td')[index + 1])) == cell
def test_show_all_columns_if_there_are_duplicate_recipient_columns( def test_show_all_columns_if_there_are_duplicate_recipient_columns(