Code

Opened 22 months ago

Closed 22 months ago

Last modified 13 months ago

#19002 closed Uncategorized (needsinfo)

Hardcoded url in admin

Reported by: riccardodivirgilio 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 riccardodivirgilio 22 months ago.

Download all attachments as: .zip

Change History (5)

Changed 22 months ago by riccardodivirgilio

comment:1 Changed 22 months ago by riccardodivirgilio

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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

comment:2 Changed 22 months ago by anonymous

  • Component changed from Uncategorized to contrib.admin

comment:3 Changed 22 months ago by ramiro

  • Resolution set to needsinfo
  • Status changed from new to closed

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().

Version 1, edited 22 months ago by ramiro (previous) (next) (diff)

comment:4 Changed 13 months ago by jalpedrinha

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.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.