From 3e7bb42323a7bd381ffb6d12258b44267e76ba6e Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 3 Feb 2016 14:36:11 +0000 Subject: [PATCH] Replace message previews on check page with table MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first 3/last 3 messages didn’t test well, it wasn’t immediately obvious what was going on. This commit replaces it with just a preview of the first message, and a table showing the details of the subsequent messages. --- app/assets/stylesheets/components/table.scss | 1 + app/main/views/sms.py | 1 + app/templates/views/check-sms.html | 57 +++++++------------- tests/app/main/views/test_sms.py | 53 +++--------------- 4 files changed, 28 insertions(+), 84 deletions(-) diff --git a/app/assets/stylesheets/components/table.scss b/app/assets/stylesheets/components/table.scss index f5c0c94b0..a78886a13 100644 --- a/app/assets/stylesheets/components/table.scss +++ b/app/assets/stylesheets/components/table.scss @@ -4,6 +4,7 @@ .table-heading { text-align: left; + margin: 40px 0 5px 0; } %table-field, diff --git a/app/main/views/sms.py b/app/main/views/sms.py index 3a93e1d55..cf82770ac 100644 --- a/app/main/views/sms.py +++ b/app/main/views/sms.py @@ -103,6 +103,7 @@ def check_sms(service_id, upload_id): 'views/check-sms.html', upload_result=upload_result, message_template=template['content'], + original_file_name=original_file_name, template_id=template_id, service_id=service_id ) diff --git a/app/templates/views/check-sms.html b/app/templates/views/check-sms.html index 82c078a46..2d3f02093 100644 --- a/app/templates/views/check-sms.html +++ b/app/templates/views/check-sms.html @@ -1,6 +1,6 @@ {% extends "withnav_template.html" %} {% from "components/sms-message.html" import sms_message %} -{% from "components/table.html" import table, field %} +{% from "components/table.html" import list_table, field %} {% from "components/placeholder.html" import placeholder %} {% from "components/page-footer.html" import page_footer %} @@ -21,44 +21,25 @@ {% else %} -

Check and confirm

+ {{ sms_message( + message_template|replace_placeholders(upload_result.valid[0]), + name='Preview' + )}} +
- - {{ page_footer( - button_text = "Send {} text messages".format(upload_result.valid|count), - back_link = url_for(".send_sms", service_id=service_id, template_id=template_id) - )}} - - {% if upload_result.valid | count > 6 %} -

First three message in file

- {% for recipient in upload_result.valid[:3] %} - {{ sms_message(message_template|replace_placeholders( - recipient), - '{}'.format(recipient['phone']) - )}} - {% endfor %} -

Last three messages in file

- {% for recipient in upload_result.valid[-3:] %} - {{ sms_message(message_template|replace_placeholders( - recipient), - '{}'.format(recipient['phone']) - )}} - {% endfor %} - {% else %} -

All messages in file

- {% for recipient in upload_result.valid %} - {{ sms_message(message_template|replace_placeholders( - recipient), - '{}'.format(recipient['phone']) - )}} - {% endfor %} - {% endif %} - - {{ page_footer( - button_text = "Send {} text messages".format(upload_result.valid|count), - back_link = url_for(".send_sms", service_id=service_id, template_id=template_id) - )}} - + +
+ + {% call(item) list_table( + upload_result.valid, + caption=original_file_name, + field_headings=['Phone number'] + ) %} + {% call field() %} + {{ item.phone }} + {% endcall %} + {% endcall %} + {% endif %} {% endblock %} diff --git a/tests/app/main/views/test_sms.py b/tests/app/main/views/test_sms.py index 98635d609..fd79dcf08 100644 --- a/tests/app/main/views/test_sms.py +++ b/tests/app/main/views/test_sms.py @@ -87,50 +87,13 @@ def test_upload_csvfile_with_invalid_phone_shows_check_page_with_errors(app_, @moto.mock_s3 -def test_upload_csvfile_with_valid_phone_shows_first3_and_last3_numbers(app_, - mocker, - api_user_active, - mock_get_user, - mock_get_user_by_email, - mock_login, - mock_get_service_template): - contents = 'phone\n+44 7700 900981\n+44 7700 900982\n+44 7700 900983\n+44 7700 900984\n+44 7700 900985\n+44 7700 900986\n+44 7700 900987\n+44 7700 900988\n+44 7700 900989' # noqa - - file_data = (BytesIO(contents.encode('utf-8')), 'valid.csv') - - with app_.test_request_context(): - with app_.test_client() as client: - client.login(api_user_active) - upload_data = {'file': file_data} - response = client.post(url_for('main.send_sms', service_id=12345, template_id=54321), - data=upload_data, - follow_redirects=True) - - content = response.get_data(as_text=True) - - assert response.status_code == 200 - assert 'Check and confirm' in content - assert 'First three message in file' in content - assert 'Last three messages in file' in content - assert '+44 7700 900981' in content - assert '+44 7700 900982' in content - assert '+44 7700 900983' in content - assert '+44 7700 900984' not in content - assert '+44 7700 900985' not in content - assert '+44 7700 900986' not in content - assert '+44 7700 900987' in content - assert '+44 7700 900988' in content - assert '+44 7700 900989' in content - - -@moto.mock_s3 -def test_upload_csvfile_with_valid_phone_shows_all_if_6_or_less_numbers(app_, - mocker, - api_user_active, - mock_get_user, - mock_get_user_by_email, - mock_login, - mock_get_service_template): +def test_upload_csvfile_with_valid_phone_shows_all_numbers(app_, + mocker, + api_user_active, + mock_get_user, + mock_get_user_by_email, + mock_login, + mock_get_service_template): contents = 'phone\n+44 7700 900981\n+44 7700 900982\n+44 7700 900983\n+44 7700 900984\n+44 7700 900985\n+44 7700 900986' # noqa @@ -147,8 +110,6 @@ def test_upload_csvfile_with_valid_phone_shows_all_if_6_or_less_numbers(app_, content = response.get_data(as_text=True) assert response.status_code == 200 - assert 'Check and confirm' in content - assert 'All messages in file' in content assert '+44 7700 900981' in content assert '+44 7700 900982' in content assert '+44 7700 900983' in content