[Django] #26184: Unable to use custom lookups or transforms in admin `search_fields`
Django
---------------------------------+--------------------
Reporter: BertrandBordage | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+--------------------
When you define a `ModelAdmin` like this:
{{{#!python
class MyAdmin(ModelAdmin):
search_fields = ['my_field__unaccent']
}}}
a `FieldDoesNotExist` is raised when you try to search something.
This is because the admin fails to identify `__unaccent` as a valid lookup
because `django.contrib.admin.utils.lookup_needs_distinct` relies on two
kludges:
- it assumes that the last part of the lookup expression is always a
lookup (`iexact`, `icontains`, `search`, etc), and that all other parts
are fields. In the unaccent scenario, the penultimate part is a transform
since the admin generates this expression: `myfield__unaccent__icontains`
- it only checks that lookups are in
`django.db.models.sql.constants.QUERY_TERMS`. But **`QUERY_TERMS` is no
longer used and should be removed**, it only contains "classic" lookups
The solution is to use
[https://docs.djangoproject.com/en/1.9/ref/models/lookups/#registration-
api the new registration API] instead, with the `get_lookup` and
`get_transform` methods.
A better solution would be to make `search_fields` more consistent with
the rest of Django by removing the `^`, `=`, & `@` shortcuts. If not,
providing them on the whole ORM would be consistent. In my opinion, they
should be removed, they complicate Django while limiting its possibilities
(what if someone wants `exact` instead of `iexact`?). If someone has a lot
of fields with the same transforms/lookups, this can be done:
{{{#!python
class BookAdmin(ModelAdmin):
search_fields = ('title', 'subtitle', 'author_first_name',
'author_last_name', 'publisher')
search_fields = [le + '__unaccent__icontains' for le in search_fields]
}}}
or even this if you need it often:
{{{#!python
class SearchMixin:
def get_search_fields(self, request):
return [le + '__unaccent__icontains' for le in self.search_fields]
class BookAdmin(SearchMixin, ModelAdmin):
search_fields = ('title', 'subtitle', 'author_first_name',
'author_last_name', 'publisher')
}}}
I’m working on an implementation for one of my projects, I’ll submit it
when done.
--
Ticket URL: <https://code.djangoproject.com/ticket/26184>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: BertrandBordage | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by BertrandBordage):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
Here’s a workaround that could work as a solution:
[https://github.com/dezede/dezede/commit/5e8be29ef4f24ea016a78a5d78085e222bca79b3
dezede/dezede@5e8be29].
I kept `get_search_results` as is to leave the `^`, `=`, & `@`
functionalities unchanged, but it should of course be changed, as
suggested above.
--
Ticket URL: <https://code.djangoproject.com/ticket/26184#comment:1>
Django
Reporter: BertrandBordage | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/26184#comment:2>
Django
Reporter: BertrandBordage | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
Todo when implementing: check if it can solve #26001.
--
Ticket URL: <https://code.djangoproject.com/ticket/26184#comment:3>
Django
Reporter: BertrandBordage | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by berkerpeksag):
* cc: berker.peksag@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/26184#comment:4>
Django
Reporter: Bertrand Bordage | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Camilo Nova):
* cc: camilo.nova@… (added)
Comment:
@Bertrand Bordage Did you work on that implementation?
--
Ticket URL: <https://code.djangoproject.com/ticket/26184#comment:5>
Django
Reporter: Bertrand Bordage | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Bertrand Bordage):
@Camilo Nova: No, I almost never use Django admin now, I switched to
Wagtail.
--
Ticket URL: <https://code.djangoproject.com/ticket/26184#comment:6>
Django
Reporter: Bertrand Bordage | Owner: Krzysztof
| Nazarewski
Type: Bug | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Krzysztof Nazarewski):
* owner: nobody => Krzysztof Nazarewski
* status: new => assigned
* has_patch: 0 => 1
Comment:
Fixed in https://github.com/django/django/pull/8704
--
Ticket URL: <https://code.djangoproject.com/ticket/26184#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Bertrand Bordage | Owner: Krzysztof
| Nazarewski
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* needs_better_patch: 0 => 1
* type: Bug => New feature
--
Ticket URL: <https://code.djangoproject.com/ticket/26184#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: Bertrand Bordage | Owner: Krzysztof
| Nazarewski
Type: New feature | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* needs_better_patch: 1 => 0
Comment:
New [https://github.com/django/django/pull/9357 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/26184#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: Bertrand Bordage | Owner: Krzysztof
| Nazarewski
Component: contrib.admin | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"244cc401559e924355cf943b6b8e66ccf2f6da3a" 244cc401]:
{{{
#!CommitTicketReference repository=""
revision="244cc401559e924355cf943b6b8e66ccf2f6da3a"
Fixed #26184 -- Allowed using any lookups in ModelAdmin.search_fields.
Thanks Krzysztof Nazarewski for the initial patch.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26184#comment:10>