diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index fc7f2a675..7dce7adba 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -184,6 +184,14 @@ def service_switch_can_send_letters(service_id): return redirect(url_for('.service_settings', service_id=service_id)) +@main.route("/services//service-settings/deactivate", methods=['GET', 'POST']) +@login_required +@user_has_permissions('manage_settings', admin_override=True) +def deactivate_service(service_id): + service_api_client.deactivate_service(service_id) + return redirect(url_for('.service_settings', service_id=service_id)) + + @main.route("/services//service-settings/set-reply-to-email", methods=['GET', 'POST']) @login_required @user_has_permissions('manage_settings', admin_override=True) diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index b3df8d18f..9766d876c 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -1,11 +1,11 @@ from __future__ import unicode_literals from flask import url_for -from notifications_python_client.notifications import NotificationsAPIClient +from notifications_python_client.base import BaseAPIClient from app.utils import BrowsableItem from app.notify_client import _attach_current_user -class ServiceAPIClient(NotificationsAPIClient): +class ServiceAPIClient(BaseAPIClient): # Fudge assert in the super __init__ so # we can set those variables later. def __init__(self): @@ -97,6 +97,9 @@ class ServiceAPIClient(NotificationsAPIClient): def update_service_with_properties(self, service_id, properties): return self.update_service(service_id, **properties) + def deactivate_service(self, service_id): + return self.post('/service/{}/deactivate'.format(service_id), data=None) + def remove_user_from_service(self, service_id, user_id): """ Remove a user from a service diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html index 3fda72be4..041fe301f 100644 --- a/app/templates/views/service-settings.html +++ b/app/templates/views/service-settings.html @@ -98,6 +98,13 @@ {{ 'Stop sending letters' if current_service.can_send_letters else 'Allow to send letters' }} + {% if current_service.active %} +
  • + + Deactivate service + +
    • + {% endif %} {% endif %} diff --git a/tests/app/main/views/test_api_keys.py b/tests/app/main/views/test_api_keys.py index 0de62599f..12462d0de 100644 --- a/tests/app/main/views/test_api_keys.py +++ b/tests/app/main/views/test_api_keys.py @@ -1,6 +1,7 @@ import uuid - from collections import OrderedDict + +import pytest from flask import url_for from bs4 import BeautifulSoup @@ -244,48 +245,50 @@ def test_should_redirect_after_revoking_api_key(app_, mock_get_api_keys.assert_called_once_with(service_id=fake_uuid, key_id=fake_uuid) +@pytest.mark.parametrize('route', [ + 'main.api_keys', + 'main.create_api_key', + 'main.revoke_api_key' +]) def test_route_permissions(mocker, app_, api_user_active, service_one, - mock_get_api_keys): - routes = [ - 'main.api_keys', - 'main.create_api_key', - 'main.revoke_api_key'] + mock_get_api_keys, + route): with app_.test_request_context(): - for route in routes: - validate_route_permission( - mocker, - app_, - "GET", - 200, - url_for(route, service_id=service_one['id'], key_id=123), - ['manage_api_keys'], - api_user_active, - service_one) + validate_route_permission( + mocker, + app_, + "GET", + 200, + url_for(route, service_id=service_one['id'], key_id=123), + ['manage_api_keys'], + api_user_active, + service_one) +@pytest.mark.parametrize('route', [ + 'main.api_keys', + 'main.create_api_key', + 'main.revoke_api_key' +]) def test_route_invalid_permissions(mocker, app_, api_user_active, service_one, - mock_get_api_keys): - routes = [ - 'main.api_keys', - 'main.create_api_key', - 'main.revoke_api_key'] + mock_get_api_keys, + route): with app_.test_request_context(): - for route in routes: - validate_route_permission( - mocker, - app_, - "GET", - 403, - url_for(route, service_id=service_one['id'], key_id=123), - ['view_activity'], - api_user_active, - service_one) + validate_route_permission( + mocker, + app_, + "GET", + 403, + url_for(route, service_id=service_one['id'], key_id=123), + ['view_activity'], + api_user_active, + service_one) def test_should_show_whitelist_page( diff --git a/tests/app/main/views/test_dashboard.py b/tests/app/main/views/test_dashboard.py index 09a243e8c..1d8b79c24 100644 --- a/tests/app/main/views/test_dashboard.py +++ b/tests/app/main/views/test_dashboard.py @@ -440,22 +440,16 @@ def test_route_for_service_permissions(mocker, mock_get_template_statistics, mock_get_detailed_service, mock_get_usage): - routes = [ - 'main.service_dashboard'] with app_.test_request_context(): - # Just test that the user is part of the service - for route in routes: - validate_route_permission( - mocker, - app_, - "GET", - 200, - url_for( - route, - service_id=service_one['id']), - ['view_activity'], - api_user_active, - service_one) + validate_route_permission( + mocker, + app_, + "GET", + 200, + url_for('main.service_dashboard', service_id=service_one['id']), + ['view_activity'], + api_user_active, + service_one) def test_aggregate_template_stats(): diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index fbcd64280..5c7736972 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -1,12 +1,14 @@ -import pytest -import re -from itertools import repeat from io import BytesIO from os import path from glob import glob -from bs4 import BeautifulSoup +import re +from itertools import repeat from functools import partial + +import pytest +from bs4 import BeautifulSoup from flask import url_for + from tests import validate_route_permission template_types = ['email', 'sms'] @@ -447,6 +449,12 @@ def test_check_messages_should_revalidate_file_when_uploading_file( assert 'There is a problem with your data' in response.get_data(as_text=True) +@pytest.mark.parametrize('route, response_code', [ + ('main.choose_template', 200), + ('main.send_messages', 200), + ('main.get_example_csv', 200), + ('main.send_test', 302) +]) def test_route_permissions(mocker, app_, api_user_active, @@ -457,35 +465,17 @@ def test_route_permissions(mocker, mock_get_notifications, mock_create_job, mock_s3_upload, - fake_uuid): - routes = [ - 'main.choose_template', - 'main.send_messages', - 'main.get_example_csv'] - with app_.test_request_context(): - for route in routes: - validate_route_permission( - mocker, - app_, - "GET", - 200, - url_for( - route, - service_id=service_one['id'], - template_type='sms', - template_id=fake_uuid), - ['send_texts', 'send_emails', 'send_letters'], - api_user_active, - service_one) - + fake_uuid, + route, + response_code): with app_.test_request_context(): validate_route_permission( mocker, app_, "GET", - 302, + response_code, url_for( - 'main.send_test', + route, service_id=service_one['id'], template_type='sms', template_id=fake_uuid), @@ -494,6 +484,12 @@ def test_route_permissions(mocker, service_one) +@pytest.mark.parametrize('route', [ + 'main.choose_template', + 'main.send_messages', + 'main.get_example_csv', + 'main.send_test' +]) def test_route_invalid_permissions(mocker, app_, api_user_active, @@ -503,27 +499,22 @@ def test_route_invalid_permissions(mocker, mock_get_jobs, mock_get_notifications, mock_create_job, - fake_uuid): - routes = [ - 'main.choose_template', - 'main.send_messages', - 'main.get_example_csv', - 'main.send_test'] + fake_uuid, + route): with app_.test_request_context(): - for route in routes: - validate_route_permission( - mocker, - app_, - "GET", - 403, - url_for( - route, - service_id=service_one['id'], - template_type='sms', - template_id=fake_uuid), - ['blah'], - api_user_active, - service_one) + validate_route_permission( + mocker, + app_, + "GET", + 403, + url_for( + route, + service_id=service_one['id'], + template_type='sms', + template_id=fake_uuid), + ['blah'], + api_user_active, + service_one) def test_route_choose_template_manage_service_permissions(mocker, diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 5505283e6..e8e30f01d 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -416,6 +416,7 @@ def test_route_permissions(mocker, app_, api_user_active, service_one, route): 'main.service_switch_live', 'main.service_switch_research_mode', 'main.service_switch_can_send_letters', + 'main.deactivate_service', ]) def test_route_invalid_permissions(mocker, app_, api_user_active, service_one, route): with app_.test_request_context(): @@ -448,22 +449,23 @@ def test_route_for_platform_admin(mocker, app_, platform_admin_user, service_one service_one) -def test_route_for_platform_admin_update_service(mocker, app_, platform_admin_user, service_one): - routes = [ - 'main.service_switch_live', - 'main.service_switch_research_mode', - 'main.service_switch_can_send_letters' - ] +@pytest.mark.parametrize('route', [ + 'main.service_switch_live', + 'main.service_switch_research_mode', + 'main.service_switch_can_send_letters', + 'main.deactivate_service', +]) +def test_route_for_platform_admin_update_service(mocker, app_, platform_admin_user, service_one, route): + mocker.patch('app.service_api_client.deactivate_service') with app_.test_request_context(): - for route in routes: - validate_route_permission(mocker, - app_, - "GET", - 302, - url_for(route, service_id=service_one['id']), - [], - platform_admin_user, - service_one) + validate_route_permission(mocker, + app_, + "GET", + 302, + url_for(route, service_id=service_one['id']), + [], + platform_admin_user, + service_one) def test_set_reply_to_email_address( @@ -748,3 +750,25 @@ def test_switch_service_disable_letters(client, platform_admin_user, mocker): assert response.status_code == 302 assert response.location == url_for('main.service_settings', service_id=service['id'], _external=True) assert mocked_fn.call_args == call(service['id'], {"can_send_letters": False}) + + +def test_deactivate_service(client, platform_admin_user, service_one, mocker): + mocked_fn = mocker.patch('app.service_api_client.post', return_value=service_one) + + client.login(platform_admin_user, mocker, service_one) + response = client.get(url_for('main.deactivate_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/{}/deactivate'.format(service_one['id']), data=None) + + +def test_cant_deactivate_inactive_service(client, platform_admin_user, service_one, mocker): + service_one['active'] = False + + client.login(platform_admin_user, mocker, service_one) + response = client.get(url_for('main.service_settings', service_id=service_one['id'])) + + assert response.status_code == 200 + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + assert 'Deactivate service' not in {a.text for a in page.find_all('a', class_='button')}