Opened 13 years ago
Closed 12 years ago
#17200 closed Bug (fixed)
"View on site" link breaks when quote(object_id) != object_id
Reported by: | David Chandek-Stark | Owned by: | dlanger |
---|---|---|---|
Component: | contrib.admin | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | David Chandek-Stark, ldelaveau@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | no |
Description
The change_view method of django.contrib.admin.options.ModelAdmin returns object_id as passed in. This means that the "View on site" link breaks in cases where the django.contrib.admin.util.quote(object_id) != object_id.
There are two possible solutions:
1) Call django.contrib.admin.util.unquote() on object_id before returning to change_view template;
2) Call django.contrib.admin.util.unquote() on object_id in django.contrib.contenttypes.views.shortcut in this line: obj = content_type.get_object_for_this_type(pk=object_id).
Attached are svn diffs for each.
Attachments (3)
Change History (12)
by , 13 years ago
Attachment: | django.contrib.admin.options.diff added |
---|
by , 13 years ago
Attachment: | django.contrib.contenttypes.views.diff added |
---|
comment:1 by , 13 years ago
Cc: | added |
---|
follow-up: 3 comment:2 by , 13 years ago
Component: | Uncategorized → contrib.admin |
---|---|
Needs tests: | set |
Triage Stage: | Unreviewed → Accepted |
comment:3 by , 13 years ago
Replying to julien:
Solution 1) is the right one. Could you provide a test case? Thanks!
Happy to take a crack at a test case. Since I've not contributed one to Django before, I'm not sure the more helpful way to do this. My first guess is to write a test_viewonsite_link method in the AdminViewStringPrimaryKeyTest class of tests/regressiontests/admin_views/tests.py.
by , 13 years ago
Attachment: | 17200-patch-and-test.diff added |
---|
Initial attempt at patch + unit test. Note that i18n not applied to "View on site" in test.
comment:4 by , 13 years ago
I realized that the new test could just be added to the test_get_change_view() method. As noted, i18n may need to be applied to "View on site" text in test.
comment:5 by , 13 years ago
Needs tests: | unset |
---|
comment:6 by , 13 years ago
I guess that this issue might have been fixed along #18444. Can someone confirm?
comment:7 by , 12 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:8 by , 12 years ago
Cc: | added |
---|
I have the feeling that the test test_shortcut_view_with_escaping
at line 1453 in tests/regressiontests/admin_views/tests.py confirms that the issue is fixed:
def test_shortcut_view_with_escaping(self): "'View on site should' work properly with char fields" model = ModelWithStringPrimaryKey(pk='abc_123') model.save() response = self.client.get('/test_admin/admin/admin_views/modelwithstringprimarykey/%s/' % quote(model.pk)) should_contain = '/%s/" class="viewsitelink">' % model.pk self.assertContains(response, should_contain)
comment:9 by , 12 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
OK, let's consider it fixed.
Solution 1) is the right one. Could you provide a test case? Thanks!