diff --git a/superset/dashboards/commands/export.py b/superset/dashboards/commands/export.py index 33fcc7d04..302119791 100644 --- a/superset/dashboards/commands/export.py +++ b/superset/dashboards/commands/export.py @@ -20,13 +20,14 @@ import json import logging import random import string -from typing import Any, Dict, Iterator, List, Optional, Tuple +from typing import Any, Dict, Iterator, Optional, Set, Tuple import yaml from werkzeug.utils import secure_filename from superset.charts.commands.export import ExportChartsCommand from superset.dashboards.commands.exceptions import DashboardNotFoundError +from superset.dashboards.commands.importers.v1.utils import find_chart_uuids from superset.dashboards.dao import DashboardDAO from superset.commands.export import ExportModelsCommand from superset.models.dashboard import Dashboard @@ -49,27 +50,35 @@ def suffix(length: int = 8) -> str: ) -def default_position(title: str, charts: List[Slice]) -> Dict[str, Any]: - chart_hashes = [f"CHART-{suffix()}" for _ in charts] - row_hash = f"ROW-N-{suffix()}" - position = { +def get_default_position(title: str) -> Dict[str, Any]: + return { "DASHBOARD_VERSION_KEY": "v2", "ROOT_ID": {"children": ["GRID_ID"], "id": "ROOT_ID", "type": "ROOT"}, "GRID_ID": { - "children": [row_hash], + "children": [], "id": "GRID_ID", "parents": ["ROOT_ID"], "type": "GRID", }, "HEADER_ID": {"id": "HEADER_ID", "meta": {"text": title}, "type": "HEADER"}, - row_hash: { + } + + +def append_charts(position: Dict[str, Any], charts: Set[Slice]) -> Dict[str, Any]: + chart_hashes = [f"CHART-{suffix()}" for _ in charts] + + # if we have ROOT_ID/GRID_ID, append orphan charts to a new row inside the grid + row_hash = None + if "ROOT_ID" in position and "GRID_ID" in position["ROOT_ID"]["children"]: + row_hash = f"ROW-N-{suffix()}" + position["GRID_ID"]["children"].append(row_hash) + position[row_hash] = { "children": chart_hashes, "id": row_hash, "meta": {"0": "ROOT_ID", "background": "BACKGROUND_TRANSPARENT"}, - "parents": ["ROOT_ID", "GRID_ID"], "type": "ROW", - }, - } + "parents": ["ROOT_ID", "GRID_ID"], + } for chart_hash, chart in zip(chart_hashes, charts): position[chart_hash] = { @@ -82,9 +91,10 @@ def default_position(title: str, charts: List[Slice]) -> Dict[str, Any]: "uuid": str(chart.uuid), "width": DEFAULT_CHART_WIDTH, }, - "parents": ["ROOT_ID", "GRID_ID", row_hash], "type": "CHART", } + if row_hash: + position[chart_hash]["parents"] = ["ROOT_ID", "GRID_ID", row_hash] return position @@ -117,9 +127,17 @@ class ExportDashboardsCommand(ExportModelsCommand): payload[new_name] = {} # the mapping between dashboard -> charts is inferred from the position - # attributes, so if it's not present we need to add a default config + # attribute, so if it's not present we need to add a default config if not payload.get("position"): - payload["position"] = default_position(model.dashboard_title, model.slices) + payload["position"] = get_default_position(model.dashboard_title) + + # if any charts or not referenced in position, we need to add them + # in a new row + referenced_charts = find_chart_uuids(payload["position"]) + orphan_charts = { + chart for chart in model.slices if str(chart.uuid) not in referenced_charts + } + payload["position"] = append_charts(payload["position"], orphan_charts) payload["version"] = EXPORT_VERSION diff --git a/tests/dashboards/commands_tests.py b/tests/dashboards/commands_tests.py index 8be5bcc98..f3d96f37a 100644 --- a/tests/dashboards/commands_tests.py +++ b/tests/dashboards/commands_tests.py @@ -16,8 +16,9 @@ # under the License. # pylint: disable=no-self-use, invalid-name +import itertools import json -from unittest.mock import patch +from unittest.mock import MagicMock, patch import pytest import yaml @@ -28,7 +29,11 @@ from superset.commands.exceptions import CommandInvalidError from superset.commands.importers.exceptions import IncorrectVersionError from superset.connectors.sqla.models import SqlaTable from superset.dashboards.commands.exceptions import DashboardNotFoundError -from superset.dashboards.commands.export import ExportDashboardsCommand +from superset.dashboards.commands.export import ( + append_charts, + ExportDashboardsCommand, + get_default_position, +) from superset.dashboards.commands.importers import v0, v1 from superset.models.core import Database from superset.models.dashboard import Dashboard @@ -201,6 +206,139 @@ class TestExportDashboardsCommand(SupersetTestCase): "version", ] + @patch("superset.dashboards.commands.export.suffix") + def test_append_charts(self, mock_suffix): + """Test that oprhaned charts are added to the dashbaord position""" + # return deterministic IDs + mock_suffix.side_effect = (str(i) for i in itertools.count(1)) + + position = get_default_position("example") + chart_1 = db.session.query(Slice).filter_by(id=1).one() + new_position = append_charts(position, {chart_1}) + assert new_position == { + "DASHBOARD_VERSION_KEY": "v2", + "ROOT_ID": {"children": ["GRID_ID"], "id": "ROOT_ID", "type": "ROOT"}, + "GRID_ID": { + "children": ["ROW-N-2"], + "id": "GRID_ID", + "parents": ["ROOT_ID"], + "type": "GRID", + }, + "HEADER_ID": { + "id": "HEADER_ID", + "meta": {"text": "example"}, + "type": "HEADER", + }, + "ROW-N-2": { + "children": ["CHART-1"], + "id": "ROW-N-2", + "meta": {"0": "ROOT_ID", "background": "BACKGROUND_TRANSPARENT"}, + "type": "ROW", + "parents": ["ROOT_ID", "GRID_ID"], + }, + "CHART-1": { + "children": [], + "id": "CHART-1", + "meta": { + "chartId": 1, + "height": 50, + "sliceName": "Region Filter", + "uuid": str(chart_1.uuid), + "width": 4, + }, + "type": "CHART", + "parents": ["ROOT_ID", "GRID_ID", "ROW-N-2"], + }, + } + + chart_2 = db.session.query(Slice).filter_by(id=2).one() + new_position = append_charts(new_position, {chart_2}) + assert new_position == { + "DASHBOARD_VERSION_KEY": "v2", + "ROOT_ID": {"children": ["GRID_ID"], "id": "ROOT_ID", "type": "ROOT"}, + "GRID_ID": { + "children": ["ROW-N-2", "ROW-N-4"], + "id": "GRID_ID", + "parents": ["ROOT_ID"], + "type": "GRID", + }, + "HEADER_ID": { + "id": "HEADER_ID", + "meta": {"text": "example"}, + "type": "HEADER", + }, + "ROW-N-2": { + "children": ["CHART-1"], + "id": "ROW-N-2", + "meta": {"0": "ROOT_ID", "background": "BACKGROUND_TRANSPARENT"}, + "type": "ROW", + "parents": ["ROOT_ID", "GRID_ID"], + }, + "ROW-N-4": { + "children": ["CHART-3"], + "id": "ROW-N-4", + "meta": {"0": "ROOT_ID", "background": "BACKGROUND_TRANSPARENT"}, + "type": "ROW", + "parents": ["ROOT_ID", "GRID_ID"], + }, + "CHART-1": { + "children": [], + "id": "CHART-1", + "meta": { + "chartId": 1, + "height": 50, + "sliceName": "Region Filter", + "uuid": str(chart_1.uuid), + "width": 4, + }, + "type": "CHART", + "parents": ["ROOT_ID", "GRID_ID", "ROW-N-2"], + }, + "CHART-3": { + "children": [], + "id": "CHART-3", + "meta": { + "chartId": 2, + "height": 50, + "sliceName": "World's Population", + "uuid": str(chart_2.uuid), + "width": 4, + }, + "type": "CHART", + "parents": ["ROOT_ID", "GRID_ID", "ROW-N-4"], + }, + } + + position = {"DASHBOARD_VERSION_KEY": "v2"} + new_position = append_charts(position, [chart_1, chart_2]) + assert new_position == { + "CHART-5": { + "children": [], + "id": "CHART-5", + "meta": { + "chartId": 1, + "height": 50, + "sliceName": "Region Filter", + "uuid": str(chart_1.uuid), + "width": 4, + }, + "type": "CHART", + }, + "CHART-6": { + "children": [], + "id": "CHART-6", + "meta": { + "chartId": 2, + "height": 50, + "sliceName": "World's Population", + "uuid": str(chart_2.uuid), + "width": 4, + }, + "type": "CHART", + }, + "DASHBOARD_VERSION_KEY": "v2", + } + class TestImportDashboardsCommand(SupersetTestCase): def test_import_v0_dashboard_cli_export(self):