From 33277ce3e265c2af5c67e2d5ae1e5d9f47df677c Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Tue, 5 Jun 2018 17:23:47 +0100 Subject: [PATCH] Make a contact_link required when turning upload_document permission on When a platform admin user clicks on the button to add the 'upload_document' permission to a service, they should be taken to the page to add a contact_link if the service does not already have one. --- app/main/views/service_settings.py | 21 +++++- .../views/service-settings/contact_link.html | 6 +- tests/app/main/views/test_service_settings.py | 73 ++++++++++++++++++- 3 files changed, 94 insertions(+), 6 deletions(-) diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index e45979324..76873d67a 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -322,12 +322,27 @@ def service_switch_can_send_precompiled_letter(service_id): return redirect(url_for('.service_settings', service_id=service_id)) -@main.route("/services//service-settings/can-upload-document") +@main.route("/services//service-settings/can-upload-document", methods=['GET', 'POST']) @login_required @user_is_platform_admin def service_switch_can_upload_document(service_id): - switch_service_permissions(service_id, 'upload_document') - return redirect(url_for('.service_settings', service_id=service_id)) + form = ServiceContactLinkForm() + + # If turning the permission off, or turning it on and the service already has a contact_link, + # don't show the form to add the link + if 'upload_document' in current_service['permissions'] or current_service.get('contact_link'): + switch_service_permissions(service_id, 'upload_document') + return redirect(url_for('.service_settings', service_id=service_id)) + + if form.validate_on_submit(): + service_api_client.update_service( + current_service['id'], + contact_link=form.url.data + ) + switch_service_permissions(service_id, 'upload_document') + return redirect(url_for('.service_settings', service_id=service_id)) + + return render_template('views/service-settings/contact_link.html', form=form) @main.route("/services//service-settings/archive", methods=['GET', 'POST']) diff --git a/app/templates/views/service-settings/contact_link.html b/app/templates/views/service-settings/contact_link.html index 4b3bdb5c2..4846b8fa4 100644 --- a/app/templates/views/service-settings/contact_link.html +++ b/app/templates/views/service-settings/contact_link.html @@ -3,13 +3,15 @@ {% from "components/page-footer.html" import page_footer %} {% block service_page_title %} - Change link on ‘Download your document’ page + {{ 'Change link on' if 'upload_document' in current_service.permissions else 'Add link for' }} ‘Download your document’ page {% endblock %} {% block maincolumn_content %}
-

Change link on ‘Download your document’ page

+

+ {{ 'Change link on' if 'upload_document' in current_service.permissions else 'Add link for' }} ‘Download your document’ page +

When you send users a document to download, you need to include a link to your service on the download page. This is so users can contact you if there’s a problem (for example, diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index ee2b61af8..4d775c066 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -1977,6 +1977,77 @@ def test_switch_service_enable_international_sms( assert mocked_fn.call_args[0][0] == service_one['id'] +@pytest.mark.parametrize('start_permissions, contact_link, end_permissions', [ + (['upload_document'], 'http://example.com/', []), + (['upload_document'], None, []), + ([], 'http://example.com/', ['upload_document']), +]) +def test_service_switch_can_upload_document_changes_the_permission_if_not_adding_the_permission_without_a_contact_link( + logged_in_platform_admin_client, + service_one, + mock_update_service, + mock_get_service_settings_page_common, + mock_get_service_organisation, + no_reply_to_email_addresses, + no_letter_contact_blocks, + single_sms_sender, + start_permissions, + contact_link, + end_permissions, +): + service_one['permissions'] = start_permissions + service_one['contact_link'] = contact_link + + response = logged_in_platform_admin_client.get( + url_for('main.service_switch_can_upload_document', service_id=SERVICE_ONE_ID), + follow_redirects=True + ) + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + + assert service_one['permissions'] == end_permissions + assert page.h1.text.strip() == 'Settings' + + +def test_service_switch_can_upload_document_turning_permission_on_with_no_contact_link_shows_link_form( + logged_in_platform_admin_client, + service_one, + mock_get_service_settings_page_common, + mock_get_service_organisation, + no_reply_to_email_addresses, + no_letter_contact_blocks, + single_sms_sender, +): + response = logged_in_platform_admin_client.get( + url_for('main.service_switch_can_upload_document', service_id=SERVICE_ONE_ID), + follow_redirects=True + ) + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + + assert 'upload_document' not in service_one['permissions'] + assert page.h1.text.strip() == "Add link for ‘Download your document’ page" + + +def test_service_switch_can_upload_document_lets_contact_link_be_added_and_switches_permission( + logged_in_platform_admin_client, + service_one, + mock_update_service, + mock_get_service_settings_page_common, + mock_get_service_organisation, + no_reply_to_email_addresses, + no_letter_contact_blocks, + single_sms_sender, +): + response = logged_in_platform_admin_client.post( + url_for('main.service_switch_can_upload_document', service_id=SERVICE_ONE_ID), + data={'url': 'http://example.com/'}, + follow_redirects=True + ) + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + + assert 'upload_document' in service_one['permissions'] + assert page.h1.text.strip() == 'Settings' + + def test_archive_service_after_confirm( logged_in_platform_admin_client, service_one, @@ -2200,7 +2271,7 @@ def test_service_set_contact_link_does_not_update_invalid_link( _follow_redirects=True ) - assert page.h1.text == "Change link on ‘Download your document’ page" + assert page.h1.text.strip() == "Add link for ‘Download your document’ page" update_mock.assert_not_called()