diff --git a/app/main/views/agreement.py b/app/main/views/agreement.py index cb58c236f..b3e41dbb5 100644 --- a/app/main/views/agreement.py +++ b/app/main/views/agreement.py @@ -13,10 +13,8 @@ def agreement(): agreement_info = AgreementInfo.from_current_user() - agreement_info.crown_status_or_404 - return render_template( - 'views/agreement.html', + 'views/{}.html'.format(agreement_info.as_jinja_template), owner=agreement_info.owner, navigation_links=features_nav(), ) diff --git a/app/main/views/feedback.py b/app/main/views/feedback.py index 4ff510aae..0a641a14b 100644 --- a/app/main/views/feedback.py +++ b/app/main/views/feedback.py @@ -12,20 +12,16 @@ from app import ( ) from app.main import main from app.main.forms import Feedback, Problem, SupportType, Triage -from app.utils import AgreementInfo QUESTION_TICKET_TYPE = 'ask-question-give-feedback' PROBLEM_TICKET_TYPE = "report-problem" def get_prefilled_message(): - agreement_info = AgreementInfo.from_current_user() return { 'agreement': ( - agreement_info.as_request_for_agreement() - ), - 'agreement-with-owner': ( - agreement_info.as_request_for_agreement(with_owner=True) + 'Please can you tell me if there’s an agreement in place ' + 'between GOV.UK Notify and my organisation?' ), }.get( request.args.get('body'), '' diff --git a/app/templates/views/agreement-choose.html b/app/templates/views/agreement-choose.html new file mode 100644 index 000000000..8ab9d1926 --- /dev/null +++ b/app/templates/views/agreement-choose.html @@ -0,0 +1,58 @@ +{% extends "withoutnav_template.html" %} +{% from "components/sub-navigation.html" import sub_navigation %} + +{% block per_page_title %} + Download the GOV.UK Notify data sharing and financial agreement +{% endblock %} + +{% block maincolumn_content %} + +
+ Before you can go live on GOV.UK Notify, your organisation needs to agree to our data sharing and financial agreement. +
++ Download the crown agreement. +
++ Download the non-crown agreement. +
++ Contact us if you’re + not sure whether your organisation is a crown or non-crown body. +
++ The agreement contains commercially sensitive information, so don’t share it more widely than you need to. +
++ Your organisation ({{ owner }}) has already accepted the GOV.UK + Notify data sharing and financial agreement. You can + download a copy. +
++ The agreement contains commercially sensitive information, so don’t share it more widely than you need to. +
+You can find details of how to pay for Notify in our data sharing and financial agreement.
- Contact us to get a copy of the agreement - {% if agreement_info.agreement_signed %} - ({{ agreement_info.owner }} has already accepted it). - {% else %} - {% if agreement_info.owner and agreement_info.agreement_signed != None %} - ({{ agreement_info.owner }} hasn’t accepted it yet). - {% else %} - or find out if we already have one in place with your organisation. - {% endif %} - {% endif %} + {{ agreement_info.as_pricing_paragraph( + pricing_link=url_for('main.sign_in', next=url_for('main.pricing', _anchor='paying')), + download_link=url_for('main.agreement'), + support_link=url_for('.feedback', ticket_type='ask-question-give-feedback', body='agreement'), + signed_in=current_user.is_authenticated, + ) }}
diff --git a/app/templates/views/terms-of-use.html b/app/templates/views/terms-of-use.html index 540b7f1e1..f0576bbbd 100644 --- a/app/templates/views/terms-of-use.html +++ b/app/templates/views/terms-of-use.html @@ -22,8 +22,10 @@ Terms of use{{ agreement_info.as_terms_of_use_paragraph( + terms_link=url_for('main.sign_in', next=url_for('main.terms')), download_link=url_for('.agreement'), - contact_link=url_for('.feedback', ticket_type='ask-question-give-feedback', body='agreement-with-owner') + support_link=url_for('.feedback', ticket_type='ask-question-give-feedback', body='agreement'), + signed_in=current_user.is_authenticated )}}
diff --git a/app/utils.py b/app/utils.py index 13a0aec44..69c85c280 100644 --- a/app/utils.py +++ b/app/utils.py @@ -472,26 +472,72 @@ class AgreementInfo: else: return 'Can’t tell' + @property + def as_jinja_template(self): + if self.crown_status is None: + return 'agreement-choose' + if self.agreement_signed: + return 'agreement-signed' + return 'agreement' + 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): + def _as_terms_of_use_paragraph(self, terms_link, download_link, support_link, signed_in): - if self.agreement_signed: - return ( - 'Your organisation ({}) has already accepted the ' - 'GOV.UK Notify data sharing and financial ' - 'agreement.'.format(self.owner) - ) + if not signed_in: + return (( + '{} Sign in to download a copy ' + 'or find out if one is already in place.' + ).format(self._acceptance_required, terms_link)) - if self.crown_status is not None: + if self.agreement_signed is None: + return (( + '{} Download the agreement or ' + 'contact us to find out if we already ' + 'have one in place with your organisation.' + ).format(self._acceptance_required, download_link, support_link)) + + if self.agreement_signed 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)) + return ( + 'Your organisation ({}) has already accepted the ' + 'GOV.UK Notify data sharing and financial ' + 'agreement.'.format(self.owner) + ) + + def as_pricing_paragraph(self, **kwargs): + return Markup(self._as_pricing_paragraph(**kwargs)) + + def _as_pricing_paragraph(self, pricing_link, download_link, support_link, signed_in): + + if not signed_in: + return (( + 'Sign in to download a copy or find ' + 'out if one is already in place with your organisation.' + ).format(pricing_link)) + + if self.agreement_signed is None: + return (( + 'Download the agreement or ' + 'contact us to find out if we already ' + 'have one in place with your organisation.' + ).format(download_link, support_link)) + + return ( + 'Download the agreement ' + '({} {}).'.format( + download_link, + self.owner, + { + True: 'has already accepted it', + False: 'hasn’t accepted it yet' + }.get(self.agreement_signed) + ) + ) @property def _acceptance_required(self): @@ -508,17 +554,6 @@ class AgreementInfo: abort(404) return self.crown_status - def as_request_for_agreement(self, with_owner=False): - if with_owner and self.owner: - return ( - 'Please send me a copy of the GOV.UK Notify data sharing ' - 'and financial agreement for {} to sign.'.format(self.owner) - ) - return ( - 'Please send me a copy of the GOV.UK Notify data sharing ' - 'and financial agreement.' - ) - @staticmethod def get_matching_function(email_address_or_domain): diff --git a/tests/app/main/views/test_agreement.py b/tests/app/main/views/test_agreement.py index c29d394ca..02bc6b34e 100644 --- a/tests/app/main/views/test_agreement.py +++ b/tests/app/main/views/test_agreement.py @@ -1,3 +1,4 @@ +from functools import partial from io import BytesIO import pytest @@ -15,25 +16,45 @@ class _MockS3Object(): return {'Body': BytesIO(self.data)} -@pytest.mark.parametrize('email_address, expected_status', [ - ('test@cabinet-office.gov.uk', 200), - ('test@aylesburytowncouncil.gov.uk', 200), - ('test@unknown.gov.uk', 404), +@pytest.mark.parametrize('email_address, expected_links', [ + ( + 'test@cabinet-office.gov.uk', + [ + partial(url_for, 'main.download_agreement'), + ] + ), + ( + 'test@aylesburytowncouncil.gov.uk', + [ + partial(url_for, 'main.download_agreement'), + lambda: 'mailto:notify-support@digital.cabinet-office.gov.uk', + ] + ), + ( + 'test@unknown.gov.uk', + [ + partial(url_for, 'main.public_download_agreement', variant='crown'), + partial(url_for, 'main.public_download_agreement', variant='non-crown'), + partial(url_for, 'main.support'), + lambda: 'mailto:notify-support@digital.cabinet-office.gov.uk', + ] + ), ]) def test_show_agreement_page( client_request, mocker, fake_uuid, email_address, - expected_status, + expected_links, ): 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, - ) + page = client_request.get('main.agreement') + links = page.select('main .column-two-thirds a') + assert len(links) == len(expected_links) + for index, link in enumerate(links): + assert link['href'] == expected_links[index]() @pytest.mark.parametrize('email_address, expected_file_fetched, expected_file_served', [ diff --git a/tests/app/main/views/test_feedback.py b/tests/app/main/views/test_feedback.py index 63aba1e40..87f6cf4d6 100644 --- a/tests/app/main/views/test_feedback.py +++ b/tests/app/main/views/test_feedback.py @@ -88,16 +88,8 @@ def test_get_feedback_page(client, ticket_type, expected_status_code): ( 'agreement', ( - 'Please send me a copy of the GOV.UK Notify data sharing ' - 'and financial agreement.' - ) - ), - ( - 'agreement-with-owner', - ( - 'Please send me a copy of the GOV.UK Notify data sharing ' - 'and financial agreement for Marine Management ' - 'Organisation to sign.' + 'Please can you tell me if there’s an agreement in place ' + 'between GOV.UK Notify and my organisation?' ) ), ( diff --git a/tests/app/main/views/test_index.py b/tests/app/main/views/test_index.py index fc4fe1bd4..be6348ac3 100644 --- a/tests/app/main/views/test_index.py +++ b/tests/app/main/views/test_index.py @@ -101,7 +101,25 @@ def test_terms_is_generic_if_user_is_not_logged_in( assert normalize_spaces(page.select('main p')[1].text) == ( 'Your organisation must also accept our data sharing and ' - 'financial agreement. Contact us to get a copy.' + 'financial agreement. Sign in to download a copy or find out ' + 'if one is already in place.' + ) + + +def test_pricing_is_generic_if_user_is_not_logged_in( + client +): + response = client.get(url_for('main.pricing')) + assert response.status_code == 200 + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + last_paragraph = page.select('main p')[-1] + assert normalize_spaces(last_paragraph.text) == ( + 'Sign in to download a copy or find out if one is already ' + 'in place with your organisation.' + ) + assert last_paragraph.select_one('a')['href'] == url_for( + 'main.sign_in', + next=url_for('main.pricing', _anchor='paying'), ) @@ -119,7 +137,7 @@ def test_terms_is_generic_if_user_is_not_logged_in( ), None, ( - 'Contact us to get a copy of the agreement ' + 'Download the agreement ' '(Cabinet Office has already accepted it).' ), ), @@ -135,7 +153,7 @@ def test_terms_is_generic_if_user_is_not_logged_in( 'main.agreement', ), ( - 'Contact us to get a copy of the agreement ' + 'Download the agreement ' '(Aylesbury Town Council hasn’t accepted it yet).' ), ), @@ -143,16 +161,16 @@ def test_terms_is_generic_if_user_is_not_logged_in( 'larry@downing-street.gov.uk', ( 'Your organisation must also accept our data sharing and ' - 'financial agreement. Contact us to get a copy.' + 'financial agreement. Download the agreement or contact us ' + 'to find out if we already have one in place with your ' + 'organisation.' ), partial( url_for, - 'main.feedback', - ticket_type='ask-question-give-feedback', - body='agreement-with-owner', + 'main.agreement', ), ( - 'Contact us to get a copy of the agreement or find out if ' + 'Download the agreement or contact us to find out if ' 'we already have one in place with your organisation.' ), ), @@ -167,8 +185,8 @@ def test_terms_is_generic_if_user_is_not_logged_in( 'main.agreement', ), ( - 'Contact us to get a copy of the agreement (Met Office ' - 'hasn’t accepted it yet).' + 'Download the agreement (Met Office hasn’t accepted it ' + 'yet).' ), ), ])