From 0a337a26633db3a14f5a2321e35d4fc49a43b283 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 7 Jun 2016 14:44:46 +0100 Subject: [PATCH 1/7] On the activity page, only link to template or job MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the notification has come from an API call, the template is the only thing that exists If it’s a job, then we need to tell you name of the file. But you can click though to see the template. --- app/templates/views/notifications.html | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/app/templates/views/notifications.html b/app/templates/views/notifications.html index ec544443b..cee5a602f 100644 --- a/app/templates/views/notifications.html +++ b/app/templates/views/notifications.html @@ -55,12 +55,11 @@ {{ item.to }}

- {{ item.template.name }} - sent from {% if item.job %} - {{ item.job.original_file_name }} + From {{ item.job.original_file_name }} {% else %} - an API call + {{ item.template.name }} + from an API call {% endif %}

{% endcall %} From b1852f4b78da1ec4a0aa3214883327cf6a1b1eb0 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 7 Jun 2016 16:35:03 +0100 Subject: [PATCH 2/7] Make permanent failure status more human MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace with ‘Phone number doesn’t exist’ or ‘Email address doesn’t’ exist. --- app/__init__.py | 28 +++++++++++++++++--------- app/templates/views/notifications.html | 15 ++++++++------ 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/app/__init__.py b/app/__init__.py index cb370773b..e2f4a6f2b 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -229,15 +229,25 @@ def valid_phone_number(phone_number): return False -def format_notification_status(status): - m = {'failed': 'Failed', - 'technical-failure': 'Technical failure', - 'temporary-failure': 'Temporarily failed', - 'permanent-failure': 'Permanently failed', - 'delivered': 'Delivered', - 'sending': 'Sending' - } - return m.get(status, status) +def format_notification_status(status, template_type): + return { + 'email': { + 'failed': 'Failed', + 'technical-failure': 'Technical failure', + 'temporary-failure': 'Temporary failure', + 'permanent-failure': 'Email address does not exist', + 'delivered': 'Delivered', + 'sending': 'Sending' + }, + 'sms': { + 'failed': 'Failed', + 'technical-failure': 'Technical failure', + 'temporary-failure': 'Temporary failure', + 'permanent-failure': 'Phone number does not exist', + 'delivered': 'Delivered', + 'sending': 'Sending' + } + }.get(template_type).get(status, status) @login_manager.user_loader diff --git a/app/templates/views/notifications.html b/app/templates/views/notifications.html index cee5a602f..41bd3b6cb 100644 --- a/app/templates/views/notifications.html +++ b/app/templates/views/notifications.html @@ -46,7 +46,7 @@ caption="Recent activity", caption_visible=False, empty_message='No messages found', - field_headings=['Recipient', 'Status', 'Started'], + field_headings=['Recipient', 'Error', 'Status', 'Started'], field_headings_visible=False ) %} @@ -56,19 +56,22 @@

{% if item.job %} - From {{ item.job.original_file_name }} + From {{ item.job.original_file_name }} {% else %} - {{ item.template.name }} + {{ item.template.name }} from an API call {% endif %}

{% endcall %} - {{ text_field(item.status|format_notification_status) }} + {{ date_field( + (item.updated_at or item.created_at)|format_datetime_short + ) }} - {% call field(align='right') %} - {{ (item.updated_at or item.created_at)|format_datetime_short }} + {% call field(status='', align='right') %} + {{ item.status|format_notification_status(item.template.template_type) }} {% endcall %} + {% endcall %} {{ previous_next_navigation(prev_page, next_page) }} From d213e2cc67b03c96adf3668399bf25546becdd64 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 8 Jun 2016 13:41:02 +0100 Subject: [PATCH 3/7] Give each row in a table a heading MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first columns of our tables are always headings for the subsequent columns, even though they go horizontally. HTML has the `` tag, which doesn’t just have to be used for headings along the top of a table. So this commit changes the first column to be a ``. This then allows us to style these elements differently, specifically making them 50% wide. This makes pages like the dashboard align more nicely. --- app/assets/stylesheets/components/table.scss | 15 +++++++++++++++ app/assets/stylesheets/views/dashboard.scss | 3 +-- app/templates/components/table.html | 12 ++++++++++++ app/templates/views/dashboard/_jobs.html | 16 ++++++++-------- app/templates/views/notifications.html | 10 +++++----- 5 files changed, 41 insertions(+), 15 deletions(-) diff --git a/app/assets/stylesheets/components/table.scss b/app/assets/stylesheets/components/table.scss index efe70d644..7e6f0c8f8 100644 --- a/app/assets/stylesheets/components/table.scss +++ b/app/assets/stylesheets/components/table.scss @@ -1,5 +1,6 @@ .table { margin-bottom: $gutter; + width: 100%; } .table-heading { @@ -13,6 +14,16 @@ } } +.table-row { + th { + width: 52.5%; + + a { + max-height: 1.25em; + } + } +} + %table-field, .table-field { @@ -77,6 +88,10 @@ width: 15px; } + &-date { + white-space: nowrap; + } + p { margin: 0 0 5px 0; } diff --git a/app/assets/stylesheets/views/dashboard.scss b/app/assets/stylesheets/views/dashboard.scss index 1f4d2022e..04c2cc622 100644 --- a/app/assets/stylesheets/views/dashboard.scss +++ b/app/assets/stylesheets/views/dashboard.scss @@ -3,8 +3,7 @@ table { th { @include core-16; - padding-top: 0; - border: 0; + border-bottom: 0; } td { diff --git a/app/templates/components/table.html b/app/templates/components/table.html index 5f96508ce..95ba0264f 100644 --- a/app/templates/components/table.html +++ b/app/templates/components/table.html @@ -55,12 +55,24 @@ {%- endmacro %} +{% macro row_heading() -%} + + {{ caller() }} + +{%- endmacro %} + {% macro index_field(text) -%} {{ text }} {%- endmacro %} +{% macro date_field(text) -%} + + {{ text }} + +{% endmacro %} + {% macro text_field(text) -%} {% call field() %} {{ text }} diff --git a/app/templates/views/dashboard/_jobs.html b/app/templates/views/dashboard/_jobs.html index 26dc0acca..4b21598f8 100644 --- a/app/templates/views/dashboard/_jobs.html +++ b/app/templates/views/dashboard/_jobs.html @@ -1,4 +1,4 @@ -{% from "components/table.html" import list_table, field, right_aligned_field_heading %} +{% from "components/table.html" import list_table, field, right_aligned_field_heading, row_heading %} {% from "components/big-number.html" import big_number %} {% call(item, row_number) list_table( @@ -8,28 +8,28 @@ empty_message='You haven’t sent any batch messages yet', field_headings=[ 'File', - right_aligned_field_heading('Sending'), - right_aligned_field_heading('Delivered'), - right_aligned_field_heading('Failed') + 'Sending', + 'Delivered', + 'Failed' ], field_headings_visible=True ) %} - {% call field() %} + {% call row_heading() %}
{{ item.original_file_name }} Uploaded {{ item.created_at|format_datetime_short }}
{{ item.to }}

{% if item.job %} - From {{ item.job.original_file_name }} + From {{ item.job.original_file_name }} {% else %} - {{ item.template.name }} + {{ item.template.name }} from an API call {% endif %}

From c31762265cd3c8e2815c911eb0a0fb620b9d004d Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 9 Jun 2016 10:15:37 +0100 Subject: [PATCH 4/7] Colour code notification statuses Makes the table easier to scan. Delivered stays as before. Sending is greyed out. All other statuses are failure, and stand out by being red and bold. --- app/__init__.py | 12 ++++++++++++ app/templates/views/notifications.html | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/app/__init__.py b/app/__init__.py index e2f4a6f2b..d4211a033 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -111,6 +111,7 @@ def create_app(): application.add_template_filter(format_date_normal) application.add_template_filter(format_date_short) application.add_template_filter(format_notification_status) + application.add_template_filter(format_notification_status_as_field_status) application.after_request(useful_headers_after_request) application.after_request(save_service_after_request) @@ -250,6 +251,17 @@ def format_notification_status(status, template_type): }.get(template_type).get(status, status) +def format_notification_status_as_field_status(status): + return { + 'failed': 'error', + 'technical-failure': 'error', + 'temporary-failure': 'error', + 'permanent-failure': 'error', + 'delivered': None, + 'sending': 'default' + }.get(status, 'error') + + @login_manager.user_loader def load_user(user_id): return user_api_client.get_user(user_id) diff --git a/app/templates/views/notifications.html b/app/templates/views/notifications.html index ff70f7cc6..c42216def 100644 --- a/app/templates/views/notifications.html +++ b/app/templates/views/notifications.html @@ -68,7 +68,7 @@ (item.updated_at or item.created_at)|format_datetime_short ) }} - {% call field(status='', align='right') %} + {% call field(status=item.status|format_notification_status_as_field_status, align='right') %} {{ item.status|format_notification_status(item.template.template_type) }} {% endcall %} From 2e8e650733755ea8d32024ba955208afc355fd00 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 9 Jun 2016 10:25:52 +0100 Subject: [PATCH 5/7] Put same info in both tables of notifications MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We have tables listing notifications on: - the job page - the ‘activity’ page Previously that had subtly different information, in a different order. This commit makes them exactly the same. --- app/assets/stylesheets/components/table.scss | 1 + app/templates/partials/jobs/notifications.html | 15 ++++++++++----- tests/app/main/views/test_jobs.py | 6 ++++-- 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/app/assets/stylesheets/components/table.scss b/app/assets/stylesheets/components/table.scss index 7e6f0c8f8..45a86e33e 100644 --- a/app/assets/stylesheets/components/table.scss +++ b/app/assets/stylesheets/components/table.scss @@ -17,6 +17,7 @@ .table-row { th { width: 52.5%; + font-weight: normal; a { max-height: 1.25em; diff --git a/app/templates/partials/jobs/notifications.html b/app/templates/partials/jobs/notifications.html index 2dd4ecdc8..9241aa83f 100644 --- a/app/templates/partials/jobs/notifications.html +++ b/app/templates/partials/jobs/notifications.html @@ -1,4 +1,4 @@ -{% from "components/table.html" import list_table, field, right_aligned_field_heading %} +{% from "components/table.html" import list_table, field, right_aligned_field_heading, date_field, row_heading %}
Date: Thu, 9 Jun 2016 11:05:54 +0100 Subject: [PATCH 6/7] =?UTF-8?q?Truncate=20items=20that=20don=E2=80=99t=20f?= =?UTF-8?q?it=20in=20the=20first=20column?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first column of a table is a heading, and will always be 50% wide. It makes the table harder to scan when the information in the first column breaks onto multiple lines, and introduces uneven whitespace in the table. This commit adds some CSS to force things in the first column to only ever be one line. If they are too long to fit, they get truncated with an ellipsis (`…`) --- app/assets/stylesheets/components/table.scss | 12 ++++++++++-- app/assets/stylesheets/views/dashboard.scss | 4 ++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/components/table.scss b/app/assets/stylesheets/components/table.scss index 45a86e33e..05079605f 100644 --- a/app/assets/stylesheets/components/table.scss +++ b/app/assets/stylesheets/components/table.scss @@ -1,6 +1,7 @@ .table { margin-bottom: $gutter; width: 100%; + table-layout: fixed; } .table-heading { @@ -16,11 +17,14 @@ .table-row { th { + display: table-cell; width: 52.5%; font-weight: normal; - a { - max-height: 1.25em; + .hint { + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; } } } @@ -101,6 +105,10 @@ .table-field-heading { + &-first { + width: 52.5%; + } + &:last-child { padding-right: 0; } diff --git a/app/assets/stylesheets/views/dashboard.scss b/app/assets/stylesheets/views/dashboard.scss index 04c2cc622..6069a8f20 100644 --- a/app/assets/stylesheets/views/dashboard.scss +++ b/app/assets/stylesheets/views/dashboard.scss @@ -58,6 +58,10 @@ &-filename { @include bold-19; + display: block; + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; } &-hint { From 04ef730fd191507b513d32d2601929eacc1c8e8d Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 9 Jun 2016 11:26:45 +0100 Subject: [PATCH 7/7] Remove redundant `if` statement on job page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We had this if statement to lay out the table differently with and without row numbers. Since we don’t show row numbers at all, this isn’t needed. --- .../partials/jobs/notifications.html | 73 ++++++------------- 1 file changed, 24 insertions(+), 49 deletions(-) diff --git a/app/templates/partials/jobs/notifications.html b/app/templates/partials/jobs/notifications.html index 9241aa83f..cb87d6280 100644 --- a/app/templates/partials/jobs/notifications.html +++ b/app/templates/partials/jobs/notifications.html @@ -9,55 +9,30 @@ {% endif %} > -
- {% if notifications|length > 0 and notifications[0].job_row_number is not none %} - {% call(item, row_number) list_table( - notifications, - caption=uploaded_file_name, - caption_visible=False, - empty_message="No messages to show yet", - field_headings=[ - 'Recipient', - 'Time', - right_aligned_field_heading('Status') - ], - field_headings_visible=False - ) %} - {% call row_heading() %} - {{ item.to }} - {% endcall %} - {{ date_field( - (item.updated_at or item.created_at)|format_datetime_short - ) }} - {% call field( - align='right', - status=item.status|format_notification_status_as_field_status - ) %} - {{ item.status|format_notification_status(item.template.template_type) }} - {% endcall %} - {% endcall %} - {% else %} - {% call(item, row_number) list_table( - notifications, - caption=uploaded_file_name, - caption_visible=False, - empty_message="No messages to show yet", - field_headings=[ - 'Recipient', - right_aligned_field_heading('Status') - ] - ) %} - {% call field() %} - {{ item.to }} - {% endcall %} - {% call field( - align='right', - status='error' if item.status == 'Failed' else 'default' - ) %} - {{ item.status|format_notification_status }} at {{ item.updated_at|format_time }} - {% endcall %} + {% call(item, row_number) list_table( + notifications, + caption=uploaded_file_name, + caption_visible=False, + empty_message="No messages to show yet", + field_headings=[ + 'Recipient', + 'Time', + 'Status' + ], + field_headings_visible=False + ) %} + {% call row_heading() %} + {{ item.to }} {% endcall %} - {% endif %} -
+ {{ date_field( + (item.updated_at or item.created_at)|format_datetime_short + ) }} + {% call field( + align='right', + status=item.status|format_notification_status_as_field_status + ) %} + {{ item.status|format_notification_status(item.template.template_type) }} + {% endcall %} + {% endcall %}