From 592ab58f372eaf5e8af808088fc2c290df0d8e55 Mon Sep 17 00:00:00 2001 From: karlchillmaid Date: Wed, 18 Sep 2019 16:11:46 +0100 Subject: [PATCH 1/5] Update spreadsheet validation errors --- app/templates/views/check/column-errors.html | 70 +++++++++++--------- 1 file changed, 39 insertions(+), 31 deletions(-) diff --git a/app/templates/views/check/column-errors.html b/app/templates/views/check/column-errors.html index 50e68ea73..66dd8f7da 100644 --- a/app/templates/views/check/column-errors.html +++ b/app/templates/views/check/column-errors.html @@ -38,7 +38,7 @@ {% elif not count_of_recipients %}

- Your file is missing some rows + Your file is missing rows

{% if recipients.missing_column_headers %}

@@ -56,32 +56,38 @@ {% elif not recipients.has_recipient_columns %}

- Your file needs {{ ( - recipients.missing_column_headers - if template.template_type == 'letter' else required_recipient_columns - ) | formatted_list( - prefix='a column called', - prefix_plural='columns called' - ) }} + Your file is missing columns

-

- Right now it has {{ recipients.column_headers | formatted_list( - prefix='one column, called ', - prefix_plural='columns called ' - ) }}. -

+

+ It needs {{ ( + recipients.missing_column_headers + if template.template_type == 'letter' else required_recipient_columns + ) | formatted_list( + prefix='a column called', + prefix_plural='columns called' + ) }}. +

+

+ Right now it has {{ recipients.column_headers | formatted_list( + prefix='one column, called ', + prefix_plural='columns called ' + ) }}. +

{% elif recipients.duplicate_recipient_column_headers %}

- Your file has more than one column called {{ ( - recipients.duplicate_recipient_column_headers - ) | formatted_list( - conjunction='or', - prefix='', - prefix_plural='' - ) }} + Your file has duplicate columns

+

+ We found more than one column called {{ ( + recipients.duplicate_recipient_column_headers + ) | formatted_list( + conjunction='or', + prefix='', + prefix_plural='' + ) }}. +

Delete or rename one of these columns and try again.

@@ -89,16 +95,18 @@ {% elif recipients.missing_column_headers %}

- The columns in your file need to match the double brackets in - your template + Your file is missing columns

-

- Your file is missing {{ recipients.missing_column_headers | formatted_list( - conjunction='and', - prefix='a column called ', - prefix_plural='columns called ' - ) }}. -

+

+ The column names need to match the double brackets in your template. +

+

+ Your file is missing {{ recipients.missing_column_headers | formatted_list( + conjunction='and', + prefix='a column called ', + prefix_plural='columns called ' + ) }}. +

{% elif sent_previously %} @@ -141,7 +149,7 @@ {{ file_upload( form.file, action=url_for('.send_messages', service_id=current_service.id, template_id=template.id), - button_text='Re-upload your file' + button_text='Upload a new file' ) }} {% endif %} From 05b2930826a1335e737ab050431189a18bc38009 Mon Sep 17 00:00:00 2001 From: karlchillmaid Date: Wed, 18 Sep 2019 16:13:16 +0100 Subject: [PATCH 2/5] Add 'some' for readability --- app/templates/views/check/column-errors.html | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/templates/views/check/column-errors.html b/app/templates/views/check/column-errors.html index 66dd8f7da..02828f3a5 100644 --- a/app/templates/views/check/column-errors.html +++ b/app/templates/views/check/column-errors.html @@ -38,7 +38,7 @@ {% elif not count_of_recipients %}

- Your file is missing rows + Your file is missing some rows

{% if recipients.missing_column_headers %}

@@ -56,7 +56,7 @@ {% elif not recipients.has_recipient_columns %}

- Your file is missing columns + Your file is missing some columns

It needs {{ ( @@ -95,7 +95,7 @@ {% elif recipients.missing_column_headers %}

- Your file is missing columns + Your file is missing some columns

The column names need to match the double brackets in your template. From 2e5b724adb26710f043a9e8362283bf7011ba66b Mon Sep 17 00:00:00 2001 From: karlchillmaid Date: Fri, 27 Sep 2019 12:01:32 +0100 Subject: [PATCH 3/5] Update error messages --- app/templates/views/check/column-errors.html | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/app/templates/views/check/column-errors.html b/app/templates/views/check/column-errors.html index 02828f3a5..09159d2a0 100644 --- a/app/templates/views/check/column-errors.html +++ b/app/templates/views/check/column-errors.html @@ -56,10 +56,10 @@ {% elif not recipients.has_recipient_columns %}

- Your file is missing some columns + There’s a problem with your column names

- It needs {{ ( + Your file needs {{ ( recipients.missing_column_headers if template.template_type == 'letter' else required_recipient_columns ) | formatted_list( @@ -95,11 +95,8 @@ {% elif recipients.missing_column_headers %}

- Your file is missing some columns + Your column names do not match the double brackets in your template

-

- The column names need to match the double brackets in your template. -

Your file is missing {{ recipients.missing_column_headers | formatted_list( conjunction='and', From dd1ff2cbf4bbf71264a74bacd8d4d06576a138e2 Mon Sep 17 00:00:00 2001 From: karlchillmaid Date: Fri, 27 Sep 2019 12:03:10 +0100 Subject: [PATCH 4/5] Update button text --- app/templates/views/check/column-errors.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/templates/views/check/column-errors.html b/app/templates/views/check/column-errors.html index 09159d2a0..c87e08769 100644 --- a/app/templates/views/check/column-errors.html +++ b/app/templates/views/check/column-errors.html @@ -146,7 +146,7 @@ {{ file_upload( form.file, action=url_for('.send_messages', service_id=current_service.id, template_id=template.id), - button_text='Upload a new file' + button_text='Upload your file again' ) }} {% endif %} From 3d2f7c3fcf50e4323bc0948ad2bd354a228f713c Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 3 Oct 2019 14:24:28 +0100 Subject: [PATCH 5/5] Refine error messages and fix tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removing the word ‘duplicate’ because: - it suggests that the whole column is the same, which it might not be - it suggests that having duplicate column names is a problem, which is only true in the case of recipient columns Reverts back to saying the column names ‘need to’ match, because we feel it’s more instructive. --- app/templates/views/check/column-errors.html | 4 ++-- tests/app/main/views/test_send.py | 11 +++++++---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/app/templates/views/check/column-errors.html b/app/templates/views/check/column-errors.html index c87e08769..3269850da 100644 --- a/app/templates/views/check/column-errors.html +++ b/app/templates/views/check/column-errors.html @@ -77,7 +77,7 @@ {% elif recipients.duplicate_recipient_column_headers %}

- Your file has duplicate columns + There’s a problem with your column names

We found more than one column called {{ ( @@ -95,7 +95,7 @@ {% elif recipients.missing_column_headers %}

- Your column names do not match the double brackets in your template + Your column names need to match the double brackets in your template

Your file is missing {{ recipients.missing_column_headers | formatted_list( diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index 87e2a8f89..1d6e50854 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -427,7 +427,8 @@ def test_upload_csv_file_with_errors_shows_check_page_with_errors( +447700900986 """, ( - 'Your file needs a column called ‘phone number’ ' + 'There’s a problem with your column names ' + 'Your file needs a column called ‘phone number’. ' 'Right now it has columns called ‘telephone’ and ‘name’. ' 'Skip to file contents' ) @@ -438,7 +439,7 @@ def test_upload_csv_file_with_errors_shows_check_page_with_errors( +447700900986 """, ( - 'The columns in your file need to match the double brackets in your template ' + 'Your column names need to match the double brackets in your template ' 'Your file is missing a column called ‘name’. ' 'Skip to file contents' ) @@ -449,7 +450,8 @@ def test_upload_csv_file_with_errors_shows_check_page_with_errors( +447700900111,+447700900222,+447700900333, """, ( - 'Your file has more than one column called ‘phone number’ or ‘PHONE_NUMBER’ ' + 'There’s a problem with your column names ' + 'We found more than one column called ‘phone number’ or ‘PHONE_NUMBER’. ' 'Delete or rename one of these columns and try again. ' 'Skip to file contents' ) @@ -2863,7 +2865,8 @@ def test_check_messages_column_error_doesnt_show_optional_columns( ) assert normalize_spaces(page.select_one('.banner-dangerous').text) == ( - 'Your file needs a column called ‘postcode’ ' + 'There’s a problem with your column names ' + 'Your file needs a column called ‘postcode’. ' 'Right now it has columns called ‘address_line_1’, ‘address_line_2’ and ‘foo’. ' 'Skip to file contents' )