Determine agreement by service not user’s org

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.
This commit is contained in:
Chris Hill-Scott
2019-07-02 17:04:26 +01:00
parent 5368ddabbc
commit 1f02a2d3ab
5 changed files with 70 additions and 5 deletions

View File

@@ -8,6 +8,7 @@ from app.main import main
from app.main.forms import AcceptAgreementForm from app.main.forms import AcceptAgreementForm
from app.main.views.sub_navigation_dictionaries import features_nav from app.main.views.sub_navigation_dictionaries import features_nav
from app.s3_client.s3_mou_client import get_mou from app.s3_client.s3_mou_client import get_mou
from app.utils import user_has_permissions
@main.route('/agreement') @main.route('/agreement')
@@ -29,6 +30,15 @@ def service_agreement(service_id):
) )
@main.route('/services/<uuid:service_id>/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/<uuid:service_id>/agreement/accept', methods=['GET', 'POST']) @main.route('/services/<uuid:service_id>/agreement/accept', methods=['GET', 'POST'])
@login_required @login_required
def service_accept_agreement(service_id): def service_accept_agreement(service_id):

View File

@@ -252,6 +252,7 @@ class HeaderNavigation(Navigation):
'service_dashboard_updates', 'service_dashboard_updates',
'service_delete_email_reply_to', 'service_delete_email_reply_to',
'service_delete_sms_sender', 'service_delete_sms_sender',
'service_download_agreement',
'service_edit_email_reply_to', 'service_edit_email_reply_to',
'service_edit_letter_contact', 'service_edit_letter_contact',
'service_edit_sms_sender', 'service_edit_sms_sender',
@@ -539,6 +540,7 @@ class MainNavigation(Navigation):
'service_dashboard_updates', 'service_dashboard_updates',
'service_delete_email_reply_to', 'service_delete_email_reply_to',
'service_delete_sms_sender', 'service_delete_sms_sender',
'service_download_agreement',
'service_letter_validation_preview', 'service_letter_validation_preview',
'service_switch_can_upload_document', 'service_switch_can_upload_document',
'service_switch_count_as_live', 'service_switch_count_as_live',
@@ -777,6 +779,7 @@ class CaseworkNavigation(Navigation):
'service_dashboard_updates', 'service_dashboard_updates',
'service_delete_email_reply_to', 'service_delete_email_reply_to',
'service_delete_sms_sender', 'service_delete_sms_sender',
'service_download_agreement',
'service_edit_email_reply_to', 'service_edit_email_reply_to',
'service_edit_letter_contact', 'service_edit_letter_contact',
'service_edit_sms_sender', 'service_edit_sms_sender',
@@ -1048,6 +1051,7 @@ class OrgNavigation(Navigation):
'service_dashboard_updates', 'service_dashboard_updates',
'service_delete_email_reply_to', 'service_delete_email_reply_to',
'service_delete_sms_sender', 'service_delete_sms_sender',
'service_download_agreement',
'service_edit_email_reply_to', 'service_edit_email_reply_to',
'service_edit_letter_contact', 'service_edit_letter_contact',
'service_edit_sms_sender', 'service_edit_sms_sender',

View File

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

View File

@@ -9,7 +9,7 @@
It needs to be accepted by, or on behalf of someone who can sign contracts for your organisation. It needs to be accepted by, or on behalf of someone who can sign contracts for your organisation.
</p> </p>
<p> <p>
<a href="{{ url_for('main.download_agreement') }}">Download a copy of the agreement</a>. <a href="{{ url_for('main.service_download_agreement', service_id=current_service.id) }}">Download a copy of the agreement</a>.
</p> </p>
<p> <p>
The agreement contains commercially sensitive information, so dont share it more widely than you need to. The agreement contains commercially sensitive information, so dont share it more widely than you need to.

View File

@@ -46,20 +46,20 @@ class _MockS3Object():
( (
True, True, True, True,
[ [
partial(url_for, 'main.download_agreement'), partial(url_for, 'main.service_download_agreement', service_id=SERVICE_ONE_ID),
] ]
), ),
( (
False, False, 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), partial(url_for, 'main.service_accept_agreement', service_id=SERVICE_ONE_ID),
] ]
), ),
( (
False, True, 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), 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]() 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( def test_show_accept_agreement_page(
client_request, client_request,
mocker, mocker,