[Django] #29711: One can use as actions functions generated only by the first call to another function
Django
another function
-----------------------------------------+------------------------
Reporter: przemub | Owner: nobody
Type: Uncategorized | Status: new
Component: contrib.admin | Version: 2.1
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+------------------------
It might be that I don't understand something, but I wrote this piece of
code yesterday:
{{{
# admin.py
def action(fun):
def immediately(modeladmin, request, queryset):
result = fun(queryset)
if result:
modeladmin.message_user(request, result)
immediately.short_description = fun.short_description + " (now)"
def queue(modeladmin, request, queryset):
fun.delay(queryset)
modeladmin.message_user(request, "Added to the queue.")
queue.short_description = fun.short_description + " (add to the
queue)"
return immediately, queue
class CompanyAdmin(admin.ModelAdmin):
actions = (*action(telephone), *action(mail))
# (...)
}}}
And only functions generated by the first call to action() get added. (In
the case above two actions from "telephone" only; when reversed, from
"mail" only).
I suspect that Django might detect them as duplicates.
--
Ticket URL: <https://code.djangoproject.com/ticket/29711>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
another function
Reporter: przemub | Owner: nobody
Type: Uncategorized | Status: closed
Component: contrib.admin | Version: 2.1
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* status: new => closed
* resolution: => invalid
Comment:
The issue
[https://github.com/django/django/blob/7b7fb2eca51dacb5002a4e6b6e1417b36bd5dfd7/django/contrib/admin/options.py#L902-L906
occurs here in the `get_actions()` call]:
{{{
# Convert the actions into an OrderedDict keyed by name.
return OrderedDict(
(name, (func, name, desc))
for func, name, desc in actions
)
}}}
Here you've got two pairs of `(func, name, desc)` tuples keyed by the
`immediately` and `queue` names. Thus when they're passed to the
`OrderedDict` constructor, the keys conflict.
> And only functions generated by the first call to action() get added...
This doesn't seem right. We should expect the opposite:
> If a key occurs more than once, the last value for that key becomes the
corresponding value in the new dictionary.
> [https://docs.python.org/3.7/library/stdtypes.html#dict Python Docs]
The
[https://github.com/django/django/blob/7b7fb2eca51dacb5002a4e6b6e1417b36bd5dfd7/django/contrib/admin/options.py#L928
`name` comes from the `__name__` attribute of the action function].
You should be able to adjust this to ensure unique names.
Perhaps something like:
{{{
immediately.__name__ = 'immediately_{}'.format(fun.__name__)
}}}
... and so on.
--
Ticket URL: <https://code.djangoproject.com/ticket/29711#comment:1>
Django
another function
Reporter: Przemysław | Owner: nobody
Buczkowski |
Type: Uncategorized | Status: closed
Component: contrib.admin | Version: 2.1
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Przemysław Buczkowski):
Yes, you're right, the opposite happens - I got a bit confused :)
Do you think that a warning should be added to Django docs? I can provide
a patch to the docs since I don't think that this behaviour is obvious,
even if it's intended - but maybe that's just me.
--
Ticket URL: <https://code.djangoproject.com/ticket/29711#comment:2>
Django
another function
Reporter: Przemysław | Owner: nobody
Buczkowski |
Type: Uncategorized | Status: closed
Severity: Normal | Resolution: invalid
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Adam (Chainz) Johnson):
A system check could be added to ensure that every action in `actions` has
a unique `__name__` - since the `__name__` collapsing into an
`OrderedDict` isn't done until runtime.
--
Ticket URL: <https://code.djangoproject.com/ticket/29711#comment:3>
Django
another function
Reporter: Przemysław | Owner: nobody
Buczkowski |
Component: contrib.admin | Version: 2.1
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Przemysław Buczkowski):
* status: closed => new
* resolution: invalid =>
Comment:
@Adam That would be great :)
--
Ticket URL: <https://code.djangoproject.com/ticket/29711#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Przemysław | Owner: nobody
Buczkowski |
Cleanup/optimization |
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* stage: Unreviewed => Accepted
* type: Uncategorized => Cleanup/optimization
* version: 2.1 => master
Comment:
Yes OK. Przemysław, if you'd like to code that up, that would be great.
--
Ticket URL: <https://code.djangoproject.com/ticket/29711#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Przemysław | Owner:
Type: | Status: assigned
Cleanup/optimization |
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Przemysław Buczkowski):
* owner: nobody => Przemysław Buczkowski
* status: new => assigned
Comment:
Gladly.
--
Ticket URL: <https://code.djangoproject.com/ticket/29711#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Przemysław | Owner:
Buczkowski | Przemysław Buczkowski
Type: | Status: assigned
Cleanup/optimization |
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Przemysław Buczkowski):
Just a quick heads-up that I'll be getting down to it, I'm just reading on
[https://docs.djangoproject.com/en/dev/internals/contributing/writing-code
/submitting-patches/ this] and friends in before :)
--
Ticket URL: <https://code.djangoproject.com/ticket/29711#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Przemysław | Owner:
Buczkowski | Przemysław Buczkowski
Type: | Status: assigned
Cleanup/optimization |
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Przemysław Buczkowski):
* has_patch: 0 => 1
Comment:
https://github.com/django/django/pull/10386
--
Ticket URL: <https://code.djangoproject.com/ticket/29711#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: Przemysław | Owner:
Buczkowski | Przemysław Buczkowski
Type: | Status: assigned
Cleanup/optimization |
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/29711#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: Przemysław | Owner:
Buczkowski | Przemysław Buczkowski
Type: | Status: assigned
Cleanup/optimization |
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Comment (by Przemysław Buczkowski):
The patch was modified.
--
Ticket URL: <https://code.djangoproject.com/ticket/29711#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: Przemysław | Owner:
Buczkowski | Przemysław Buczkowski
Cleanup/optimization |
Component: contrib.admin | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"70d0a1ca02f42c0f8984b6234ca0f9d7e354a135" 70d0a1c]:
{{{
#!CommitTicketReference repository=""
revision="70d0a1ca02f42c0f8984b6234ca0f9d7e354a135"
Fixed #29711 -- Added a system check for uniquness of admin actions'
__name__.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29711#comment:11>