-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
console: update QueryAnalyzer to support subscriptions #4965
Conversation
- this commit should address #2451
Deploy preview for hasura-docs ready! Built with commit 0976ac4 |
Review app for commit c7458fb deployed to Heroku: https://hge-ci-pull-4965.herokuapp.com |
console/src/components/Services/ApiExplorer/Analyzer/QueryAnalyzer.js
Outdated
Show resolved
Hide resolved
console/src/components/Services/ApiExplorer/Analyzer/QueryAnalyzer.js
Outdated
Show resolved
Hide resolved
console/src/components/Services/ApiExplorer/Analyzer/QueryAnalyzer.js
Outdated
Show resolved
Hide resolved
Review app for commit aa11a54 deployed to Heroku: https://hge-ci-pull-4965.herokuapp.com |
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.
Changelog + UX approved
Review app for commit 55eee20 deployed to Heroku: https://hge-ci-pull-4965.herokuapp.com |
- renderAnalyzeText is now, a diff. component - AnalyzeText - minor changes on QueryAnalyzer
Review app for commit c3afd20 deployed to Heroku: https://hge-ci-pull-4965.herokuapp.com |
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.
Leaving some TypeScript related comments.
Let's add a type for explain data:
type ExplainPayload = {
field: string;
sql: string;
plan: string[];
}
in console/src/components/Services/ApiExplorer/Analyzer/AnalyzeText.tsx
and use it to type component's props:
type RootFieldsProps = { // I'm also changing the name of the component
data: ExplainPayload[];
activeNode: number;
onClick: React.MouseEvent<HTMLDivElement> // <- added generic argument here: HTMLDivElement
}
const RootFields: React.FC<RootFieldsProps> = {...}
console/src/components/Services/ApiExplorer/Analyzer/AnalyzeText.tsx
Outdated
Show resolved
Hide resolved
console/src/components/Services/ApiExplorer/Analyzer/AnalyzeText.tsx
Outdated
Show resolved
Hide resolved
console/src/components/Services/ApiExplorer/Analyzer/QueryAnalyzer.js
Outdated
Show resolved
Hide resolved
- fixed types on AnalyzeText
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.
Almost there!
console/src/components/Services/ApiExplorer/Analyzer/AnalyzeText.tsx
Outdated
Show resolved
Hide resolved
console/src/components/Services/ApiExplorer/Analyzer/AnalyzeText.tsx
Outdated
Show resolved
Hide resolved
Review app for commit ead51a0 deployed to Heroku: https://hge-ci-pull-4965.herokuapp.com |
Review app for commit 935a65f deployed to Heroku: https://hge-ci-pull-4965.herokuapp.com |
console/src/components/Services/ApiExplorer/Analyzer/RootFields.tsx
Outdated
Show resolved
Hide resolved
console/src/components/Services/ApiExplorer/Analyzer/RootFields.tsx
Outdated
Show resolved
Hide resolved
Review app for commit dba9886 deployed to Heroku: https://hge-ci-pull-4965.herokuapp.com |
Review app for commit db92040 deployed to Heroku: https://hge-ci-pull-4965.herokuapp.com |
Review app https://hge-ci-pull-4965.herokuapp.com is deleted |
this should resolve #2541
Description
Analyze
button onGraphiQL
will now display the analysis data forsubscriptions
too.Changelog
CHANGELOG.md
is updated with user-facing content relevant to this PR. If no changelog is required, then add theno-changelog-required
label.Affected components
Related Issues
#2541
Solution and Design
The
explain
call forsubscriptions
returns an object unlike the same call forquery
. The solution is just to check whether an object, if so, then wrap the response in an array.Breaking changes