feat: Dataset Creation Footer Component #21241

AAfghahi · 2022-08-29T16:35:47Z

SUMMARY

This creates the footer component for the new dataset creation flow. Currently it does not set the name of the dataset, so default naming is used. However, this will be added after this: #21189 is merged in.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Screen.Recording.2022-09-08.at.3.39.05.PM.mov

TESTING INSTRUCTIONS

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-08-29T17:58:26Z

Codecov Report

Merging #21241 (8e2b2c1) into master (f27e20e) will decrease coverage by 0.02%.
The diff coverage is 52.83%.

@@            Coverage Diff             @@
##           master   #21241      +/-   ##
==========================================
- Coverage   66.66%   66.64%   -0.03%     
==========================================
  Files        1793     1793              
  Lines       68499    68852     +353     
  Branches     7278     7415     +137     
==========================================
+ Hits        45666    45886     +220     
- Misses      20969    21067      +98     
- Partials     1864     1899      +35

Flag	Coverage Δ
hive	`53.08% <ø> (ø)`
javascript	`52.88% <52.83%> (+0.06%)`	⬆️
mysql	`78.20% <ø> (ø)`
postgres	`78.27% <ø> (ø)`
presto	`52.98% <ø> (ø)`
python	`81.41% <ø> (ø)`
sqlite	`76.76% <ø> (ø)`
unit	`51.06% <ø> (ø)`

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

Impacted Files	Coverage Δ
...iews/CRUD/data/dataset/AddDataset/Footer/index.tsx	`33.33% <33.33%> (-66.67%)`	⬇️
...s/CRUD/data/dataset/AddDataset/LeftPanel/index.tsx	`47.61% <54.54%> (+0.34%)`	⬆️
superset-frontend/src/logger/LogUtils.ts	`96.00% <100.00%> (+1.26%)`	⬆️
...d/src/views/CRUD/data/dataset/AddDataset/index.tsx	`52.94% <100.00%> (+10.08%)`	⬆️
...set-frontend/src/views/CRUD/data/dataset/styles.ts	`100.00% <100.00%> (ø)`
superset-frontend/src/SqlLab/constants.ts	`100.00% <0.00%> (ø)`
...src/dashboard/components/PropertiesModal/index.tsx	`61.97% <0.00%> (+0.15%)`	⬆️
...et-frontend/src/components/TableSelector/index.tsx	`80.00% <0.00%> (+4.00%)`	⬆️
...frontend/src/SqlLab/components/SqlEditor/index.jsx	`56.90% <0.00%> (+5.48%)`	⬆️
...tend/plugins/plugin-chart-table/src/TableChart.tsx	`48.33% <0.00%> (+7.87%)`	⬆️

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

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

pkdotson · 2022-09-08T19:51:28Z

/testenv up

github-actions · 2022-09-08T19:52:17Z

@pkdotson Container image not yet published for this PR. Please try again when build is complete.

github-actions · 2022-09-08T19:52:18Z

@pkdotson Ephemeral environment creation failed. Please check the Actions logs for details.

Antonio-RiveroMartnez · 2022-09-08T19:50:02Z

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


-export default function Footer() {
-  return <div>Footer</div>;
+interface FooterObject {


Could we rename this to be FooterProps? So it's more consistent with how we name our prop interfaces in other components.

hughhhh · 2022-09-08T20:01:53Z

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

@@ -253,7 +273,14 @@ export default function LeftPanel({
          <div className="options-list" data-test="options-list">
            {!refresh &&
              tableOptions.map((o, i) => (


label this as option instead of o, and idx instead of i

Antonio-RiveroMartnez · 2022-09-08T20:08:37Z

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

 describe('Footer', () => {
-  it('renders a blank state Footer', () => {
-    render(<Footer />);


It seems like we were rendering this component before without required props, and now, url and addDangerToast are required. Question, are there any other call to this component from before that was not updated to send the new props?

This is a good question. This was a test that was created as a placeholder while we were implementing the blank state PR. Now that it is finished we are changing it to be reflective of the actual behavior.

Antonio-RiveroMartnez · 2022-09-08T20:12:37Z

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

 }
+
+function Footer({ url, datasetObject, addDangerToast }: FooterObject) {


I just realize we have two Footer components:
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/Footer/Footer.tsx
superset-frontend/src/views/CRUD/data/dataset/AddDataset/Footer/index.tsx

Isn't weird to have two components called<Footer> with different FooterProps in the same codebase? Should we consider a rename for one of them?

I don't think it is that weird. One of these is the footer for the native Filter the other is for AddDataset. I think that the folder structure helps differentiate them. How would you rename them?

Well, it depends, if we are keeping them separately like two individual Footers, I'd probably Call them FilterFooter and DatasetFooter or similar, so I know which Footer I'm working with. But yeah, the path is descriptive as you mention, not a blocker, more likely just personal taste, so nvm.

Antonio-RiveroMartnez

LGTM!

AAfghahi · 2022-09-12T17:09:20Z

/testenv up

github-actions · 2022-09-12T17:11:52Z

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

pkdotson

lgtm!

eschutho · 2022-09-14T17:57:17Z

superset-frontend/src/logger/LogUtils.ts

@@ -56,6 +65,14 @@ export const LOG_EVENT_TYPE_USER = new Set([
  LOG_ACTIONS_MOUNT_EXPLORER,
 ]);

+export const LOG_EVENT_DATASET_TYPE_DATASET_CREATION = new Set([


what is this set used for?

Yeah I was trying to figure that out as well, because you end up just importing them straight into the component either way. It was the pattern used by the other logging actions above so I copied it, but those sets are also not invoked anywhere. Happy to delete.

I removed the set in this commit.

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

jinghua-qa · 2022-09-15T06:08:03Z

Found 2 issues:
1, there is empty line between the search box and the table list, is this expected?

2, when user searched a non-existed table name and deleted the name before table list refresh, the selected database table section will be refreshed too, is this expected?

delete.table.search.mov

jinghua-qa · 2022-09-15T07:02:39Z

One more suggestion:
can we add a delete button to remove search word in the search box?

lyndsiWilliams · 2022-09-20T14:23:54Z

2, when user searched a non-existed table name and deleted the name before table list refresh, the selected database table section will be refreshed too, is this expected?

delete.table.search.mov

@jinghua-qa The left panel initially used server side searching, which may have cause this. Server side searching was disabled recently so currently the table search isn't working. I'll be fixing the search so that it uses client side searching in my next PR, I will watch for this behavior while I'm fixing it but the search will not work in this PR.

For clarity: The entire table list should still display once a schema is chosen, you just can't search through it until my next fix PR.

lyndsiWilliams · 2022-09-20T16:45:08Z

/testenv up

github-actions · 2022-09-20T16:58:44Z

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

lyndsiWilliams · 2022-09-20T17:02:21Z

@jinghua-qa I addressed all your comments above, could you take a look and make sure everything looks right now?

jinghua-qa · 2022-09-23T07:35:34Z

Re-test in http://35.92.221.221:8080/, find an issue that the table list did not change with the search keyword:
repo steps:
1, input keyword in the search box
expect:
table list will show match result with keyword
actual:
table list did not change

Screen.Recording.2022-09-23.at.12.32.23.AM.mov

lyndsiWilliams · 2022-09-23T11:38:51Z

Re-test in http://35.92.221.221:8080/, find an issue that the table list did not change with the search keyword: repo steps: 1, input keyword in the search box expect: table list will show match result with keyword actual: table list did not change

Screen.Recording.2022-09-23.at.12.32.23.AM.mov

Yes, I mentioned this a couple of comments above. Server side searching was disabled recently so currently the table search isn't working. I'll be fixing the search so that it uses client side searching in my next PR.

jinghua-qa · 2022-09-23T16:36:54Z

Re-test in http://35.92.221.221:8080/, find an issue that the table list did not change with the search keyword: repo steps: 1, input keyword in the search box expect: table list will show match result with keyword actual: table list did not change
Screen.Recording.2022-09-23.at.12.32.23.AM.mov

Yes, I mentioned this a couple of comments above. Server side searching was disabled recently so currently the table search isn't working. I'll be fixing the search so that it uses client side searching in my next PR.

Sorry have missed your previous comment, i think other than that i did not find other issues~ LGTM

github-actions · 2022-09-23T23:40:30Z

Ephemeral environment shutdown and build artifacts deleted.

pull-request-size bot added the size/M label Aug 29, 2022

AAfghahi force-pushed the arash.afghahi/sc-52806/create-dataset-footer branch from 3885487 to e635fa0 Compare August 29, 2022 16:43

lyndsiWilliams reviewed Aug 29, 2022

View reviewed changes

superset-frontend/src/views/CRUD/data/dataset/AddDataset/Footer/index.tsx Show resolved Hide resolved

pull-request-size bot added size/L and removed size/M labels Aug 31, 2022

AAfghahi force-pushed the arash.afghahi/sc-52806/create-dataset-footer branch from b7d4731 to dc07a5c Compare August 31, 2022 15:39

AAfghahi force-pushed the arash.afghahi/sc-52806/create-dataset-footer branch from dc07a5c to ed55343 Compare September 7, 2022 21:00

AAfghahi marked this pull request as ready for review September 8, 2022 19:41

AAfghahi force-pushed the arash.afghahi/sc-52806/create-dataset-footer branch from e09362a to 18d86fc Compare September 8, 2022 19:49

AAfghahi changed the title ~~feat: rough draft of footer component~~ feat: Dataset Creation Footer Component Sep 8, 2022

AAfghahi force-pushed the arash.afghahi/sc-52806/create-dataset-footer branch from 18d86fc to c2aea81 Compare September 8, 2022 19:55

Antonio-RiveroMartnez reviewed Sep 8, 2022

View reviewed changes

AAfghahi force-pushed the arash.afghahi/sc-52806/create-dataset-footer branch from c2aea81 to 7ff2200 Compare September 8, 2022 20:00

hughhhh reviewed Sep 8, 2022

View reviewed changes

Antonio-RiveroMartnez reviewed Sep 8, 2022

View reviewed changes

AAfghahi force-pushed the arash.afghahi/sc-52806/create-dataset-footer branch 2 times, most recently from 6804515 to c9a8954 Compare September 8, 2022 21:57

Antonio-RiveroMartnez approved these changes Sep 8, 2022

View reviewed changes

pkdotson approved these changes Sep 12, 2022

View reviewed changes

AAfghahi force-pushed the arash.afghahi/sc-52806/create-dataset-footer branch 3 times, most recently from bb2af99 to c365cb0 Compare September 14, 2022 17:14

eschutho reviewed Sep 14, 2022

View reviewed changes

superset-frontend/src/views/CRUD/data/dataset/AddDataset/Footer/index.tsx Show resolved Hide resolved

AAfghahi and others added 8 commits September 19, 2022 23:51

rough draft of footer component

82f4ef8

added tests

d2cc2d9

with create functionality

dce8cb2

implemented feedback

e859b53

fixed tests

a027f51

added logging

1af4659

lint errors

9583534

Rebase with master and address review comments

8e2b2c1

lyndsiWilliams force-pushed the arash.afghahi/sc-52806/create-dataset-footer branch from c365cb0 to 8e2b2c1 Compare September 20, 2022 14:15

lyndsiWilliams added the need:qa-review Requires QA review label Sep 20, 2022

eschutho approved these changes Sep 23, 2022

View reviewed changes

lyndsiWilliams merged commit c4638fa into master Sep 23, 2022

lyndsiWilliams deleted the arash.afghahi/sc-52806/create-dataset-footer branch September 23, 2022 23:40

mistercrunch added the 🚢 2.1.3 label Feb 18, 2024

mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 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

feat: Dataset Creation Footer Component #21241

feat: Dataset Creation Footer Component #21241

AAfghahi commented Aug 29, 2022 •

edited

codecov bot commented Aug 29, 2022 •

edited

pkdotson commented Sep 8, 2022

github-actions bot commented Sep 8, 2022

github-actions bot commented Sep 8, 2022

Antonio-RiveroMartnez Sep 8, 2022

AAfghahi Sep 8, 2022

hughhhh Sep 8, 2022

hughhhh Sep 8, 2022 •

edited

AAfghahi Sep 8, 2022

Antonio-RiveroMartnez Sep 8, 2022

AAfghahi Sep 8, 2022

Antonio-RiveroMartnez Sep 8, 2022

AAfghahi Sep 8, 2022

Antonio-RiveroMartnez Sep 8, 2022

Antonio-RiveroMartnez left a comment

AAfghahi commented Sep 12, 2022

github-actions bot commented Sep 12, 2022

pkdotson left a comment

eschutho Sep 14, 2022

AAfghahi Sep 14, 2022

lyndsiWilliams Sep 20, 2022

jinghua-qa commented Sep 15, 2022 •

edited

jinghua-qa commented Sep 15, 2022

lyndsiWilliams commented Sep 20, 2022

lyndsiWilliams commented Sep 20, 2022

github-actions bot commented Sep 20, 2022

lyndsiWilliams commented Sep 20, 2022

jinghua-qa commented Sep 23, 2022 •

edited

lyndsiWilliams commented Sep 23, 2022

jinghua-qa commented Sep 23, 2022

github-actions bot commented Sep 23, 2022

		}

		function Footer({ url, datasetObject, addDangerToast }: FooterObject) {

feat: Dataset Creation Footer Component #21241

feat: Dataset Creation Footer Component #21241

Conversation

AAfghahi commented Aug 29, 2022 • edited

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

codecov bot commented Aug 29, 2022 • edited

Codecov Report

pkdotson commented Sep 8, 2022

github-actions bot commented Sep 8, 2022

github-actions bot commented Sep 8, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hughhhh Sep 8, 2022 • edited

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

Choose a reason for hiding this comment

Antonio-RiveroMartnez left a comment

Choose a reason for hiding this comment

AAfghahi commented Sep 12, 2022

github-actions bot commented Sep 12, 2022

pkdotson left a 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

jinghua-qa commented Sep 15, 2022 • edited

jinghua-qa commented Sep 15, 2022

lyndsiWilliams commented Sep 20, 2022

lyndsiWilliams commented Sep 20, 2022

github-actions bot commented Sep 20, 2022

lyndsiWilliams commented Sep 20, 2022

jinghua-qa commented Sep 23, 2022 • edited

lyndsiWilliams commented Sep 23, 2022

jinghua-qa commented Sep 23, 2022

github-actions bot commented Sep 23, 2022

AAfghahi commented Aug 29, 2022 •

edited

codecov bot commented Aug 29, 2022 •

edited

hughhhh Sep 8, 2022 •

edited

jinghua-qa commented Sep 15, 2022 •

edited

jinghua-qa commented Sep 23, 2022 •

edited