Opened 8 years ago

Closed 7 years ago

Last modified 5 years ago

#10223 closed (wontfix)

Admin has a bug for primary keys that contain slashes

Reported by: HuCy Owned by: nobody
Component: contrib.admin Version: master
Severity: Keywords:
Cc: matthew@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: UI/UX:

Description

If your Model's primary key contains slashes,
the "View on Site" button does not work, because the wrong number of arguments are
passed to django.contrib.contenttypes.views.shortcut.

The attached patch fixes this issue by splitting to a maximum number of 3 elements.

Patch done by Christoph Borgolte and Christian Klein

Attachments (4)

admin-pk.diff (630 bytes) - added by HuCy 8 years ago.
10223-test.diff (1.4 KB) - added by mmarshall 7 years ago.
10223-charfield_pk_with_slashes_view_on_site-1.0.x.diff (2.2 KB) - added by Ramiro Morales 7 years ago.
Patch from HuCy and tests from mmarshall adapted for the 1.0.x branch as of r10618
10223-charfield_pk_with_slashes_view_on_site.diff (3.0 KB) - added by Ramiro Morales 7 years ago.
Patch from HuCy? and tests from mmarshall, modified to actually test things with the legacy AdminSite?.root view

Download all attachments as: .zip

Change History (14)

Changed 8 years ago by HuCy

Attachment: admin-pk.diff added

comment:1 Changed 8 years ago by Jacob

milestone: 1.1
Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedAccepted

comment:2 Changed 7 years ago by Jacob

Needs tests: set

Changed 7 years ago by mmarshall

Attachment: 10223-test.diff added

comment:3 Changed 7 years ago by mmarshall

The problem only applies to the AdminSite.root, which is deprecated. The new method using include(admin.site.urls) uses an re with .+

I've submitted a test, but it tests against the new method.

comment:4 Changed 7 years ago by mmarshall

Needs tests: unset

comment:5 Changed 7 years ago by mmarshall

Cc: matthew@… added

comment:6 Changed 7 years ago by Ramiro Morales

Patch needs improvement: set

Changed 7 years ago by Ramiro Morales

Patch from HuCy and tests from mmarshall adapted for the 1.0.x branch as of r10618

comment:7 Changed 7 years ago by Ramiro Morales

Patch needs improvement: unset

Changed 7 years ago by Ramiro Morales

Patch from HuCy? and tests from mmarshall, modified to actually test things with the legacy AdminSite?.root view

comment:8 Changed 7 years ago by Russell Keith-Magee

Patch needs improvement: set

The problem appears to be much deeper than just the link on site. Consider a model with a CharField primary key, and an instance with a key value of foo/bar/whiz, using the include() style of admin URL declaration:

  1. If you use the admin to add an object with a slash in the primary key, and use the "save and continue editing" button, you get redirected to /admin/app/model/foo/bar/whiz, but if you use save, then use the admin to go back into the object definition, the url is /admin/app/model/foo_2Fbar_2Fwhiz/
  2. If you are on /admin/app/model/foo/bar/whiz, the breadcrumb links are wrong - they slice the path at the wrong slash.
  3. If you are on /admin/app/model/foo/bar/whiz, the view on site links to /admin/myapp/charpkmodel/r/28/foo/bar/whiz/, not /admin/r/28/foo/bar/whiz/
  4. If you are on /admin/app/model/foo_2Fbar_2Fwhiz/, the link on site is to /admin/r/28/foo_2Fbar_2Fwhiz/ - which raises a 404, since there is no object with PK foo_2Fbar_2Fwhiz

I'm guessing there will be other problems.

comment:9 Changed 7 years ago by Jacob

Resolution: wontfix
Status: newclosed

Since this only happens with the legacy version of admin URLs, I don't see the point in fixing it. If you have this bug, just don't use AdminSite.root.

comment:10 Changed 5 years ago by Jacob

milestone: 1.1

Milestone 1.1 deleted

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