From 66c50abc388e85da04eefc999eb4f72eaa77b4a4 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Tue, 7 Dec 2021 13:42:44 +0000 Subject: [PATCH] Add new 'Billing' page for organisations We want organisation team members to be able to see the MOU details for their organisation. This change creates a new page called billing, which contains these details. It's only visible to platform admin users now - the plan is to add more information to this page, then to make it visible to all organisation users. The page showing the MOU covers the case of when agreement_signed is True, when an agreement_signed is False, and when agreement_signed is None. The case when an agreement_signed is None is very rare - it signifies that the agreement is not signed but that we have some service-specific agreements in place. We only have a few organisations in this state, so it's unlikely that the content for this scenario will be seen. When an organisation has signed the agreement we may know the full details (signing date, version signed, the person who signed it or who it was signed on behalf of), or we may only have the name of the person who signed the agreement. We show the more detailed content if possible, and a less detailed version of the content if not. There's a new route for downloading the agreement which is almost identical to the existing `.service_download_agreement` route (plus the test is almost the same), except that it takes an organisation ID instead of a service ID. --- app/main/views/organisations.py | 27 +++- app/navigation.py | 3 + app/templates/org_nav.html | 1 + .../organisations/organisation/billing.html | 50 ++++++ .../views/organisations/test_organisations.py | 142 ++++++++++++++++++ tests/app/main/views/test_agreement.py | 6 +- tests/app/test_navigation.py | 2 + 7 files changed, 227 insertions(+), 4 deletions(-) create mode 100644 app/templates/views/organisations/organisation/billing.html diff --git a/app/main/views/organisations.py b/app/main/views/organisations.py index c229d3e91..5e4f2bea9 100644 --- a/app/main/views/organisations.py +++ b/app/main/views/organisations.py @@ -2,7 +2,15 @@ from collections import OrderedDict from datetime import datetime from functools import partial -from flask import flash, redirect, render_template, request, session, url_for +from flask import ( + flash, + redirect, + render_template, + request, + send_file, + session, + url_for, +) from flask_login import current_user from notifications_python_client.errors import HTTPError from werkzeug.exceptions import abort @@ -44,6 +52,7 @@ from app.main.views.dashboard import ( from app.main.views.service_settings import get_branding_as_value_and_label from app.models.organisation import AllOrganisations, Organisation from app.models.user import InvitedOrgUser, User +from app.s3_client.s3_mou_client import get_mou from app.utils.csv import Spreadsheet from app.utils.user import user_has_permissions, user_is_platform_admin @@ -640,3 +649,19 @@ def edit_organisation_billing_details(org_id): 'views/organisations/organisation/settings/edit-organisation-billing-details.html', form=form, ) + + +@main.route("/organisations//billing") +@user_is_platform_admin +def organisation_billing(org_id): + return render_template( + 'views/organisations/organisation/billing.html' + ) + + +@main.route('/organisations//agreement.pdf') +@user_is_platform_admin +def organisation_download_agreement(org_id): + return send_file(**get_mou( + current_organisation.crown_status_or_404 + )) diff --git a/app/navigation.py b/app/navigation.py index 8fb983ea0..8d3b4a7e5 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -349,5 +349,8 @@ class OrgNavigation(Navigation): }, 'trial-services': { 'organisation_trial_mode_services', + }, + 'billing': { + 'organisation_billing', } } diff --git a/app/templates/org_nav.html b/app/templates/org_nav.html index 7d7156ab4..4e775e619 100644 --- a/app/templates/org_nav.html +++ b/app/templates/org_nav.html @@ -5,6 +5,7 @@ {% if current_user.platform_admin %}
  • Settings
  • Trial mode services
    • +
  • Billing
    • {% endif %} diff --git a/app/templates/views/organisations/organisation/billing.html b/app/templates/views/organisations/organisation/billing.html new file mode 100644 index 000000000..5c272fef1 --- /dev/null +++ b/app/templates/views/organisations/organisation/billing.html @@ -0,0 +1,50 @@ +{% extends "org_template.html" %} +{% from "components/page-header.html" import page_header %} + +{% block org_page_title %} + Billing +{% endblock %} + +{% block maincolumn_content %} + {{ page_header("Billing", size="medium") }} +
    +
    +

    + Data sharing and financial agreement +

    + + {% if current_org.agreement_signed_at %} +

    + Your organisation accepted version {{ current_org.agreement_signed_version }} of the GOV.UK Notify data sharing + and financial agreement on {{ current_org.agreement_signed_at | format_date_normal }}. +

    +

    + {{ current_org.agreement_signed_by.name or current_org.agreement_signed_on_behalf_of_name }} signed the agreement + on behalf of {{ current_org.name}}. +

    +

    + Download the current version of the agreement + +

    + {% elif current_org.agreement_signed %} +

    + {{ current_org.name}} has accepted the GOV.UK Notify data sharing and financial agreement. +

    +

    + Download the current version of the agreement + +

    + {% elif current_org.agreement_signed is false %} +

    + {{ current_org.name}} needs to accept the GOV.UK Notify data sharing and financial agreement. +

    + {% elif current_org.agreement_signed is none %} +

    + {{ current_org.name}} has not accepted the GOV.UK Notify data sharing and financial agreement, but we have some service-specific agreements in place. +

    + {% endif %} +
    +
    +{% endblock %} diff --git a/tests/app/main/views/organisations/test_organisations.py b/tests/app/main/views/organisations/test_organisations.py index 973078f04..c39ec2f46 100644 --- a/tests/app/main/views/organisations/test_organisations.py +++ b/tests/app/main/views/organisations/test_organisations.py @@ -5,6 +5,7 @@ from freezegun import freeze_time from notifications_python_client.errors import HTTPError from tests import organisation_json, service_json +from tests.app.main.views.test_agreement import MockS3Object from tests.conftest import ( ORGANISATION_ID, SERVICE_ONE_ID, @@ -1583,3 +1584,144 @@ def test_update_organisation_billing_details_errors_when_user_not_platform_admin _data={'notes': "Very fluffy"}, _expected_status=403, ) + + +def test_organisation_billing_page_not_accessible_if_not_platform_admin( + client_request, + mock_get_organisation, +): + client_request.get( + '.organisation_billing', + org_id=ORGANISATION_ID, + _expected_status=403 + ) + + +@pytest.mark.parametrize('signed_by_id, signed_by_name, expected_signatory', [ + ('1234', None, 'Test User'), + (None, 'The Org Manager', 'The Org Manager'), +]) +def test_organisation_billing_page_when_the_agreement_is_signed_by_a_known_person( + organisation_one, + platform_admin_client, + api_user_active, + mocker, + platform_admin_user, + signed_by_id, + signed_by_name, + expected_signatory, +): + api_user_active['id'] = '1234' + + organisation_one['agreement_signed'] = True + organisation_one['agreement_signed_version'] = 2.5 + organisation_one['agreement_signed_by_id'] = signed_by_id + organisation_one['agreement_signed_on_behalf_of_name'] = signed_by_name + organisation_one['agreement_signed_at'] = 'Thu, 20 Feb 2020 00:00:00 GMT' + + mocker.patch('app.organisations_client.get_organisation', return_value=organisation_one) + mocker.patch('app.user_api_client.get_user', side_effect=[platform_admin_user, api_user_active]) + + response = platform_admin_client.get( + url_for('.organisation_billing', org_id=ORGANISATION_ID) + ) + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + + assert page.h1.string == 'Billing' + assert '2.5 of the GOV.UK Notify data sharing and financial agreement on 20 February 2020' in normalize_spaces( + page.text) + assert f'{expected_signatory} signed' in page.text + assert page.select_one('main a')['href'] == url_for('.organisation_download_agreement', org_id=ORGANISATION_ID) + + +def test_organisation_billing_page_when_the_agreement_is_signed_by_an_unknown_person( + organisation_one, + platform_admin_client, + mocker, +): + organisation_one['agreement_signed'] = True + mocker.patch('app.organisations_client.get_organisation', return_value=organisation_one) + + response = platform_admin_client.get( + url_for('.organisation_billing', org_id=ORGANISATION_ID) + ) + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + + assert page.h1.string == 'Billing' + assert (f'{organisation_one["name"]} has accepted the GOV.UK Notify data ' + 'sharing and financial agreement.') in page.text + assert page.select_one('main a')['href'] == url_for('.organisation_download_agreement', org_id=ORGANISATION_ID) + + +@pytest.mark.parametrize('agreement_signed, expected_content', [ + (False, 'needs to accept'), + (None, 'has not accepted'), +]) +def test_organisation_billing_page_when_the_agreement_is_not_signed( + organisation_one, + platform_admin_client, + mocker, + agreement_signed, + expected_content, +): + organisation_one['agreement_signed'] = agreement_signed + mocker.patch('app.organisations_client.get_organisation', return_value=organisation_one) + + response = platform_admin_client.get( + url_for('.organisation_billing', org_id=ORGANISATION_ID) + ) + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + + assert page.h1.string == 'Billing' + assert f'{organisation_one["name"]} {expected_content}' in page.text + + +@pytest.mark.parametrize('crown, expected_status, expected_file_fetched, expected_file_served', ( + ( + True, 200, 'crown.pdf', + 'GOV.UK Notify data sharing and financial agreement.pdf', + ), + ( + False, 200, 'non-crown.pdf', + 'GOV.UK Notify data sharing and financial agreement (non-crown).pdf', + ), + ( + None, 404, None, + None, + ), +)) +def test_download_organisation_agreement( + platform_admin_client, + mocker, + crown, + expected_status, + expected_file_fetched, + expected_file_served, +): + mocker.patch( + 'app.models.organisation.organisations_client.get_organisation', + return_value=organisation_json( + crown=crown + ) + ) + mock_get_s3_object = mocker.patch( + 'app.s3_client.s3_mou_client.get_s3_object', + return_value=MockS3Object(b'foo') + ) + + response = platform_admin_client.get(url_for( + 'main.organisation_download_agreement', + org_id=ORGANISATION_ID, + )) + assert response.status_code == expected_status + + if expected_file_served: + assert response.get_data() == b'foo' + assert response.headers['Content-Type'] == 'application/pdf' + assert response.headers['Content-Disposition'] == ( + f'attachment; filename="{expected_file_served}"' + ) + mock_get_s3_object.assert_called_once_with('test-mou', expected_file_fetched) + else: + assert not expected_file_fetched + assert mock_get_s3_object.called is False diff --git a/tests/app/main/views/test_agreement.py b/tests/app/main/views/test_agreement.py index e2c0ffcd6..e92f79e06 100644 --- a/tests/app/main/views/test_agreement.py +++ b/tests/app/main/views/test_agreement.py @@ -10,7 +10,7 @@ from tests import organisation_json from tests.conftest import ORGANISATION_ID, SERVICE_ONE_ID, normalize_spaces -class _MockS3Object(): +class MockS3Object(): def __init__(self, data=None): self.data = data or b'' @@ -157,7 +157,7 @@ def test_download_service_agreement( ) mock_get_s3_object = mocker.patch( 'app.s3_client.s3_mou_client.get_s3_object', - return_value=_MockS3Object(b'foo') + return_value=MockS3Object(b'foo') ) response = logged_in_client.get(url_for( @@ -506,7 +506,7 @@ def test_show_public_agreement_page( ): mocker.patch( 'app.s3_client.s3_mou_client.get_s3_object', - return_value=_MockS3Object() + return_value=MockS3Object() ) response = client.get(url_for( endpoint, diff --git a/tests/app/test_navigation.py b/tests/app/test_navigation.py index 0842d7773..55feeecf4 100644 --- a/tests/app/test_navigation.py +++ b/tests/app/test_navigation.py @@ -172,7 +172,9 @@ EXCLUDED_ENDPOINTS = tuple(map(Navigation.get_endpoint_with_blueprint, { 'old_service_dashboard', 'old_terms', 'old_using_notify', + 'organisation_billing', 'organisation_dashboard', + 'organisation_download_agreement', 'organisation_preview_email_branding', 'organisation_preview_letter_branding', 'organisation_settings',