[Django] #32466: Closed ticket #29138 breaks autocomplete for inherited models
Django
-----------------------------------------+------------------------------
Reporter: dlis | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 3.2
Severity: Normal | Keywords: autocomplete
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+------------------------------
Unfortunately, Closed ticket #29138 breaks autocomplete for the case of
inherited models. For example, we have the following code:
{{{
# models.py
from django.db import models
class Tag(models.Model):
pass
class Foo(models.Model):
tags = models.ManyToManyField(Tag)
class Bar(Foo):
pass
# admin.py
from django.contrib import admin
from . import models
@admin.register(models.Foo)
class Foo(admin.ModelAdmin):
autocomplete_fields = ('tags',)
@admin.register(models.Bar)
class Bar(admin.ModelAdmin):
autocomplete_fields = ('tags',)
}}}
Now, autocomplete for `admin.Foo` will work but not for `admin.Bar`
because `django.contrib.admin.widgets.AutocompleteMixin.optgroups()`
calculates a wrong value of a variable `to_field_name`, namely `foo_ptr`
instead of `id`, whereupon following look up at
`self.choices.queryset.using(self.db).filter(**{'%s__in' % to_field_name:
selected_choices})` raises an exception because `models.Tag` does not have
`foo_ptr`.
--
Ticket URL: <https://code.djangoproject.com/ticket/32466>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: dlis | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 3.2
Keywords: autocomplete | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Simon Charette):
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
Comment:
Thanks for testing out the Django 3.2 beta.
--
Ticket URL: <https://code.djangoproject.com/ticket/32466#comment:1>
Django
Reporter: dlis | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 3.2
Keywords: autocomplete | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak):
Regression in 3071660acfbdf4b5c59457c8e9dc345d5e8894c5.
--
Ticket URL: <https://code.djangoproject.com/ticket/32466#comment:2>
Django
Reporter: dlis | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 3.2
Keywords: autocomplete | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* cc: Johannes Maron (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/32466#comment:3>
Django
Reporter: dlis | Owner: Johannes
| Maron
Type: Bug | Status: assigned
Component: contrib.admin | Version: 3.2
Severity: Release blocker | Resolution:
Keywords: autocomplete | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Johannes Maron):
* owner: nobody => Johannes Maron
* status: new => assigned
Comment:
Thanks, good find. I am on it.
--
Ticket URL: <https://code.djangoproject.com/ticket/32466#comment:4>
Django
Reporter: dlis | Owner: Johannes
| Maron
Component: contrib.admin | Version: 3.2
Severity: Release blocker | Resolution: invalid
Keywords: autocomplete | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Johannes Maron):
* status: assigned => closed
* resolution: => invalid
Comment:
Hi there,
I tested the reported error first on my projects, where I use MTI and then
on the example (which is incomplete). Both didn't yield the error or the
`to_field_value` that was described. That is also unlikely, since the
`to_field_value` is correctly based on the remote model. Therefore, I must
conclude this issue is invalid. Feel free to reopen it, preferably with a
valid example – I have been wrong in the past.
Should the problem just be in your projects setup, please feel free to
reach out. I am happy to help.
Best,
Joe
--
Ticket URL: <https://code.djangoproject.com/ticket/32466#comment:5>
Django
Reporter: dlis | Owner: Johannes
| Maron
Component: contrib.admin | Version: 3.2
Keywords: autocomplete | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by dlis):
* status: closed => new
* resolution: invalid =>
Comment:
Unfortunately, previous code was a little bit abstract. Here you can find
a correct application – https://bitbucket.org/dlis/buggy/. Please, try
open `buggy.admin.Bar` in admin. The application works perfectly on Django
3.1.x but raises the following error on Django 3.2.x:
"FieldError at /admin/buggy/bar/add/
Cannot resolve keyword 'foo_ptr' into field. Choices are: bar, id"
--
Ticket URL: <https://code.djangoproject.com/ticket/32466#comment:6>
Django
Reporter: dlis | Owner: Johannes
| Maron
Component: contrib.admin | Version: 3.2
Keywords: autocomplete | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Johannes Maron):
Thanks for the code, that usually really helps. I will review it tomorrow,
it's family time in my timezone ;)
--
Ticket URL: <https://code.djangoproject.com/ticket/32466#comment:7>
Django
Reporter: dlis | Owner: Johannes
| Maron
Component: contrib.admin | Version: 3.2
Keywords: autocomplete | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Johannes Maron):
* stage: Accepted => Unreviewed
--
Ticket URL: <https://code.djangoproject.com/ticket/32466#comment:8>
Django
Reporter: dlis | Owner: Johannes
| Maron
Component: contrib.admin | Version: 3.2
Keywords: autocomplete | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Johannes Maron):
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/32466#comment:9>
Django
Reporter: dlis | Owner: Johannes
| Maron
Component: contrib.admin | Version: 3.2
Keywords: autocomplete | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Johannes Maron):
@dlis, thanks again for reporting this issue. I could very if and just now
created a patch. If you were so kind as to review the patch, that would
really help. Once you are done, please consider moving triage to "ready
for checkin" so a fellow can clear the release blocker.
--
Ticket URL: <https://code.djangoproject.com/ticket/32466#comment:10>
Django
Reporter: dlis | Owner: Johannes
| Maron
Component: contrib.admin | Version: 3.2
Keywords: autocomplete | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by dlis):
Sure! I replaced `Django==3.2b1` with
`git+git://github.com/codingjoe/django.git@711bbfde3e275a857a31cbb56dcf3e362ce7dc60`
in my requirements.txt. Inherited models in admin open without raising the
exception "FieldError" but, unfortunately, autocompleting stopped working.
Log shoes that requests to /admin/autocomplete/ raise the following:
{{{
development_1 | Forbidden (Permission denied): /admin/autocomplete/
development_1 | Traceback (most recent call last):
development_1 | File "/usr/local/lib/python3.9/site-
packages/django/core/handlers/exception.py", line 47, in inner
development_1 | response = get_response(request)
development_1 | File "/usr/local/lib/python3.9/site-
packages/django/core/handlers/base.py", line 181, in _get_response
development_1 | response = wrapped_callback(request, *callback_args,
**callback_kwargs)
development_1 | File "/usr/local/lib/python3.9/site-
packages/django/contrib/admin/sites.py", line 250, in wrapper
development_1 | return self.admin_view(view, cacheable)(*args,
**kwargs)
development_1 | File "/usr/local/lib/python3.9/site-
packages/django/utils/decorators.py", line 130, in _wrapped_view
development_1 | response = view_func(request, *args, **kwargs)
development_1 | File "/usr/local/lib/python3.9/site-
packages/django/views/decorators/cache.py", line 44, in _wrapped_view_func
development_1 | response = view_func(request, *args, **kwargs)
development_1 | File "/usr/local/lib/python3.9/site-
packages/django/contrib/admin/sites.py", line 232, in inner
development_1 | return view(request, *args, **kwargs)
development_1 | File "/usr/local/lib/python3.9/site-
packages/django/contrib/admin/sites.py", line 417, in autocomplete_view
development_1 | return
AutocompleteJsonView.as_view(admin_site=self)(request)
development_1 | File "/usr/local/lib/python3.9/site-
packages/django/views/generic/base.py", line 70, in view
development_1 | return self.dispatch(request, *args, **kwargs)
development_1 | File "/usr/local/lib/python3.9/site-
packages/django/views/generic/base.py", line 98, in dispatch
development_1 | return handler(request, *args, **kwargs)
development_1 | File "/usr/local/lib/python3.9/site-
packages/django/contrib/admin/views/autocomplete.py", line 20, in get
development_1 | self.term, self.model_admin, self.source_field,
to_field_name = self.process_request(request)
development_1 | File "/usr/local/lib/python3.9/site-
packages/django/contrib/admin/views/autocomplete.py", line 95, in
process_request
development_1 | raise PermissionDenied
development_1 | django.core.exceptions.PermissionDenied
development_1 | [22/Feb/2021 17:57:40] "GET
/admin/autocomplete/?app_label=foo&model_name=bar&field_name=baz HTTP/1.1"
403 135
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32466#comment:11>
Django
Reporter: dlis | Owner: Johannes
| Maron
Component: contrib.admin | Version: 3.2
Keywords: autocomplete | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by dlis):
Yes, I've just checked that replacing `to_field_name =
getattr(source_field.remote_field, 'field_name', 'pk')` with
`to_field_name = getattr(source_field.remote_field, 'field_name', 'id')`
solves the autocompleting, but it would be better to get the name of pk-
field for the case of custom pk.
--
Ticket URL: <https://code.djangoproject.com/ticket/32466#comment:12>
Django
Reporter: dlis | Owner: Johannes
| Maron
Component: contrib.admin | Version: 3.2
Keywords: autocomplete | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by Johannes Maron):
* needs_better_patch: 0 => 1
* has_patch: 0 => 1
* needs_tests: 0 => 1
Comment:
Thanks for the quick review. In that case, I need to write more tests, to
avoid regression and also improve the patch. I'll get on it tomorrow.
--
Ticket URL: <https://code.djangoproject.com/ticket/32466#comment:13>
Django
-------------------------------------+-------------------------------------
Reporter: Dlis | Owner: Johannes
| Maron
Type: Bug | Status: new
Component: contrib.admin | Version: 3.2
Keywords: autocomplete | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Johannes Maron):
* needs_better_patch: 1 => 0
* needs_tests: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/32466#comment:14>
Django
-------------------------------------+-------------------------------------
Reporter: Dlis | Owner: Johannes
| Maron
Component: contrib.admin | Version: 3.2
Keywords: autocomplete | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Johannes Maron):
Would you mind giving it another go? I added a test that should recreate
your test scenario, but better safe than sorry.
--
Ticket URL: <https://code.djangoproject.com/ticket/32466#comment:15>
Django
-------------------------------------+-------------------------------------
Reporter: Dlis | Owner: Johannes
| Maron
Component: contrib.admin | Version: 3.2
Keywords: autocomplete | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/32466#comment:16>
Django
-------------------------------------+-------------------------------------
Reporter: Dlis | Owner: Johannes
| Maron
Component: contrib.admin | Version: 3.2
Keywords: autocomplete | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* needs_tests: 0 => 1
* stage: Ready for checkin => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/32466#comment:17>
Django
-------------------------------------+-------------------------------------
Reporter: Dlis | Owner: Johannes
| Maron
Component: contrib.admin | Version: 3.2
Keywords: autocomplete | Triage Stage: Ready for
| checkin
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/32466#comment:18>
Django
-------------------------------------+-------------------------------------
Reporter: Dlis | Owner: Johannes
| Maron
Component: contrib.admin | Version: 3.2
Severity: Release blocker | Resolution: fixed
Keywords: autocomplete | Triage Stage: Ready for
| checkin
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson <carlton.gibson@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"03d0f12c823239812da21e5180aaa74dc6fd146e" 03d0f12c]:
{{{
#!CommitTicketReference repository=""
revision="03d0f12c823239812da21e5180aaa74dc6fd146e"
Fixed #32466 -- Corrected autocomplete to_field resolution for complex
cases.
In MTI or ForeignKey as primary key cases, it is required to fetch the
attname
from the field instance on the remote model in order to reliably resolve
the
to_field_name.
Co-authored-by: Johannes Maron <in...@johanneshoppe.com>
Co-authored-by: Mariusz Felisiak <felisiak...@gmail.com>
Co-authored-by: Carlton Gibson <carlton...@noumenal.es>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32466#comment:19>
Django
-------------------------------------+-------------------------------------
Reporter: Dlis | Owner: Johannes
| Maron
Type: Bug | Status: closed
Severity: Release blocker | Resolution: fixed
Keywords: autocomplete | Triage Stage: Ready for
| checkin
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Carlton Gibson <carlton.gibson@…>):
In [changeset:"a8fef6daaf75cd24262d973b154fb6580efd99a4" a8fef6da]:
{{{
#!CommitTicketReference repository=""
revision="a8fef6daaf75cd24262d973b154fb6580efd99a4"
[3.2.x] Fixed #32466 -- Corrected autocomplete to_field resolution for
complex cases.
In MTI or ForeignKey as primary key cases, it is required to fetch the
attname
from the field instance on the remote model in order to reliably resolve
the
to_field_name.
Backport of ceb4b9ee68dffc6ab0398886f1758f15f037c472 from main Backport of
03d0f12c823239812da21e5180aaa74dc6fd146e from main
Co-authored-by: Johannes Maron <in...@johanneshoppe.com>
Co-authored-by: Mariusz Felisiak <felisiak...@gmail.com>
Co-authored-by: Carlton Gibson <carlton...@noumenal.es>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32466#comment:20>