From dd9548da67587432dc1acae3903ec14f620bdf69 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 7 Apr 2016 14:30:28 +0100 Subject: [PATCH 1/2] Change wording when there are hidden rows This caused some anxiety about why the rows were being hidden. Were there problems with them? This commit reframes the wording to talk about the rows that are shown instead. --- app/templates/views/check.html | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/app/templates/views/check.html b/app/templates/views/check.html index 37db226d8..255a84dfd 100644 --- a/app/templates/views/check.html +++ b/app/templates/views/check.html @@ -97,7 +97,15 @@ {% if count_of_displayed_recipients < count_of_recipients %} + {% elif rows_have_errors %} + {% endif %} From 2912ea18062a465189dcd8e4de8196ace46e3bf7 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 7 Apr 2016 14:30:51 +0100 Subject: [PATCH 2/2] Increase number of rows shown MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We probably shouldn’t hide the contents of the CSV when people are just testing the app, or if they’re starting off with small jobs. A limit of 15 rows displayed was awkwardly on the cusp between just testing and sending a small batch. This commit increases the limit to 50; I reckon that over 50 recipients no-one will be wanting to check them all individually. --- app/main/views/send.py | 4 ++-- tests/app/main/views/test_send.py | 32 +++++++------------------------ 2 files changed, 9 insertions(+), 27 deletions(-) diff --git a/app/main/views/send.py b/app/main/views/send.py index a44626776..ab87b8615 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -227,8 +227,8 @@ def check_messages(service_id, template_type, upload_id): contents, template_type=template.template_type, placeholders=template.placeholders, - max_initial_rows_shown=15, - max_errors_shown=15, + max_initial_rows_shown=50, + max_errors_shown=50, whitelist=itertools.chain.from_iterable( [user.mobile_number, user.email_address] for user in users ) if current_service['restricted'] else None diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index 02c942920..27a7cc3e1 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -157,27 +157,9 @@ def test_upload_csvfile_with_valid_phone_shows_all_numbers( mocker.patch( 'app.main.views.send.s3download', - return_value=""" - phone number - 07700 900701 - 07700 900702 - 07700 900703 - 07700 900704 - 07700 900705 - 07700 900706 - 07700 900707 - 07700 900708 - 07700 900709 - 07700 900710 - 07700 900711 - 07700 900712 - 07700 900713 - 07700 900714 - 07700 900715 - 07700 900799 - 07700 900799 - 07700 900799 - """ + return_value='\n'.join(['phone number'] + [ + '07700 9007{0:02d}'.format(final_two) for final_two in range(0, 53) + ]) ) with app_.test_request_context(): @@ -192,14 +174,14 @@ def test_upload_csvfile_with_valid_phone_shows_all_numbers( with client.session_transaction() as sess: assert int(sess['upload_data']['template_id']) == 54321 assert sess['upload_data']['original_file_name'] == 'valid.csv' - assert sess['upload_data']['notification_count'] == 18 + assert sess['upload_data']['notification_count'] == 53 content = response.get_data(as_text=True) assert response.status_code == 200 assert '07700 900701' in content - assert '07700 900715' in content - assert '07700 900716' not in content - assert '3 rows not shown' in content + assert '07700 900749' in content + assert '07700 900750' not in content + assert 'Only showing the first 50 rows with errors' in content def test_create_job_should_call_api(