Make people sign in to get the agreement

Currently we have a bunch of users who aren’t signed in asking us for
the agreement.

This is bad because:
- it’s slower (for them) than just being able to download it
- it creates work for us

We can’t just offer the agreement to anyone, but we can offer to it to
anyone who’s signed in because we now let people self-select which
version to download when we can’t tell which one to give them.

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/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

@@ -475,23 +475,61 @@ class AgreementInfo:
     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 +546,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_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).'
         ),
     ),
 ])