Opened 17 years ago

Closed 17 years ago

Last modified 16 years ago

#4718 closed (wontfix)

Add get_admin_url() to Model

Reported by: cbrand@… Owned by: Adrian Holovaty
Component: Core (Other) Version: dev
Severity: Keywords:
Cc: brooks.travis@… Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

From a discussion on django-users, I came up with this after contributions from two other people, one who originally got the code from elsewhere :

    def get_admin_url(self):
        pk = getattr(self, self._meta.pk.attname)
        return ('django.contrib.admin.views.main.change_stage',
                (self._meta.app_label,
                 self._meta.module_name,
                 pk))
    get_admin_url = models.permalink(get_admin_url)

There's nothing there that couldn't be done in the base Model class, and the fact that there were a number of people who'd seen a need for it indicates that it should at least be considered for inclusion int he base Model class.

On the downside, there's the question of what to do if the pk is None.
Also, admin is in contrib, which means that it could be omitted, but I wouldn't expect people who aren't using the admin to call the new method.

Change History (4)

comment:1 by Adrian Holovaty, 17 years ago

Resolution: wontfix
Status: newclosed

It would be tight coupling to include this as a default model method. The model layer knows nothing about views, or templates, or the admin site.

I'd suggest adding this to the Django wiki, or djangosnippets.org.

comment:2 by cbrand@…, 17 years ago

I guess I don't see a big difference between the presence of this method and the presence of the Admin class within the model (although I recognise that isn't in the base Model class).

Seems a great shame to have this block of code repeated in (potentially) every class derived from Model when it could be in the base class.

I guess separation of MVC outranks DRY in this case... ah well :-)

comment:3 by James Bennett, 17 years ago

Well, keep in mind that the Admin class is in the process of being moved out of model definitions -- once the newforms-admin branch hits trunk, you won't define admin options inside the model class anymore. So that's not really a good example to use as a counterargument ;)

comment:4 by brooks.travis@…, 16 years ago

Cc: brooks.travis@… added

While the method presented here isn't the way to handle this, would there be a way to register the method on the model via it's ModelAdmin class? That would even allow the developer to provide a customized version of the method for models they need to have one. Just a thought. Maybe we could reopen this ticket against newforms-admin with this proposal rather than the original?

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