diff --git a/app/__init__.py b/app/__init__.py index 4daf81b80..ac6386e02 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -1,11 +1,11 @@ -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, @@ -291,7 +291,7 @@ def format_time_24h(date): return utc_string_to_aware_gmt_datetime(date).strftime('%H:%M') -def get_human_day(time): +def get_human_day(time, date_prefix=''): # Add 1 minute to transform 00:00 into ‘midnight today’ instead of ‘midnight tomorrow’ date = (utc_string_to_aware_gmt_datetime(time) - timedelta(minutes=1)).date() @@ -304,8 +304,15 @@ def get_human_day(time): if date == (now - timedelta(days=1)).date(): return 'yesterday' if date.strftime('%Y') != now.strftime('%Y'): - return '{} {}'.format(_format_datetime_short(date), date.strftime('%Y')) - return _format_datetime_short(date) + return '{} {} {}'.format( + date_prefix, + _format_datetime_short(date), + date.strftime('%Y'), + ).strip() + return '{} {}'.format( + date_prefix, + _format_datetime_short(date), + ).strip() def format_time(date): @@ -334,10 +341,29 @@ def format_date_human(date): return get_human_day(date) +def format_datetime_human(date, date_prefix=''): + return '{} at {}'.format( + get_human_day(date, date_prefix='on'), + format_time(date), + ) + + +def format_day_of_week(date): + return utc_string_to_aware_gmt_datetime(date).strftime('%A') + + 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,12 +374,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): @@ -584,8 +615,10 @@ def useful_headers_after_request(response): def register_errorhandlers(application): # noqa (C901 too complex) - def _error_response(error_code): - resp = make_response(render_template("error/{0}.html".format(error_code)), error_code) + def _error_response(error_code, error_page_template=None): + if error_page_template is None: + error_page_template = error_code + resp = make_response(render_template("error/{0}.html".format(error_page_template)), error_code) return useful_headers_after_request(resp) @application.errorhandler(HTTPError) @@ -596,15 +629,10 @@ def register_errorhandlers(application): # noqa (C901 too complex) error.message )) error_code = error.status_code - if error_code == 400: - if isinstance(error.message, str): - msg = [error.message] - else: - msg = list(itertools.chain(*[error.message[x] for x in error.message.keys()])) - resp = make_response(render_template("error/400.html", message=msg)) - return useful_headers_after_request(resp) - elif error_code not in [401, 404, 403, 410]: - # probably a 500 or 503 + if error_code not in [401, 404, 403, 410]: + # probably a 500 or 503. + # it might be a 400, which we should handle as if it's an internal server error. If the API might + # legitimately return a 400, we should handle that within the view or the client that calls it. application.logger.exception("API {} failed with status {} message {}".format( error.response.url if error.response else 'unknown', error.status_code, @@ -614,8 +642,10 @@ def register_errorhandlers(application): # noqa (C901 too complex) return _error_response(error_code) @application.errorhandler(400) - def handle_400(error): - return _error_response(400) + def handle_client_error(error): + # This is tripped if we call `abort(400)`. + application.logger.exception('Unhandled 400 client error') + return _error_response(400, error_page_template=500) @application.errorhandler(410) def handle_gone(error): @@ -658,19 +688,11 @@ def register_errorhandlers(application): # noqa (C901 too complex) u'csrf.invalid_token: Aborting request, user_id: {user_id}', extra={'user_id': session['user_id']}) - resp = make_response(render_template( - "error/400.html", - message=['Something went wrong, please go back and try again.'] - ), 400) - return useful_headers_after_request(resp) + return _error_response(400, error_page_template=500) @application.errorhandler(405) - def handle_405(error): - resp = make_response(render_template( - "error/400.html", - message=['Something went wrong, please go back and try again.'] - ), 405) - return useful_headers_after_request(resp) + def handle_method_not_allowed(error): + return _error_response(405, error_page_template=500) @application.errorhandler(WerkzeugHTTPException) def handle_http_error(error): @@ -742,8 +764,11 @@ def add_template_filters(application): format_date_human, format_date_normal, format_date_short, + format_datetime_human, 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/assets/javascripts/analytics/analytics.js b/app/assets/javascripts/analytics/analytics.js index 711f6ec80..a776f93a2 100644 --- a/app/assets/javascripts/analytics/analytics.js +++ b/app/assets/javascripts/analytics/analytics.js @@ -15,6 +15,7 @@ window.ga('set', 'anonymizeIp', config.anonymizeIp); window.ga('set', 'allowAdFeatures', config.allowAdFeatures); window.ga('set', 'transport', config.transport); + window.ga('set', 'title', 'GOV.UK Notify'); }; 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/main/views/uploads.py b/app/main/views/uploads.py index 60c28c894..332b6f9aa 100644 --- a/app/main/views/uploads.py +++ b/app/main/views/uploads.py @@ -234,7 +234,6 @@ def uploaded_letter_preview(service_id, file_id): @main.route("/services//preview-letter-image/") @user_has_permissions('send_messages') def view_letter_upload_as_preview(service_id, file_id): - try: page = int(request.args.get('page')) except ValueError: 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 09b931173..3f94bbc9f 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -352,6 +352,8 @@ class MainNavigation(Navigation): 'conversation', 'inbox', 'monthly', + 'returned_letter_summary', + 'returned_letters', 'service_dashboard', 'template_usage', 'view_job', @@ -579,8 +581,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/components/current-service-link.html b/app/templates/components/current-service-link.html deleted file mode 100644 index 807ba8ac4..000000000 --- a/app/templates/components/current-service-link.html +++ /dev/null @@ -1,7 +0,0 @@ -{% macro current_service_link(endpoint, link_text) %} - {% if current_service %} - {{ link_text }} - {% else %} - {{ link_text|capitalize }} - {% endif %} -{% endmacro %} diff --git a/app/templates/components/service-link.html b/app/templates/components/service-link.html new file mode 100644 index 000000000..b5906cf5f --- /dev/null +++ b/app/templates/components/service-link.html @@ -0,0 +1,7 @@ +{% macro service_link(service, endpoint, link_text) %} + {% if service %} + {{ link_text }} + {% else %} + {{ link_text|capitalize }} + {% endif %} +{% endmacro %} diff --git a/app/templates/error/400.html b/app/templates/error/400.html deleted file mode 100644 index f874031c1..000000000 --- a/app/templates/error/400.html +++ /dev/null @@ -1,11 +0,0 @@ -{% extends "withoutnav_template.html" %} -{% block per_page_title %}Bad request{% endblock %} -{% block maincolumn_content %} -
-
-

- {{ message|join(",")}} -

-
-
-{% endblock %} 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/dashboard/inbox.html b/app/templates/views/dashboard/inbox.html index 39189c3de..8ea050b0b 100644 --- a/app/templates/views/dashboard/inbox.html +++ b/app/templates/views/dashboard/inbox.html @@ -1,4 +1,5 @@ {% from "components/ajax-block.html" import ajax_block %} +{% from "components/page-header.html" import page_header %} {% extends "withnav_template.html" %} @@ -8,9 +9,10 @@ {% block maincolumn_content %} -

- Received text messages -

+ {{ page_header( + 'Received text messages', + back_link=url_for('main.service_dashboard', service_id=current_service.id) + ) }} {{ ajax_block( partials, diff --git a/app/templates/views/get-started.html b/app/templates/views/get-started.html index b54ad105d..489791f17 100644 --- a/app/templates/views/get-started.html +++ b/app/templates/views/get-started.html @@ -43,17 +43,17 @@

  • Write some messages

    -

    Add message templates with examples of the content you plan to send. You can use our guidance to help you.

    +

    Add message templates with examples of the content you plan to send. You can use our guidance to help you.

  • Set up your service

    {% if not current_user.is_authenticated or not current_service %}

    Review your settings to add message branding, reply-to addresses and sender information.

    -

    Add team members and check their permissions.

    +

    Add team members and check their permissions.

    {% else %}

    Review your settings to add message branding, reply-to addresses and sender information.

    -

    Add team members and check their permissions.

    +

    Add team members and check their permissions.

    {% endif %}
    • diff --git a/app/templates/views/guidance/branding-and-customisation.html b/app/templates/views/guidance/branding-and-customisation.html index 6e01cbc1a..0d017d65d 100644 --- a/app/templates/views/guidance/branding-and-customisation.html +++ b/app/templates/views/guidance/branding-and-customisation.html @@ -1,5 +1,5 @@ {% extends "content_template.html" %} -{% from "components/current-service-link.html" import current_service_link %} +{% from "components/service-link.html" import service_link %} {% block per_page_title %} Branding and customisation @@ -25,7 +25,7 @@

    To change your email branding:

      -
    1. Go to the Email settings section of the {{ current_service_link('main.service_settings', 'settings') }} page.
      2. +
    2. Go to the Email settings section of the {{ service_link(current_service, 'main.service_settings', 'settings') }} page.
    3. Select Change on the Email branding row.
    @@ -36,7 +36,7 @@

    To add a reply-to email address:

      -
    1. Go to the Email settings section of the {{ current_service_link('main.service_settings', 'settings') }} page.
      2. +
    2. Go to the Email settings section of the {{ service_link(current_service, 'main.service_settings', 'settings') }} page.
    3. Select Manage on the Reply-to email addresses row.
    4. Select Add reply-to address.
    @@ -48,7 +48,7 @@

    To change the text message sender from the default of ‘GOVUK’:

      -
    1. Go to the Text message settings section of the {{ current_service_link('main.service_settings', 'settings') }} page.
      2. +
    2. Go to the Text message settings section of the {{ service_link(current_service, 'main.service_settings', 'settings') }} page.
    3. Select Manage on the Text message senders row.
    4. Select Change or Add text message sender.
    @@ -60,7 +60,7 @@

    To add a logo to your letters:

      -
    1. Go to the Letter settings section of the {{ current_service_link('main.service_settings', 'settings') }} page.
      2. +
    2. Go to the Letter settings section of the {{ service_link(current_service, 'main.service_settings', 'settings') }} page.
    3. Select Change on the Letter branding row.
    diff --git a/app/templates/views/guidance/create-and-send-messages.html b/app/templates/views/guidance/create-and-send-messages.html index b889ff12f..a72f8eb64 100644 --- a/app/templates/views/guidance/create-and-send-messages.html +++ b/app/templates/views/guidance/create-and-send-messages.html @@ -1,5 +1,5 @@ {% extends "content_template.html" %} -{% from "components/current-service-link.html" import current_service_link %} +{% from "components/service-link.html" import service_link %} {% block per_page_title %} Send messages @@ -15,7 +15,7 @@

    To send a batch of messages at once:

      -
    1. Go to the {{ current_service_link('main.choose_template', 'templates') }} page and choose an existing template.
      2. +
    2. Go to the {{ service_link(current_service, 'main.choose_template', 'templates') }} page and choose an existing template.
    3. Select Send.
    4. If you’re sending emails, select Upload a list of email addresses. If you’re sending text messages, select Upload a list of phone numbers. If you’re sending letters, select Upload a list of addresses.
      5. diff --git a/app/templates/views/guidance/edit-and-format-messages.html b/app/templates/views/guidance/edit-and-format-messages.html index 18e0bdf33..a97d57484 100644 --- a/app/templates/views/guidance/edit-and-format-messages.html +++ b/app/templates/views/guidance/edit-and-format-messages.html @@ -1,5 +1,5 @@ {% extends "content_template.html" %} -{% from "components/current-service-link.html" import current_service_link %} +{% from "components/service-link.html" import service_link %} {% block per_page_title %} Edit and format messages @@ -22,7 +22,7 @@

      You can see a list of formatting instructions on the edit template page:

        -
      1. Go to the {{ current_service_link('main.choose_template', 'templates') }} page.
        2. +
      2. Go to the {{ service_link(current_service, 'main.choose_template', 'templates') }} page.
      3. Add a new template or choose an existing template and select Edit.
      @@ -61,7 +61,7 @@

      To add a placeholder to the template:

        -
      1. Go to the {{ current_service_link('main.choose_template', 'templates') }} page.
        2. +
      2. Go to the {{ service_link(current_service, 'main.choose_template', 'templates') }} page.
      3. Add a new template or choose an existing template and select Edit.
      4. Add a placeholder using double brackets. For example: Hello ((first name)), your reference is ((ref number)).
      5. Select Save.
        6. @@ -81,7 +81,7 @@

        To add optional content to your emails and text messages:

          -
        1. Go to the {{ current_service_link('main.choose_template', 'templates') }} page.
          2. +
        2. Go to the {{ service_link(current_service, 'main.choose_template', 'templates') }} page.
        3. Add a new template or choose an existing template and select Edit.
        4. Use double brackets and ?? to define optional content. For example, if you only want to show something to people who are under 18: ((under18??Please get your application signed by a parent or guardian.))
        5. Select Save.
          6. diff --git a/app/templates/views/guidance/upload-a-letter.html b/app/templates/views/guidance/upload-a-letter.html index d4c36ab5f..8211c3de4 100644 --- a/app/templates/views/guidance/upload-a-letter.html +++ b/app/templates/views/guidance/upload-a-letter.html @@ -1,5 +1,5 @@ {% extends "content_template.html" %} -{% from "components/current-service-link.html" import current_service_link %} +{% from "components/service-link.html" import service_link %} {% block per_page_title %} Upload a letter @@ -14,7 +14,7 @@

          To upload and send a letter from a PDF file:

            -
          1. Go to the {{ current_service_link('main.uploads', 'uploads') }} page.
            2. +
          2. Go to the {{ service_link(current_service, 'main.uploads', 'uploads') }} page.
          3. Select Upload a letter.
          4. Select Choose file.
          diff --git a/app/templates/views/notifications/notification.html b/app/templates/views/notifications/notification.html index 006ac0bef..288cb7ee4 100644 --- a/app/templates/views/notifications/notification.html +++ b/app/templates/views/notifications/notification.html @@ -17,7 +17,11 @@ ) }}

          {% if is_precompiled_letter %} - Provided as PDF + {% if created_by %} + Uploaded + {% else %} + Provided as PDF + {% endif %} {% else %} {% if help %} ‘{{ template.name }}’ @@ -34,7 +38,7 @@ {% elif created_by %} by {{ created_by.name }} {% endif %} - on {{ created_at|format_datetime_short }} + {{ created_at|format_datetime_human }}

          {% if template.template_type == 'letter' %} @@ -67,7 +71,7 @@ {{ letter_print_day }}

          - Estimated delivery date: {{ estimated_letter_delivery_date|string|format_date_short }} + Estimated delivery date: {{ estimated_letter_delivery_date|format_day_of_week }} {{ estimated_letter_delivery_date|format_date_short }}

          {% endif %} {% 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/test_errorhandlers.py b/tests/app/main/test_errorhandlers.py index 6c52612bb..eeb6bac00 100644 --- a/tests/app/main/test_errorhandlers.py +++ b/tests/app/main/test_errorhandlers.py @@ -51,8 +51,8 @@ def test_csrf_returns_400(logged_in_client, mocker): assert response.status_code == 400 page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - assert page.h1.string.strip() == 'Something went wrong, please go back and try again.' - assert page.title.string.strip() == 'Bad request – GOV.UK Notify' + assert page.h1.string.strip() == 'Sorry, there’s a problem with GOV.UK Notify' + assert page.title.string.strip() == 'Sorry, there’s a problem with the service – GOV.UK Notify' def test_csrf_redirects_to_sign_in_page_if_not_signed_in(client, mocker): @@ -70,5 +70,5 @@ def test_405_returns_something_went_wrong_page(client, mocker): assert response.status_code == 405 page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - assert page.h1.string.strip() == 'Something went wrong, please go back and try again.' - assert page.title.string.strip() == 'Bad request – GOV.UK Notify' + assert page.h1.string.strip() == 'Sorry, there’s a problem with GOV.UK Notify' + assert page.title.string.strip() == 'Sorry, there’s a problem with the service – GOV.UK Notify' 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_index.py b/tests/app/main/views/test_index.py index 3b0de3657..bd4cea5a4 100644 --- a/tests/app/main/views/test_index.py +++ b/tests/app/main/views/test_index.py @@ -5,7 +5,7 @@ from bs4 import BeautifulSoup from flask import url_for from app.main.forms import FieldWithNoneOption -from tests.conftest import normalize_spaces, sample_uuid +from tests.conftest import SERVICE_ONE_ID, normalize_spaces, sample_uuid def test_non_logged_in_user_can_see_homepage( @@ -109,6 +109,34 @@ def test_static_pages( request() +def test_guidance_pages_link_to_service_pages_when_signed_in( + client_request, +): + request = partial(client_request.get, 'main.edit_and_format_messages') + selector = '.list-number li a' + + # Check the page loads when user is signed in + page = request() + assert page.select_one(selector)['href'] == url_for( + 'main.choose_template', + service_id=SERVICE_ONE_ID, + ) + + # Check it still works when they don’t have a recent service + with client_request.session_transaction() as session: + session['service_id'] = None + page = request() + assert not page.select_one(selector) + + # Check it still works when they sign out + client_request.logout() + with client_request.session_transaction() as session: + session['service_id'] = None + session['user_id'] = None + page = request() + assert not page.select_one(selector) + + @pytest.mark.parametrize('view, expected_view', [ ('information_risk_management', 'security'), ('old_integration_testing', 'integration_testing'), diff --git a/tests/app/main/views/test_notifications.py b/tests/app/main/views/test_notifications.py index 012ccaa1a..590e61c89 100644 --- a/tests/app/main/views/test_notifications.py +++ b/tests/app/main/views/test_notifications.py @@ -152,22 +152,43 @@ def test_notification_status_shows_expected_back_link( assert back_link is None -@freeze_time("2012-01-01 01:01") +@pytest.mark.parametrize('time_of_viewing_page, expected_message', ( + ('2012-01-01 01:01', ( + "‘sample template’ was sent by Test User today at 1:01am" + )), + ('2012-01-02 01:01', ( + "‘sample template’ was sent by Test User yesterday at 1:01am" + )), + ('2012-01-03 01:01', ( + "‘sample template’ was sent by Test User on 1 January at 1:01am" + )), + ('2013-01-03 01:01', ( + "‘sample template’ was sent by Test User on 1 January 2012 at 1:01am" + )), +)) def test_notification_page_doesnt_link_to_template_in_tour( + mocker, client_request, fake_uuid, mock_get_notification, + time_of_viewing_page, + expected_message, ): - page = client_request.get( - 'main.view_notification', - service_id=SERVICE_ONE_ID, - notification_id=fake_uuid, - help=3, - ) + with freeze_time('2012-01-01 01:01'): + notification = create_notification() + mocker.patch('app.notification_api_client.get_notification', return_value=notification) + + with freeze_time(time_of_viewing_page): + page = client_request.get( + 'main.view_notification', + service_id=SERVICE_ONE_ID, + notification_id=fake_uuid, + help=3, + ) assert normalize_spaces(page.select('main p:nth-of-type(1)')[0].text) == ( - "‘sample template’ was sent by Test User on 1 January at 1:01am" + expected_message ) assert len(page.select('main p:nth-of-type(1) a')) == 0 @@ -196,13 +217,13 @@ def test_notification_page_shows_page_for_letter_notification( ) assert normalize_spaces(page.select('main p:nth-of-type(1)')[0].text) == ( - "‘sample template’ was sent by Test User on 1 January at 1:01am" + "‘sample template’ was sent by Test User today at 1:01am" ) assert normalize_spaces(page.select('main p:nth-of-type(2)')[0].text) == ( 'Printing starts today at 5:30pm' ) assert normalize_spaces(page.select('main p:nth-of-type(3)')[0].text) == ( - 'Estimated delivery date: 6 January' + 'Estimated delivery date: Wednesday 6 January' ) assert len(page.select('.letter-postage')) == 1 assert normalize_spaces(page.select_one('.letter-postage').text) == ( @@ -226,17 +247,62 @@ def test_notification_page_shows_page_for_letter_notification( assert mock_page_count.call_args_list[0][1]['values'] == {'name': 'Jo'} +@freeze_time("2020-01-01 00:00") +def test_notification_page_shows_uploaded_letter( + client_request, + mocker, + fake_uuid, +): + mocker.patch( + 'app.main.views.notifications.view_letter_notification_as_preview', + return_value=(b'foo', { + 'message': '', + 'invalid_pages': '[]', + 'page_count': '1' + }) + ) + mocker.patch( + 'app.main.views.notifications.pdf_page_count', + return_value=1 + ) + mocker.patch( + 'app.main.views.notifications.get_page_count_for_letter', + return_value=1, + ) + + notification = create_notification( + notification_status='created', + template_type='letter', + is_precompiled_letter=True, + sent_one_off=True, + ) + mocker.patch('app.notification_api_client.get_notification', return_value=notification) + + page = client_request.get( + 'main.view_notification', + service_id=SERVICE_ONE_ID, + notification_id=fake_uuid, + ) + + assert normalize_spaces(page.select('main p:nth-of-type(1)')[0].text) == ( + 'Uploaded by Test User yesterday at midnight' + ) + assert normalize_spaces(page.select('main p:nth-of-type(2)')[0].text) == ( + 'Printing starts today at 5:30pm' + ) + + @freeze_time("2016-01-01 01:01") @pytest.mark.parametrize('is_precompiled_letter, expected_p1, expected_p2, expected_postage', ( ( True, - 'Provided as PDF on 1 January at 1:01am', + 'Provided as PDF today at 1:01am', 'This letter passed our checks, but we will not print it because you used a test key.', 'Postage: second class' ), ( False, - '‘sample template’ was sent on 1 January at 1:01am', + '‘sample template’ was sent today at 1:01am', 'We will not print this letter because you used a test key.', 'Postage: second class', ), @@ -377,7 +443,7 @@ def test_notification_page_shows_cancelled_or_failed_letter( ) assert normalize_spaces(page.select('main p')[0].text) == ( - "‘sample template’ was sent by Test User on 1 January at 1:01am" + "‘sample template’ was sent by Test User today at 1:01am" ) assert normalize_spaces(page.select('main p')[1].text) == ( expected_message @@ -473,7 +539,9 @@ def test_notification_page_shows_page_for_first_class_letter_notification( ) assert normalize_spaces(page.select('main p:nth-of-type(2)')[0].text) == 'Printing starts tomorrow at 5:30pm' - assert normalize_spaces(page.select('main p:nth-of-type(3)')[0].text) == 'Estimated delivery date: 5 January' + assert normalize_spaces(page.select('main p:nth-of-type(3)')[0].text) == ( + 'Estimated delivery date: Tuesday 5 January' + ) assert normalize_spaces(page.select_one('.letter-postage').text) == ( 'Postage: first class' ) 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/app/main/views/test_uploads.py b/tests/app/main/views/test_uploads.py index b4d1c9804..081e2fe35 100644 --- a/tests/app/main/views/test_uploads.py +++ b/tests/app/main/views/test_uploads.py @@ -504,6 +504,7 @@ def test_uploaded_letter_preview_image_400s_for_bad_page_type( file_id=fake_uuid, service_id=SERVICE_ONE_ID, page='foo', + _test_page_title=False, _expected_status=400, ) 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(), diff --git a/tests/javascripts/analytics/analytics.test.js b/tests/javascripts/analytics/analytics.test.js index ea4b91fe2..1f11544e8 100644 --- a/tests/javascripts/analytics/analytics.test.js +++ b/tests/javascripts/analytics/analytics.test.js @@ -52,6 +52,7 @@ describe("Analytics", () => { expect(setUpArguments[1]).toEqual(['set', 'anonymizeIp', true]); expect(setUpArguments[2]).toEqual(['set', 'allowAdFeatures', false]); expect(setUpArguments[3]).toEqual(['set', 'transport', 'beacon']); + expect(setUpArguments[4]).toEqual(['set', 'title', 'GOV.UK Notify']); }); diff --git a/tests/javascripts/analytics/init.test.js b/tests/javascripts/analytics/init.test.js index 8384b826f..6bf7a31c8 100644 --- a/tests/javascripts/analytics/init.test.js +++ b/tests/javascripts/analytics/init.test.js @@ -111,10 +111,10 @@ describe("Analytics init", () => { test("A pageview will be registered", () => { - expect(window.ga.mock.calls.length).toEqual(5); + expect(window.ga.mock.calls.length).toEqual(6); - // The first 4 calls configure the analytics tracker. All subsequent calls send data - expect(window.ga.mock.calls[4]).toEqual(['send', 'pageview', '/privacy']); + // The first 5 calls configure the analytics tracker. All subsequent calls send data + expect(window.ga.mock.calls[5]).toEqual(['send', 'pageview', '/privacy']); }); diff --git a/tests/javascripts/cookieSettings.test.js b/tests/javascripts/cookieSettings.test.js index 0fbef9677..d4416ab73 100644 --- a/tests/javascripts/cookieSettings.test.js +++ b/tests/javascripts/cookieSettings.test.js @@ -231,8 +231,8 @@ describe("Cookie settings", () => { expect(window.GOVUK.initAnalytics).toHaveBeenCalled(); expect(window.ga).toHaveBeenCalled(); - // the first 4 calls are configuration - expect(window.ga.mock.calls[4]).toEqual(['send', 'pageview', '/privacy']); + // the first 5 calls are configuration + expect(window.ga.mock.calls[5]).toEqual(['send', 'pageview', '/privacy']); });