From e3b1813ed7e9f5753c8c1df40a553d99e1ebfa9f Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 3 Jan 2020 14:14:38 +0000 Subject: [PATCH 1/4] Add a test for the no notifications message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This wasn’t tested before. --- tests/app/main/views/test_jobs.py | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/tests/app/main/views/test_jobs.py b/tests/app/main/views/test_jobs.py index 2b3e8f707..477794706 100644 --- a/tests/app/main/views/test_jobs.py +++ b/tests/app/main/views/test_jobs.py @@ -294,7 +294,27 @@ def test_should_show_job_in_progress( service_id=service_one['id'], job_id=fake_uuid, ) - assert page.find('p', {'class': 'hint'}).text.strip() == 'Report is 50% complete…' + assert page.select_one('p.hint').text.strip() == 'Report is 50% complete…' + + +def test_should_show_job_without_notifications( + client_request, + service_one, + active_user_with_permissions, + mock_get_service_template, + mock_get_job_in_progress, + mocker, + mock_get_notifications_with_no_notifications, + mock_get_service_data_retention, + fake_uuid, +): + page = client_request.get( + 'main.view_job', + service_id=service_one['id'], + job_id=fake_uuid, + ) + assert page.select_one('p.hint').text.strip() == 'Report is 50% complete…' + assert page.select_one('tbody').text.strip() == 'No messages to show' @freeze_time("2016-01-01 11:09:00.061258") From d2f976c8eececffa5d6d9c5e466091ffd91527d5 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 30 Dec 2019 16:25:09 +0000 Subject: [PATCH 2/4] =?UTF-8?q?Don=E2=80=99t=20show=20progress=20once=20no?= =?UTF-8?q?tifications=20are=20gone?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We work out how complete a job’s processing is by looking at how many notifications have been created. Later, we deleted all the notifications, according to the data retention schedule. This makes it look like the job has gone back to 0% processed. This commit accounts for this by not showing the % complete message once a finished job has had its notifications deleted. --- .../partials/jobs/notifications.html | 2 +- tests/app/main/views/test_jobs.py | 21 +++++++++++++++++++ tests/conftest.py | 3 ++- 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/app/templates/partials/jobs/notifications.html b/app/templates/partials/jobs/notifications.html index 7ff70cc7f..b11b396cb 100644 --- a/app/templates/partials/jobs/notifications.html +++ b/app/templates/partials/jobs/notifications.html @@ -28,7 +28,7 @@ {% if template.template_type == 'letter' %}
{% endif %} - {% if percentage_complete < 100 %} + {% if percentage_complete < 100 and job.job_status != 'finished' %}

Report is {{ "{:.0f}%".format(percentage_complete * 0.99) }} complete…

diff --git a/tests/app/main/views/test_jobs.py b/tests/app/main/views/test_jobs.py index 477794706..fe68a16b9 100644 --- a/tests/app/main/views/test_jobs.py +++ b/tests/app/main/views/test_jobs.py @@ -317,6 +317,27 @@ def test_should_show_job_without_notifications( assert page.select_one('tbody').text.strip() == 'No messages to show' +def test_should_show_old_job( + client_request, + service_one, + active_user_with_permissions, + mock_get_service_template, + mock_get_job, + mocker, + mock_get_notifications_with_no_notifications, + mock_get_service_data_retention, + fake_uuid, +): + page = client_request.get( + 'main.view_job', + service_id=service_one['id'], + job_id=fake_uuid, + ) + assert not page.select('p.hint') + assert not page.select('a[download]') + assert page.select_one('tbody').text.strip() == 'No messages to show' + + @freeze_time("2016-01-01 11:09:00.061258") def test_should_show_letter_job( client_request, diff --git a/tests/conftest.py b/tests/conftest.py index c5e4b371f..d99f974fc 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1818,7 +1818,8 @@ def mock_get_job_in_progress(mocker, api_user_active): return {"data": job_json( service_id, api_user_active, job_id=job_id, notification_count=10, - notifications_requested=5 + notifications_requested=5, + job_status='processing', )} return mocker.patch('app.job_api_client.get_job', side_effect=_get_job) From 9446b35e3b127e4b4f0765762122c4480c8bdcaa Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 30 Dec 2019 16:37:50 +0000 Subject: [PATCH 3/4] Add a more accurate missing notifications message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Notifications could be missing because: - none have been created yet - they’ve been deleted This commit adds separate error messages for each case, rather than a less helpful generic one. --- app/main/views/jobs.py | 1 + app/templates/partials/jobs/notifications.html | 2 +- tests/app/main/views/test_jobs.py | 6 ++++-- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index ba4972a3b..b7f9a5d70 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -480,6 +480,7 @@ def get_job_partials(job, template): job=job, template=template, template_version=job['template_version'], + service_data_retention_days=service_data_retention_days, ), 'status': render_template( 'partials/jobs/status.html', diff --git a/app/templates/partials/jobs/notifications.html b/app/templates/partials/jobs/notifications.html index b11b396cb..29ee92311 100644 --- a/app/templates/partials/jobs/notifications.html +++ b/app/templates/partials/jobs/notifications.html @@ -47,7 +47,7 @@ notifications, caption=uploaded_file_name, caption_visible=False, - empty_message="No messages to show", + empty_message='These messages have been deleted because they were sent more than {} days ago'.format(service_data_retention_days) if job.job_status == 'finished' else 'No messages to show yet…', field_headings=[ 'Recipient', 'Status' diff --git a/tests/app/main/views/test_jobs.py b/tests/app/main/views/test_jobs.py index fe68a16b9..7832f8b20 100644 --- a/tests/app/main/views/test_jobs.py +++ b/tests/app/main/views/test_jobs.py @@ -314,7 +314,7 @@ def test_should_show_job_without_notifications( job_id=fake_uuid, ) assert page.select_one('p.hint').text.strip() == 'Report is 50% complete…' - assert page.select_one('tbody').text.strip() == 'No messages to show' + assert page.select_one('tbody').text.strip() == 'No messages to show yet…' def test_should_show_old_job( @@ -335,7 +335,9 @@ def test_should_show_old_job( ) assert not page.select('p.hint') assert not page.select('a[download]') - assert page.select_one('tbody').text.strip() == 'No messages to show' + assert page.select_one('tbody').text.strip() == ( + 'These messages have been deleted because they were sent more than 7 days ago' + ) @freeze_time("2016-01-01 11:09:00.061258") From 93d2d47f4e0862708a26d4b6001978b754451dbc Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 30 Dec 2019 16:53:45 +0000 Subject: [PATCH 4/4] =?UTF-8?q?Don=E2=80=99t=20show=20blue=20boxes=20once?= =?UTF-8?q?=20notifications=20have=20gone?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit You can click the blue boxes to filter the list of notifications. Once the notifications have gone there’s nothing to filter, so we should just show the numbers but without them being clickable. --- app/main/views/jobs.py | 5 ++++- app/templates/partials/count.html | 17 +++++++++++++++-- tests/app/main/views/test_jobs.py | 13 +++++++++++++ 3 files changed, 32 insertions(+), 3 deletions(-) diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index b7f9a5d70..147078e5c 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -446,7 +446,10 @@ def get_job_partials(job, template): counts = render_template( 'partials/count.html', counts=_get_job_counts(job), - status=filter_args['status'] + status=filter_args['status'], + notifications_deleted=( + job['job_status'] == 'finished' and not notifications['notifications'] + ), ) service_data_retention_days = current_service.get_days_of_retention(template['template_type']) can_letter_job_be_cancelled = False diff --git a/app/templates/partials/count.html b/app/templates/partials/count.html index eb2990161..c66b6bff2 100644 --- a/app/templates/partials/count.html +++ b/app/templates/partials/count.html @@ -1,5 +1,18 @@ +{% from "components/big-number.html" import big_number %} {% from "components/pill.html" import pill %} -
- {{ pill(counts, request.args.get('status', '')) }} +
+ {% if notifications_deleted %} +
+ {% for label, query_param, url, count in counts %} +
+ {{ big_number(count, label, smaller=True) }} +
+ {% endfor %} +
+ {% else %} +
+ {{ pill(counts, request.args.get('status', '')) }} +
+ {% endif %}
diff --git a/tests/app/main/views/test_jobs.py b/tests/app/main/views/test_jobs.py index 7832f8b20..62c78641f 100644 --- a/tests/app/main/views/test_jobs.py +++ b/tests/app/main/views/test_jobs.py @@ -294,6 +294,12 @@ def test_should_show_job_in_progress( service_id=service_one['id'], job_id=fake_uuid, ) + assert [ + normalize_spaces(link.text) + for link in page.select('.pill a') + ] == [ + '10 sending', '0 delivered', '0 failed' + ] assert page.select_one('p.hint').text.strip() == 'Report is 50% complete…' @@ -313,6 +319,12 @@ def test_should_show_job_without_notifications( service_id=service_one['id'], job_id=fake_uuid, ) + assert [ + normalize_spaces(link.text) + for link in page.select('.pill a') + ] == [ + '10 sending', '0 delivered', '0 failed' + ] assert page.select_one('p.hint').text.strip() == 'Report is 50% complete…' assert page.select_one('tbody').text.strip() == 'No messages to show yet…' @@ -333,6 +345,7 @@ def test_should_show_old_job( service_id=service_one['id'], job_id=fake_uuid, ) + assert not page.select('.pill a') assert not page.select('p.hint') assert not page.select('a[download]') assert page.select_one('tbody').text.strip() == (