From 3e7bb42323a7bd381ffb6d12258b44267e76ba6e Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 3 Feb 2016 14:36:11 +0000 Subject: [PATCH 01/10] 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 From 856296df5d47a99f0e1b8ee0592f03a4aa23c464 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 3 Feb 2016 14:43:16 +0000 Subject: [PATCH 02/10] Updates to display of jobs This commit: - adds the template to the jobs page (and puts it at the top of the send SMS page) so that it consistently appears in the same place throughout the journey - put the real data about a job on the jobs page and on the dashboard --- app/assets/stylesheets/components/banner.scss | 3 +- app/assets/stylesheets/components/table.scss | 1 - app/main/views/dashboard.py | 5 ++-- app/main/views/jobs.py | 5 ++-- app/templates/components/table.html | 28 +++++++++-------- app/templates/flash_messages.html | 5 ++-- app/templates/views/check-sms.html | 5 ++-- app/templates/views/job.html | 30 +++++++++++-------- app/templates/views/send-sms.html | 5 +++- app/templates/views/service_dashboard.html | 19 +++++------- tests/app/main/views/test_dashboard.py | 3 +- tests/app/main/views/test_jobs.py | 1 + tests/app/main/views/test_sign_out.py | 3 +- 13 files changed, 63 insertions(+), 50 deletions(-) diff --git a/app/assets/stylesheets/components/banner.scss b/app/assets/stylesheets/components/banner.scss index ae237e56d..02c12dfc2 100644 --- a/app/assets/stylesheets/components/banner.scss +++ b/app/assets/stylesheets/components/banner.scss @@ -13,7 +13,8 @@ } -.banner-with-tick { +.banner-with-tick, +.banner-default-with-tick { @extend %banner; padding: $gutter-half ($gutter + $gutter-half); diff --git a/app/assets/stylesheets/components/table.scss b/app/assets/stylesheets/components/table.scss index a78886a13..97b14296c 100644 --- a/app/assets/stylesheets/components/table.scss +++ b/app/assets/stylesheets/components/table.scss @@ -57,7 +57,6 @@ .table-empty-message { @include core-16; color: $secondary-text-colour; - border-top: 1px solid $border-colour; border-bottom: 1px solid $border-colour; padding: 0.75em 0 0.5625em 0; } diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index 9c25fb21f..2db8b620e 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -4,7 +4,7 @@ from app.main import main from app.main.dao.services_dao import get_service_by_id from app.main.dao import templates_dao from client.errors import HTTPError -from ._jobs import jobs +from app import job_api_client @main.route("/services//dashboard") @@ -12,6 +12,7 @@ from ._jobs import jobs def service_dashboard(service_id): try: templates = templates_dao.get_service_templates(service_id)['data'] + jobs = job_api_client.get_job(service_id)['data'] except HTTPError as e: if e.status_code == 404: abort(404) @@ -27,7 +28,7 @@ def service_dashboard(service_id): raise e return render_template( 'views/service_dashboard.html', - jobs=jobs, + jobs=reversed(jobs), free_text_messages_remaining='25,000', spent_this_month='0.00', template_count=len(templates), diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index 413bd2491..f18fc4470 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -11,6 +11,7 @@ from client.errors import HTTPError from app import job_api_client from app.main import main +from app.main.dao import templates_dao now = time.strftime('%H:%M') @@ -37,6 +38,7 @@ def view_jobs(service_id): def view_job(service_id, job_id): try: job = job_api_client.get_job(service_id, job_id)['data'] + template = templates_dao.get_service_template(service_id, job['template'])['data'] messages = [] return render_template( 'views/job.html', @@ -53,8 +55,7 @@ def view_job(service_id, job_id): cost=u'£0.00', uploaded_file_name=job['original_file_name'], uploaded_file_time=job['created_at'], - template_used=job['template'], - flash_message="We’ve accepted {} for processing".format(job['original_file_name']), + template=template, service_id=service_id ) except HTTPError as e: diff --git a/app/templates/components/table.html b/app/templates/components/table.html index a37a5ef89..ed9b35902 100644 --- a/app/templates/components/table.html +++ b/app/templates/components/table.html @@ -25,19 +25,21 @@ {% macro list_table(items, caption='', empty_message='', field_headings=[], field_headings_visible=True, caption_visible=True) -%} {% set parent_caller = caller %} - {% if items %} - {% call mapping_table(caption, field_headings, field_headings_visible, caption_visible) %} - {% for item in items %} - {% call row() %} - {{ parent_caller(item) }} - {% endcall %} - {% endfor %} - {%- endcall %} - {% else %} -

- {{ empty_message }} -

- {% endif %} + + {% call mapping_table(caption, field_headings, field_headings_visible, caption_visible) %} + {% for item in items %} + {% call row() %} + {{ parent_caller(item) }} + {% endcall %} + {% endfor %} + {% if not items %} + {% call row() %} + + {{ empty_message }} + + {% endcall %} + {% endif %} + {%- endcall %} {%- endmacro %} diff --git a/app/templates/flash_messages.html b/app/templates/flash_messages.html index aa775c788..85be2d13d 100644 --- a/app/templates/flash_messages.html +++ b/app/templates/flash_messages.html @@ -4,8 +4,9 @@ {% for category, message in messages %} {{ banner( message, - 'default' if category == 'default' else 'dangerous', - delete_button="Yes, delete this template" if 'delete' == category else None + 'default' if category == 'default' or 'default_with_tick' else 'dangerous', + delete_button="Yes, delete this template" if 'delete' == category else None, + with_tick=True if category == 'default_with_tick' else False )}} {% endfor %} {% endif %} diff --git a/app/templates/views/check-sms.html b/app/templates/views/check-sms.html index 2d3f02093..556384867 100644 --- a/app/templates/views/check-sms.html +++ b/app/templates/views/check-sms.html @@ -10,7 +10,7 @@ {% block maincolumn_content %} -

Send text messages

+

Check and confirm

{% if upload_result.rejects %}

The following numbers are invalid

@@ -22,8 +22,7 @@ {% else %} {{ sms_message( - message_template|replace_placeholders(upload_result.valid[0]), - name='Preview' + message_template|replace_placeholders(upload_result.valid[0]) )}}
diff --git a/app/templates/views/job.html b/app/templates/views/job.html index 15435f040..e0dfd9473 100644 --- a/app/templates/views/job.html +++ b/app/templates/views/job.html @@ -2,23 +2,33 @@ {% from "components/table.html" import list_table, field, right_aligned_field_heading %} {% from "components/big-number.html" import big_number %} {% from "components/banner.html" import banner %} +{% from "components/sms-message.html" import sms_message %} {% block page_title %} -GOV.UK Notify | Notifications activity + GOV.UK Notify | Notifications activity {% endblock %} {% block maincolumn_content %}

- {{ uploaded_file_name }} + Sent text messages

-

- {{ banner(flash_message, with_tick=True) }} + {{ sms_message( + template['content'], + )}} + +

+ Started {{ uploaded_file_time|format_datetime }}

    -
  • +
  • + {{ big_number( + counts.total, 'queued' + )}} +
    • +
  • {{ big_number( counts.total, 'text message' if 1 == counts.total else 'text messages' @@ -27,7 +37,7 @@ GOV.UK Notify | Notifications activity
  • {{ big_number( counts.failed, - 'failed delivery' if 1 == counts.failed else 'failed deliveries' + 'failed' )}}
  • @@ -37,14 +47,10 @@ GOV.UK Notify | Notifications activity
-

- Sent with template {{ template_used }} on {{ uploaded_file_time | format_datetime}} -

- {% call(item) list_table( messages, - caption='Messages', - caption_visible=False, + caption=uploaded_file_name, + empty_message="Messages go here", field_headings=[ 'To', 'Message', diff --git a/app/templates/views/send-sms.html b/app/templates/views/send-sms.html index 5ccbec5c1..e33d8fdc2 100644 --- a/app/templates/views/send-sms.html +++ b/app/templates/views/send-sms.html @@ -8,9 +8,12 @@ {% endblock %} {% block maincolumn_content %} + +

Send text messages

+ -

Send text messages

+ {{ sms_message(template.content) }} {{ banner( 'You can only send notifications to yourself', diff --git a/app/templates/views/service_dashboard.html b/app/templates/views/service_dashboard.html index 7caa0cb66..14bab0275 100644 --- a/app/templates/views/service_dashboard.html +++ b/app/templates/views/service_dashboard.html @@ -1,5 +1,5 @@ {% extends "withnav_template.html" %} -{% from "components/table.html" import list_table, field %} +{% from "components/table.html" import list_table, field, right_aligned_field_heading %} {% from "components/big-number.html" import big_number %} {% block page_title %} @@ -31,7 +31,7 @@ subhead='Get started', type="tip" )}} - {% else %} + {% elif not jobs %} {{ banner( 'Try sending a text message'.format( url_for(".choose_sms_template", service_id=service_id) @@ -41,23 +41,20 @@ )}} {% endif %} - {% if [] %} + {% if jobs %} {% call(item) list_table( - [], + jobs, caption="Recent text messages", empty_message='You haven’t sent any text messages yet', - field_headings=['Job', 'File', 'Time', 'Status'] + field_headings=['Job', 'Created', right_aligned_field_heading('Status')] ) %} {% call field() %} - {{ item.file }} + {{ item.original_file_name }} {% endcall %} {% call field() %} - {{ item.job }} + {{ item.created_at|format_datetime }} {% endcall %} - {% call field() %} - {{ item.time }} - {% endcall %} - {% call field() %} + {% call field(align='right') %} {{ item.status }} {% endcall %} {% endcall %} diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index 7a2a3ad67..d5b3386ea 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -7,7 +7,8 @@ def test_should_show_recent_jobs_on_dashboard(app_, mock_get_service_templates, mock_get_user, mock_get_user_by_email, - mock_login): + mock_login, + mock_get_jobs): with app_.test_request_context(): with app_.test_client() as client: diff --git a/tests/app/main/views/test_jobs.py b/tests/app/main/views/test_jobs.py index 6b32dd549..98f99e514 100644 --- a/tests/app/main/views/test_jobs.py +++ b/tests/app/main/views/test_jobs.py @@ -27,6 +27,7 @@ def test_should_show_page_for_one_job(app_, mock_login, mock_get_user, mock_get_user_by_email, + mock_get_service_template, job_data, mock_get_job): service_id = job_data['service'] diff --git a/tests/app/main/views/test_sign_out.py b/tests/app/main/views/test_sign_out.py index 03f7edfb1..cf7b5a665 100644 --- a/tests/app/main/views/test_sign_out.py +++ b/tests/app/main/views/test_sign_out.py @@ -18,7 +18,8 @@ def test_sign_out_user(app_, mock_get_user, mock_get_user_by_email, mock_get_service_templates, - mock_login): + mock_login, + mock_get_jobs): with app_.test_request_context(): email = 'valid@example.gov.uk' password = 'val1dPassw0rd!' From 4280c21b2355b4b111df95fbbe658e51d30f93c8 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 3 Feb 2016 15:24:50 +0000 Subject: [PATCH 03/10] Updates to send and check SMS pages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a back button instead of a back link (more prominent, you’re likely to go back on these pages if you’ve made a mistake) Tweaks to wording --- app/assets/stylesheets/components/banner.scss | 1 + .../stylesheets/components/page-footer.scss | 6 +++-- app/main/forms.py | 2 +- app/templates/views/check-sms.html | 1 + app/templates/views/job.html | 16 +++++++++----- app/templates/views/send-sms.html | 22 +++++-------------- 6 files changed, 23 insertions(+), 25 deletions(-) diff --git a/app/assets/stylesheets/components/banner.scss b/app/assets/stylesheets/components/banner.scss index 02c12dfc2..35ceff1fd 100644 --- a/app/assets/stylesheets/components/banner.scss +++ b/app/assets/stylesheets/components/banner.scss @@ -10,6 +10,7 @@ margin: $gutter-half 0 $gutter 0; text-align: left; position: relative; + clear: both; } diff --git a/app/assets/stylesheets/components/page-footer.scss b/app/assets/stylesheets/components/page-footer.scss index 9b8b2feb1..4aea437c4 100644 --- a/app/assets/stylesheets/components/page-footer.scss +++ b/app/assets/stylesheets/components/page-footer.scss @@ -3,8 +3,10 @@ margin-bottom: 50px; &-back-link { - display: block; - margin-top: $gutter; + @include button($grey-1); + display: inline-block; + padding: 0.52632em 0.78947em 0.26316em 0.78947em; + margin-left: 10px; } &-delete-link { diff --git a/app/main/forms.py b/app/main/forms.py index cd255f029..9c6787548 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -217,7 +217,7 @@ class ChangePasswordForm(Form): class CsvUploadForm(Form): - file = FileField('Upload a CSV file to add your recipients’ details', validators=[DataRequired( + file = FileField('Add your recipients by uploading a CSV file', validators=[DataRequired( message='Please pick a file'), CsvFileValidator()]) diff --git a/app/templates/views/check-sms.html b/app/templates/views/check-sms.html index 556384867..419eb2fe3 100644 --- a/app/templates/views/check-sms.html +++ b/app/templates/views/check-sms.html @@ -28,6 +28,7 @@ + Back {% call(item) list_table( diff --git a/app/templates/views/job.html b/app/templates/views/job.html index e0dfd9473..c22b5f595 100644 --- a/app/templates/views/job.html +++ b/app/templates/views/job.html @@ -11,7 +11,7 @@ {% block maincolumn_content %}

- Sent text messages + {{ uploaded_file_name }}

{{ sms_message( @@ -30,31 +30,35 @@
  • {{ big_number( - counts.total, - 'text message' if 1 == counts.total else 'text messages' + 0, 'sent' )}}
    • -
  • +
  • {{ big_number( counts.failed, 'failed' )}}
    • -
  • +
  • {{ big_number( cost, 'total cost' )}}
    • + {{ sms_message( + 'Your application has been received. Register online at www.gov.uk/register-to-vote', + )}} + {% call(item) list_table( messages, caption=uploaded_file_name, + caption_visible=False, empty_message="Messages go here", field_headings=[ 'To', 'Message', - right_aligned_field_heading('Delivery status') + right_aligned_field_heading('Status') ] ) %} {% call field() %} diff --git a/app/templates/views/send-sms.html b/app/templates/views/send-sms.html index e33d8fdc2..e227a3185 100644 --- a/app/templates/views/send-sms.html +++ b/app/templates/views/send-sms.html @@ -15,15 +15,11 @@ {{ sms_message(template.content) }} - {{ banner( - 'You can only send notifications to yourself', - subhead='Trial mode', - type='info' - ) }} + {{file_upload(form.file)}} - {{ sms_message( - template.content, name='Preview' - ) }} +

    + Download an example CSV file +

    {{ banner( 'You can only send messages to yourself until you request to go live'.format( @@ -32,16 +28,10 @@ type='info' ) }} - {{file_upload(form.file)}} - -

    - Download an example CSV to test with. -

    - {{ page_footer( - "Continue", + "Continue to preview", back_link=url_for(".choose_sms_template", service_id=service_id), - back_link_text="Back to templates" + back_link_text="Back" ) }} From 28f700366c6982e1ba198dc2928eb7901da387db Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 3 Feb 2016 15:49:28 +0000 Subject: [PATCH 04/10] Fix spacing of first page headers This commit customises the margins of the first header on each page so that it lines up with the navigation. --- app/assets/stylesheets/app.scss | 8 ++++++++ app/templates/views/choose-sms-template.html | 5 +++-- app/templates/views/documentation.html | 8 ++++---- app/templates/withnav_template.html | 2 +- 4 files changed, 16 insertions(+), 7 deletions(-) diff --git a/app/assets/stylesheets/app.scss b/app/assets/stylesheets/app.scss index 3fba6dad2..e88b13ae4 100644 --- a/app/assets/stylesheets/app.scss +++ b/app/assets/stylesheets/app.scss @@ -38,3 +38,11 @@ a:visited { width: 5em; } } + +.column-main { + + > .heading-large { + margin-top: $gutter; + } + +} diff --git a/app/templates/views/choose-sms-template.html b/app/templates/views/choose-sms-template.html index 8205df27f..3c00892d9 100644 --- a/app/templates/views/choose-sms-template.html +++ b/app/templates/views/choose-sms-template.html @@ -8,9 +8,10 @@ {% endblock %} {% block maincolumn_content %} -
    -

    Send text messages

    +

    Send text messages

    + + {% if templates %}
    diff --git a/app/templates/views/documentation.html b/app/templates/views/documentation.html index 3472631a2..924c467ec 100644 --- a/app/templates/views/documentation.html +++ b/app/templates/views/documentation.html @@ -8,13 +8,13 @@ GOV.UK Notify | API keys and documentation {% block maincolumn_content %} +

    + Developer documentation +

    +
    -

    - Developer documentation -

    -

    How to integrate GOV.UK Notify into your service

    diff --git a/app/templates/withnav_template.html b/app/templates/withnav_template.html index 73ce81224..d981762a1 100644 --- a/app/templates/withnav_template.html +++ b/app/templates/withnav_template.html @@ -5,7 +5,7 @@
    {% include "main_nav.html" %}
    -
    +
    {% include 'flash_messages.html' %} {% block maincolumn_content %}{% endblock %}
    From 48df622aba8207aba4633c265df89bc6cc265244 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 4 Feb 2016 09:58:56 +0000 Subject: [PATCH 05/10] Add two steps to onboarding banner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It’s not obvious what the first steps are when you’ve just signed up. This commit changes the banner on the dashboard to make it obvious. --- app/assets/stylesheets/components/banner.scss | 4 +++ app/main/views/dashboard.py | 2 +- app/templates/views/service_dashboard.html | 28 +++++++++---------- 3 files changed, 19 insertions(+), 15 deletions(-) diff --git a/app/assets/stylesheets/components/banner.scss b/app/assets/stylesheets/components/banner.scss index 35ceff1fd..75674611b 100644 --- a/app/assets/stylesheets/components/banner.scss +++ b/app/assets/stylesheets/components/banner.scss @@ -77,4 +77,8 @@ text-decoration: underline; } + ol { + list-style-type: decimal; + } + } diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index 2db8b620e..fe3c9fd1b 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -28,7 +28,7 @@ def service_dashboard(service_id): raise e return render_template( 'views/service_dashboard.html', - jobs=reversed(jobs), + jobs=list(reversed(jobs)), free_text_messages_remaining='25,000', spent_this_month='0.00', template_count=len(templates), diff --git a/app/templates/views/service_dashboard.html b/app/templates/views/service_dashboard.html index 14bab0275..f2dca758d 100644 --- a/app/templates/views/service_dashboard.html +++ b/app/templates/views/service_dashboard.html @@ -23,25 +23,25 @@ - {% if not template_count %} + {% if not jobs %} {{ banner( - 'Add a text message template'.format( - url_for(".add_service_template", service_id=service_id) + """ +
      +
    1. + Add a template +
      2. +
    2. + Send yourself a text message +
      3. +
    + """.format( + url_for(".add_service_template", service_id=service_id), + url_for(".choose_sms_template", service_id=service_id) )|safe, subhead='Get started', type="tip" )}} - {% elif not jobs %} - {{ banner( - 'Try sending a text message'.format( - url_for(".choose_sms_template", service_id=service_id) - )|safe, - subhead='Next step', - type="tip" - )}} - {% endif %} - - {% if jobs %} + {% else %} {% call(item) list_table( jobs, caption="Recent text messages", From 54cad9cf24758454ce903725ec872991d0aaf1c1 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 4 Feb 2016 11:07:33 +0000 Subject: [PATCH 06/10] Remove dummy content --- app/templates/views/job.html | 4 ---- 1 file changed, 4 deletions(-) diff --git a/app/templates/views/job.html b/app/templates/views/job.html index c22b5f595..0a2e86ca2 100644 --- a/app/templates/views/job.html +++ b/app/templates/views/job.html @@ -46,10 +46,6 @@ - {{ sms_message( - 'Your application has been received. Register online at www.gov.uk/register-to-vote', - )}} - {% call(item) list_table( messages, caption=uploaded_file_name, From 51208a9eb2adad93c653c5698fc7f96e0538c68a Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 4 Feb 2016 11:31:33 +0000 Subject: [PATCH 07/10] Make banner on template page real --- app/main/views/templates.py | 3 +++ app/templates/views/manage-templates.html | 17 ++++++++++------- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index 2cb9176c8..38d175a92 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -3,6 +3,7 @@ from flask_login import login_required from app.main import main from app.main.forms import TemplateForm +from app import job_api_client from app.main.dao.services_dao import get_service_by_id from app.main.dao import templates_dao as tdao from app.main.dao import services_dao as sdao @@ -13,6 +14,7 @@ from client.errors import HTTPError @login_required def manage_service_templates(service_id): try: + jobs = job_api_client.get_job(service_id)['data'] templates = tdao.get_service_templates(service_id)['data'] except HTTPError as e: if e.status_code == 404: @@ -22,6 +24,7 @@ def manage_service_templates(service_id): return render_template( 'views/manage-templates.html', service_id=service_id, + has_jobs=bool(jobs), templates=[tdao.TemplatesBrowsableItem(x) for x in templates]) diff --git a/app/templates/views/manage-templates.html b/app/templates/views/manage-templates.html index 8f8b179ce..b6e0f2c83 100644 --- a/app/templates/views/manage-templates.html +++ b/app/templates/views/manage-templates.html @@ -11,13 +11,16 @@ GOV.UK Notify | Manage templates

    Templates

    - {{ banner( - 'Try sending a text message'.format( - url_for(".choose_sms_template", service_id=service_id) - )|safe, - subhead='Next step', - type="tip" - )}} + + {% if not has_jobs %} + {{ banner( + 'Try sending a text message'.format( + url_for(".choose_sms_template", service_id=service_id) + )|safe, + subhead='Next step', + type="tip" + )}} + {% endif %}
    From 26adcc64c170313326528f6c2634df54dd5a5def Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 4 Feb 2016 12:20:24 +0000 Subject: [PATCH 08/10] =?UTF-8?q?Updates=20to=20=E2=80=98send=20SMS?= =?UTF-8?q?=E2=80=99=20page?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Based on discussion with Pete. Make the blue banner an ‘important’ banner (copied from Register to Vote, used because it’s not as boxy and fits on the page better). Remove the back button because you haven’t changed any data yet. If you need to go back you can just press back or start again. Make the filename stand out more. Remove the ‘download example’ link. Will need to revist the best way of doing this. Make text messages consistently 2/3rd width. --- app/assets/images/icon-important-2x.png | Bin 0 -> 918 bytes app/assets/stylesheets/components/banner.scss | 11 +++++++++++ .../stylesheets/components/file-upload.scss | 1 + app/assets/stylesheets/main.scss | 3 +++ app/main/forms.py | 2 +- app/main/views/add_service.py | 4 ++-- app/main/views/dashboard.py | 2 +- app/templates/components/file-upload.html | 4 ++-- app/templates/views/add-service.html | 5 +++-- app/templates/views/check-sms.html | 10 +++++++--- app/templates/views/job.html | 2 +- app/templates/views/manage-templates.html | 2 +- app/templates/views/send-sms.html | 18 ++++++++---------- 13 files changed, 41 insertions(+), 23 deletions(-) create mode 100644 app/assets/images/icon-important-2x.png diff --git a/app/assets/images/icon-important-2x.png b/app/assets/images/icon-important-2x.png new file mode 100644 index 0000000000000000000000000000000000000000..c04e9c2c942b611fd885f4a2ba48f2c43d7a3dcf GIT binary patch literal 918 zcmeAS@N?(olHy`uVBq!ia0vp^E+EXo3?%u_CO0rJFmea@gt-3y{~stq3jj1ZH8nLi zH}~@8%V}w8j~+dG`SRt54XMR@@bK`sxHx8J=74~JTeoiAzI|I>UY?7KOGHG3m6f%=zW&XdH+Syb z2?`2gWMtgEdv`@e#jjt#baZr%963@~SNG}Dr`4-h_w@8knl#DB$45|5@Y%Cx8#iv$ z)YLqE`t+hji}?BZSy)(XZEXt*3UqaKuXuHQ2L^;}NswPKgUa^c)%QQLpI`9kr_qe} zHgDJ(ekh-b`B&Hx(%e!cH!qxlfl1cW#WAEJZtacsb&m`LS|1v!Ou69HaDdhC!MA&_ zzul|8|Gj?FRioWh5;(KYt?VF=3PbUtiNXerQx43@dLrWE>12>DWyvh;#q%si`FTRp z$_B~o6rN>|I@V;SJn1-+pl4ddsWwgEOskvTftJ4bnza+N4OBS~#N3^|gGu;=z&*A} zhoZIE?)7-Eyn9z$F!6Oo&4&+x(`@UQmc6l9G2iaA+Pvly)^5ivg7xDrO+46sef699 z*Gnos8vc4__UW3bo9gjrJFDE;t@cOl+232UAe{T3ro4UTW8
    diff --git a/app/templates/views/add-service.html b/app/templates/views/add-service.html index eb1e4af9c..27074cf92 100644 --- a/app/templates/views/add-service.html +++ b/app/templates/views/add-service.html @@ -16,7 +16,8 @@ GOV.UK Notify | Set up service

    - Users will see your service name: + Users will see your service name when they receive messages through GOV.UK + Notify:

      @@ -30,7 +31,7 @@ GOV.UK Notify | Set up service
    - {{ textbox(form.name) }} + {{ textbox(form.name, hint="You can change this later") }} {{ page_footer('Continue') }} diff --git a/app/templates/views/check-sms.html b/app/templates/views/check-sms.html index 419eb2fe3..61950804b 100644 --- a/app/templates/views/check-sms.html +++ b/app/templates/views/check-sms.html @@ -21,9 +21,13 @@ {% else %} - {{ sms_message( - message_template|replace_placeholders(upload_result.valid[0]) - )}} +
    +
    + {{ sms_message( + message_template|replace_placeholders(upload_result.valid[0]) + )}} +
    +
    diff --git a/app/templates/views/job.html b/app/templates/views/job.html index 0a2e86ca2..2891e73da 100644 --- a/app/templates/views/job.html +++ b/app/templates/views/job.html @@ -25,7 +25,7 @@
    • {{ big_number( - counts.total, 'queued' + 1, 'queued' )}}
    • diff --git a/app/templates/views/manage-templates.html b/app/templates/views/manage-templates.html index b6e0f2c83..a5c1519c9 100644 --- a/app/templates/views/manage-templates.html +++ b/app/templates/views/manage-templates.html @@ -14,7 +14,7 @@ GOV.UK Notify | Manage templates {% if not has_jobs %} {{ banner( - 'Try sending a text message'.format( + 'Send yourself a text message'.format( url_for(".choose_sms_template", service_id=service_id) )|safe, subhead='Next step', diff --git a/app/templates/views/send-sms.html b/app/templates/views/send-sms.html index e227a3185..2f5c62935 100644 --- a/app/templates/views/send-sms.html +++ b/app/templates/views/send-sms.html @@ -13,25 +13,23 @@ - {{ sms_message(template.content) }} +
      +
      + {{ sms_message(template.content) }} +
      +
      - {{file_upload(form.file)}} - -

      - Download an example CSV file -

      + {{file_upload(form.file, button_text='Choose a CSV file')}} {{ banner( 'You can only send messages to yourself until you request to go live'.format( url_for('.service_request_to_go_live', service_id=service_id) )|safe, - type='info' + type='important' ) }} {{ page_footer( - "Continue to preview", - back_link=url_for(".choose_sms_template", service_id=service_id), - back_link_text="Back" + "Continue to preview" ) }}
      • From 7516ec6abaf784c9ee6a39fec0a567f13c7bf8bc Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 4 Feb 2016 12:38:35 +0000 Subject: [PATCH 09/10] Fake phone number on job page To show what kind of data we want to surface here. --- app/templates/views/job.html | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/app/templates/views/job.html b/app/templates/views/job.html index 2891e73da..eda5d8875 100644 --- a/app/templates/views/job.html +++ b/app/templates/views/job.html @@ -14,9 +14,13 @@ {{ uploaded_file_name }} - {{ sms_message( - template['content'], - )}} +
      +
      + {{ sms_message( + template['content'], + )}} +
      +

      Started {{ uploaded_file_time|format_datetime }} @@ -47,27 +51,29 @@

    {% call(item) list_table( - messages, + [ + {'phone': '+447700 900995', 'template': template['name'], 'status': 'queued'} + ], caption=uploaded_file_name, caption_visible=False, empty_message="Messages go here", field_headings=[ - 'To', - 'Message', + 'Recipient', + 'Template', right_aligned_field_heading('Status') ] ) %} {% call field() %} - {{item.phone}} + {{item.phone}} {% endcall %} {% call field() %} - {{item.message[:50]}}… + {{item.template}} {% endcall %} {% call field( align='right', status='error' if item.status == 'Failed' else 'default' ) %} - {{ item.status }} {{ item.time }} + {{ item.status }} {% endcall %} {% endcall %} From a7d6d85d8bd977ad7f569240397fb79ea609bb4a Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 4 Feb 2016 14:02:16 +0000 Subject: [PATCH 10/10] =?UTF-8?q?Make=20first=20page=20of=20=E2=80=98send?= =?UTF-8?q?=20texts=E2=80=99=20use=20links=20not=20form?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This doesn’t need to be a form—it’s not changing any data. And having the primary action on the page as ‘Use this template’ it makes it clear what the page is for. --- .../stylesheets/components/sms-message.scss | 7 ++++++- app/main/views/sms.py | 7 +------ app/templates/views/choose-sms-template.html | 17 ++++++++++------- tests/app/main/views/test_sms.py | 18 ------------------ tests/app/main/views/test_templates.py | 3 ++- 5 files changed, 19 insertions(+), 33 deletions(-) diff --git a/app/assets/stylesheets/components/sms-message.scss b/app/assets/stylesheets/components/sms-message.scss index 268225eac..d019a2b60 100644 --- a/app/assets/stylesheets/components/sms-message.scss +++ b/app/assets/stylesheets/components/sms-message.scss @@ -24,7 +24,7 @@ .sms-message-name { @include bold-19; - margin: 30px 0 10px 0; + margin: 20px 0 10px 0; } .sms-message-picker { @@ -61,3 +61,8 @@ label.sms-message-option { } } + +.sms-message-use-link { + margin-top: 70px; + @include bold-19; +} diff --git a/app/main/views/sms.py b/app/main/views/sms.py index cf82770ac..07aefbff5 100644 --- a/app/main/views/sms.py +++ b/app/main/views/sms.py @@ -32,13 +32,8 @@ from app.main.utils import ( ) -@main.route("/services//sms/send", methods=['GET', 'POST']) +@main.route("/services//sms/send", methods=['GET']) def choose_sms_template(service_id): - if request.method == 'POST': - return redirect(url_for('.send_sms', - service_id=service_id, - template_id=request.form.get('template'))) - try: templates = templates_dao.get_service_templates(service_id)['data'] except HTTPError as e: diff --git a/app/templates/views/choose-sms-template.html b/app/templates/views/choose-sms-template.html index 3c00892d9..9478c45ab 100644 --- a/app/templates/views/choose-sms-template.html +++ b/app/templates/views/choose-sms-template.html @@ -14,15 +14,18 @@
    {% if templates %} -
    +
    {% for template in templates %} - {{ sms_message( - template.content, name=template.name, input_name='template', input_index=template.id - ) }} +
    + {{ sms_message(template.content, name=template.name) }} +
    +
    +
    + Use this template +
    +
    {% endfor %} -
    - - {{ page_footer("Continue") }} +
    {% else %} {{ banner( 'Add a text message template to start sending messages'.format( diff --git a/tests/app/main/views/test_sms.py b/tests/app/main/views/test_sms.py index fd79dcf08..b42e6d3af 100644 --- a/tests/app/main/views/test_sms.py +++ b/tests/app/main/views/test_sms.py @@ -23,24 +23,6 @@ def test_choose_sms_template(app_, assert 'template two content' in content -def test_choose_sms_template_redirects(app_, - api_user_active, - mock_get_user, - mock_get_service_templates, - mock_check_verify_code, - mock_get_service_template): - with app_.test_request_context(): - with app_.test_client() as client: - client.login(api_user_active) - response = client.post( - url_for('main.choose_sms_template', service_id=12345), - data={'template': '54321'} - ) - - assert response.status_code == 302 - assert response.location == url_for('main.send_sms', service_id=12345, template_id=54321, _external=True) - - def test_upload_empty_csvfile_returns_to_upload_page(app_, api_user_active, mock_get_user, diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py index 525c3bcb4..fb2548a65 100644 --- a/tests/app/main/views/test_templates.py +++ b/tests/app/main/views/test_templates.py @@ -9,7 +9,8 @@ def test_should_return_list_of_all_templates(app_, mock_get_service_templates, mock_get_user, mock_get_user_by_email, - mock_login): + mock_login, + mock_get_jobs): with app_.test_request_context(): with app_.test_client() as client: client.login(api_user_active)