fix: Create dataset polish/bug fix #22262

lyndsiWilliams · 2022-11-29T16:26:16Z

SUMMARY

This PR implements the following polishes:

Pagination on the table should be 25 by default (previously set to 10)
Align refresh buttons in the left panel
Align width of selected tables in left panel with search input
- The width was only wrong when there was no scrollbar present. I implemented a function to detect if a scrollbar is present and apply padding only if there is no scrollbar
Remove save button and ... dropdown menu from the top right of the header
- An editing prop was added in the header component so that it can be reused when building the edit dataset component
Brought the "Manage your databases here" blank state from SQL lab to the left panel
Fixed a bug where the table wouldn't take the full height of the dataset panel when there is no "This table already has a dataset" banner at the top

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

BEFORE:

AFTER:

TESTING INSTRUCTIONS

Go to http://localhost:9000/dataset/add/?testing
Select a database and a schema
- Select a schema with over 25 pages to see default for pagination
Observe all the changes described above

ADDITIONAL INFORMATION

Has associated issue:
Required feature flags:
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- Migration is atomic, supports rollback & is backwards-compatible
- Confirm DB migration upgrade and downgrade tested
- Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API

codecov · 2022-11-29T17:42:46Z

Codecov Report

Merging #22262 (985f6a9) into master (04b7a26) will increase coverage by 0.04%.
The diff coverage is 76.66%.

@@            Coverage Diff             @@
##           master   #22262      +/-   ##
==========================================
+ Coverage   66.86%   66.90%   +0.04%     
==========================================
  Files        1846     1847       +1     
  Lines       70510    71241     +731     
  Branches     7723     8039     +316     
==========================================
+ Hits        47144    47665     +521     
- Misses      21364    21534     +170     
- Partials     2002     2042      +40

Flag	Coverage Δ
hive	`52.53% <ø> (ø)`
javascript	`54.04% <76.66%> (+0.25%)`	⬆️
mysql	`?`
postgres	`78.02% <ø> (ø)`
presto	`52.42% <ø> (ø)`
python	`81.20% <ø> (-0.05%)`	⬇️
sqlite	`76.49% <ø> (ø)`
unit	`50.92% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...d/src/SqlLab/components/SqlEditorLeftBar/index.tsx	`52.11% <ø> (+0.05%)`	⬆️
...iews/CRUD/data/dataset/AddDataset/Header/index.tsx	`57.14% <33.33%> (-24.68%)`	⬇️
...s/CRUD/data/dataset/AddDataset/LeftPanel/index.tsx	`84.61% <50.00%> (-2.06%)`	⬇️
...erset-frontend/src/components/EmptyState/index.tsx	`77.55% <75.00%> (-0.23%)`	⬇️
...a/dataset/AddDataset/DatasetPanel/DatasetPanel.tsx	`90.19% <100.00%> (-1.48%)`	⬇️
...set-frontend/src/views/CRUD/data/dataset/styles.ts	`100.00% <100.00%> (ø)`
...d/src/views/CRUD/data/dataset/AddDataset/index.tsx	`51.61% <0.00%> (-8.39%)`	⬇️
superset-frontend/src/views/CRUD/data/hooks.ts	`57.69% <0.00%> (-5.95%)`	⬇️
superset/common/utils/dataframe_utils.py	`90.47% <0.00%> (-4.77%)`	⬇️
... and 44 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

lyndsiWilliams · 2022-11-29T19:50:47Z

/testenv up

github-actions · 2022-11-29T19:52:44Z

@lyndsiWilliams Ephemeral environment spinning up at http://52.24.161.135:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

Antonio-RiveroMartnez · 2022-12-05T20:45:24Z

superset-frontend/src/views/CRUD/data/dataset/AddDataset/DatasetPanel/DatasetPanel.tsx

@@ -248,6 +258,9 @@ const DatasetPanel = ({
  const theme = useTheme();
  const hasColumns = columnList?.length > 0 ?? false;
  const datasetNames = datasets?.map(dataset => dataset.table_name);
+  const tablesWithDatasets = datasets?.find(


should we rename this to singular? tablesWithDatasets -> tableWithDatasets since find only returns one element?

Oh yeah that does make more sense, adjusted in this commit.

Antonio-RiveroMartnez · 2022-12-05T20:47:50Z

superset-frontend/src/views/CRUD/data/dataset/AddDataset/Header/index.tsx

+        />
+      ) : (
+        <StyledCreateDatasetTitle>
+          {title ?? DEFAULT_TITLE}


Do we want to render empty string if passed as title? If not, we should change this to be ||

Good call! Fixed in this commit.

hughhhh · 2022-12-16T17:56:17Z

superset-frontend/src/views/CRUD/data/dataset/AddDataset/DatasetPanel/DatasetPanel.tsx

+                  columns={tableColumnDefinition}
+                  data={columnList}
+                  pageSizeOptions={pageSizeOptions}
+                  defaultPageSize={25}


can we make this a constant at the top?

Can do! Done in this commit.

hughhhh

LGTM, just update the page min to be a constant at the top

AAfghahi · 2022-12-16T17:57:28Z

superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx

+const MANAGE_YOUR_DATABASES_TEXT = t('Manage your databases');
+const HERE_TEXT = t('here');
+
+export const emptyStateComponent = (emptyResultsWithSearch: boolean) => (


If this is going to be exported, I think that we should move it out of the SQlEditorLeftBar? Where are the other empty state components stored? THat's where it should be.

Good thinking! Done in this commit.

…mponents/EmptyState

github-actions · 2022-12-20T16:06:11Z

Ephemeral environment shutdown and build artifacts deleted.

pull-request-size bot added the size/L label Nov 29, 2022

Antonio-RiveroMartnez reviewed Dec 5, 2022

View reviewed changes

Base automatically changed from lyndsi/tables-with-preexisting-datasets to master December 5, 2022 21:43

pull-request-size bot added size/XL and removed size/L labels Dec 5, 2022

lyndsiWilliams added 5 commits December 5, 2022 18:03

optimized setDatasets call

6aa68d7

Remove console.log

9968e6c

Create dataset polish/bug fix

88b089e

Fix dataset tests

50ef62a

Fix rebase issue

8d11c74

lyndsiWilliams force-pushed the lyndsi/create-dataset-polish branch from fc87b75 to 8d11c74 Compare December 6, 2022 00:26

pull-request-size bot added size/L and removed size/XL labels Dec 6, 2022

lyndsiWilliams added 2 commits December 5, 2022 20:25

Implement Eric's table height fix

17d3779

Implement Antonio's review comments

924f67e

hughhhh reviewed Dec 16, 2022

View reviewed changes

hughhhh approved these changes Dec 16, 2022

View reviewed changes

AAfghahi requested changes Dec 16, 2022

View reviewed changes

Make defaultPageSize a const and move emptyStateComponent into src/co…

985f6a9

…mponents/EmptyState

AAfghahi approved these changes Dec 20, 2022

View reviewed changes

AAfghahi merged commit 6b20e74 into master Dec 20, 2022

lyndsiWilliams deleted the lyndsi/create-dataset-polish branch December 20, 2022 17:26

This was referenced Jan 3, 2023

chore: Migrate /superset/search_queries to API v1 #22579

Merged

chore: Migrate /superset/queries/<last_updated_ms> to API v1 #22611

Merged

mistercrunch added the 🚢 2.1.3 label Feb 18, 2024

mistercrunch added the 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels label Mar 13, 2024

mistercrunch added 🚢 2.1.0 and removed 🚢 2.1.3 labels Mar 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Create dataset polish/bug fix #22262

fix: Create dataset polish/bug fix #22262

lyndsiWilliams commented Nov 29, 2022

codecov bot commented Nov 29, 2022 •

edited

lyndsiWilliams commented Nov 29, 2022

github-actions bot commented Nov 29, 2022

Antonio-RiveroMartnez Dec 5, 2022

lyndsiWilliams Dec 6, 2022

Antonio-RiveroMartnez Dec 5, 2022

lyndsiWilliams Dec 6, 2022

hughhhh Dec 16, 2022

lyndsiWilliams Dec 19, 2022

hughhhh left a comment

AAfghahi Dec 16, 2022

lyndsiWilliams Dec 19, 2022

github-actions bot commented Dec 20, 2022

fix: Create dataset polish/bug fix #22262

fix: Create dataset polish/bug fix #22262

Conversation

lyndsiWilliams commented Nov 29, 2022

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

BEFORE:

AFTER:

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

codecov bot commented Nov 29, 2022 • edited

Codecov Report

lyndsiWilliams commented Nov 29, 2022

github-actions bot commented Nov 29, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hughhhh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Dec 20, 2022

codecov bot commented Nov 29, 2022 •

edited