Opened 12 years ago

Closed 12 years ago

#17091 closed Bug (fixed)

Djano 1.4 SimpleListFilter 'selected' option issue

Reported by: tejinderss@… Owned by: Julien Phalip
Component: contrib.admin Version: dev
Severity: Normal Keywords: SimpleListFilter, FilterSpec
Cc: Triage Stage: Design decision needed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I have written a SimpleListFilter, here is the code: http://dpaste.com/639578/

It displays in the admin list properly, but i am having an issue, The selected option does not get highlighted in the custom filter. Only 'All' highlights but not the custom options. Here is the screenshot to illustrate that:

http://imgur.com/IyrYk

Attachments (1)

17091.changelist-lookup-filters-untangling.diff (25.5 KB ) - added by Julien Phalip 12 years ago.

Download all attachments as: .zip

Change History (17)

comment:1 by Julien Phalip, 12 years ago

Triage Stage: UnreviewedDesign decision needed

What is happening here is that your parameter name ends with "__in" which is interpreted as if the value passed is a list. So for example "?status__in=0,30,40" is interpreted as if the value passed is [0,30,40], not "0,30,40". To be honest, at this stage I'm not sure if it's a bug or a feature.

If you renamed your parameter to "status_in" (with just one underscore) then it should just work. I'd like to understand your use case better though. Could you please explain your original motivation for having 2 underscores in the parameter name?

I'm keen to know to figure out whether we should treat it as a bug or rather as an opportunity to build a new feature around it. Thanks!

comment:2 by anonymous, 12 years ago

I am using in operator to filter results by multiple status values, i.e. "?status__in=0,30,40" filters results with status values 0, 30 and 40. Thats what i want. status_in (with single _) does not work as expected, it just redirects to ?e=1 without filtered results. Even the default filters use 2 _ notation. Like if i use default filter "Status", the url is "?status__exact=30". Thats the default notation i guess and only "__" works. Here is the example of default django filter behavior.

http://i.imgur.com/RwRTI.png

Last edited 12 years ago by Ramiro Morales (previous) (diff)

comment:3 by tejinderss@…, 12 years ago

I am using in operator to filter results by multiple status values, i.e. !?status__in=0,30,40 filters results with status values 0, 30 and 40. Thats what i want. status_in (with single _) does not work as expected, it just redirects to ?e=1 without filtered results. Even the default filters use 2 _ notation. Like if i use default filter "Status", the url is ?status__exact=30. Thats the default notation i guess and only __ works. Here is the example of default django filter behavior.

http://i.imgur.com/RwRTI.png

Edit: Sorry for messed up formatting in previous reply.

comment:4 by Julien Phalip, 12 years ago

Could you please provide all your relevant admin and model definitions? I think this might be a case where the SimpleListFilter should *not* be used and where the Django documentation could be improved.

comment:5 by anonymous, 12 years ago

Here is the model definition:
http://dpaste.com/640035/

Here is the ResAdmin definition:
http://dpaste.com/640036/

comment:6 by anonymous, 12 years ago

How else i can achieve this? Filtering result using multiple choices if not SimpleListFilter ?

comment:7 by anonymous, 12 years ago

Actually I am exporting the filtered results to CSV file, hence i chose this parameter name so that export_csv file can get the parameter status__in=0,30,40 and get fetch the filtered queryset and export to csv file. This the the reason of using the status__in parameter in the url.

comment:8 by anonymous, 12 years ago

s/export_csv file/export_csv view

comment:9 by Julien Phalip, 12 years ago

In theory, I think the proper way would be to do:

class ResAdminListFilter(SimpleListFilter):
    title = 'Multiple Status' 

    parameter_name = "status"  # Do not put double underscores in here

    def lookups(self, request, model_admin):
        return (
            ('0,30,40', _('All but BN')),
            ('200,300', _('Only BN')),
        )
    def queryset(self, request, queryset): # Write the code below (instead of just "pass")
        statuses = self.value().split(',')
        return queryset.filter(status__in=statuses)
class ResAdmin(MultilingualAdminMixin, ResAdminMixin, ModelAdminDMY):
    list_filter = ('supplier', 'created', ResAdminListFilter, )  # Same as before but without 'status'
    ...

Could you try that and see if that works?

comment:10 by anonymous, 12 years ago

Database error
Something's wrong with your database installation. Make sure the appropriate database tables have been created, and make sure the database is readable by the appropriate user.

comment:11 by anonymous, 12 years ago

Ok this thing worked:

def queryset(self, request, queryset):

if self.value() == 'nobn':

return queryset.filter(statusin=(0, 30, 40))

if self.value() == 'bn':

return queryset.filter(statusin=(200, 300))

Alternatively i also tried overriding the value() method in my custom filter like this:

def value(self): #Bug fix, passing '30,40' changes to list and it does not match the params val

val = self.params.get(self.parameter_name, None)
if isinstance(val, str):

return val

if isinstance(val, list):

return ','.join(val)

Maybe we can add this feature? But i guess the first approach is better. Thanks for the help. I guess this ticket is closed now.

comment:12 by Julien Phalip, 12 years ago

Owner: changed from nobody to Julien Phalip

Well, the first approach (which works) is the expected use and the documented one :-)

Now there's still one small bug i.e. that the value of a parameter whose name finishes with "__in" should not be interpreted as a list. I'll fix that soon, so you can leave this ticket open.

I'll also see if the doc can be clarified to avoid any confusion. Thanks for your patience!

comment:13 by anonymous, 12 years ago

Thanks a lot.

comment:14 by Julien Phalip, 12 years ago

While looking at the ChangeList class' internals for the needs of this ticket, I realized that there were a few issues with the sequence in which admin filters and other query string lookups were processed. This led me to operate a significant refactor of those internals to untangle things a bit. With this patch, the query string lookups are processed once instead of twice and some bugs are avoided —in particular the SimpleListFilter parameter name being mistaken for a model field in some cases. The code for the ChangeList's get_filters() and get_query_set() methods is now also much more streamlined.

comment:15 by Julien Phalip, 12 years ago

In [17145]:

(The changeset message doesn't reference this ticket)

comment:16 by Julien Phalip, 12 years ago

Resolution: fixed
Status: newclosed

I don't think we need to modify the documentation as it's clear-enough in relation to the problem reported in this ticket. I believe the original confusion here was in fact caused by that bug, which is now fixed. Therefore considering this ticket fixed as well.

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