fix(dashboard): use datasource id from slice metadata #12483

ktmud · 2021-01-13T04:47:02Z

SUMMARY

Fixes #12480

Always use datasource uid (or parameter, or key, whatever you call it) from the slice.datasource_id and slice.datasource_type, instead of the datasource field stored in form_data (slice.params), as they can be out of sync.
Update the import scripts to account for this and update datasource in slice.params with real dataset id associated with uuid.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before

Adding charts from the new samples data in dashboard edit mode throws an error:

After

It does not throw an error

TEST PLAN

CI

ADDITIONAL INFORMATION

Has associated issue: feat: add new dashboard with FCC data #12163 feat: add new dashboard with video game sales #12162 [Dashboard] Adding charts from new samples data in Dashboard edit mode throws unexpected error #12480
Changes UI
Requires DB Migration.
Confirm DB Migration upgrade and downgrade tested.
Introduces new feature or API
Removes existing feature or API

codecov-io · 2021-01-13T04:58:53Z

Codecov Report

Merging #12483 (b6d9a19) into master (8f83afc) will increase coverage by 0.75%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master   #12483      +/-   ##
==========================================
+ Coverage   62.77%   63.52%   +0.75%     
==========================================
  Files        1015      486     -529     
  Lines       49537    29909   -19628     
  Branches     5079        0    -5079     
==========================================
- Hits        31096    19001   -12095     
+ Misses      18232    10908    -7324     
+ Partials      209        0     -209

Flag	Coverage Δ
javascript	`?`
python	`63.52% <100.00%> (-0.56%)`	⬇️

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

Impacted Files	Coverage Δ
superset/charts/commands/importers/v1/__init__.py	`100.00% <100.00%> (ø)`
superset/db_engines/hive.py	`0.00% <0.00%> (-85.72%)`	⬇️
superset/sql_validators/postgres.py	`50.00% <0.00%> (-50.00%)`	⬇️
superset/db_engine_specs/hive.py	`54.61% <0.00%> (-29.24%)`	⬇️
superset/db_engine_specs/presto.py	`70.56% <0.00%> (-11.48%)`	⬇️
superset/db_engine_specs/postgres.py	`82.45% <0.00%> (-3.51%)`	⬇️
superset/examples/world_bank.py	`97.10% <0.00%> (-2.90%)`	⬇️
superset/views/database/mixins.py	`80.70% <0.00%> (-1.76%)`	⬇️
superset/models/core.py	`88.04% <0.00%> (-0.82%)`	⬇️
superset/views/base_api.py	`97.68% <0.00%> (-0.47%)`	⬇️
... and 536 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 8f83afc...b6d9a19. Read the comment docs.

junlincc

LGTM!

Manually tested by adding a few charts created with new sample data, the reported issue seems gone. Thank you for the quick fix! @ktmud

@adam-stasiak could you please test?

NOTE⚠️:

This is only Product and QA approval with NO code reviews
This approval indicates that all UI changes work as expected, with no noticeable regression
This PR is ready for code reviews

adam-stasiak · 2021-01-13T13:37:31Z

manually tested - ok.

villebro

Minor nit, LGTM

villebro · 2021-01-13T16:36:00Z

superset/charts/commands/importers/v1/__init__.py

@@ -69,26 +70,29 @@ def _import(
                database_ids[str(database.uuid)] = database.id

        # import datasets with the correct parent ref
-        dataset_info: Dict[str, Dict[str, Any]] = {}
+        datasets: Dict[str, SqlaTable] = {}


While this is nowadays practically always SqlaTable, strictly speaking I think this should be BaseDatasource.

Current importer doesn't support Druid datasource anyway, BaseDatasource does not have other attributes referenced at L92 and L94.

villebro · 2021-01-13T16:49:45Z

Merging for inclusion in 1.0.0rc2.

* release: bump to 1.0.0 and CHANGELOG * fix(explore): long metric name display (#12387) * fix(explore): long metric name display * add tooltip to control * chore: Show datasets when search input is empty (#12391) * chore: Fix typo “Rest” to “Reset” (#12392) * chore: upgrade eslint, babel, and prettier (#12393) * feat(explore): add tooltip to timepicker label (#12401) * chore: change Datasource to Dataset in Explore ui (#12402) * chore(explore):change dataset to datasource in ui * modal * Add space * Changing it back🤦🏾‍♀️ * Chargeback * fix: Refresh Interval Modal dropdown (#12406) * fix(native-filters): incorrect queriesData state (#12409) * refactor: from superset.utils.core break down date_parser (#12408) * Fixes control panel fields styling (#12236) (#12326) * feat: Resizable dataset and controls panels on Explore view (#12411) * Implement resizable panels on explore view * Optimize chart rendering while resizing * Make dataset column narrower Co-authored-by: Evan Rusackas <evan@preset.io> * fix(dashboard): artefacts shown while drag and dropping deck.gl charts (#12418) * [12181] Fix artifacts while drag and dropping deck.gl charts. * Run prettier * bump superset-ui packages for rolling window change (#12426) * chore: bump superset-ui deckgl plugin (#12466) * fix: do not show vertical scrollbar for charts in dashboard (#12478) * fix: do not show vertical scrollbar for charts in dashboard * Proper fix for #11419 Co-authored-by: Jesse Yang <jesse.yang@airbnb.com> * fix(dashboard): use datasource id from slice metadata (#12483) * fix(timepicker): make pyparsing thread safe (#12489) * fix: make pyparsing thread safe * remove parenthesis for decorator * fix (SQL Lab): disappearing results on tab switch (#12472) * fix (SQL Lab): disappearing results on tab switch * Remove state * Fix test * fix: import ZIP files that have been modified (#12425) * fix: import ZIP files that have been modified * Add unit test * update changelog with rc2 entries * fix: impose dataset ownership check on old API (#12491) * fix: impose dataset ownership check on old API * update UPDATING.md * partially protect the old MVC also * prevent metric and column add and update * ci: remove refs/tags from docker tags on a release (#12518) * ci: remove refs/tags from docker tags on a release * wider head * fix: lowercase all columns in examples (#12530) * fix(explore): time table control panel (#12532) * fix(explore): Add Time section back to FilterBox (#12537) * Fixing Pinot queries for time granularities: WEEKS/MONTHS/QUARTERS/YEARS (#12536) * fix: Select options overflowing Save chart modal on Explore view (#12522) * Fix select options overflowing modal * fix unit test Co-authored-by: Ville Brofeldt <ville.v.brofeldt@gmail.com> * Fix list filters vertical alignment (#12497) * feat(db-engine): Add support for Apache Solr (#12403) * [db engine] Add support for Apache Solr * Fixing typo * chore: rename docker image in build_docker_image.sh, docker-compose.yml and helm values.yaml (#12337) * add rc3 changelog entries * fix: Popover closes on change of dropdowns values (#12410) * fix: Add MAX_SQL_ROW value to LIMIT_DROPDOWN (#12555) * fix(viz): missing groupby and broken adhoc metrics for boxplot (#12556) * fix: height on grid results (#12558) * fix: case expression should not have double quotes (#12562) * Fix 500 error when loading dashboards with slice having deleted dataset (#12535) * add rc4 changelog entries * Fixed typo on line 348 * Added files Co-authored-by: Daniel Gaspar <danielvazgaspar@gmail.com> Co-authored-by: Yongjie Zhao <yongjie.zhao@gmail.com> Co-authored-by: Geido <60598000+geido@users.noreply.github.com> Co-authored-by: Junlin Chen <junlin@preset.io> Co-authored-by: Jesse Yang <jesse.yang@airbnb.com> Co-authored-by: Agata Stawarz <47450693+agatapst@users.noreply.github.com> Co-authored-by: Ville Brofeldt <33317356+villebro@users.noreply.github.com> Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> Co-authored-by: Kamil Gabryjelski <kamil.gabryjelski@gmail.com> Co-authored-by: Evan Rusackas <evan@preset.io> Co-authored-by: Kasia Kucharczyk <2536609+kkucharc@users.noreply.github.com> Co-authored-by: Phillip Kelley-Dotson <pkelleydotson@yahoo.com> Co-authored-by: Grace Guo <grace.guo@airbnb.com> Co-authored-by: Beto Dealmeida <roberto@dealmeida.net> Co-authored-by: Ville Brofeldt <ville.v.brofeldt@gmail.com> Co-authored-by: Xiang Fu <fx19880617@gmail.com> Co-authored-by: Ahmed Adel <github@aadel.io> Co-authored-by: Amit Miran <47772523+amitmiran137@users.noreply.github.com> Co-authored-by: Hugh A. Miles II <hughmil3s@gmail.com> Co-authored-by: Shuyao Bi <shuyaob@andrew.cmu.edu> Co-authored-by: Lyndsi Kay Williams <lyndsikaywilliams@Lyndsis-MacBook-Pro.local>

pull-request-size bot added the size/M label Jan 13, 2021

ktmud requested review from betodealmeida and junlincc January 13, 2021 04:47

fix(dashboard): use datasource id from slice metadata

b6d9a19

ktmud force-pushed the add-chart-to-dashboard branch from eca3460 to b6d9a19 Compare January 13, 2021 04:57

junlincc approved these changes Jan 13, 2021

View reviewed changes

junlincc added the v1.0 label Jan 13, 2021

villebro approved these changes Jan 13, 2021

View reviewed changes

villebro merged commit f8270b0 into apache:master Jan 13, 2021

villebro pushed a commit that referenced this pull request Jan 13, 2021

fix(dashboard): use datasource id from slice metadata (#12483)

b213c1c

junlincc mentioned this pull request Jan 13, 2021

[Sample]View data section shows no data of new samples dataset #12504

Closed

amitmiran137 pushed a commit to nielsen-oss/superset that referenced this pull request Jan 14, 2021

fix(dashboard): use datasource id from slice metadata (apache#12483)

55ae8d7

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(dashboard): use datasource id from slice metadata #12483

fix(dashboard): use datasource id from slice metadata #12483

ktmud commented Jan 13, 2021

codecov-io commented Jan 13, 2021 •

edited

Loading

junlincc left a comment

adam-stasiak commented Jan 13, 2021

villebro left a comment

villebro Jan 13, 2021

ktmud Jan 13, 2021

villebro commented Jan 13, 2021

fix(dashboard): use datasource id from slice metadata #12483

fix(dashboard): use datasource id from slice metadata #12483

Conversation

ktmud commented Jan 13, 2021

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before

After

TEST PLAN

ADDITIONAL INFORMATION

codecov-io commented Jan 13, 2021 • edited Loading

Codecov Report

junlincc left a comment

Choose a reason for hiding this comment

adam-stasiak commented Jan 13, 2021

villebro left a comment

Choose a reason for hiding this comment

villebro Jan 13, 2021

Choose a reason for hiding this comment

ktmud Jan 13, 2021

Choose a reason for hiding this comment

villebro commented Jan 13, 2021

codecov-io commented Jan 13, 2021 •

edited

Loading