From dbf8b22d3cb416c14b51f39cbc5f9abc63e423f0 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 1 May 2019 07:55:26 +0100 Subject: [PATCH 1/5] Organise agreement pages into a folder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This keeps them all in one place so they’re easier to work with. --- app/main/views/agreement.py | 4 ++-- app/templates/views/{ => agreement}/agreement-choose.html | 0 app/templates/views/{ => agreement}/agreement-public.html | 0 app/templates/views/{ => agreement}/agreement-signed.html | 0 app/templates/views/{ => agreement}/agreement.html | 0 5 files changed, 2 insertions(+), 2 deletions(-) rename app/templates/views/{ => agreement}/agreement-choose.html (100%) rename app/templates/views/{ => agreement}/agreement-public.html (100%) rename app/templates/views/{ => agreement}/agreement-signed.html (100%) rename app/templates/views/{ => agreement}/agreement.html (100%) diff --git a/app/main/views/agreement.py b/app/main/views/agreement.py index 88fedeb8c..5f9638d2c 100644 --- a/app/main/views/agreement.py +++ b/app/main/views/agreement.py @@ -10,7 +10,7 @@ from app.s3_client.s3_mou_client import get_mou @login_required def agreement(): return render_template( - 'views/{}.html'.format(current_user.default_organisation.as_jinja_template), + 'views/agreement/{}.html'.format(current_user.default_organisation.as_jinja_template), owner=current_user.default_organisation.name, navigation_links=features_nav(), ) @@ -37,7 +37,7 @@ def public_agreement(variant): )) return render_template( - 'views/agreement-public.html', + 'views/agreement/agreement-public.html', owner=current_user.default_organisation.name, download_link=url_for('.public_download_agreement', variant=variant), ) diff --git a/app/templates/views/agreement-choose.html b/app/templates/views/agreement/agreement-choose.html similarity index 100% rename from app/templates/views/agreement-choose.html rename to app/templates/views/agreement/agreement-choose.html diff --git a/app/templates/views/agreement-public.html b/app/templates/views/agreement/agreement-public.html similarity index 100% rename from app/templates/views/agreement-public.html rename to app/templates/views/agreement/agreement-public.html diff --git a/app/templates/views/agreement-signed.html b/app/templates/views/agreement/agreement-signed.html similarity index 100% rename from app/templates/views/agreement-signed.html rename to app/templates/views/agreement/agreement-signed.html diff --git a/app/templates/views/agreement.html b/app/templates/views/agreement/agreement.html similarity index 100% rename from app/templates/views/agreement.html rename to app/templates/views/agreement/agreement.html From 734666ee53a82a52b35526251c5469bb3ea91bf9 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 1 May 2019 08:00:25 +0100 Subject: [PATCH 2/5] Refactor agreement pages into includes So we can re-use the same content in multiple places. --- .../views/agreement/_agreement-choose.html | 35 ++++++++++++++++++ .../views/agreement/_agreement-signed.html | 8 ++++ app/templates/views/agreement/_agreement.html | 17 +++++++++ .../views/agreement/agreement-choose.html | 37 +------------------ .../views/agreement/agreement-signed.html | 10 +---- app/templates/views/agreement/agreement.html | 18 +-------- 6 files changed, 65 insertions(+), 60 deletions(-) create mode 100644 app/templates/views/agreement/_agreement-choose.html create mode 100644 app/templates/views/agreement/_agreement-signed.html create mode 100644 app/templates/views/agreement/_agreement.html diff --git a/app/templates/views/agreement/_agreement-choose.html b/app/templates/views/agreement/_agreement-choose.html new file mode 100644 index 000000000..4538dad62 --- /dev/null +++ b/app/templates/views/agreement/_agreement-choose.html @@ -0,0 +1,35 @@ +

+ Before you can go live on GOV.UK Notify, your organisation needs to agree to our data sharing and financial agreement. +

+

+ Crown bodies +

+

+ Download the crown agreement. +

+

+ Non-crown bodies +

+

+ Download the non-crown agreement. +

+
+

+ Contact us if you’re + not sure whether your organisation is a crown or non-crown body. +

+
+

+ Next steps +

+
    +
  1. + Get the agreement signed by someone who has the authority to do so on behalf of {{ owner or 'your organisation' }}. +
    2. +
  2. + Return the signed copy to notify-support@digital.cabinet-office.gov.uk. +
    3. +
+

+ The agreement contains commercially sensitive information, so don’t share it more widely than you need to. +

diff --git a/app/templates/views/agreement/_agreement-signed.html b/app/templates/views/agreement/_agreement-signed.html new file mode 100644 index 000000000..35819b126 --- /dev/null +++ b/app/templates/views/agreement/_agreement-signed.html @@ -0,0 +1,8 @@ +

+ Your organisation ({{ owner }}) has already accepted the GOV.UK + Notify data sharing and financial agreement. You can + download a copy. +

+

+ The agreement contains commercially sensitive information, so don’t share it more widely than you need to. +

diff --git a/app/templates/views/agreement/_agreement.html b/app/templates/views/agreement/_agreement.html new file mode 100644 index 000000000..2e462192a --- /dev/null +++ b/app/templates/views/agreement/_agreement.html @@ -0,0 +1,17 @@ +

+ Before you can go live on GOV.UK Notify, your organisation needs to agree to our data sharing and financial agreement. +

+
    +
  1. + Download the agreement. +
    2. +
  2. + Get it signed by someone who has the authority to do so on behalf of {{ owner }}. +
    3. +
  3. + Return the signed copy to notify-support@digital.cabinet-office.gov.uk. +
    4. +
+

+ The agreement contains commercially sensitive information, so don’t share it more widely than you need to. +

diff --git a/app/templates/views/agreement/agreement-choose.html b/app/templates/views/agreement/agreement-choose.html index 592eb9533..9f0d6ae5f 100644 --- a/app/templates/views/agreement/agreement-choose.html +++ b/app/templates/views/agreement/agreement-choose.html @@ -17,41 +17,8 @@ GOV.UK Notify data sharing and financial agreement -

- Before you can go live on GOV.UK Notify, your organisation needs to agree to our data sharing and financial agreement. -

-

- Crown bodies -

-

- Download the crown agreement. -

-

- Non-crown bodies -

-

- Download the non-crown agreement. -

-
-

- Contact us if you’re - not sure whether your organisation is a crown or non-crown body. -

-
-

- Next steps -

-
    -
  1. - Get the agreement signed by someone who has the authority to do so on behalf of {{ owner or 'your organisation' }}. -
    2. -
  2. - Return the signed copy to notify-support@digital.cabinet-office.gov.uk. -
    3. -
-

- The agreement contains commercially sensitive information, so don’t share it more widely than you need to. -

+ {% include 'views/agreement/_agreement-choose.html'%} + diff --git a/app/templates/views/agreement/agreement-signed.html b/app/templates/views/agreement/agreement-signed.html index b38c76193..4cd1c6946 100644 --- a/app/templates/views/agreement/agreement-signed.html +++ b/app/templates/views/agreement/agreement-signed.html @@ -17,14 +17,8 @@ GOV.UK Notify data sharing and financial agreement -

- Your organisation ({{ owner }}) has already accepted the GOV.UK - Notify data sharing and financial agreement. You can - download a copy. -

-

- The agreement contains commercially sensitive information, so don’t share it more widely than you need to. -

+ {% include 'views/agreement/_agreement-signed.html' %} + diff --git a/app/templates/views/agreement/agreement.html b/app/templates/views/agreement/agreement.html index 16f72689d..a5fbfcacd 100644 --- a/app/templates/views/agreement/agreement.html +++ b/app/templates/views/agreement/agreement.html @@ -17,23 +17,7 @@ GOV.UK Notify data sharing and financial agreement -

- Before you can go live on GOV.UK Notify, your organisation needs to agree to our data sharing and financial agreement. -

-
    -
  1. - Download the agreement. -
    2. -
  2. - Get it signed by someone who has the authority to do so on behalf of {{ owner }}. -
    3. -
  3. - Return the signed copy to notify-support@digital.cabinet-office.gov.uk. -
    4. -
-

- The agreement contains commercially sensitive information, so don’t share it more widely than you need to. -

+ {% include 'views/agreement/_agreement.html'%} From 35bf42b024ae49b61ea1cf59ab8f2f6e09ca9537 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 1 May 2019 08:29:32 +0100 Subject: [PATCH 3/5] Add service-specific versions of agreement page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Our usability testing found that jumping out of the service when going to download the agreement made it difficult for people to find their way back to the ‘Request to go live’ page. This commit adds a duplicate, service-specific versions of these pages which have the same content but: - keep the service navigation - have a link back to the ‘Request to go live’ page --- app/main/views/agreement.py | 9 ++++++ app/navigation.py | 4 +++ .../agreement/service-agreement-choose.html | 23 +++++++++++++++ .../agreement/service-agreement-signed.html | 23 +++++++++++++++ .../views/agreement/service-agreement.html | 23 +++++++++++++++ tests/app/main/views/test_agreement.py | 28 +++++++++++++++++-- 6 files changed, 107 insertions(+), 3 deletions(-) create mode 100644 app/templates/views/agreement/service-agreement-choose.html create mode 100644 app/templates/views/agreement/service-agreement-signed.html create mode 100644 app/templates/views/agreement/service-agreement.html diff --git a/app/main/views/agreement.py b/app/main/views/agreement.py index 5f9638d2c..d0c7959a9 100644 --- a/app/main/views/agreement.py +++ b/app/main/views/agreement.py @@ -16,6 +16,15 @@ def agreement(): ) +@main.route('/service//agreement') +@login_required +def service_agreement(service_id): + return render_template( + 'views/agreement/service-{}.html'.format(current_user.default_organisation.as_jinja_template), + owner=current_user.default_organisation.name, + ) + + @main.route('/agreement.pdf') @login_required def download_agreement(): diff --git a/app/navigation.py b/app/navigation.py index 829a50489..03d55a2e6 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -234,6 +234,7 @@ class HeaderNavigation(Navigation): 'service_add_email_reply_to', 'service_add_letter_contact', 'service_add_sms_sender', + 'service_agreement', 'service_confirm_delete_email_reply_to', 'service_confirm_delete_sms_sender', 'service_dashboard', @@ -365,6 +366,7 @@ class MainNavigation(Navigation): 'service_add_email_reply_to', 'service_add_letter_contact', 'service_add_sms_sender', + 'service_agreement', 'service_confirm_delete_email_reply_to', 'service_confirm_delete_sms_sender', 'service_edit_email_reply_to', @@ -735,6 +737,7 @@ class CaseworkNavigation(Navigation): 'service_add_email_reply_to', 'service_add_letter_contact', 'service_add_sms_sender', + 'service_agreement', 'service_confirm_delete_email_reply_to', 'service_confirm_delete_sms_sender', 'service_dashboard', @@ -991,6 +994,7 @@ class OrgNavigation(Navigation): 'service_add_email_reply_to', 'service_add_letter_contact', 'service_add_sms_sender', + 'service_agreement', 'service_confirm_delete_email_reply_to', 'service_confirm_delete_sms_sender', 'service_dashboard', diff --git a/app/templates/views/agreement/service-agreement-choose.html b/app/templates/views/agreement/service-agreement-choose.html new file mode 100644 index 000000000..5cddbc94d --- /dev/null +++ b/app/templates/views/agreement/service-agreement-choose.html @@ -0,0 +1,23 @@ +{% extends "withnav_template.html" %} +{% from "components/page-header.html" import page_header %} + +{% block service_page_title %} + GOV.UK Notify data sharing and financial agreement +{% endblock %} + +{% block maincolumn_content %} + +
+
+ + {{ page_header( + 'GOV.UK Notify data sharing and financial agreement', + back_link=url_for('main.request_to_go_live', service_id=current_service.id) + )}} + + {% include 'views/agreement/_agreement-choose.html'%} + +
+
+ +{% endblock %} diff --git a/app/templates/views/agreement/service-agreement-signed.html b/app/templates/views/agreement/service-agreement-signed.html new file mode 100644 index 000000000..9dac55925 --- /dev/null +++ b/app/templates/views/agreement/service-agreement-signed.html @@ -0,0 +1,23 @@ +{% extends "withnav_template.html" %} +{% from "components/page-header.html" import page_header %} + +{% block service_page_title %} + GOV.UK Notify data sharing and financial agreement +{% endblock %} + +{% block maincolumn_content %} + +
+
+ + {{ page_header( + 'GOV.UK Notify data sharing and financial agreement', + back_link=url_for('main.request_to_go_live', service_id=current_service.id) + )}} + + {% include 'views/agreement/_agreement-signed.html' %} + +
+
+ +{% endblock %} diff --git a/app/templates/views/agreement/service-agreement.html b/app/templates/views/agreement/service-agreement.html new file mode 100644 index 000000000..2d147154d --- /dev/null +++ b/app/templates/views/agreement/service-agreement.html @@ -0,0 +1,23 @@ +{% extends "withnav_template.html" %} +{% from "components/page-header.html" import page_header %} + +{% block service_page_title %} + GOV.UK Notify data sharing and financial agreement +{% endblock %} + +{% block maincolumn_content %} + +
+
+ + {{ page_header( + 'GOV.UK Notify data sharing and financial agreement', + back_link=url_for('main.request_to_go_live', service_id=current_service.id) + )}} + + {% include 'views/agreement/_agreement.html'%} + +
+
+ +{% endblock %} diff --git a/tests/app/main/views/test_agreement.py b/tests/app/main/views/test_agreement.py index 0b3806c15..cb284d681 100644 --- a/tests/app/main/views/test_agreement.py +++ b/tests/app/main/views/test_agreement.py @@ -4,7 +4,7 @@ from io import BytesIO import pytest from flask import url_for -from tests.conftest import mock_get_organisation_by_domain +from tests.conftest import SERVICE_ONE_ID, mock_get_organisation_by_domain class _MockS3Object(): @@ -16,6 +16,22 @@ class _MockS3Object(): return {'Body': BytesIO(self.data)} +@pytest.mark.parametrize('endpoint, extra_args, link_selector, expected_back_links', [ + ( + 'main.agreement', + {}, + 'main .column-two-thirds a', + [] + ), + ( + 'main.service_agreement', + {'service_id': SERVICE_ONE_ID}, + '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, @@ -44,17 +60,23 @@ def test_show_agreement_page( client_request, mocker, fake_uuid, + mock_has_jobs, agreement_signed, crown, expected_links, + endpoint, + extra_args, + link_selector, + expected_back_links, ): mock_get_organisation_by_domain( mocker, crown=crown, agreement_signed=agreement_signed, ) - page = client_request.get('main.agreement') - links = page.select('main .column-two-thirds a') + expected_links = expected_back_links + expected_links + page = client_request.get(endpoint, **extra_args) + links = page.select(link_selector) assert len(links) == len(expected_links) for index, link in enumerate(links): assert link['href'] == expected_links[index]() From 041c7f89677a4d4d527ca97e4e03226dafeee87c Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 1 May 2019 15:16:15 +0100 Subject: [PATCH 4/5] Link request to go live to service agreement page To make it easier for users to go back and forth. --- app/templates/views/service-settings/request-to-go-live.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/templates/views/service-settings/request-to-go-live.html b/app/templates/views/service-settings/request-to-go-live.html index 3205dd6ca..d6fa61fdd 100644 --- a/app/templates/views/service-settings/request-to-go-live.html +++ b/app/templates/views/service-settings/request-to-go-live.html @@ -52,7 +52,7 @@ {{ task_list_item( agreement_signed, 'Sign our data sharing and financial agreement', - url_for('main.agreement'), + url_for('main.service_agreement', service_id=current_service.id), ) }} {% endif %} {% endcall %} From acd564873af98ae3215bc6c44678025541b828ae Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 1 May 2019 15:41:43 +0100 Subject: [PATCH 5/5] Say `services` in URL path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It’s what we do everywhere else… Co-Authored-By: quis --- app/main/views/agreement.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/main/views/agreement.py b/app/main/views/agreement.py index d0c7959a9..2de1b2873 100644 --- a/app/main/views/agreement.py +++ b/app/main/views/agreement.py @@ -16,7 +16,7 @@ def agreement(): ) -@main.route('/service//agreement') +@main.route('/services//agreement') @login_required def service_agreement(service_id): return render_template(