From 5368ddabbcecb2c3622800d05759946a71499a21 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 2 Jul 2019 16:46:23 +0100 Subject: [PATCH 1/2] Fix hard coded organisation name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 😳 --- app/templates/views/agreement/_agreement.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/templates/views/agreement/_agreement.html b/app/templates/views/agreement/_agreement.html index c9d1dfa9b..2cd9a48b4 100644 --- a/app/templates/views/agreement/_agreement.html +++ b/app/templates/views/agreement/_agreement.html @@ -1,5 +1,5 @@

- To use GOV.UK Notify your organisation (Cabinet Office) must accept the GOV.UK Notify data sharing and financial agreement. + To use GOV.UK Notify your organisation ({{ current_service.organisation.name }}) must accept the GOV.UK Notify data sharing and financial agreement.

This agreement only needs to be accepted once and will then cover all Notify services from From 1f02a2d3ab1891d19f54e72276c9a529c30debd4 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 2 Jul 2019 17:04:26 +0100 Subject: [PATCH 2/2] =?UTF-8?q?Determine=20agreement=20by=20service=20not?= =?UTF-8?q?=20user=E2=80=99s=20org?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A user might not have a guessable organisation type, even if the service they’re working on does have an organisation set. This can happen for users with @nhs.net email addresses, for example. --- app/main/views/agreement.py | 10 ++++ app/navigation.py | 4 ++ .../views/agreement/_agreement-signed.html | 2 +- app/templates/views/agreement/_agreement.html | 2 +- tests/app/main/views/test_agreement.py | 57 ++++++++++++++++++- 5 files changed, 70 insertions(+), 5 deletions(-) diff --git a/app/main/views/agreement.py b/app/main/views/agreement.py index 9e2b8de27..96008c5ac 100644 --- a/app/main/views/agreement.py +++ b/app/main/views/agreement.py @@ -8,6 +8,7 @@ from app.main import main from app.main.forms import AcceptAgreementForm from app.main.views.sub_navigation_dictionaries import features_nav from app.s3_client.s3_mou_client import get_mou +from app.utils import user_has_permissions @main.route('/agreement') @@ -29,6 +30,15 @@ def service_agreement(service_id): ) +@main.route('/services//agreement.pdf') +@login_required +@user_has_permissions('manage_service') +def service_download_agreement(service_id): + return send_file(**get_mou( + current_service.organisation.crown_status_or_404 + )) + + @main.route('/services//agreement/accept', methods=['GET', 'POST']) @login_required def service_accept_agreement(service_id): diff --git a/app/navigation.py b/app/navigation.py index ad3be0723..39f4238b6 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -252,6 +252,7 @@ class HeaderNavigation(Navigation): 'service_dashboard_updates', 'service_delete_email_reply_to', 'service_delete_sms_sender', + 'service_download_agreement', 'service_edit_email_reply_to', 'service_edit_letter_contact', 'service_edit_sms_sender', @@ -539,6 +540,7 @@ class MainNavigation(Navigation): 'service_dashboard_updates', 'service_delete_email_reply_to', 'service_delete_sms_sender', + 'service_download_agreement', 'service_letter_validation_preview', 'service_switch_can_upload_document', 'service_switch_count_as_live', @@ -777,6 +779,7 @@ class CaseworkNavigation(Navigation): 'service_dashboard_updates', 'service_delete_email_reply_to', 'service_delete_sms_sender', + 'service_download_agreement', 'service_edit_email_reply_to', 'service_edit_letter_contact', 'service_edit_sms_sender', @@ -1048,6 +1051,7 @@ class OrgNavigation(Navigation): 'service_dashboard_updates', 'service_delete_email_reply_to', 'service_delete_sms_sender', + 'service_download_agreement', 'service_edit_email_reply_to', 'service_edit_letter_contact', 'service_edit_sms_sender', diff --git a/app/templates/views/agreement/_agreement-signed.html b/app/templates/views/agreement/_agreement-signed.html index 35819b126..72af4a9f3 100644 --- a/app/templates/views/agreement/_agreement-signed.html +++ b/app/templates/views/agreement/_agreement-signed.html @@ -1,7 +1,7 @@

Your organisation ({{ owner }}) has already accepted the GOV.UK Notify data sharing and financial agreement. You can - download a copy. + download a copy.

The agreement contains commercially sensitive information, so don’t share it more widely than you need to. diff --git a/app/templates/views/agreement/_agreement.html b/app/templates/views/agreement/_agreement.html index 2cd9a48b4..2f79f7ab1 100644 --- a/app/templates/views/agreement/_agreement.html +++ b/app/templates/views/agreement/_agreement.html @@ -9,7 +9,7 @@ It needs to be accepted by, or on behalf of someone who can sign contracts for your organisation.

- Download a copy of the agreement. + Download a copy of the agreement.

The agreement contains commercially sensitive information, so don’t share it more widely than you need to. diff --git a/tests/app/main/views/test_agreement.py b/tests/app/main/views/test_agreement.py index 2002d4265..2dbf29274 100644 --- a/tests/app/main/views/test_agreement.py +++ b/tests/app/main/views/test_agreement.py @@ -46,20 +46,20 @@ class _MockS3Object(): ( True, True, [ - partial(url_for, 'main.download_agreement'), + partial(url_for, 'main.service_download_agreement', service_id=SERVICE_ONE_ID), ] ), ( False, False, [ - partial(url_for, 'main.download_agreement'), + partial(url_for, 'main.service_download_agreement', service_id=SERVICE_ONE_ID), partial(url_for, 'main.service_accept_agreement', service_id=SERVICE_ONE_ID), ] ), ( False, True, [ - partial(url_for, 'main.download_agreement'), + partial(url_for, 'main.service_download_agreement', service_id=SERVICE_ONE_ID), partial(url_for, 'main.service_accept_agreement', service_id=SERVICE_ONE_ID), ] ), @@ -98,6 +98,57 @@ def test_show_agreement_page( assert link['href'] == expected_links[index]() +@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_service_agreement( + logged_in_client, + mocker, + crown, + expected_status, + expected_file_fetched, + expected_file_served, +): + mocker.patch( + 'app.models.organisation.organisations_client.get_service_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 = logged_in_client.get(url_for( + 'main.service_download_agreement', + service_id=SERVICE_ONE_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'] == ( + 'attachment; filename="{}"'.format(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 + + def test_show_accept_agreement_page( client_request, mocker,