From 05a493f1e867e77a1cfad755654824a2373fa16c Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 22 Aug 2016 14:39:57 +0100 Subject: [PATCH] =?UTF-8?q?Remove=20=E2=80=98suspend=20API=20keys=E2=80=99?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This was an early reckon feature. There were a few of problems with it: - it worked on the service, not just on the API keys as described - it was back to front, ‘suspending’ a service set `active` to `True`, reactivating it set `active` to `False` - no part of the API actually respected the `active` flag on a service The same intent can be acheived by either: - revoking an API key - having a platform admin put your service back into trial mode So this commit removes the link and the code behind it. --- app/main/views/service_settings.py | 35 ------- app/templates/views/service-settings.html | 85 +++++++++-------- .../views/service-settings/status.html | 38 -------- tests/app/main/views/test_service_settings.py | 95 ------------------- 4 files changed, 46 insertions(+), 207 deletions(-) delete mode 100644 app/templates/views/service-settings/status.html diff --git a/app/main/views/service_settings.py b/app/main/views/service_settings.py index 746c4de3f..301aafd61 100644 --- a/app/main/views/service_settings.py +++ b/app/main/views/service_settings.py @@ -157,41 +157,6 @@ def service_switch_research_mode(service_id): return redirect(url_for('.service_settings', service_id=service_id)) -@main.route("/services//service-settings/status", methods=['GET', 'POST']) -@login_required -@user_has_permissions('manage_settings', admin_override=True) -def service_status_change(service_id): - if request.method == 'GET': - return render_template( - 'views/service-settings/status.html' - ) - elif request.method == 'POST': - return redirect(url_for('.service_status_change_confirm', service_id=service_id)) - - -@main.route("/services//service-settings/status/confirm", methods=['GET', 'POST']) -@login_required -@user_has_permissions('manage_settings', admin_override=True) -def service_status_change_confirm(service_id): - # Validate password for form - def _check_password(pwd): - return user_api_client.verify_password(current_user.id, pwd) - - form = ConfirmPasswordForm(_check_password) - - if form.validate_on_submit(): - service_api_client.update_service( - current_service['id'], - active=True - ) - return redirect(url_for('.service_settings', service_id=service_id)) - return render_template( - 'views/service-settings/confirm.html', - heading='Turn off all outgoing notifications', - destructive=True, - form=form) - - @main.route("/services//service-settings/delete", methods=['GET', 'POST']) @login_required @user_has_permissions('manage_settings', admin_override=True) diff --git a/app/templates/views/service-settings.html b/app/templates/views/service-settings.html index 14c5d28fd..07b0b243e 100644 --- a/app/templates/views/service-settings.html +++ b/app/templates/views/service-settings.html @@ -1,6 +1,6 @@ {% extends "withnav_template.html" %} {% from "components/browse-list.html" import browse_list %} -{% from "components/table.html" import mapping_table, row, text_field, edit_field %} +{% from "components/table.html" import mapping_table, row, text_field, edit_field, field %} {% block page_title %} Settings – GOV.UK Notify @@ -10,46 +10,53 @@

Settings

- {% call mapping_table( - caption='Settings', - field_headings=['Label', 'Value', 'Action'], - field_headings_visible=False, - caption_visible=False - ) %} - {% call row() %} - {{ text_field('Service name' )}} - {{ text_field(current_service.name) }} - {{ edit_field('Change', url_for('.service_name_change', service_id=current_service.id)) }} +
+ + {% call mapping_table( + caption='Settings', + field_headings=['Label', 'Value', 'Action'], + field_headings_visible=False, + caption_visible=False + ) %} + {% call row() %} + {{ text_field('Service name' )}} + {{ text_field(current_service.name) }} + {{ edit_field('Change', url_for('.service_name_change', service_id=current_service.id)) }} + {% endcall %} + {% call row() %} + {{ text_field('Email reply to address')}} + {{ text_field( + current_service.reply_to_email_address, + status='' if current_service.reply_to_email_address else 'default' + ) }} + {{ edit_field('Change', url_for('.service_set_reply_to_email', service_id=current_service.id)) }} + {% endcall %} + {% call row() %} + {{ text_field('Text message sender')}} + {{ text_field(current_service.sms_sender or '40604') }} + {{ edit_field('Change', url_for('.service_set_sms_sender', service_id=current_service.id)) }} + {% endcall %} {% endcall %} - {% call row() %} - {{ text_field('Email reply to address')}} - {{ text_field( - current_service.reply_to_email_address, - status='' if current_service.reply_to_email_address else 'default' - ) }} - {{ edit_field('Change', url_for('.service_set_reply_to_email', service_id=current_service.id)) }} + + {% call mapping_table( + caption='Restrictions', + field_headings=['Label', 'Value', 'Action'], + field_headings_visible=False, + caption_visible=True + ) %} + {% call row() %} + {% if current_service.restricted %} + {{ text_field('Trial mode') }} + {{ text_field('On') }} + {{ edit_field('Request to go live', url_for('.service_request_to_go_live', service_id=current_service.id)) }} + {% else %} + {{ text_field('Trial mode') }} + {{ text_field('Off')}} + {{ edit_field() }} + {% endif %} + {% endcall %} {% endcall %} - {% call row() %} - {{ text_field('Text message sender')}} - {{ text_field(current_service.sms_sender or '40604') }} - {{ edit_field('Change', url_for('.service_set_sms_sender', service_id=current_service.id)) }} - {% endcall %} - {% call row() %} - {{ text_field('Mode')}} - {% if current_service.restricted %} - {{ text_field('Trial') }} - {{ edit_field('Go live', url_for('.service_request_to_go_live', service_id=current_service.id)) }} - {% else %} - {{ text_field('Live') }} - {{ edit_field() }} - {% endif %} - {% endcall %} - {% call row() %} - {{ text_field('Active')}} - {{ text_field(current_service.active) }} - {{ edit_field('Suspend', url_for('.service_status_change', service_id=current_service.id)) }} - {% endcall %} - {% endcall %} +
{% if current_user.has_permissions([], admin_override=True) %} diff --git a/app/templates/views/service-settings/status.html b/app/templates/views/service-settings/status.html deleted file mode 100644 index d2648a5d7..000000000 --- a/app/templates/views/service-settings/status.html +++ /dev/null @@ -1,38 +0,0 @@ -{% extends "withnav_template.html" %} -{% from "components/page-footer.html" import page_footer %} - -{% block page_title %} - Temporrily suspend API keys – GOV.UK Notify -{% endblock %} - -{% block maincolumn_content %} - -

Temporarily suspend API keys

- -
-
- -

- You’ll still be able to send notifications to yourself by uploading a - CSV file. -

- -

- You can start sending notifications again when you’re ready. -

- -
- {{ page_footer( - 'Suspend API keys', - destructive=True, - back_link=url_for('.service_settings', service_id=current_service.id), - back_link_text='Back to settings' - ) }} -
- -
-
- - - -{% endblock %} diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index ee81e2d24..6f9585f62 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -321,95 +321,6 @@ def test_log_error_on_request_to_go_live( ) -def test_should_show_status_page(app_, - api_user_active, - mock_get_service, - mock_get_user, - mock_get_user_by_email, - mock_login, - mock_has_permissions, - fake_uuid): - with app_.test_request_context(): - with app_.test_client() as client: - client.login(api_user_active) - service_id = fake_uuid - response = client.get(url_for( - 'main.service_status_change', service_id=service_id)) - - assert response.status_code == 200 - resp_data = response.get_data(as_text=True) - assert 'Suspend API keys' in resp_data - assert mock_get_service.called - - -def test_should_show_redirect_after_status_change(app_, - api_user_active, - mock_get_service, - mock_get_user, - mock_get_user_by_email, - mock_login, - mock_has_permissions, - fake_uuid): - with app_.test_request_context(): - with app_.test_client() as client: - client.login(api_user_active) - service_id = fake_uuid - response = client.post(url_for( - 'main.service_status_change', service_id=service_id)) - - assert response.status_code == 302 - redirect_url = url_for( - 'main.service_status_change_confirm', service_id=service_id, _external=True) - assert redirect_url == response.location - assert mock_get_service.called - - -def test_should_show_status_confirmation(app_, - api_user_active, - mock_get_service, - mock_get_user, - mock_get_user_by_email, - mock_login, - mock_has_permissions, - fake_uuid): - with app_.test_request_context(): - with app_.test_client() as client: - client.login(api_user_active) - service_id = fake_uuid - response = client.get(url_for( - 'main.service_status_change_confirm', service_id=service_id)) - - assert response.status_code == 200 - resp_data = response.get_data(as_text=True) - assert 'Turn off all outgoing notifications' in resp_data - assert mock_get_service.called - - -def test_should_redirect_after_status_confirmation(app_, - api_user_active, - mock_get_service, - mock_update_service, - mock_get_user, - mock_get_user_by_email, - mock_login, - mock_verify_password, - mock_has_permissions, - fake_uuid): - with app_.test_request_context(): - with app_.test_client() as client: - client.login(api_user_active) - service_id = fake_uuid - response = client.post(url_for( - 'main.service_status_change_confirm', service_id=service_id)) - - assert response.status_code == 302 - settings_url = url_for( - 'main.service_settings', service_id=service_id, _external=True) - assert settings_url == response.location - assert mock_get_service.called - assert mock_update_service.called - - def test_should_show_delete_page(app_, api_user_active, mock_login, @@ -502,8 +413,6 @@ def test_route_permissions(mocker, app_, api_user_active, service_one): 'main.service_name_change', 'main.service_name_change_confirm', 'main.service_request_to_go_live', - 'main.service_status_change', - 'main.service_status_change_confirm', 'main.service_delete', 'main.service_delete_confirm'] with app_.test_request_context(): @@ -527,8 +436,6 @@ def test_route_invalid_permissions(mocker, app_, api_user_active, service_one): 'main.service_request_to_go_live', 'main.service_switch_live', 'main.service_switch_research_mode', - 'main.service_status_change', - 'main.service_status_change_confirm', 'main.service_delete', 'main.service_delete_confirm'] with app_.test_request_context(): @@ -550,8 +457,6 @@ def test_route_for_platform_admin(mocker, app_, platform_admin_user, service_one 'main.service_name_change', 'main.service_name_change_confirm', 'main.service_request_to_go_live', - 'main.service_status_change', - 'main.service_status_change_confirm', 'main.service_delete', 'main.service_delete_confirm' ]