Always show live key, reword key labels

This is trying to resolve these confusions:

- that you’re in trial mode, which means you can’t have a live key yet (
  or you can but it wont work, which is what we used to have)
- what does simulate mean

The create key page is the right place to resolve these confusions
because it’s where users are actively reading.

This commit also removes the trial mode banner from API integration
page because this where users _aren’t_ actively reading. A whole bunch
of users weren’t seeing this banner at all.

The implementation of the disabled API key options is kinda clunky
because WTForms doesn’t have a native way of doing this.

¯\_(ツ)_/¯
This commit is contained in:
Chris Hill-Scott
2016-11-01 15:34:04 +00:00
parent 9ec1f21f72
commit 4ae28386a6
7 changed files with 50 additions and 27 deletions

View File

@@ -214,3 +214,12 @@ summary::-webkit-details-marker {
details .arrow { details .arrow {
font-size: 16px; font-size: 16px;
} }
.block-label-hint {
@include core-16;
margin-top: 5px;
}
.block-label input[disabled] {
opacity: 0.5;
}

View File

@@ -1,4 +1,4 @@
from flask import request, render_template, redirect, url_for, flash from flask import request, render_template, redirect, url_for, flash, Markup, abort
from flask_login import login_required from flask_login import login_required
from app.main import main from app.main import main
from app.main.forms import CreateKeyForm, Whitelist from app.main.forms import CreateKeyForm, Whitelist
@@ -68,13 +68,22 @@ def create_api_key(service_id):
key['name'] for key in api_key_api_client.get_api_keys(service_id=service_id)['apiKeys'] key['name'] for key in api_key_api_client.get_api_keys(service_id=service_id)['apiKeys']
] ]
form = CreateKeyForm(key_names) form = CreateKeyForm(key_names)
form.key_type.choices = filter(None, [ form.key_type.choices = [
(KEY_TYPE_NORMAL, 'Send messages to anyone') (KEY_TYPE_NORMAL, 'Send messages to anyone'),
if not current_service['restricted'] else None, (KEY_TYPE_TEAM, 'Send messages to anyone on my whitelist'),
(KEY_TYPE_TEST, 'Simulate sending messages to anyone'), (KEY_TYPE_TEST, 'Pretend to send messages to anyone'),
(KEY_TYPE_TEAM, 'Only send messages to your team or whitelist') ]
]) if current_service['restricted']:
disabled_options = [KEY_TYPE_NORMAL]
option_hints = {KEY_TYPE_NORMAL: Markup(
'This option is not available because your service is in '
'<a href="{}">trial mode</a>'.format(url_for(".trial_mode"))
)}
else:
disabled_options, option_hints = [], {}
if form.validate_on_submit(): if form.validate_on_submit():
if form.key_type.data in disabled_options:
abort(400)
secret = api_key_api_client.create_api_key( secret = api_key_api_client.create_api_key(
service_id=service_id, service_id=service_id,
key_name=form.key_name.data, key_name=form.key_name.data,
@@ -88,7 +97,9 @@ def create_api_key(service_id):
) )
return render_template( return render_template(
'views/api/keys/create.html', 'views/api/keys/create.html',
form=form form=form,
disabled_options=disabled_options,
option_hints=option_hints
) )

View File

@@ -1,6 +1,8 @@
{% macro radios( {% macro radios(
field, field,
hint=None hint=None,
disable=[],
option_hints={}
) %} ) %}
<div class="form-group {% if field.errors %} error{% endif %}"> <div class="form-group {% if field.errors %} error{% endif %}">
<fieldset> <fieldset>
@@ -14,8 +16,21 @@
</legend> </legend>
{% for option in field %} {% for option in field %}
<label class="block-label" for="{{ option.id }}"> <label class="block-label" for="{{ option.id }}">
{{ option }} <input
id="{{ option.id }}" name="{{ option.name }}" type="radio" value="{{ option.data }}"
{% if option.data in disable %}
disabled
{% endif %}
{% if option.checked %}
checked
{% endif %}
>
{{ option.label.text }} {{ option.label.text }}
{% if option_hints[option.data] %}
<div class="block-label-hint">
{{ option_hints[option.data] }}
</div>
{% endif %}
</label> </label>
{% endfor %} {% endfor %}
</fieldset> </fieldset>

View File

@@ -13,15 +13,6 @@
API integration API integration
</h1> </h1>
{% if current_service.restricted %}
{% call banner_wrapper(type='warning') %}
<h2 class="heading-medium">Your service is in trial mode</h2>
<p>
You can only send messages to people in your team or whitelist.
</p>
{% endcall %}
{% endif %}
<nav class="grid-row bottom-gutter-1-2"> <nav class="grid-row bottom-gutter-1-2">
<div class="column-one-third"> <div class="column-one-third">
<a class="pill-separate-item" href="{{ url_for('.api_keys', service_id=current_service.id) }}">API keys</a> <a class="pill-separate-item" href="{{ url_for('.api_keys', service_id=current_service.id) }}">API keys</a>

View File

@@ -37,9 +37,9 @@
{% if item.key_type == 'normal' %} {% if item.key_type == 'normal' %}
<span class="visually-hidden">Normal</span> <span class="visually-hidden">Normal</span>
{% elif item.key_type == 'team' %} {% elif item.key_type == 'team' %}
Only sends to team members or whitelist Sends to anyone on your whitelist
{% elif item.key_type == 'test' %} {% elif item.key_type == 'test' %}
Simulates sending messages Pretends to send messages
{% endif %} {% endif %}
</span> </span>
</div> </div>

View File

@@ -2,6 +2,7 @@
{% from "components/page-footer.html" import page_footer %} {% from "components/page-footer.html" import page_footer %}
{% from "components/textbox.html" import textbox %} {% from "components/textbox.html" import textbox %}
{% from "components/radios.html" import radios %} {% from "components/radios.html" import radios %}
{% from "components/banner.html" import banner_wrapper %}
{% block page_title %} {% block page_title %}
Create an API key GOV.UK Notify Create an API key GOV.UK Notify
@@ -14,8 +15,8 @@
</h1> </h1>
<form method="post"> <form method="post">
{{ radios(form.key_type) }}
{{ textbox(form.key_name, label='Name for this key') }} {{ textbox(form.key_name, label='Name for this key') }}
{{ radios(form.key_type, disable=disabled_options, option_hints=option_hints) }}
{{ page_footer('Continue') }} {{ page_footer('Continue') }}
</form> </form>

View File

@@ -21,7 +21,6 @@ def test_should_show_api_page(
assert response.status_code == 200 assert response.status_code == 200
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
assert page.h1.string.strip() == 'API integration' assert page.h1.string.strip() == 'API integration'
assert 'Your service is in trial mode' in page.find('div', {'class': 'banner-warning'}).text
rows = page.find_all('details') rows = page.find_all('details')
assert len(rows) == 5 assert len(rows) == 5
for index, row in enumerate(rows): for index, row in enumerate(rows):
@@ -201,10 +200,7 @@ def test_cant_create_normal_api_key_in_trial_mode(
'key_type': 'normal' 'key_type': 'normal'
} }
) )
assert response.status_code == 200 assert response.status_code == 400
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
assert page.find('span', {'class': 'error-message'}).text.strip() == 'Not a valid choice'
mock_post.assert_not_called() mock_post.assert_not_called()