8000 refactor: Replace PageAdmin.delete_view by two smaller methods (#7995) · django-cms/django-cms@cca00a5 · GitHub
[go: up one dir, main page]
Skip to content

Commit cca00a5

Browse files
jriefGithub Release Action
jrief
and
Github Release Action
authored
refactor: Replace PageAdmin.delete_view by two smaller methods (#7995)
* Refactor PageAdmin.delete_view to prevent code duplication * Let `pageadmin.has_change_permission(obj=None)` return `page_permissions.user_can_change_at_least_one_page` instead of `False. * Fix: Clear lru_cache of user object after deleting a page * Fix typos --------- Co-authored-by: Github Release Action <info@django-cms.org>
1 parent 4f1cbc5 commit cca00a5

File tree

4 files changed

+44
-109
lines changed

4 files changed

+44
-109
lines changed

‎cms/admin/pageadmin.py

Lines changed: 21 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@
7676
get_language_tuple,
7777
get_site_language_from_request,
7878
)
79+
from cms.utils.permissions import clear_permission_lru_caches
7980
from cms.utils.plugins import copy_plugins_to_placeholder
8081
from cms.utils.urlutils import admin_reverse
8182

@@ -97,7 +98,6 @@ def get_site(request):
9798

9899
@admin.register(Page)
99100
class PageAdmin(admin.ModelAdmin):
100-
change_list_template = "admin/cms/page/tree/base.html"
101101
actions_menu_template = 'admin/cms/page/tree/actions_dropdown.html'
102102

103103
form = AdvancedSettingsForm
@@ -113,10 +113,13 @@ def has_change_permission(self, request, obj=None):
113113
Return true if the current user has permission on the page.
114114
Return the string 'All' if the user has all rights.
115115
"""
116-
if obj is None:
117-
return
118-
119116
site = get_site(request)
117+
if obj is None:
118+
# Checks if user can change at least one page
119+
return page_permissions.user_can_change_at_least_one_page(
120+
user=request.user,
121+
site=site,
122+
)
120123
return page_permissions.user_can_change_page(request.user, page=obj, site=site)
121124

122125
def has_change_advanced_settings_permission(self, request, obj=None):
@@ -334,105 +337,22 @@ def get_list(self, *args, **kwargs):
334337
return HttpResponseForbidden()
335338

336339
def changelist_view(self, request, extra_context=None):
337-
can_change_any_page = page_permissions.user_can_change_at_least_one_page(
338-
user=request.user,
339-
site=get_site(request),
340-
use_cache=False,
341-
)
342-
343-
if not can_change_any_page:
344-
raise Http404
345-
return HttpResponseRedirect(admin_reverse('cms_pagecontent_changelist'))
346-
347-
@transaction.atomic
348-
def delete_view(self, request, object_id, extra_context=None):
349-
# This is an unfortunate copy/paste from django's delete view.
350-
# The reason is to add the descendant pages to the deleted objects list.
351-
opts = self.model._meta
352-
app_label = opts.app_label
353-
354-
obj = self.get_object(request, object_id=object_id)
355-
356-
if not self.has_delete_permission(request, obj):
357-
raise PermissionDenied
358-
359-
if obj is None:
360-
raise self._get_404_exception(object_id)
361-
362-
# Populate deleted_objects, a data structure of all related objects that
363-
# will also be deleted.
364-
objs = [obj] + list(obj.get_descendant_pages())
340+
parameter = "?" + request.GET.urlencode() if request.GET else ""
341+
return HttpResponseRedirect(admin_reverse('cms_pagecontent_changelist') + parameter)
365342

366-
get_deleted_objects_additional_kwargs = {'request': request}
367-
(deleted_objects, model_count, perms_needed, protected) = get_deleted_objects(
368-
objs, admin_site=self.admin_site,
369-
**get_deleted_objects_additional_kwargs
370-
)
371-
372-
if request.POST and not protected: # The user has confirmed the deletion.
373-
if perms_needed:
374-
raise PermissionDenied
375-
obj_display = force_str(obj)
376-
obj_id = obj.serializable_value(opts.pk.attname)
377-
self.log_deletion(request, obj, obj_display)
378-
self.delete_model(request, obj)
379-
380-
if IS_POPUP_VAR in request.POST:
381-
popup_response_data = json.dumps({
382-
'action': 'delete',
383-
'value': str(obj_id),
384-
})
385-
return TemplateResponse(request, self.popup_response_template or [
386-
f'admin/{opts.app_label}/{opts.model_name}/popup_response.html',
387-
'admin/%s/popup_response.html' % opts.app_label,
388-
'admin/popup_response.html',
389-
], {'popup_response_data': popup_response_data})
390-
391-
self.message_user(
392-
request,
393-
_('The %(name)s "%(obj)s" was deleted successfully.') % {
394-
'name': force_str(opts.verbose_name),
395-
'obj': force_str(obj_display),
396-
},
397-
messages.SUCCESS,
398-
)
399-
400-
can_change_any_page = page_permissions.user_can_change_at_least_one_page(
401-
user=request.user,
402-
site=get_site(request),
403-
use_cache=False,
404-
)
405-
406-
if can_change_any_page:
407-
query = self.get_preserved_filters(request)
408-
post_url = admin_reverse('cms_pagecontent_changelist') + '?' + query
409-
else:
410-
post_url = admin_reverse('index')
411-
return HttpResponseRedirect(post_url)
412-
413-
object_name = force_str(opts.verbose_name)
343+
def response_delete(self, request, obj_display, obj_id):
344+
"""
345+
Determine the HttpResponse for the delete_view stage. Clear the user's permission
346+
lru cache
347+
"""
348+
clear_permission_lru_caches(request.user)
349+
return super().response_delete(request, obj_display, obj_id)
414350

415-
if perms_needed or protected:
416-
title = _("Cannot delete %(name)s") % {"name": object_name}
417-
else:
418-
title = _("Are you sure?")
419-
420-
context = dict(
421-
self.admin_site.each_context(request),
422-
title=title,
423-
object_name=object_name,
424-
object=obj,
425-
deleted_objects=deleted_objects,
426-
model_count=dict(model_count).items(),
427-
perms_lacking=perms_needed,
428-
protected=protected,
429-
opts=opts,
430-
app_label=app_label,
431-
is_popup=(IS_POPUP_VAR in request.POST or IS_POPUP_VAR in request.GET),
432-
to_field=None,
433-
)
434-
context.update(extra_context or {})
435-
return self.render_delete_form(request, context)
351+
def get_deleted_objects(self, objs, request):
352+
deleted_objs = list(objs)
353+
for obj in objs:
354+
deleted_objs.extend(obj.get_descendant_pages())
355+
return super().get_deleted_objects(deleted_objs, request)
436356

437357
def delete_model(self, request, obj):
438358
operation_token = send_pre_page_operation(
@@ -1061,7 +981,6 @@ def has_change_permission(self, request, obj=None):
1061981
can_change_page = page_permissions.user_can_change_at_least_one_page(
1062982
user=request.user,
1063983
site=site,
1064-
use_cache=False,
1065984
)
1066985
return can_change_page
1067986

‎cms/tests/test_admin.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ def test_delete(self):
136136
add_plugin(body, 'TextPlugin', 'en', body='text')
137137
with self.login_user_context(admin_user):
138138
data = {'post': 'yes'}
139-
response = self.client.post(URL_CMS_PAGE_DELETE % page.pk, data)
139+
response = self.client.post(URL_CMS_PAGE_DELETE % page.pk, data, follow=True)
140140
self.assertRedirects(response, self.get_pages_admin_list_uri('en'))
141141

142142
def test_delete_diff_language(self):
@@ -148,7 +148,8 @@ def test_delete_diff_language(self):
148148
add_plugin(body, 'TextPlugin', 'en', body='text')
149149
with self.login_user_context(admin_user):
150150
data = {'post': 'yes'}
151-
response = self.client.post(URL_CMS_PAGE_DELETE % page.pk, data)
151+
response = self.client.post(URL_CMS_PAGE_DELETE % page.pk, data, follow=True)
152+
# follow=True, since page changelist redirects to page content changelist
152153
self.assertRedirects(response, self.get_pages_admin_list_uri('en'))
153154

154155
def test_search_fields(self):

‎cms/tests/test_page_admin.py

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1876,7 +1876,8 @@ def test_user_can_delete_empty_page(self):
18761876

18771877
with self.login_user_context(staff_user):
18781878
data = {'post': 'yes'}
1879-
response = self.client.post(endpoint, data)
1879+
response = self.client.post(endpoint, data, follow=True)
1880+
# follow=True, since page changelist redirects to page content changelist
18801881

18811882
self.assertRedirects(response, redirect_to)
18821883
self.assertFalse(self._page_exists())
@@ -1932,7 +1933,8 @@ def test_user_can_delete_non_empty_page(self):
19321933

19331934
with self.login_user_context(staff_user):
19341935
data = {'post': 'yes'}
1935-
response = self.client.post(endpoint, data)
1936+
response = self.client.post(endpoint, data, follow=True)
1937+
# follow=True, since page changelist redirects to page content changelist
19361938

19371939
self.assertRedirects(response, redirect_to)
19381940
self.assertFalse(self._page_exists())
@@ -3359,7 +3361,7 @@ def test_user_can_delete_empty_page(self):
33593361

33603362
with self.login_user_context(staff_user):
33613363
data = {'post': 'yes'}
3362-
response = self.client.post(endpoint, data)
3364+
response = self.client.post(endpoint, data, follow=True)
33633365

33643366
self.assertRedirects(response, redirect_to)
33653367
self.assertFalse(self._page_exists())
@@ -3423,10 +3425,9 @@ def test_user_can_delete_non_empty_page(self):
34233425
can_change=True,
34243426
can_delete=True,
34253427
)
3426-
34273428
with self.login_user_context(staff_user):
34283429
data = {'post': 'yes'}
3429-
response = self.client.post(endpoint, data)
3430+
response = self.client.post(endpoint, data, follow=True)
34303431

34313432
self.assertRedirects(response, redirect_to)
34323433
self.assertFalse(self._page_exists())

‎cms/utils/permissions.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,20 @@ def cached_func(user, *args, **kwargs):
158158
return cached_func
159159

160160

161+
def clear_func_cache(user, func):
162+
func_cache_name = '_djangocms_cached_func_%s' % func.__name__
163+
if hasattr(user, func_cache_name):
164+
delattr(user, func_cache_name)
165+
166+
167+
def clear_permission_lru_caches(user):
168+
"""
169+
Clear all python lru caches used by the permission system
170+
"""
171+
clear_func_cache(user, get_global_actions_for_user)
172+
clear_func_cache(user, get_page_actions_for_user)
173+
174+
161175
@cached_func
162176
def get_global_actions_for_user(user, site):
163177
actions = set()

0 commit comments

Comments
 (0)
0