fix: append orphan charts (#12320)
* fix: append orphan charts * Add unit tests
This commit is contained in:
parent
9997abeab6
commit
24fccdb2bc
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
|
|
|
|||
Loading…
Reference in New Issue