fix: complete theme management system import/export (#34850)

Co-authored-by: Claude <noreply@anthropic.com>
(cherry picked from commit 4695be5cc5)

tests/integration_tests/dashboards/api_tests.py

@@ -2456,27 +2456,16 @@ class TestDashboardApi(ApiOwnersTestCaseMixin, InsertChartMixin, SupersetTestCas
         response = json.loads(rv.data.decode("utf-8"))
 
         assert rv.status_code == 422
-        assert response == {
-            "errors": [
-                {
-                    "message": "Error importing dashboard",
-                    "error_type": "GENERIC_COMMAND_ERROR",
-                    "level": "warning",
-                    "extra": {
-                        "dashboards/dashboard.yaml": "Dashboard 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."
-                                ),
-                            }
-                        ],
-                    },
-                }
-            ]
-        }
+        assert len(response["errors"]) == 1
+        error = response["errors"][0]
+        assert error["message"].startswith("Error importing dashboard")
+        assert error["error_type"] == "GENERIC_COMMAND_ERROR"
+        assert error["level"] == "warning"
+        assert "dashboards/dashboard.yaml" in str(error["extra"])
+        assert "Dashboard 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("dashboard")
@@ -2519,27 +2508,16 @@ class TestDashboardApi(ApiOwnersTestCaseMixin, InsertChartMixin, SupersetTestCas
         response = json.loads(rv.data.decode("utf-8"))
 
         assert rv.status_code == 422
-        assert response == {
-            "errors": [
-                {
-                    "message": "Error importing dashboard",
-                    "error_type": "GENERIC_COMMAND_ERROR",
-                    "level": "warning",
-                    "extra": {
-                        "metadata.yaml": {"type": ["Must be equal to Dashboard."]},
-                        "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 dashboard")
+        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 Dashboard."]
         }
+        assert error["extra"]["issue_codes"][0]["code"] == 1010
 
     def test_get_all_related_roles(self):
         """

tests/integration_tests/dashboards/commands_tests.py

@@ -239,7 +239,7 @@ class TestExportDashboardsCommand(SupersetTestCase):
             },
             "metadata": {"mock_key": "mock_value"},
             "version": "1.0.0",
-            "theme_id": None,
+            "theme_uuid": None,
         }
 
     # @pytest.mark.usefixtures("load_covid_dashboard")
@@ -318,8 +318,8 @@ class TestExportDashboardsCommand(SupersetTestCase):
     @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices")
     @patch("superset.security.manager.g")
     @patch("superset.views.base.g")
-    def test_export_dashboard_command_key_order(self, mock_g1, mock_g2):
-        """Test that they keys in the YAML have the same order as export_fields"""
+    def test_export_dashboard_command_required_fields(self, mock_g1, mock_g2):
+        """Test that all required keys are present in the exported YAML"""
         mock_g1.user = security_manager.find_user("admin")
         mock_g2.user = security_manager.find_user("admin")
 
@@ -332,11 +332,11 @@ class TestExportDashboardsCommand(SupersetTestCase):
         metadata = yaml.safe_load(
             contents[f"dashboards/World_Banks_Data_{example_dashboard.id}.yaml"]()
         )
-        assert list(metadata.keys()) == [
+        assert set(metadata.keys()) == {
             "dashboard_title",
             "description",
             "css",
-            "theme_id",
+            "theme_uuid",
             "slug",
             "certified_by",
             "certification_details",
@@ -345,7 +345,7 @@ class TestExportDashboardsCommand(SupersetTestCase):
             "position",
             "metadata",
             "version",
-        ]
+        }
 
     @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices")
     @patch("superset.commands.dashboard.export.suffix")
@@ -727,7 +727,7 @@ class TestImportDashboardsCommand(SupersetTestCase):
         command = v1.ImportDashboardsCommand(contents)
         with pytest.raises(CommandInvalidError) as excinfo:
             command.run()
-        assert str(excinfo.value) == "Error importing dashboard"
+        assert str(excinfo.value).startswith("Error importing dashboard")
         assert excinfo.value.normalized_messages() == {
             "metadata.yaml": {"type": ["Must be equal to Dashboard."]}
         }
@@ -740,7 +740,7 @@ class TestImportDashboardsCommand(SupersetTestCase):
         command = v1.ImportDashboardsCommand(contents)
         with pytest.raises(CommandInvalidError) as excinfo:
             command.run()
-        assert str(excinfo.value) == "Error importing dashboard"
+        assert str(excinfo.value).startswith("Error importing dashboard")
         assert excinfo.value.normalized_messages() == {
             "datasets/imported_dataset.yaml": {
                 "table_name": ["Missing data for required field."],

tests/integration_tests/dashboards/test_theme_integration.py

@@ -17,10 +17,14 @@
 """Integration tests for dashboard-theme functionality"""
 
 import uuid
+from unittest.mock import patch
 
 import pytest
+import yaml
 
-from superset import db
+from superset import db, security_manager
+from superset.commands.dashboard.export import ExportDashboardsCommand
+from superset.commands.dashboard.importers import v1
 from superset.models.core import Theme
 from superset.models.dashboard import Dashboard
 from superset.utils import json
@@ -359,3 +363,139 @@ class TestDashboardThemeIntegration(SupersetTestCase):
             # Clean up system theme
             db.session.delete(system_theme)
             db.session.commit()
+
+    @patch("superset.security.manager.g")
+    @patch("superset.views.base.g")
+    def test_dashboard_export_includes_theme(self, mock_g1, mock_g2):
+        """Test that dashboard export includes theme when dashboard has a theme"""
+        mock_g1.user = security_manager.find_user("admin")
+        mock_g2.user = security_manager.find_user("admin")
+
+        # Assign theme to dashboard
+        self.dashboard.theme_id = self.theme.id
+        db.session.commit()
+
+        # Export dashboard
+        command = ExportDashboardsCommand([self.dashboard.id])
+        contents = dict(command.run())
+
+        # Verify theme file is included in export
+        theme_files = [path for path in contents.keys() if path.startswith("themes/")]
+        assert len(theme_files) == 1
+
+        theme_path = theme_files[0]
+        theme_content = yaml.safe_load(contents[theme_path]())
+
+        # Verify theme content
+        assert theme_content["theme_name"] == f"Test Theme {self.test_id}"
+        assert theme_content["uuid"] == str(self.theme.uuid)
+        assert "json_data" in theme_content
+
+        # Verify dashboard includes theme_uuid
+        dashboard_files = [
+            path for path in contents.keys() if path.startswith("dashboards/")
+        ]
+        assert len(dashboard_files) == 1
+
+        dashboard_content = yaml.safe_load(contents[dashboard_files[0]]())
+        assert dashboard_content["theme_uuid"] == str(self.theme.uuid)
+
+    @patch("superset.utils.core.g")
+    @patch("superset.security.manager.g")
+    def test_dashboard_import_with_theme_uuid(self, sm_g, utils_g):
+        """Test dashboard import with theme UUID resolution"""
+        sm_g.user = utils_g.user = security_manager.find_user("admin")
+        # Create theme config
+        theme_config = {
+            "theme_name": f"Import Test Theme {self.test_id}",
+            "json_data": {"algorithm": "dark", "token": {"colorPrimary": "#ff0000"}},
+            "uuid": str(uuid.uuid4()),
+            "version": "1.0.0",
+        }
+
+        # Create dashboard config with theme reference
+        dashboard_config = {
+            "dashboard_title": f"Import Test Dashboard {self.test_id}",
+            "description": None,
+            "css": "",
+            "slug": f"import-test-{self.test_id}",
+            "uuid": str(uuid.uuid4()),
+            "theme_uuid": theme_config["uuid"],
+            "position": {},
+            "metadata": {},
+            "version": "1.0.0",
+        }
+
+        # Import dashboard with theme
+        contents = {
+            "metadata.yaml": yaml.safe_dump({"version": "1.0.0", "type": "Dashboard"}),
+            f"themes/test_theme_{self.test_id}.yaml": yaml.safe_dump(theme_config),
+            f"dashboards/test_dashboard_{self.test_id}.yaml": yaml.safe_dump(
+                dashboard_config
+            ),
+        }
+
+        command = v1.ImportDashboardsCommand(contents)
+        command.run()
+
+        # Verify theme was imported
+        imported_theme = (
+            db.session.query(Theme).filter_by(uuid=theme_config["uuid"]).first()
+        )
+        assert imported_theme is not None
+        assert imported_theme.theme_name == theme_config["theme_name"]
+
+        # Verify dashboard was imported with theme reference
+        imported_dashboard = (
+            db.session.query(Dashboard).filter_by(uuid=dashboard_config["uuid"]).first()
+        )
+        assert imported_dashboard is not None
+        assert imported_dashboard.theme_id == imported_theme.id
+        assert imported_dashboard.theme.uuid == imported_theme.uuid
+
+        # Clean up
+        db.session.delete(imported_dashboard)
+        db.session.delete(imported_theme)
+        db.session.commit()
+
+    @patch("superset.utils.core.g")
+    @patch("superset.security.manager.g")
+    def test_dashboard_import_missing_theme_graceful_fallback(self, sm_g, utils_g):
+        """Test dashboard import with missing theme falls back gracefully"""
+        sm_g.user = utils_g.user = security_manager.find_user("admin")
+        # Create dashboard config with non-existent theme UUID
+        nonexistent_theme_uuid = str(uuid.uuid4())
+        dashboard_config = {
+            "dashboard_title": f"Missing Theme Test {self.test_id}",
+            "description": None,
+            "css": "",
+            "slug": f"missing-theme-test-{self.test_id}",
+            "uuid": str(uuid.uuid4()),
+            "theme_uuid": nonexistent_theme_uuid,
+            "position": {},
+            "metadata": {},
+            "version": "1.0.0",
+        }
+
+        # Import dashboard without the referenced theme
+        contents = {
+            "metadata.yaml": yaml.safe_dump({"version": "1.0.0", "type": "Dashboard"}),
+            f"dashboards/test_dashboard_{self.test_id}.yaml": yaml.safe_dump(
+                dashboard_config
+            ),
+        }
+
+        command = v1.ImportDashboardsCommand(contents)
+        command.run()
+
+        # Verify dashboard was imported with theme_id = None (graceful fallback)
+        imported_dashboard = (
+            db.session.query(Dashboard).filter_by(uuid=dashboard_config["uuid"]).first()
+        )
+        assert imported_dashboard is not None
+        assert imported_dashboard.theme_id is None
+        assert imported_dashboard.theme is None
+
+        # Clean up
+        db.session.delete(imported_dashboard)
+        db.session.commit()