From 16d83faa7217ed74a6846f8d3119a28d81db9360 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 25 May 2016 13:36:35 +0100 Subject: [PATCH] Put uploaded files on the dashboard This commit depends on and uses the data returned by: - [x] https://github.com/alphagov/notifications-api/pull/345 - [x] https://github.com/alphagov/notifications-api/pull/347 - [x] https://github.com/alphagov/notifications-admin/pull/612 It puts the last 5 jobs on the dashboard. This should be changed to all the jobs from the last 7 days when that parameter is available. It also: - links to the jobs page - makes the numbers on the jobs page consistent with the dashboard - makes the numbers on an individual job consistent with the appearance of the dashboard --- app/__init__.py | 2 +- app/assets/stylesheets/views/dashboard.scss | 31 ++++++++++-- app/main/views/dashboard.py | 2 + app/main/views/jobs.py | 36 +++++++------- app/templates/components/big-number.html | 2 +- app/templates/partials/jobs/count.html | 13 +++-- .../partials/jobs/notifications.html | 4 +- app/templates/partials/jobs/status.html | 6 +-- app/templates/views/dashboard/_jobs.html | 35 ++++++++++++++ app/templates/views/dashboard/jobs.html | 3 +- app/templates/views/dashboard/today.html | 13 ++++- app/templates/views/jobs/jobs.html | 40 ++-------------- tests/app/main/views/test_jobs.py | 48 ++++++------------- tests/conftest.py | 2 +- 14 files changed, 130 insertions(+), 107 deletions(-) create mode 100644 app/templates/views/dashboard/_jobs.html diff --git a/app/__init__.py b/app/__init__.py index 643a9cd02..2f86e12d4 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -202,7 +202,7 @@ def format_datetime_normal(date): def format_datetime_short(date): - return gmt_timezones(date).strftime('%d %B at %H:%M') + return gmt_timezones(date).strftime('%d %B at %H:%M').lstrip('0') def format_time(date): diff --git a/app/assets/stylesheets/views/dashboard.scss b/app/assets/stylesheets/views/dashboard.scss index a2697f7b3..1f4d2022e 100644 --- a/app/assets/stylesheets/views/dashboard.scss +++ b/app/assets/stylesheets/views/dashboard.scss @@ -1,6 +1,16 @@ .dashboard { - table th { - font-size: 0; + + table { + th { + @include core-16; + padding-top: 0; + border: 0; + } + + td { + @include core-19; + border: 0; + } } } @@ -15,12 +25,9 @@ box-sizing: border-box; display: block; width: 100%; - text-align: right; margin-bottom: $gutter-half; height: $gutter-half; color: $govuk-blue; - text-align: left; - color: $govuk-blue; span { box-sizing: border-box; @@ -47,3 +54,17 @@ } } + +.file-list { + + &-filename { + @include bold-19; + } + + &-hint { + @include core-16; + display: block; + color: $secondary-text-colour; + } + +} diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index 685d5d400..2ef455e9a 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -16,6 +16,7 @@ from flask_login import login_required from app.main import main from app import ( + job_api_client, statistics_api_client, service_api_client, template_statistics_client @@ -217,4 +218,5 @@ def get_dashboard_statistics_for_service(service_id): 'sms_allowance_remaining': max(0, (sms_free_allowance - sms_sent)), 'sms_chargeable': max(0, sms_sent - sms_free_allowance), 'sms_rate': sms_rate, + 'jobs': job_api_client.get_job(service_id, limit_days=7)['data'] } diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index 7ac4cbe2e..108281368 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -90,12 +90,13 @@ def view_job(service_id, job_id): return render_template( 'views/jobs/job.html', notifications=notifications['notifications'], - counts={ - 'queued': 0 if finished else job['notification_count'], - 'sent': job['notification_count'] if finished else 0 - }, + job=job, uploaded_at=job['created_at'], - finished_at=job['updated_at'] if finished else None, + finished=job.get('notifications_sent', 0) and (( + job.get('notifications_sent', 0) - + job.get('notifications_delivered', 0) - + job.get('notifications_failed', 0) + ) == 0), uploaded_file_name=job['original_file_name'], first_email_template=[ template for template in service_api_client.get_service_templates(service_id)['data'] @@ -104,9 +105,7 @@ def view_job(service_id, job_id): template=Template( template, prefix=current_service['name'] - ), - job_id=job_id, - created_by=job['created_by']['name'] + ) ) @@ -116,24 +115,27 @@ def view_job(service_id, job_id): def view_job_updates(service_id, job_id): job = job_api_client.get_job(service_id, job_id)['data'] notifications = notification_api_client.get_notifications_for_service(service_id, job_id) - finished = job['status'] == 'finished' + finished = ( + job.get('notifications_sent', 0) - + job.get('notifications_delivered', 0) - + job.get('notifications_failed', 0) + ) == 0 return jsonify(**{ 'counts': render_template( 'partials/jobs/count.html', - counts={ - 'queued': 0 if finished else job['notification_count'], - 'sent': job['notification_count'] if finished else 0 - } + job=job, + finished=finished ), 'notifications': render_template( 'partials/jobs/notifications.html', - notifications=notifications['notifications'] + job=job, + notifications=notifications['notifications'], + finished=finished ), 'status': render_template( 'partials/jobs/status.html', - uploaded_at=job['created_at'], - finished_at=job['updated_at'] if finished else None, - created_by=job['created_by']['name'] + job=job, + finished=finished ), }) diff --git a/app/templates/components/big-number.html b/app/templates/components/big-number.html index b52f98c6f..3a2991b0f 100644 --- a/app/templates/components/big-number.html +++ b/app/templates/components/big-number.html @@ -14,7 +14,7 @@ {% if label_link %} {{ label }} - {% else %} + {% elif label %} {{ label }} {% endif %} diff --git a/app/templates/partials/jobs/count.html b/app/templates/partials/jobs/count.html index 78583a7f6..38ea4e5e7 100644 --- a/app/templates/partials/jobs/count.html +++ b/app/templates/partials/jobs/count.html @@ -1,9 +1,9 @@ {% from "components/big-number.html" import big_number %}
  • {{ big_number( - counts.queued, 'queued' + job.get('notifications_sent', 0) - job.get('notifications_delivered', 0) - job.get('notifications_failed', 0), 'sending' )}}
  • {{ big_number( - counts.sent, 'processed' + job.get('notifications_delivered', 0), 'delivered' + )}} +
    • +
  • + {{ big_number( + job.notifications_failed, 'failed' )}}
    • diff --git a/app/templates/partials/jobs/notifications.html b/app/templates/partials/jobs/notifications.html index 4c0159efa..0bbdd2b46 100644 --- a/app/templates/partials/jobs/notifications.html +++ b/app/templates/partials/jobs/notifications.html @@ -1,9 +1,9 @@ {% from "components/table.html" import list_table, field, right_aligned_field_heading %}

    - Sent by {{ created_by }} on {{ uploaded_at|format_datetime_short }} + Uploaded by {{ job.created_by.name }} on {{ job.created_at|format_datetime_short }}

    diff --git a/app/templates/views/dashboard/_jobs.html b/app/templates/views/dashboard/_jobs.html new file mode 100644 index 000000000..26dc0acca --- /dev/null +++ b/app/templates/views/dashboard/_jobs.html @@ -0,0 +1,35 @@ +{% from "components/table.html" import list_table, field, right_aligned_field_heading %} +{% from "components/big-number.html" import big_number %} + +{% call(item, row_number) list_table( + jobs, + caption="Recent batch jobs", + caption_visible=False, + 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') + ], + field_headings_visible=True +) %} + {% call field() %} +
    + {{ item.original_file_name }} + Uploaded {{ item.created_at|format_datetime_short }} +
    {{ item.original_file_name }} + {{ item.created_at|format_datetime }} {% endcall %} {% call field() %} - {{ item.created_at|format_datetime }} + {% endcall %} {% call field(align='right') %} {{ item.notification_count }} diff --git a/app/templates/views/dashboard/today.html b/app/templates/views/dashboard/today.html index b3058951e..1b34d6e76 100644 --- a/app/templates/views/dashboard/today.html +++ b/app/templates/views/dashboard/today.html @@ -1,6 +1,7 @@ {% from "components/big-number.html" import big_number, big_number_with_status %} {% from "components/show-more.html" import show_more %} {% from "components/message-count-label.html" import message_count_label %} +{% from "components/table.html" import list_table, field, right_aligned_field_heading, hidden_field_heading %}
    - + {% if template_statistics|length %} {% include 'views/dashboard/template-statistics.html' %} {{ show_more( @@ -48,8 +49,16 @@ ) }} {% endif %} + {% if jobs %} + {% include 'views/dashboard/_jobs.html' %} + {{ show_more( + url_for('.view_jobs', service_id=current_service.id), + 'See all uploaded files' + ) }} + {% endif %} + {% if current_user.has_permissions(['manage_settings'], admin_override=True) %} -

    This year

    +

    This year

    diff --git a/app/templates/views/jobs/jobs.html b/app/templates/views/jobs/jobs.html index 58c26038a..7745a241b 100644 --- a/app/templates/views/jobs/jobs.html +++ b/app/templates/views/jobs/jobs.html @@ -1,44 +1,12 @@ {% extends "withnav_template.html" %} -{% from "components/table.html" import list_table, field, right_aligned_field_heading %} {% block page_title %} Notifications activity – GOV.UK Notify {% endblock %} {% block maincolumn_content %} - -

    Notifications activity

    -

    - All messages -

    -

    - Text messages  - Email messages -

    -

    - Successful messages  - Failed messages -

    - {% call(item, row_number) list_table( - jobs, - caption="Recent activity", - caption_visible=False, - empty_message='You haven’t sent any notifications yet', - field_headings=['Job', 'Time', 'Uploaded by', right_aligned_field_heading('Status')] - ) %} - {% call field() %} - {{ item.original_file_name }} - {% endcall %} - {% call field() %} - {{ item.created_at | format_datetime}} - {% endcall %} - {% call field() %} - {{ item.created_by.name }} - {% endcall %} - {% call field(align='right') %} - {{ item.status }} - {% endcall %} - {% endcall %} - - +

    Uploaded files

    +
    + {% include 'views/dashboard/_jobs.html' %} +
    {% endblock %} diff --git a/tests/app/main/views/test_jobs.py b/tests/app/main/views/test_jobs.py index 3326d0f92..3e5b1268e 100644 --- a/tests/app/main/views/test_jobs.py +++ b/tests/app/main/views/test_jobs.py @@ -20,7 +20,7 @@ def test_should_return_list_of_all_jobs(app_, assert response.status_code == 200 page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - assert page.h1.string == 'Notifications activity' + assert page.h1.string == 'Uploaded files' jobs = page.tbody.find_all('tr') assert len(jobs) == 5 @@ -49,6 +49,7 @@ def test_should_show_page_for_one_job( assert "Delivered at 11:10" in content +@freeze_time("2016-01-01 11:09:00.061258") def test_should_show_updates_for_one_job_as_json( app_, service_one, @@ -66,12 +67,15 @@ def test_should_show_updates_for_one_job_as_json( assert response.status_code == 200 content = json.loads(response.get_data(as_text=True)) - assert 'processed' in content['counts'] - assert 'queued' in content['counts'] + assert 'sending' in content['counts'] + assert 'delivered' in content['counts'] + assert 'failed' in content['counts'] assert 'Recipient' in content['notifications'] + assert '07123456789' in content['notifications'] assert 'Status' in content['notifications'] - assert 'Sent by Test User' in content['status'] assert job_json['status'] in content['status'] + assert 'Delivered at 11:10' in content['notifications'] + assert 'Uploaded by Test User on 1 January at 11:09' in content['status'] def test_should_show_notifications_for_a_service(app_, @@ -90,7 +94,7 @@ def test_should_show_notifications_for_a_service(app_, assert notification['to'] in content assert notification['status'] in content assert notification['template']['name'] in content - assert 'Download as a CSV file' in content + assert 'csv' in content page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert page.h1.string.strip() == 'Activity' @@ -118,7 +122,7 @@ def test_can_view_only_sms_notifications_for_a_service(app_, assert notification['to'] in content assert notification['status'] in content assert notification['template']['name'] in content - assert 'Download as a CSV file' in content + assert 'csv' in content page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert page.h1.string.strip() == 'Text messages' @@ -146,7 +150,7 @@ def test_can_view_only_email_notifications_for_a_service(app_, assert notification['to'] in content assert notification['status'] in content assert notification['template']['name'] in content - assert 'Download as a CSV file' in content + assert 'csv' in content page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert page.h1.string.strip() == 'Emails' @@ -172,7 +176,7 @@ def test_can_view_successful_notifications_for_a_service(app_, assert notification['to'] in content assert notification['status'] in content assert notification['template']['name'] in content - assert 'Download as a CSV file' in content + assert 'csv' in content page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert page.h1.string.strip() == 'Successful emails and text messages' @@ -198,7 +202,7 @@ def test_can_view_failed_notifications_for_a_service(app_, assert notification['to'] in content assert notification['status'] in content assert notification['template']['name'] in content - assert 'Download as a CSV file' in content + assert 'csv' in content page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert page.h1.string.strip() == 'Failed emails and text messages' @@ -264,30 +268,6 @@ def test_should_download_notifications_for_a_service(app_, assert 'text/csv' in response.headers['Content-Type'] -def test_should_download_api_notifications_for_a_service(app_, - api_user_active, - mock_login, - mock_get_service, - mock_get_job, - mock_get_notifications, - mock_get_template_version, - mock_has_permissions, - fake_uuid): - with app_.test_request_context(): - with app_.test_client() as client: - client.login(api_user_active) - response = client.get(url_for( - 'main.view_notifications', - service_id=fake_uuid, - download='csv')) - notifications = mock_get_notifications(fake_uuid)['notifications'] - assert all([x['job'] is None for x in notifications]) - csv_content = generate_notifications_csv(notifications) - assert response.status_code == 200 - assert response.get_data(as_text=True) == csv_content - assert 'text/csv' in response.headers['Content-Type'] - - @freeze_time("2016-01-01 11:09:00.061258") def test_should_download_notifications_for_a_job(app_, api_user_active, @@ -311,4 +291,4 @@ def test_should_download_notifications_for_a_job(app_, assert response.status_code == 200 assert response.get_data(as_text=True) == csv_content assert 'text/csv' in response.headers['Content-Type'] - assert 'sample template - 01 January at 11:09.csv"' in response.headers['Content-Disposition'] + assert 'sample template - 1 January at 11:09.csv"' in response.headers['Content-Disposition'] diff --git a/tests/conftest.py b/tests/conftest.py index 037f3814b..db0061da7 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -794,7 +794,7 @@ def mock_get_job(mocker, api_user_active): @pytest.fixture(scope='function') def mock_get_jobs(mocker, api_user_active): - def _get_jobs(service_id): + def _get_jobs(service_id, limit_days=None): data = [] for i in range(5): job_data = job_json(service_id, api_user_active)