Skip to content
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
9 changes: 9 additions & 0 deletions superset/commands/dashboard/export.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,9 @@ def _file_content(model: Dashboard) -> str:
if orphan_charts:
payload["position"] = append_charts(payload["position"], orphan_charts)

# Add theme UUID for proper cross-system imports
payload["theme_uuid"] = str(model.theme.uuid) if model.theme else None

payload["version"] = EXPORT_VERSION

# Check if the TAGGING_SYSTEM feature is enabled
Expand Down Expand Up @@ -193,6 +196,12 @@ def _export(
dashboard_ids=dashboard_ids, chart_ids=chart_ids
)

# Export related theme
if model.theme:
from superset.commands.theme.export import ExportThemesCommand

yield from ExportThemesCommand([model.theme.id]).run()

payload = model.export_to_dict(
recursive=False,
include_parent_ref=False,
Expand Down
28 changes: 27 additions & 1 deletion superset/commands/dashboard/importers/v1/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

from __future__ import annotations

import logging
from typing import Any

from marshmallow import Schema
Expand All @@ -37,6 +38,7 @@
from superset.commands.dataset.importers.v1.utils import import_dataset
from superset.commands.importers.v1 import ImportModelsCommand
from superset.commands.importers.v1.utils import import_tag
from superset.commands.theme.import_themes import import_theme
from superset.commands.utils import update_chart_config_dataset
from superset.daos.dashboard import DashboardDAO
from superset.dashboards.schemas import ImportV1DashboardSchema
Expand All @@ -45,6 +47,9 @@
from superset.extensions import feature_flag_manager
from superset.migrations.shared.native_filters import migrate_dashboard
from superset.models.dashboard import Dashboard, dashboard_slices
from superset.themes.schemas import ImportV1ThemeSchema

logger = logging.getLogger(__name__)


class ImportDashboardsCommand(ImportModelsCommand):
Expand All @@ -58,6 +63,7 @@ class ImportDashboardsCommand(ImportModelsCommand):
"dashboards/": ImportV1DashboardSchema(),
"datasets/": ImportV1DatasetSchema(),
"databases/": ImportV1DatabaseSchema(),
"themes/": ImportV1ThemeSchema(),
}
import_error = DashboardImportError

Expand All @@ -71,15 +77,19 @@ def _import(
contents: dict[str, Any] | None = None,
) -> None:
contents = {} if contents is None else contents
# discover charts and datasets associated with dashboards
# discover charts, datasets, and themes associated with dashboards
chart_uuids: set[str] = set()
dataset_uuids: set[str] = set()
theme_uuids: set[str] = set()
for file_name, config in configs.items():
if file_name.startswith("dashboards/"):
chart_uuids.update(find_chart_uuids(config["position"]))
dataset_uuids.update(
find_native_filter_datasets(config.get("metadata", {}))
)
# discover theme associated with dashboard
if config.get("theme_uuid"):
theme_uuids.add(config["theme_uuid"])

# discover datasets associated with charts
for file_name, config in configs.items():
Expand All @@ -92,6 +102,14 @@ def _import(
if file_name.startswith("datasets/") and config["uuid"] in dataset_uuids:
database_uuids.add(config["database_uuid"])

# import related themes
theme_ids: dict[str, int] = {}
for file_name, config in configs.items():
if file_name.startswith("themes/") and config["uuid"] in theme_uuids:
theme = import_theme(config, overwrite=False)
if theme:
theme_ids[str(theme.uuid)] = theme.id

# import related databases
database_ids: dict[str, int] = {}
for file_name, config in configs.items():
Expand Down Expand Up @@ -149,6 +167,14 @@ def _import(
for file_name, config in configs.items():
if file_name.startswith("dashboards/"):
config = update_id_refs(config, chart_ids, dataset_info)
# Handle theme UUID to ID mapping
if "theme_uuid" in config and config["theme_uuid"] in theme_ids:
config["theme_id"] = theme_ids[config["theme_uuid"]]
del config["theme_uuid"]
elif "theme_uuid" in config:
# Theme not found, set to None for graceful fallback
config["theme_id"] = None
del config["theme_uuid"]
dashboard = import_dashboard(config, overwrite=overwrite)
dashboards.append(dashboard)
for uuid in find_chart_uuids(config["position"]):
Expand Down
2 changes: 2 additions & 0 deletions superset/commands/dashboard/importers/v1/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,8 @@ def import_dashboard( # noqa: C901
if "metadata" in config and "show_native_filters" in config["metadata"]:
del config["metadata"]["show_native_filters"]

# Note: theme_id handling moved to higher level import logic

for key, new_name in JSON_KEYS.items():
if config.get(key) is not None:
value = config.pop(key)
Expand Down
14 changes: 12 additions & 2 deletions superset/commands/importers/v1/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,20 @@ def validate(self) -> None: # noqa: F811
self._prevent_overwrite_existing_model(exceptions)

if exceptions:
detailed_errors = []
for ex in exceptions:
logger.warning("Import Error: %s", ex)
# Extract detailed error information
if hasattr(ex, "messages") and isinstance(ex.messages, dict):
for file_name, errors in ex.messages.items():
logger.error("Validation failed for %s: %s", file_name, errors)
detailed_errors.append(f"{file_name}: {errors}")
else:
logger.error("Import validation error: %s", ex)
detailed_errors.append(str(ex))

error_summary = "; ".join(detailed_errors)
raise CommandInvalidError(
f"Error importing {self.model_name}",
f"Error importing {self.model_name}: {error_summary}",
exceptions,
)

Expand Down
7 changes: 7 additions & 0 deletions superset/commands/importers/v1/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,13 @@ def load_configs(
schema.load(config)
configs[file_name] = config
except ValidationError as exc:
logger.error(
"Schema validation failed for %s (prefix: %s): %s",
file_name,
prefix,
exc.messages,
)
logger.debug("Config content that failed validation: %s", config)
exc.messages = {file_name: exc.messages}
exceptions.append(exc)

Expand Down
11 changes: 8 additions & 3 deletions superset/commands/theme/import_themes.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,13 @@
# under the License.

import logging
from typing import Any
from typing import Any, TYPE_CHECKING

from marshmallow import Schema

if TYPE_CHECKING:
from superset.models.core import Theme

from superset.commands.importers.v1 import ImportModelsCommand
from superset.commands.theme.exceptions import ThemeImportError
from superset.daos.theme import ThemeDAO
Expand All @@ -29,7 +32,7 @@
logger = logging.getLogger(__name__)


def import_theme(config: dict[str, Any], overwrite: bool = False) -> None:
def import_theme(config: dict[str, Any], overwrite: bool = False) -> "Theme | None":
Copy link
Member

Choose a reason for hiding this comment

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

We are doing this to defer type annotation i guess, i found this SO thread on the topic but not sure where python is now currently.

"""Import a single theme from config dictionary"""
from superset import db, security_manager
from superset.models.core import Theme
Expand All @@ -40,7 +43,7 @@ def import_theme(config: dict[str, Any], overwrite: bool = False) -> None:

if existing:
if not overwrite or not can_write:
return
return existing
config["id"] = existing.id
elif not can_write:
raise ThemeImportError(
Expand All @@ -61,6 +64,8 @@ def import_theme(config: dict[str, Any], overwrite: bool = False) -> None:
theme.changed_by = user
theme.created_by = user

return theme


class ImportThemesCommand(ImportModelsCommand):
"""Import themes"""
Expand Down
2 changes: 2 additions & 0 deletions superset/dashboards/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,8 @@ class ImportV1DashboardSchema(Schema):
certification_details = fields.String(allow_none=True)
published = fields.Boolean(allow_none=True)
tags = fields.List(fields.String(), allow_none=True)
theme_uuid = fields.UUID(allow_none=True)
theme_id = fields.Integer(allow_none=True)


class EmbeddedDashboardConfigSchema(Schema):
Expand Down
3 changes: 1 addition & 2 deletions superset/models/dashboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,13 +174,12 @@ class Dashboard(AuditMixinNullable, ImportExportMixin, Model):
"json_metadata",
"description",
"css",
"theme_id",
"slug",
"certified_by",
"certification_details",
"published",
]
extra_import_fields = ["is_managed_externally", "external_url"]
extra_import_fields = ["is_managed_externally", "external_url", "theme_id"]

def __repr__(self) -> str:
return f"Dashboard<{self.id or self.slug}>"
Expand Down
39 changes: 30 additions & 9 deletions superset/themes/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,16 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
import logging
from typing import Any

from marshmallow import fields, Schema, validates, ValidationError

from superset.themes.utils import is_valid_theme
from superset.utils import json

logger = logging.getLogger(__name__)


class ImportV1ThemeSchema(Schema):
theme_name = fields.String(required=True)
Expand All @@ -42,12 +45,20 @@ def validate_json_data(self, value: dict[str, Any]) -> None:
except (TypeError, json.JSONDecodeError) as ex:
raise ValidationError("Invalid JSON configuration") from ex

# Validate theme structure
# Strict validation for all contexts - ensures consistent data quality
if not is_valid_theme(theme_config):
# Add detailed error info for debugging
logger.error("Theme validation failed. Theme config: %s", theme_config)
logger.error(
"Theme type: %s, Algorithm: %s, Has token: %s",
type(theme_config).__name__,
theme_config.get("algorithm"),
"token" in theme_config,
)
raise ValidationError("Invalid theme configuration structure")


class ThemePostSchema(Schema):
class ThemeBaseSchema(Schema):
theme_name = fields.String(required=True, allow_none=False)
json_data = fields.String(required=True, allow_none=False)

Expand All @@ -56,15 +67,25 @@ def validate_theme_name(self, value: str) -> None:
if not value or not value.strip():
raise ValidationError("Theme name cannot be empty.")

@validates("json_data")
def validate_json_data(self, value: str) -> None:
# Parse JSON string
try:
theme_config = json.loads(value)
except (TypeError, json.JSONDecodeError) as ex:
raise ValidationError("Invalid JSON configuration") from ex

class ThemePutSchema(Schema):
theme_name = fields.String(required=True, allow_none=False)
json_data = fields.String(required=True, allow_none=False)
# Strict validation for theme creation/updates
if not is_valid_theme(theme_config):
raise ValidationError("Invalid theme configuration structure")

@validates("theme_name")
def validate_theme_name(self, value: str) -> None:
if not value or not value.strip():
raise ValidationError("Theme name cannot be empty.")

class ThemePostSchema(ThemeBaseSchema):
pass


class ThemePutSchema(ThemeBaseSchema):
pass


openapi_spec_methods_override = {
Expand Down
57 changes: 18 additions & 39 deletions tests/integration_tests/charts/api_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -1839,24 +1839,16 @@ def test_import_chart_overwrite(self, mock_add_permissions):
response = json.loads(rv.data.decode("utf-8"))

assert rv.status_code == 422
assert response == {
"errors": [
{
"message": "Error importing chart",
"error_type": "GENERIC_COMMAND_ERROR",
"level": "warning",
"extra": {
"charts/chart.yaml": "Chart already exists and `overwrite=true` was not passed", # noqa: E501
"issue_codes": [
{
"code": 1010,
"message": "Issue 1010 - Superset encountered an error while running a command.", # noqa: E501
}
],
},
}
]
}
assert len(response["errors"]) == 1
error = response["errors"][0]
assert error["message"].startswith("Error importing chart")
assert error["error_type"] == "GENERIC_COMMAND_ERROR"
assert error["level"] == "warning"
assert "charts/chart.yaml" in str(error["extra"])
assert "Chart already exists and `overwrite=true` was not passed" in str(
error["extra"]
)
assert error["extra"]["issue_codes"][0]["code"] == 1010

# import with overwrite flag
buf = self.create_import_v1_zip_file("chart")
Expand Down Expand Up @@ -1900,27 +1892,14 @@ def test_import_chart_invalid(self, mock_add_permissions):
response = json.loads(rv.data.decode("utf-8"))

assert rv.status_code == 422
assert response == {
"errors": [
{
"message": "Error importing chart",
"error_type": "GENERIC_COMMAND_ERROR",
"level": "warning",
"extra": {
"metadata.yaml": {"type": ["Must be equal to Slice."]},
"issue_codes": [
{
"code": 1010,
"message": (
"Issue 1010 - Superset encountered an "
"error while running a command."
),
}
],
},
}
]
}
assert len(response["errors"]) == 1
error = response["errors"][0]
assert error["message"].startswith("Error importing chart")
assert error["error_type"] == "GENERIC_COMMAND_ERROR"
assert error["level"] == "warning"
assert "metadata.yaml" in error["extra"]
assert error["extra"]["metadata.yaml"] == {"type": ["Must be equal to Slice."]}
assert error["extra"]["issue_codes"][0]["code"] == 1010

def test_gets_created_by_user_charts_filter(self):
arguments = {
Expand Down
4 changes: 2 additions & 2 deletions tests/integration_tests/charts/commands_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ def test_import_v1_chart_validation(self, mock_add_permissions):
command = ImportChartsCommand(contents)
with pytest.raises(CommandInvalidError) as excinfo:
command.run()
assert str(excinfo.value) == "Error importing chart"
assert str(excinfo.value).startswith("Error importing chart")
assert excinfo.value.normalized_messages() == {
"metadata.yaml": {"type": ["Must be equal to Slice."]}
}
Expand All @@ -326,7 +326,7 @@ def test_import_v1_chart_validation(self, mock_add_permissions):
command = ImportChartsCommand(contents)
with pytest.raises(CommandInvalidError) as excinfo:
command.run()
assert str(excinfo.value) == "Error importing chart"
assert str(excinfo.value).startswith("Error importing chart")
assert excinfo.value.normalized_messages() == {
"databases/imported_database.yaml": {
"database_name": ["Missing data for required field."],
Expand Down
Loading
Loading