feat: save database with new dynamic form #14583

eschutho · 2021-05-12T01:48:50Z

SUMMARY

This PR loads the form dynamically and saves the database with the new form. The dynamic form is hidden behind a local flag, so it does not currently show up in the UI.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

existing visualization is mostly the same with a few small tweaks

The tabs now reach across the fill width of the modal

Display name used to say "Name your dataset" as the placeholder. It's changed to "Name your database"

with button enabled (same as before)

Screen.Recording.2021-05-12.at.5.04.51.PM.mov

Go from dynamic form to SQLAlchemy form

Screen.Recording.2021-05-20.at.5.55.30.PM.mov

Tab animation

To test this locally with the dynamic form change this value:

setDB({
        type: ActionType.dbSelected,
        payload: {
          parameters: { engine: 'postgresql' },
          configuration_method: CONFIGURATION_METHOD.SQLALCHEMY_URI,
        },

to CONFIGURATION_METHOD.DYNAMIC_FORM

NEW FORM (hidden behind the hard-coded value above)

On creation/form load

Required fields are filled out and button is enabled

After "connect"

TEST PLAN

new unit tests.. because some values are still hard coded, we can't test the new flow too much yet.

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

eschutho · 2021-05-12T01:57:26Z

This PR includes #14456

codecov · 2021-05-12T02:09:32Z

Codecov Report

Merging #14583 (ae9bb8f) into master (9729ffd) will decrease coverage by 0.03%.
The diff coverage is 67.08%.

❗ Current head ae9bb8f differs from pull request most recent head 6eae68c. Consider uploading reports for the commit 6eae68c to get more accurate results

@@            Coverage Diff             @@
##           master   #14583      +/-   ##
==========================================
- Coverage   77.45%   77.42%   -0.04%     
==========================================
  Files         959      960       +1     
  Lines       48719    48791      +72     
  Branches     6115     6122       +7     
==========================================
+ Hits        37736    37777      +41     
- Misses      10777    10811      +34     
+ Partials      206      203       -3

Flag	Coverage Δ
hive	`81.10% <100.00%> (+<0.01%)`	⬆️
mysql	`81.38% <100.00%> (+<0.01%)`	⬆️
postgres	`81.40% <100.00%> (+<0.01%)`	⬆️
python	`81.78% <100.00%> (+<0.01%)`	⬆️
sqlite	`81.02% <100.00%> (+<0.01%)`	⬆️

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

Impacted Files	Coverage Δ
superset-frontend/src/SqlLab/App.jsx	`0.00% <0.00%> (ø)`
...uperset-frontend/src/components/Form/FormLabel.tsx	`64.70% <0.00%> (-4.05%)`	⬇️
...components/DashboardBuilder/DashboardContainer.tsx	`100.00% <ø> (ø)`
...ashboard/components/gridComponents/ChartHolder.jsx	`77.77% <ø> (+0.94%)`	⬆️
...nents/nativeFilters/FilterBar/FilterSets/index.tsx	`77.00% <ø> (ø)`
superset-frontend/src/dashboard/reducers/types.ts	`0.00% <ø> (ø)`
...filters/components/GroupBy/GroupByFilterPlugin.tsx	`0.00% <0.00%> (ø)`
...d/src/filters/components/GroupBy/transformProps.ts	`22.22% <0.00%> (-11.12%)`	⬇️
...t-frontend/src/filters/components/GroupBy/types.ts	`100.00% <ø> (ø)`
...src/filters/components/Range/RangeFilterPlugin.tsx	`0.00% <0.00%> (ø)`
... and 38 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 9729ffd...6eae68c. Read the comment docs.

eschutho · 2021-05-13T00:38:44Z

superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.tsx

+        antDTabsStyles,
+        antDModalStyles(theme),
+        antDModalNoPaddingStyles,
+        formHelperStyles(theme),


in a future version, I'd like to pass a prop to the modal for theming, or use something like Modal.noPadding to make the changes more global. Inspired by @rusackas.

superset-frontend/src/views/CRUD/data/database/types.ts

AAfghahi · 2021-05-21T16:30:52Z

superset-frontend/src/views/CRUD/data/database/types.ts

+// the values should align with the database
+// model enum in superset/superset/models/core.py
+export enum CONFIGURATION_METHOD {
+  SQLALCHEMY_URI = 'sqlalchemy_form',


nit
This was changed to SQLALCHEMY_FORM in the backend:
https://github.com/preset-io/superset/blob/master/superset/models/core.py#L103

it shouldn't matter what the key is on the front end, but yeah, maybe for consistency I'll change it in a future iteration. Thanks for pointing that out!

yup! I meant for just consistency.

We should also figure out a way to centralize this enum across the backend and frontend

hughhhh · 2021-05-21T21:36:32Z

superset-frontend/src/views/CRUD/data/database/DatabaseModal/DatabaseConnectionForm.tsx

+};
+
+const FORM_FIELD_MAP = {
+  host: {


nit: go with component based mapping like @betodealmeida was mentioning in our meeting this can be a later iteration but definitely think we implement this pattern
https://app.clubhouse.io/preset/story/15898/make-databaseconnectionform-tsx-have-component-based-mapping

* 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) ...

* split db modal file * split db modal file * hook up available databases * add comment

superset-github-bot bot added the preset-io label May 12, 2021

pull-request-size bot added the size/XL label May 12, 2021

eschutho force-pushed the elizabeth/save-db-form branch 2 times, most recently from 9e31ec2 to 18af9ca Compare May 13, 2021 00:27

eschutho mentioned this pull request May 13, 2021

feat: postgres connection form #14456

Closed

8 tasks

eschutho commented May 13, 2021

View reviewed changes

eschutho mentioned this pull request May 14, 2021

feat: Surface parameters fields for database model #14646

Closed

8 tasks

This was referenced May 17, 2021

feat: Adding configuration_method column to Database Model #14433

Merged

feat: Configuration Method and expanded parameters for Database Model #14451

Merged

hughhhh reviewed May 17, 2021

View reviewed changes

superset-frontend/src/views/CRUD/data/database/types.ts Show resolved Hide resolved

eschutho force-pushed the elizabeth/save-db-form branch 2 times, most recently from 040821f to 429a793 Compare May 19, 2021 01:24

eschutho added 3 commits May 20, 2021 15:52

split db modal file

05a1bb7

split db modal file

a8afce9

hook up available databases

b69bf16

eschutho force-pushed the elizabeth/save-db-form branch from 429a793 to 7244ec3 Compare May 21, 2021 01:07

add comment

6eae68c

eschutho force-pushed the elizabeth/save-db-form branch from 7244ec3 to 6eae68c Compare May 21, 2021 16:12

AAfghahi reviewed May 21, 2021

View reviewed changes

hughhhh reviewed May 21, 2021

View reviewed changes

hughhhh self-requested a review May 21, 2021 21:38

hughhhh approved these changes May 21, 2021

View reviewed changes

hughhhh merged commit c7aee4e into apache:master May 21, 2021

eschutho deleted the elizabeth/save-db-form branch May 24, 2021 17:28

cccs-RyanS pushed a commit to CybercentreCanada/superset that referenced this pull request Dec 17, 2021

feat: save database with new dynamic form (apache#14583)

0cfa337

* split db modal file * split db modal file * hook up available databases * add comment

QAlexBall pushed a commit to QAlexBall/superset that referenced this pull request Dec 29, 2021

feat: save database with new dynamic form (apache#14583)

2300392

* split db modal file * split db modal file * hook up available databases * add comment

cccs-rc pushed a commit to CybercentreCanada/superset that referenced this pull request Mar 6, 2024

feat: save database with new dynamic form (apache#14583)

8676fbd

* split db modal file * split db modal file * hook up available databases * add comment

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: save database with new dynamic form #14583

feat: save database with new dynamic form #14583

eschutho commented May 12, 2021 •

edited

Loading

eschutho commented May 12, 2021

codecov bot commented May 12, 2021 •

edited

Loading

eschutho May 13, 2021

AAfghahi May 21, 2021 •

edited

Loading

eschutho May 21, 2021

AAfghahi May 21, 2021

hughhhh May 21, 2021

hughhhh May 21, 2021

feat: save database with new dynamic form #14583

feat: save database with new dynamic form #14583

Conversation

eschutho commented May 12, 2021 • edited Loading

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TEST PLAN

ADDITIONAL INFORMATION

eschutho commented May 12, 2021

codecov bot commented May 12, 2021 • edited Loading

Codecov Report

eschutho May 13, 2021

Choose a reason for hiding this comment

AAfghahi May 21, 2021 • edited Loading

Choose a reason for hiding this comment

eschutho May 21, 2021

Choose a reason for hiding this comment

AAfghahi May 21, 2021

Choose a reason for hiding this comment

hughhhh May 21, 2021

Choose a reason for hiding this comment

hughhhh May 21, 2021

Choose a reason for hiding this comment

eschutho commented May 12, 2021 •

edited

Loading

codecov bot commented May 12, 2021 •

edited

Loading

AAfghahi May 21, 2021 •

edited

Loading