Merge pull request #1964 from alphagov/serve-mou

Let users download the data sharing and financial agreement

app/config.py

@@ -69,6 +69,7 @@ class Config(object):
     STATSD_PORT = 8125
     NOTIFY_ENVIRONMENT = 'development'
     LOGO_UPLOAD_BUCKET_NAME = 'public-logos-local'
+    MOU_BUCKET_NAME = 'local-mou'
     ROUTE_SECRET_KEY_1 = os.environ.get('ROUTE_SECRET_KEY_1', '')
     ROUTE_SECRET_KEY_2 = os.environ.get('ROUTE_SECRET_KEY_2', '')
     CHECK_PROXY_HEADER = False
@@ -82,6 +83,7 @@ class Development(Config):
     STATSD_ENABLED = False
     CSV_UPLOAD_BUCKET_NAME = 'development-notifications-csv-upload'
     LOGO_UPLOAD_BUCKET_NAME = 'public-logos-tools'
+    MOU_BUCKET_NAME = 'notify.tools-mou'
 
     ADMIN_CLIENT_SECRET = 'dev-notify-secret-key'
     API_HOST_NAME = 'http://localhost:6011'
@@ -98,6 +100,7 @@ class Test(Development):
     WTF_CSRF_ENABLED = False
     CSV_UPLOAD_BUCKET_NAME = 'test-notifications-csv-upload'
     LOGO_UPLOAD_BUCKET_NAME = 'public-logos-test'
+    MOU_BUCKET_NAME = 'test-mou'
     NOTIFY_ENVIRONMENT = 'test'
     API_HOST_NAME = 'http://you-forgot-to-mock-an-api-call-to'
     TEMPLATE_PREVIEW_API_HOST = 'http://localhost:9999'
@@ -109,6 +112,7 @@ class Preview(Config):
     STATSD_ENABLED = True
     CSV_UPLOAD_BUCKET_NAME = 'preview-notifications-csv-upload'
     LOGO_UPLOAD_BUCKET_NAME = 'public-logos-preview'
+    MOU_BUCKET_NAME = 'notify.works-mou'
     NOTIFY_ENVIRONMENT = 'preview'
     CHECK_PROXY_HEADER = True
 
@@ -120,6 +124,7 @@ class Staging(Config):
     STATSD_ENABLED = True
     CSV_UPLOAD_BUCKET_NAME = 'staging-notify-csv-upload'
     LOGO_UPLOAD_BUCKET_NAME = 'public-logos-staging'
+    MOU_BUCKET_NAME = 'staging-notify.works-mou'
     NOTIFY_ENVIRONMENT = 'staging'
     CHECK_PROXY_HEADER = True
 
@@ -131,6 +136,7 @@ class Live(Config):
     STATSD_ENABLED = True
     CSV_UPLOAD_BUCKET_NAME = 'live-notifications-csv-upload'
     LOGO_UPLOAD_BUCKET_NAME = 'public-logos-production'
+    MOU_BUCKET_NAME = 'notifications.service.gov.uk-mou'
     NOTIFY_ENVIRONMENT = 'live'
     CHECK_PROXY_HEADER = False
 

app/domains.yml

@@ -22,6 +22,7 @@ marinemanagement.org.uk:
 crowncommercial.gov.uk: cabinet-office.gov.uk
 cabinet-office.gov.uk:
   owner: Cabinet Office
+  crown: true
   agreement_signed: true
 cica.gsi.gov.uk: cica.gov.uk
 cica.gov.uk:
@@ -159,7 +160,11 @@ rpa.gov.uk:
   agreement_signed: true
 mcga.gov.uk:
   owner: Maritime and Coastguard Agency
-  agreement_signed: true  
+  agreement_signed: true
+metoffice.gov.uk:
+  owner: Met Office
+  agreement_signed: false
+  crown: true
 
 # Local Government
 aberdeencityandshire-sdpa.gov.uk:

app/main/__init__.py

@@ -32,5 +32,6 @@ from app.main.views import (  # noqa
     conversation,
     organisations,
     notifications,
-    inbound_number
+    inbound_number,
+    agreement,
 )

app/main/s3_client.py

@@ -61,6 +61,26 @@ def s3download(service_id, upload_id):
     return contents
 
 
+def get_mou(organisation_is_crown):
+    bucket = current_app.config['MOU_BUCKET_NAME']
+    filename = 'crown.pdf' if organisation_is_crown else 'non-crown.pdf'
+    attachment_filename = 'GOV.UK Notify data sharing and financial agreement{}.pdf'.format(
+        '' if organisation_is_crown else ' (non-crown)'
+    )
+    try:
+        key = get_s3_object(bucket, filename)
+        return {
+            'filename_or_fp': key.get()['Body'],
+            'attachment_filename': attachment_filename,
+            'as_attachment': True,
+        }
+    except botocore.exceptions.ClientError as exception:
+        current_app.logger.error("Unable to download s3 file {}/{}".format(
+            bucket, filename
+        ))
+        raise exception
+
+
 def upload_logo(filename, filedata, region, user_id):
     upload_file_name = LOGO_LOCATION_STRUCTURE.format(
         temp=TEMP_TAG.format(user_id=user_id),

app/main/views/agreement.py

@@ -0,0 +1,25 @@
+from flask import render_template, send_file
+from flask_login import login_required
+
+from app.main import main
+from app.main.s3_client import get_mou
+from app.main.views.sub_navigation_dictionaries import features_nav
+from app.utils import AgreementInfo
+
+
+@main.route('/agreement')
+@login_required
+def agreement():
+    return render_template(
+        'views/agreement.html',
+        crown_status=AgreementInfo.from_current_user().crown_status_or_404,
+        navigation_links=features_nav(),
+    )
+
+
+@main.route('/agreement.pdf')
+@login_required
+def download_agreement():
+    return send_file(**get_mou(
+        AgreementInfo.from_current_user().crown_status_or_404
+    ))

app/templates/views/agreement.html

@@ -0,0 +1,37 @@
+{% 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 %}
+
+<div class="grid-row">
+  <div class="column-one-third">
+    {{ sub_navigation(navigation_links) }}
+  </div>
+  <div class="column-two-thirds">
+
+    <h1 class="heading-large">
+      Download the GOV.UK Notify data sharing and financial agreement
+    </h1>
+
+    <p>
+      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.
+    </p>
+    <p>
+      Return a signed copy to <span style="white-space: nowrap">notify-support@digital.cabinet-office.gov.uk</span>
+    </p>
+    <p class="panel panel-border-wide bottom-gutter-2-3">
+      The agreement contains commercially sensitive information.<br>
+      Do not share it more widely than you need to.
+    </p>
+    <p>
+      <a href="{{ url_for('main.download_agreement') }}">Download the agreement</a>.
+    </p>
+
+  </div>
+</div>
+
+{% endblock %}

app/templates/views/terms-of-use.html

@@ -20,21 +20,12 @@ Terms of use
       These terms apply to your service’s use of GOV.UK Notify. You must be the service manager to accept them.
     </p>
 
-    {% if agreement_info.agreement_signed %}
-      <p>Your organisation ({{ agreement_info.owner }}) has already accepted the GOV.UK&nbsp;Notify data sharing and financial agreement.</p>
-    {% else %}
-      <p>
-        Your organisation
-        {% if agreement_info.owner %}
-          ({{ agreement_info.owner }})
-          must also accept our data sharing and financial agreement.
-          <a href="{{ url_for('.feedback', ticket_type='ask-question-give-feedback', body='agreement-with-owner') }}">Contact us</a> to get a copy.
-        {% else %}
-          must also accept our data sharing and financial agreement.
-          <a href="{{ url_for('.feedback', ticket_type='ask-question-give-feedback', body='agreement') }}">Contact us</a> to get a copy.
-        {% endif %}
-      </p>
-    {% endif %}
+    <p>
+      {{ 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')
+      )}}
+    </p>
 
     <h2 class="heading-medium">When using Notify</h2>
     <p>You must:</p>

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 ((
+                '{} <a href="{}">Download a copy</a>.'
+            ).format(self._acceptance_required, download_link))
+
+        return ((
+            '{} <a href="{}">Contact us</a> 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 (

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 == []

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