diff --git a/app/event_handlers.py b/app/event_handlers.py index 6a6124513..166a694b9 100644 --- a/app/event_handlers.py +++ b/app/event_handlers.py @@ -4,12 +4,12 @@ from app import events_api_client def on_user_logged_in(_sender, user): - _send_event(event_type='sucessful_login', user_id=user.id) + _send_event('sucessful_login', user_id=user.id) def create_email_change_event(user_id, updated_by_id, original_email_address, new_email_address): _send_event( - event_type='update_user_email', + 'update_user_email', user_id=user_id, updated_by_id=updated_by_id, original_email_address=original_email_address, @@ -18,30 +18,25 @@ def create_email_change_event(user_id, updated_by_id, original_email_address, ne def create_mobile_number_change_event(user_id, updated_by_id, original_mobile_number, new_mobile_number): _send_event( - event_type='update_user_mobile_number', + 'update_user_mobile_number', user_id=user_id, updated_by_id=updated_by_id, original_mobile_number=original_mobile_number, new_mobile_number=new_mobile_number) -def _send_event(**kwargs): - if not kwargs.get('event_type'): - return +def create_archive_user_event(user_id, archived_by_id): + _send_event( + 'archive_user', + user_id=user_id, + archived_by_id=archived_by_id) + +def _send_event(event_type, **kwargs): event_data = _construct_event_data(request) - event_fields = ('user_id', - 'updated_by_id', - 'original_email_address', - 'new_email_address', - 'original_mobile_number', - 'new_mobile_number') + event_data.update(kwargs) - for field in event_fields: - if kwargs.get(field): - event_data[field] = kwargs[field] - - events_api_client.create_event(kwargs['event_type'], event_data) + events_api_client.create_event(event_type, event_data) def _construct_event_data(request): diff --git a/app/main/views/find_users.py b/app/main/views/find_users.py index 2947b0a41..3d04eb42e 100644 --- a/app/main/views/find_users.py +++ b/app/main/views/find_users.py @@ -1,7 +1,8 @@ -from flask import render_template, request -from flask_login import login_required +from flask import flash, redirect, render_template, request, url_for +from flask_login import current_user, login_required from app import user_api_client +from app.event_handlers import create_archive_user_event from app.main import main from app.main.forms import SearchUsersByEmailForm from app.models.user import User @@ -37,3 +38,17 @@ def user_information(user_id): user=user, services=services, ) + + +@main.route("/users//archive", methods=['GET', 'POST']) +@login_required +@user_is_platform_admin +def archive_user(user_id): + if request.method == 'POST': + user_api_client.archive_user(user_id) + create_archive_user_event(str(user_id), current_user.id) + + return redirect(url_for('.user_information', user_id=user_id)) + else: + flash('There\'s no way to reverse this! Are you sure you want to archive this user?', 'delete') + return user_information(user_id) diff --git a/app/navigation.py b/app/navigation.py index 623d7c7e7..82d73b849 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -80,6 +80,7 @@ class HeaderNavigation(Navigation): }, 'platform-admin': { 'add_organisation', + 'archive_user', 'clear_cache', 'create_email_branding', 'create_letter_branding', @@ -427,6 +428,7 @@ class MainNavigation(Navigation): 'add_service', 'agreement', 'archive_service', + 'archive_user', 'bat_phone', 'callbacks', 'cancel_invited_org_user', @@ -619,6 +621,7 @@ class CaseworkNavigation(Navigation): 'api_integration', 'api_keys', 'archive_service', + 'archive_user', 'bat_phone', 'branding_request', 'callbacks', @@ -902,6 +905,7 @@ class OrgNavigation(Navigation): 'api_integration', 'api_keys', 'archive_service', + 'archive_user', 'bat_phone', 'branding_request', 'callbacks', diff --git a/app/notify_client/user_api_client.py b/app/notify_client/user_api_client.py index f074e995f..091d8f1d5 100644 --- a/app/notify_client/user_api_client.py +++ b/app/notify_client/user_api_client.py @@ -65,6 +65,10 @@ class UserApiClient(NotifyAdminAPIClient): user_data = self.post(url, data=data) return user_data['data'] + @cache.delete('user-{user_id}') + def archive_user(self, user_id): + return self.post('/user/{}/archive'.format(user_id), data=None) + @cache.delete('user-{user_id}') def reset_failed_login_count(self, user_id): url = "/user/{}/reset-failed-login-count".format(user_id) diff --git a/app/templates/views/find-users/user-information.html b/app/templates/views/find-users/user-information.html index 7cfa7b8fd..1f89d2f89 100644 --- a/app/templates/views/find-users/user-information.html +++ b/app/templates/views/find-users/user-information.html @@ -38,6 +38,13 @@ {{ user.failed_login_count }} failed login attempts

{% endif %} + {% if user.state == 'active' %} + + + Archive user + + + {% endif %} {% endblock %} diff --git a/tests/app/main/views/test_find_users.py b/tests/app/main/views/test_find_users.py index 5af41fc83..703cae515 100644 --- a/tests/app/main/views/test_find_users.py +++ b/tests/app/main/views/test_find_users.py @@ -1,7 +1,10 @@ +import pytest +from bs4 import BeautifulSoup from flask import url_for from lxml import html from tests import user_json +from tests.conftest import normalize_spaces def test_find_users_by_email_page_loads_correctly(client_request, platform_admin_user): @@ -145,3 +148,88 @@ def test_user_information_page_displays_if_there_are_failed_login_attempts( document = html.fromstring(response.get_data(as_text=True)) assert document.xpath("//p/text()[normalize-space()='2 failed login attempts']") + + +def test_user_information_page_shows_archive_link_for_active_users( + logged_in_platform_admin_client, + api_user_active, + mock_get_organisations_and_services_for_user, +): + response = logged_in_platform_admin_client.get( + url_for('main.user_information', user_id=api_user_active['id']) + ) + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + archive_url = url_for('main.archive_user', user_id=api_user_active['id']) + + link = page.find('a', {'href': archive_url}) + assert normalize_spaces(link.text) == 'Archive user' + + +def test_user_information_page_does_not_show_archive_link_for_inactive_users( + mocker, + client, + platform_admin_user, + mock_get_organisations_and_services_for_user, +): + inactive_user = user_json(state='inactive') + mocker.patch('app.user_api_client.get_user', side_effect=[platform_admin_user, inactive_user], autospec=True) + client.login(platform_admin_user) + response = client.get( + url_for('main.user_information', user_id=inactive_user['id']) + ) + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + + archive_url = url_for('main.archive_user', user_id=inactive_user['id']) + assert not page.find('a', {'href': archive_url}) + + +def test_archive_user_prompts_for_confirmation( + logged_in_platform_admin_client, + api_user_active, + mock_get_organisations_and_services_for_user, +): + response = logged_in_platform_admin_client.get( + url_for('main.archive_user', user_id=api_user_active['id']) + ) + + assert response.status_code == 200 + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + assert 'Are you sure you want to archive this user?' in page.find('div', class_='banner-dangerous').text + + +def test_archive_user_posts_to_user_client( + logged_in_platform_admin_client, + api_user_active, + mocker, + mock_events, +): + mock_user_client = mocker.patch('app.user_api_client.post') + + response = logged_in_platform_admin_client.post( + url_for('main.archive_user', user_id=api_user_active['id']) + ) + + assert response.status_code == 302 + assert response.location == url_for('main.user_information', user_id=api_user_active['id'], _external=True) + mock_user_client.assert_called_once_with('/user/{}/archive'.format(api_user_active['id']), data=None) + + assert mock_events.called + + +def test_archive_user_does_not_create_event_if_user_client_raises_exception( + logged_in_platform_admin_client, + api_user_active, + mocker, + mock_events, +): + mock_user_client = mocker.patch('app.user_api_client.post', side_effect=Exception()) + + with pytest.raises(Exception): + response = logged_in_platform_admin_client.post( + url_for('main.archive_user', user_id=api_user_active.id) + ) + + assert response.status_code == 500 + assert response.location == url_for('main.user_information', user_id=api_user_active['id'], _external=True) + mock_user_client.assert_called_once_with('/user/{}/archive'.format(api_user_active['id']), data=None) + assert not mock_events.called diff --git a/tests/app/notify_client/test_user_client.py b/tests/app/notify_client/test_user_client.py index 5210a0a3e..1ab9389b0 100644 --- a/tests/app/notify_client/test_user_client.py +++ b/tests/app/notify_client/test_user_client.py @@ -194,6 +194,7 @@ def test_returns_value_from_cache( (user_api_client, 'add_user_to_organisation', [sample_uuid(), user_id], {}), (user_api_client, 'set_user_permissions', [user_id, SERVICE_ONE_ID, []], {}), (user_api_client, 'activate_user', [api_user_pending(sample_uuid())['id']], {}), + (user_api_client, 'archive_user', [user_id], {}), (service_api_client, 'remove_user_from_service', [SERVICE_ONE_ID, user_id], {}), (service_api_client, 'create_service', ['', '', 0, False, user_id, sample_uuid()], {}), (invite_api_client, 'accept_invite', [SERVICE_ONE_ID, user_id], {}), diff --git a/tests/app/test_event_handlers.py b/tests/app/test_event_handlers.py index ef72a038f..e132188af 100644 --- a/tests/app/test_event_handlers.py +++ b/tests/app/test_event_handlers.py @@ -2,6 +2,7 @@ import uuid from unittest.mock import ANY from app.event_handlers import ( + create_archive_user_event, create_email_change_event, create_mobile_number_change_event, on_user_logged_in, @@ -51,3 +52,18 @@ def test_create_mobile_number_change_event_calls_events_api(app_, mock_events): 'updated_by_id': updated_by_id, 'original_mobile_number': '07700900000', 'new_mobile_number': '07700900999'}) + + +def test_create_archive_user_event_calls_events_api(app_, mock_events): + user_id = str(uuid.uuid4()) + archived_by_id = str(uuid.uuid4()) + + with app_.test_request_context(): + create_archive_user_event(user_id, archived_by_id) + + mock_events.assert_called_with('archive_user', + {'browser_fingerprint': + {'browser': ANY, 'version': ANY, 'platform': ANY, 'user_agent_string': ''}, + 'ip_address': ANY, + 'user_id': user_id, + 'archived_by_id': archived_by_id}) diff --git a/tests/conftest.py b/tests/conftest.py index 35fb3ebf1..4944606ef 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1117,6 +1117,7 @@ def platform_admin_user(fake_uuid): 'services': [], 'organisations': [], 'current_session_id': None, + 'logged_in_at': None, } return user_data @@ -1137,6 +1138,7 @@ def api_user_active(fake_uuid, email_address='test@user.gov.uk'): 'services': [], 'organisations': [], 'current_session_id': None, + 'logged_in_at': None, } return user_data