From 35bf42b024ae49b61ea1cf59ab8f2f6e09ca9537 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 1 May 2019 08:29:32 +0100 Subject: [PATCH] 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]()