diff --git a/superset/charts/commands/importers/v1/utils.py b/superset/charts/commands/importers/v1/utils.py index 3ef0a2ed78b49..91ec58406e246 100644 --- a/superset/charts/commands/importers/v1/utils.py +++ b/superset/charts/commands/importers/v1/utils.py @@ -28,6 +28,22 @@ 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 27bb0f7f8c088..fc933203b5ae3 100755 --- a/superset/cli/importexport.py +++ b/superset/cli/importexport.py @@ -135,12 +135,13 @@ def export_datasources(datasource_file: Optional[str] = None) -> None: @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 f9785a4dd6c20..5d29bfe8178f9 100644 --- a/tests/integration_tests/charts/commands_tests.py +++ b/tests/integration_tests/charts/commands_tests.py @@ -190,6 +190,7 @@ def test_import_v1_chart(self, sm_g, utils_g): ) 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 f9195a6c26684..be591b8146e29 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 cda9dd0bcc379..aa694c7a5d047 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 @@ "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 06e0063fe93ad..f2c47734170ff 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])