Skip to content

Commit

Permalink
fix: append orphan charts (#12320)
Browse files Browse the repository at this point in the history 
* fix: append orphan charts

* Add unit tests
  • Loading branch information
@betodealmeida
betodealmeida committed Jan 7, 2021
1 parent 9997abe commit 24fccdb
Show file tree
Hide file tree
Showing 2 changed files with 171 additions and 15 deletions.
44 changes: 31 additions & 13 deletions superset/dashboards/commands/export.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,14 @@
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
Expand All @@ -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] = {
Expand All @@ -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

Expand Down Expand Up @@ -117,9 +127,17 @@ def _export(model: Dashboard) -> Iterator[Tuple[str, str]]:
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

Expand Down
142 changes: 140 additions & 2 deletions tests/dashboards/commands_tests.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -28,7 +29,11 @@
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
Expand Down Expand Up @@ -201,6 +206,139 @@ def test_export_dashboard_command_key_order(self, mock_g1, mock_g2):
"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):
Expand Down

0 comments on commit 24fccdb

Please sign in to comment.