-
Notifications
You must be signed in to change notification settings - Fork 298
DjangoFilterBackend.get_filterset_kwargs doesn't modify data dict while iterating #580
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
DjangoFilterBackend.get_filterset_kwargs doesn't modify data dict while iterating #580
Conversation
...a dictionary. Avoids delete.
The fix doesn't return other params in data. It didn't seem to have an impact, but I'd love to have someone who knows the code base comment.
Codecov Report
@@ Coverage Diff @@ ## master #580 +/- ## ========================================== - Coverage 95.64% 95.64% -0.01% ========================================== Files 55 55 Lines 2850 2849 -1 ========================================== - Hits 2726 2725 -1 Misses 124 124
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@ ## master #580 +/- ## ========================================== + Coverage 95.64% 95.65% +<.01% ========================================== Files 55 55 Lines 2850 2856 +6 ========================================== + Hits 2726 2732 +6 Misses 124 124
Continue to review full report at Codecov.
|
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.
Thanks for catching my bug!
@sliverc can you take a second look just to make sure?
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.
@mindblight
Thanks for your pull requests. The change looks good!
To ensure this fix for the future could you add a test reproducing the issue?
A changelog entry would be good as well and add yourself to AUTHORS if you like. Thanks.
@sliverc Done! It seems like 4 params is the magic number for this bug to show up.
@sliverc, It looks like the codecov checks are hanging for some reason. Is there any way to re-trigger those, or merge without them?
@mindblight Thanks. Merged.
Uh oh!
There was an error while loading. Please reload this page.
It looks like
DjangoFilterBackend.get_filterset_kwargsreused the same dictionary for the original (filter[something]) keys, the converted (something) keys, and the extra params (includeorpage). By callingdata[key] = valin while iteratingdata.items(), the indexes can be incorrectly changed: https://stackoverflow.com/questions/6777485/modifying-a-python-dict-while-iterating-over-itExample
With the path:
/api/products?filter[organization]=44cbbad6-d418-4129-8093-731637d5013b&filter[archived]=False&filter[productType.key]=biomass&include=images,product_type, different filter params were ignored based on their order in the param string.filter[productType.key]=...&filter[organization]=...&filter[archived]=False&include=images,product_typefilter[archived]filter[productType.key]=...&filter[organization]=...&filter[archived]=False&include=images,product_typefilter[organization]filter[archived]=False&filter[productType.key]=...&filter[organization]=...&include=images,product_typefilter[organization]filter[archived]=False&filter[organization]=...&filter[productType.key]=...&include=images,product_typefilter[productType.key]filter[organization]=...&filter[archived]=False&filter[productType.key]=...&include=images,product_typefilter[productType.key]filter[organization]=...&filter[productType.key]=...&filter[archived]=False&include=images,product_typefilter[archived]filter[organization]=...&filter[productType.key]=...&filter[archived]=FalseFixes #
Description of the Change
Checklist
CHANGELOG.mdupdated (only for user relevant changes)AUTHORS