From 954f43ae48a2af4d002210eab9e38a010882faec Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 29 May 2019 16:51:25 +0100 Subject: [PATCH] Let users archive their own trial mode services MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit At the moment we have a blanket rule that users can’t archive their own services, to prevent someone accidentally deleting a real live service, because that would be Very Bad. But the tickets we get from users asking us to delete services are for services they set up when they were just trying out Notify. There’s not much harm in letting users delete these services, the consequences of doing so are much lower than those of deleting a live service. And it should mean fewer support tickets for us to deal with. --- app/assets/stylesheets/_grids.scss | 2 +- app/main/views/service_settings.py | 15 ++++- .../views/dashboard/_inbox_messages.html | 3 +- app/templates/views/service-settings.html | 53 ++++++++------- .../test_service_setting_permissions.py | 2 +- tests/app/main/views/test_service_settings.py | 66 ++++++++++++++----- 6 files changed, 96 insertions(+), 45 deletions(-) diff --git a/app/assets/stylesheets/_grids.scss b/app/assets/stylesheets/_grids.scss index f73c38521..fd6d38bee 100644 --- a/app/assets/stylesheets/_grids.scss +++ b/app/assets/stylesheets/_grids.scss @@ -39,7 +39,7 @@ margin-top: $gutter * 4 / 3; } -.top-gutter-2-3 { +.top-gutter-1-2 { @extend %top-gutter; margin-top: $gutter-half; } diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 25a907e2e..f285b72b8 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -330,11 +330,22 @@ def service_switch_can_upload_document(service_id): @login_required @user_has_permissions('manage_service') def archive_service(service_id): + if not current_service.active and ( + current_service.trial_mode or current_user.platform_admin + ): + abort(403) if request.method == 'POST': service_api_client.archive_service(service_id) - return redirect(url_for('.service_settings', service_id=service_id)) + flash( + '‘{}’ was deleted'.format(current_service.name), + 'default_with_tick', + ) + return redirect(url_for('.choose_account')) else: - flash('There\'s no way to reverse this! Are you sure you want to archive this service?', 'delete') + flash( + 'Are you sure you want to delete ‘{}’? There’s no way to undo this.'.format(current_service.name), + 'delete', + ) return service_settings(service_id) diff --git a/app/templates/views/dashboard/_inbox_messages.html b/app/templates/views/dashboard/_inbox_messages.html index 37d9a317b..5e2d338f6 100644 --- a/app/templates/views/dashboard/_inbox_messages.html +++ b/app/templates/views/dashboard/_inbox_messages.html @@ -4,7 +4,7 @@
{% if messages %} -

+

Download these messages

{% endif %} @@ -37,4 +37,3 @@ {{ previous_next_navigation(prev_page, next_page) }}
- diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html index 1d64bf2fe..697a54f8e 100644 --- a/app/templates/views/service-settings.html +++ b/app/templates/views/service-settings.html @@ -353,32 +353,39 @@ {% endcall %} -

- {% if current_service.active %} - - - Archive service - - - - - Suspend service - - - {% else %} -

- Service suspended -
- - - Resume service - - - {% endif %} -

{% endif %} + {% if current_service.active and (current_service.trial_mode or current_user.platform_admin) %} +

+ + + Delete this service + + + {% if current_user.platform_admin %} + + + Suspend service + + + {% endif %} +

+ {% endif %} + {% if (not current_service.active) and current_user.platform_admin %} +

+

+ Service suspended +
+ + + Resume service + + +

+ {% endif %} + + {% endblock %} } diff --git a/tests/app/main/views/service_settings/test_service_setting_permissions.py b/tests/app/main/views/service_settings/test_service_setting_permissions.py index cb4516b65..ddfcfcfd0 100644 --- a/tests/app/main/views/service_settings/test_service_setting_permissions.py +++ b/tests/app/main/views/service_settings/test_service_setting_permissions.py @@ -83,7 +83,7 @@ def test_service_setting_toggles_show(get_service_settings_page, service_one, se @pytest.mark.parametrize('service_fields, endpoint, index, text', [ - ({'active': True}, '.archive_service', 0, 'Archive service'), + ({'active': True}, '.archive_service', 0, 'Delete this service'), ({'active': True}, '.suspend_service', 1, 'Suspend service'), ({'active': False}, '.resume_service', 0, 'Resume service'), pytest.param( diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 1f6815cf3..eb742153e 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -3621,38 +3621,72 @@ def test_service_switch_can_upload_document_lets_contact_details_be_added_and_sh assert normalize_spaces(page.h1.text) == 'Uploading documents' +@pytest.mark.parametrize('user', ( + platform_admin_user, + active_user_with_permissions, + pytest.param(active_user_no_settings_permission, marks=pytest.mark.xfail), +)) def test_archive_service_after_confirm( - logged_in_platform_admin_client, - service_one, + client_request, mocker, - mock_get_inbound_number_for_service, + mock_get_organisations_and_services_for_user, + user, + fake_uuid, ): - mocked_fn = mocker.patch('app.service_api_client.post', return_value=service_one) + mocked_fn = mocker.patch('app.service_api_client.post') + client_request.login(user(fake_uuid)) + page = client_request.post( + 'main.archive_service', + service_id=SERVICE_ONE_ID, + _follow_redirects=True, + ) - response = logged_in_platform_admin_client.post(url_for('main.archive_service', service_id=service_one['id'])) - - assert response.status_code == 302 - assert response.location == url_for('main.service_settings', service_id=service_one['id'], _external=True) - assert mocked_fn.call_args == call('/service/{}/archive'.format(service_one['id']), data=None) + mocked_fn.assert_called_once_with('/service/{}/archive'.format(SERVICE_ONE_ID), data=None) + assert normalize_spaces(page.select_one('h1').text) == 'Choose service' + assert normalize_spaces(page.select_one('.banner-default-with-tick').text) == ( + '‘service one’ was deleted' + ) +@pytest.mark.parametrize('user', ( + platform_admin_user, + active_user_with_permissions, + pytest.param(active_user_no_settings_permission, marks=pytest.mark.xfail), +)) def test_archive_service_prompts_user( - logged_in_platform_admin_client, - service_one, + client_request, mocker, single_reply_to_email_address, single_letter_contact_block, mock_get_service_organisation, single_sms_sender, mock_get_service_settings_page_common, + fake_uuid, + user, ): mocked_fn = mocker.patch('app.service_api_client.post') + client_request.login(user(fake_uuid)) - response = logged_in_platform_admin_client.get(url_for('main.archive_service', service_id=service_one['id'])) + settings_page = client_request.get( + 'main.archive_service', + service_id=SERVICE_ONE_ID + ) + delete_link = settings_page.select('.page-footer-delete-link a')[0] + assert normalize_spaces(delete_link.text) == 'Delete this service' + assert delete_link['href'] == url_for( + 'main.archive_service', + service_id=SERVICE_ONE_ID, + ) - assert response.status_code == 200 - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - assert 'Are you sure you want to archive this service?' in page.find('div', class_='banner-dangerous').text + delete_page = client_request.get( + 'main.archive_service', + service_id=SERVICE_ONE_ID, + ) + assert normalize_spaces(delete_page.select_one('.banner-dangerous').text) == ( + 'Are you sure you want to delete ‘service one’? ' + 'There’s no way to undo this. ' + 'Yes, delete' + ) assert mocked_fn.called is False @@ -3671,7 +3705,7 @@ def test_cant_archive_inactive_service( assert response.status_code == 200 page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - assert 'Archive service' not in {a.text for a in page.find_all('a', class_='button')} + assert 'Delete service' not in {a.text for a in page.find_all('a', class_='button')} def test_suspend_service_after_confirm(