darkhelm/notifications-admin
1
0
Fork 0
mirror of https://github.com/GSA/notifications-admin.git synced 2026-08-11 09:28:27 -04:00
Code Issues Packages Projects Releases Wiki Activity

Update _delete_template_cache_for_service to delete all template version cache and not just the one ending in "None"

Browse Source 
Update all methods that were previous calling @cache.delete('service-{service-id}-template-None') to instead call _delete_template_cache_for_service
Remove call to get service templates, it's not needed since all template version cache is being deleted.
This commit is contained in:
Rebecca Law
2021-01-25 14:03:16 +00:00
parent e2ecff050d
commit 01a3df6edc
4 changed files with 41 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files

32
app/notify_client/service_api_client.py
View File

@@ -7,12 +7,7 @@ from app.notify_client import NotifyAdminAPIClient, _attach_current_user, cache
class ServiceAPIClient(NotifyAdminAPIClient):
def _delete_template_cache_for_service(self, service_id):
templates_for_service = self.get_service_templates(service_id)['data']
if templates_for_service:
redis_client.delete(*[
f"service-{service_id}-template-{x['id']}-version-None"
for x in templates_for_service
])
redis_client.delete_cache_keys_by_pattern(f"service-{service_id}-template-*")
@cache.delete('user-{user_id}')
def create_service(
@@ -109,12 +104,12 @@ class ServiceAPIClient(NotifyAdminAPIClient):
'go_live_user',
'go_live_at',
'rate_limit',
'notes',
}
if disallowed_attributes:
raise TypeError('Not allowed to update service attributes: {}'.format(
", ".join(disallowed_attributes)
))
endpoint = "/service/{0}".format(service_id)
return self.post(endpoint, data)
@@ -143,7 +138,9 @@ class ServiceAPIClient(NotifyAdminAPIClient):
def archive_service(self, service_id, cached_service_user_ids):
if cached_service_user_ids:
redis_client.delete(*map('user-{}'.format, cached_service_user_ids))
return self.post('/service/{}/archive'.format(service_id), data=None)
ret = self.post('/service/{}/archive'.format(service_id), data=None)
self._delete_template_cache_for_service(str(service_id))
return ret
@cache.delete('service-{service_id}')
def suspend_service(self, service_id):
@@ -191,7 +188,6 @@ class ServiceAPIClient(NotifyAdminAPIClient):
return self.post(endpoint, data)
@cache.delete('service-{service_id}-templates')
@cache.delete('service-{service_id}-template-{id_}-version-None')
@cache.delete('service-{service_id}-template-{id_}-versions')
def update_service_template(
self, id_, name, type_, content, service_id, subject=None, process_type=None
@@ -216,40 +212,44 @@ class ServiceAPIClient(NotifyAdminAPIClient):
})
data = _attach_current_user(data)
endpoint = "/service/{0}/template/{1}".format(service_id, id_)
self._delete_template_cache_for_service(service_id)
return self.post(endpoint, data)
@cache.delete('service-{service_id}-templates')
@cache.delete('service-{service_id}-template-{id_}-version-None')
@cache.delete('service-{service_id}-template-{id_}-versions')
def redact_service_template(self, service_id, id_):
return self.post(
ret = self.post(
"/service/{}/template/{}".format(service_id, id_),
_attach_current_user(
{'redact_personalisation': True}
),
)
self._delete_template_cache_for_service(service_id)
return ret
@cache.delete('service-{service_id}-templates')
@cache.delete('service-{service_id}-template-{template_id}-version-None')
@cache.delete('service-{service_id}-template-{template_id}-versions')
def update_service_template_sender(self, service_id, template_id, reply_to):
data = {
'reply_to': reply_to,
}
data = _attach_current_user(data)
return self.post(
ret = self.post(
"/service/{0}/template/{1}".format(service_id, template_id),
data
)
self._delete_template_cache_for_service(service_id)
return ret
@cache.delete('service-{service_id}-templates')
@cache.delete('service-{service_id}-template-{template_id}-version-None')
@cache.delete('service-{service_id}-template-{template_id}-versions')
def update_service_template_postage(self, service_id, template_id, postage):
return self.post(
ret = self.post(
"/service/{0}/template/{1}".format(service_id, template_id),
_attach_current_user({'postage': postage})
)
self._delete_template_cache_for_service(service_id)
return ret
@cache.set('service-{service_id}-template-{template_id}-version-{version}')
def get_service_template(self, service_id, template_id, version=None):
@@ -301,7 +301,6 @@ class ServiceAPIClient(NotifyAdminAPIClient):
])
@cache.delete('service-{service_id}-templates')
@cache.delete('service-{service_id}-template-{template_id}-version-None')
@cache.delete('service-{service_id}-template-{template_id}-versions')
def delete_service_template(self, service_id, template_id):
"""
@@ -312,6 +311,7 @@ class ServiceAPIClient(NotifyAdminAPIClient):
'archived': True
}
data = _attach_current_user(data)
self._delete_template_cache_for_service(service_id)
return self.post(endpoint, data=data)
def is_service_name_unique(self, service_id, name, email_from):

2
package.json
View File

@@ -23,9 +23,9 @@
"@babel/preset-env": "7.4.2",
"del": "5.1.0",
"diff-dom": "2.5.1",
"govuk_frontend_toolkit": "8.1.0",
"govuk-elements-sass": "3.1.2",
"govuk-frontend": "2.13.0",
"govuk_frontend_toolkit": "8.1.0",
"gulp": "4.0.0",
"gulp-add-src": "1.0.0",
"gulp-babel": "8.0.0",

2
tests/app/main/views/test_service_settings.py
View File

@@ -3976,10 +3976,12 @@ def test_archive_service_after_confirm(
mock_get_service_and_organisation_counts,
mock_get_organisations_and_services_for_user,
mock_get_users_by_service,
mock_get_service_templates,
user,
):
mocked_fn = mocker.patch('app.service_api_client.post')
redis_delete_mock = mocker.patch('app.notify_client.service_api_client.redis_client.delete')
mocker.patch('app.notify_client.service_api_client.redis_client.delete_cache_keys_by_pattern')
client_request.login(user)
page = client_request.post(
'main.archive_service',

41
tests/app/notify_client/test_service_api_client.py
View File

@@ -12,7 +12,7 @@ FAKE_TEMPLATE_ID = uuid4()
def test_client_posts_archived_true_when_deleting_template(mocker):
mocker.patch('app.notify_client.current_user', id='1')
mock_redis_delete_by_pattern = mocker.patch('app.extensions.RedisClient.delete_cache_keys_by_pattern')
expected_data = {
'archived': True,
'created_by': '1'
@@ -21,9 +21,12 @@ def test_client_posts_archived_true_when_deleting_template(mocker):
client = ServiceAPIClient()
mock_post = mocker.patch('app.notify_client.service_api_client.ServiceAPIClient.post')
mocker.patch('app.notify_client.service_api_client.ServiceAPIClient.get',
return_value={'data': {'id': str(FAKE_TEMPLATE_ID)}})
client.delete_service_template(SERVICE_ONE_ID, FAKE_TEMPLATE_ID)
mock_post.assert_called_once_with(expected_url, data=expected_data)
assert call(f'service-{SERVICE_ONE_ID}-template-*') in mock_redis_delete_by_pattern.call_args_list
def test_client_gets_service(mocker):
@@ -436,27 +439,22 @@ def test_deletes_service_cache(
]),
('update_service_template', [FAKE_TEMPLATE_ID, 'foo', 'sms', 'bar', SERVICE_ONE_ID], [
'service-{}-template-{}-versions'.format(SERVICE_ONE_ID, FAKE_TEMPLATE_ID),
'service-{}-template-{}-version-None'.format(SERVICE_ONE_ID, FAKE_TEMPLATE_ID),
'service-{}-templates'.format(SERVICE_ONE_ID),
]),
('redact_service_template', [SERVICE_ONE_ID, FAKE_TEMPLATE_ID], [
'service-{}-template-{}-versions'.format(SERVICE_ONE_ID, FAKE_TEMPLATE_ID),
'service-{}-template-{}-version-None'.format(SERVICE_ONE_ID, FAKE_TEMPLATE_ID),
'service-{}-templates'.format(SERVICE_ONE_ID),
]),
('update_service_template_sender', [SERVICE_ONE_ID, FAKE_TEMPLATE_ID, 'foo'], [
'service-{}-template-{}-versions'.format(SERVICE_ONE_ID, FAKE_TEMPLATE_ID),
'service-{}-template-{}-version-None'.format(SERVICE_ONE_ID, FAKE_TEMPLATE_ID),
'service-{}-templates'.format(SERVICE_ONE_ID),
]),
('update_service_template_postage', [SERVICE_ONE_ID, FAKE_TEMPLATE_ID, 'first'], [
'service-{}-template-{}-versions'.format(SERVICE_ONE_ID, FAKE_TEMPLATE_ID),
'service-{}-template-{}-version-None'.format(SERVICE_ONE_ID, FAKE_TEMPLATE_ID),
'service-{}-templates'.format(SERVICE_ONE_ID),
]),
('delete_service_template', [SERVICE_ONE_ID, FAKE_TEMPLATE_ID], [
'service-{}-template-{}-versions'.format(SERVICE_ONE_ID, FAKE_TEMPLATE_ID),
'service-{}-template-{}-version-None'.format(SERVICE_ONE_ID, FAKE_TEMPLATE_ID),
'service-{}-templates'.format(SERVICE_ONE_ID),
]),
('archive_service', [SERVICE_ONE_ID, []], [
@@ -471,25 +469,32 @@ def test_deletes_caches_when_modifying_templates(
method,
extra_args,
expected_cache_deletes,
mock_get_service_templates,
):
mocker.patch('app.notify_client.current_user', id='1')
mock_redis_delete = mocker.patch('app.extensions.RedisClient.delete')
mock_redis_delete_by_pattern = mocker.patch('app.extensions.RedisClient.delete_cache_keys_by_pattern')
mock_request = mocker.patch('notifications_python_client.base.BaseAPIClient.request')
getattr(service_api_client, method)(*extra_args)
assert mock_redis_delete.call_args_list == [call(x) for x in expected_cache_deletes]
assert len(mock_request.call_args_list) == 1
if method != 'create_service_template':
# no deletes for template cach on create_service_template
assert len(mock_redis_delete_by_pattern.call_args_list) == 1
assert mock_redis_delete_by_pattern.call_args_list[0] == call(f'service-{SERVICE_ONE_ID}-template-*')
def test_deletes_cached_users_when_archiving_service(mocker):
mock_redis_delete = mocker.patch('app.notify_client.service_api_client.redis_client.delete')
mocker.patch('notifications_python_client.base.BaseAPIClient.request')
def test_deletes_cached_users_when_archiving_service(mocker, mock_get_service_templates):
mock_redis_delete = mocker.patch('app.extensions.RedisClient.delete')
mock_redis_delete_by_pattern = mocker.patch('app.extensions.RedisClient.delete_cache_keys_by_pattern')
mocker.patch('notifications_python_client.base.BaseAPIClient.request', return_value={'data': ""})
service_api_client.archive_service(SERVICE_ONE_ID, ["my-user-id1", "my-user-id2"])
assert call('user-my-user-id1', 'user-my-user-id2') in mock_redis_delete.call_args_list
assert call(f'service-{SERVICE_ONE_ID}-template-*') in mock_redis_delete_by_pattern.call_args_list
def test_client_gets_guest_list(mocker):
@@ -525,30 +530,28 @@ def test_client_doesnt_delete_service_template_cache_when_none_exist(
mocker.patch('app.notify_client.current_user', id='1')
mocker.patch('notifications_python_client.base.BaseAPIClient.request')
mock_redis_delete = mocker.patch('app.extensions.RedisClient.delete')
mock_redis_delete_by_pattern = mocker.patch('app.extensions.RedisClient.delete_cache_keys_by_pattern')
service_api_client.update_reply_to_email_address(SERVICE_ONE_ID, uuid4(), 'foo@bar.com')
assert len(mock_redis_delete.call_args_list) == 1
assert mock_redis_delete.call_args_list[0] == call('service-{}'.format(SERVICE_ONE_ID))
assert len(mock_redis_delete_by_pattern.call_args_list) == 1
def test_client_deletes_service_template_cache_when_service_is_updated(
app_,
mock_get_user,
mock_get_service_templates,
mocker
):
mocker.patch('app.notify_client.current_user', id='1')
mocker.patch('notifications_python_client.base.BaseAPIClient.request')
mock_redis_delete = mocker.patch('app.extensions.RedisClient.delete')
mock_redis_delete_by_pattern = mocker.patch('app.extensions.RedisClient.delete_cache_keys_by_pattern')
service_api_client.update_reply_to_email_address(SERVICE_ONE_ID, uuid4(), 'foo@bar.com')
assert len(mock_redis_delete.call_args_list) == 2
assert mock_redis_delete.call_args_list[1] == call('service-{}'.format(SERVICE_ONE_ID))
templates_to_delete = mock_redis_delete.call_args_list[0][0]
assert len(templates_to_delete) == 6
for template_key in templates_to_delete:
assert template_key.startswith(f'service-{SERVICE_ONE_ID}-template')
assert template_key.endswith('version-None')
assert len(mock_redis_delete.call_args_list) == 1
assert mock_redis_delete.call_args_list[0] == call(f'service-{SERVICE_ONE_ID}')
assert mock_redis_delete_by_pattern.call_args_list[0] == call(f'service-{SERVICE_ONE_ID}-template-*')