Skip to content
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

Merged
merged 9 commits into from
Jul 13, 2018

Conversation

rpkilby
Copy link
Collaborator

@rpkilby rpkilby commented Jan 15, 2018

Remove "lookup types" handling for Filter.lookup_expr, where a list or None value would trigger the use of LookupTypeField. This has been replaced with a dedicated LookupChoiceFilter.

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:

  • Add LookupChoiceFilter
  • Remove list and None value handling for lookup_expr. Using this form will fail an assertion check.
  • Remove LOOKUP_TYPES module variable from django_filters.filters.
  • Rename LookupTypeWidget to LookupChoiceWidget.
  • Rename LookupTypeField toLookupChoiceField.
  • Rename Lookup.lookup_type to Lookup.lookup_expr.
  • Change Lookup to only accept non-empty values. If a Lookup is missing a value, a None should be provided instead.

@rpkilby rpkilby changed the title Rework 'lookup types' handling into new LookupChoiceFilter class Rework 'lookup types' handling into LookupChoiceFilter Jan 15, 2018
@codecov-io
Copy link

codecov-io commented Jan 15, 2018

Codecov Report

Merging #851 into master will increase coverage by 0.03%.
The diff coverage is 100%.

Impacted file tree graph

@@            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
Impacted Files Coverage Δ
django_filters/fields.py 100% <100%> (ø) ⬆️
django_filters/widgets.py 95.27% <100%> (ø) ⬆️
django_filters/filters.py 98.48% <100%> (+0.05%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2b554a5...c88ff57. Read the comment docs.

@rpkilby
Copy link
Collaborator Author

rpkilby commented Jan 15, 2018

Yep - this does fix #843, in so far as the build no longer fails to even run 😄

@rpkilby rpkilby force-pushed the rework-lookup-types branch 2 times, most recently from b1c359a to 41c5437 Compare May 21, 2018 05:42
Copy link
Owner

@carltongibson carltongibson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Super. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Build fails against Django (pre-2.1) master
3 participants