Skip to content

Commit

Permalink
fix: Dashboard import commands not correctly replacing charts
Browse files Browse the repository at this point in the history
  • Loading branch information
lindenh committed Oct 17, 2024
1 parent bad48d0 commit ae920f5
Show file tree
Hide file tree
Showing 3 changed files with 167 additions and 16 deletions.
32 changes: 16 additions & 16 deletions superset/commands/dashboard/importers/v1/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

from marshmallow import Schema
from sqlalchemy.orm import Session # noqa: F401
from sqlalchemy.sql import select
from sqlalchemy.sql import delete, insert

from superset import db
from superset.charts.schemas import ImportV1ChartSchema
Expand Down Expand Up @@ -123,32 +123,32 @@ def _import(configs: dict[str, Any], overwrite: bool = False) -> None:
charts.append(chart)
chart_ids[str(chart.uuid)] = chart.id

# store the existing relationship between dashboards and charts
existing_relationships = db.session.execute(
select([dashboard_slices.c.dashboard_id, dashboard_slices.c.slice_id])
).fetchall()

# import dashboards
dashboards: list[Dashboard] = []
dashboard_chart_ids: list[tuple[int, int]] = []
for file_name, config in configs.items():
if file_name.startswith("dashboards/"):
config = update_id_refs(config, chart_ids, dataset_info)
dashboard = import_dashboard(config, overwrite=overwrite)
dashboards.append(dashboard)

# set ref in the dashboard_slices table
dashboard_chart_ids: list[dict[str, int]] = []
for uuid in find_chart_uuids(config["position"]):
if uuid not in chart_ids:
break
chart_id = chart_ids[uuid]
if (dashboard.id, chart_id) not in existing_relationships:
dashboard_chart_ids.append((dashboard.id, chart_id))

# set ref in the dashboard_slices table
values = [
{"dashboard_id": dashboard_id, "slice_id": chart_id}
for (dashboard_id, chart_id) in dashboard_chart_ids
]
db.session.execute(dashboard_slices.insert(), values)
dashboard_chart_id = {
"dashboard_id": dashboard.id,
"slice_id": chart_id,
}
dashboard_chart_ids.append(dashboard_chart_id)

db.execute(
delete(dashboard_slices).where(
dashboard_slices.c.dashboard_id == dashboard.id
)
)
db.session.execute(insert(dashboard_slices).values(dashboard_chart_ids))

# Migrate any filter-box charts to native dashboard filters.
for dashboard in dashboards:
Expand Down
61 changes: 61 additions & 0 deletions tests/integration_tests/dashboards/commands_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,9 @@
from tests.integration_tests.base_tests import SupersetTestCase
from tests.integration_tests.fixtures.importexport import (
chart_config,
chart_config_2,
dashboard_config,
dashboard_config_2,
dashboard_export,
dashboard_metadata_config,
database_config,
Expand Down Expand Up @@ -720,6 +722,65 @@ def test_import_v1_dashboard_validation(self):
}
}

@patch("superset.security.manager.g")
def test_import_v1_dashboard_overwrite_removes_deleted_charts(self, mock_g):
"""Test that existing dashboards are updated without old charts."""
mock_g.user = security_manager.find_user("admin")

num_dashboards = db.session.query(Dashboard).count()
num_charts = db.session.query(Slice).count()

contents = {
"metadata.yaml": yaml.safe_dump(dashboard_metadata_config),
"databases/imported_database.yaml": yaml.safe_dump(database_config),
"datasets/imported_dataset.yaml": yaml.safe_dump(dataset_config),
"charts/imported_chart.yaml": yaml.safe_dump(chart_config),
"dashboards/imported_dashboard.yaml": yaml.safe_dump(dashboard_config),
}
command = v1.ImportDashboardsCommand(contents)
command.run()

dashboard = (
db.session.query(Dashboard).filter_by(uuid=dashboard_config["uuid"]).one()
)

old_chart = dashboard.slices[0]
assert str(old_chart.uuid) == chart_config["uuid"]

overwrite_contents = {
"metadata.yaml": yaml.safe_dump(dashboard_metadata_config),
"databases/imported_database.yaml": yaml.safe_dump(database_config),
"datasets/imported_dataset.yaml": yaml.safe_dump(dataset_config),
"charts/imported_chart.yaml": yaml.safe_dump(chart_config_2),
"dashboards/imported_dashboard.yaml": yaml.safe_dump(dashboard_config_2),
}
overwrite_command = v1.ImportDashboardsCommand(
overwrite_contents, overwrite=True
)
overwrite_command.run()

new_num_dashboards = db.session.query(Dashboard).count()
assert new_num_dashboards == num_dashboards + 1

dashboard = (
db.session.query(Dashboard).filter_by(uuid=dashboard_config["uuid"]).one()
)

assert len(dashboard.slices) == 1

new_chart = dashboard.slices[0]
assert str(new_chart.uuid) == chart_config_2["uuid"]

dataset = old_chart.table
database = dataset.database

db.session.delete(dashboard)
db.session.delete(old_chart)
db.session.delete(new_chart)
db.session.delete(dataset)
db.session.delete(database)
db.session.commit()


class TestCopyDashboardCommand(SupersetTestCase):
@pytest.mark.usefixtures("load_world_bank_dashboard_with_slices")
Expand Down
90 changes: 90 additions & 0 deletions tests/integration_tests/fixtures/importexport.py
Original file line number Diff line number Diff line change
Expand Up @@ -681,6 +681,96 @@
},
"version": "1.0.0",
}

chart_config_2: dict[str, Any] = {
"slice_name": "Deck Path 2",
"viz_type": "deck_path",
"params": {
"color_picker": {"a": 1, "b": 135, "g": 122, "r": 0},
"datasource": "12__table",
"js_columns": ["color"],
"js_data_mutator": r"data => data.map(d => ({\n ...d,\n color: colors.hexToRGB(d.extraProps.color)\n}));",
"js_onclick_href": "",
"js_tooltip": "",
"line_column": "path_json",
"line_type": "json",
"line_width": 100,
"mapbox_style": "mapbox://styles/mapbox/light-v9",
"reverse_long_lat": False,
"row_limit": 5000,
"slice_id": 43,
"time_grain_sqla": None,
"time_range": " : ",
"viewport": {
"altitude": 1.5,
"bearing": 0,
"height": 1094,
"latitude": 37.73671752604488,
"longitude": -122.18885402582598,
"maxLatitude": 85.05113,
"maxPitch": 10,
"maxZoom": 40,
"minLatitude": -85.05113,
"minPitch": 0,
"minZoom": 0,
"pitch": 0,
"width": 669,
"zoom": 9.51847667620428,
},
"viz_type": "deck_path",
},
"query_context": '{"datasource":{"id":12,"type":"table"},"force":false,"queries":[{"time_range":" : ","filters":[],"extras":{"time_grain_sqla":null,"having":"","where":""},"applied_time_extras":{},"columns":[],"metrics":[],"annotation_layers":[],"row_limit":5000,"timeseries_limit":0,"order_desc":true,"url_params":{},"custom_params":{},"custom_form_data":{}}],"result_format":"json","result_type":"full"}',
"cache_timeout": None,
"uuid": "82bb160d-a859-44ce-86cc-9a92de113514",
"version": "1.0.0",
"dataset_uuid": "10808100-158b-42c4-842e-f32b99d88dfb",
}

dashboard_config_2: dict[str, Any] = {
"dashboard_title": "Test dash",
"description": None,
"css": "",
"slug": None,
"uuid": "c4b28c4e-a1fe-4cf8-a5ac-d6f11d6fdd51",
"position": {
"CHART-PsLqYTEu9ZS": {
"children": [],
"id": "CHART-PsLqYTEu9ZS",
"meta": {
"chartId": 283,
"height": 50,
"sliceName": "Games",
"uuid": "82bb160d-a859-44ce-86cc-9a92de113514",
"width": 4,
},
"parents": ["ROOT_ID", "GRID_ID", "ROW-dP_CHaK2q"],
"type": "CHART",
},
"DASHBOARD_VERSION_KEY": "v2",
"GRID_ID": {
"children": ["ROW-dP_CHaK2q"],
"id": "GRID_ID",
"parents": ["ROOT_ID"],
"type": "GRID",
},
"HEADER_ID": {
"id": "HEADER_ID",
"meta": {"text": "Test dash"},
"type": "HEADER",
},
"ROOT_ID": {"children": ["GRID_ID"], "id": "ROOT_ID", "type": "ROOT"},
"ROW-dP_CHaK2q": {
"children": ["CHART-PsLqYTEu9ZS"],
"id": "ROW-dP_CHaK2q",
"meta": {"0": "ROOT_ID", "background": "BACKGROUND_TRANSPARENT"},
"parents": ["ROOT_ID", "GRID_ID"],
"type": "ROW",
},
},
"metadata": {},
"version": "1.0.0",
}

saved_queries_config = {
"schema": "public",
"label": "Test Saved Query",
Expand Down

0 comments on commit ae920f5

Please sign in to comment.