darkhelm/notifications-admin
1
0
Fork 0
mirror of https://github.com/GSA/notifications-admin.git synced 2026-08-04 13:41:38 -04:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #3054 from alphagov/remove-old-agreement-pages

Browse Source 
Remove the user-specific agreement pages
This commit is contained in:
Chris Hill-Scott
2019-07-17 13:07:39 +01:00
committed by GitHub
parent 42300371b0 bc5d03ff22
commit 2e78981648
16 changed files with 100 additions and 407 deletions
Download Patch File Download Diff File Expand all files Collapse all files

30
app/main/views/agreement.py
View File

@@ -6,28 +6,18 @@ from flask_login import current_user
from app import current_service
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, user_is_logged_in
@main.route('/agreement')
@user_is_logged_in
def agreement():
return render_template(
'views/agreement/{}.html'.format(current_user.default_organisation.as_jinja_template),
owner=current_user.default_organisation.name,
navigation_links=features_nav(),
)
from app.utils import user_has_permissions
@main.route('/services/<uuid:service_id>/agreement')
@user_has_permissions('manage_service')
def service_agreement(service_id):
return render_template(
'views/agreement/service-{}.html'.format(current_service.organisation.as_jinja_template),
owner=current_service.organisation.name,
)
if current_service.organisation.crown is None:
return render_template('views/agreement/service-agreement-choose.html')
if current_service.organisation.agreement_signed:
return render_template('views/agreement/service-agreement-signed.html')
return render_template('views/agreement/service-agreement.html')
@main.route('/services/<uuid:service_id>/agreement.pdf')
@@ -82,14 +72,6 @@ def service_confirm_agreement(service_id):
return render_template('views/agreement/agreement-confirm.html')
@main.route('/agreement.pdf')
@user_is_logged_in
def download_agreement():
return send_file(**get_mou(
current_user.default_organisation.crown_status_or_404
))
@main.route('/agreement/<variant>', endpoint='public_agreement')
@main.route('/agreement/<variant>.pdf', endpoint='public_download_agreement')
def public_agreement(variant):

4
app/main/views/service_settings.py
View File

@@ -211,7 +211,9 @@ def submit_request_to_go_live(service_id):
service_name=current_service.name,
service_dashboard=url_for('main.service_dashboard', service_id=current_service.id, _external=True),
organisation_type=str(current_service.organisation_type).title(),
agreement=current_service.organisation.as_human_readable(current_user.email_domain),
agreement=current_service.organisation.as_agreement_statement_for_go_live_request(
current_user.email_domain
),
volume_email_formatted=format_thousands(current_service.volume_email),
volume_sms_formatted=format_thousands(current_service.volume_sms),
volume_letter_formatted=format_thousands(current_service.volume_letter),

50
app/models/organisation.py
View File

@@ -1,4 +1,4 @@
from flask import Markup, abort
from flask import abort
from werkzeug.utils import cached_property
from app.models import JSONModel, ModelList
@@ -71,7 +71,7 @@ class Organisation(JSONModel):
self.organisation_type = None
self.request_to_go_live_notes = None
def as_human_readable(self, fallback_domain):
def as_agreement_statement_for_go_live_request(self, fallback_domain):
if self.agreement_signed:
agreement_statement = 'Yes, on behalf of {}.'.format(self.name)
elif self.name:
@@ -98,52 +98,6 @@ class Organisation(JSONModel):
def as_info_for_branding_request(self, fallback_domain):
return self.name or 'Can’t tell (domain is {})'.format(fallback_domain)
@property
def as_jinja_template(self):
if self.crown 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, terms_link, download_link, support_link, signed_in):
if not signed_in:
return ((
'{} <a href="{}">Sign in</a> to download a copy '
'or find out if one is already in place.'
).format(self._acceptance_required, terms_link))
if self.agreement_signed is None:
return ((
'{} <a href="{}">Download the agreement</a> or '
'<a href="{}">contact us</a> 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 ((
'{} <a href="{}">Download a copy</a>.'
).format(self._acceptance_required, download_link))
return (
'Your organisation ({}) has already accepted the '
'GOV.UK&nbsp;Notify data sharing and financial '
'agreement.'.format(self.name)
)
@property
def _acceptance_required(self):
return (
'Your organisation {} must also accept our data sharing '
'and financial agreement.'.format(
'({})'.format(self.name) if self.name else '',
)
)
@property
def crown_status_or_404(self):
if self.crown is None:

8
app/navigation.py
View File

@@ -122,7 +122,6 @@ class HeaderNavigation(Navigation):
'add_organisation',
'add_service',
'add_service_template',
'agreement',
'api_callbacks',
'api_documentation',
'api_integration',
@@ -162,7 +161,6 @@ class HeaderNavigation(Navigation):
'delivery_and_failure',
'delivery_status_callback',
'design_content',
'download_agreement',
'download_notifications_csv',
'edit_data_retention',
'edit_organisation_agreement',
@@ -431,7 +429,6 @@ class MainNavigation(Navigation):
'add_data_retention',
'add_organisation',
'add_service',
'agreement',
'archive_service',
'archive_user',
'bat_phone',
@@ -459,7 +456,6 @@ class MainNavigation(Navigation):
'delivery_and_failure',
'design_content',
'documentation',
'download_agreement',
'download_notifications_csv',
'edit_data_retention',
'edit_organisation_agreement',
@@ -623,7 +619,6 @@ class CaseworkNavigation(Navigation):
'add_organisation',
'add_service',
'add_service_template',
'agreement',
'api_callbacks',
'api_documentation',
'api_integration',
@@ -674,7 +669,6 @@ class CaseworkNavigation(Navigation):
'delivery_status_callback',
'design_content',
'documentation',
'download_agreement',
'download_notifications_csv',
'edit_data_retention',
'edit_organisation_agreement',
@@ -911,7 +905,6 @@ class OrgNavigation(Navigation):
'add_organisation',
'add_service',
'add_service_template',
'agreement',
'api_callbacks',
'api_documentation',
'api_integration',
@@ -956,7 +949,6 @@ class OrgNavigation(Navigation):
'delivery_status_callback',
'design_content',
'documentation',
'download_agreement',
'download_notifications_csv',
'edit_data_retention',
'edit_provider',

12
app/templates/views/agreement/_agreement-choose.html
View File

@@ -1,12 +0,0 @@
<p>
Before you can go live on GOV.UK Notify, your organisation needs to
accept to our data sharing and financial agreement.
</p>
<p>
There are different agreements for crown and non-crown organisations.
</p>
<p>
<a href="{{ url_for('main.support') }}">Get in touch</a> to tell us
whether or not you work for a crown organisation. If you’re not sure
we’ll help you work it out.
</p>

8
app/templates/views/agreement/_agreement-signed.html
View File

@@ -1,8 +0,0 @@
<p>
Your organisation ({{ owner }}) has already accepted the GOV.UK
Notify data sharing and financial agreement. You can
<a href="{{ url_for('main.service_download_agreement', service_id=current_service.id) }}">download a copy</a>.
</p>
<p>
The agreement contains commercially sensitive information, so don’t share it more widely than you need to.
</p>

17
app/templates/views/agreement/_agreement.html
View File

@@ -1,17 +0,0 @@
<p>
To use GOV.UK Notify your organisation ({{ current_service.organisation.name }}) must accept the GOV.UK Notify data sharing and financial agreement.
</p>
<p>
This agreement only needs to be accepted once and will then cover all Notify services from
{{ current_service.organisation.name }}.
</p>
<p>
It needs to be accepted by, or on behalf of someone who can sign contracts for your organisation.
</p>
<p>
<a href="{{ url_for('main.service_download_agreement', service_id=current_service.id) }}">Download a copy of the agreement</a>.
</p>
<p>
The agreement contains commercially sensitive information, so don’t share it more widely than you need to.
</p>
<a href="{{ url_for('main.service_accept_agreement', service_id=current_service.id) }}" class="button">Continue</a>

25
app/templates/views/agreement/agreement-choose.html
View File

@@ -1,25 +0,0 @@
{% extends "withoutnav_template.html" %}
{% from "components/sub-navigation.html" import sub_navigation %}
{% block per_page_title %}
GOV.UK Notify data sharing and financial agreement
{% endblock %}
{% block maincolumn_content %}
<div class="grid-row">
<div class="column-one-third">
{{ sub_navigation(navigation_links) }}
</div>
<div class="column-two-thirds">
<h1 class="heading-large">
GOV.UK Notify data sharing and financial agreement
</h1>
{% include 'views/agreement/_agreement-choose.html'%}
</div>
</div>
{% endblock %}

25
app/templates/views/agreement/agreement-signed.html
View File

@@ -1,25 +0,0 @@
{% extends "withoutnav_template.html" %}
{% from "components/sub-navigation.html" import sub_navigation %}
{% block per_page_title %}
GOV.UK Notify data sharing and financial agreement
{% endblock %}
{% block maincolumn_content %}
<div class="grid-row">
<div class="column-one-third">
{{ sub_navigation(navigation_links) }}
</div>
<div class="column-two-thirds">
<h1 class="heading-large">
GOV.UK Notify data sharing and financial agreement
</h1>
{% include 'views/agreement/_agreement-signed.html' %}
</div>
</div>
{% endblock %}

25
app/templates/views/agreement/agreement.html
View File

@@ -1,25 +0,0 @@
{% extends "withoutnav_template.html" %}
{% from "components/sub-navigation.html" import sub_navigation %}
{% block per_page_title %}
GOV.UK Notify data sharing and financial agreement
{% endblock %}
{% block maincolumn_content %}
<div class="grid-row">
<div class="column-one-third">
{{ sub_navigation(navigation_links) }}
</div>
<div class="column-two-thirds">
<h1 class="heading-large">
GOV.UK Notify data sharing and financial agreement
</h1>
{% include 'views/agreement/_agreement.html'%}
</div>
</div>
{% endblock %}

13
app/templates/views/agreement/service-agreement-choose.html
View File

@@ -15,7 +15,18 @@
back_link=url_for('main.request_to_go_live', service_id=current_service.id)
)}}
{% include 'views/agreement/_agreement-choose.html'%}
<p>
Before you can go live on GOV.UK Notify, your organisation needs to
accept to our data sharing and financial agreement.
</p>
<p>
There are different agreements for crown and non-crown organisations.
</p>
<p>
<a href="{{ url_for('main.support') }}">Get in touch</a> to tell us
whether or not you work for a crown organisation. If you’re not sure
we’ll help you work it out.
</p>
</div>
</div>

9
app/templates/views/agreement/service-agreement-signed.html
View File

@@ -15,7 +15,14 @@
back_link=url_for('main.request_to_go_live', service_id=current_service.id)
)}}
{% include 'views/agreement/_agreement-signed.html' %}
<p>
Your organisation ({{ current_service.organisation.name }}) has already accepted the GOV.UK
Notify data sharing and financial agreement. You can
<a href="{{ url_for('main.service_download_agreement', service_id=current_service.id) }}">download a copy</a>.
</p>
<p>
The agreement contains commercially sensitive information, so don’t share it more widely than you need to.
</p>
</div>
</div>

18
app/templates/views/agreement/service-agreement.html
View File

@@ -15,7 +15,23 @@
back_link=url_for('main.request_to_go_live', service_id=current_service.id)
)}}
{% include 'views/agreement/_agreement.html'%}
<p>
To use GOV.UK Notify your organisation ({{ current_service.organisation.name }}) must accept the GOV.UK Notify data sharing and financial agreement.
</p>
<p>
This agreement only needs to be accepted once and will then cover all Notify services from
{{ current_service.organisation.name }}.
</p>
<p>
It needs to be accepted by, or on behalf of someone who can sign contracts for your organisation.
</p>
<p>
<a href="{{ url_for('main.service_download_agreement', service_id=current_service.id) }}">Download a copy of the agreement</a>.
</p>
<p>
The agreement contains commercially sensitive information, so don’t share it more widely than you need to.
</p>
<a href="{{ url_for('main.service_accept_agreement', service_id=current_service.id) }}" class="button">Continue</a>
</div>
</div>

9
app/templates/views/terms-of-use.html
View File

@@ -13,15 +13,6 @@
These terms apply to your service’s use of GOV.UK&nbsp;Notify. You must be the service manager to accept them.
</p>
<p>
{{ current_user.default_organisation.as_terms_of_use_paragraph(
terms_link=url_for('main.sign_in', next=url_for('main.terms')),
download_link=url_for('.agreement'),
support_link=url_for('.feedback', ticket_type='ask-question-give-feedback', body='agreement'),
signed_in=current_user.is_authenticated
)}}
</p>
<h2 class="heading-medium">When using Notify</h2>
<p>You must:</p>
<ul class="list list-bullet">

152
tests/app/main/views/test_agreement.py
View File

@@ -9,7 +9,6 @@ from freezegun import freeze_time
from tests import organisation_json
from tests.conftest import (
SERVICE_ONE_ID,
mock_get_organisation_by_domain,
mock_get_service_organisation,
normalize_spaces,
)
@@ -24,49 +23,65 @@ class _MockS3Object():
return {'Body': BytesIO(self.data)}
@pytest.mark.parametrize('endpoint, extra_args, organisation_mock, link_selector, expected_back_links', [
(
'main.agreement',
{},
mock_get_organisation_by_domain,
'main .column-two-thirds a',
[]
),
(
'main.service_agreement',
{'service_id': SERVICE_ONE_ID},
mock_get_service_organisation,
'main .column-five-sixths a',
[
partial(url_for, 'main.request_to_go_live', service_id=SERVICE_ONE_ID)
]
),
])
@pytest.mark.parametrize('agreement_signed, crown, expected_links', [
(
True, True,
[
partial(url_for, 'main.service_download_agreement', service_id=SERVICE_ONE_ID),
(
['govuk-back-link'],
partial(url_for, 'main.request_to_go_live', service_id=SERVICE_ONE_ID),
),
(
[],
partial(url_for, 'main.service_download_agreement', service_id=SERVICE_ONE_ID),
),
]
),
(
False, False,
[
partial(url_for, 'main.service_download_agreement', service_id=SERVICE_ONE_ID),
partial(url_for, 'main.service_accept_agreement', service_id=SERVICE_ONE_ID),
(
['govuk-back-link'],
partial(url_for, 'main.request_to_go_live', service_id=SERVICE_ONE_ID),
),
(
[],
partial(url_for, 'main.service_download_agreement', service_id=SERVICE_ONE_ID),
),
(
['button'],
partial(url_for, 'main.service_accept_agreement', service_id=SERVICE_ONE_ID),
),
]
),
(
False, True,
[
partial(url_for, 'main.service_download_agreement', service_id=SERVICE_ONE_ID),
partial(url_for, 'main.service_accept_agreement', service_id=SERVICE_ONE_ID),
(
['govuk-back-link'],
partial(url_for, 'main.request_to_go_live', service_id=SERVICE_ONE_ID),
),
(
[],
partial(url_for, 'main.service_download_agreement', service_id=SERVICE_ONE_ID),
),
(
['button'],
partial(url_for, 'main.service_accept_agreement', service_id=SERVICE_ONE_ID),
),
]
),
(
None, None,
[
partial(url_for, 'main.support'),
(
['govuk-back-link'],
partial(url_for, 'main.request_to_go_live', service_id=SERVICE_ONE_ID),
),
(
[],
partial(url_for, 'main.support'),
),
]
),
])
@@ -75,27 +90,22 @@ def test_show_agreement_page(
mocker,
fake_uuid,
mock_has_jobs,
mock_get_service_organisation,
agreement_signed,
crown,
expected_links,
endpoint,
extra_args,
organisation_mock,
link_selector,
expected_back_links,
):
organisation_mock(
mock_get_service_organisation(
mocker,
crown=crown,
agreement_signed=agreement_signed,
)
expected_links = expected_back_links + expected_links
page = client_request.get(endpoint, **extra_args)
links = page.select(link_selector)
page = client_request.get('main.service_agreement', service_id=SERVICE_ONE_ID)
links = page.select('main .column-five-sixths a')
assert len(links) == len(expected_links)
for index, link in enumerate(links):
assert link['href'] == expected_links[index]()
classes, url = expected_links[index]
assert link.get('class', []) == classes
assert link['href'] == url()
@pytest.mark.parametrize('crown, expected_status, expected_file_fetched, expected_file_served', (
@@ -448,76 +458,6 @@ def test_confirm_agreement_page_persists(
)
@pytest.mark.parametrize('crown, expected_file_fetched, expected_file_served', [
(
True,
'crown.pdf',
'GOV.UK Notify data sharing and financial agreement.pdf',
),
(
False,
'non-crown.pdf',
'GOV.UK Notify data sharing and financial agreement (non-crown).pdf',
),
])
def test_downloading_agreement(
logged_in_client,
mocker,
fake_uuid,
crown,
expected_file_fetched,
expected_file_served,
):
mock_get_s3_object = mocker.patch(
'app.s3_client.s3_mou_client.get_s3_object',
return_value=_MockS3Object(b'foo')
)
mock_get_organisation_by_domain(
mocker,
crown=crown,
)
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.s3_client.s3_mou_client.get_s3_object',
return_value=_MockS3Object()
)
mock_get_organisation_by_domain(
mocker,
crown=None,
)
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.s3_client.s3_mou_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 == []
@pytest.mark.parametrize('endpoint', (
'main.public_agreement',
'main.public_download_agreement',

102
tests/app/main/views/test_index.py
View File

@@ -1,15 +1,9 @@
from functools import partial
import pytest
from bs4 import BeautifulSoup
from flask import url_for
from app.main.forms import FieldWithNoneOption
from tests.conftest import (
mock_get_organisation_by_domain,
normalize_spaces,
sample_uuid,
)
from tests.conftest import normalize_spaces, sample_uuid
def test_non_logged_in_user_can_see_homepage(
@@ -144,96 +138,12 @@ def test_old_integration_testing_page(
)
def test_terms_is_generic_if_user_is_not_logged_in(
client
):
response = client.get(url_for('main.terms'))
assert response.status_code == 200
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
assert normalize_spaces(page.select('main p')[1].text) == (
'Your organisation must also accept our data sharing and '
'financial agreement. Sign in to download a copy or find out '
'if one is already in place.'
)
@pytest.mark.parametrize((
'name,'
'agreement_signed,'
'expected_terms_paragraph,'
'expected_terms_link,'
), [
(
'Cabinet Office',
True,
(
'Your organisation (Cabinet Office) has already accepted '
'the GOV.UK Notify data sharing and financial agreement.'
),
None,
),
(
'Aylesbury Town Council',
False,
(
'Your organisation (Aylesbury Town Council) must also '
'accept our data sharing and financial agreement. Download '
'a copy.'
),
partial(
url_for,
'main.agreement',
),
),
(
None,
None,
(
'Your organisation must also accept our data sharing and '
'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.agreement',
),
),
(
'Met Office',
False,
(
'Your organisation (Met Office) must also accept our data '
'sharing and financial agreement. Download a copy.'
),
partial(
url_for,
'main.agreement',
),
),
])
def test_terms_tells_logged_in_users_what_we_know_about_their_agreement(
mocker,
fake_uuid,
client_request,
name,
agreement_signed,
expected_terms_paragraph,
expected_terms_link,
):
mock_get_organisation_by_domain(
mocker,
name=name,
agreement_signed=agreement_signed,
)
def test_terms_page_has_correct_content(client_request):
terms_page = client_request.get('main.terms')
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(terms_page.select('main p')[0].text) == (
'These terms apply to your service’s use of GOV.UK Notify. '
'You must be the service manager to accept them.'
)
def test_css_is_served_from_correct_path(client_request):