rusackas commented on code in PR #37120:
URL: https://github.com/apache/superset/pull/37120#discussion_r3653912877
##########
superset/commands/dashboard/export.py:
##########
@@ -441,4 +450,7 @@ def _export(
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
+ )
Review Comment:
Same as the comment above, pre-existing and out of scope for this PR.
##########
tests/integration_tests/dashboards/commands_tests.py:
##########
@@ -533,6 +533,100 @@ def test_export_dashboard_command_unicode_chars(self,
mock_g1, mock_g2):
{"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)
+
+ # 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"}),
+ )
Review Comment:
Good catch, fixed! Added a `db.session.flush()` before the chart's created
so `datasource_id` isn't `None` at construction time.
##########
tests/integration_tests/dashboards/commands_tests.py:
##########
@@ -533,6 +533,100 @@ def test_export_dashboard_command_unicode_chars(self,
mock_g1, mock_g2):
{"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)
+
+ # 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()
+
+ # 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}"
+ )
+
+ # Clean up
+ 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()
Review Comment:
Same as the other cleanup comment on this test, it's wrapped in try/finally
now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
