Code

Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#17972 closed Bug (fixed)

regression: admin list_filter does not work with to_field

Reported by: kmtracey Owned by: julien
Component: contrib.admin Version: master
Severity: Release blocker Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

This is #9994 exactly, recently reported as having re-appeared. Opening a new ticket rather than re-opening a 3-year-old fixed ticket. Given these models:

class Department(models.Model):
        code = models.CharField(max_length=4, unique=True)
        description = models.CharField(max_length=50, blank=True, null=True)

class Employee(models.Model):
        department = models.ForeignKey(Department, to_field="code")
        name = models.CharField(max_length=100)

And this admin definition:

class EmployeeAdmin(admin.ModelAdmin):
        list_display = ['name', 'department']
        list_filter = ['department']

Create a couple of departments (more than 1 needed to make the list filter show up), and an employee. Then on the employee list page, attempting to filter by a department fetches a url like:

http://localhost:8000/admin/ttt/employee/?department__id__exact=FINC

which results in an exception:

Environment:


Request Method: GET
Request URL: http://localhost:8000/admin/ttt/employee/?department__id__exact=FINC

Django Version: 1.4
Python Version: 2.7.2
Installed Applications:
('django.contrib.auth',
 'django.contrib.contenttypes',
 'django.contrib.sessions',
 'django.contrib.sites',
 'django.contrib.messages',
 'django.contrib.staticfiles',
 'django.contrib.admin',
 'ttt')
Installed Middleware:
('django.middleware.common.CommonMiddleware',
 'django.contrib.sessions.middleware.SessionMiddleware',
 'django.middleware.csrf.CsrfViewMiddleware',
 'django.contrib.auth.middleware.AuthenticationMiddleware',
 'django.contrib.messages.middleware.MessageMiddleware',
 'ttt.middleware.TestMiddleware')


Traceback:
File "C:\Users\kmtracey\django\trunk\django\core\handlers\base.py" in get_response
  111.                         response = callback(request, *callback_args, **callback_kwargs)
File "C:\Users\kmtracey\django\trunk\django\contrib\admin\options.py" in wrapper
  366.                 return self.admin_site.admin_view(view)(*args, **kwargs)
File "C:\Users\kmtracey\django\trunk\django\utils\decorators.py" in _wrapped_view
  91.                     response = view_func(request, *args, **kwargs)
File "C:\Users\kmtracey\django\trunk\django\views\decorators\cache.py" in _wrapped_view_func
  89.         response = view_func(request, *args, **kwargs)
File "C:\Users\kmtracey\django\trunk\django\contrib\admin\sites.py" in inner
  196.             return view(request, *args, **kwargs)
File "C:\Users\kmtracey\django\trunk\django\utils\decorators.py" in _wrapper
  25.             return bound_func(*args, **kwargs)
File "C:\Users\kmtracey\django\trunk\django\utils\decorators.py" in _wrapped_view
  91.                     response = view_func(request, *args, **kwargs)
File "C:\Users\kmtracey\django\trunk\django\utils\decorators.py" in bound_func
  21.                 return func(self, *args2, **kwargs2)
File "C:\Users\kmtracey\django\trunk\django\contrib\admin\options.py" in changelist_view
  1128.                 self)
File "C:\Users\kmtracey\django\trunk\django\contrib\admin\views\main.py" in __init__
  71.         self.query_set = self.get_query_set(request)
File "C:\Users\kmtracey\django\trunk\django\contrib\admin\views\main.py" in get_query_set
  306.             new_qs = filter_spec.queryset(request, qs)
File "C:\Users\kmtracey\django\trunk\django\contrib\admin\filters.py" in queryset
  132.         return queryset.filter(**self.used_parameters)
File "C:\Users\kmtracey\django\trunk\django\db\models\query.py" in filter
  621.         return self._filter_or_exclude(False, *args, **kwargs)
File "C:\Users\kmtracey\django\trunk\django\db\models\query.py" in _filter_or_exclude
  639.             clone.query.add_q(Q(*args, **kwargs))
File "C:\Users\kmtracey\django\trunk\django\db\models\sql\query.py" in add_q
  1250.                             can_reuse=used_aliases, force_having=force_having)
File "C:\Users\kmtracey\django\trunk\django\db\models\sql\query.py" in add_filter
  1185.                 connector)
File "C:\Users\kmtracey\django\trunk\django\db\models\sql\where.py" in add
  69.             value = obj.prepare(lookup_type, value)
File "C:\Users\kmtracey\django\trunk\django\db\models\sql\where.py" in prepare
  320.             return self.field.get_prep_lookup(lookup_type, value)
File "C:\Users\kmtracey\django\trunk\django\db\models\fields\__init__.py" in get_prep_lookup
  310.             return self.get_prep_value(value)
File "C:\Users\kmtracey\django\trunk\django\db\models\fields\__init__.py" in get_prep_value
  537.         return int(value)

Exception Type: ValueError at /admin/ttt/employee/
Exception Value: invalid literal for int() with base 10: 'FINC'

#9994 had been fixed, unfortunately without tests. The above scenario works at r14673. The filtered URL differs from above, it is:

http://localhost:8000/admin/ttt/employee/?department__code__exact=FINC

This correctly filters by department. r14674 introduces a regression, the list filter link href goes back to:

http://localhost:8000/admin/ttt/employee/?department__id__exact=FINC

Attempting to follow that link redirects to:

http://localhost:8000/admin/ttt/employee/?e=1

So something subsequent to r14674 (which I have not tracked down) removes the admin's "I'm confused error" redirect and as a result we now get a server error for this case.

Furthermore attempting to manually "correct" the url to the version that would work results in:

Exception Type: SuspiciousOperation at /admin/ttt/employee/
Exception Value: Filtering by department__code__exact not allowed

So there may be more to fixing this than getting the list filter to generate the old url that did work.

Attachments (2)

17972.listfilter-fk-with-tofield.diff (7.2 KB) - added by julien 2 years ago.
17972.listfilter-fk-with-tofield.1-3-X.diff (7.1 KB) - added by julien 2 years ago.
Same patch for the 1.3.X branch

Download all attachments as: .zip

Change History (7)

comment:1 Changed 2 years ago by julien

  • Owner changed from nobody to julien
  • Triage Stage changed from Unreviewed to Accepted

Thanks for the detailed report, Karen. I'm working on a patch.

Changed 2 years ago by julien

comment:2 Changed 2 years ago by julien

  • Severity changed from Normal to Release blocker

The attached patch fixes the problem in trunk. The same fix would need to be applied to the 1.3.X branch as the regression was initially introduced in 1.3. I'll wait till we decide the new branch policy before pushing those fixes.

Changed 2 years ago by julien

Same patch for the 1.3.X branch

comment:3 Changed 2 years ago by julien

  • Resolution set to fixed
  • Status changed from new to closed

In [17854]:

Fixed #17972 -- Ensured that admin filters on a foreign key respect the to_field attribute. This fixes a regression introduced in [14674] and Django 1.3. Thanks to graveyboat and Karen Tracey for the report.

comment:4 Changed 2 years ago by julien

In [17857]:

[1.3.X] Fixed #17972 -- Ensured that admin filters on a foreign key respect the to_field attribute. This fixes a regression introduced in [14674] and Django 1.3. Thanks to graveyboat and Karen Tracey for the report.

Backport of r17854 from trunk.

comment:5 Changed 2 years ago by julien

In [17858]:

[1.4.X] Fixed #17972 -- Ensured that admin filters on a foreign key respect the to_field attribute. This fixes a regression introduced in [14674] and Django 1.3. Thanks to graveyboat and Karen Tracey for the report.

Backport of r17854 from trunk.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.