superset icon indicating copy to clipboard operation
superset copied to clipboard

refactor: serialize extra json in state

Open eschutho opened this issue 3 years ago • 3 comments

SUMMARY

refactoring the extra json state in the database connection modal so that it is always serialized, as the api will expect it. Each component will deserialize and push it back to the reducer, which will then serialize it again. This should reduce some duplication of types and state.

This is also in preparation for adding a databricks form which will need to have extra json property as a field.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

No visual changes

TESTING INSTRUCTIONS

Adding and removing extra json in the advanced section should work as expected

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

eschutho avatar Sep 20 '22 00:09 eschutho

/testenv up

eschutho avatar Sep 20 '22 01:09 eschutho

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

github-actions[bot] avatar Sep 20 '22 01:09 github-actions[bot]

Codecov Report

Merging #21523 (9cf9ea1) into master (bd3166b) will increase coverage by 0.02%. The diff coverage is 45.16%.

@@            Coverage Diff             @@
##           master   #21523      +/-   ##
==========================================
+ Coverage   66.88%   66.90%   +0.02%     
==========================================
  Files        1802     1805       +3     
  Lines       68987    69051      +64     
  Branches     7345     7366      +21     
==========================================
+ Hits        46139    46197      +58     
- Misses      20951    20952       +1     
- Partials     1897     1902       +5     
Flag Coverage Δ
javascript 53.29% <45.16%> (+0.06%) :arrow_up:

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

Impacted Files Coverage Δ
...c/views/CRUD/data/database/DatabaseModal/index.tsx 41.99% <41.66%> (+7.86%) :arrow_up:
.../CRUD/data/database/DatabaseModal/ExtraOptions.tsx 75.00% <57.14%> (-11.96%) :arrow_down:
...ontrols/DateFilterControl/utils/dateFilterUtils.ts 39.39% <0.00%> (-60.61%) :arrow_down:
.../explore/exploreUtils/getParsedExploreURLParams.ts 83.33% <0.00%> (-8.10%) :arrow_down:
...nts/controls/DateFilterControl/DateFilterLabel.tsx 35.71% <0.00%> (-5.70%) :arrow_down:
...iews/CRUD/data/dataset/AddDataset/Header/index.tsx 81.81% <0.00%> (-5.69%) :arrow_down:
...perset-frontend/src/views/CRUD/chart/ChartList.tsx 56.61% <0.00%> (-2.36%) :arrow_down:
...ontrols/DndColumnSelectControl/DndFilterSelect.tsx 44.03% <0.00%> (-1.01%) :arrow_down:
...et-ui-chart-controls/src/shared-controls/index.tsx 53.44% <0.00%> (-0.79%) :arrow_down:
superset-frontend/src/explore/constants.ts 100.00% <0.00%> (ø)
... and 40 more

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

codecov[bot] avatar Sep 20 '22 01:09 codecov[bot]

@eschutho when opening the "Other" tab under "Advanced" the page crashes. I checked it on master and it is working fine, the bug appears only on the ephemeral env. I think it is worth having a look

geido avatar Sep 21 '22 12:09 geido

@eschutho when opening the "Other" tab under "Advanced" the page crashes. I checked it on master and it is working fine, the bug appears only on the ephemeral env. I think it is worth having a look

Thanks for testing! I'll take a look.

eschutho avatar Sep 21 '22 17:09 eschutho

The "Allow this database to be explored" setting should be checked by default in the Database Connection UI. I can't repro the issue in latest master.

https://user-images.githubusercontent.com/90777564/192780216-1ef52100-541c-4c47-9d6e-2db69c2afac6.mp4

andrey-zayats avatar Sep 28 '22 12:09 andrey-zayats

"Cancel query on window unload event" shouldn't be checked by default in db settings (Advanced -> Performance). I can't repro the issue in latest master.

image

UPD: User can't even uncheck the checkbox

andrey-zayats avatar Sep 28 '22 13:09 andrey-zayats

User can't create a dataset using newly connected gsheet.

https://user-images.githubusercontent.com/90777564/192787621-d8ee0ab9-31dc-46c4-8ae0-1bc8145af6e5.mp4

andrey-zayats avatar Sep 28 '22 13:09 andrey-zayats

/testenv up

eschutho avatar Oct 12 '22 19:10 eschutho

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

github-actions[bot] avatar Oct 12 '22 21:10 github-actions[bot]

Ephemeral environment shutdown and build artifacts deleted.

github-actions[bot] avatar Oct 14 '22 21:10 github-actions[bot]