-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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: shorten url with extra request parameters #10693
fix: shorten url with extra request parameters #10693
Conversation
Codecov Report
@@ Coverage Diff @@
## master #10693 +/- ##
==========================================
+ Coverage 64.31% 64.36% +0.05%
==========================================
Files 786 786
Lines 36924 36931 +7
Branches 3514 3514
==========================================
+ Hits 23746 23769 +23
+ Misses 13069 13054 -15
+ Partials 109 108 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
9b73f17
to
c8bd5bb
Compare
c8bd5bb
to
1b124cb
Compare
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.
LGTM. At some point it would be nice to start porting these to TS and do some additional simplification, especially exploreUtils
has some very loaded functions.
(cherry picked from commit 7fc227c)
SUMMARY
In #10651 I added a feature that standalone iframe can use shorten url.
But later I found there is issue for the existed solution: we store full url in
url
tbl, which supposed to be content forform_data
parameter only. For standalone chart, its full url is like:form_data={key1:value1, key2: value2}&standalone=true&height=400
.This PR is to fix this issue:
http://localhost:8080/superset/explore/?r=8015&standalone=true&height=600
TEST PLAN
CI and manual test
ADDITIONAL INFORMATION
@mistercrunch @michellethomas