-
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
[datasets] new, listview (react) #9197
Conversation
c81ed53
to
b78a771
Compare
b78a771
to
abfd113
Compare
Codecov Report
@@ Coverage Diff @@
## master #9197 +/- ##
==========================================
+ Coverage 58.90% 59.10% +0.19%
==========================================
Files 373 374 +1
Lines 12026 12159 +133
Branches 2953 2982 +29
==========================================
+ Hits 7084 7186 +102
- Misses 4763 4794 +31
Partials 179 179
Continue to review full report at Codecov.
|
e0d01af
to
487abac
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.
Two type nits
@villebro Thanks for catching that, I also found that my implementation of the database filter was incorrect. Both issues have been addressed. I've also added an exception which should help catch incorrect filter config moving forward. |
fe5d676
to
e701b21
Compare
e701b21
to
5d70c25
Compare
|
||
handleDashboardDelete = ({ id, table_name: tableName }: Dataset) => | ||
SupersetClient.delete({ | ||
endpoint: `/api/v1/dashboard/${id}`, |
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 think you want a different endpoint here
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.
There seems to be some JS lint, otherwise LGTM. This is a really nice improvement to the user experience, great work! 👍
I just realized this change will remove the create button for datasets(tables). For Charts and Dashboards there was the |
9011584
to
22c68ea
Compare
CATEGORY
Choose one
SUMMARY
Adds a new react listview to tables (soon to be renamed to datasets).
similar to #8845 and #8999
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TEST PLAN
ENABLE_REACT_CRUD_VIEWS
) provides the same functionality as previous MVC crud viewsENABLE_REACT_CRUD_VIEWS
ADDITIONAL INFORMATION
REVIEWERS