Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#19318 closed Bug (fixed)

Admin's SimpleListFilter option not being displayed as selected if the lookup's first element is not a string

Reported by: Sebastián Magrí Owned by: nobody
Component: contrib.admin Version: 1.4
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: yes
Easy pickings: yes UI/UX: no

Description

I had the following lookups method for a SimpleListFilter:

def lookups(self, request, model_admin):
    return set(itertools.chain(*[
        [(o.deliverable.id, o.deliverable.name)
         for o in user.orders.all()]
        for user in model_admin.queryset(request).all()
    ]))

In this case, the filters in the change list didn't display the option as selected on activation.

Changing the above code to

def lookups(self, request, model_admin):
    return set(itertools.chain(*[
        [(str(o.deliverable.id), o.deliverable.name)
         for o in user.orders.all()]
        for user in model_admin.queryset(request).all()
    ]))

Fixed the issue.

This seems to be caused by django/contrib/admin/filters.py:105, where

yield {
    'selected': self.value() == lookup,
    'query_string': cl.get_query_string({
         self.parameter_name: lookup,
    }, []),
    'display': title,
}

Might be

yield {
    'selected': self.value() == str(lookup),
    'query_string': cl.get_query_string({
         self.parameter_name: lookup,
    }, []),
    'display': title,
}

I've created a pull request at GitHub with this change.

Change History (7)

comment:1 by Julien Phalip, 11 years ago

Needs tests: set
Patch needs improvement: set
Triage Stage: UnreviewedAccepted
UI/UX: unset

Thanks for the report. Using force_unicode would be more appropriate than str.

comment:2 by Sebastián Magrí, 11 years ago

I have updated the pull request to use force_unicode instead of str.

Regarding tests, I would like to get some help on it as I don't have too much experience with testing.

comment:3 by Julien Phalip, 11 years ago

Take a look at the existing tests in tests/regressiontests/admin_filters/tests.py. You should find multiple examples to get started. Have a go at it and then come back with questions if you still need any further assistance.

comment:4 by Sebastián Magrí, 11 years ago

I've closed the old pull requests and I have created two new pull requests, one for 1.4.x and the other for 1.5.x. The fix in different in both cases. Because of the py3k compatibility in 1.5 the force_unicode utility function have been renamed to force_text.

Pull requests are 539 and 540.

Please let me know if there is any other requirement.

Best Regards,

comment:5 by Julien Phalip <jphalip@…>, 11 years ago

Resolution: fixed
Status: newclosed

In 88e17156393be86df64d44554de8ab77a6d3ed7d:

Fixed #19318 -- Ensured that the admin's SimpleListFilter options can be displayed as selected even if the lookup's first element is not a string.

comment:6 by Julien Phalip <jphalip@…>, 11 years ago

In 237a404d3ec678d1088dcd0b3630a3ea67ce08ea:

[1.5.x] Fixed #19318 -- Ensured that the admin's SimpleListFilter options can be displayed as selected even if the lookup's first element is not a string.
Backport of 88e17156393b

comment:7 by Julien Phalip <jphalip@…>, 11 years ago

In c72172244ef81f793d1eca9a54f58a11f27b0917:

[1.4.x] Fixed #19318 -- Ensured that the admin's SimpleListFilter options can be displayed as selected even if the lookup's first element is not a string.
Backport of 88e17156393b

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