fix: async with new filter API (assert queryset is wrong)

strawberry-graphql / strawberry-django

Strawberry GraphQL Django extension

https://strawberry.rocks/docs/django

MIT License

406 stars 118 forks source link

fix: async with new filter API (assert queryset is wrong) #504

Closed devkral closed 6 months ago

devkral commented 6 months ago

This PR fixes a problem with the new filter API in async environments It is likely a typo

Types of Changes

[ ] Core
[x] Bugfix
[ ] New feature
[ ] Enhancement/optimization
[ ] Documentation

Checklist

[x] My code follows the code style of this project.
[ ] My change requires a change to the documentation.
[ ] I have updated the documentation accordingly.
[x] I have read the CONTRIBUTING document.
[x] I have added tests to cover my changes.
[x] I have tested the changes and verified that they work and don't break anything (as well as I can manage).

codecov-commenter commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 88.49%. Comparing base (40a489b) to head (8ef0931).

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #504 +/- ## ======================================= Coverage 88.49% 88.49% ======================================= Files 40 40 Lines 3398 3398 ======================================= Hits 3007 3007 Misses 391 391 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

devkral commented 6 months ago

interesting: sourcery-ai doesn't filter removed lines. What it proposes is exactly what I am doing.

It is quite a mini fix but this bug breaks my code.

devkral commented 6 months ago

we need to upgrade the severity of the issue: the wrong assertion will fail also for empty querysets, so it can cause major breakage

devkral commented 6 months ago

I added some tests for the bug

devkral commented 6 months ago

workaround: don't use the queryset parameter when writing custom filters