Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#19414 closed New feature (fixed)

Add a "register" class decorator for admin.

Reported by: Stavros Korokithakis Owned by: BHold
Component: contrib.admin Version: 1.4
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The syntax for registering an admin class is a bit unpythonic, so I propose that the admin.site.register method should also have the capability of being used as a decorator.

A simple implementation is below:

def register_admin(model):
    def wrapper(klass):
        admin.site.register(model, klass)
        return klass
    return wrapper

I feel that

@register(MyModel)
class AdminClass:
    pass

is more pythonic than

class AdminClass:
    pass
register(MyModel, AdminClass)

which just screams "decorator".

Change History (12)

comment:1 by Julien Phalip, 11 years ago

Component: Uncategorizedcontrib.admin
Triage Stage: UnreviewedAccepted

comment:2 by BHold, 11 years ago

Owner: changed from nobody to BHold

comment:3 by BHold, 11 years ago

Has patch: set

Added pull request: https://github.com/django/django/pull/599

I added a decorator that registers Model/ModelAdmin pairs with AdminSites. I mostly followed how the AdminSite.register method currently does this.

Passes all tests, including those I added.

This is only my 2nd time contributing to the project, and first time writing Sphinx docs, so please let me know if I didn't follow any conventions!

comment:4 by Stavros Korokithakis, 11 years ago

Why not just reuse admin.site.register? It looks like you reimplemented the method?

in reply to:  4 comment:5 by BHold, 11 years ago

Replying to stavros:

Why not just reuse admin.site.register? It looks like you reimplemented the method?

The decorator function should behave a little differently than the normal register method of the Site. For one, it should accept a 'site' argument so that a user can use it with a custom Site model. It also should be able to accept multiple models as arguments that will all be linked to the decorated ModelAdmin class in the Site's registry.

Last edited 11 years ago by BHold (previous) (diff)

comment:6 by Stavros Korokithakis, 11 years ago

Sure, but you don't have to reimplement the whole admin.site.register method, you can just wrap it and add the extra functionality needed.

in reply to:  6 comment:7 by BHold, 11 years ago

Replying to stavros:

Sure, but you don't have to reimplement the whole admin.site.register method, you can just wrap it and add the extra functionality needed.

True, I pushed up a new commit that does just that! What do you think?

comment:8 by Stavros Korokithakis, 11 years ago

Much better! I would not have added the "Unsupported arguments" check if I wrote it, just because someone might want to construct a dict with extra stuff and pass it (plus, ignoring kwargs isn't bad for you), but otherwise it looks great to me.

Can someone official chime in? This pull request looks very good.

Last edited 11 years ago by Stavros Korokithakis (previous) (diff)

in reply to:  8 comment:9 by BHold, 11 years ago

Replying to stavros:

Much better! I would not have added the "Unsupported arguments" check if I wrote it, just because someone might want to construct a dict with extra stuff and pass it (plus, ignoring kwargs isn't bad for you), but otherwise it looks great to me.

Can someone official chime in? This pull request looks very good.

Yeah, perhaps that Unsupported Arguments check was unneeded, I removed it.

comment:10 by Rafal Stozek, 11 years ago

I think you should also import this decorator in django/contrib/admin/__init__.py.

comment:11 by Tim Graham <timograham@…>, 11 years ago

Resolution: fixed
Status: newclosed

In 98514849dce07acfaa224a90a784bba9d97249e5:

Fixed #19414 -- Added admin registration decorator

Thanks stavros for the suggestion.

comment:12 by Tim Graham <timograham@…>, 11 years ago

In e23de9e350d716c4d9ebe0b27c9f2752fe1aa543:

Fixed typo in exception message; refs #19414

Thanks Alexey Boriskin for the report.

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