Skip to content
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

feat: Move Database Import option into DB Connection modal #19314

Merged
merged 35 commits into from
Apr 8, 2022

Conversation

lyndsiWilliams
Copy link
Member

@lyndsiWilliams lyndsiWilliams commented Mar 22, 2022

SUMMARY

This PR moves the import database from file option inside of the database connection modal.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

BEFORE:

importBefore

passwordBefore

overwriteBefore

AFTER:

importAfter

passwordAfter

overwriteAfter

pwowAfter

TESTING INSTRUCTIONS

  • Open the database connection modal and click "Import database from file" at the bottom
  • Import a database from file that requires a password to see the new password confirmation
  • Import a database from file that already exists to see the new overwrite confirmation
  • If you import a database that doesn't already exist and doesn't require a password, the modal should close and add the database with no further confirmation.

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

@superset-github-bot superset-github-bot bot added the Superset-Community-Partners Preset community partner program participants label Mar 22, 2022
@lyndsiWilliams lyndsiWilliams changed the title [WIP] feature: Move Database Import option into DB Connection modal feature: Move Database Import option into DB Connection modal Mar 29, 2022
@apache apache deleted a comment from github-actions bot Mar 29, 2022
@apache apache deleted a comment from github-actions bot Mar 29, 2022
@apache apache deleted a comment from github-actions bot Mar 29, 2022
@apache apache deleted a comment from github-actions bot Mar 29, 2022
@lyndsiWilliams lyndsiWilliams changed the title feature: Move Database Import option into DB Connection modal feat: Move Database Import option into DB Connection modal Mar 29, 2022
@lyndsiWilliams lyndsiWilliams removed the Superset-Community-Partners Preset community partner program participants label Mar 29, 2022
@lyndsiWilliams lyndsiWilliams marked this pull request as ready for review March 29, 2022 11:47
@lyndsiWilliams
Copy link
Member Author

/testenv up

@github-actions
Copy link
Contributor

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

@github-actions
Copy link
Contributor

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

@lyndsiWilliams
Copy link
Member Author

/testenv up

@github-actions
Copy link
Contributor

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

@codecov
Copy link

codecov bot commented Mar 29, 2022

Codecov Report

Merging #19314 (b4c6b6a) into master (b689ac2) will increase coverage by 0.00%.
The diff coverage is 40.21%.

@@           Coverage Diff            @@
##           master   #19314    +/-   ##
========================================
  Coverage   66.43%   66.44%            
========================================
  Files        1670     1678     +8     
  Lines       63958    64243   +285     
  Branches     6506     6584    +78     
========================================
+ Hits        42490    42684   +194     
- Misses      19782    19856    +74     
- Partials     1686     1703    +17     
Flag Coverage Δ
javascript 51.46% <40.21%> (+0.14%) ⬆️

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

Impacted Files Coverage Δ
superset-frontend/src/components/Button/index.tsx 100.00% <ø> (ø)
...tend/src/views/CRUD/data/database/DatabaseList.tsx 67.94% <ø> (+0.55%) ⬆️
...c/views/CRUD/data/database/DatabaseModal/index.tsx 31.80% <37.17%> (-1.10%) ⬇️
...s/CRUD/data/database/DatabaseModal/ModalHeader.tsx 66.66% <37.50%> (-3.71%) ⬇️
superset-frontend/src/views/CRUD/hooks.ts 46.40% <66.66%> (-0.76%) ⬇️
...c/views/CRUD/data/database/DatabaseModal/styles.ts 76.69% <100.00%> (+0.69%) ⬆️
.../components/ExploreAdditionalActionsMenu/index.jsx 57.14% <0.00%> (-38.32%) ⬇️
...set-frontend/src/components/ReportModal/styles.tsx 87.50% <0.00%> (-12.50%) ⬇️
...src/BigNumber/BigNumberWithTrendline/buildQuery.ts 0.00% <0.00%> (-11.12%) ⬇️
...rc/explore/components/ExploreChartHeader/index.jsx 42.10% <0.00%> (-6.58%) ⬇️
... and 117 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b689ac2...b4c6b6a. Read the comment docs.

@github-actions
Copy link
Contributor

github-actions bot commented Apr 5, 2022

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

@@ -381,6 +381,7 @@ interface ImportResourceState {
loading: boolean;
passwordsNeeded: string[];
alreadyExists: string[];
errored: boolean;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: name this failed

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can do! Fixed in this commit.

@lyndsiWilliams
Copy link
Member Author

/testenv up

@github-actions
Copy link
Contributor

github-actions bot commented Apr 7, 2022

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

@yousoph yousoph closed this Apr 7, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Apr 7, 2022

Ephemeral environment shutdown and build artifacts deleted.

@yousoph yousoph reopened this Apr 7, 2022
@yousoph
Copy link
Member

yousoph commented Apr 7, 2022

/testenv up

@github-actions
Copy link
Contributor

github-actions bot commented Apr 7, 2022

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

@github-actions
Copy link
Contributor

github-actions bot commented Apr 7, 2022

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

@lyndsiWilliams
Copy link
Member Author

/testenv up

@github-actions
Copy link
Contributor

github-actions bot commented Apr 7, 2022

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

if (onDatabaseAdd) {
onDatabaseAdd();
}
if (onDatabaseAdd) onDatabaseAdd();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit, but a little cleaner for next time:

Suggested change
if (onDatabaseAdd) onDatabaseAdd();
onDatabaseAdd && onDatabaseAdd();

setHasConnectedDb(false);
}
if (editNewDb) setHasConnectedDb(false);
if (importingModal) setImportingModal(false);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

another nit that we can prob address in a later cleanup. The name importingModal sounds like a current state where the modal is importing.

Copy link
Member

@eschutho eschutho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job @lyndsiWilliams!

@eschutho eschutho merged commit d52e386 into apache:master Apr 8, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Apr 8, 2022

Ephemeral environment shutdown and build artifacts deleted.

philipher29 pushed a commit to ValtechMobility/superset that referenced this pull request Jun 9, 2022
)

* rebase

* more progress

* Fix unintended changes

* DB import goes to step 3

* debugging

* DB list refreshing properly

* import screens flowing properly

* Code cleanup

* Fixed back button on import flow

* Remove import db tooltip test

* Fix test

* Password field resets properly

* Changed import modal state dictators and removed unneeded comment

* Removed unneeded param pass and corrected modal spelling

* Fixed typos

* Changed file to fileList

* Clarified import footer comment

* Cleaned passwordNeededField and confirmOverwriteField state checks

* debugging

* Import state flow fixed

* Removed unneeded importModal check in unreachable area

* Fixed import db footer behavior when pressing back on step 2

* Import db button now at 14px

* Removed animation from import db button

* Fixed doble-loading successToast

* Fixed errored import behavior

* Updated import password check info box text

* Connect button disables while importing db is loading

* Connect button disables while overwrite confirmation is still needed

* Connect button disables while password confirmation is still needed

* Removed gray line under upload filename

* Hide trashcan icon on import filename

* Modal scroll behavior fixed for importing filename

* Changed errored to failed

* RTL testing for db import
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.0.0 labels Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels preset-io size/XL 🚢 2.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants