Opened 6 years ago

Closed 5 years ago

#30311 closed Bug (fixed)

Can't replace global admin actions with specialized ones per-admin

Reported by: Aarni Koskela Owned by: hashlash
Component: contrib.admin Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

f9ff1df1daac8ae1fc22b27f48735148cb5488dd landed in 2.2 (discussion in #29917), which makes it impossible to replace a generic site-wide action (such as the built-in delete_selected) with a new one. It fails with the admin.E130 system check error.

We're seeing this with the qsessions app, which has to delete its session objects in non-bulk mode in order to clear caches: https://github.com/QueraTeam/django-qsessions/blob/c21d602a50c4746da7f698a8d39317ef214e7d05/qsessions/admin.py#L41-L46
(For this particular use case, it seems a fix is to instead override modeladmin.delete_queryset within qsessions's SessionAdmin, as that's what the built-in delete_selected action does per https://github.com/django/django/blob/851d9eac23e08ff10a2d6fe5368b02798761663c/django/contrib/admin/actions.py#L40 .)

Change History (10)

comment:1 by Carlton Gibson, 6 years ago

Resolution: invalid
Status: newclosed

comment:2 by Aarni Koskela, 6 years ago

Resolution: invalid
Status: closednew

Thanks! I did read the release notes, but the section linked has no mention of global admin actions and it doesn't exactly apply, see below. (Reopening for that reason.)

This issue only arises when

  • a global action, say expect_inquisition (or the built-in default delete_selected) has been defined, which is implicitly on every single ModelAdmin
  • that global action is still enabled
  • and you attempt to explicitly add an action with the same __name__ on any given ModelAdmin class

comment:3 by Carlton Gibson, 6 years ago

Resolution: invalid
Status: newclosed

Right, OK, gotcha.

Multiple defined actions with the same name are not supported. The system check added in #29711 led directly to uncovering #29917, which had been latent for many years.

As per the discussion on #29917, and the mailing list thread (that I linked above), the decision was taken to adjust this behaviour.

You need to adjust your code to use site wide actions, or as an alternative you can use subclassing, but now according to Python's normal inheritance rules.

This was an example from the discussion:

class WithCustom(AdminBase):
    actions = AdminBase.actions + ['custom_action']

You're free to adjust actions any way you need (at class, definition, in __init__(), in _get_base_actions(), in get_actions()...) See the Disabling actions section of the docs. There are plenty of strategies there.

Version 0, edited 6 years ago by Carlton Gibson (next)

comment:4 by Carlton Gibson, 5 years ago

Resolution: invalid
Status: closednew
Triage Stage: UnreviewedAccepted

Reopening based on mailing list discussion https://groups.google.com/d/topic/django-developers/jDz-0wfowqg/discussion

Patch should at least consider whether we'll handle just delete_selected or any action (expect_inquisition say).

(Original triage should have been wontfix rather than invalid looking again.)

comment:5 by hashlash, 5 years ago

Owner: changed from nobody to hashlash
Status: newassigned

Hi!

May I work on this? I'm kind of new and want to contribute to this community. I've read the discussion mentioned (https://groups.google.com/d/topic/django-developers/jDz-0wfowqg/discussion) and I think I can implement the solution. Will update as soon as I finish implementing the tests and patches

Cheers

comment:7 by אורי, 5 years ago

What is delete_selected and how do you use it? What are generic site-wide actions?

in reply to:  7 comment:8 by hashlash, 5 years ago

Replying to אורי:

What is delete_selected and how do you use it? What are generic site-wide actions?

delete_selected is a built-in Django's admin action. For more information (including site-wide actions) please read the docs: https://docs.djangoproject.com/en/2.2/ref/contrib/admin/actions/

comment:9 by Mariusz Felisiak, 5 years ago

Triage Stage: AcceptedReady for checkin
Version: 2.2master

comment:10 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

Resolution: fixed
Status: assignedclosed

In dfbd9ca0:

Fixed #30311 -- Restored ability to override global admin actions.

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