Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: allow creating option in table select in dataset add modal #10369

Merged
merged 1 commit into from
Jul 24, 2020

Conversation

nytai
Copy link
Member

@nytai nytai commented Jul 21, 2020

SUMMARY

There is currently an issue with adding google sheets as datasets. The dataset add modal expects there to be a schema selected before table select is enabled. Since google sheets don't have schemas there is no way to add a google sheet as a dataset. The fix is:

  • Allow creating option in dataset add modal. Google sheets urls can now be pasted in the select field.
  • schema is now optional. Simply leave it blank to add a google sheet.

Trade Offs: there will now be a higher possibility for errors as users will be able to submit the form without shema and the table option now allows input so users can paste non-existent tables into the select field. Long term we should consider an api that can tell us if a database 1. supports schemas, 2. allows free-form table inputs

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Screen Shot 2020-07-20 at 5 34 40 PM

Screen Shot 2020-07-20 at 5 35 00 PM

TEST PLAN

  • can add a google sheet as a dataset.
  • can add a regular table as a dataset
  • unit tests pass.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Changes UI (behind ENABLE_REACT_CRUD_VIEWS flag)
  • Requires DB Migration.
  • Confirm DB Migration upgrade and downgrade tested.
  • Introduces new feature or API
  • Removes existing feature or API

@codecov-commenter
Copy link

Codecov Report

Merging #10369 into master will increase coverage by 0.74%.
The diff coverage is 62.50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #10369      +/-   ##
==========================================
+ Coverage   69.70%   70.45%   +0.74%     
==========================================
  Files         196      603     +407     
  Lines       18950    32431   +13481     
  Branches        0     3294    +3294     
==========================================
+ Hits        13210    22848    +9638     
- Misses       5740     9477    +3737     
- Partials        0      106     +106     
Flag Coverage Δ
#cypress 55.38% <66.66%> (?)
#javascript 59.33% <62.50%> (?)
#python 69.78% <ø> (+0.07%) ⬆️

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

Impacted Files Coverage Δ
...frontend/src/views/datasetList/AddDatasetModal.tsx 57.14% <0.00%> (ø)
superset-frontend/src/components/TableSelector.jsx 85.81% <83.33%> (ø)
superset/utils/pandas_postprocessing.py 76.96% <0.00%> (-12.98%) ⬇️
superset/errors.py 93.75% <0.00%> (-6.25%) ⬇️
superset/exceptions.py 95.12% <0.00%> (-4.88%) ⬇️
superset/views/base.py 72.50% <0.00%> (-0.92%) ⬇️
superset/viz.py 57.04% <0.00%> (-0.34%) ⬇️
superset/viz_sip38.py 0.00% <0.00%> (ø)
superset/models/slice.py 84.65% <0.00%> (ø)
superset/charts/schemas.py 100.00% <0.00%> (ø)
... and 421 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 1a41ea4...ae7f6c1. Read the comment docs.

@nytai nytai marked this pull request as ready for review July 21, 2020 22:46
Copy link
Member

@pkdotson pkdotson left a comment

Choose a reason for hiding this comment

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

LGTM!

@nytai nytai force-pushed the tai/fix-dataset-edit branch from ae7f6c1 to 02135ec Compare July 24, 2020 19:55
@nytai nytai merged commit 09dfbab into apache:master Jul 24, 2020
@nytai nytai deleted the tai/fix-dataset-edit branch July 24, 2020 20:17
craig-rueda pushed a commit to preset-io/superset that referenced this pull request Aug 21, 2020
craig-rueda pushed a commit to preset-io/superset that referenced this pull request Aug 24, 2020
villebro pushed a commit that referenced this pull request Sep 5, 2020
auxten pushed a commit to auxten/incubator-superset that referenced this pull request Nov 20, 2020
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.38.0 labels Mar 12, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/M 🚢 0.38.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants