+ This agreement needs to be signed by someone who has the authority to do so on behalf of your whole organisation. Typically this is a director of digital or head of finance. +
++ Return a signed copy to notify-support@digital.cabinet-office.gov.uk +
+
+ The agreement contains commercially sensitive information.
+ Do not share it more widely than you need to.
+
Your organisation ({{ agreement_info.owner }}) has already accepted the GOV.UK Notify data sharing and financial agreement.
- {% else %} -- Your organisation - {% if agreement_info.owner %} - ({{ agreement_info.owner }}) - must also accept our data sharing and financial agreement. - Contact us to get a copy. - {% else %} - must also accept our data sharing and financial agreement. - Contact us to get a copy. - {% endif %} -
- {% endif %} ++ {{ agreement_info.as_terms_of_use_paragraph( + download_link=url_for('.agreement'), + contact_link=url_for('.feedback', ticket_type='ask-question-give-feedback', body='agreement-with-owner') + )}} +
You must:
diff --git a/app/utils.py b/app/utils.py index 75a5a2138..3aeb6f373 100644 --- a/app/utils.py +++ b/app/utils.py @@ -15,7 +15,15 @@ import dateutil import pyexcel import pytz import yaml -from flask import abort, current_app, redirect, request, session, url_for +from flask import ( + Markup, + abort, + current_app, + redirect, + request, + session, + url_for, +) from flask_login import current_user from notifications_utils.recipients import RecipientCSV from notifications_utils.template import ( @@ -464,6 +472,42 @@ class AgreementInfo: else: return 'Can’t tell' + def as_terms_of_use_paragraph(self, **kwargs): + return Markup(self._as_terms_of_use_paragraph(**kwargs)) + + def _as_terms_of_use_paragraph(self, download_link, contact_link): + + if self.agreement_signed: + return ( + 'Your organisation ({}) has already accepted the ' + 'GOV.UK Notify data sharing and financial ' + 'agreement.'.format(self.owner) + ) + + if self.crown_status is False: + return (( + '{} Download a copy.' + ).format(self._acceptance_required, download_link)) + + return (( + '{} Contact us to get a copy.' + ).format(self._acceptance_required, contact_link)) + + @property + def _acceptance_required(self): + return ( + 'Your organisation {} must also accept our data sharing ' + 'and financial agreement.'.format( + '({})'.format(self.owner) if self.owner else '', + ) + ) + + @property + def crown_status_or_404(self): + if self.crown_status in {None, True}: + abort(404) + return self.crown_status + def as_request_for_agreement(self, with_owner=False): if with_owner and self.owner: return ( diff --git a/tests/app/main/views/test_agreement.py b/tests/app/main/views/test_agreement.py new file mode 100644 index 000000000..532de16da --- /dev/null +++ b/tests/app/main/views/test_agreement.py @@ -0,0 +1,103 @@ +from io import BytesIO + +import pytest +from flask import url_for +from tests.conftest import active_user_with_permissions + + +class _MockS3Object(): + + def __init__(self, data=None): + self.data = data or b'' + + def get(self): + return {'Body': BytesIO(self.data)} + + +@pytest.mark.parametrize('email_address, expected_status', [ + ('test@cabinet-office.gov.uk', 404), + ('test@aylesburytowncouncil.gov.uk', 200), + ('test@unknown.gov.uk', 404), +]) +def test_show_agreement_page( + client_request, + mocker, + fake_uuid, + email_address, + expected_status, +): + user = active_user_with_permissions(fake_uuid) + user.email_address = email_address + mocker.patch('app.user_api_client.get_user', return_value=user) + client_request.get( + 'main.agreement', + _expected_status=expected_status, + ) + + +@pytest.mark.parametrize('email_address, expected_file_fetched, expected_file_served', [ + pytest.mark.xfail(( + 'test@cabinet-office.gov.uk', + 'crown.pdf', + 'GOV.UK Notify data sharing and financial agreement.pdf', + ), raises=AssertionError), + ( + 'test@aylesburytowncouncil.gov.uk', + 'non-crown.pdf', + 'GOV.UK Notify data sharing and financial agreement (non-crown).pdf', + ), +]) +def test_downloading_agreement( + logged_in_client, + mocker, + fake_uuid, + email_address, + expected_file_fetched, + expected_file_served, +): + mock_get_s3_object = mocker.patch( + 'app.main.s3_client.get_s3_object', + return_value=_MockS3Object(b'foo') + ) + user = active_user_with_permissions(fake_uuid) + user.email_address = email_address + mocker.patch('app.user_api_client.get_user', return_value=user) + response = logged_in_client.get(url_for('main.download_agreement')) + assert response.status_code == 200 + 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) + + +def test_agreement_cant_be_downloaded_unknown_crown_status( + logged_in_client, + mocker, + fake_uuid, +): + mock_get_s3_object = mocker.patch( + 'app.main.s3_client.get_s3_object', + return_value=_MockS3Object() + ) + user = active_user_with_permissions(fake_uuid) + user.email_address = 'test@unknown.gov.uk' + mocker.patch('app.user_api_client.get_user', return_value=user) + response = logged_in_client.get(url_for('main.download_agreement')) + assert response.status_code == 404 + assert mock_get_s3_object.call_args_list == [] + + +def test_agreement_requires_login( + client, + mocker, +): + mock_get_s3_object = mocker.patch( + 'app.main.s3_client.get_s3_object', + return_value=_MockS3Object() + ) + response = client.get(url_for('main.download_agreement')) + assert response.status_code == 302 + assert response.location == 'http://localhost/sign-in?next=%2Fagreement.pdf' + assert mock_get_s3_object.call_args_list == [] diff --git a/tests/app/main/views/test_index.py b/tests/app/main/views/test_index.py index 4ee404698..f87b43951 100644 --- a/tests/app/main/views/test_index.py +++ b/tests/app/main/views/test_index.py @@ -1,3 +1,5 @@ +from functools import partial + import pytest from bs4 import BeautifulSoup from flask import url_for @@ -102,13 +104,19 @@ def test_terms_is_generic_if_user_is_not_logged_in( ) -@pytest.mark.parametrize('email_address, expected_terms_paragraph, expected_pricing_paragraph', [ +@pytest.mark.parametrize(( + 'email_address,' + 'expected_terms_paragraph,' + 'expected_terms_link,' + 'expected_pricing_paragraph' +), [ ( 'test@cabinet-office.gov.uk', ( 'Your organisation (Cabinet Office) has already accepted ' 'the GOV.UK Notify data sharing and financial agreement.' ), + None, ( 'Contact us to get a copy of the agreement ' '(Cabinet Office has already accepted it).' @@ -118,8 +126,12 @@ def test_terms_is_generic_if_user_is_not_logged_in( 'test@aylesburytowncouncil.gov.uk', ( 'Your organisation (Aylesbury Town Council) must also ' - 'accept our data sharing and financial agreement. Contact ' - 'us to get a copy.' + 'accept our data sharing and financial agreement. Download ' + 'a copy.' + ), + partial( + url_for, + 'main.agreement', ), ( 'Contact us to get a copy of the agreement ' @@ -132,11 +144,34 @@ def test_terms_is_generic_if_user_is_not_logged_in( 'Your organisation must also accept our data sharing and ' 'financial agreement. Contact us to get a copy.' ), + partial( + url_for, + 'main.feedback', + ticket_type='ask-question-give-feedback', + body='agreement-with-owner', + ), ( 'Contact us to get a copy of the agreement or find out if ' 'we already have one in place with your organisation.' ), ), + ( + 'michael.fish@metoffice.gov.uk', + ( + 'Your organisation (Met Office) must also accept our data ' + 'sharing and financial agreement. Contact us to get a copy.' + ), + partial( + url_for, + 'main.feedback', + ticket_type='ask-question-give-feedback', + body='agreement-with-owner', + ), + ( + 'Contact us to get a copy of the agreement (Met Office ' + 'hasn’t accepted it yet).' + ), + ), ]) def test_terms_tells_logged_in_users_what_we_know_about_their_agreement( mocker, @@ -144,6 +179,7 @@ def test_terms_tells_logged_in_users_what_we_know_about_their_agreement( client_request, email_address, expected_terms_paragraph, + expected_terms_link, expected_pricing_paragraph, ): user = active_user_with_permissions(fake_uuid) @@ -152,4 +188,8 @@ def test_terms_tells_logged_in_users_what_we_know_about_their_agreement( terms_page = client_request.get('main.terms') pricing_page = client_request.get('main.pricing') assert normalize_spaces(terms_page.select('main p')[1].text) == expected_terms_paragraph + if expected_terms_link: + assert terms_page.select_one('main p a')['href'] == expected_terms_link() + else: + assert not terms_page.select_one('main p').select('a') assert normalize_spaces(pricing_page.select('main p')[-1].text) == expected_pricing_paragraph