fix(import): only import FORMULA annotations (apache#26652)

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:

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"])
 

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"],

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)

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",

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])