strawberry-graphql / strawberry-django

Strawberry GraphQL Django extension
MIT License
393 stars 115 forks source link

compatibility ASGI/websockets get_request, login and logout #393

Closed sdobbelaere closed 8 months ago

sdobbelaere commented 8 months ago

The context object can turn into a dict when using ASGI. This pull requests accounts for both ASGI and WSGI conditions and replaces the use of info.context.request with get_request(info)

codecov-commenter commented 8 months ago

Codecov Report

Attention: 19 lines in your changes are missing coverage. Please review.

Comparison is base (25e3ca2) 87.46% compared to head (82f9e07) 87.04%. Report is 4 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #393 +/- ## ========================================== - Coverage 87.46% 87.04% -0.43% ========================================== Files 35 36 +1 Lines 3024 3056 +32 ========================================== + Hits 2645 2660 +15 - Misses 379 396 +17 ``` | [Files](https://app.codecov.io/gh/strawberry-graphql/strawberry-graphql-django/pull/393?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=strawberry-graphql) | Coverage Δ | | |---|---|---| | [strawberry\_django/auth/utils.py](https://app.codecov.io/gh/strawberry-graphql/strawberry-graphql-django/pull/393?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=strawberry-graphql#diff-c3RyYXdiZXJyeV9kamFuZ28vYXV0aC91dGlscy5weQ==) | `66.66% <60.00%> (+3.50%)` | :arrow_up: | | [strawberry\_django/utils/requests.py](https://app.codecov.io/gh/strawberry-graphql/strawberry-graphql-django/pull/393?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=strawberry-graphql#diff-c3RyYXdiZXJyeV9kamFuZ28vdXRpbHMvcmVxdWVzdHMucHk=) | `75.00% <75.00%> (ø)` | | | [strawberry\_django/auth/mutations.py](https://app.codecov.io/gh/strawberry-graphql/strawberry-graphql-django/pull/393?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=strawberry-graphql#diff-c3RyYXdiZXJyeV9kamFuZ28vYXV0aC9tdXRhdGlvbnMucHk=) | `74.13% <46.42%> (-25.87%)` | :arrow_down: | ... and [3 files with indirect coverage changes](https://app.codecov.io/gh/strawberry-graphql/strawberry-graphql-django/pull/393/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=strawberry-graphql)

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

sdobbelaere commented 8 months ago

It would seem that login and logout mutations were also non compatible with ASGI/websockets. This was addressed by using the channels authentication rather then the default django.

sdobbelaere commented 8 months ago

It would seem that login and logout mutations were also non compatible with ASGI/websockets. This was addressed by using the channels authentication rather then the default django.

sdobbelaere commented 8 months ago

@bellini666 Not sure if you need anything else from mee on this pull-request? All tests seem to be passing.

sdobbelaere commented 8 months ago

Sorry for not replying earlier to this.

No problem :)

I was out during last week for the djangocon event (I was a speaker there https://2023.djangocon.us/talks/building-high-performance-type-safe-graphql-apis-with-strawberry-and-django/).

Nice! Some publicity for the library is a good idea indeed!

I made a small suggestion regarding the logout that you pointed out. If you want to fix it, I'll merge it after. Otherwise just tell me and this is fine to be merged already

Yes, I'll take a look. I hope it's a detail, but I fear that logout function may not work as advertised otherwise the login wouldn't work - which it does.

sdobbelaere commented 8 months ago

Logout mutation seems to be working fine. Excess line removed.

sdobbelaere commented 8 months ago

All done. Ready for merge from my end @bellini666