You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2024/01/25 14:57:38 UTC
(superset) 01/01: fix(import): only import FORMULA annotations (#26652)
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 3.0
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 1391932d1daf006d7f2c6223d65d4cb1bbeed7dc
Author: Maxime Beauchemin <ma...@gmail.com>
AuthorDate: Fri Jan 19 15:13:18 2024 -0800
fix(import): only import FORMULA annotations (#26652)
---
superset/charts/commands/importers/v1/utils.py | 18 +++++++++++
superset/cli/importexport.py | 3 +-
tests/integration_tests/charts/commands_tests.py | 1 +
tests/integration_tests/cli_tests.py | 12 +++++---
tests/integration_tests/fixtures/importexport.py | 35 ++++++++++++++++++++++
.../charts/commands/importers/v1/import_test.py | 19 ++++++++++++
6 files changed, 83 insertions(+), 5 deletions(-)
diff --git a/superset/charts/commands/importers/v1/utils.py b/superset/charts/commands/importers/v1/utils.py
index 3ef0a2ed78..91ec58406e 100644
--- a/superset/charts/commands/importers/v1/utils.py
+++ b/superset/charts/commands/importers/v1/utils.py
@@ -28,6 +28,22 @@ from superset.commands.exceptions import ImportFailedError
from superset.migrations.shared.migrate_viz import processors
from superset.migrations.shared.migrate_viz.base import MigrateViz
from superset.models.slice import Slice
+from superset.utils.core import AnnotationType
+
+
+def filter_chart_annotations(chart_config: dict[str, Any]) -> None:
+ """
+ Mutating the chart's config params to keep only the annotations of
+ type FORMULA.
+ TODO:
+ handle annotation dependencies on either other charts or
+ annotation layers objects.
+ """
+ params = chart_config.get("params", {})
+ als = params.get("annotation_layers", [])
+ params["annotation_layers"] = [
+ al for al in als if al.get("annotationType") == AnnotationType.FORMULA
+ ]
def import_chart(
@@ -47,6 +63,8 @@ def import_chart(
"Chart doesn't exist and user doesn't have permission to create charts"
)
+ filter_chart_annotations(config)
+
# TODO (betodealmeida): move this logic to import_from_dict
config["params"] = json.dumps(config["params"])
diff --git a/superset/cli/importexport.py b/superset/cli/importexport.py
index 27bb0f7f8c..fc933203b5 100755
--- a/superset/cli/importexport.py
+++ b/superset/cli/importexport.py
@@ -135,12 +135,13 @@ if feature_flags.get("VERSIONED_EXPORT"):
@click.option(
"--path",
"-p",
+ required=True,
help="Path to a single ZIP file",
)
@click.option(
"--username",
"-u",
- default=None,
+ required=True,
help="Specify the user name to assign dashboards to",
)
def import_dashboards(path: str, username: Optional[str]) -> None:
diff --git a/tests/integration_tests/charts/commands_tests.py b/tests/integration_tests/charts/commands_tests.py
index f9785a4dd6..5d29bfe817 100644
--- a/tests/integration_tests/charts/commands_tests.py
+++ b/tests/integration_tests/charts/commands_tests.py
@@ -190,6 +190,7 @@ class TestImportChartsCommand(SupersetTestCase):
)
dataset = chart.datasource
assert json.loads(chart.params) == {
+ "annotation_layers": [],
"color_picker": {"a": 1, "b": 135, "g": 122, "r": 0},
"datasource": dataset.uid,
"js_columns": ["color"],
diff --git a/tests/integration_tests/cli_tests.py b/tests/integration_tests/cli_tests.py
index f9195a6c26..be591b8146 100644
--- a/tests/integration_tests/cli_tests.py
+++ b/tests/integration_tests/cli_tests.py
@@ -235,7 +235,8 @@ def test_import_dashboards_versioned_export(import_dashboards_command, app_conte
runner = app.test_cli_runner()
response = runner.invoke(
- superset.cli.importexport.import_dashboards, ("-p", "dashboards.json")
+ superset.cli.importexport.import_dashboards,
+ ("-p", "dashboards.json", "-u", "admin"),
)
assert response.exit_code == 0
@@ -249,7 +250,8 @@ def test_import_dashboards_versioned_export(import_dashboards_command, app_conte
runner = app.test_cli_runner()
response = runner.invoke(
- superset.cli.importexport.import_dashboards, ("-p", "dashboards.zip")
+ superset.cli.importexport.import_dashboards,
+ ("-p", "dashboards.zip", "-u", "admin"),
)
assert response.exit_code == 0
@@ -283,7 +285,8 @@ def test_failing_import_dashboards_versioned_export(
runner = app.test_cli_runner()
response = runner.invoke(
- superset.cli.importexport.import_dashboards, ("-p", "dashboards.json")
+ superset.cli.importexport.import_dashboards,
+ ("-p", "dashboards.json", "-u", "admin"),
)
assert_cli_fails_properly(response, caplog)
@@ -295,7 +298,8 @@ def test_failing_import_dashboards_versioned_export(
runner = app.test_cli_runner()
response = runner.invoke(
- superset.cli.importexport.import_dashboards, ("-p", "dashboards.zip")
+ superset.cli.importexport.import_dashboards,
+ ("-p", "dashboards.zip", "-u", "admin"),
)
assert_cli_fails_properly(response, caplog)
diff --git a/tests/integration_tests/fixtures/importexport.py b/tests/integration_tests/fixtures/importexport.py
index cda9dd0bcc..aa694c7a5d 100644
--- a/tests/integration_tests/fixtures/importexport.py
+++ b/tests/integration_tests/fixtures/importexport.py
@@ -14,6 +14,7 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
+from copy import deepcopy
from typing import Any
# example V0 import/export format
@@ -573,6 +574,40 @@ chart_config: dict[str, Any] = {
"version": "1.0.0",
"dataset_uuid": "10808100-158b-42c4-842e-f32b99d88dfb",
}
+chart_config_with_mixed_annotations: dict[str, Any] = deepcopy(chart_config)
+chart_config_with_mixed_annotations["params"]["annotation_layers"] = [
+ {
+ "name": "Formula test layer",
+ "annotationType": "FORMULA",
+ "color": None,
+ "descriptionColumns": [],
+ "hideLine": False,
+ "opacity": "",
+ "overrides": {"time_range": None},
+ "show": True,
+ "showLabel": False,
+ "showMarkers": False,
+ "style": "solid",
+ "value": "100000",
+ "width": 1,
+ },
+ {
+ "name": "Native layer to be removed on import",
+ "annotationType": "EVENT",
+ "sourceType": "NATIVE",
+ "color": None,
+ "opacity": "",
+ "style": "solid",
+ "width": 1,
+ "showMarkers": False,
+ "hideLine": False,
+ "value": 2,
+ "overrides": {"time_range": None},
+ "show": True,
+ "showLabel": False,
+ "descriptionColumns": [],
+ },
+]
dashboard_config: dict[str, Any] = {
"dashboard_title": "Test dash",
diff --git a/tests/unit_tests/charts/commands/importers/v1/import_test.py b/tests/unit_tests/charts/commands/importers/v1/import_test.py
index 06e0063fe9..f2c4773417 100644
--- a/tests/unit_tests/charts/commands/importers/v1/import_test.py
+++ b/tests/unit_tests/charts/commands/importers/v1/import_test.py
@@ -108,3 +108,22 @@ def test_import_chart_without_permission(
str(excinfo.value)
== "Chart doesn't exist and user doesn't have permission to create charts"
)
+
+
+def test_filter_chart_annotations(mocker: MockFixture, session: Session) -> None:
+ """
+ Test importing a chart.
+ """
+ from superset import security_manager
+ from superset.charts.commands.importers.v1.utils import filter_chart_annotations
+ from tests.integration_tests.fixtures.importexport import (
+ chart_config_with_mixed_annotations,
+ )
+
+ config = copy.deepcopy(chart_config_with_mixed_annotations)
+ filter_chart_annotations(config)
+ params = config["params"]
+ annotation_layers = params["annotation_layers"]
+
+ assert len(annotation_layers) == 1
+ assert all([al["annotationType"] == "FORMULA" for al in annotation_layers])