Skip to content

fix: database modal should close on connect with tab layout#14771

Merged
hughhhh merged 1 commit into
apache:masterfrom
preset-io:elizabeth/close-modal-on-save
May 24, 2021
Merged

fix: database modal should close on connect with tab layout#14771
hughhhh merged 1 commit into
apache:masterfrom
preset-io:elizabeth/close-modal-on-save

Conversation

@eschutho
Copy link
Copy Markdown
Member

SUMMARY

With the new database modal, there is a tabbed layout and a step layout. The step layout doesn't close the modal on save. This PR fixes a bug where the modal wasn't being closed on the tab layout.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Screen.Recording.2021-05-23.at.1.28.53.PM.mov

TESTING INSTRUCTIONS

Open the database modal and create a new database and then hit save. The modal should close when the database saved successfully.
I also removed the enzyme tests because they were being covered by the RTL tests.

ADDITIONAL INFORMATION

  • Has associated issue:
  • 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

// todo we haven't hooked this up to load dynamically yet so
// we can't currently test it
});
it('should close the modal on save if using the sqlalchemy form', async () => {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

new test starts here

@eschutho
Copy link
Copy Markdown
Member Author

thanks @betodealmeida for catching this bug!

@codecov
Copy link
Copy Markdown

codecov Bot commented May 23, 2021

Codecov Report

Merging #14771 (71a206a) into master (d71b8b3) will increase coverage by 0.08%.
The diff coverage is 90.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #14771      +/-   ##
==========================================
+ Coverage   77.62%   77.71%   +0.08%     
==========================================
  Files         962      962              
  Lines       49017    49020       +3     
  Branches     6155     6156       +1     
==========================================
+ Hits        38050    38094      +44     
+ Misses      10763    10723      -40     
+ Partials      204      203       -1     
Flag Coverage Δ
javascript 72.60% <90.00%> (+0.18%) ⬆️

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

Impacted Files Coverage Δ
...RUD/data/database/DatabaseModal/SqlAlchemyForm.tsx 100.00% <ø> (ø)
...c/views/CRUD/data/database/DatabaseModal/index.tsx 76.12% <90.00%> (+12.31%) ⬆️
.../CRUD/data/database/DatabaseModal/ExtraOptions.tsx 93.54% <0.00%> (ø)
.../database/DatabaseModal/DatabaseConnectionForm.tsx 54.54% <0.00%> (+4.54%) ⬆️
superset-frontend/src/views/CRUD/hooks.ts 53.48% <0.00%> (+6.51%) ⬆️
...c/views/CRUD/data/database/DatabaseModal/styles.ts 97.36% <0.00%> (+10.52%) ⬆️

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 d71b8b3...71a206a. Read the comment docs.

Copy link
Copy Markdown
Member

@geido geido left a comment

Choose a reason for hiding this comment

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

LGTM!

@hughhhh hughhhh self-requested a review May 24, 2021 17:13
@hughhhh hughhhh merged commit fbe6f16 into apache:master May 24, 2021
@eschutho eschutho deleted the elizabeth/close-modal-on-save branch May 24, 2021 23:10
amitmiran137 pushed a commit to nielsen-oss/superset that referenced this pull request May 25, 2021
* master: (163 commits)
  fix(native-filters): Manage default value of filters by superset (apache#14785)
  fix: Additional ResultSet tests (apache#14741)
  chore: added BasicParametersMixin to Redshift (apache#14752)
  fix: make dataset list sort case insensitive (apache#14528)
  fix: use encodeURIComponent when getting table metadata (apache#14790)
  fix: ensure engine is outside parameters (apache#14787)
  database modal should close on connect with tab layout (apache#14771)
  feat(native-filters): add search all filter options (apache#14710)
  fix: extra query in Dashboard when native filter enabled (apache#14770)
  chore: Improves the native filters UI/UX - iteration 2 (apache#14753)
  fix(native filters): Fix explore state (apache#14779)
  fix(explore): DndColumnSelect not handling controls with "multi: false" (apache#14737)
  feat: Create BigQuery Parameters for DatabaseModal (apache#14721)
  feat: enable user impersonation in GSheets (apache#14767)
  fix: add DB should not say it's Postgres (apache#14766)
  Revert "fix(dashboard): multiple query trigger when native filter enabled (apache#14734)" (apache#14762)
  feat: save database with new dynamic form (apache#14583)
  fix: save non-parameter DBs (apache#14759)
  chore: Removes ColorSchemeControl.less (apache#14199)
  fix(explore): Icons width (apache#14717)
  ...
cccs-RyanS pushed a commit to CybercentreCanada/superset that referenced this pull request Dec 17, 2021
QAlexBall pushed a commit to QAlexBall/superset that referenced this pull request Dec 29, 2021
cccs-rc pushed a commit to CybercentreCanada/superset that referenced this pull request Mar 6, 2024
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.3.0 First shipped in 1.3.0 labels Mar 12, 2024
qfcwell pushed a commit to qfcwell/superset that referenced this pull request May 12, 2026
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/L 🚢 1.3.0 First shipped in 1.3.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants