Remove update_organisation_name model method

It’s weird to have a method just for updating one attribute. I think
the reason for doing this was to only invalidate the
`organisation-{}-name` cache when absolutely necessary, but:
- we don’t need a separate method to check whether it’s the name being
  updated
- it was easy to get around this by calling
  `OrganisationsClient.update_organisation` directly, leaving a stale
  value in the cache
This commit is contained in:
Chris Hill-Scott
2022-01-11 14:56:58 +00:00
parent c630faf3b4
commit a09af2acc8
2 changed files with 24 additions and 8 deletions

View File

@@ -56,11 +56,10 @@ class OrganisationsClient(NotifyAdminAPIClient):
if cached_service_ids:
redis_client.delete(*map('service-{}'.format, cached_service_ids))
return api_response
if 'name' in kwargs:
redis_client.delete(f'organisation-{org_id}-name')
@cache.delete('organisation-{org_id}-name')
def update_organisation_name(self, org_id, name):
return self.update_organisation(org_id, name=name)
return api_response
@cache.delete('service-{service_id}')
@cache.delete('live-service-and-organisation-counts')

View File

@@ -140,17 +140,34 @@ def test_deletes_domain_cache(
assert len(mock_request.call_args_list) == 1
def test_update_organisation_when_not_updating_org_type(mocker, fake_uuid):
@pytest.mark.parametrize('post_data, expected_cache_delete_calls', (
({'foo': 'bar'}, [
call('organisations'),
call('domains'),
]),
({'name': 'new name'}, [
call('organisation-6ce466d0-fd6a-11e5-82f5-e0accb9d11a6-name'),
call('organisations'),
call('domains'),
]),
))
def test_update_organisation_when_not_updating_org_type(
mocker,
fake_uuid,
post_data,
expected_cache_delete_calls,
):
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')
organisations_client.update_organisation(fake_uuid, **post_data)
mock_post.assert_called_with(
url='/organisations/{}'.format(fake_uuid),
data={'foo': 'bar'}
data=post_data
)
assert mock_redis_delete.call_args_list == [call('organisations'), call('domains')]
assert mock_redis_delete.call_args_list == expected_cache_delete_calls
def test_update_organisation_when_updating_org_type_and_org_has_services(mocker, fake_uuid):