From 9c25b2d2da24236d333d142509d6a26a105bcb56 Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Tue, 3 Oct 2017 13:15:52 +0100
Subject: [PATCH] Add note about letters
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When trying to send letters using the API, the ‘team and whitelist’ key
is confusing. We don’t have addresses for your team members, nor is
there a whitelist for letter addresses. The actual behaviour is that
you’ll get an error if you try to use this key to send letters.

So, for services who have letters available, we should add a hint
telling users that team and whitelist is probably not the key they’re
looking for.
---
 app/main/views/api_keys.py            |  9 +++++----
 tests/app/main/views/test_api_keys.py | 12 ++++++++++++
 tests/conftest.py                     | 13 +++++++++++++
 3 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/app/main/views/api_keys.py b/app/main/views/api_keys.py
index 68a93f8d2..a35b95558 100644
--- a/app/main/views/api_keys.py
+++ b/app/main/views/api_keys.py
@@ -69,14 +69,15 @@ def create_api_key(service_id):
         (KEY_TYPE_TEAM, 'Team and whitelist – limits who you can send to'),
         (KEY_TYPE_TEST, 'Test – pretends to send messages'),
     ]
+    disabled_options, option_hints = [], {}
     if current_service['restricted']:
         disabled_options = [KEY_TYPE_NORMAL]
-        option_hints = {KEY_TYPE_NORMAL: Markup(
+        option_hints[KEY_TYPE_NORMAL] = Markup(
             'This option is not available because your service is in '
             '<a href="{}#trial-mode">trial mode</a>'.format(url_for(".using_notify"))
-        )}
-    else:
-        disabled_options, option_hints = [], {}
+        )
+    if 'letter' in current_service['permissions']:
+        option_hints[KEY_TYPE_TEAM] = 'Can’t be used to send letters'
     if form.validate_on_submit():
         if form.key_type.data in disabled_options:
             abort(400)
diff --git a/tests/app/main/views/test_api_keys.py b/tests/app/main/views/test_api_keys.py
index bed5c5d00..1b680774b 100644
--- a/tests/app/main/views/test_api_keys.py
+++ b/tests/app/main/views/test_api_keys.py
@@ -10,6 +10,7 @@ from tests import validate_route_permission
 from tests.conftest import (
     mock_get_service,
     mock_get_live_service,
+    mock_get_service_with_letters,
     normalize_spaces,
     SERVICE_ONE_ID,
 )
@@ -145,6 +146,17 @@ def test_should_show_api_keys_page(
             'Test – pretends to send messages',
         ]
     ),
+    (
+        mock_get_service_with_letters,
+        [
+            'Live – sends to anyone',
+            (
+                'Team and whitelist – limits who you can send to '
+                'Can’t be used to send letters'
+            ),
+            'Test – pretends to send messages',
+        ]
+    ),
 ])
 def test_should_show_create_api_key_page(
     client_request,
diff --git a/tests/conftest.py b/tests/conftest.py
index 47412355c..af0c64b62 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -268,6 +268,19 @@ def mock_get_live_service(mocker, api_user_active):
     return mocker.patch('app.service_api_client.get_service', side_effect=_get)
 
 
+@pytest.fixture(scope='function')
+def mock_get_service_with_letters(mocker, api_user_active):
+    def _get(service_id):
+        return {'data': service_json(
+            service_id,
+            users=[api_user_active.id],
+            restricted=False,
+            permissions=['email', 'sms', 'letter']
+        )}
+
+    return mocker.patch('app.service_api_client.get_service', side_effect=_get)
+
+
 @pytest.fixture(scope='function')
 def mock_create_service(mocker):
     def _create(service_name, message_limit, restricted, user_id, email_from):