Opened 4 years ago

Closed 7 months ago

Last modified 7 months ago

#16311 closed New feature (fixed)

Option to limit list_filter to related_objects of instances of the list

Reported by: Stanislas Guerra <stan@…> Owned by: nobody
Component: contrib.admin Version: master
Severity: Normal Keywords: admin, list_filter, limit_choices_to
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I know there are many snippets and blog entries to do that but it is a common use case to limit the FK/related model choices to the very objects displayed in the list and maybe the core team would consider to implement it directly in the Django admin.

The +10 lines patch I provide allow you to limit the choices for a FK/M2M to the current object list by adding the attribute RELATED_FIELD_filter_related_only=True to your ModelAdmin.

By example :

class Order(models.Model):
    ...
    support = models.ForeignKey(Support)
    parutions = models.ManyToManyField(Parution, through='OrderParution', related_name='order')


class OrderAdmin(admin.ModelAdmin):
    ...
    list_filter = ('support', 'agency', 'parutions')
    parutions_filter_related_only=True
    support_filter_related_only=True


Attachments (4)

patch_list_filter_limit_to_related.diff (2.5 KB) - added by Stanislas Guerra <stan@…> 4 years ago.
Patch for django 1.3
patch_list_filter_limit_to_related2.diff (3.0 KB) - added by Stan <stanislas.guerra@…> 4 years ago.
Patch against trunk with a subclass of RelatedFieldListFilter.
patch_list_filter_limit_to_related_with_tests.diff (6.5 KB) - added by Stanislas Guerra <stan@…> 4 years ago.
Regresssion tests added.
patch16311-1.4.1.diff (7.0 KB) - added by Stanislas <stanislas.guerra@…> 2 years ago.
Patch against Django-1.4.1.

Download all attachments as: .zip

Change History (18)

Changed 4 years ago by Stanislas Guerra <stan@…>

Patch for django 1.3

comment:1 Changed 4 years ago by Matt@…

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Bad choice using a dict as a default argument. If you add something to this, it will be changed in all calls to the method. Use None instead.

comment:2 Changed 4 years ago by russellm

  • Needs documentation set
  • Needs tests set
  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted

Procedurally -- the dict argument is important feedback. The patch also requires tests and documentation.

I'm inclined to say that this shouldn't be a top-level ModelAdmin argument, but a configuration option of the list_filter item itself. The new ListFilter class (in trunk) should provide a better interface to handle this.

comment:3 Changed 4 years ago by schinckel

Yeah, sorry that was a bit terse. Written in bed from iPad, wife complained about tap-tap-tap.

What it should say is:

When you have a literal dict or list as a default argument to a function or method, this generally results in undesired behaviour. If you modify this variable, rather than assign to it, then you change the default value for _every_ call to the function/method.

ie (this is a list, but a dict is the same behaviour):

def foo(bar=[]):
  return bar
 
baz = foo()
baz # =>[]
baz.append(None)
baz # => [None]
foo() # => [None]

You almost always expect that last call to foo() to return [], not [None].

comment:4 Changed 4 years ago by julien

I agree with russellm for the API design; the class you specifically want to look at is contrib.admin.filters.RelatedFieldListFilter. The functionality of that class could be extended to accept configuration options. The developer could then assign a custom filter class to a given field:

class MyModelAdmin(ModelAdmin):
    list_filter = (('my_field', MyRelatedFieldListFilter),)

comment:5 Changed 4 years ago by Stanislas Guerra <stan@…>

Thanks for the feedback.

I have rewritten the patch for the trunk according to your suggestions and the usage is now :

from django.contrib.admin.filters import RelatedOnlyFieldListFilter

class OrderAdmin(admin.ModelAdmin):
    ...
    list_filter = ('support', 'agency', ('parutions', RelatedOnlyFieldListFilter))

Writting some tests now.

By the way, is list_filer a typo in contrib.admin.views.main (line 82) ?

           for list_filer in self.list_filter:
                if callable(list_filer):
                    # This is simply a custom list filter class.
                    spec = list_filer(request, cleaned_params,
                        self.model, self.model_admin)
                else:
                    field_path = None
                    if isinstance(list_filer, (tuple, list)):
                        # This is a custom FieldListFilter class for a given field.
                        field, field_list_filter_class = list_filer
                    else:
                        # This is simply a field name, so use the default
                        # FieldListFilter class that has been registered for
                        # the type of the given field.
                        field, field_list_filter_class = list_filer, FieldListFilter.create

Changed 4 years ago by Stan <stanislas.guerra@…>

Patch against trunk with a subclass of RelatedFieldListFilter.

comment:6 Changed 4 years ago by jezdez

In [16445]:

Fixed typos in admin views wrt list_filter. Refs #16311.

Changed 4 years ago by Stanislas Guerra <stan@…>

Regresssion tests added.

Changed 2 years ago by Stanislas <stanislas.guerra@…>

Patch against Django-1.4.1.

comment:8 Changed 18 months ago by stanislas.guerra@…

  • Version changed from 1.3 to 1.5

comment:9 Changed 9 months ago by stanislas.guerra@…

  • Version changed from 1.5 to master

comment:10 follow-up: Changed 8 months ago by timo

  • Needs tests unset
  • Patch needs improvement unset

The PR lacks documentation.

comment:11 in reply to: ↑ 10 Changed 8 months ago by stanislas.guerra@…

  • Needs documentation unset

Replying to timo:

The PR lacks documentation.

Hi Timo,

PR updated.

comment:12 Changed 7 months ago by Tim Graham <timograham@…>

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

In 98e8da3709e400d549e87c7ff1450a2685c0adcf:

Fixed #16311 -- Added a RelatedOnlyFieldListFilter class in admin.filters.

comment:13 Changed 7 months ago by Tim Graham <timograham@…>

In b50ea73e1e0179c1925167832318a6d0316ee50b:

Fixed two tests in previous commit; refs #16311.

comment:14 Changed 7 months ago by stanislas.guerra@…

That is great, many thanks Tim!

Note: See TracTickets for help on using tickets.
Back to Top