You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by di...@apache.org on 2024/08/09 14:30:04 UTC
(superset) branch master updated: fix(Embedded): Deleting Embedded Dashboards does not commit the transaction (#29894)
This is an automated email from the ASF dual-hosted git repository.
diegopucci pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new b323bf0fb6 fix(Embedded): Deleting Embedded Dashboards does not commit the transaction (#29894)
b323bf0fb6 is described below
commit b323bf0fb661dcaaa1786ef92352139aa7a5619d
Author: Geido <60...@users.noreply.github.com>
AuthorDate: Fri Aug 9 17:29:57 2024 +0300
fix(Embedded): Deleting Embedded Dashboards does not commit the transaction (#29894)
---
superset/commands/dashboard/delete.py | 16 +++++++++-
superset/commands/dashboard/exceptions.py | 4 +++
superset/dashboards/api.py | 7 ++--
.../integration_tests/dashboards/commands_tests.py | 37 ++++++++++++++++++++++
4 files changed, 61 insertions(+), 3 deletions(-)
diff --git a/superset/commands/dashboard/delete.py b/superset/commands/dashboard/delete.py
index 0135c4303f..fd53519a50 100644
--- a/superset/commands/dashboard/delete.py
+++ b/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
diff --git a/superset/commands/dashboard/exceptions.py b/superset/commands/dashboard/exceptions.py
index ea753034b0..91a4ec6832 100644
--- a/superset/commands/dashboard/exceptions.py
+++ b/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")
diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py
index 49a9ccd8b1..53371925a9 100644
--- a/superset/dashboards/api.py
+++ b/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",))
diff --git a/tests/integration_tests/dashboards/commands_tests.py b/tests/integration_tests/dashboards/commands_tests.py
index d7f0a43ce3..f38774a1b4 100644
--- a/tests/integration_tests/dashboards/commands_tests.py
+++ b/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