Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Jan 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions superset-frontend/src/dashboard/actions/sliceEntities.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,17 +74,15 @@ export function fetchAllSlices(userId) {
const slices = {};
json.result.forEach(slice => {
let form_data = JSON.parse(slice.params);
let { datasource } = form_data;
if (!datasource) {
datasource = getDatasourceParameter(
slice.datasource_id,
slice.datasource_type,
);
form_data = {
...form_data,
datasource,
};
}
form_data = {
...form_data,
// force using datasource stored in relational table prop
datasource:
getDatasourceParameter(
slice.datasource_id,
slice.datasource_type,
) || form_data.datasource,
};
slices[slice.id] = {
slice_id: slice.id,
slice_url: slice.url,
Expand All @@ -93,6 +91,8 @@ export function fetchAllSlices(userId) {
form_data,
datasource_name: slice.datasource_name_text,
datasource_url: slice.datasource_url,
datasource_id: slice.datasource_id,
datasource_type: slice.datasource_type,
changed_on: new Date(slice.changed_on_utc).getTime(),
description: slice.description,
description_markdown: slice.description_markeddown,
Expand Down
26 changes: 15 additions & 11 deletions superset/charts/commands/importers/v1/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
from superset.charts.dao import ChartDAO
from superset.charts.schemas import ImportV1ChartSchema
from superset.commands.importers.v1 import ImportModelsCommand
from superset.connectors.sqla.models import SqlaTable
from superset.databases.commands.importers.v1.utils import import_database
from superset.databases.schemas import ImportV1DatabaseSchema
from superset.datasets.commands.importers.v1.utils import import_dataset
Expand Down Expand Up @@ -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] = {}
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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

for file_name, config in configs.items():
if (
file_name.startswith("datasets/")
and config["database_uuid"] in database_ids
):
config["database_id"] = database_ids[config["database_uuid"]]
dataset = import_dataset(session, config, overwrite=False)
dataset_info[str(dataset.uuid)] = {
"datasource_id": dataset.id,
"datasource_type": "view" if dataset.is_sqllab_view else "table",
"datasource_name": dataset.table_name,
}
datasets[str(dataset.uuid)] = dataset

# import charts with the correct parent ref
for file_name, config in configs.items():
if (
file_name.startswith("charts/")
and config["dataset_uuid"] in dataset_info
):
if file_name.startswith("charts/") and config["dataset_uuid"] in datasets:
# update datasource id, type, and name
config.update(dataset_info[config["dataset_uuid"]])
dataset = datasets[config["dataset_uuid"]]
config.update(
{
"datasource_id": dataset.id,
"datasource_type": "view"
if dataset.is_sqllab_view
else "table",
"datasource_name": dataset.table_name,
}
)
config["params"].update({"datasource": dataset.uid})
import_chart(session, config, overwrite=overwrite)
7 changes: 5 additions & 2 deletions tests/charts/commands_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,10 +140,13 @@ def test_import_v1_chart(self):
command = ImportChartsCommand(contents)
command.run()

chart = db.session.query(Slice).filter_by(uuid=chart_config["uuid"]).one()
chart: Slice = db.session.query(Slice).filter_by(
uuid=chart_config["uuid"]
).one()
dataset = chart.datasource
assert json.loads(chart.params) == {
"color_picker": {"a": 1, "b": 135, "g": 122, "r": 0},
"datasource": "12__table",
"datasource": dataset.uid,
"js_columns": ["color"],
"js_data_mutator": "data => data.map(d => ({\\n ...d,\\n color: colors.hexToRGB(d.extraProps.color)\\n}));",
"js_onclick_href": "",
Expand Down