Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#13743 closed (fixed)

CommentsAdmin blows up when 'delete_selected' action is disabled.

Reported by: daniellindsley Owned by: nobody
Component: contrib.comments Version: 1.3-alpha
Severity: Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

In our admin, we disable the delete_selected action due to too many users shooting themselves in the foot. We also have non-superuser accounts that can access the admin.

The CommentsAdmin.get_actions method assumes various actions will be in the list, but the actions.pop statements will fail as a result.

Attached is a traceback and a patch.

Traceback:
File "/home/code.django-1.0/django/core/handlers/base.py" in get_response
100. response = callback(request, *callback_args, **callback_kwargs)
File "/home/code.django-1.0/django/contrib/admin/options.py" in wrapper
239. return self.admin_site.admin_view(view)(*args, **kwargs)
File "/home/code.django-1.0/django/utils/decorators.py" in _wrapped_view
76. response = view_func(request, *args, **kwargs)
File "/home/code.django-1.0/django/views/decorators/cache.py" in _wrapped_view_func
69. response = view_func(request, *args, **kwargs)
File "/home/code.django-1.0/django/contrib/admin/sites.py" in inner
190. return view(request, *args, **kwargs)
File "/home/code.django-1.0/django/utils/decorators.py" in _wrapper
21. return decorator(bound_func)(*args, **kwargs)
File "/home/code.django-1.0/django/utils/decorators.py" in _wrapped_view
76. response = view_func(request, *args, **kwargs)
File "/home/code.django-1.0/django/utils/decorators.py" in bound_func
17. return func(self, *args2, **kwargs2)
File "/home/code.django-1.0/django/contrib/admin/options.py" in changelist_view
955. actions = self.get_actions(request)
File "/home/code.django-1.0/django/contrib/comments/admin.py" in get_actions
32. actions.pop('delete_selected')
File "/home/code.django-1.0/django/utils/datastructures.py" in pop
123. result = super(SortedDict, self).pop(k, *args)

Exception Type: KeyError at /admin/comments/comment/
Exception Value: 'delete_selected'

Attachments (2)

comments_admin.diff (1011 bytes) - added by daniellindsley 5 years ago.
Patch (without tests)
comments_admin_with_tests.diff (2.8 KB) - added by daniellindsley 4 years ago.
Patch with tests.

Download all attachments as: .zip

Change History (11)

Changed 5 years ago by daniellindsley

Patch (without tests)

comment:1 Changed 5 years ago by ubernostrum

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 5 years ago by ubernostrum

  • Needs tests set

comment:3 Changed 5 years ago by d0ugal

I've been working on this and I can confirm the patch and test apply cleanly - they also solve the issue and I've tested it in an example project. I've not managed to figure out a automated test yet that works correctly - any suggestions would be welcome.

Currently I've tried the following in AdminActionsTests (regressiontests/comment_tests/tests/moderation_view_tests.py)

    def testDisableDeleteSelected(self):
        comments = self.createSomeComments()
        makeModerator("normaluser")
        
        self.client.login(username="normaluser", password="normaluser")
        
        from django.contrib import admin
        admin.site.disable_action('delete_selected')
        
        response = self.client.get("/admin/comments/comment/")
        self.assertEqual(response.status_code, 200)

I'm not that familiar with the disable_action but it doesn't seem to be having any effect when placed like this.

comment:4 Changed 4 years ago by daniellindsley

  • Needs tests unset
  • Version changed from 1.2 to 1.3-alpha

Alright, got a patch that works against trunk that also includes tests.

Changed 4 years ago by daniellindsley

Patch with tests.

comment:5 Changed 4 years ago by d0ugal

  • Triage Stage changed from Accepted to Ready for checkin

This looks good to me and passes. Marking RFC.

comment:6 Changed 4 years ago by jezdez

  • milestone set to 1.3

comment:7 Changed 4 years ago by jezdez

  • Resolution set to fixed
  • Status changed from new to closed

(In [14996]) Fixed #13743 -- Fixed CommentsAdmin to not blow up if the delete_selected action is disabled. Thanks, Daniel Lindsley.

comment:8 Changed 4 years ago by jezdez

(In [15001]) [1.2.X] Fixed #13743 -- Fixed CommentsAdmin to not blow up if the delete_selected action is disabled. Thanks, Daniel Lindsley.

Backport from trunk (r14996).

comment:9 Changed 4 years ago by jacob

  • milestone 1.3 deleted

Milestone 1.3 deleted

Note: See TracTickets for help on using tickets.
Back to Top