Merge pull request #1000 from alphagov/letters-flag

Letters flag

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):

app/main/__init__.py

@@ -26,5 +26,6 @@ from app.main.views import (
     invites,
     feedback,
     providers,
-    platform_admin
+    platform_admin,
+    letters
 )

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/<service_id>/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')

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

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_id>/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))
 

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',

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) %}
     <li><a href="{{ url_for('.choose_template', service_id=current_service.id, template_type='email') }}">Email templates</a></li>
     <li><a href="{{ url_for('.choose_template', service_id=current_service.id, template_type='sms') }}">Text message templates</a></li>
+    {% if current_service.can_send_letters %}
+      <li><a href="{{ url_for('.letters', service_id=current_service.id) }}">Letter templates</a></li>
+    {% endif %}
   {% endif %}
   {% if current_user.has_permissions(['manage_users', 'manage_settings'], admin_override=True) %}
     <li><a href="{{ url_for('.manage_users', service_id=current_service.id) }}">Team members</a></li>

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 %}
+
+  <h1 class="heading-large">Letters 📩📨💌📮📜📑👌👌👌</h1>
+  <p>
+    [insert content here]
+  </p>
+
+{% endblock %}

app/templates/views/service-settings.html

@@ -88,11 +88,16 @@
             {{ 'Make service live' if current_service.restricted else 'Revert service to trial mode' }}
           </a>
         </li>
-        <li>
+        <li class="bottom-gutter">
           <a href="{{ url_for('.service_switch_research_mode', service_id=current_service.id) }}" class="button">
             {{ 'Take service out of research mode' if current_service.research_mode else 'Put into research mode' }}
           </a>
         </li>
+        <li class="bottom-gutter">
+          <a href="{{ url_for('.service_switch_can_send_letters', service_id=current_service.id) }}" class="button">
+            {{ 'Stop sending letters' if current_service.can_send_letters else 'Allow to send letters' }}
+          </a>
+        </li>
       </ul>
 
     {% endif %}

tests/__init__.py

@@ -40,9 +40,9 @@ def created_by_json(id_, name='', email_address=''):
 
 
 def service_json(
-    id_,
+    id_='1234',
-    name,
+    name='Test Service',
-    users,
+    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())

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

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

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):
+@pytest.mark.parametrize('route', [
-    routes = [
+    'main.service_settings',
-        'main.service_settings',
+    'main.service_name_change',
-        'main.service_name_change',
+    'main.service_name_change_confirm',
-        'main.service_name_change_confirm',
+    'main.service_request_to_go_live',
-        'main.service_request_to_go_live',
+    'main.service_delete',
-        'main.service_delete',
+    'main.service_delete_confirm'
-        '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(
-            validate_route_permission(
+            mocker,
-                mocker,
+            app_,
-                app_,
+            "GET",
-                "GET",
+            200,
-                200,
+            url_for(route, service_id=service_one['id']),
-                url_for(route, service_id=service_one['id']),
+            ['manage_settings'],
-                ['manage_settings'],
+            api_user_active,
-                api_user_active,
+            service_one)
-                service_one)
 
 
-def test_route_invalid_permissions(mocker, app_, api_user_active, service_one, mock_get_organisation):
+@pytest.mark.parametrize('route', [
-    routes = [
+    'main.service_settings',
-        'main.service_settings',
+    'main.service_name_change',
-        'main.service_name_change',
+    'main.service_name_change_confirm',
-        'main.service_name_change_confirm',
+    'main.service_request_to_go_live',
-        'main.service_request_to_go_live',
+    'main.service_switch_live',
-        'main.service_switch_live',
+    'main.service_switch_research_mode',
-        'main.service_switch_research_mode',
+    'main.service_switch_can_send_letters',
-        'main.service_delete',
+    'main.service_delete',
-        'main.service_delete_confirm']
+    '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(
-            validate_route_permission(
+            mocker,
-                mocker,
+            app_,
-                app_,
+            "GET",
-                "GET",
+            403,
-                403,
+            url_for(route, service_id=service_one['id']),
-                url_for(route, service_id=service_one['id']),
+            ['blah'],
-                ['blah'],
+            api_user_active,
-                api_user_active,
+            service_one)
-                service_one)
 
 
-def test_route_for_platform_admin(mocker, app_, platform_admin_user, service_one, mock_get_organisation):
+@pytest.mark.parametrize('route', [
-    routes = [
+    'main.service_settings',
-        'main.service_settings',
+    'main.service_name_change',
-        'main.service_name_change',
+    'main.service_name_change_confirm',
-        'main.service_name_change_confirm',
+    'main.service_request_to_go_live',
-        'main.service_request_to_go_live',
+    'main.service_delete',
-        'main.service_delete',
+    'main.service_delete_confirm'
-        '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,
-            validate_route_permission(mocker,
+                                  app_,
-                                      app_,
+                                  "GET",
-                                      "GET",
+                                  200,
-                                      200,
+                                  url_for(route, service_id=service_one['id']),
-                                      url_for(route, service_id=service_one['id']),
+                                  [],
-                                      [],
+                                  platform_admin_user,
-                                      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_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})

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