Opened 6 years ago

Closed 6 years ago

Last modified 4 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 6 years ago.
10223-test.diff (1.4 KB) - added by mmarshall 6 years ago.
10223-charfield_pk_with_slashes_view_on_site-1.0.x.diff (2.2 KB) - added by ramiro 6 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 6 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 6 years ago by HuCy

comment:1 Changed 6 years ago by jacob

  • milestone set to 1.1
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 6 years ago by jacob

  • Needs tests set

Changed 6 years ago by mmarshall

comment:3 Changed 6 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 6 years ago by mmarshall

  • Needs tests unset

comment:5 Changed 6 years ago by mmarshall

  • Cc matthew@… added

comment:6 Changed 6 years ago by ramiro

  • Patch needs improvement set

Changed 6 years ago by ramiro

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

comment:7 Changed 6 years ago by ramiro

  • Patch needs improvement unset

Changed 6 years ago by ramiro

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

comment:8 Changed 6 years ago by russellm

  • 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 6 years ago by jacob

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

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 4 years ago by jacob

  • milestone 1.1 deleted

Milestone 1.1 deleted

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