From 5b658d924c491b23b1a1f2319bc027bb8ce0cc98 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Wed, 5 Jan 2022 14:53:21 +0000 Subject: [PATCH] Tweak and add test for OrganisationsClient.remove_user_from_organisation This stops adding the current user to the data sent to the API when removing a user from an organisation. API only needs to know the organisation_id and the id of the user we want to remove from the organisation, so we don't need to pass through the id of the current user too. The other change made is to clear the user cache of the user who has been removed from the org. We don't need to clear any of the organisation caches, since these values don't contain lists of users for the orgs. --- app/notify_client/organisations_api_client.py | 7 +++---- .../notify_client/test_organisation_client.py | 16 ++++++++++++++++ 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/app/notify_client/organisations_api_client.py b/app/notify_client/organisations_api_client.py index 467ab6791..c073ba9b2 100644 --- a/app/notify_client/organisations_api_client.py +++ b/app/notify_client/organisations_api_client.py @@ -3,7 +3,7 @@ from itertools import chain from notifications_python_client.errors import HTTPError from app.extensions import redis_client -from app.notify_client import NotifyAdminAPIClient, _attach_current_user, cache +from app.notify_client import NotifyAdminAPIClient, cache class OrganisationsClient(NotifyAdminAPIClient): @@ -76,10 +76,9 @@ class OrganisationsClient(NotifyAdminAPIClient): def get_organisation_services(self, org_id): return self.get(url="/organisations/{}/services".format(org_id)) + @cache.delete('user-{user_id}') def remove_user_from_organisation(self, org_id, user_id): - endpoint = '/organisations/{}/users/{}'.format(org_id, user_id) - data = _attach_current_user({}) - return self.delete(endpoint, data) + return self.delete(f'/organisations/{org_id}/users/{user_id}') def get_services_and_usage(self, org_id, year): return self.get( diff --git a/tests/app/notify_client/test_organisation_client.py b/tests/app/notify_client/test_organisation_client.py index aba5fa5c1..92b0e7167 100644 --- a/tests/app/notify_client/test_organisation_client.py +++ b/tests/app/notify_client/test_organisation_client.py @@ -229,3 +229,19 @@ def test_update_service_organisation_deletes_cache(mocker, fake_uuid): url='/organisations/{}/service'.format(fake_uuid), data=ANY ) + + +def test_remove_user_from_organisation_deletes_user_cache(mocker): + mock_redis_delete = mocker.patch('app.extensions.RedisClient.delete') + mock_delete = mocker.patch('app.notify_client.organisations_api_client.OrganisationsClient.delete') + + org_id = 'abcd-1234' + user_id = 'efgh-5678' + + organisations_client.remove_user_from_organisation( + org_id=org_id, + user_id=user_id, + ) + + assert mock_redis_delete.call_args_list == [call(f'user-{user_id}')] + mock_delete.assert_called_with(f'/organisations/{org_id}/users/{user_id}')