From b7301ac88a6dff6ad41eacd13b2f4588304fc880 Mon Sep 17 00:00:00 2001 From: Milad Rashidikhah Date: Fri, 28 Aug 2026 06:57:54 +0330 Subject: [PATCH] fix: Dashboard export with charts from multiple databases (#37120) Co-authored-by: rusackas Co-authored-by: Claude Opus 4.8 --- superset/commands/chart/export.py | 15 ++- superset/commands/dashboard/export.py | 26 ++++- superset/commands/database/export.py | 6 +- superset/commands/dataset/export.py | 61 ++++++---- superset/commands/export/models.py | 40 +++++-- superset/commands/query/export.py | 42 ++++--- superset/commands/tag/export.py | 4 +- superset/commands/theme/export.py | 6 +- .../dashboards/commands_tests.py | 104 ++++++++++++++++++ 9 files changed, 239 insertions(+), 65 deletions(-) diff --git a/superset/commands/chart/export.py b/superset/commands/chart/export.py index 47ea7e67376..89a8a015b9f 100644 --- a/superset/commands/chart/export.py +++ b/superset/commands/chart/export.py @@ -91,8 +91,10 @@ class ExportChartsCommand(ExportModelsCommand): def enable_tag_export(cls) -> None: cls._include_tags = True - def run(self) -> Iterator[tuple[str, Callable[[], str]]]: - yield from super().run() + def run( + self, seen: set[str] | None = None + ) -> Iterator[tuple[str, Callable[[], str]]]: + yield from super().run(seen=seen) # Tags are exported once for all requested charts (rather than per # chart in `_export`) so a multi-chart export doesn't lose tags to @@ -108,12 +110,17 @@ class ExportChartsCommand(ExportModelsCommand): @staticmethod def _export( - model: Slice, export_related: bool = True + model: Slice, export_related: bool = True, seen: set[str] | None = None ) -> Iterator[tuple[str, Callable[[], str]]]: + # Initialize seen set if not provided + if seen is None: + seen = set() + yield ( ExportChartsCommand._file_name(model), lambda: ExportChartsCommand._file_content(model), ) if model.table and export_related: - yield from ExportDatasetsCommand([model.table.id]).run() + # Pass the shared seen set to the dataset export command + yield from ExportDatasetsCommand([model.table.id]).run(seen=seen) diff --git a/superset/commands/dashboard/export.py b/superset/commands/dashboard/export.py index 9ea8e1ee900..2618889e20b 100644 --- a/superset/commands/dashboard/export.py +++ b/superset/commands/dashboard/export.py @@ -383,8 +383,12 @@ class ExportDashboardsCommand(ExportModelsCommand): @staticmethod # ruff: noqa: C901 def _export( - model: Dashboard, export_related: bool = True + model: Dashboard, export_related: bool = True, seen: set[str] | None = None ) -> Iterator[tuple[str, Callable[[], str]]]: + # Initialize seen set if not provided + if seen is None: + seen = set() + yield ( ExportDashboardsCommand._file_name(model), lambda: ExportDashboardsCommand._file_content(model), @@ -395,8 +399,11 @@ class ExportDashboardsCommand(ExportModelsCommand): dashboard_ids = model.id command = ExportChartsCommand(chart_ids) command.disable_tag_export() - yield from command.run() - command.enable_tag_export() + try: + # Pass the shared seen set to the chart export command + yield from command.run(seen=seen) + finally: + command.enable_tag_export() if feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM"): yield from ExportTagsCommand( dashboard_ids=dashboard_ids, chart_ids=chart_ids @@ -406,7 +413,8 @@ class ExportDashboardsCommand(ExportModelsCommand): if model.theme: from superset.commands.theme.export import ExportThemesCommand - yield from ExportThemesCommand([model.theme.id]).run() + # Pass the shared seen set to the theme export command + yield from ExportThemesCommand([model.theme.id]).run(seen=seen) payload = model.export_to_dict( recursive=False, @@ -435,7 +443,10 @@ class ExportDashboardsCommand(ExportModelsCommand): if dataset_id is not None: dataset = DatasetDAO.find_by_id(dataset_id) if dataset: - yield from ExportDatasetsCommand([dataset_id]).run() + # Pass the shared seen set to the dataset export command + yield from ExportDatasetsCommand([dataset_id]).run( + seen=seen + ) # Export datasets referenced by display controls for customization in ( @@ -446,4 +457,7 @@ class ExportDashboardsCommand(ExportModelsCommand): if dataset_id is not None: dataset = DatasetDAO.find_by_id(dataset_id) if dataset: - yield from ExportDatasetsCommand([dataset_id]).run() + # Pass the shared seen set to the dataset export command + yield from ExportDatasetsCommand([dataset_id]).run( + seen=seen + ) diff --git a/superset/commands/database/export.py b/superset/commands/database/export.py index 3ca1e74b1df..638746ae7bd 100644 --- a/superset/commands/database/export.py +++ b/superset/commands/database/export.py @@ -113,8 +113,12 @@ class ExportDatabasesCommand(ExportModelsCommand): @staticmethod def _export( - model: Database, export_related: bool = True + model: Database, export_related: bool = True, seen: set[str] | None = None ) -> Iterator[tuple[str, Callable[[], str]]]: + # Initialize seen set if not provided + if seen is None: + seen = set() + yield ( ExportDatabasesCommand._file_name(model), lambda: ExportDatabasesCommand._file_content(model), diff --git a/superset/commands/dataset/export.py b/superset/commands/dataset/export.py index de373a886d1..26448f194d5 100644 --- a/superset/commands/dataset/export.py +++ b/superset/commands/dataset/export.py @@ -89,8 +89,12 @@ class ExportDatasetsCommand(ExportModelsCommand): @staticmethod def _export( - model: SqlaTable, export_related: bool = True + model: SqlaTable, export_related: bool = True, seen: set[str] | None = None ) -> Iterator[tuple[str, Callable[[], str]]]: + # Initialize seen set if not provided + if seen is None: + seen = set() + yield ( ExportDatasetsCommand._file_name(model), lambda: ExportDatasetsCommand._file_content(model), @@ -103,32 +107,41 @@ class ExportDatasetsCommand(ExportModelsCommand): ) file_path = f"databases/{db_file_name}.yaml" - payload = model.database.export_to_dict( - recursive=False, - include_parent_ref=False, - include_defaults=True, - export_uuids=True, - ) - # TODO (betodealmeida): move this logic to export_to_dict once this - # becomes the default export endpoint - if payload.get("extra"): - try: - payload["extra"] = json.loads(payload["extra"]) - except json.JSONDecodeError: - logger.info("Unable to decode `extra` field: %s", payload["extra"]) - - if ssh_tunnel := model.database.ssh_tunnel: - ssh_tunnel_payload = ssh_tunnel.export_to_dict( + # Only yield the database file if not already seen. This is + # critical to fix the issue where databases were being + # duplicated and potentially overwritten when charts from + # different databases were exported. + if file_path not in seen: + payload = model.database.export_to_dict( recursive=False, include_parent_ref=False, include_defaults=True, - export_uuids=False, + export_uuids=True, ) - payload["ssh_tunnel"] = mask_password_info(ssh_tunnel_payload) + # TODO (betodealmeida): move this logic to export_to_dict once this + # becomes the default export endpoint + if payload.get("extra"): + try: + payload["extra"] = json.loads(payload["extra"]) + except json.JSONDecodeError: + logger.info( + "Unable to decode `extra` field: %s", payload["extra"] + ) - payload["version"] = EXPORT_VERSION + if ssh_tunnel := model.database.ssh_tunnel: + ssh_tunnel_payload = ssh_tunnel.export_to_dict( + recursive=False, + include_parent_ref=False, + include_defaults=True, + export_uuids=False, + ) + payload["ssh_tunnel"] = mask_password_info(ssh_tunnel_payload) - yield ( - file_path, - lambda: yaml.safe_dump(payload, sort_keys=False, allow_unicode=True), - ) + payload["version"] = EXPORT_VERSION + + yield ( + file_path, + lambda: yaml.safe_dump( + payload, sort_keys=False, allow_unicode=True + ), + ) diff --git a/superset/commands/export/models.py b/superset/commands/export/models.py index f46368d5321..ead283fddb0 100644 --- a/superset/commands/export/models.py +++ b/superset/commands/export/models.py @@ -47,27 +47,45 @@ class ExportModelsCommand(BaseCommand): @staticmethod def _file_content(model: Model) -> str: - raise NotImplementedError("Subclasses MUST implement _export") + raise NotImplementedError("Subclasses MUST implement _file_content") @staticmethod def _export( - model: Model, export_related: bool = True + model: Model, export_related: bool = True, seen: set[str] | None = None ) -> Iterator[tuple[str, Callable[[], str]]]: raise NotImplementedError("Subclasses MUST implement _export") - def run(self) -> Iterator[tuple[str, Callable[[], str]]]: + def run( + self, seen: set[str] | None = None + ) -> Iterator[tuple[str, Callable[[], str]]]: self.validate() - metadata = { - "version": EXPORT_VERSION, - "type": self.dao.model_cls.__name__, # type: ignore - "timestamp": datetime.now(tz=timezone.utc).isoformat(), - } - yield METADATA_FILE_NAME, lambda: yaml.safe_dump(metadata, sort_keys=False) + # Use provided seen set or create new one + if seen is None: + seen = set() + should_add_metadata = True + else: + # If seen set is provided, we're being called from another command + should_add_metadata = False + + # Only add metadata if this is the root command + if should_add_metadata: + metadata = { + "version": EXPORT_VERSION, + "type": self.dao.model_cls.__name__, # type: ignore + "timestamp": datetime.now(tz=timezone.utc).isoformat(), + } + if METADATA_FILE_NAME not in seen: + yield ( + METADATA_FILE_NAME, + lambda: yaml.safe_dump(metadata, sort_keys=False), + ) + seen.add(METADATA_FILE_NAME) - seen = {METADATA_FILE_NAME} for model in self._models: - for file_name, file_content in self._export(model, self.export_related): + for file_name, file_content in self._export( + model, self.export_related, seen + ): if file_name not in seen: yield file_name, file_content seen.add(file_name) diff --git a/superset/commands/query/export.py b/superset/commands/query/export.py index e63cde5f28a..fc12057a99e 100644 --- a/superset/commands/query/export.py +++ b/superset/commands/query/export.py @@ -67,8 +67,12 @@ class ExportSavedQueriesCommand(ExportModelsCommand): @staticmethod def _export( - model: SavedQuery, export_related: bool = True + model: SavedQuery, export_related: bool = True, seen: set[str] | None = None ) -> Iterator[tuple[str, Callable[[], str]]]: + # Initialize seen set if not provided + if seen is None: + seen = set() + yield ( ExportSavedQueriesCommand._file_name(model), lambda: ExportSavedQueriesCommand._file_content(model), @@ -79,21 +83,25 @@ class ExportSavedQueriesCommand(ExportModelsCommand): database_slug = secure_filename(model.database.database_name) file_name = f"databases/{database_slug}.yaml" - payload = model.database.export_to_dict( - recursive=False, - include_parent_ref=False, - include_defaults=True, - export_uuids=True, - ) - # TODO (betodealmeida): move this logic to export_to_dict once this - # becomes the default export endpoint - if "extra" in payload: - try: - payload["extra"] = json.loads(payload["extra"]) - except json.JSONDecodeError: - logger.info("Unable to decode `extra` field: %s", payload["extra"]) + # Only yield if not already seen (similar to dataset export) + if file_name not in seen: + payload = model.database.export_to_dict( + recursive=False, + include_parent_ref=False, + include_defaults=True, + export_uuids=True, + ) + # TODO (betodealmeida): move this logic to export_to_dict once this + # becomes the default export endpoint + if "extra" in payload: + try: + payload["extra"] = json.loads(payload["extra"]) + except (TypeError, json.JSONDecodeError): + logger.info( + "Unable to decode `extra` field: %s", payload["extra"] + ) - payload["version"] = EXPORT_VERSION + payload["version"] = EXPORT_VERSION - file_content = yaml.safe_dump(payload, sort_keys=False) - yield file_name, lambda: file_content + file_content = yaml.safe_dump(payload, sort_keys=False) + yield file_name, lambda: file_content diff --git a/superset/commands/tag/export.py b/superset/commands/tag/export.py index 99c17c34816..1d2ee6c43c4 100644 --- a/superset/commands/tag/export.py +++ b/superset/commands/tag/export.py @@ -46,7 +46,9 @@ class ExportTagsCommand(ExportModelsCommand): self.dashboard_ids = dashboard_ids self.chart_ids = chart_ids - def run(self) -> Iterator[tuple[str, Callable[[], str]]]: + def run( + self, seen: set[str] | None = None + ) -> Iterator[tuple[str, Callable[[], str]]]: if not feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM"): return diff --git a/superset/commands/theme/export.py b/superset/commands/theme/export.py index cfb705e7e7e..d46811fe846 100644 --- a/superset/commands/theme/export.py +++ b/superset/commands/theme/export.py @@ -67,8 +67,12 @@ class ExportThemesCommand(ExportModelsCommand): @staticmethod def _export( - model: Theme, export_related: bool = True + model: Theme, export_related: bool = True, seen: set[str] | None = None ) -> Iterator[tuple[str, Callable[[], str]]]: + # Initialize seen set if not provided (for consistency) + if seen is None: + seen = set() + yield ( ExportThemesCommand._file_name(model), lambda: ExportThemesCommand._file_content(model), diff --git a/tests/integration_tests/dashboards/commands_tests.py b/tests/integration_tests/dashboards/commands_tests.py index 13324fa1acc..2ed70e1e4c9 100644 --- a/tests/integration_tests/dashboards/commands_tests.py +++ b/tests/integration_tests/dashboards/commands_tests.py @@ -533,6 +533,110 @@ class TestExportDashboardsCommand(SupersetTestCase): {"dashboard_title": "World Bank's Data"}, ) + @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices") + @patch("superset.security.manager.g") + @patch("superset.views.base.g") + def test_export_dashboard_cross_database_charts(self, mock_g1, mock_g2): + """ + Test that dashboards with charts from multiple databases export correctly. + This reproduces issue #37113 where charts from different databases were missing. + """ + mock_g1.user = security_manager.find_user("admin") + mock_g2.user = security_manager.find_user("admin") + + # Create a second database for testing + second_db = Database(database_name="test_db_2", sqlalchemy_uri="sqlite://") + db.session.add(second_db) + + # Create a dataset in the second database + second_dataset = SqlaTable( + table_name="second_dataset", + database=second_db, + database_id=second_db.id, + columns=[], + ) + db.session.add(second_dataset) + # Flush so `second_dataset.id` is populated before it's read below; + # otherwise the chart would be constructed with `datasource_id=None` + # and never actually link back to this dataset. + db.session.flush() + + # Create a chart using the second database's dataset + chart_from_second_db = Slice( + slice_name="Chart from Second Database", + datasource_type="table", + datasource_id=second_dataset.id, + datasource_name=second_dataset.table_name, + viz_type="bar", + params=json.dumps({"viz_type": "bar"}), + ) + db.session.add(chart_from_second_db) + + # Get the example dashboard and add the new chart + example_dashboard = ( + db.session.query(Dashboard).filter_by(slug="world_health").one() + ) + + # Store original charts count + original_charts_count = len(example_dashboard.slices) + + # Add the new chart from different database to the dashboard + example_dashboard.slices.append(chart_from_second_db) + db.session.commit() + + try: + # Export the dashboard + command = ExportDashboardsCommand([example_dashboard.id]) + contents = dict(command.run()) + + # Verify all databases are exported + db_files = [key for key in contents.keys() if key.startswith("databases/")] + assert len(db_files) >= 2, ( + f"Expected at least 2 database files, got {db_files}" + ) + + # Verify the second database is included + assert "databases/test_db_2.yaml" in contents.keys(), ( + f"Second database not found in export. Keys: {list(contents.keys())}" + ) + + # Verify all charts are exported (original + new one) + chart_files = [key for key in contents.keys() if key.startswith("charts/")] + assert len(chart_files) == original_charts_count + 1, ( + f"Expected {original_charts_count + 1} charts, got {len(chart_files)}" + ) + + # Verify the new chart from second database is included + chart_from_second_db_file = None + for key in chart_files: + if f"Chart_from_Second_Database_{chart_from_second_db.id}" in key: + chart_from_second_db_file = key + break + + assert chart_from_second_db_file is not None, ( + f"Chart from second database not found in export. " + f"Chart files: {chart_files}" + ) + + # Verify the dataset from second database is included + dataset_files = [ + key for key in contents.keys() if key.startswith("datasets/") + ] + second_dataset_file = ( + f"datasets/test_db_2/second_dataset_{second_dataset.id}.yaml" + ) + assert second_dataset_file in contents.keys(), ( + f"Second dataset not found. Dataset files: {dataset_files}" + ) + finally: + # Clean up, even if an assertion above failed, so a failing run + # doesn't leave extra Database/Slice/SqlaTable rows for later tests. + example_dashboard.slices.remove(chart_from_second_db) + db.session.delete(chart_from_second_db) + db.session.delete(second_dataset) + db.session.delete(second_db) + db.session.commit() + class TestImportDashboardsCommand(SupersetTestCase): def test_import_v0_dashboard_cli_export(self):