Opened 16 years ago

Closed 15 years ago

#6470 closed (fixed)

Admin urls should use urlpatterns

Reported by: jdetaeye Owned by: Alex Gaynor
Component: contrib.admin Version: dev
Severity: Keywords: nfa-someday
Cc: schlaber@…, jay.wineinger@…, carl@…, ross@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

If your model has a string as key, there are some admin change pages that are not accessible.

If the primary key of your object ends in "add", "history" or "delete", you can't access the change form any more.

Culprit is the following code in admin\options.py, responsible for parsing the URL:

        if url is None:
            return self.changelist_view(request)
        elif url.endswith('add'):
            return self.add_view(request)
        elif url.endswith('history'):
            return self.history_view(request, unquote(url[:-8]))
        elif url.endswith('delete'):
            return self.delete_view(request, unquote(url[:-7]))
        else:
            return self.change_view(request, unquote(url))

A corrected version is:

        if url is None:
            return self.changelist_view(request)
        elif url == 'add':
            return self.add_view(request)
        elif url.endswith('/history'):
            return self.history_view(request, unquote(url[:-8]))
        elif url.endswith('/delete'):
            return self.delete_view(request, unquote(url[:-7]))
        else:
            return self.change_view(request, unquote(url))

This applies also to the current admin, but in the current only an object with a key equal to "add" will not be accessible (since the url processing is better).

Attachments (9)

nfa-options.diff (849 bytes ) - added by tlpinney 16 years ago.
nfa-urls.diff (16.2 KB ) - added by Alex Gaynor 16 years ago.
Initial work on implementing them using a property, not complete(although more or less functional), tests do not all pass
admin-urlpatterns-6470.diff (19.5 KB ) - added by Michael Newman 16 years ago.
An update to the patch to apply cleanly to r8129
admin-urlpatterns-6470.2.diff (15.5 KB ) - added by Alex Gaynor 15 years ago.
Latest version, full tests pass, and should be backwards compatible, updated docs
admin-urlpatterns.diff (15.9 KB ) - added by Alex Gaynor 15 years ago.
up to date version of the patch
admin-urlpatterns.2.diff (15.9 KB ) - added by Alex Gaynor 15 years ago.
small update to always use raw strings
admin-urlpatterns.3.diff (14.8 KB ) - added by Alex Gaynor 15 years ago.
admin-urlpatterns.4.diff (17.3 KB ) - added by Alex Gaynor 15 years ago.
admin-urlpatterns.5.diff (17.3 KB ) - added by Alex Gaynor 15 years ago.

Download all attachments as: .zip

Change History (25)

comment:1 by Jacob, 16 years ago

Triage Stage: UnreviewedAccepted

comment:2 by Brian Rosner, 16 years ago

Keywords: nfa-blocker added

Agreed. This should be fixed.

by tlpinney, 16 years ago

Attachment: nfa-options.diff added

comment:3 by jkocherhans, 16 years ago

Summary: Admin URLs are not adequately examinedAdmin urls should use urlpatterns

The fix for this ticket is waiting for some changes to the way url dispatch works. There isn't currently a way for the AdminSite class to supply urlpatterns.

comment:4 by Alex Gaynor, 16 years ago

Keywords: nfa-someday added; nfa-blocker removed

This won't(and doesn't need to) happen pre-merge.

by Alex Gaynor, 16 years ago

Attachment: nfa-urls.diff added

Initial work on implementing them using a property, not complete(although more or less functional), tests do not all pass

comment:5 by Alex Gaynor, 16 years ago

Version: newforms-adminSVN

by Michael Newman, 16 years ago

Attachment: admin-urlpatterns-6470.diff added

An update to the patch to apply cleanly to r8129

comment:6 by Bernd, 16 years ago

Cc: schlaber@… added

comment:7 by anonymous, 16 years ago

Cc: jay.wineinger@… added

comment:8 by Carl Meyer, 16 years ago

Cc: carl@… added

by Alex Gaynor, 15 years ago

Latest version, full tests pass, and should be backwards compatible, updated docs

comment:9 by Alex Gaynor, 15 years ago

Owner: changed from nobody to Alex Gaynor

comment:10 by Alex Gaynor, 15 years ago

Has patch: set

comment:11 by Alex Gaynor, 15 years ago

Just saw malcolm's comment on the 1.1 features list(about how multiple AdminSite instances will cause a probelm), and I'm 100% in agreement that that is an issue, however it's not an issue specific to my proposed method for handling this, you need to have a distinct name for each admin site's url, as such the admin site itself needs to have some concept of it's own name.

comment:12 by Ross Poulton, 15 years ago

Cc: ross@… added

by Alex Gaynor, 15 years ago

Attachment: admin-urlpatterns.diff added

up to date version of the patch

by Alex Gaynor, 15 years ago

Attachment: admin-urlpatterns.2.diff added

small update to always use raw strings

comment:13 by Jacob, 15 years ago

(In [9728]) In urlconfs, include() may now be used on an iterable of patterns instead of just a module string. Refs #6470 -- making the admin use a urlconf is much easier with this work done. Thanks, Alex Gaynor.

by Alex Gaynor, 15 years ago

Attachment: admin-urlpatterns.3.diff added

by Alex Gaynor, 15 years ago

Attachment: admin-urlpatterns.4.diff added

by Alex Gaynor, 15 years ago

Attachment: admin-urlpatterns.5.diff added

comment:14 by Jacob, 15 years ago

Resolution: fixed
Status: newclosed

(In [9739]) Fixed #6470: made the admin use a URL resolver.

This *is* backwards compatible, but admin.site.root() has been deprecated. The new style is ('^admin/', include(admin.site.urls)); users will need to update their code to take advantage of the new customizable admin URLs.

Thanks to Alex Gaynor.

comment:15 by anonymous, 15 years ago

Resolution: fixed
Status: closedreopened

There is a typo in contrib/admin/sites.py [9739], isn't it?

@@ -199,7 +199,7 @@
                 name='%sadmin_index' % self.name),
             url(r'^logout/$',
                 wrap(self.logout),
-                name='%sadmin_logout'),
+                name='%sadmin_logout' % self.name),
             url(r'^password_change/$',
                 wrap(self.password_change),
                 name='%sadmin_password_change' % self.name),

comment:16 by dc, 15 years ago

Resolution: fixed
Status: reopenedclosed

There is a separate ticket for this error.

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