fix(Embedded): Deleting Embedded Dashboards does not commit the transaction (#29894)

superset/commands/dashboard/delete.py

@@ -23,12 +23,13 @@ from flask_babel import lazy_gettext as _
 from superset import security_manager
 from superset.commands.base import BaseCommand
 from superset.commands.dashboard.exceptions import (
+    DashboardDeleteEmbeddedFailedError,
     DashboardDeleteFailedError,
     DashboardDeleteFailedReportsExistError,
     DashboardForbiddenError,
     DashboardNotFoundError,
 )
-from superset.daos.dashboard import DashboardDAO
+from superset.daos.dashboard import DashboardDAO, EmbeddedDashboardDAO
 from superset.daos.report import ReportScheduleDAO
 from superset.exceptions import SupersetSecurityException
 from superset.models.dashboard import Dashboard
@@ -37,6 +38,19 @@ from superset.utils.decorators import on_error, transaction
 logger = logging.getLogger(__name__)
 
 
+class DeleteEmbeddedDashboardCommand(BaseCommand):
+    def __init__(self, dashboard: Dashboard):
+        self._dashboard = dashboard
+
+    @transaction(on_error=partial(on_error, reraise=DashboardDeleteEmbeddedFailedError))
+    def run(self) -> None:
+        self.validate()
+        return EmbeddedDashboardDAO.delete(self._dashboard.embedded)
+
+    def validate(self) -> None:
+        pass
+
+
 class DeleteDashboardCommand(BaseCommand):
     def __init__(self, model_ids: list[int]):
         self._model_ids = model_ids

superset/commands/dashboard/exceptions.py

@@ -62,6 +62,10 @@ class DashboardDeleteFailedError(DeleteFailedError):
     message = _("Dashboard could not be deleted.")
 
 
+class DashboardDeleteEmbeddedFailedError(DeleteFailedError):
+    message = _("Embedded dashboard could not be deleted.")
+
+
 class DashboardDeleteFailedReportsExistError(DashboardDeleteFailedError):
     message = _("There are associated alerts or reports")
 

superset/dashboards/api.py

@@ -36,7 +36,10 @@ from superset import db, is_feature_enabled, thumbnail_cache
 from superset.charts.schemas import ChartEntityResponseSchema
 from superset.commands.dashboard.copy import CopyDashboardCommand
 from superset.commands.dashboard.create import CreateDashboardCommand
-from superset.commands.dashboard.delete import DeleteDashboardCommand
+from superset.commands.dashboard.delete import (
+    DeleteDashboardCommand,
+    DeleteEmbeddedDashboardCommand,
+)
 from superset.commands.dashboard.exceptions import (
     DashboardAccessDeniedError,
     DashboardCopyError,
@@ -1549,7 +1552,7 @@ class DashboardRestApi(BaseSupersetModelRestApi):
             500:
               $ref: '#/components/responses/500'
         """
-        EmbeddedDashboardDAO.delete(dashboard.embedded)
+        DeleteEmbeddedDashboardCommand(dashboard).run()
         return self.response(200, message="OK")
 
     @expose("/<id_or_slug>/copy/", methods=("POST",))

tests/integration_tests/dashboards/commands_tests.py

@@ -23,6 +23,7 @@ from werkzeug.utils import secure_filename
 
 from superset import db, security_manager
 from superset.commands.dashboard.copy import CopyDashboardCommand
+from superset.commands.dashboard.delete import DeleteEmbeddedDashboardCommand
 from superset.commands.dashboard.exceptions import (
     DashboardForbiddenError,
     DashboardInvalidError,
@@ -39,6 +40,7 @@ from superset.commands.importers.exceptions import IncorrectVersionError
 from superset.connectors.sqla.models import SqlaTable
 from superset.models.core import Database
 from superset.models.dashboard import Dashboard
+from superset.models.embedded_dashboard import EmbeddedDashboard
 from superset.models.slice import Slice
 from superset.utils import json
 from superset.utils.core import override_user
@@ -722,3 +724,38 @@ class TestCopyDashboardCommand(SupersetTestCase):
             command = CopyDashboardCommand(example_dashboard, invalid_copy_data)
             with self.assertRaises(DashboardInvalidError):
                 command.run()
+
+
+class TestDeleteEmbeddedDashboardCommand(SupersetTestCase):
+    @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices")
+    def test_delete_embedded_dashboard_command(self):
+        """Test that an admin user can add and then delete an embedded dashboard"""
+        with self.client.application.test_request_context():
+            example_dashboard = (
+                db.session.query(Dashboard).filter_by(slug="world_health").one()
+            )
+
+            # Step 1: Add an embedded dashboard
+            new_embedded_dashboard = EmbeddedDashboard(
+                dashboard_id=example_dashboard.id
+            )
+            db.session.add(new_embedded_dashboard)
+            db.session.commit()
+
+            # Step 2: Assert that the embedded dashboard was added
+            embedded_dashboards = example_dashboard.embedded
+            assert len(embedded_dashboards) > 0
+            assert new_embedded_dashboard in embedded_dashboards
+
+            # Step 3: Delete the embedded dashboard
+            with override_user(security_manager.find_user("admin")):
+                command = DeleteEmbeddedDashboardCommand(example_dashboard)
+                command.run()
+
+            # Step 4: Assert that the embedded dashboard was deleted
+            deleted_embedded_dashboard = (
+                db.session.query(EmbeddedDashboard)
+                .filter_by(uuid=new_embedded_dashboard.uuid)
+                .one_or_none()
+            )
+            assert deleted_embedded_dashboard is None