Opened 14 years ago
Closed 13 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 , 14 years ago
| Attachment: | django.contrib.admin.options.diff added |
|---|
by , 14 years ago
| Attachment: | django.contrib.contenttypes.views.diff added |
|---|
comment:1 by , 14 years ago
| Cc: | added |
|---|
follow-up: 3 comment:2 by , 14 years ago
| Component: | Uncategorized → contrib.admin |
|---|---|
| Needs tests: | set |
| Triage Stage: | Unreviewed → Accepted |
comment:3 by , 14 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 , 14 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 , 14 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 , 13 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:8 by , 13 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 , 13 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!