-
Notifications
You must be signed in to change notification settings - Fork 14.3k
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
fix(sqllab): Add templateParams on kv store #22013
fix(sqllab): Add templateParams on kv store #22013
Conversation
@@ -72,6 +72,7 @@ const unsavedQueryEditor = { | |||
schema: 'query_schema_updated', | |||
sql: 'SELECT * FROM Updated Limit 100', | |||
autorun: true, | |||
templateParams: '{ "my_value": "foo" }', |
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.
@justinpark how are these template parameters actually leveraged in the test? It's not apparent to me.
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.
@john-bodley the following block verify that this extra templateParams payload is being passed through api.
(expected
includes this extra parameter)
Codecov Report
@@ Coverage Diff @@
## master #22013 +/- ##
==========================================
- Coverage 67.03% 66.99% -0.05%
==========================================
Files 1813 1815 +2
Lines 69425 69522 +97
Branches 7448 7479 +31
==========================================
+ Hits 46542 46579 +37
- Misses 20963 21013 +50
- Partials 1920 1930 +10
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
be6df1f
to
09a7d01
Compare
09a7d01
to
4f8542b
Compare
@eschutho can you help this review? |
@eschutho we're going to merge this change next Monday if you don't have any critical issue. |
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.
Looks great!
SUMMARY
When "copy link" on SqlLab, it ignores the template parameters. Therefore, it's difficult to share a query with the template variables.
This commit adds the template parameters in kv store.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before:
before--kv-template-params.mov
After:
after--kv-template-params.mov
TESTING INSTRUCTIONS
enable
SHARE_QUERIES_VIA_KV_STORE
featurego to SqlLab and add template parameters
click "copy link" and then paste the link in the new tab
check the template parameters in new tab
ADDITIONAL INFORMATION
cc: @ktmud @john-bodley