DjangoFilterBackend.get_filterset_kwargs doesn't modify data dict while iterating

[mindblight]

It looks like DjangoFilterBackend.get_filterset_kwargs reused the same dictionary for the original (filter[something]) keys, the converted (something) keys, and the extra params (include or page). By calling data[key] = val in while iterating data.items(), the indexes can be incorrectly changed: https://stackoverflow.com/questions/6777485/modifying-a-python-dict-while-iterating-over-it

Example

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_type
  • ignores filter[archived]
• filter[productType.key]=...&filter[organization]=...&filter[archived]=False&include=images,product_type
  • ignores filter[organization]
• filter[archived]=False&filter[productType.key]=...&filter[organization]=...&include=images,product_type
  • ignores filter[organization]
• filter[archived]=False&filter[organization]=...&filter[productType.key]=...&include=images,product_type
  • ignores filter[productType.key]
• filter[organization]=...&filter[archived]=False&filter[productType.key]=...&include=images,product_type
  • ignores filter[productType.key]
• filter[organization]=...&filter[productType.key]=...&filter[archived]=False&include=images,product_type
  • ignores filter[archived]
• filter[organization]=...&filter[productType.key]=...&filter[archived]=False
  • works perfectly

Fixes #

Description of the Change

Checklist

• PR only contains one change (considered splitting up PR)
• unit-test added
• documentation updated
• CHANGELOG.md updated (only for user relevant changes)
• author name in AUTHORS

[mindblight]

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]

Codecov Report

Merging #580 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            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

Impacted Files	Coverage Δ
rest_framework_json_api/django_filters/backends.py	100% <100%> (ø)	⬆️

---

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 75e2f82...b478f06. Read the comment docs.

[codecov]

Codecov Report

Merging #580 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #580      +/-   ##
==========================================
+ Coverage   95.64%   95.65%   +<.01%     
==========================================
  Files          55       55              
  Lines        2850     2856       +6     
==========================================
+ Hits         2726     2732       +6     
  Misses        124      124

Impacted Files	Coverage Δ
rest_framework_json_api/django_filters/backends.py	100% <100%> (ø)	⬆️
example/tests/test_filters.py	100% <100%> (ø)	⬆️

---

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 30ca94b...8eca777. Read the comment docs.

[n2ygk]

Thanks for catching my bug!

@sliverc can you take a second look just to make sure?

[sliverc]

@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.

[mindblight]

@sliverc Done! It seems like 4 params is the magic number for this bug to show up.

[mindblight]

@sliverc, It looks like the codecov checks are hanging for some reason. Is there any way to re-trigger those, or merge without them?

[sliverc]

@mindblight Thanks. Merged.