diff --git a/superset/cli/importexport.py b/superset/cli/importexport.py index 3d4f87dd26bf8..fc6a9ad3c4682 100755 --- a/superset/cli/importexport.py +++ b/superset/cli/importexport.py @@ -130,12 +130,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/superset/commands/chart/importers/v1/utils.py b/superset/commands/chart/importers/v1/utils.py index d27b631f97fde..f905f8cc3ad4f 100644 --- a/superset/commands/chart/importers/v1/utils.py +++ b/superset/commands/chart/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/tests/integration_tests/charts/commands_tests.py b/tests/integration_tests/charts/commands_tests.py index 87c7823ae5ab8..b6adf197f5751 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 2441809b0da64..1b2c5f8b4eeb1 100644 --- a/tests/integration_tests/cli_tests.py +++ b/tests/integration_tests/cli_tests.py @@ -160,7 +160,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 @@ -174,7 +175,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 @@ -205,7 +207,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) @@ -217,7 +220,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 8fa61539dcc4b..5096c272335bc 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 @@ -575,6 +576,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 f0d142644df25..903b7468baf21 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.commands.chart.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])