-
Notifications
You must be signed in to change notification settings - Fork 771
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
Fix filterset multiple inheritance bug #1131
Fix filterset multiple inheritance bug #1131
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1131 +/- ##
==========================================
+ Coverage 98.1% 99.44% +1.33%
==========================================
Files 15 15
Lines 1216 1255 +39
==========================================
+ Hits 1193 1248 +55
+ Misses 23 7 -16
Continue to review full report at Codecov.
|
We sure we’re not changing a behaviour from the dawn of time here? 🙂 |
django-filter/django_filters/filterset.py Lines 33 to 40 in 62516ad
However, I refactored |
oh wait, I read that completely wrong. 🤦♂ Either way, like DRF, I think the test case demonstrates that the current behavior is incorrect. I just don't think this multiple inheritance use case is incredibly common, which is why it hasn't been raised here yet (and why it hasn't seen much attention in DRF either). |
OK... Let me look into it. I want to be 100% sure. (And we'll need a release note stating the BC change.) |
Just as a note, this PR needs to be updated, pending review of the DRF PR. |
@carltongibson updated this from the DRF pr. This should be good to go assuming tests pass. |
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.
Super, yes. Good work!
Declared filter resolution was mostly copied from DRF. This fixes a multiple inheritance bug described in encode/django-rest-framework#6980