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/07/30 12:47:20 UTC
(superset) 01/02: Add CopyDashboardCommand
This is an automated email from the ASF dual-hosted git repository.
diegopucci pushed a commit to branch geido/fix/copy-dash-missing-transaction
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 7273d5c93b203474277449fec191601cb62d305e
Author: Diego Pucci <di...@gmail.com>
AuthorDate: Tue Jul 30 15:42:45 2024 +0300
Add CopyDashboardCommand
---
superset/commands/dashboard/copy.py | 49 ++++++++++++++++++++
superset/dashboards/api.py | 5 +-
.../integration_tests/dashboards/commands_tests.py | 54 +++++++++++++++++++++-
3 files changed, 106 insertions(+), 2 deletions(-)
diff --git a/superset/commands/dashboard/copy.py b/superset/commands/dashboard/copy.py
new file mode 100644
index 0000000000..24ef561a10
--- /dev/null
+++ b/superset/commands/dashboard/copy.py
@@ -0,0 +1,49 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import logging
+from functools import partial
+from typing import Any, Optional
+
+from flask import g
+from marshmallow import ValidationError
+
+from superset import is_feature_enabled, security_manager
+from superset.commands.base import BaseCommand
+from superset.commands.dashboard.exceptions import (
+ DashboardForbiddenError,
+ DashboardInvalidError,
+)
+from superset.daos.dashboard import DashboardDAO
+from superset.models.dashboard import Dashboard
+from superset.utils.decorators import on_error, transaction
+
+logger = logging.getLogger(__name__)
+
+
+class CopyDashboardCommand(BaseCommand):
+ def __init__(self, original_dash: Dashboard, data: dict[str, Any]) -> None:
+ self._original_dash = original_dash
+ self._properties = data.copy()
+
+ @transaction(on_error=partial(on_error, reraise=DashboardInvalidError))
+ def run(self) -> Dashboard:
+ self.validate()
+ return DashboardDAO.copy_dashboard(self._original_dash, self._properties)
+
+ def validate(self) -> None:
+ if is_feature_enabled("DASHBOARD_RBAC") and not security_manager.is_owner(self._original_dash):
+ raise DashboardForbiddenError()
diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py
index 1e0ecacb65..aeb4d34857 100644
--- a/superset/dashboards/api.py
+++ b/superset/dashboards/api.py
@@ -34,6 +34,7 @@ from werkzeug.wsgi import FileWrapper
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.exceptions import (
@@ -1606,9 +1607,11 @@ class DashboardRestApi(BaseSupersetModelRestApi):
return self.response_400(message=error.messages)
try:
- dash = DashboardDAO.copy_dashboard(original_dash, data)
+ dash = CopyDashboardCommand(original_dash, data).run()
except DashboardForbiddenError:
return self.response_403()
+ except DashboardInvalidError:
+ return self.response_400()
return self.response(
200,
diff --git a/tests/integration_tests/dashboards/commands_tests.py b/tests/integration_tests/dashboards/commands_tests.py
index 334e0425cf..701347b187 100644
--- a/tests/integration_tests/dashboards/commands_tests.py
+++ b/tests/integration_tests/dashboards/commands_tests.py
@@ -22,7 +22,8 @@ import yaml
from werkzeug.utils import secure_filename
from superset import db, security_manager
-from superset.commands.dashboard.exceptions import DashboardNotFoundError
+from superset.commands.dashboard.copy import CopyDashboardCommand
+from superset.commands.dashboard.exceptions import DashboardForbiddenError, DashboardInvalidError, DashboardNotFoundError
from superset.commands.dashboard.export import (
append_charts,
ExportDashboardsCommand,
@@ -36,6 +37,7 @@ from superset.models.core import Database
from superset.models.dashboard import Dashboard
from superset.models.slice import Slice
from superset.utils import json
+from superset.utils.core import override_user
from tests.integration_tests.base_tests import SupersetTestCase
from tests.integration_tests.fixtures.importexport import (
chart_config,
@@ -660,3 +662,53 @@ class TestImportDashboardsCommand(SupersetTestCase):
"table_name": ["Missing data for required field."],
}
}
+
+class TestCopyDashboardCommand(SupersetTestCase):
+ @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices")
+ def test_copy_dashboard_command(self):
+ """Test that an admin user can copy a dashboard"""
+ with self.client.application.test_request_context():
+ example_dashboard = (
+ db.session.query(Dashboard).filter_by(slug="world_health").one()
+ )
+ copy_data = {"dashboard_title": "Copied Dashboard", "json_metadata": "{}"}
+
+ with override_user(security_manager.find_user("admin")):
+ command = CopyDashboardCommand(example_dashboard, copy_data)
+ copied_dashboard = command.run()
+
+ assert copied_dashboard.dashboard_title == "Copied Dashboard"
+ assert copied_dashboard.slug != example_dashboard.slug
+ assert copied_dashboard.slices == example_dashboard.slices
+
+ db.session.delete(copied_dashboard)
+ db.session.commit()
+
+ @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices")
+ def test_copy_dashboard_command_no_access(self):
+ """Test that a non-owner user cannot copy a dashboard if DASHBOARD_RBAC is enabled"""
+ with self.client.application.test_request_context():
+ example_dashboard = (
+ db.session.query(Dashboard).filter_by(slug="world_health").one()
+ )
+ copy_data = {"dashboard_title": "Copied Dashboard", "json_metadata": "{}"}
+
+ with override_user(security_manager.find_user("gamma")):
+ with patch("superset.commands.dashboard.copy.is_feature_enabled", return_value=True):
+ command = CopyDashboardCommand(example_dashboard, copy_data)
+ with self.assertRaises(DashboardForbiddenError):
+ command.run()
+
+ @pytest.mark.usefixtures("load_world_bank_dashboard_with_slices")
+ def test_copy_dashboard_command_invalid_data(self):
+ """Test that invalid data raises a DashboardInvalidError"""
+ with self.client.application.test_request_context():
+ example_dashboard = (
+ db.session.query(Dashboard).filter_by(slug="world_health").one()
+ )
+ invalid_copy_data = {"dashboard_title": ""}
+
+ with override_user(security_manager.find_user("admin")):
+ command = CopyDashboardCommand(example_dashboard, invalid_copy_data)
+ with self.assertRaises(DashboardInvalidError):
+ command.run()