Merge pull request #2462 from alphagov/api-key-sort-order

Sort API keys alphabetically

tests/app/main/test_create_api_key_form.py

@@ -5,10 +5,12 @@ from app.main.forms import CreateKeyForm
 
 
 def test_return_validation_error_when_key_name_exists(client):
-    def _get_names():
-        return ['some key', 'another key']
+    _existing_keys = [
+        {'name': 'some key'},
+        {'name': 'another key'},
+    ]
 
-    form = CreateKeyForm(_get_names(),
+    form = CreateKeyForm(_existing_keys,
                          formdata=MultiDict([('key_name', 'Some key')]))
     form.key_type.choices = [('a', 'a'), ('b', 'b')]
     form.validate()

tests/app/main/views/test_api_integration.py

@@ -186,26 +186,21 @@ def test_should_show_empty_api_keys_page(
     assert response.status_code == 200
     assert 'You haven’t created any API keys yet' in response.get_data(as_text=True)
     assert 'Create an API key' in response.get_data(as_text=True)
-    mock_get_no_api_keys.assert_called_once_with(service_id=service_id)
+    mock_get_no_api_keys.assert_called_once_with(service_id)
 
 
 def test_should_show_api_keys_page(
-    logged_in_client,
-    api_user_active,
-    mock_login,
+    client_request,
     mock_get_api_keys,
-    mock_get_service,
-    mock_has_permissions,
-    fake_uuid,
 ):
-    response = logged_in_client.get(url_for('main.api_keys', service_id=fake_uuid))
+    page = client_request.get('main.api_keys', service_id=SERVICE_ONE_ID)
+    rows = [normalize_spaces(row.text) for row in page.select('main tr')]
 
-    assert response.status_code == 200
-    resp_data = response.get_data(as_text=True)
-    assert 'some key name' in resp_data
-    assert 'another key name' in resp_data
-    assert 'Revoked 1 January at 1:00am' in resp_data
-    mock_get_api_keys.assert_called_once_with(service_id=fake_uuid)
+    assert rows[0] == 'API keys Action'
+    assert rows[1] == 'another key name Revoked 1 January at 1:00am'
+    assert rows[2] == 'some key name Revoke'
+
+    mock_get_api_keys.assert_called_once_with(SERVICE_ONE_ID)
 
 
 @pytest.mark.parametrize('service_mock, expected_options', [
@@ -319,15 +314,21 @@ def test_should_show_confirm_revoke_api_key(
     )
     assert mock_get_api_keys.call_args_list == [
         call(
-            key_id=fake_uuid,
-            service_id='596364a0-858e-42c8-9062-a8fe822260eb',
-        ),
-        call(
-            service_id='596364a0-858e-42c8-9062-a8fe822260eb'
+            '596364a0-858e-42c8-9062-a8fe822260eb'
         ),
     ]
 
 
+def test_should_404_for_api_key_that_doesnt_exist(
+    client_request,
+    mock_get_api_keys,
+):
+    client_request.get(
+        'main.revoke_api_key', service_id=SERVICE_ONE_ID, key_id='key-doesn’t-exist',
+        _expected_status=404,
+    )
+
+
 def test_should_redirect_after_revoking_api_key(
     logged_in_client,
     api_user_active,
@@ -338,12 +339,12 @@ def test_should_redirect_after_revoking_api_key(
     mock_has_permissions,
     fake_uuid,
 ):
-    response = logged_in_client.post(url_for('main.revoke_api_key', service_id=fake_uuid, key_id=fake_uuid))
+    response = logged_in_client.post(url_for('main.revoke_api_key', service_id=SERVICE_ONE_ID, key_id=fake_uuid))
 
     assert response.status_code == 302
-    assert response.location == url_for('.api_keys', service_id=fake_uuid, _external=True)
-    mock_revoke_api_key.assert_called_once_with(service_id=fake_uuid, key_id=fake_uuid)
-    mock_get_api_keys.assert_called_once_with(service_id=fake_uuid, key_id=fake_uuid)
+    assert response.location == url_for('.api_keys', service_id=SERVICE_ONE_ID, _external=True)
+    mock_revoke_api_key.assert_called_once_with(service_id=SERVICE_ONE_ID, key_id=fake_uuid)
+    mock_get_api_keys.assert_called_once_with(SERVICE_ONE_ID,)
 
 
 @pytest.mark.parametrize('route', [
@@ -354,6 +355,7 @@ def test_should_redirect_after_revoking_api_key(
 def test_route_permissions(
     mocker,
     app_,
+    fake_uuid,
     api_user_active,
     service_one,
     mock_get_api_keys,
@@ -365,7 +367,7 @@ def test_route_permissions(
             app_,
             "GET",
             200,
-            url_for(route, service_id=service_one['id'], key_id=123),
+            url_for(route, service_id=service_one['id'], key_id=fake_uuid),
             ['manage_api_keys'],
             api_user_active,
             service_one)
@@ -379,6 +381,7 @@ def test_route_permissions(
 def test_route_invalid_permissions(
     mocker,
     app_,
+    fake_uuid,
     api_user_active,
     service_one,
     mock_get_api_keys,
@@ -390,7 +393,7 @@ def test_route_invalid_permissions(
             app_,
             "GET",
             403,
-            url_for(route, service_id=service_one['id'], key_id=123),
+            url_for(route, service_id=service_one['id'], key_id=fake_uuid),
             ['view_activity'],
             api_user_active,
             service_one)

tests/conftest.py

@@ -1639,10 +1639,12 @@ def mock_revoke_api_key(mocker):
 
 
 @pytest.fixture(scope='function')
-def mock_get_api_keys(mocker):
+def mock_get_api_keys(mocker, fake_uuid):
     def _get_keys(service_id, key_id=None):
-        keys = {'apiKeys': [api_key_json(service_id, 'some key name'),
-                            api_key_json(service_id, 'another key name', expiry_date=str(date.fromtimestamp(0)))]}
+        keys = {'apiKeys': [
+            api_key_json(id_=fake_uuid, name='some key name',),
+            api_key_json(id_='1234567', name='another key name', expiry_date=str(date.fromtimestamp(0)))
+        ]}
         return keys
 
     return mocker.patch('app.api_key_api_client.get_api_keys', side_effect=_get_keys)