Add system check to ensure uniqueness of admin actions' __name__.

[Przemysław Buczkowski]

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.

[Carlton Gibson]

The issue ​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.
​Python Docs

The ​`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.

[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.

[Adam 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.

[Przemysław Buczkowski]

@Adam That would be great :)

[Carlton Gibson]

Yes OK. Przemysław, if you'd like to code that up, that would be great.

[Przemysław Buczkowski]

Gladly.

[Przemysław Buczkowski]

Just a quick heads-up that I'll be getting down to it, I'm just reading on ​this and friends in before :)

[Przemysław Buczkowski]

​https://github.com/django/django/pull/10386

[Przemysław Buczkowski]

The patch was modified.

[Tim Graham <timograham@…>]

In 70d0a1c:

Fixed #29711 -- Added a system check for uniquness of admin actions' name.