Merge pull request #667 from alphagov/make-tables-line-up

Make tables line up

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)
@@ -229,15 +230,36 @@ 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)
+
+
+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

app/assets/stylesheets/components/table.scss

@@ -1,5 +1,7 @@
 .table {
   margin-bottom: $gutter;
+  width: 100%;
+  table-layout: fixed;
 }
 
 .table-heading {
@@ -13,6 +15,20 @@
   }
 }
 
+.table-row {
+  th {
+    display: table-cell;
+    width: 52.5%;
+    font-weight: normal;
+
+    .hint {
+      white-space: nowrap;
+      overflow: hidden;
+      text-overflow: ellipsis;
+    }
+  }
+}
+
 %table-field,
 .table-field {
 
@@ -77,6 +93,10 @@
     width: 15px;
   }
 
+  &-date {
+    white-space: nowrap;
+  }
+
   p {
     margin: 0 0 5px 0;
   }
@@ -85,6 +105,10 @@
 
 .table-field-heading {
 
+  &-first {
+    width: 52.5%;
+  }
+
   &:last-child {
     padding-right: 0;
   }

app/assets/stylesheets/views/dashboard.scss

@@ -3,8 +3,7 @@
   table {
     th {
       @include core-16;
-      padding-top: 0;
-      border: 0;
+      border-bottom: 0;
     }
 
     td {
@@ -59,6 +58,10 @@
 
   &-filename {
     @include bold-19;
+    display: block;
+    white-space: nowrap;
+    overflow: hidden;
+    text-overflow: ellipsis;
   }
 
   &-hint {

app/templates/components/table.html

@@ -55,12 +55,24 @@
   </td>
 {%- endmacro %}
 
+{% macro row_heading() -%}
+  <th class="table-field">
+    <span>{{ caller() }}</span>
+  </th>
+{%- endmacro %}
+
 {% macro index_field(text) -%}
   <td class="table-field-index">
     <span>{{ text }}</span>
   </td>
 {%- endmacro %}
 
+{% macro date_field(text) -%}
+  <td class="table-field-date">
+    <span>{{ text }}</span>
+  </td>
+{% endmacro %}
+
 {% macro text_field(text) -%}
   {% call field() %}
     {{ text }}

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 %}
 
 <div
   {% if not finished %}
@@ -9,50 +9,30 @@
   {% endif %}
 >
 
-  <div>
-    {% 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',
-          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 or item.created_at)|format_time }}
-        {% 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 %}
-  </div>
+    {{ 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 %}
 
 </div>

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() %}
     <div class="file-list">
       <a class="file-list-filename" href="{{ url_for('.view_job', service_id=current_service.id, job_id=item.id) }}">{{ item.original_file_name }}</a>
       <span class="file-list-hint">Uploaded {{ item.created_at|format_datetime_short }}</span>
     </div
   {% endcall %}
-  {% call field(align='right') %}
+  {% call field() %}
     {{ big_number(
       item.get('notifications_sent', 0) - item.get('notifications_delivered', 0) - item.get('notifications_failed', 0),
       smaller=True
     ) }}
   {% endcall %}
-  {% call field(align='right') %}
+  {% call field() %}
     {{ big_number(item.get('notifications_delivered', 0), smaller=True) }}
   {% endcall %}
-  {% call field(align='right', status='error' if 0 else '') %}
+  {% call field(status='error' if 0 else '') %}
     {{ big_number(item.get('notifications_failed', 0), smaller=True) }}
   {% endcall %}
 {% endcall %}

app/templates/views/notifications.html

@@ -1,5 +1,5 @@
 {% extends "withnav_template.html" %}
-{% from "components/table.html" import list_table, field, text_field, link_field, right_aligned_field_heading, hidden_field_heading %}
+{% from "components/table.html" import list_table, field, text_field, link_field, right_aligned_field_heading, hidden_field_heading, row_heading, date_field %}
 {% from "components/previous-next-navigation.html" import previous_next_navigation %}
 {% from "components/page-footer.html" import page_footer %}
 {% from "components/pill.html" import pill %}
@@ -46,30 +46,32 @@
     caption="Recent activity",
     caption_visible=False,
     empty_message='No messages found',
-    field_headings=['Recipient', 'Status', 'Started'],
+    field_headings=['Recipient', 'Started', 'Status'],
     field_headings_visible=False
   ) %}
 
-    {% call field() %}
+    {% call row_heading() %}
       <p>
         {{ item.to }}
       </p>
       <p class="hint">
-        <a href="{{ url_for('.view_template_version', service_id=current_service.id, template_id=item.template.id, version=item.template_version) }}">{{ item.template.name }}</a>
-        sent from
         {% if item.job %}
-          <a href="{{ url_for(".view_job", service_id=current_service.id, job_id=item.job.id) }}">{{ item.job.original_file_name }}</a>
+          From <a href="{{ url_for(".view_job", service_id=current_service.id, job_id=item.job.id) }}">{{ item.job.original_file_name }}</a>
         {% else %}
-          an API call
+          <a href="{{ url_for('.view_template_version', service_id=current_service.id, template_id=item.template.id, version=item.template_version) }}">{{ item.template.name }}</a>
+          from an API call
         {% endif %}
       </p>
     {% 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=item.status|format_notification_status_as_field_status, align='right') %}
+      {{ item.status|format_notification_status(item.template.template_type) }}
     {% endcall %}
+
   {% endcall %}
 
   {{ previous_next_navigation(prev_page, next_page) }}

tests/app/main/views/test_jobs.py

@@ -47,7 +47,8 @@ def test_should_show_page_for_one_job(
         content = response.get_data(as_text=True)
         assert "{}: Your vehicle tax is about to expire".format(service_one['name']) in content
         assert file_name in content
-        assert "Delivered at 11:10" in content
+        assert 'Delivered' in content
+        assert '11:10' in content
 
 
 @freeze_time("2016-01-01 11:09:00.061258")
@@ -75,7 +76,8 @@ def test_should_show_updates_for_one_job_as_json(
         assert '07123456789' in content['notifications']
         assert 'Status' in content['notifications']
         assert job_json['status'] in content['status']
-        assert 'Delivered at 11:10' in content['notifications']
+        assert 'Delivered' in content['notifications']
+        assert '11:10' in content['notifications']
         assert 'Uploaded by Test User on 1 January at 11:09' in content['status']