Merge pull request #3459 from alphagov/delete-cache-on-archive

Delete cached users and templates when archiving a service

app/main/views/service_settings.py

@@ -302,7 +302,11 @@ def archive_service(service_id):
     ):
         abort(403)
     if request.method == 'POST':
-        service_api_client.archive_service(service_id)
+        # We need to purge the cache for the services users as otherwise, although they will have had their permissions
+        # removed in the DB, they would still have permissions in the cache to view/edit/manage this service
+        cached_service_user_ids = [user.id for user in current_service.active_users]
+
+        service_api_client.archive_service(service_id, cached_service_user_ids)
         flash(
             '‘{}’ was deleted'.format(current_service.name),
             'default_with_tick',

app/notify_client/service_api_client.py

@@ -129,7 +129,10 @@ class ServiceAPIClient(NotifyAdminAPIClient):
         return self.update_service(service_id, **properties)
 
     @cache.delete('service-{service_id}')
-    def archive_service(self, service_id):
+    @cache.delete('service-{service_id}-templates')
+    def archive_service(self, service_id, cached_service_user_ids):
+        if cached_service_user_ids:
+            cache.delete(*map('user-{}'.format, cached_service_user_ids))
         return self.post('/service/{}/archive'.format(service_id), data=None)
 
     @cache.delete('service-{service_id}')

tests/app/main/views/test_service_settings.py

@@ -3807,9 +3807,11 @@ def test_archive_service_after_confirm(
     mock_get_organisations,
     mock_get_service_and_organisation_counts,
     mock_get_organisations_and_services_for_user,
+    mock_get_users_by_service,
     user,
 ):
     mocked_fn = mocker.patch('app.service_api_client.post')
+    cache_delete_mock = mocker.patch('app.notify_client.service_api_client.cache.delete')
     client_request.login(user)
     page = client_request.post(
         'main.archive_service',
@@ -3822,6 +3824,8 @@ def test_archive_service_after_confirm(
     assert normalize_spaces(page.select_one('.banner-default-with-tick').text) == (
         '‘service one’ was deleted'
     )
+    # The one user which is part of this service has the sample_uuid as it's user ID
+    cache_delete_mock.assert_called_once_with(f"user-{sample_uuid()}")
 
 
 @pytest.mark.parametrize('user', (

tests/app/notify_client/test_service_api_client.py

@@ -389,7 +389,7 @@ def test_returns_value_from_cache(
 @pytest.mark.parametrize('client, method, extra_args, extra_kwargs', [
     (service_api_client, 'update_service', [SERVICE_ONE_ID], {'name': 'foo'}),
     (service_api_client, 'update_service_with_properties', [SERVICE_ONE_ID], {'properties': {}}),
-    (service_api_client, 'archive_service', [SERVICE_ONE_ID], {}),
+    (service_api_client, 'archive_service', [SERVICE_ONE_ID, []], {}),
     (service_api_client, 'suspend_service', [SERVICE_ONE_ID], {}),
     (service_api_client, 'resume_service', [SERVICE_ONE_ID], {}),
     (service_api_client, 'remove_user_from_service', [SERVICE_ONE_ID, ''], {}),
@@ -458,6 +458,10 @@ def test_deletes_service_cache(
         'template-{}-version-None'.format(FAKE_TEMPLATE_ID),
         'service-{}-templates'.format(SERVICE_ONE_ID),
     ]),
+    ('archive_service', [SERVICE_ONE_ID, []], [
+        'service-{}-templates'.format(SERVICE_ONE_ID),
+        'service-{}'.format(SERVICE_ONE_ID),
+    ]),
 ])
 def test_deletes_caches_when_modifying_templates(
     app_,
@@ -475,3 +479,12 @@ def test_deletes_caches_when_modifying_templates(
 
     assert mock_redis_delete.call_args_list == list(map(call, expected_cache_deletes))
     assert len(mock_request.call_args_list) == 1
+
+
+def test_deletes_cached_users_when_archiving_service(mocker):
+    mock_redis_delete = mocker.patch('app.notify_client.service_api_client.cache.delete')
+    mocker.patch('notifications_python_client.base.BaseAPIClient.request')
+
+    service_api_client.archive_service(SERVICE_ONE_ID, ["my-user-id1", "my-user-id2"])
+
+    assert mock_redis_delete.call_args_list == [call('user-my-user-id1', 'user-my-user-id2')]