Opened 4 years ago
Last modified 16 months ago
#30386 new Bug
Admin foreign key widgets don't quote keys.
Reported by: | Joshua Goodwin | Owned by: | |
---|---|---|---|
Component: | contrib.admin | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
In django.contrib.admin.ModelAdmin, _changeform_view, _delete_view and history_view and do unquote(object_id):
However, ForeignKeyRawIdWidget and RelatedFieldWidgetWrapper create links to this view without calling quote():
Steps to reproduce:
- Create two models: Topping with a CharField primary key, and Pizza with a ForeignKey to Topping. Register both models with the admin site.
- Create a Topping with the primary key '_40', and a Pizza with that topping
- In the admin site, go the the /change/ page for the Pizza, and click on the 'change' icon for the Topping foreign key, or (if using ForeignKeyRawIdWidget) the link to the Topping '_40'.
- See message 'Topping with ID "@" doesn't exist. Perhaps it was deleted?'.
Attachments (1)
Change History (8)
comment:1 Changed 4 years ago by
Summary: | Admin foreign key widgets don't quote keys → Admin foreign key widgets don't quote keys. |
---|---|
Triage Stage: | Unreviewed → Accepted |
Version: | 2.2 → master |
comment:2 Changed 4 years ago by
Owner: | changed from nobody to zeynel |
---|---|
Status: | new → assigned |
I believe there are two main parts to handle to fix this bug.
- Quoting
obj.pk
value on https://github.com/django/django/blob/89a2216486fa8a0513cbb1d49d2d587d4116c60b/django/contrib/admin/widgets.py#L191 - Quoting object pk(or sending quoted values in context) on client side when user changes the selection on RelatedFieldWidget https://github.com/django/django/blob/89a2216486fa8a0513cbb1d49d2d587d4116c60b/django/contrib/admin/static/admin/js/admin/RelatedObjectLookups.js#L69
comment:3 Changed 4 years ago by
Has patch: | set |
---|
comment:4 Changed 3 years ago by
Triage Stage: | Accepted → Ready for checkin |
---|
comment:5 Changed 3 years ago by
Patch needs improvement: | set |
---|---|
Triage Stage: | Ready for checkin → Accepted |
I still have some issue when saving models, please check attached project and try to add a new pizza with e.g. _40
topping on admin/test_one/pizza/add/
.
Changed 3 years ago by
Attachment: | ticket_30386.zip added |
---|
comment:6 Changed 16 months ago by
I'm dive deeper into this and found that quote(obj.id)
maybe not the best option and should be debated but the presence of a warning message when not isinstance(obj.pk, int)
would be great and will lead people to this thread. Because stackoverflow is full of questions about quoting the link in admin but no one provide refs to this ticket that I found after couple hours of code investigating and testing.
Something like this in django.db.models.fields.RelatedField:
class RelatedField(FieldCacheMixin, Field): def check(self, **kwargs): return [ # ... *self._check_pk_int(), ] def _check_pk_int(self): remote_pk = getattr(self.remote_field, 'pk', None) if isinstance(remote_pk, int): return [] else: return [ checks.Warning( 'Not integer primary key may lead you to unpredictable url in admin when ForeignKeyRawIdWidget is used', hint='Use extra quote() in this relation', obj=self, id='fields.W346', ) ]
comment:7 Changed 16 months ago by
Owner: | zeynel deleted |
---|---|
Status: | assigned → new |
Reproduced at 25b5eea8cdc69a353bb2d22ea2012b09df6c62e4.