From 74e70ed8bc30bac99dbaef04eda70c2aacc3bfc4 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 12 Feb 2020 14:03:51 +0000 Subject: [PATCH 1/8] Refactor summaries into model MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This lets us encapsulate some of the logic that’s currently cluttering up the view/template layer. --- app/main/views/dashboard.py | 4 ---- app/main/views/returned_letters.py | 5 ++--- app/models/service.py | 10 ++++++++++ app/templates/views/dashboard/_inbox.html | 12 ++++++------ 4 files changed, 18 insertions(+), 13 deletions(-) diff --git a/app/main/views/dashboard.py b/app/main/views/dashboard.py index 2ae4394b0..b66eedf8f 100644 --- a/app/main/views/dashboard.py +++ b/app/main/views/dashboard.py @@ -296,10 +296,6 @@ def get_dashboard_partials(service_id): ), 'inbox': render_template( 'views/dashboard/_inbox.html', - inbound_sms_summary=( - service_api_client.get_inbound_sms_summary(service_id) - if current_service.has_permission('inbound_sms') else None - ), ), 'totals': render_template( 'views/dashboard/_totals.html', diff --git a/app/main/views/returned_letters.py b/app/main/views/returned_letters.py index cdc06e464..bb81f3449 100644 --- a/app/main/views/returned_letters.py +++ b/app/main/views/returned_letters.py @@ -2,7 +2,7 @@ from collections import OrderedDict from flask import render_template -from app import service_api_client +from app import current_service, service_api_client from app.main import main from app.utils import Spreadsheet, user_has_permissions @@ -10,10 +10,9 @@ from app.utils import Spreadsheet, user_has_permissions @main.route("/services//returned-letters") @user_has_permissions('view_activity') def returned_letter_summary(service_id): - summary = service_api_client.get_returned_letter_summary(service_id) return render_template( 'views/returned-letter-summary.html', - data=summary, + data=current_service.returned_letter_summary, ) diff --git a/app/models/service.py b/app/models/service.py index 9e4fad965..c7b3f2737 100644 --- a/app/models/service.py +++ b/app/models/service.py @@ -480,6 +480,12 @@ class Service(JSONModel): def has_inbound_number(self): return bool(self.inbound_number) + @cached_property + def inbound_sms_summary(self): + if not self.has_permission('inbound_sms'): + return None + return service_api_client.get_inbound_sms_summary(self.id) + @cached_property def all_template_folders(self): return sorted( @@ -660,3 +666,7 @@ class Service(JSONModel): ): if test: yield BASE + '_incomplete' + tag + + @cached_property + def returned_letter_summary(self): + return service_api_client.get_returned_letter_summary(self.id) diff --git a/app/templates/views/dashboard/_inbox.html b/app/templates/views/dashboard/_inbox.html index 9d3586903..57a9d85b6 100644 --- a/app/templates/views/dashboard/_inbox.html +++ b/app/templates/views/dashboard/_inbox.html @@ -2,17 +2,17 @@ {% from "components/message-count-label.html" import message_count_label %}
- {% if inbound_sms_summary != None %} - From f369f76ae4860d3505a0924b7a2bdbdbc89a013e Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 12 Feb 2020 14:19:21 +0000 Subject: [PATCH 2/8] Count recently-returned letters on the dashboard Currently you have no way of getting to the returned letter page. This commit adds a link to it from the dashboard, following the pattern of the new received text messages banner. --- app/__init__.py | 17 +++ app/models/service.py | 25 ++++ app/templates/views/dashboard/_inbox.html | 13 ++ tests/app/main/views/test_accept_invite.py | 2 + tests/app/main/views/test_dashboard.py | 160 +++++++++++++++++++-- tests/app/main/views/test_sign_out.py | 1 + tests/conftest.py | 8 ++ 7 files changed, 218 insertions(+), 8 deletions(-) diff --git a/app/__init__.py b/app/__init__.py index 4daf81b80..bffed68b2 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -356,6 +356,22 @@ def format_delta(date): ) +def format_delta_days(date): + now = datetime.now(timezone.utc) + date = utc_string_to_aware_gmt_datetime(date) + delta = now - date + if date.strftime('%Y-%M-%D') == now.strftime('%Y-%M-%D'): + return "today" + if date.strftime('%Y-%M-%D') == (now - timedelta(days=1)).strftime('%Y-%M-%D'): + return "yesterday" + return ago.human( + delta, + precision=1, + future_tense='{} from now', + past_tense='{} ago', + ) + + def valid_phone_number(phone_number): try: validate_phone_number(phone_number) @@ -744,6 +760,7 @@ def add_template_filters(application): format_date_short, format_datetime_relative, format_delta, + format_delta_days, format_notification_status, format_notification_type, format_notification_status_as_time, diff --git a/app/models/service.py b/app/models/service.py index c7b3f2737..132ac157c 100644 --- a/app/models/service.py +++ b/app/models/service.py @@ -1,4 +1,8 @@ +from datetime import datetime, timedelta + +from dateutil.parser import parse from flask import abort, current_app +from notifications_utils.timezones import local_timezone from werkzeug.utils import cached_property from app.models import JSONModel @@ -670,3 +674,24 @@ class Service(JSONModel): @cached_property def returned_letter_summary(self): return service_api_client.get_returned_letter_summary(self.id) + + @property + def most_recent_returned_letter_report(self): + if not self.returned_letter_summary: + return None + return parse( + self.returned_letter_summary[0]['reported_at'] + " 00:00:00" + ).replace(tzinfo=local_timezone) + + @property + def count_of_returned_letters_in_last_7_days(self): + seven_days_ago = ( + datetime.now() - timedelta(days=7) + ).replace( + hour=0, minute=0, second=0 + ) + return sum( + report['returned_letter_count'] + for report in self.returned_letter_summary + if parse(report['reported_at'] + " 00:00:00") >= seven_days_ago + ) diff --git a/app/templates/views/dashboard/_inbox.html b/app/templates/views/dashboard/_inbox.html index 57a9d85b6..ebcd6fb84 100644 --- a/app/templates/views/dashboard/_inbox.html +++ b/app/templates/views/dashboard/_inbox.html @@ -17,4 +17,17 @@ {% endif %} {% endif %} + {% if current_service.returned_letter_summary %} + + {% endif %}
diff --git a/tests/app/main/views/test_accept_invite.py b/tests/app/main/views/test_accept_invite.py index cbd280ffe..b00b4209b 100644 --- a/tests/app/main/views/test_accept_invite.py +++ b/tests/app/main/views/test_accept_invite.py @@ -149,6 +149,7 @@ def test_accepting_invite_removes_invite_from_session( mock_get_billable_units, mock_get_free_sms_fragment_limit, mock_get_inbound_sms_summary, + mock_get_returned_letter_summary_with_no_returned_letters, fake_uuid, user, landing_page_title, @@ -470,6 +471,7 @@ def test_new_invited_user_verifies_and_added_to_service( mock_get_service_statistics, mock_get_usage, mock_get_free_sms_fragment_limit, + mock_get_returned_letter_summary_with_no_returned_letters, mock_create_event, mocker, ): diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index fd95ee7b9..0aafafb88 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -143,7 +143,8 @@ def test_get_started( mock_get_service_statistics, mock_get_usage, mock_get_free_sms_fragment_limit, - mock_get_inbound_sms_summary + mock_get_inbound_sms_summary, + mock_get_returned_letter_summary_with_no_returned_letters, ): mocker.patch( 'app.template_statistics_client.get_template_statistics_for_service', @@ -167,7 +168,8 @@ def test_get_started_is_hidden_once_templates_exist( mock_get_service_statistics, mock_get_usage, mock_get_free_sms_fragment_limit, - mock_get_inbound_sms_summary + mock_get_inbound_sms_summary, + mock_get_returned_letter_summary_with_no_returned_letters, ): mocker.patch( 'app.template_statistics_client.get_template_statistics_for_service', @@ -184,6 +186,7 @@ def test_get_started_is_hidden_once_templates_exist( def test_inbound_messages_not_visible_to_service_without_permissions( client_request, + mocker, service_one, mock_get_service_templates_when_no_templates_exist, mock_get_jobs, @@ -191,7 +194,8 @@ def test_inbound_messages_not_visible_to_service_without_permissions( mock_get_template_statistics, mock_get_usage, mock_get_free_sms_fragment_limit, - mock_get_inbound_sms_summary + mock_get_inbound_sms_summary, + mock_get_returned_letter_summary_with_no_returned_letters, ): service_one['permissions'] = [] @@ -216,6 +220,7 @@ def test_inbound_messages_shows_count_of_messages_when_there_are_messages( mock_get_usage, mock_get_free_sms_fragment_limit, mock_get_inbound_sms_summary, + mock_get_returned_letter_summary_with_no_returned_letters, ): service_one['permissions'] = ['inbound_sms'] page = client_request.get( @@ -242,6 +247,7 @@ def test_inbound_messages_shows_count_of_messages_when_there_are_no_messages( mock_get_usage, mock_get_free_sms_fragment_limit, mock_get_inbound_sms_summary_with_no_messages, + mock_get_returned_letter_summary_with_no_returned_letters, ): service_one['permissions'] = ['inbound_sms'] page = client_request.get( @@ -472,6 +478,126 @@ def test_download_inbox_strips_formulae( assert expected_cell in response.get_data(as_text=True).split('\r\n')[1] +def test_returned_letters_not_visible_if_service_has_no_returned_letters( + client_request, + mocker, + service_one, + mock_get_service_templates_when_no_templates_exist, + mock_get_jobs, + mock_get_service_statistics, + mock_get_template_statistics, + mock_get_usage, + mock_get_free_sms_fragment_limit, + mock_get_inbound_sms_summary, + mock_get_returned_letter_summary_with_no_returned_letters, +): + page = client_request.get( + 'main.service_dashboard', + service_id=SERVICE_ONE_ID, + ) + assert not page.select('#total-returned-letters') + + +@freeze_time('2020-01-10') +def test_returned_letters_shows_count_of_recently_returned_letters( + client_request, + mocker, + service_one, + mock_get_service_templates_when_no_templates_exist, + mock_get_jobs, + mock_get_service_statistics, + mock_get_template_statistics, + mock_get_usage, + mock_get_free_sms_fragment_limit, + mock_get_inbound_sms_summary, +): + mocker.patch( + 'app.service_api_client.get_returned_letter_summary', + return_value=[ + # Today (should be counted) + { + 'returned_letter_count': 1000, 'reported_at': '2020-01-10' + }, + # Just within the last 7 days (should be counted) + { + 'returned_letter_count': 3000, 'reported_at': '2020-01-3' + }, + # Just after the last 7 days (should not be counted) + { + 'returned_letter_count': 2000, 'reported_at': '2020-01-2' + }, + ], + ) + page = client_request.get( + 'main.service_dashboard', + service_id=SERVICE_ONE_ID, + ) + banner = page.select_one('#total-returned-letters') + assert normalize_spaces( + banner.text + ) == '4,000 returned letters latest report today' + assert banner['href'] == url_for( + 'main.returned_letter_summary', service_id=SERVICE_ONE_ID + ) + + +@pytest.mark.parametrize('reporting_date, expected_message', ( + ('2020-02-02', ( + '1 returned letter latest report today' + )), + ('2020-02-01', ( + '1 returned letter latest report yesterday' + )), + ('2020-01-31', ( + '1 returned letter latest report 2 days ago' + )), + ('2020-01-26', ( + '1 returned letter latest report 7 days ago' + )), + ('2020-01-25', ( + '0 returned letters latest report 8 days ago' + )), + ('2019-09-09', ( + '0 returned letters latest report 146 days ago' + )), + ('2010-10-10', ( + '0 returned letters latest report 9 years ago' + )), +)) +@freeze_time('2020-02-02') +def test_returned_letters_only_counts_recently_returned_letters( + client_request, + mocker, + service_one, + mock_get_service_templates_when_no_templates_exist, + mock_get_jobs, + mock_get_service_statistics, + mock_get_template_statistics, + mock_get_usage, + mock_get_free_sms_fragment_limit, + mock_get_inbound_sms_summary_with_no_messages, + reporting_date, + expected_message, +): + mocker.patch( + 'app.service_api_client.get_returned_letter_summary', + return_value=[ + { + 'returned_letter_count': 1, 'reported_at': reporting_date + }, + ], + ) + page = client_request.get( + 'main.service_dashboard', + service_id=SERVICE_ONE_ID, + ) + banner = page.select_one('#total-returned-letters') + assert normalize_spaces(banner.text) == expected_message + assert banner['href'] == url_for( + 'main.returned_letter_summary', service_id=SERVICE_ONE_ID + ) + + def test_should_show_recent_templates_on_dashboard( client_request, mocker, @@ -480,7 +606,8 @@ def test_should_show_recent_templates_on_dashboard( mock_get_service_statistics, mock_get_usage, mock_get_free_sms_fragment_limit, - mock_get_inbound_sms_summary + mock_get_inbound_sms_summary, + mock_get_returned_letter_summary_with_no_returned_letters, ): mock_template_stats = mocker.patch('app.template_statistics_client.get_template_statistics_for_service', return_value=copy.deepcopy(stub_template_stats)) @@ -534,6 +661,7 @@ def test_should_not_show_recent_templates_on_dashboard_if_only_one_template_used mock_get_usage, mock_get_free_sms_fragment_limit, mock_get_inbound_sms_summary, + mock_get_returned_letter_summary_with_no_returned_letters, stats, ): mock_template_stats = mocker.patch( @@ -687,7 +815,8 @@ def test_should_show_upcoming_jobs_on_dashboard( mock_get_jobs, mock_get_usage, mock_get_free_sms_fragment_limit, - mock_get_inbound_sms_summary + mock_get_inbound_sms_summary, + mock_get_returned_letter_summary_with_no_returned_letters, ): page = client_request.get( 'main.service_dashboard', @@ -738,6 +867,7 @@ def test_correct_font_size_for_big_numbers( mock_get_jobs, mock_get_usage, mock_get_free_sms_fragment_limit, + mock_get_returned_letter_summary_with_no_returned_letters, service_one, permissions, totals, @@ -773,7 +903,8 @@ def test_should_show_recent_jobs_on_dashboard( mock_get_jobs, mock_get_usage, mock_get_free_sms_fragment_limit, - mock_get_inbound_sms_summary + mock_get_inbound_sms_summary, + mock_get_returned_letter_summary_with_no_returned_letters, ): page = client_request.get( 'main.service_dashboard', @@ -981,6 +1112,7 @@ def test_menu_send_messages( mock_get_usage, mock_get_inbound_sms_summary, mock_get_free_sms_fragment_limit, + mock_get_returned_letter_summary_with_no_returned_letters, ): service_one['permissions'] = ['email', 'sms', 'letter', 'upload_letters'] @@ -1039,6 +1171,7 @@ def test_menu_manage_service( mock_get_service_statistics, mock_get_usage, mock_get_inbound_sms_summary, + mock_get_returned_letter_summary_with_no_returned_letters, mock_get_free_sms_fragment_limit, ): with app_.test_request_context(): @@ -1070,6 +1203,7 @@ def test_menu_manage_api_keys( mock_get_service_statistics, mock_get_usage, mock_get_inbound_sms_summary, + mock_get_returned_letter_summary_with_no_returned_letters, mock_get_free_sms_fragment_limit, ): with app_.test_request_context(): @@ -1099,6 +1233,7 @@ def test_menu_all_services_for_platform_admin_user( mock_get_service_statistics, mock_get_usage, mock_get_inbound_sms_summary, + mock_get_returned_letter_summary_with_no_returned_letters, mock_get_free_sms_fragment_limit, ): with app_.test_request_context(): @@ -1130,7 +1265,8 @@ def test_route_for_service_permissions( mock_get_service_statistics, mock_get_usage, mock_get_free_sms_fragment_limit, - mock_get_inbound_sms_summary + mock_get_inbound_sms_summary, + mock_get_returned_letter_summary_with_no_returned_letters, ): with app_.test_request_context(): validate_route_permission( @@ -1183,7 +1319,8 @@ def test_service_dashboard_updates_gets_dashboard_totals( mock_get_jobs, mock_get_usage, mock_get_free_sms_fragment_limit, - mock_get_inbound_sms_summary + mock_get_inbound_sms_summary, + mock_get_returned_letter_summary_with_no_returned_letters, ): mocker.patch('app.main.views.dashboard.get_dashboard_totals', return_value={ 'email': {'requested': 123, 'delivered': 0, 'failed': 0}, @@ -1368,6 +1505,7 @@ def test_should_show_all_jobs_with_valid_statuses( mock_get_jobs, mock_get_usage, mock_get_inbound_sms_summary, + mock_get_returned_letter_summary_with_no_returned_letters, mock_get_free_sms_fragment_limit, ): logged_in_client.get(url_for('main.service_dashboard', service_id=SERVICE_ONE_ID)) @@ -1397,6 +1535,7 @@ def test_org_breadcrumbs_do_not_show_if_service_has_no_org( mock_get_jobs, mock_get_usage, mock_get_free_sms_fragment_limit, + mock_get_returned_letter_summary_with_no_returned_letters, ): page = client_request.get('main.service_dashboard', service_id=SERVICE_ONE_ID) @@ -1411,6 +1550,7 @@ def test_org_breadcrumbs_do_not_show_if_user_is_not_an_org_member( active_caseworking_user, client_request, mock_get_template_folders, + mock_get_returned_letter_summary_with_no_returned_letters, ): # active_caseworking_user is not an org member @@ -1433,6 +1573,7 @@ def test_org_breadcrumbs_show_if_user_is_a_member_of_the_services_org( mock_get_jobs, mock_get_usage, mock_get_free_sms_fragment_limit, + mock_get_returned_letter_summary_with_no_returned_letters, active_user_with_permissions, client_request, ): @@ -1460,6 +1601,7 @@ def test_org_breadcrumbs_do_not_show_if_user_is_a_member_of_the_services_org_but mock_get_jobs, mock_get_usage, mock_get_free_sms_fragment_limit, + mock_get_returned_letter_summary_with_no_returned_letters, active_user_with_permissions, client_request, ): @@ -1484,6 +1626,7 @@ def test_org_breadcrumbs_show_if_user_is_platform_admin( mock_get_jobs, mock_get_usage, mock_get_free_sms_fragment_limit, + mock_get_returned_letter_summary_with_no_returned_letters, platform_admin_user, platform_admin_client, ): @@ -1515,6 +1658,7 @@ def test_should_show_usage_on_dashboard( mock_get_jobs, mock_get_usage, mock_get_free_sms_fragment_limit, + mock_get_returned_letter_summary_with_no_returned_letters, permissions, ): service_one['permissions'] = permissions diff --git a/tests/app/main/views/test_sign_out.py b/tests/app/main/views/test_sign_out.py index 12e93f33b..2d1be4c2f 100644 --- a/tests/app/main/views/test_sign_out.py +++ b/tests/app/main/views/test_sign_out.py @@ -31,6 +31,7 @@ def test_sign_out_user( mock_get_usage, mock_get_free_sms_fragment_limit, mock_get_inbound_sms_summary, + mock_get_returned_letter_summary_with_no_returned_letters, ): with client_request.session_transaction() as session: assert session.get('user_id') is not None diff --git a/tests/conftest.py b/tests/conftest.py index dfa844724..f08fd2d9c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -3342,6 +3342,14 @@ def mock_get_service_history(mocker): }) +@pytest.fixture(scope='function') +def mock_get_returned_letter_summary_with_no_returned_letters(mocker): + return mocker.patch( + 'app.service_api_client.get_returned_letter_summary', + return_value=[], + ) + + def create_api_user_active(with_unique_id=False): return { 'id': str(uuid4()) if with_unique_id else sample_uuid(), From 9590643527643f264e2abe940a9a1f1769843140 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 14 Feb 2020 15:09:03 +0000 Subject: [PATCH 3/8] Use humanize for fuzzy time differences MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It seems to do a bit better than ago (e.g. 4 months vs 146 days), and looks like it’s maintained more often. --- app/__init__.py | 17 +++-------------- requirements-app.txt | 1 + requirements.txt | 5 +++-- tests/app/main/views/test_dashboard.py | 8 ++++---- 4 files changed, 11 insertions(+), 20 deletions(-) diff --git a/app/__init__.py b/app/__init__.py index bffed68b2..6b7197f3c 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -5,7 +5,7 @@ from datetime import datetime, timedelta, timezone from functools import partial from time import monotonic -import ago +import humanize import jinja2 from flask import ( Markup, @@ -348,28 +348,17 @@ def format_delta(date): return "just now" if delta < timedelta(seconds=60): return "in the last minute" - return ago.human( - delta, - future_tense='{} from now', # No-one should ever see this - past_tense='{} ago', - precision=1 - ) + return humanize.naturaltime(delta) def format_delta_days(date): now = datetime.now(timezone.utc) date = utc_string_to_aware_gmt_datetime(date) - delta = now - date if date.strftime('%Y-%M-%D') == now.strftime('%Y-%M-%D'): return "today" if date.strftime('%Y-%M-%D') == (now - timedelta(days=1)).strftime('%Y-%M-%D'): return "yesterday" - return ago.human( - delta, - precision=1, - future_tense='{} from now', - past_tense='{} ago', - ) + return humanize.naturaltime(now - date) def valid_phone_number(phone_number): diff --git a/requirements-app.txt b/requirements-app.txt index d2bd53f6d..ea804c360 100644 --- a/requirements-app.txt +++ b/requirements-app.txt @@ -2,6 +2,7 @@ # with package version changes made in requirements-app.txt ago==0.0.93 +humanize==1.0.0 Flask==1.1.1 Flask-WTF==0.14.3 Flask-Login==0.4.1 diff --git a/requirements.txt b/requirements.txt index 89e522545..9b7ca375f 100644 --- a/requirements.txt +++ b/requirements.txt @@ -4,6 +4,7 @@ # with package version changes made in requirements-app.txt ago==0.0.93 +humanize==1.0.0 Flask==1.1.1 Flask-WTF==0.14.3 Flask-Login==0.4.1 @@ -29,10 +30,10 @@ git+https://github.com/alphagov/notifications-utils.git@36.6.0#egg=notifications git+https://github.com/alphagov/govuk-frontend-jinja.git@v0.5.1-alpha#egg=govuk-frontend-jinja==0.5.1-alpha ## The following requirements were added by pip freeze: -awscli==1.17.15 +awscli==1.18.2 bleach==3.1.0 boto3==1.10.38 -botocore==1.14.15 +botocore==1.15.2 certifi==2019.11.28 chardet==3.0.4 Click==7.0 diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index 0aafafb88..6920b840c 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -262,9 +262,9 @@ def test_inbound_messages_shows_count_of_messages_when_there_are_no_messages( @pytest.mark.parametrize('index, expected_row', enumerate([ - '07900 900000 message-1 1 hour ago', - '07900 900000 message-2 1 hour ago', - '07900 900000 message-3 1 hour ago', + '07900 900000 message-1 an hour ago', + '07900 900000 message-2 an hour ago', + '07900 900000 message-3 an hour ago', '07900 900002 message-4 3 hours ago', '07900 900004 message-5 5 hours ago', '07900 900006 message-6 7 hours ago', @@ -558,7 +558,7 @@ def test_returned_letters_shows_count_of_recently_returned_letters( '0 returned letters latest report 8 days ago' )), ('2019-09-09', ( - '0 returned letters latest report 146 days ago' + '0 returned letters latest report 4 months ago' )), ('2010-10-10', ( '0 returned letters latest report 9 years ago' From 64074eed03e597c7e2342d15f8dd832fe6ff3bab Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Sat, 15 Feb 2020 18:29:58 +0000 Subject: [PATCH 4/8] =?UTF-8?q?Say=20=E2=80=981=20hour/month=20ago?= =?UTF-8?q?=E2=80=99=20not=20=E2=80=98an=20hour/a=20month=20ago=E2=80=99?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I think it read better without the indefinite article when it’s, for example, placed alongside messages that read ‘2 hours ago’. --- app/__init__.py | 13 +++++++++++-- tests/app/main/views/test_dashboard.py | 9 ++++++--- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/app/__init__.py b/app/__init__.py index 6b7197f3c..494c05720 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -1,5 +1,6 @@ import itertools import os +import re import urllib from datetime import datetime, timedelta, timezone from functools import partial @@ -338,6 +339,14 @@ def _format_datetime_short(datetime): return datetime.strftime('%d %B').lstrip('0') +def naturaltime_without_indefinite_article(date): + return re.sub( + 'an? (.*) ago', + lambda match: '1 {} ago'.format(match.group(1)), + humanize.naturaltime(date), + ) + + def format_delta(date): delta = ( datetime.now(timezone.utc) @@ -348,7 +357,7 @@ def format_delta(date): return "just now" if delta < timedelta(seconds=60): return "in the last minute" - return humanize.naturaltime(delta) + return naturaltime_without_indefinite_article(delta) def format_delta_days(date): @@ -358,7 +367,7 @@ def format_delta_days(date): return "today" if date.strftime('%Y-%M-%D') == (now - timedelta(days=1)).strftime('%Y-%M-%D'): return "yesterday" - return humanize.naturaltime(now - date) + return naturaltime_without_indefinite_article(now - date) def valid_phone_number(phone_number): diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index 6920b840c..3f09b4e18 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -262,9 +262,9 @@ def test_inbound_messages_shows_count_of_messages_when_there_are_no_messages( @pytest.mark.parametrize('index, expected_row', enumerate([ - '07900 900000 message-1 an hour ago', - '07900 900000 message-2 an hour ago', - '07900 900000 message-3 an hour ago', + '07900 900000 message-1 1 hour ago', + '07900 900000 message-2 1 hour ago', + '07900 900000 message-3 1 hour ago', '07900 900002 message-4 3 hours ago', '07900 900004 message-5 5 hours ago', '07900 900006 message-6 7 hours ago', @@ -557,6 +557,9 @@ def test_returned_letters_shows_count_of_recently_returned_letters( ('2020-01-25', ( '0 returned letters latest report 8 days ago' )), + ('2020-01-01', ( + '0 returned letters latest report 1 month ago' + )), ('2019-09-09', ( '0 returned letters latest report 4 months ago' )), From d00e37541cee92391f09b022998634f03a23a7d4 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 17 Feb 2020 10:23:59 +0000 Subject: [PATCH 5/8] Make dashboard navigation active for returned letters --- app/navigation.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/navigation.py b/app/navigation.py index 503e47ebe..dd6aeecc4 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -354,6 +354,8 @@ class MainNavigation(Navigation): 'conversation', 'inbox', 'monthly', + 'returned_letter_summary', + 'returned_letters', 'service_dashboard', 'template_usage', 'view_job', @@ -581,8 +583,6 @@ class MainNavigation(Navigation): 'resend_email_link', 'resend_email_verification', 'resume_service', - 'returned_letter_summary', - 'returned_letters', 'returned_letters_report', 'revalidate_email_sent', 'roadmap', From 678c3df53ce9c77c9846e034bcbdae3ad5626da0 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 20 Feb 2020 11:48:34 +0000 Subject: [PATCH 6/8] Add some context to the list of reports We reckon people need some context/expectation setting about what the date of the report is. --- .../views/returned-letter-summary.html | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/app/templates/views/returned-letter-summary.html b/app/templates/views/returned-letter-summary.html index 5be1fc0b3..a68c2b267 100644 --- a/app/templates/views/returned-letter-summary.html +++ b/app/templates/views/returned-letter-summary.html @@ -1,5 +1,6 @@ {% from "components/table.html" import list_table, row_heading %} {% from "components/message-count-label.html" import message_count_label %} +{% from "components/page-header.html" import page_header %} {% extends "withnav_template.html" %} {% block service_page_title %} @@ -7,9 +8,20 @@ {% endblock %} {% block maincolumn_content %} -

- Returned letters -

+
+
+ {{ page_header( + 'Returned letters', + back_link=url_for('main.service_dashboard', service_id=current_service.id) + ) }} +

+ Reports are published once a month. +

+

+ You’ll only get a report if one or more of your letters is returned. +

+
+
{% call(item, row_number) list_table( data, From 0e8143da0405e4da2335dbb2963d9abaf6fd9b25 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 20 Feb 2020 11:51:41 +0000 Subject: [PATCH 7/8] =?UTF-8?q?Remove=20=E2=80=98originally=E2=80=99=20fro?= =?UTF-8?q?m=20returned=20letters=20report?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We should remove Originally on the individual report pages. It sounds like we're saying the letter has been sent more than once since that date. The dictionary definition for 'originally' as an adverb says: > used to describe the situation that existed at the beginning of a > particular period or activity, especially before something was changed Examples given include: > The book was originally published in 1935. --- app/templates/views/returned-letters.html | 2 +- tests/app/main/views/test_returned_letters.py | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/templates/views/returned-letters.html b/app/templates/views/returned-letters.html index fe0e85462..f033a33bc 100644 --- a/app/templates/views/returned-letters.html +++ b/app/templates/views/returned-letters.html @@ -42,7 +42,7 @@ {% call field(align='right') %} - Originally sent {{ item.created_at|format_date_normal }} + Sent {{ item.created_at|format_date_normal }} {% endcall %} diff --git a/tests/app/main/views/test_returned_letters.py b/tests/app/main/views/test_returned_letters.py index 89b944e03..c8aadafc5 100644 --- a/tests/app/main/views/test_returned_letters.py +++ b/tests/app/main/views/test_returned_letters.py @@ -88,12 +88,12 @@ def test_returned_letters_page( assert [ 'Template name Originally sent', - 'Example template Reference ABC123 Originally sent 24 December 2019', - 'Example template Sent from Example spreadsheet.xlsx Originally sent 24 December 2019', - 'Example template No reference provided Originally sent 24 December 2019', - 'Example precompiled.pdf Reference DEF456 Originally sent 24 December 2019', - 'Example one-off.pdf No reference provided Originally sent 24 December 2019', - 'Provided as PDF Reference XYZ999 Originally sent 24 December 2019', + 'Example template Reference ABC123 Sent 24 December 2019', + 'Example template Sent from Example spreadsheet.xlsx Sent 24 December 2019', + 'Example template No reference provided Sent 24 December 2019', + 'Example precompiled.pdf Reference DEF456 Sent 24 December 2019', + 'Example one-off.pdf No reference provided Sent 24 December 2019', + 'Provided as PDF Reference XYZ999 Sent 24 December 2019', ] == [ normalize_spaces(row.text) for row in page.select('tr') ] From a5b7e3b93aa1af24d5213ad13a88de4f724514a3 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 21 Feb 2020 11:37:01 +0000 Subject: [PATCH 8/8] Add Design System link classes Co-Authored-By: Tom Byers --- app/templates/views/dashboard/_inbox.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/templates/views/dashboard/_inbox.html b/app/templates/views/dashboard/_inbox.html index ebcd6fb84..a73ee296c 100644 --- a/app/templates/views/dashboard/_inbox.html +++ b/app/templates/views/dashboard/_inbox.html @@ -18,7 +18,7 @@ {% endif %} {% if current_service.returned_letter_summary %} -