-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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(chart): ensure samples data is displayed #16900
Conversation
Codecov Report
@@ Coverage Diff @@
## master #16900 +/- ##
==========================================
- Coverage 77.13% 77.01% -0.13%
==========================================
Files 1036 1038 +2
Lines 55729 56001 +272
Branches 7627 7742 +115
==========================================
+ Hits 42989 43131 +142
- Misses 12484 12612 +128
- Partials 256 258 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
/testenv up |
superset-frontend/src/explore/components/DataTablesPane/index.tsx
Outdated
Show resolved
Hide resolved
Can we add some unit tests? |
@kgabryje Ephemeral environment spinning up at http://52.12.82.82:8080. Credentials are |
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.
superset-frontend/src/explore/components/DataTablesPane/index.tsx
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 nice! I left a comment with a suggestion how to simplify the logic of setting column names
superset-frontend/src/explore/components/DataTablesPane/index.tsx
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.
LGTM
Ephemeral environment shutdown and build artifacts deleted. |
* initial fix * use data keys for cols * add e2e test * remove code * trim logic
SUMMARY
This pr fixes an issue where the view samples tab does not show the sample data. This problem was seen because the column names weren't updated for the samples tab since the old legacy endpoint does not return columns.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
after
https://user-images.githubusercontent.com/17326228/135015945-ba291b18-3526-4d94-866f-9b980af24f3d.mov
TESTING INSTRUCTIONS
Open up charts that use both legacy and new /chart enpoints and ensure that the VIEW SAMPLES show up in the datatables pane.
ADDITIONAL INFORMATION