From ddef996f34a1067bdcdc8d2d769f96535dc9047c Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 29 Jun 2016 15:03:28 +0100 Subject: [PATCH 1/8] =?UTF-8?q?Rename=20=E2=80=98API=20keys=E2=80=99=20pag?= =?UTF-8?q?e=20to=20=E2=80=98API=20integration=E2=80=99?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since this page is more than just your API keys, it should be named something else. Hopefully the word ‘integration’ will give non-techical users a clue that it’s possible to use Notify without doing the CSV dance. --- app/assets/stylesheets/components/table.scss | 7 ++++++- app/templates/main_nav.html | 2 +- app/templates/views/api-keys.html | 6 +++--- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/app/assets/stylesheets/components/table.scss b/app/assets/stylesheets/components/table.scss index 79f5eae88..4342891e0 100644 --- a/app/assets/stylesheets/components/table.scss +++ b/app/assets/stylesheets/components/table.scss @@ -111,7 +111,12 @@ } -.table-field-heading { +.table-field-headings, +.table-field-headings-visible { + + th { + @include bold-19; + } .dashboard-table &-first { width: 52.5%; diff --git a/app/templates/main_nav.html b/app/templates/main_nav.html index 1fdfb461c..f812e1977 100644 --- a/app/templates/main_nav.html +++ b/app/templates/main_nav.html @@ -58,7 +58,7 @@
  • Team members
    • {% endif %} {% if current_user.has_permissions(['manage_api_keys']) %} -
  • API keys
    • +
  • API integration
    • {% endif %} diff --git a/app/templates/views/api-keys.html b/app/templates/views/api-keys.html index e44d5a718..e231928c6 100644 --- a/app/templates/views/api-keys.html +++ b/app/templates/views/api-keys.html @@ -11,7 +11,7 @@

    - API keys + API integration

    @@ -23,8 +23,8 @@ keys, empty_message="You haven’t created any API keys yet", caption="API keys", - caption_visible=False, - field_headings=['Key name', hidden_field_heading('Action')] + caption_visible=false, + field_headings=['API keys', hidden_field_heading('Action')], ) %} {% call field() %} {{ item.name }} From 49d8e68589c4cd275716741c2cbec7c5c5c7c643 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 29 Jun 2016 17:09:55 +0100 Subject: [PATCH 2/8] =?UTF-8?q?Rename=20=E2=80=98Add=20a=20new=20API=20key?= =?UTF-8?q?=E2=80=99=20page?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ‘Create’ matches the text of the button on the previous page. --- app/templates/views/api-keys/create.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/templates/views/api-keys/create.html b/app/templates/views/api-keys/create.html index 1be4bf326..6b4a0d97d 100644 --- a/app/templates/views/api-keys/create.html +++ b/app/templates/views/api-keys/create.html @@ -9,7 +9,7 @@ {% block maincolumn_content %}

    - Add a new API key + Create an API key

    From 7fcd56dc022d76c8b55888850823532f87ee99a5 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 29 Jun 2016 17:10:49 +0100 Subject: [PATCH 3/8] Add radio buttons for choosing the API key type Best-guess wording for what the labels and question should be. Adds a macro for rendering radio buttons from a WTForms field. --- app/main/forms.py | 15 +++++++++++++- app/main/views/api_keys.py | 2 +- app/templates/components/radios.html | 23 ++++++++++++++++++++++ app/templates/views/api-keys/create.html | 4 +++- tests/app/main/test_create_api_key_form.py | 20 ++++++++++++++++++- tests/app/main/views/test_api_keys.py | 9 +++++++-- 6 files changed, 67 insertions(+), 6 deletions(-) create mode 100644 app/templates/components/radios.html diff --git a/app/main/forms.py b/app/main/forms.py index 31ff09737..ec78e6cae 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -12,7 +12,8 @@ from wtforms import ( FileField, BooleanField, HiddenField, - IntegerField + IntegerField, + RadioField ) from wtforms.fields.html5 import EmailField, TelField from wtforms.validators import (DataRequired, Email, Length, Regexp) @@ -291,6 +292,18 @@ class CreateKeyForm(Form): self.existing_key_names = [x.lower() for x in existing_key_names] super(CreateKeyForm, self).__init__(*args, **kwargs) + key_type = RadioField( + 'What should Notify do when you use this key?', + choices=[ + ('normal', 'Send messages to anyone'), + ('test', 'Simulate sending messages to anyone'), + ('team', 'Only send messages to members of your team') + ], + validators=[ + DataRequired() + ] + ) + key_name = StringField(u'Description of key', validators=[ DataRequired(message='You need to give the key a name') ]) diff --git a/app/main/views/api_keys.py b/app/main/views/api_keys.py index aad602179..cd35e8964 100644 --- a/app/main/views/api_keys.py +++ b/app/main/views/api_keys.py @@ -30,7 +30,7 @@ def create_api_key(service_id): key_name=form.key_name.data) return render_template( 'views/api-keys/create.html', - key_name=form.key_name + form=form ) diff --git a/app/templates/components/radios.html b/app/templates/components/radios.html new file mode 100644 index 000000000..88beb1220 --- /dev/null +++ b/app/templates/components/radios.html @@ -0,0 +1,23 @@ +{% macro radios( + field, + hint=None +) %} +
    +
    + + {{ field.label }} + {% if field.errors %} + + {{ field.errors[0] }} + + {% endif %} + + {% for option in field %} + + {% endfor %} +
    +
    +{% endmacro %} diff --git a/app/templates/views/api-keys/create.html b/app/templates/views/api-keys/create.html index 6b4a0d97d..474475618 100644 --- a/app/templates/views/api-keys/create.html +++ b/app/templates/views/api-keys/create.html @@ -1,6 +1,7 @@ {% extends "withnav_template.html" %} {% from "components/page-footer.html" import page_footer %} {% from "components/textbox.html" import textbox %} +{% from "components/radios.html" import radios %} {% block page_title %} Add a new API key – GOV.UK Notify @@ -13,7 +14,8 @@ - {{ textbox(key_name, hint='eg CRM application') }} + {{ radios(form.key_type) }} + {{ textbox(form.key_name, label='Name for this key') }} {{ page_footer('Continue') }} diff --git a/tests/app/main/test_create_api_key_form.py b/tests/app/main/test_create_api_key_form.py index c13fe9fe3..498edc504 100644 --- a/tests/app/main/test_create_api_key_form.py +++ b/tests/app/main/test_create_api_key_form.py @@ -1,3 +1,5 @@ +import pytest + from werkzeug.datastructures import MultiDict from app.main.forms import CreateKeyForm @@ -11,4 +13,20 @@ def test_return_validation_error_when_key_name_exists(app_): form = CreateKeyForm(_get_names(), formdata=MultiDict([('key_name', 'Some key')])) form.validate() - assert {'key_name': ['A key with this name already exists']} == form.errors + assert form.errors['key_name'] == ['A key with this name already exists'] + + +@pytest.mark.parametrize( + 'key_type, expected_error', [ + ('', 'This field is required.'), + ('invalid', 'Not a valid choice') + ] +) +def test_return_validation_error_when_key_type_not_chosen(app_, key_type, expected_error): + + with app_.test_request_context(): + form = CreateKeyForm( + [], + formdata=MultiDict([('key_name', 'Some key'), ('key_type', key_type)])) + form.validate() + assert form.errors['key_type'] == [expected_error] diff --git a/tests/app/main/views/test_api_keys.py b/tests/app/main/views/test_api_keys.py index a1807a380..d7c51fcfc 100644 --- a/tests/app/main/views/test_api_keys.py +++ b/tests/app/main/views/test_api_keys.py @@ -72,8 +72,13 @@ def test_should_create_api_key_with_type_normal(app_, with app_.test_request_context(), app_.test_client() as client: client.login(api_user_active) - response = client.post(url_for('main.create_api_key', service_id=service_id), - data={'key_name': 'some default key name'}) + response = client.post( + url_for('main.create_api_key', service_id=service_id), + data={ + 'key_name': 'some default key name', + 'key_type': 'normal' + } + ) assert response.status_code == 200 assert 'some default key name' in response.get_data(as_text=True) From 01df039aab35d070e9c6ba4da93a527592c2d019 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Wed, 29 Jun 2016 16:04:00 +0100 Subject: [PATCH 4/8] Indicate the type of key in the table of keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When looking at your keys, it’s useful to know which are `test`/`team`. Default (`normal`) keys don’t get a label. --- app/templates/views/api-keys.html | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/app/templates/views/api-keys.html b/app/templates/views/api-keys.html index e231928c6..47bbaa1b6 100644 --- a/app/templates/views/api-keys.html +++ b/app/templates/views/api-keys.html @@ -24,11 +24,24 @@ empty_message="You haven’t created any API keys yet", caption="API keys", caption_visible=false, - field_headings=['API keys', hidden_field_heading('Action')], + field_headings=[ + 'API keys', + hidden_field_heading('Key type'), + hidden_field_heading('Action') + ], ) %} {% call field() %} {{ item.name }} {% endcall %} + {% call field(status='default') %} + {% if item.key_type == 'normal' %} + + {% elif item.key_type == 'team' %} + Only sends to team members + {% elif item.key_type == 'test' %} + Simulates sending messages + {% endif %} + {% endcall %} {% if item.expiry_date %} {% call field(align='right', status='default') %} Revoked {{ item.expiry_date|format_datetime }} From c0c516ba7a6953fdf982a2a698aa42a1bbb60e5d Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 1 Jul 2016 16:46:14 +0100 Subject: [PATCH 5/8] Make documentation its own section of the page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This visually chunks the page up into three parts: - keys - service ID - documentation Also fixes the documentation link (because it’s not service-specific). --- app/templates/views/api-keys.html | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/templates/views/api-keys.html b/app/templates/views/api-keys.html index 47bbaa1b6..6f1ce19ae 100644 --- a/app/templates/views/api-keys.html +++ b/app/templates/views/api-keys.html @@ -57,10 +57,11 @@ {{ api_key(current_service.id, "Service ID", thing="service ID") }}
    +

    Documentation

    - API usage is described in the - - API documentation. + See the + + API documentation for clients and specfications.

    {% endblock %} From 630b5df552607e45aeadf408820e64438451ccef Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Wed, 6 Jul 2016 15:10:36 +0100 Subject: [PATCH 6/8] Save api_key.key_type from radio buttons --- app/main/forms.py | 7 ++++--- app/main/views/api_keys.py | 6 +++++- app/notify_client/api_key_api_client.py | 5 +++-- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index ec78e6cae..1c728a2b5 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -19,6 +19,7 @@ from wtforms.fields.html5 import EmailField, TelField from wtforms.validators import (DataRequired, Email, Length, Regexp) from app.main.validators import (Blacklist, CsvFileValidator, ValidEmailDomainRegex, NoCommasInPlaceHolders) +from app.notify_client.api_key_api_client import KEY_TYPE_NORMAL, KEY_TYPE_TEST, KEY_TYPE_TEAM def email_address(label='Email address'): @@ -295,9 +296,9 @@ class CreateKeyForm(Form): key_type = RadioField( 'What should Notify do when you use this key?', choices=[ - ('normal', 'Send messages to anyone'), - ('test', 'Simulate sending messages to anyone'), - ('team', 'Only send messages to members of your team') + (KEY_TYPE_NORMAL, 'Send messages to anyone'), + (KEY_TYPE_TEST, 'Simulate sending messages to anyone'), + (KEY_TYPE_TEAM, 'Only send messages to members of your team') ], validators=[ DataRequired() diff --git a/app/main/views/api_keys.py b/app/main/views/api_keys.py index cd35e8964..9dc1465f0 100644 --- a/app/main/views/api_keys.py +++ b/app/main/views/api_keys.py @@ -25,7 +25,11 @@ def create_api_key(service_id): ] form = CreateKeyForm(key_names) if form.validate_on_submit(): - secret = api_key_api_client.create_api_key(service_id=service_id, key_name=form.key_name.data) + secret = api_key_api_client.create_api_key( + service_id=service_id, + key_name=form.key_name.data, + key_type=form.key_type.data + ) return render_template('views/api-keys/show.html', secret=secret, key_name=form.key_name.data) return render_template( diff --git a/app/notify_client/api_key_api_client.py b/app/notify_client/api_key_api_client.py index 1e67e34df..797678541 100644 --- a/app/notify_client/api_key_api_client.py +++ b/app/notify_client/api_key_api_client.py @@ -5,6 +5,7 @@ from app.notify_client import _attach_current_user # must match key types in notifications-api/app/models.py KEY_TYPE_NORMAL = 'normal' KEY_TYPE_TEAM = 'team' +KEY_TYPE_TEST = 'test' class ApiKeyApiClient(BaseAPIClient): @@ -24,10 +25,10 @@ class ApiKeyApiClient(BaseAPIClient): else: return self.get(url='/service/{}/api-keys'.format(service_id)) - def create_api_key(self, service_id, key_name): + def create_api_key(self, service_id, key_name, key_type): data = { 'name': key_name, - 'key_type': KEY_TYPE_NORMAL + 'key_type': key_type } _attach_current_user(data) key = self.post(url='/service/{}/api-key'.format(service_id), data=data) From 8b9704df0e9712a75313ee550bd00df4d67a8f9b Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 7 Jul 2016 09:00:12 +0100 Subject: [PATCH 7/8] Restore API keys table to 2 columns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Because it get ugly when there’s 3 columns and everything is wrapping. --- app/templates/views/api-keys.html | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/app/templates/views/api-keys.html b/app/templates/views/api-keys.html index 6f1ce19ae..c28089159 100644 --- a/app/templates/views/api-keys.html +++ b/app/templates/views/api-keys.html @@ -26,24 +26,25 @@ caption_visible=false, field_headings=[ 'API keys', - hidden_field_heading('Key type'), hidden_field_heading('Action') ], ) %} {% call field() %} - {{ item.name }} - {% endcall %} - {% call field(status='default') %} - {% if item.key_type == 'normal' %} - - {% elif item.key_type == 'team' %} - Only sends to team members - {% elif item.key_type == 'test' %} - Simulates sending messages - {% endif %} +
    + {{ item.name }} + + {% if item.key_type == 'normal' %} + + {% elif item.key_type == 'team' %} + Only sends to team members + {% elif item.key_type == 'test' %} + Simulates sending messages + {% endif %} + +
    {% endcall %} {% if item.expiry_date %} - {% call field(align='right', status='default') %} + {% call field(align='right') %} Revoked {{ item.expiry_date|format_datetime }} {% endcall %} {% else %} From 60f79afb6b8bc4583f3b61b6ff0dd64436bc679e Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 7 Jul 2016 09:06:07 +0100 Subject: [PATCH 8/8] Make the date take up less space --- app/templates/views/api-keys.html | 2 +- tests/app/main/views/test_api_keys.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/templates/views/api-keys.html b/app/templates/views/api-keys.html index c28089159..34dac7304 100644 --- a/app/templates/views/api-keys.html +++ b/app/templates/views/api-keys.html @@ -45,7 +45,7 @@ {% endcall %} {% if item.expiry_date %} {% call field(align='right') %} - Revoked {{ item.expiry_date|format_datetime }} + Revoked {{ item.expiry_date|format_datetime_short }} {% endcall %} {% else %} {% call field(align='right', status='error') %} diff --git a/tests/app/main/views/test_api_keys.py b/tests/app/main/views/test_api_keys.py index d7c51fcfc..1b2561566 100644 --- a/tests/app/main/views/test_api_keys.py +++ b/tests/app/main/views/test_api_keys.py @@ -39,7 +39,7 @@ def test_should_show_api_keys_page(app_, resp_data = response.get_data(as_text=True) assert 'some key name' in resp_data assert 'another key name' in resp_data - assert 'Revoked Thursday 01 January 1970 at 01:00' in resp_data + assert 'Revoked 1 January at 01:00' in resp_data mock_get_api_keys.assert_called_once_with(service_id=fake_uuid)