-
Notifications
You must be signed in to change notification settings - Fork 13.9k
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(embedded): CSV download for chart #20261
fix(embedded): CSV download for chart #20261
Conversation
Codecov Report
@@ Coverage Diff @@
## master #20261 +/- ##
===========================================
- Coverage 66.70% 54.94% -11.76%
===========================================
Files 1739 1739
Lines 65148 65150 +2
Branches 6897 6899 +2
===========================================
- Hits 43457 35798 -7659
- Misses 19938 27598 +7660
- Partials 1753 1754 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
…ix-embedded-csv-download
…ix-embedded-csv-download
/testenv up |
@rusackas Ephemeral environment spinning up at http://54.185.245.155:8080. Credentials are |
Just checking if I understand this pull request correctly. Currently the CSV download option is not available in 1.5.0 when embedding using the sdk. I pulled master and enabled embedding and embedded in a test page and was able to see the option to download the csv from a chart, but selecting the download raised errors in the console about the iframe not having certain permissions (allow-forms, allow popups). Does this pull request correct issue allow user of an embedded dashboard to download the csv from charts? |
@justin-tomlinson this PR will fix issue with downloading CSV but we need to add the allow-forms and allow popups permission to the SDK. I will open other PR to add these two permission to the SDK |
superset-frontend/packages/superset-ui-core/test/connection/SupersetClientClass.test.ts
Outdated
Show resolved
Hide resolved
superset-frontend/packages/superset-ui-core/test/connection/SupersetClientClass.test.ts
Show resolved
Hide resolved
superset-frontend/packages/superset-ui-core/test/connection/SupersetClientClass.test.ts
Outdated
Show resolved
Hide resolved
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 good! Requested changes on some tests which I hope will be considered before a merge, but I feel good approving this.
…persetClientClass.test.ts Co-authored-by: David Aaron Suddjian <1858430+suddjian@users.noreply.github.com>
Ephemeral environment shutdown and build artifacts deleted. |
* move postForm to superset client * lint * fix lint * fix type * update tests * add tests * add test for form submit * add test for request form * lint * fix test * fix tests * more tests * more tests * test * lint * more test for postForm * lint * Update superset-frontend/packages/superset-ui-core/test/connection/SupersetClientClass.test.ts Co-authored-by: David Aaron Suddjian <1858430+suddjian@users.noreply.github.com> * update tests * remove useless test * make test cover happy * make test cover happy * make test cover happy * make codecov happy * make codecov happy Co-authored-by: David Aaron Suddjian <1858430+suddjian@users.noreply.github.com> (cherry picked from commit ab9f72f)
SUMMARY
CSV download didn't work
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION