diff --git a/app/main/views/organisations.py b/app/main/views/organisations.py index 058b543cf..edcc60f98 100644 --- a/app/main/views/organisations.py +++ b/app/main/views/organisations.py @@ -253,42 +253,28 @@ def invite_org_user(org_id): ) -@main.route("/organisations//users/", methods=['GET', 'POST']) +@main.route("/organisations//users/", methods=['GET']) @user_has_permissions() -def edit_user_org_permissions(org_id, user_id): +def edit_organisation_user(org_id, user_id): + # The only action that can be done to an org user is to remove them from the org. + # This endpoint is used to get the ID of the user to delete without passing it as a + # query string, but it uses the template for all org team members in order to avoid + # having a page containing a single link. return render_template( - 'views/organisations/organisation/users/user/index.html', - user=User.from_id(user_id) + 'views/organisations/organisation/users/index.html', + users=current_organisation.team_members, + show_search_box=(len(current_organisation.team_members) > 7), + form=SearchUsersForm(), + user_to_remove=User.from_id(user_id) ) -@main.route("/organisations//users//delete", methods=['GET', 'POST']) +@main.route("/organisations//users//delete", methods=['POST']) @user_has_permissions() def remove_user_from_organisation(org_id, user_id): - user = User.from_id(user_id) - if request.method == 'POST': - try: - organisations_client.remove_user_from_organisation(org_id, user_id) - except HTTPError as e: - msg = "You cannot remove the only user for a service" - if e.status_code == 400 and msg in e.message: - flash(msg, 'info') - return redirect(url_for( - '.manage_org_users', - org_id=org_id)) - else: - abort(500, e) + organisations_client.remove_user_from_organisation(org_id, user_id) - return redirect(url_for( - '.manage_org_users', - org_id=org_id - )) - - flash('Are you sure you want to remove {}?'.format(user.name), 'remove') - return render_template( - 'views/organisations/organisation/users/user/index.html', - user=user, - ) + return redirect(url_for('.show_accounts_or_dashboard')) @main.route("/organisations//cancel-invited-user/", methods=['GET']) diff --git a/app/navigation.py b/app/navigation.py index 7bed95211..8e2710fa2 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -341,7 +341,7 @@ class OrgNavigation(Navigation): }, 'team-members': { - 'edit_user_org_permissions', + 'edit_organisation_user', 'invite_org_user', 'manage_org_users', 'remove_user_from_organisation', diff --git a/app/templates/views/organisations/organisation/users/index.html b/app/templates/views/organisations/organisation/users/index.html index fd466eb34..56fa4fd82 100644 --- a/app/templates/views/organisations/organisation/users/index.html +++ b/app/templates/views/organisations/organisation/users/index.html @@ -10,6 +10,15 @@ {% block maincolumn_content %} + {% if user_to_remove %} + {{ banner( + 'Are you sure you want to remove {}?'.format(user_to_remove.name), + type='dangerous', + delete_button='Yes, remove', + action=url_for('.remove_user_from_organisation', org_id=current_org.id, user_id=user_to_remove.id) + ) }} + {% endif %} +

Team members

@@ -45,6 +54,8 @@
{% if user.status == 'pending' %} Cancel invitation + {% else %} + Remove {{ user.name }} {{ user.email_address }} {% endif %}
diff --git a/tests/app/main/views/organisations/test_organisations.py b/tests/app/main/views/organisations/test_organisations.py index 7cb8e40c8..da67bb313 100644 --- a/tests/app/main/views/organisations/test_organisations.py +++ b/tests/app/main/views/organisations/test_organisations.py @@ -768,6 +768,82 @@ def test_organisation_trial_mode_services_doesnt_work_if_not_platform_admin( ) +def test_manage_org_users_shows_correct_link_next_to_each_user( + client_request, + mock_get_organisation, + mock_get_users_for_organisation, + mock_get_invited_users_for_organisation, +): + page = client_request.get( + '.manage_org_users', + org_id=ORGANISATION_ID, + ) + + # No banner confirming a user to be deleted shown + assert not page.select_one('.banner-dangerous') + + users = page.find_all(class_='user-list-item') + + # The first user is an invited user, so has the link to cancel the invitation. + # The second two users are active users, so have the link to be removed from the org + assert normalize_spaces(users[0].text) == 'invited_user@test.gov.uk (invited) Cancel invitation' + assert normalize_spaces(users[1].text) == 'Test User 1 test@gov.uk Remove Test User 1 test@gov.uk' + assert normalize_spaces(users[2].text) == 'Test User 2 testt@gov.uk Remove Test User 2 testt@gov.uk' + + assert users[0].a['href'] == url_for( + '.cancel_invited_org_user', + org_id=ORGANISATION_ID, + invited_user_id='73616d70-6c65-4f6f-b267-5f696e766974' + ) + assert users[1].a['href'] == url_for('.edit_organisation_user', org_id=ORGANISATION_ID, user_id='1234') + assert users[2].a['href'] == url_for('.edit_organisation_user', org_id=ORGANISATION_ID, user_id='5678') + + +def test_edit_organisation_user_shows_the_delete_confirmation_banner( + client_request, + mock_get_organisation, + mock_get_invites_for_organisation, + mock_get_users_for_organisation, + active_user_with_permissions, +): + page = client_request.get( + '.edit_organisation_user', + org_id=ORGANISATION_ID, + user_id=active_user_with_permissions['id'] + ) + + assert normalize_spaces(page.h1) == 'Team members' + + banner = page.select_one('.banner-dangerous') + assert "Are you sure you want to remove Test User?" in normalize_spaces(banner.contents[0]) + assert banner.form.attrs['action'] == url_for( + 'main.remove_user_from_organisation', + org_id=ORGANISATION_ID, + user_id=active_user_with_permissions['id'] + ) + + +def test_remove_user_from_organisation_makes_api_request_to_remove_user( + client_request, + mocker, + mock_get_organisation, + fake_uuid, +): + mock_remove_user = mocker.patch('app.organisations_client.remove_user_from_organisation') + + client_request.post( + '.remove_user_from_organisation', + org_id=ORGANISATION_ID, + user_id=fake_uuid, + _expected_redirect=url_for( + 'main.show_accounts_or_dashboard', + _external=True, + ), + ) + + mock_remove_user.assert_called_with(ORGANISATION_ID, fake_uuid) + + def test_cancel_invited_org_user_cancels_user_invitations( client_request, mock_get_invites_for_organisation, diff --git a/tests/app/test_navigation.py b/tests/app/test_navigation.py index 5052413dd..54dd81dc3 100644 --- a/tests/app/test_navigation.py +++ b/tests/app/test_navigation.py @@ -101,6 +101,7 @@ EXCLUDED_ENDPOINTS = tuple(map(Navigation.get_endpoint_with_blueprint, { 'edit_organisation_name', 'edit_organisation_notes', 'edit_organisation_type', + 'edit_organisation_user', 'edit_provider', 'edit_service_billing_details', 'edit_service_notes', @@ -109,7 +110,6 @@ EXCLUDED_ENDPOINTS = tuple(map(Navigation.get_endpoint_with_blueprint, { 'edit_template_postage', 'edit_user_email', 'edit_user_mobile_number', - 'edit_user_org_permissions', 'edit_user_permissions', 'email_branding', 'email_not_received',