From b75681dfbc87f13196fde53522352d270a990438 Mon Sep 17 00:00:00 2001 From: Alexey Bezhan Date: Thu, 6 Sep 2018 16:34:23 +0100 Subject: [PATCH] Add a platform admin page to submit returned letter references A platform admin form accepts a list of references (one per line) received from DVLA and sends them to the API to update notification statuses. References we get from DVLA start with `NOTIFY00\d`, which isn't part of the reference we store in the database, so we remove them before sending the data to the API. The new `returned-letter` status should be treated as `delivered` for now until we decide a way to display returned letters to users. --- app/__init__.py | 2 + app/main/forms.py | 9 ++++ app/main/views/platform_admin.py | 42 ++++++++++++++++++- app/navigation.py | 4 ++ app/notify_client/letter_jobs_client.py | 6 +++ .../views/platform-admin/_base_template.html | 3 +- .../platform-admin/returned-letters.html | 21 ++++++++++ app/utils.py | 2 +- tests/app/main/views/test_activity.py | 4 +- tests/app/main/views/test_jobs.py | 5 ++- tests/app/main/views/test_platform_admin.py | 34 +++++++++++++++ 11 files changed, 124 insertions(+), 8 deletions(-) create mode 100644 app/templates/views/platform-admin/returned-letters.html diff --git a/app/__init__.py b/app/__init__.py index 37781f2ef..a244407d6 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -385,6 +385,7 @@ def format_notification_status(status, template_type): 'sent': 'Delivered', 'pending-virus-check': 'Pending virus check', 'virus-scan-failed': 'Virus detected', + 'returned-letter': 'Delivered', } }[template_type].get(status, status) @@ -410,6 +411,7 @@ def format_notification_status_as_field_status(status, notification_type): 'accepted': None, 'pending-virus-check': None, 'virus-scan-failed': 'error', + 'returned-letter': None, } }.get( notification_type, diff --git a/app/main/forms.py b/app/main/forms.py index 00d50f15b..20cd9ad89 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -1067,3 +1067,12 @@ class ServiceDataRetentionEditForm(StripWhitespaceForm): validators=[validators.NumberRange(min=3, max=90, message="Must be between 3 and 90")], ) + + +class ReturnedLettersForm(StripWhitespaceForm): + references = TextAreaField( + u'Letter references', + validators=[ + DataRequired(message="Can’t be empty"), + ] + ) diff --git a/app/main/views/platform_admin.py b/app/main/views/platform_admin.py index 2074a5f90..ed710a50c 100644 --- a/app/main/views/platform_admin.py +++ b/app/main/views/platform_admin.py @@ -1,16 +1,19 @@ import itertools +import re from datetime import datetime -from flask import abort, render_template, request, url_for +from flask import abort, flash, redirect, render_template, request, url_for from flask_login import login_required +from notifications_python_client.errors import HTTPError from app import ( complaint_api_client, + letter_jobs_client, platform_stats_api_client, service_api_client, ) from app.main import main -from app.main.forms import DateFilterForm +from app.main.forms import DateFilterForm, ReturnedLettersForm from app.statistics_utils import ( get_formatted_percentage, get_formatted_percentage_two_dp, @@ -202,6 +205,41 @@ def platform_admin_list_complaints(): ) +@main.route("/platform-admin/returned-letters", methods=["GET", "POST"]) +@login_required +@user_is_platform_admin +def platform_admin_returned_letters(): + form = ReturnedLettersForm() + + if form.validate_on_submit(): + references = [ + re.sub('NOTIFY00[0-9]', '', r.strip()) + for r in form.references.data.split('\n') + if r.strip() + ] + + try: + letter_jobs_client.submit_returned_letters(references) + except HTTPError as error: + if error.status_code == 400: + error_references = [ + re.match('references (.*) does not match', e['message']).group(1) + for e in error.message + ] + form.references.errors.append("Invalid references: {}".format(', '.join(error_references))) + else: + raise error + else: + flash('Submitted {} letter references'.format(len(references)), 'default') + return redirect( + url_for('.platform_admin_returned_letters') + ) + return render_template( + 'views/platform-admin/returned-letters.html', + form=form, + ) + + def sum_service_usage(service): total = 0 for notification_type in service['statistics'].keys(): diff --git a/app/navigation.py b/app/navigation.py index f95426767..1b9e556a5 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -81,6 +81,7 @@ class HeaderNavigation(Navigation): 'organisations', 'platform_admin', 'platform_admin_list_complaints', + 'platform_admin_returned_letters', 'suspend_service', 'trial_services', 'update_email_branding', @@ -440,6 +441,7 @@ class MainNavigation(Navigation): 'organisations', 'platform_admin', 'platform_admin_list_complaints', + 'platform_admin_returned_letters', 'pricing', 'privacy', 'public_agreement', @@ -624,6 +626,7 @@ class CaseworkNavigation(Navigation): 'organisations', 'platform_admin', 'platform_admin_list_complaints', + 'platform_admin_returned_letters', 'pricing', 'privacy', 'public_agreement', @@ -848,6 +851,7 @@ class OrgNavigation(Navigation): 'organisations', 'platform_admin', 'platform_admin_list_complaints', + 'platform_admin_returned_letters', 'pricing', 'privacy', 'public_agreement', diff --git a/app/notify_client/letter_jobs_client.py b/app/notify_client/letter_jobs_client.py index efa35e696..079d31e12 100644 --- a/app/notify_client/letter_jobs_client.py +++ b/app/notify_client/letter_jobs_client.py @@ -14,3 +14,9 @@ class LetterJobsClient(NotifyAdminAPIClient): url='/send-letter-jobs', data={"job_ids": job_ids} )['data'] + + def submit_returned_letters(self, references): + return self.post( + url='/letters/returned', + data={'references': references} + ) diff --git a/app/templates/views/platform-admin/_base_template.html b/app/templates/views/platform-admin/_base_template.html index 7ea7ce43c..9f93036a7 100644 --- a/app/templates/views/platform-admin/_base_template.html +++ b/app/templates/views/platform-admin/_base_template.html @@ -21,7 +21,8 @@ ('Letter jobs', url_for('main.letter_jobs')), ('Inbound SMS numbers', url_for('main.inbound_sms_admin')), ('Find users by email', url_for('main.find_users_by_email')), - ('Email Complaints', url_for('main.platform_admin_list_complaints')) + ('Email Complaints', url_for('main.platform_admin_list_complaints')), + ('Returned letters', url_for('main.platform_admin_returned_letters')), ] %}
  • diff --git a/app/templates/views/platform-admin/returned-letters.html b/app/templates/views/platform-admin/returned-letters.html new file mode 100644 index 000000000..d8b2080cf --- /dev/null +++ b/app/templates/views/platform-admin/returned-letters.html @@ -0,0 +1,21 @@ +{% extends "views/platform-admin/_base_template.html" %} +{% from "components/textbox.html" import textbox %} +{% from "components/page-footer.html" import page_footer %} + +{% block per_page_title %} + {{ page_title|capitalize }} +{% endblock %} + +{% block platform_admin_content %} + +
    +
    +

    Submit returned letters

    +
    + {{ textbox(form.references, width='1-1', rows=8) }} + {{ page_footer("Submit") }} +
    +
    +
    + +{% endblock %} diff --git a/app/utils.py b/app/utils.py index 7b4dd5ca3..ca885084b 100644 --- a/app/utils.py +++ b/app/utils.py @@ -39,7 +39,7 @@ from orderedset._orderedset import OrderedSet from werkzeug.datastructures import MultiDict SENDING_STATUSES = ['created', 'pending', 'sending', 'pending-virus-check'] -DELIVERED_STATUSES = ['delivered', 'sent'] +DELIVERED_STATUSES = ['delivered', 'sent', 'returned-letter'] FAILURE_STATUSES = ['failed', 'temporary-failure', 'permanent-failure', 'technical-failure', 'virus-scan-failed'] REQUESTED_STATUSES = SENDING_STATUSES + DELIVERED_STATUSES + FAILURE_STATUSES diff --git a/tests/app/main/views/test_activity.py b/tests/app/main/views/test_activity.py index 2dfc2a198..1f5e312ca 100644 --- a/tests/app/main/views/test_activity.py +++ b/tests/app/main/views/test_activity.py @@ -32,7 +32,7 @@ from tests.conftest import ( '', [ 'created', 'pending', 'sending', 'pending-virus-check', - 'delivered', 'sent', + 'delivered', 'sent', 'returned-letter', 'failed', 'temporary-failure', 'permanent-failure', 'technical-failure', 'virus-scan-failed', ] ), @@ -42,7 +42,7 @@ from tests.conftest import ( ), ( 'delivered', - ['delivered', 'sent'] + ['delivered', 'sent', 'returned-letter'] ), ( 'failed', diff --git a/tests/app/main/views/test_jobs.py b/tests/app/main/views/test_jobs.py index 04de7bcb6..5f7ab65c8 100644 --- a/tests/app/main/views/test_jobs.py +++ b/tests/app/main/views/test_jobs.py @@ -155,7 +155,7 @@ def test_jobs_page_doesnt_show_scheduled_on_page_2( '', [ 'created', 'pending', 'sending', 'pending-virus-check', - 'delivered', 'sent', + 'delivered', 'sent', 'returned-letter', 'failed', 'temporary-failure', 'permanent-failure', 'technical-failure', 'virus-scan-failed', ] ), @@ -165,7 +165,7 @@ def test_jobs_page_doesnt_show_scheduled_on_page_2( ), ( 'delivered', - ['delivered', 'sent'] + ['delivered', 'sent', 'returned-letter'] ), ( 'failed', @@ -306,6 +306,7 @@ def test_should_show_letter_job( 'pending-virus-check', 'delivered', 'sent', + 'returned-letter', 'failed', 'temporary-failure', 'permanent-failure', diff --git a/tests/app/main/views/test_platform_admin.py b/tests/app/main/views/test_platform_admin.py index d67702178..77468123c 100644 --- a/tests/app/main/views/test_platform_admin.py +++ b/tests/app/main/views/test_platform_admin.py @@ -724,3 +724,37 @@ def test_platform_admin_displays_stats_in_right_boxes_and_with_correct_styling( # Letter virus scan failure status box - number is correct and failure class is used assert '1 virus scan failures' in page.find_all('div', class_='column-third')[2].find( 'div', class_='big-number-status-failing').text + + +def test_platform_admin_submit_returned_letters(mocker, client, platform_admin_user): + mock_get_user(mocker, user=platform_admin_user) + client.login(platform_admin_user) + + mock_client = mocker.patch('app.letter_jobs_client.submit_returned_letters') + + response = client.post( + url_for('main.platform_admin_returned_letters'), + data={'references': ' NOTIFY000REF1 \n NOTIFY002REF2 '} + ) + + mock_client.assert_called_once_with(['REF1', 'REF2']) + + assert response.status_code == 302 + assert response.location == url_for('main.platform_admin_returned_letters', _external=True) + + +def test_platform_admin_submit_empty_returned_letters(mocker, client, platform_admin_user): + mock_get_user(mocker, user=platform_admin_user) + client.login(platform_admin_user) + + mock_client = mocker.patch('app.letter_jobs_client.submit_returned_letters') + + response = client.post( + url_for('main.platform_admin_returned_letters'), + data={'references': ' \n '} + ) + + assert not mock_client.called + + assert response.status_code == 200 + assert "Can’t be empty" in response.get_data(as_text=True)