-
-
Notifications
You must be signed in to change notification settings - Fork 535
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
Stop micromanaging WS support in IDEs #3660
Stop micromanaging WS support in IDEs #3660
Conversation
Reviewer's Guide by SourceryThis pull request removes the Updated class diagram for GraphQL viewsclassDiagram
class BaseView {
-_ide_replace_variables: bool
-subscriptions_enabled: bool
-_ide_subscription_enabled: bool
+graphql_ide_html: str
+allow_queries_via_get: bool
+multipart_uploads_enabled: bool
}
class GraphQLView {
-subscriptions_enabled: bool
+graphiql: Optional[bool]
+graphql_ide: Optional[GraphQL_IDE]
+allow_queries_via_get: bool
}
class AsyncGraphQLView {
-subscriptions_enabled: bool
+graphiql: Optional[bool]
+graphql_ide: Optional[GraphQL_IDE]
+allow_queries_via_get: bool
}
class BaseGraphQLView {
-_ide_subscription_enabled: bool
+graphql_ide: Optional[GraphQL_IDE]
}
class GraphQLWebsocketCommunicator {
-subscriptions_enabled: bool
+allow_queries_via_get: bool
}
BaseView <|-- GraphQLView
BaseView <|-- AsyncGraphQLView
BaseView <|-- BaseGraphQLView
BaseView <|-- GraphQLWebsocketCommunicator
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
for more information, see https://pre-commit.ci
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.
Hey @DoctorJohn - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Thanks for adding the Here's a preview of the changelog: This release removes the dated Here's the tweet text:
|
CodSpeed Performance ReportMerging #3660 will not alter performanceComparing Summary
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3660 +/- ##
=======================================
Coverage 96.82% 96.82%
=======================================
Files 503 503
Lines 33409 33392 -17
Branches 5583 5586 +3
=======================================
- Hits 32348 32332 -16
Misses 830 830
+ Partials 231 230 -1 |
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.
❤️
@DoctorJohn feel free to merge when Pre commit is fine :D |
Description
This PR removes the
subscriptions_enabled
flag from the Django and Channels views. It also removes two internal flags with the same purpose from all other views.These flags were originally added to address #334. However, the behavior described in that issue has since been changed in GraphiQL, making our flag useless. (Tested locally using the Django integration, setting
subscriptions_enabled
to true, opening GraphiQL, and observing the browser's network tab).Types of Changes
Issues Fixed or Closed by This PR
Summary by Sourcery
Remove the
subscriptions_enabled
flag from various views and update the documentation accordingly. Enable WebSocket support by default in all GraphQL IDEs, simplifying the configuration process.Enhancements:
subscriptions_enabled
flag from Django and Channels views, as well as from other internal views, to simplify the configuration and enable WebSocket support by default.Documentation:
subscriptions_enabled
parameter from the GraphQLView and AsyncGraphQLView in Django and Channels integrations.Chores:
subscriptions_enabled
setting and the default enabling of WebSocket support.