Code

Opened 7 years ago

Closed 5 years ago

Last modified 5 years ago

#3096 closed defect (fixed)

Filtering choices in admin list view don't respond to limit_choices_to parameter

Reported by: Archatas (aidas.bendoraitis at gmail.com) Owned by: nobody
Component: contrib.admin Version: newforms-admin
Severity: normal Keywords: nfa-someday yandex-sprint ep2008
Cc: myer0052@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description (last modified by gwilson)

Administration list view shows choices in filters not responding to the limit_choices_to field for the field.

For example, if we have

class Color(models.Model):
    value=models.CharField(...)
    warm=models.BooleanField(...)

class Thing(models.Model):
    title=models.CharField(...)
    color=models.ForeignKey(Color, limit_choices_to={'warm': True})
    class Admin:
        list_filter = ('color',)

We will get all the choices of Color objects listed in the Filter/By Color section instead of getting only those, which are warm.

To fix this, the line

self.lookup_choices = f.rel.to._default_manager.all()

at class RelatedFilterSpec(FilterSpec) in the file django/contrib/admin/filterspecs.py has to be changed to

if f.rel.limit_choices_to:
    self.lookup_choices = f.rel.to._default_manager.filter(**f.rel.limit_choices_to)
else:
    self.lookup_choices = f.rel.to._default_manager.all()

Attachments (4)

3096-filterspecs.patch (786 bytes) - added by Robert Myers <myer0052@…> 7 years ago.
filterspecs limit_choices_to patch
3096-filterspecs-2.patch (606 bytes) - added by Will Hardy 7 years ago.
Take limit_choices_to into account for list_filter in Admin
3096-filterspecs-newforms-admin.diff (669 bytes) - added by Will Hardy 7 years ago.
newforms-admin version of patch to take limit_choices_to into account for list_filter in Admin
3096-get_choices.diff (1.2 KB) - added by Ivan Sagalaev <Maniac@…> 6 years ago.
Patch using get_choices

Download all attachments as: .zip

Change History (20)

comment:1 Changed 7 years ago by anonymous

  • milestone Version 1.0 deleted

Milestone Version 1.0 deleted

comment:2 Changed 7 years ago by Robert Myers <myer0052@…>

  • Cc myer0052@… added

comment:3 Changed 7 years ago by Gary Wilson <gary.wilson@…>

  • Has patch set
  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted

This seems like the logical thing to do. If someone disagrees, feel free to set back to decision needed. Can someone provide a real patch please.

Changed 7 years ago by Robert Myers <myer0052@…>

filterspecs limit_choices_to patch

comment:4 Changed 7 years ago by Robert Myers <myer0052@…>

  • Patch needs improvement unset

I tested out the patch and it works. Not sure it this should have a unit test or not it seems really simple and none of the other filters are currently being tested.

comment:5 Changed 7 years ago by Archatas

I found one case, where it fails: that is when Q() objects instead of dictionaries are used to limit the choices. Here is the simple fix:

if f.rel.limit_choices_to:
    if type(f.rel.limit_choices_to).__name__ == "dict":
        self.lookup_choices = f.rel.to._default_manager.filter(**f.rel.limit_choices_to)
    else:
        self.lookup_choices = f.rel.to._default_manager.filter(f.rel.limit_choices_to)
else:
    self.lookup_choices = f.rel.to._default_manager.all()

comment:6 Changed 7 years ago by rob.slotboom at wanadoo.nl

The patch by Archatas doesn't work with the following limit_choices_to:

limit_choices_to = {'parentisnull': False}

Changed 7 years ago by Will Hardy

Take limit_choices_to into account for list_filter in Admin

comment:7 Changed 7 years ago by anonymous

The same thing is done in the get_choices() method in /django/db/models/fields/__init__.py, ideally we would reuse this, but because I don't want to rewrite a function I don't completely understand, I have just copied the approach taken in get_choices().

The patch I just attached should work exactly the same way as get_choices().

comment:8 Changed 7 years ago by brosner

  • Patch needs improvement set
  • Summary changed from Filtering choices in admin list view don't respond to limit_choices_to parameter to [newforms-admin] - Filtering choices in admin list view don't respond to limit_choices_to parameter
  • Version changed from SVN to newforms-admin

The admin code in trunk won't be updated. Moving this to newforms-admin. It does look to still be a problem in that branch. Please submit patches against newforms-admin.

comment:9 Changed 7 years ago by mtredinnick

  • Summary changed from [newforms-admin] - Filtering choices in admin list view don't respond to limit_choices_to parameter to Filtering choices in admin list view don't respond to limit_choices_to parameter

(Please don't change ticket titles to make them less readable by chewing up space with an unnecessary prefix. We already have the version field and patch checkboxes and the like to differentiate between things.)

Changed 7 years ago by Will Hardy

newforms-admin version of patch to take limit_choices_to into account for list_filter in Admin

comment:10 Changed 6 years ago by gwilson

  • Description modified (diff)

formatted ticket description

comment:11 Changed 6 years ago by Karen Tracey <kmtracey@…>

  • Keywords nfa-someday added

Problem originally reported against old admin, should not block merge of newforms-admin.

comment:12 Changed 6 years ago by poelzi

doesn't matter as the problem still exists and is major as it may render admin pages unusable.

Changed 6 years ago by Ivan Sagalaev <Maniac@…>

Patch using get_choices

comment:13 Changed 6 years ago by Ivan Sagalaev <Maniac@…>

  • Keywords yandex-sprint added
  • Patch needs improvement unset

As suggested by comment:7 new patch uses field's get_choices to construct a list of (id, title) pairs. Though a bit bigger (3 lines instead of 1) this looks cleaner to me.

comment:14 Changed 6 years ago by Honza_Kral

  • Keywords ep2008 added
  • Triage Stage changed from Accepted to Ready for checkin

comment:15 Changed 5 years ago by kmtracey

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

(In [9241]) Fixed #3096 -- Make admin list_filters respect limit_choices_to.

comment:16 Changed 5 years ago by kmtracey

(In [9242]) [1.0.X] Fixed #3096 -- Make admin list_filters respect limit_choices_to.

Backport of [9241] 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.