diff --git a/app/__init__.py b/app/__init__.py index c2921bafc..6ceb1fdbe 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -1,11 +1,12 @@ import itertools import os +import re import urllib from datetime import datetime, timedelta, timezone from functools import partial from time import monotonic -import ago +import humanize import jinja2 from flask import ( Markup, @@ -356,6 +357,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) @@ -366,12 +375,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 naturaltime_without_indefinite_article(delta) + + +def format_delta_days(date): + now = datetime.now(timezone.utc) + date = utc_string_to_aware_gmt_datetime(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 naturaltime_without_indefinite_article(now - date) def valid_phone_number(phone_number): @@ -764,6 +778,7 @@ def add_template_filters(application): format_datetime_relative, format_day_of_week, format_delta, + format_delta_days, format_notification_status, format_notification_type, format_notification_status_as_time, 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..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 @@ -480,6 +484,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 +670,28 @@ 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) + + @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/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', diff --git a/app/templates/views/dashboard/_inbox.html b/app/templates/views/dashboard/_inbox.html index 9d3586903..a73ee296c 100644 --- a/app/templates/views/dashboard/_inbox.html +++ b/app/templates/views/dashboard/_inbox.html @@ -2,19 +2,32 @@ {% from "components/message-count-label.html" import message_count_label %}
- {% if inbound_sms_summary != None %} - {% endif %} + {% if current_service.returned_letter_summary %} + + {% endif %}
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, 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/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_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..3f09b4e18 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,129 @@ 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' + )), + ('2020-01-01', ( + '0 returned letters latest report 1 month ago' + )), + ('2019-09-09', ( + '0 returned letters latest report 4 months 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 +609,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 +664,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 +818,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 +870,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 +906,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 +1115,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 +1174,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 +1206,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 +1236,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 +1268,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 +1322,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 +1508,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 +1538,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 +1553,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 +1576,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 +1604,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 +1629,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 +1661,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_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') ] 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(),