diff --git a/app/__init__.py b/app/__init__.py index 17b17c563..0fd40481c 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -127,10 +127,10 @@ def create_app(): application.after_request(save_service_after_request) application.before_request(load_service_before_request) + @application.context_processor def _attach_current_service(): return {'current_service': current_service} - application.context_processor(_attach_current_service) register_errorhandlers(application) setup_event_handlers() @@ -338,10 +338,7 @@ def load_service_before_request(): else session.get('service_id') from flask.globals import _request_ctx_stack if _request_ctx_stack.top is not None: - setattr( - _request_ctx_stack.top, - 'service', - service_api_client.get_service(service_id)['data'] if service_id else None) + _request_ctx_stack.top.service = service_api_client.get_service(service_id)['data'] if service_id else None def save_service_after_request(response): diff --git a/app/main/__init__.py b/app/main/__init__.py index b70ccb942..de243703f 100644 --- a/app/main/__init__.py +++ b/app/main/__init__.py @@ -26,5 +26,6 @@ from app.main.views import ( invites, feedback, providers, - platform_admin + platform_admin, + letters ) diff --git a/app/main/views/letters.py b/app/main/views/letters.py new file mode 100644 index 000000000..4c8e1bd12 --- /dev/null +++ b/app/main/views/letters.py @@ -0,0 +1,15 @@ +from flask import render_template, abort +from flask_login import login_required + +from app import current_service +from app.main import main +from app.utils import user_has_permissions + + +@main.route("/services//letters") +@login_required +@user_has_permissions('manage_templates', 'send_letters', admin_override=True, any_=True) +def letters(service_id): + if not current_service['can_send_letters']: + abort(403) + return render_template('views/letters.html') diff --git a/app/main/views/send.py b/app/main/views/send.py index 2ef85dd24..9ed33784d 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -1,6 +1,4 @@ -import json import itertools -from datetime import datetime, timedelta from string import ascii_uppercase from contextlib import suppress diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 71a038bed..b65f449d0 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -168,7 +168,18 @@ def service_switch_live(service_id): 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} + {"research_mode": not current_service['research_mode']} + ) + return redirect(url_for('.service_settings', service_id=service_id)) + + +@main.route("/services//service-settings/can-send-letters") +@login_required +@user_has_permissions(admin_override=True) +def service_switch_can_send_letters(service_id): + service_api_client.update_service_with_properties( + service_id, + {"can_send_letters": not current_service['can_send_letters']} ) return redirect(url_for('.service_settings', service_id=service_id)) diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index dfa64de51..17ff797be 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -88,6 +88,7 @@ class ServiceAPIClient(NotificationsAPIClient): 'email_from', 'reply_to_email_address', 'research_mode', + 'can_send_letters', 'sms_sender', 'created_by', 'branding', diff --git a/app/templates/main_nav.html b/app/templates/main_nav.html index 5d51023ce..01711286a 100644 --- a/app/templates/main_nav.html +++ b/app/templates/main_nav.html @@ -50,6 +50,9 @@ {% if current_user.has_permissions(['view_activity', 'manage_templates', 'manage_api_keys'], admin_override=True, any_=True) %}
  • Email templates
  • Text message templates
    • + {% if current_service.can_send_letters %} +
  • Letter templates
    • + {% endif %} {% endif %} {% if current_user.has_permissions(['manage_users', 'manage_settings'], admin_override=True) %}
  • Team members
    • diff --git a/app/templates/views/letters.html b/app/templates/views/letters.html new file mode 100644 index 000000000..3a5fdf16f --- /dev/null +++ b/app/templates/views/letters.html @@ -0,0 +1,19 @@ +{% extends "withnav_template.html" %} +{% from "components/sms-message.html" import sms_message %} +{% from "components/email-message.html" import email_message %} +{% from "components/page-footer.html" import page_footer %} +{% from "components/file-upload.html" import file_upload %} +{% from "components/table.html" import list_table, text_field, index_field, index_field_heading %} + +{% block page_title %} + Upload recipients – GOV.UK Notify +{% endblock %} + +{% block maincolumn_content %} + +

    Letters 📩📨💌📮📜📑👌👌👌

    +

    + [insert content here] +

    + +{% endblock %} diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html index 1cbb9307c..3fda72be4 100644 --- a/app/templates/views/service-settings.html +++ b/app/templates/views/service-settings.html @@ -88,11 +88,16 @@ {{ 'Make service live' if current_service.restricted else 'Revert service to trial mode' }} -
  • +
  • {{ 'Take service out of research mode' if current_service.research_mode else 'Put into research mode' }}
    • +
  • + + {{ 'Stop sending letters' if current_service.can_send_letters else 'Allow to send letters' }} + +
    • {% endif %} diff --git a/tests/__init__.py b/tests/__init__.py index e23f597b5..c1ffb0d4f 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -40,9 +40,9 @@ def created_by_json(id_, name='', email_address=''): def service_json( - id_, - name, - users, + id_='1234', + name='Test Service', + users=None, message_limit=1000, active=False, restricted=True, @@ -50,9 +50,12 @@ def service_json( reply_to_email_address=None, sms_sender=None, research_mode=False, - organisation='organisation-id', + can_send_letters=False, + organisation=None, branding='govuk' ): + if users is None: + users = [] return { 'id': id_, 'name': name, @@ -64,6 +67,7 @@ def service_json( 'reply_to_email_address': reply_to_email_address, 'sms_sender': sms_sender, 'research_mode': research_mode, + 'can_send_letters': can_send_letters, 'organisation': organisation, 'branding': branding, 'created_at': str(datetime.utcnow()) diff --git a/tests/app/main/views/test_letters.py b/tests/app/main/views/test_letters.py new file mode 100644 index 000000000..8eef4599a --- /dev/null +++ b/tests/app/main/views/test_letters.py @@ -0,0 +1,56 @@ +import pytest +from flask import url_for + +from tests import service_json + + +@pytest.mark.parametrize('can_send_letters, response_code', [ + (True, 200), + (False, 403) +]) +def test_letters_access_restricted(logged_in_client, mocker, can_send_letters, response_code): + service = service_json(can_send_letters=can_send_letters) + mocker.patch('app.service_api_client.get_service', return_value={"data": service}) + + response = logged_in_client.get(url_for('main.letters', service_id=service['id'])) + + assert response.status_code == response_code + + +@pytest.mark.parametrize('permission', [ + 'send_letters', + 'manage_templates' +]) +def test_letters_lets_in_with_permissions( + client, + mocker, + mock_login, + mock_has_permissions, + api_user_active, + permission, +): + service = service_json(can_send_letters=True) + mocker.patch('app.service_api_client.get_service', return_value={"data": service}) + + api_user_active._permissions[str(service['id'])] = [permission] + + client.login(api_user_active) + response = client.get(url_for('main.letters', service_id=service['id'])) + + assert response.status_code == 200 + + +def test_letters_rejects_without_permissions( + client, + mocker, + mock_login, + mock_has_permissions, + api_user_active, +): + service = service_json(can_send_letters=True) + mocker.patch('app.service_api_client.get_service', return_value={"data": service}) + + client.login(api_user_active) + response = client.get(url_for('main.letters', service_id=service['id'])) + + assert response.status_code == 200 diff --git a/tests/app/main/views/test_main_nav.py b/tests/app/main/views/test_main_nav.py new file mode 100644 index 000000000..f0cf23870 --- /dev/null +++ b/tests/app/main/views/test_main_nav.py @@ -0,0 +1,26 @@ +from flask import url_for +from bs4 import BeautifulSoup + +from tests import service_json + + +def test_can_see_letters_if_allowed(logged_in_client, mocker): + service = service_json(can_send_letters=True) + mocker.patch('app.service_api_client.get_service', return_value={"data": service}) + + response = logged_in_client.get(url_for('main.service_settings', service_id=service['id'])) + + assert response.status_code == 200 + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + assert 'Letter templates' in page.find('nav', class_='navigation').text + + +def test_cant_see_letters_if_not_allowed(logged_in_client, mocker): + service = service_json(can_send_letters=False) + mocker.patch('app.service_api_client.get_service', return_value={"data": service}) + + response = logged_in_client.get(url_for('main.service_settings', service_id=service['id'])) + + assert response.status_code == 200 + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + assert 'Letter templates' not in page.find('nav', class_='navigation').text diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index cb26e0a9b..60ff51134 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -1,12 +1,13 @@ +from unittest.mock import call, ANY, Mock + import pytest from flask import url_for +from bs4 import BeautifulSoup +from werkzeug.exceptions import InternalServerError import app from app.utils import email_safe from tests import validate_route_permission, service_json -from bs4 import BeautifulSoup -from unittest.mock import ANY, Mock -from werkzeug.exceptions import InternalServerError def test_should_show_overview( @@ -474,75 +475,76 @@ def test_should_redirect_delete_confirmation(app_, assert mock_delete_service.called -def test_route_permissions(mocker, app_, api_user_active, service_one, mock_get_organisation): - routes = [ - 'main.service_settings', - 'main.service_name_change', - 'main.service_name_change_confirm', - 'main.service_request_to_go_live', - 'main.service_delete', - 'main.service_delete_confirm'] +@pytest.mark.parametrize('route', [ + 'main.service_settings', + 'main.service_name_change', + 'main.service_name_change_confirm', + 'main.service_request_to_go_live', + 'main.service_delete', + 'main.service_delete_confirm' +]) +def test_route_permissions(mocker, app_, api_user_active, service_one, 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']), - ['manage_settings'], - api_user_active, - service_one) + validate_route_permission( + mocker, + app_, + "GET", + 200, + url_for(route, service_id=service_one['id']), + ['manage_settings'], + api_user_active, + service_one) -def test_route_invalid_permissions(mocker, app_, api_user_active, service_one, mock_get_organisation): - routes = [ - 'main.service_settings', - '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_delete', - 'main.service_delete_confirm'] +@pytest.mark.parametrize('route', [ + 'main.service_settings', + '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_switch_can_send_letters', + 'main.service_delete', + 'main.service_delete_confirm' +]) +def test_route_invalid_permissions(mocker, app_, api_user_active, service_one, 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']), - ['blah'], - api_user_active, - service_one) + validate_route_permission( + mocker, + app_, + "GET", + 403, + url_for(route, service_id=service_one['id']), + ['blah'], + api_user_active, + service_one) -def test_route_for_platform_admin(mocker, app_, platform_admin_user, service_one, mock_get_organisation): - routes = [ - 'main.service_settings', - 'main.service_name_change', - 'main.service_name_change_confirm', - 'main.service_request_to_go_live', - 'main.service_delete', - 'main.service_delete_confirm' - ] +@pytest.mark.parametrize('route', [ + 'main.service_settings', + 'main.service_name_change', + 'main.service_name_change_confirm', + 'main.service_request_to_go_live', + 'main.service_delete', + 'main.service_delete_confirm' +]) +def test_route_for_platform_admin(mocker, app_, platform_admin_user, service_one, 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']), - [], - platform_admin_user, - service_one) + validate_route_permission(mocker, + app_, + "GET", + 200, + url_for(route, service_id=service_one['id']), + [], + platform_admin_user, + 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_research_mode', + 'main.service_switch_can_send_letters' ] with app_.test_request_context(): for route in routes: @@ -814,3 +816,27 @@ def test_should_set_branding_and_organisations( branding='org', organisation='organisation-id' ) + + +def test_switch_service_enable_letters(client, platform_admin_user, service_one, mocker): + mocked_fn = mocker.patch('app.service_api_client.update_service_with_properties', return_value=service_one) + + client.login(platform_admin_user, mocker, service_one) + response = client.get(url_for('main.service_switch_can_send_letters', 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_one['id'], {'can_send_letters': True}) + + +def test_switch_service_disable_letters(client, platform_admin_user, mocker): + service = service_json("1234", "Test Service", [], can_send_letters=True) + mocker.patch('app.service_api_client.get_service', return_value={"data": service}) + mocked_fn = mocker.patch('app.service_api_client.update_service_with_properties', return_value=service) + + client.login(platform_admin_user, mocker, service) + response = client.get(url_for('main.service_switch_can_send_letters', service_id=service['id'])) + + 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}) diff --git a/tests/conftest.py b/tests/conftest.py index 5a110ff71..7dd5c9359 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1307,3 +1307,16 @@ def mock_update_whitelist(mocker): def client(app_): with app_.test_request_context(), app_.test_client() as client: yield client + + +@pytest.fixture(scope='function') +def logged_in_client( + client, + active_user_with_permissions, + mock_login, + mock_get_user, + mock_get_service, + mock_has_permissions +): + client.login(active_user_with_permissions) + yield client