From f872294235f69296892cd2b8c334021d5bdd3757 Mon Sep 17 00:00:00 2001
From: Leo Hemsted <leo.hemsted@digital.cabinet-office.gov.uk>
Date: Tue, 26 Mar 2019 15:51:44 +0000
Subject: [PATCH] remove the remove-user get method

remove `confirm` from `confirm_remove_user_from_service` as there's
only one action now that the initial confirmation prompt takes place
on the edit permissions page
---
 app/main/views/manage_users.py            | 14 +------
 app/navigation.py                         |  4 --
 tests/app/main/views/test_manage_users.py | 50 +++++++++++------------
 3 files changed, 26 insertions(+), 42 deletions(-)

diff --git a/app/main/views/manage_users.py b/app/main/views/manage_users.py
index a0479d0b0..1a45f3aed 100644
--- a/app/main/views/manage_users.py
+++ b/app/main/views/manage_users.py
@@ -128,22 +128,10 @@ def edit_user_permissions(service_id, user_id):
     )
 
 
-@main.route("/services/<service_id>/users/<user_id>/delete", methods=['GET'])
-@login_required
-@user_has_permissions('manage_service')
-def remove_user_from_service(service_id, user_id):
-    return redirect(url_for(
-        '.edit_user_permissions',
-        service_id=service_id,
-        user_id=user_id,
-        delete='yes'
-    ))
-
-
 @main.route("/services/<service_id>/users/<user_id>/delete", methods=['POST'])
 @login_required
 @user_has_permissions('manage_service')
-def confirm_remove_user_from_service(service_id, user_id):
+def remove_user_from_service(service_id, user_id):
     try:
         service_api_client.remove_user_from_service(service_id, user_id)
     except HTTPError as e:
diff --git a/app/navigation.py b/app/navigation.py
index ef8984176..33d7d07ad 100644
--- a/app/navigation.py
+++ b/app/navigation.py
@@ -212,7 +212,6 @@ class HeaderNavigation(Navigation):
         'register_from_org_invite',
         'registration_continue',
         'remove_user_from_organisation',
-        'confirm_remove_user_from_service',
         'remove_user_from_service',
         'request_letter_branding',
         'request_to_go_live',
@@ -342,7 +341,6 @@ class MainNavigation(Navigation):
         'team-members': {
             'confirm_edit_user_email',
             'confirm_edit_user_mobile_number',
-            'confirm_remove_user_from_service',
             'edit_user_email',
             'edit_user_mobile_number',
             'edit_user_permissions',
@@ -619,7 +617,6 @@ class CaseworkNavigation(Navigation):
         'confirm_edit_user_email',
         'confirm_edit_user_mobile_number',
         'confirm_redact_template',
-        'confirm_remove_user_from_service',
         'conversation',
         'conversation_reply',
         'conversation_reply_with_template',
@@ -955,7 +952,6 @@ class OrgNavigation(Navigation):
         'register_from_invite',
         'register_from_org_invite',
         'registration_continue',
-        'confirm_remove_user_from_service',
         'remove_user_from_service',
         'request_letter_branding',
         'request_to_go_live',
diff --git a/tests/app/main/views/test_manage_users.py b/tests/app/main/views/test_manage_users.py
index d5eef75a1..7980362fd 100644
--- a/tests/app/main/views/test_manage_users.py
+++ b/tests/app/main/views/test_manage_users.py
@@ -879,37 +879,14 @@ def test_no_permission_manage_users_page(
     assert "Team members" not in resp_text
 
 
-def test_remove_user_from_service_redirects(
-    client_request,
-    active_user_with_permissions,
-    service_one,
-    mock_get_users_by_service,
-    mock_get_template_folders,
-    mocker,
-):
-    page = client_request.get(
-        'main.remove_user_from_service',
-        service_id=service_one['id'],
-        user_id=active_user_with_permissions.id,
-        _follow_redirects=True
-    )
-    banner = page.find('div', class_='banner-dangerous')
-    assert banner.contents[0].strip() == "Are you sure you want to remove Test User?"
-    assert banner.form.attrs['action'] == url_for(
-        'main.confirm_remove_user_from_service',
-        service_id=service_one['id'],
-        user_id=active_user_with_permissions.id
-    )
-
-
-def test_confirm_remove_user_from_service(
+def test_remove_user_from_service(
     client_request,
     active_user_with_permissions,
     service_one,
     mock_remove_user_from_service,
 ):
     client_request.post(
-        'main.confirm_remove_user_from_service',
+        'main.remove_user_from_service',
         service_id=service_one['id'],
         user_id=active_user_with_permissions.id,
         _expected_redirect=url_for('main.manage_users', service_id=service_one['id'], _external=True)
@@ -1139,6 +1116,29 @@ def test_edit_user_permissions_page_displays_redacted_mobile_number_and_change_l
     )
 
 
+def test_edit_user_permissions_with_delete_query_shows_banner(
+    client_request,
+    active_user_with_permissions,
+    mock_get_users_by_service,
+    mock_get_template_folders,
+    service_one
+):
+    page = client_request.get(
+        'main.edit_user_permissions',
+        service_id=service_one['id'],
+        user_id=active_user_with_permissions.id,
+        delete=1
+    )
+
+    banner = page.find('div', class_='banner-dangerous')
+    assert banner.contents[0].strip() == "Are you sure you want to remove Test User?"
+    assert banner.form.attrs['action'] == url_for(
+        'main.remove_user_from_service',
+        service_id=service_one['id'],
+        user_id=active_user_with_permissions.id
+    )
+
+
 def test_edit_user_mobile_number_page(
     client_request,
     active_user_with_permissions,