Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#19002 closed Uncategorized (needsinfo)

Hardcoded url in admin

Reported by: Riccardo Di Virgilio Owned by: nobody
Component: contrib.admin Version: 1.4
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

in django.contrib.admin.sites there are a lot or reverse url called with hard coded "admin:"
for example reverse('admin:%s_%s_changelist'

i've got multiple admin sites with different names, and without this patch the link do not works at all...

they must be called with admin_site.name attribute instead... i attached a diff file

Attachments (1)

sites.diff (6.4 KB ) - added by Riccardo Di Virgilio 11 years ago.

Download all attachments as: .zip

Change History (5)

by Riccardo Di Virgilio, 11 years ago

Attachment: sites.diff added

comment:1 by Riccardo Di Virgilio, 11 years ago

sorry i've made a diff from an older version of django... just watch the reverse url part of the diff

comment:2 by anonymous, 11 years ago

Component: Uncategorizedcontrib.admin

comment:3 by Ramiro Morales, 11 years ago

Resolution: needsinfo
Status: newclosed

What do you mean with "the link do not works at all"? Where do these URLs point to? What do you mean with "I've got multiple admin sites with different names"? Are they AdminSite instances?

Referencing the admin docs (https://docs.djangoproject.com/en/1.4/ref/contrib/admin/#multiple-admin-sites-in-the-same-urlconf) and the namespaced URLs docs (https://docs.djangoproject.com/en/1.4/topics/http/urls/#defining-url-namespaces and https://docs.djangoproject.com/en/1.4/topics/http/urls/#url-namespaces) all the admin app instances share the same application namespace ('admin'). That's th reason we are using the 'admin: prefix in the URL names.

What allows to differenciate/choose among the URL namespaces of these app instances when reversing named URLs with reverse() (or equivalent url template tag occurrence) is the value of the current_app argument. In the case of the admin app it's the AdminSite.name value which is the one you can control and which is the one we are passing as current_app in all calls to reverse().

Last edited 11 years ago by Ramiro Morales (previous) (diff)

comment:4 by jalpedrinha, 11 years ago

Actually I believe this is a bad pattern.

When you use url('admin', include(admin.site.urls)), since admin.site is a AdminSite instance and the urls property has the following code:

@property
def urls(self):

return self.get_urls(), self.app_name, self.name

This code makes the namespace dynamic by using self.app_name, but further on the code it uses the admin namespace harcoded, makes no sense to me.
It would be better to use something like reverse(self.app_name + ':%s_%s_changelist')

I've come to this when I was trying to extend the admin behavior. I've created an instance of AdminSite with a different namespace and is impossible to use it like this.
Is there any other reason for this to be hardcoded? AdminSite becomes irrelevant outside of the admin context the way it is right now and the same happens on ModelAdmin.

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