-
Notifications
You must be signed in to change notification settings - Fork 767
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rework 'lookup types' handling into LookupChoiceFilter #851
Rework 'lookup types' handling into LookupChoiceFilter #851
Conversation
LookupChoiceFilter
class
Codecov Report
@@ Coverage Diff @@
## master #851 +/- ##
==========================================
+ Coverage 97.95% 97.98% +0.03%
==========================================
Files 15 15
Lines 1122 1140 +18
==========================================
+ Hits 1099 1117 +18
Misses 23 23
Continue to review full report at Codecov.
|
Yep - this does fix #843, in so far as the build no longer fails to even run 😄 |
b1c359a
to
41c5437
Compare
These tests were used for comparing a filter's behavior when it was provided a list of lookup expressions. This comparison is no longer relevant, since the 'lookup choices' behavior is being moved to a separate filter class.
41c5437
to
c88ff57
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Super. 👍
Remove "lookup types" handling for
Filter.lookup_expr
, where alist
orNone
value would trigger the use ofLookupTypeField
. This has been replaced with a dedicatedLookupChoiceFilter
.I decided to replace the feature outright and raise an assertion with the relevant removal message, instead of attempting to go through a deprecation path. While it should be possible, it didn't seem worth the effort to try support/deprecate the old functionality, especially given the clean break 2.0 provides.
Should also fix #843 (at least the reliance on the defunct
QUERY_TERMS
).Changes:
LookupChoiceFilter
list
andNone
value handling forlookup_expr
. Using this form will fail an assertion check.LOOKUP_TYPES
module variable fromdjango_filters.filters
.LookupTypeWidget
toLookupChoiceWidget
.LookupTypeField
toLookupChoiceField
.Lookup.lookup_type
toLookup.lookup_expr
.Lookup
to only accept non-empty values. If aLookup
is missing a value, aNone
should be provided instead.