Merge pull request #2074 from alphagov/sign-in-for-agreement

Let people choose which agreement to download if signed in

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(),
     )

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'), ''

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 %}
+
+<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>
+      Before you can go live on GOV.UK Notify, your organisation needs to agree to our data sharing and financial agreement.
+    </p>
+    <h2 class="heading-small">
+      Crown bodies
+    </h2>
+    <p>
+      Download the <a href="{{ url_for('main.public_download_agreement', variant='crown') }}">crown agreement</a>.
+    </p>
+    <h2 class="heading-small">
+      Non-crown bodies
+    </h2>
+    <p>
+      Download the <a href="{{ url_for('main.public_download_agreement', variant='non-crown') }}">non-crown agreement</a>.
+    </p>
+    <div class="panel panel-border-wide">
+      <p>
+        <a href="{{ url_for('main.support') }}">Contact us</a> if you’re
+        not sure whether your organisation is a crown or non-crown body.
+      </p>
+    </div>
+    <h2 class="heading-small">
+      Next steps
+    </h2>
+    <ol class="list list-number">
+      <li>
+        Get the agreement signed by someone who has the authority to do so on behalf of {{ owner or 'your organisation' }}.
+      </li>
+      <li>
+        Return the signed copy to <a href="mailto:notify-support@digital.cabinet-office.gov.uk">notify-support@digital.cabinet-office.gov.uk</a>.
+      </li>
+    </ol>
+    <p>
+      The agreement contains commercially sensitive information, so don’t share it more widely than you need to.
+    </p>
+  </div>
+</div>
+
+{% endblock %}

app/templates/views/agreement-signed.html

@@ -0,0 +1,31 @@
+{% 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>
+      Your organisation ({{ owner }}) has already accepted the GOV.UK
+      Notify data sharing and financial agreement. You can
+      <a href="{{ url_for('main.download_agreement') }}">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>
+
+{% endblock %}

app/templates/views/pricing.html

@@ -125,16 +125,12 @@
     <h2 class="heading-medium" id="paying">How to pay</h2>
     <p>You can find details of how to pay for Notify in our data sharing and financial agreement.</p>
     <p>
-      <a href="{{url_for('.feedback', ticket_type='ask-question-give-feedback', body='agreement')}}">Contact us</a> 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,
+      ) }}
     </p>
   </div>
 </div>

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

@@ -22,8 +22,10 @@ Terms of use
 
     <p>
       {{ 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
       )}}
     </p>
 

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 ((
+                '{} <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.crown_status is not None:
+        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 ((
-            '{} <a href="{}">Contact us</a> to get a copy.'
-        ).format(self._acceptance_required, contact_link))
+        return (
+            'Your organisation ({}) has already accepted the '
+            'GOV.UK&nbsp;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 ((
+                '<a href="{}">Sign in</a> 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 ((
+                '<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(download_link, support_link))
+
+        return (
+            '<a href="{}">Download the agreement</a> '
+            '({} {}).'.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):
 

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', [

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?'
         )
     ),
     (

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).'
         ),
     ),
 ])