From dc1c73c647990261fda37ce852b0528fea118d12 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Wed, 24 Jul 2019 11:57:42 +0100 Subject: [PATCH] Delete service cache when changing an organisation's sector When we change an organisation's sector we now also change the sector of all its services, so we need to delete those services from Redis. --- app/main/views/organisations.py | 3 ++ app/notify_client/organisations_api_client.py | 10 +++- .../views/organisations/test_organisation.py | 37 +++++++++++-- .../notify_client/test_organisation_client.py | 54 +++++++++++++++++++ 4 files changed, 99 insertions(+), 5 deletions(-) diff --git a/app/main/views/organisations.py b/app/main/views/organisations.py index 38f42ec8a..6e94d3a0f 100644 --- a/app/main/views/organisations.py +++ b/app/main/views/organisations.py @@ -216,8 +216,11 @@ def edit_organisation_type(org_id): ) if form.validate_on_submit(): + org_service_ids = [service['id'] for service in current_organisation.services] + organisations_client.update_organisation( current_organisation.id, + cached_service_ids=org_service_ids, organisation_type=form.organisation_type.data, ) return redirect(url_for('.organisation_settings', org_id=org_id)) diff --git a/app/notify_client/organisations_api_client.py b/app/notify_client/organisations_api_client.py index a31625784..b984c21fb 100644 --- a/app/notify_client/organisations_api_client.py +++ b/app/notify_client/organisations_api_client.py @@ -2,6 +2,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 @@ -45,8 +46,13 @@ class OrganisationsClient(NotifyAdminAPIClient): @cache.delete('domains') @cache.delete('organisations') - def update_organisation(self, org_id, **kwargs): - return self.post(url="/organisations/{}".format(org_id), data=kwargs) + def update_organisation(self, org_id, cached_service_ids=None, **kwargs): + api_response = self.post(url="/organisations/{}".format(org_id), data=kwargs) + + if kwargs.get('organisation_type') and cached_service_ids: + redis_client.delete(*map('service-{}'.format, cached_service_ids)) + + return api_response def update_organisation_name(self, org_id, name): return self.update_organisation(org_id, name=name) diff --git a/tests/app/main/views/organisations/test_organisation.py b/tests/app/main/views/organisations/test_organisation.py index 7e5eb5d34..cd7cd37fb 100644 --- a/tests/app/main/views/organisations/test_organisation.py +++ b/tests/app/main/views/organisations/test_organisation.py @@ -359,17 +359,17 @@ def test_view_organisation_settings( ( '.edit_organisation_type', {'organisation_type': 'central'}, - {'organisation_type': 'central'}, + {'cached_service_ids': [], 'organisation_type': 'central'}, ), ( '.edit_organisation_type', {'organisation_type': 'local'}, - {'organisation_type': 'local'}, + {'cached_service_ids': [], 'organisation_type': 'local'}, ), ( '.edit_organisation_type', {'organisation_type': 'nhs_local'}, - {'organisation_type': 'nhs_local'}, + {'cached_service_ids': [], 'organisation_type': 'nhs_local'}, ), ( '.edit_organisation_crown_status', @@ -412,6 +412,7 @@ def test_view_organisation_settings( ), )) def test_update_organisation_settings( + mocker, client_request, fake_uuid, organisation_one, @@ -422,6 +423,7 @@ def test_update_organisation_settings( expected_persisted, user, ): + mocker.patch('app.organisations_client.get_organisation_services', return_value=[]) client_request.login(user(fake_uuid)) client_request.post( @@ -442,6 +444,35 @@ def test_update_organisation_settings( ) +def test_update_organisation_sector_sends_service_id_data_to_api_client( + client_request, + mock_get_organisation, + organisation_one, + mock_get_organisation_services, + mock_update_organisation, + platform_admin_user, +): + client_request.login(platform_admin_user) + + client_request.post( + 'main.edit_organisation_type', + org_id=organisation_one['id'], + _data={'organisation_type': 'central'}, + _expected_status=302, + _expected_redirect=url_for( + 'main.organisation_settings', + org_id=organisation_one['id'], + _external=True, + ), + ) + + mock_update_organisation.assert_called_once_with( + organisation_one['id'], + cached_service_ids=['12345', '67890', SERVICE_ONE_ID], + organisation_type='central' + ) + + @pytest.mark.parametrize('user', ( pytest.param( platform_admin_user, diff --git a/tests/app/notify_client/test_organisation_client.py b/tests/app/notify_client/test_organisation_client.py index d0cc36612..d171b0fb4 100644 --- a/tests/app/notify_client/test_organisation_client.py +++ b/tests/app/notify_client/test_organisation_client.py @@ -138,3 +138,57 @@ def test_deletes_domain_cache( assert call('domains') in mock_redis_delete.call_args_list assert len(mock_request.call_args_list) == 1 + + +def test_update_organisation_when_not_updating_org_type(mocker, fake_uuid): + mock_redis_delete = mocker.patch('app.extensions.RedisClient.delete') + mock_post = mocker.patch('app.notify_client.organisations_api_client.OrganisationsClient.post') + + organisations_client.update_organisation(fake_uuid, foo='bar') + + mock_post.assert_called_with( + url='/organisations/{}'.format(fake_uuid), + data={'foo': 'bar'} + ) + assert mock_redis_delete.call_args_list == [call('domains'), call('organisations')] + + +def test_update_organisation_when_updating_org_type_and_org_has_services(mocker, fake_uuid): + mock_redis_delete = mocker.patch('app.extensions.RedisClient.delete') + mock_post = mocker.patch('app.notify_client.organisations_api_client.OrganisationsClient.post') + + organisations_client.update_organisation( + fake_uuid, + cached_service_ids=['a', 'b', 'c'], + organisation_type='central', + ) + + mock_post.assert_called_with( + url='/organisations/{}'.format(fake_uuid), + data={'organisation_type': 'central'} + ) + assert mock_redis_delete.call_args_list == [ + call('domains'), + call('organisations'), + call('service-a', 'service-b', 'service-c'), + ] + + +def test_update_organisation_when_updating_org_type_but_org_has_no_services(mocker, fake_uuid): + mock_redis_delete = mocker.patch('app.extensions.RedisClient.delete') + mock_post = mocker.patch('app.notify_client.organisations_api_client.OrganisationsClient.post') + + organisations_client.update_organisation( + fake_uuid, + cached_service_ids=[], + organisation_type='central', + ) + + mock_post.assert_called_with( + url='/organisations/{}'.format(fake_uuid), + data={'organisation_type': 'central'} + ) + assert mock_redis_delete.call_args_list == [ + call('domains'), + call('organisations'), + ]