diff --git a/app/__init__.py b/app/__init__.py index bd2f3d619..cb370773b 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -285,6 +285,7 @@ def register_errorhandlers(application): @application.errorhandler(HTTPError) def render_http_error(error): + application.logger.error("API called failed with status {} message {}".format(error.status_code, error.message)) error_code = error.status_code if error_code not in [401, 404, 403, 500]: error_code = 500 diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 7e3b6c37a..023515b9a 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -55,10 +55,10 @@ def service_name_change(service_id): @login_required @user_has_permissions('manage_settings', admin_override=True) def service_name_change_confirm(service_id): - # Validate password for form def _check_password(pwd): return user_api_client.verify_password(current_user.id, pwd) + form = ConfirmPasswordForm(_check_password) if form.validate_on_submit(): @@ -94,7 +94,6 @@ def service_name_change_confirm(service_id): @login_required @user_has_permissions('manage_settings', admin_override=True) def service_request_to_go_live(service_id): - form = RequestToGoLiveForm() if form.validate_on_submit(): @@ -126,7 +125,7 @@ def service_request_to_go_live(service_id): "Deskpro create ticket request failed with {} '{}'".format( resp.status_code, resp.json()) - ) + ) abort(500, "Request to go live submission failed") flash('We’ve received your request to go live', 'default') @@ -152,6 +151,17 @@ def service_switch_live(service_id): return redirect(url_for('.service_settings', service_id=service_id)) +@main.route("/services//service-settings/research-mode") +@login_required +@user_has_permissions(admin_override=True) +def service_switch_research_mode(service_id): + service_api_client.update_service_with_properties( + service_id, + {"research_mode": False if current_service['research_mode'] else True} + ) + return redirect(url_for('.service_settings', service_id=service_id)) + + @main.route("/services//service-settings/status", methods=['GET', 'POST']) @login_required @user_has_permissions('manage_settings', admin_override=True) @@ -171,6 +181,7 @@ def service_status_change_confirm(service_id): # Validate password for form def _check_password(pwd): return user_api_client.verify_password(current_user.id, pwd) + form = ConfirmPasswordForm(_check_password) if form.validate_on_submit(): @@ -195,7 +206,6 @@ def service_status_change_confirm(service_id): @login_required @user_has_permissions('manage_settings', admin_override=True) def service_delete(service_id): - if request.method == 'GET': return render_template( 'views/service-settings/delete.html' @@ -208,10 +218,10 @@ def service_delete(service_id): @login_required @user_has_permissions('manage_settings', admin_override=True) def service_delete_confirm(service_id): - # Validate password for form def _check_password(pwd): return user_api_client.verify_password(current_user.id, pwd) + form = ConfirmPasswordForm(_check_password) if form.validate_on_submit(): diff --git a/app/notify_client/api_client.py b/app/notify_client/api_client.py index 397d371a5..a2c3af94f 100644 --- a/app/notify_client/api_client.py +++ b/app/notify_client/api_client.py @@ -6,7 +6,6 @@ from app.notify_client import _attach_current_user class ServiceAPIClient(NotificationsAPIClient): - # Fudge assert in the super __init__ so # we can set those variables later. def __init__(self): @@ -81,6 +80,11 @@ class ServiceAPIClient(NotificationsAPIClient): endpoint = "/service/{0}".format(service_id) return self.post(endpoint, data) + def update_service_with_properties(self, service_id, properties): + _attach_current_user(properties) + endpoint = "/service/{0}".format(service_id) + return self.post(endpoint, properties) + def remove_user_from_service(self, service_id, user_id): """ Remove a user from a service @@ -181,7 +185,6 @@ class ServiceAPIClient(NotificationsAPIClient): class ServicesBrowsableItem(BrowsableItem): - @property def title(self): return self._item['name'] diff --git a/app/templates/admin_template.html b/app/templates/admin_template.html index 2789e3302..43722d1cc 100644 --- a/app/templates/admin_template.html +++ b/app/templates/admin_template.html @@ -111,6 +111,9 @@ {% endblock %} diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html index c86f88d80..44bf6bda8 100644 --- a/app/templates/views/service-settings.html +++ b/app/templates/views/service-settings.html @@ -40,6 +40,14 @@ 'link': url_for('.service_switch_live', service_id=current_service.id) } if not current_service.restricted and current_user.has_permissions([], admin_override=True) else { }, + { + 'title': 'Put service into research mode', + 'link': url_for('.service_switch_research_mode', service_id=current_service.id) + } if not current_service.research_mode and current_user.has_permissions([], admin_override=True) else { + 'title': 'Take service out of research mode', + 'link': url_for('.service_switch_research_mode', service_id=current_service.id) + } if current_service.research_mode and current_user.has_permissions([], admin_override=True) else { + }, ]) }} {% endblock %} diff --git a/tests/__init__.py b/tests/__init__.py index f5b68a2e3..b92117cf1 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -39,7 +39,16 @@ def created_by_json(id_, name='', email_address=''): return {'id': id_, 'name': name, 'email_address': email_address} -def service_json(id_, name, users, message_limit=1000, active=False, restricted=True, email_from=None, reply_to_email_address=None): # noqa +def service_json( + id_, + name, + users, + message_limit=1000, + active=False, + restricted=True, + email_from=None, + reply_to_email_address=None, + research_mode=False): return { 'id': id_, 'name': name, @@ -48,7 +57,8 @@ def service_json(id_, name, users, message_limit=1000, active=False, restricted= 'active': active, 'restricted': restricted, 'email_from': email_from, - 'reply_to_email_address': reply_to_email_address + 'reply_to_email_address': reply_to_email_address, + 'research_mode': research_mode } @@ -223,6 +233,8 @@ def validate_route_permission(mocker, mocker.patch( 'app.service_api_client.get_services', return_value={'data': []}) + mocker.patch('app.service_api_client.update_service', return_value=service) + mocker.patch('app.service_api_client.update_service_with_properties', return_value=service) mocker.patch('app.user_api_client.get_user', return_value=usr) mocker.patch('app.user_api_client.get_user_by_email', return_value=usr) mocker.patch('app.service_api_client.get_service', return_value={'data': service}) @@ -238,5 +250,6 @@ def validate_route_permission(mocker, else: pytest.fail("Invalid method call {}".format(method)) if resp.status_code != response_code: + print(resp.status_code) pytest.fail("Invalid permissions set for endpoint {}".format(route)) return resp diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index f14c2b300..eb226b6e7 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -3,7 +3,7 @@ from flask import url_for import app from app.utils import email_safe -from tests import validate_route_permission +from tests import validate_route_permission, service_json from bs4 import BeautifulSoup from unittest.mock import ANY, Mock from werkzeug.exceptions import InternalServerError @@ -215,12 +215,12 @@ def test_should_show_request_to_go_live(app_, def test_should_redirect_after_request_to_go_live( - app_, - api_user_active, - mock_get_user, - mock_get_service, - mock_has_permissions, - mocker + app_, + api_user_active, + mock_get_user, + mock_get_service, + mock_has_permissions, + mocker ): mock_post = mocker.patch( 'app.main.views.feedback.requests.post', @@ -241,6 +241,7 @@ def test_should_redirect_after_request_to_go_live( 'department_id': ANY, 'agent_team_id': ANY, 'message': 'From Test User on behalf of Test Service (http://localhost/services/6ce466d0-fd6a-11e5-82f5-e0accb9d11a6/dashboard)\n\nUsage estimate\n---\n\nOne million messages', # noqa + # noqa 'person_email': ANY }, headers=ANY @@ -254,12 +255,12 @@ def test_should_redirect_after_request_to_go_live( def test_log_error_on_request_to_go_live( - app_, - api_user_active, - mock_get_user, - mock_get_service, - mock_has_permissions, - mocker + app_, + api_user_active, + mock_get_user, + mock_get_service, + mock_has_permissions, + mocker ): mock_post = mocker.patch( 'app.main.views.service_settings.requests.post', @@ -489,6 +490,8 @@ def test_route_invalid_permissions(mocker, app_, api_user_active, service_one): 'main.service_name_change', 'main.service_name_change_confirm', 'main.service_request_to_go_live', + 'main.service_switch_live', + 'main.service_switch_research_mode', 'main.service_status_change', 'main.service_status_change_confirm', 'main.service_delete', @@ -516,7 +519,7 @@ def test_route_for_platform_admin(mocker, app_, platform_admin_user, service_one 'main.service_status_change_confirm', 'main.service_delete', 'main.service_delete_confirm' - ] + ] with app_.test_request_context(): for route in routes: validate_route_permission(mocker, @@ -529,6 +532,24 @@ 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' + ] + 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) + + + def test_set_reply_to_email_address(app_, active_user_with_permissions, mocker, @@ -556,7 +577,6 @@ def test_if_reply_to_email_address_set_then_form_populated(app_, active_user_with_permissions, mocker, service_one): - service_one['reply_to_email_address'] = 'test@service.gov.uk' with app_.test_request_context(): with app_.test_client() as client: @@ -566,3 +586,109 @@ def test_if_reply_to_email_address_set_then_form_populated(app_, assert response.status_code == 200 page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') assert page.find(id='email_address')['value'] == 'test@service.gov.uk' + + +def test_switch_service_to_research_mode( + app_, + service_one, + mock_login, + mock_get_user, + active_user_with_permissions, + mock_get_service, + mock_has_permissions, + mocker): + with app_.test_request_context(): + with app_.test_client() as client: + mocker.patch('app.service_api_client.update_service_with_properties', return_value=service_one) + + client.login(active_user_with_permissions) + response = client.get(url_for('main.service_switch_research_mode', 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) + app.service_api_client.update_service_with_properties.assert_called_with( + service_one['id'], {"research_mode": True} + ) + + +def test_switch_service_from_research_mode_to_normal( + app_, + service_one, + mock_login, + mock_get_user, + active_user_with_permissions, + mock_get_service, + mock_has_permissions, + mocker): + with app_.test_request_context(): + with app_.test_client() as client: + service = service_json( + "1234", + "Test Service", + [active_user_with_permissions.id], + message_limit=1000, + active=False, + restricted=True, + research_mode=True + ) + mocker.patch('app.service_api_client.get_service', return_value={"data": service}) + mocker.patch('app.service_api_client.update_service_with_properties', return_value=service_one) + + client.login(active_user_with_permissions) + response = client.get(url_for('main.service_switch_research_mode', 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) + app.service_api_client.update_service_with_properties.assert_called_with( + service_one['id'], {"research_mode": False} + ) + + +def test_shows_research_mode_indicator( + app_, + service_one, + mock_login, + mock_get_user, + active_user_with_permissions, + mock_get_service, + mock_has_permissions, + mocker): + with app_.test_request_context(): + with app_.test_client() as client: + service = service_json( + "1234", + "Test Service", + [active_user_with_permissions.id], + message_limit=1000, + active=False, + restricted=True, + research_mode=True + ) + mocker.patch('app.service_api_client.get_service', return_value={"data": service}) + mocker.patch('app.service_api_client.update_service_with_properties', return_value=service_one) + + client.login(active_user_with_permissions) + 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') + element = page.find('span', {"id": "research-mode"}) + assert element.text == 'research mode' + + +def test_does_not_show_research_mode_indicator( + app_, + service_one, + mock_login, + mock_get_user, + active_user_with_permissions, + mock_get_service, + mock_has_permissions, + mocker): + with app_.test_request_context(): + with app_.test_client() as client: + client.login(active_user_with_permissions) + 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') + element = page.find('span', {"id": "research-mode"}) + assert not element \ No newline at end of file