Opened 10 years ago

Closed 10 years ago

#23055 closed Bug (fixed)

Filters don't use ModelAdmin get_queryset()

Reported by: Ramiro Morales Owned by: Ramiro Morales
Component: contrib.admin Version: dev
Severity: Normal Keywords: admin filters list_filter multi-db get_queryset modeladmin
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Ramiro Morales)

If one is using a ModelAdmin with a custom get_queryset() method (e.g. as described in https://docs.djangoproject.com/en/1.7/topics/db/multi-db/#exposing-multiple-databases-in-django-s-admin-interface to handle routing of models hosted in a multi-DB setup), displaying the change list works as expected.

But when usage of the list_filter feature is added, a DB error is generated reporting that no table object for the model at hand is found in the default Django DB.

This is because the filter machinery doesn't use the custom ModelAdmin-dictated QuerySet but the model's default manager all() method:

(https://github.com/django/django/blob/011abb7d96c75f6154c15a8a0f997d8c27698679/django/contrib/admin/filters.py#L364)

    queryset = parent_model._default_manager.all()

Replacing it with:

diff --git a/django/contrib/admin/filters.py b/django/contrib/admin/filters.py
index d5f31ab..d04d5cc 100644
--- a/django/contrib/admin/filters.py
+++ b/django/contrib/admin/filters.py
@@ -361,7 +361,7 @@ class AllValuesFieldListFilter(FieldListFilter):
         self.lookup_val_isnull = request.GET.get(self.lookup_kwarg_isnull,
                                                  None)
         parent_model, reverse_path = reverse_field_path(model, field_path)
-        queryset = parent_model._default_manager.all()
+        queryset = model_admin.get_queryset(request)
         # optional feature: limit choices base on existing relationships
         # queryset = queryset.complex_filter(
         #    {'%s__isnull' % reverse_path: False})

solves the problem.

Change History (12)

comment:1 by Ramiro Morales, 10 years ago

Description: modified (diff)

comment:2 by Tim Graham, 10 years ago

Triage Stage: UnreviewedAccepted

comment:3 by Federico Frenguelli, 10 years ago

Hi ramiro,

I'm trying to reproduce the issue but I can't. Can you help me?

This is my current setup: I have a foo application with just one Stuff model

DATABASES = {
    'default': {
        'ENGINE': 'django.db.backends.sqlite3',
        'NAME': os.path.join(BASE_DIR, 'db.sqlite3'),
    },
    'other': {
        'ENGINE': 'django.db.backends.sqlite3',
        'NAME': os.path.join(BASE_DIR, 'other.sqlite3'),
    },
}

DATABASE_ROUTERS = ['help.routers.FooRouter']

and that's my ModelAdmin

class MultiDBModelAdmin(admin.ModelAdmin):
    # A handy constant for the name of the alternate database.
    using = 'other'

    def get_queryset(self, request):
        # Tell Django to look for objects on the 'other' database.
        return super(MultiDBModelAdmin, self).get_queryset(request).using(self.using)

# Register your models here.
class StuffAdmin(MultiDBModelAdmin):
    list_filter = ("empty",)

The router is used to create the foo_stuff table only in the other db.

in reply to:  3 comment:4 by Ramiro Morales, 10 years ago

Replying to synasius:

Hi ramiro,

I'm trying to reproduce the issue but I can't. Can you help me?

No router should be used (or rather, no custom DB routing scheme should be in place, just use the default setup). That's why I didn't mention them in the report.

This setup could well be a corner case but in practice there is no need to use multi-DB to experiece the bug:

Imagine you use just one database, and you limit the set of available objects that can be handled in the admin by returning a filtered queryset in the relevant ModelAdmin get_queryset() method and so the changelist will list only the model instances resulting from such filtering. But because of this bug, the right-side filters will be created based on the full unfiltered queryset of the model's default manager.

comment:5 by Ola Sitarska <ola@…>, 10 years ago

Owner: changed from nobody to anonymous
Status: newassigned

comment:6 by Ola Sitarska, 10 years ago

Owner: changed from anonymous to Ola Sitarska

Sorry, I didn't login :)

comment:7 by Ola Sitarska, 10 years ago

Ok, so I wrote a test for that: https://github.com/olasitarska/django/commit/b36252346b78f5a387133bba2b889ed9e253edfe

And when I tried implementing solution proposed here by ramiro, it is breaking another test.

ERROR: test_fieldlistfilter_underscorelookup_tuple (admin_filters.tests.ListFiltersTests)
----------------------------------------------------------------------
FieldError: Cannot resolve keyword 'email' into field. Choices are: author, author_id, contributors, date_registered, id, is_best_seller, no, title, year

It happens because in scenario when we are filtering by author__email, this line:

queryset = parent_model._default_manager.all()

would return a User Queryset, while this line:

queryset = model_admin.get_queryset(request)

still returns a Book Queryset.

I am totally not an expert (it's actually my first contribution). We can probably check if model connected to modeladmin is the same as model and act accordingly, but how can we pass along those filters from custom modeladmin queryset?

comment:8 by Ola Sitarska, 10 years ago

I dug some more into the code and I think that issue ramiro is referring to in his last comment is actually somewhere else - filters like that are rendered by class RelatedFieldListFilter which returns choices directly from Model field.

comment:9 by Ola Sitarska, 10 years ago

Owner: Ola Sitarska removed
Status: assignednew

comment:10 by Ramiro Morales, 10 years ago

Has patch: set
Owner: set to Ramiro Morales
Status: newassigned

comment:11 by Tim Graham, 10 years ago

Triage Stage: AcceptedReady for checkin

Minor test failure on non-sqlite; looks good otherwise.

comment:12 by Ramiro Morales <cramm0@…>, 10 years ago

Resolution: fixed
Status: assignedclosed

In 3a297d78169b7d3c6d688cf0e349f4663acc6bad:

Fixed #23055 -- Made generic admin filter obey ModelAdmin queryset.

Thanks to Trac user synasius and to Ola Sitarska for helping in
identifying the issue and helping with the test case.

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