-
Notifications
You must be signed in to change notification settings - Fork 219
Add --query-file flag to app execute and app bulk execute for loading GraphQL queries from files
#6733
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
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
--query-file to both app execute and app bulk execute--query-file flag to app execute and app bulk execute for loading GraphQL queries from files
Coverage report
Test suite run success3570 tests passing in 1416 suites. Report generated by 🧪jest coverage report action from 1ffab0d |
|
We detected some changes at Caution DO NOT create changesets for features which you do not wish to be included in the public changelog of the next CLI release. |
| if (flags.query) { | ||
| query = flags.query | ||
| } else if (flags['query-file']) { |
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.
what if a user submits both?
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.
OCLIF should prevent that based on the flag config
| /** | ||
| * Validates bulk operation-specific constraints for variables. | ||
| */ | ||
| function validateBulkOperationVariables(graphqlOperation: string, variablesJsonl?: string): void { |
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.
I'm assuming this means that normal (non-bulk) GraphQL queries can have variables?
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.
Correct, this is only invoked for bulk operations
gonzaloriestra
left a comment
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.
👏

WHY are these changes introduced?
Writing example scripts with
app executeandapp bulk executemade it clear that an argument for this would make the code cleaner and intent more obvious.WHAT is this pull request doing?
Adds a new
--query-fileflag to theapp executeandapp bulk executecommands that allows loading GraphQL queries from a file.Key changes:
--query-fileflag to both operation and bulk operation commands--queryand--query-fileflags mutually exclusiveHow to test your changes?
query.graphqlshopify app execute --query-file query.graphql--queryand--query-filetogether and confirm it failsMeasuring impact
How do we know this change was effective? Please choose one:
Checklist