feat: Add Area chart migration and tweaks the Timeseries chart migration

pytest.ini

@@ -17,4 +17,4 @@
 [pytest]
 testpaths =
     tests
-python_files = *_test.py test_*.py *_tests.py
+python_files = *_test.py test_*.py *_tests.py *viz/utils.py

superset/charts/commands/importers/v1/utils.py

@@ -75,7 +75,6 @@ def migrate_chart(config: dict[str, Any]) -> dict[str, Any]:
         if isclass(class_)
         and issubclass(class_, MigrateViz)
         and hasattr(class_, "source_viz_type")
-        and class_ != processors.MigrateAreaChart  # incomplete
     }
 
     output = copy.deepcopy(config)

superset/migrations/shared/migrate_viz/base.py

@@ -160,9 +160,7 @@ def upgrade(cls, session: Session) -> None:
         slices = session.query(Slice).filter(Slice.viz_type == cls.source_viz_type)
         for slc in paginated_update(
             slices,
-            lambda current, total: print(
-                f"  Updating {current}/{total} charts", end="\r"
-            ),
+            lambda current, total: print(f"Upgraded {current}/{total} charts"),
         ):
             new_viz = cls.upgrade_slice(slc)
             session.merge(new_viz)
@@ -177,9 +175,7 @@ def downgrade(cls, session: Session) -> None:
         )
         for slc in paginated_update(
             slices,
-            lambda current, total: print(
-                f"  Downgrading {current}/{total} charts", end="\r"
-            ),
+            lambda current, total: print(f"Downgraded {current}/{total} charts"),
         ):
             new_viz = cls.downgrade_slice(slc)
             session.merge(new_viz)

superset/migrations/shared/migrate_viz/processors.py

@@ -36,40 +36,6 @@ def _pre_action(self) -> None:
             self.data["metric"] = self.data["metrics"][0]
 
 
-class MigrateAreaChart(MigrateViz):
-    """
-    Migrate area charts.
-
-    This migration is incomplete, see https://github.com/apache/superset/pull/24703#discussion_r1265222611
-    for more details. If you fix this migration, please update the ``migrate_chart``
-    function in ``superset/charts/commands/importers/v1/utils.py`` so that it gets
-    applied in chart imports.
-    """
-
-    source_viz_type = "area"
-    target_viz_type = "echarts_area"
-    remove_keys = {"contribution", "stacked_style", "x_axis_label"}
-
-    def _pre_action(self) -> None:
-        if self.data.get("contribution"):
-            self.data["contributionMode"] = "row"
-
-        if stacked := self.data.get("stacked_style"):
-            stacked_map = {
-                "expand": "Expand",
-                "stack": "Stack",
-            }
-            self.data["show_extra_controls"] = True
-            self.data["stack"] = stacked_map.get(stacked)
-
-        if x_axis := self.data.get("granularity_sqla"):
-            self.data["x_axis"] = x_axis
-
-        if x_axis_label := self.data.get("x_axis_label"):
-            self.data["x_axis_title"] = x_axis_label
-            self.data["x_axis_title_margin"] = 30
-
-
 class MigratePivotTable(MigrateViz):
     source_viz_type = "pivot_table"
     target_viz_type = "pivot_table_v2"
@@ -137,10 +103,22 @@ class MigrateSunburst(MigrateViz):
 
 class TimeseriesChart(MigrateViz):
     has_x_axis_control = True
+    rename_keys = {
+        "bottom_margin": "x_axis_title_margin",
+        "left_margin": "y_axis_title_margin",
+        "show_controls": "show_extra_controls",
+        "x_axis_label": "x_axis_title",
+        "x_axis_format": "x_axis_time_format",
+        "x_ticks_layout": "xAxisLabelRotation",
+        "y_axis_label": "y_axis_title",
+        "y_axis_showminmax": "truncateYAxis",
+        "y_log_scale": "logAxis",
+    }
+    remove_keys = {"contribution", "show_brush", "show_markers"}
 
     def _pre_action(self) -> None:
         self.data["contributionMode"] = "row" if self.data.get("contribution") else None
-        self.data["zoomable"] = self.data.get("show_brush") != "no"
+        self.data["zoomable"] = self.data.get("show_brush") == "yes"
         self.data["markerEnabled"] = self.data.get("show_markers") or False
         self.data["y_axis_showminmax"] = True
 
@@ -163,23 +141,19 @@ def _pre_action(self) -> None:
             "difference" if comparison_type == "absolute" else comparison_type
         )
 
+        if x_ticks_layout := self.data.get("x_ticks_layout"):
+            self.data["x_ticks_layout"] = 45 if x_ticks_layout == "45°" else 0
+
 
 class MigrateLineChart(TimeseriesChart):
     source_viz_type = "line"
     target_viz_type = "echarts_timeseries_line"
-    rename_keys = {
-        "x_axis_label": "x_axis_title",
-        "bottom_margin": "x_axis_title_margin",
-        "x_axis_format": "x_axis_time_format",
-        "y_axis_label": "y_axis_title",
-        "left_margin": "y_axis_title_margin",
-        "y_axis_showminmax": "truncateYAxis",
-        "y_log_scale": "logAxis",
-    }
 
     def _pre_action(self) -> None:
         super()._pre_action()
 
+        self.remove_keys.add("line_interpolation")
+
         line_interpolation = self.data.get("line_interpolation")
         if line_interpolation == "cardinal":
             self.target_viz_type = "echarts_timeseries_smooth"
@@ -189,3 +163,24 @@ def _pre_action(self) -> None:
         elif line_interpolation == "step-after":
             self.target_viz_type = "echarts_timeseries_step"
             self.data["seriesType"] = "end"
+
+
+class MigrateAreaChart(TimeseriesChart):
+    source_viz_type = "area"
+    target_viz_type = "echarts_area"
+    stacked_map = {
+        "expand": "Expand",
+        "stack": "Stack",
+        "stream": "Stream",
+    }
+
+    def _pre_action(self) -> None:
+        super()._pre_action()
+
+        self.remove_keys.add("stacked_style")
+
+        self.data["stack"] = self.stacked_map.get(
+            self.data.get("stacked_style") or "stack"
+        )
+
+        self.data["opacity"] = 0.7

tests/unit_tests/charts/commands/importers/v1/utils_test.py

@@ -31,13 +31,21 @@ def test_migrate_chart_area() -> None:
         "description": None,
         "certified_by": None,
         "certification_details": None,
-        "viz_type": "area",
+        "viz_type": "echarts_area",
         "query_context": None,
         "params": json.dumps(
             {
-                "adhoc_filters": [],
+                "adhoc_filters": [
+                    {
+                        "clause": "WHERE",
+                        "subject": "ds",
+                        "operator": "TEMPORAL_RANGE",
+                        "comparator": "No filter",
+                        "expressionType": "SIMPLE",
+                    }
+                ],
                 "annotation_layers": [],
-                "bottom_margin": "auto",
+                "x_axis_title_margin": "auto",
                 "color_scheme": "supersetColors",
                 "comparison_type": "values",
                 "dashboards": [],

tests/unit_tests/migrations/viz/dual_line_to_mixed_chart_test.py

@@ -14,9 +14,10 @@
 # KIND, either express or implied.  See the License for the
 # specific language governing permissions and limitations
 # under the License.
-import json
+from typing import Any
 
 from superset.migrations.shared.migrate_viz import MigrateDualLine
+from tests.unit_tests.migrations.viz.utils import migrate_and_assert
 
 ADHOC_FILTERS = [
     {
@@ -28,7 +29,7 @@
     }
 ]
 
-SOURCE_FORM_DATA = {
+SOURCE_FORM_DATA: dict[str, Any] = {
     "metric": "num_boys",
     "y_axis_format": ",d",
     "y_axis_bounds": [50, 100],
@@ -42,7 +43,7 @@
     "yAxisIndex": 0,
 }
 
-TARGET_FORM_DATA = {
+TARGET_FORM_DATA: dict[str, Any] = {
     "metrics": ["num_boys"],
     "y_axis_format": ",d",
     "y_axis_bounds": [50, 100],
@@ -64,34 +65,4 @@
 def test_migration() -> None:
     source = SOURCE_FORM_DATA.copy()
     target = TARGET_FORM_DATA.copy()
-    upgrade_downgrade(source, target)
-
-
-def upgrade_downgrade(source, target) -> None:
-    from superset.models.slice import Slice
-
-    dumped_form_data = json.dumps(source)
-
-    slc = Slice(
-        viz_type=MigrateDualLine.source_viz_type,
-        datasource_type="table",
-        params=dumped_form_data,
-        query_context=f'{{"form_data": {dumped_form_data}}}',
-    )
-
-    # upgrade
-    slc = MigrateDualLine.upgrade_slice(slc)
-
-    # verify form_data
-    new_form_data = json.loads(slc.params)
-    assert new_form_data == target
-    assert new_form_data["form_data_bak"] == source
-
-    # verify query_context
-    new_query_context = json.loads(slc.query_context)
-    assert new_query_context["form_data"]["viz_type"] == "mixed_timeseries"
-
-    # downgrade
-    slc = MigrateDualLine.downgrade_slice(slc)
-    assert slc.viz_type == MigrateDualLine.source_viz_type
-    assert json.loads(slc.params) == source
+    migrate_and_assert(MigrateDualLine, source, target)

tests/unit_tests/migrations/viz/nvd3_area_chart_to_echarts_test.py

@@ -0,0 +1,42 @@
+# 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.
+from typing import Any
+
+from superset.migrations.shared.migrate_viz import MigrateAreaChart
+from tests.unit_tests.migrations.viz.utils import (
+    migrate_and_assert,
+    TIMESERIES_SOURCE_FORM_DATA,
+    TIMESERIES_TARGET_FORM_DATA,
+)
+
+SOURCE_FORM_DATA: dict[str, Any] = {
+    "viz_type": "area",
+    "stacked_style": "stream",
+}
+
+TARGET_FORM_DATA: dict[str, Any] = {
+    "form_data_bak": SOURCE_FORM_DATA,
+    "viz_type": "echarts_area",
+    "opacity": 0.7,
+    "stack": "Stream",
+}
+
+
+def test_migration() -> None:
+    SOURCE_FORM_DATA.update(TIMESERIES_SOURCE_FORM_DATA)
+    TARGET_FORM_DATA.update(TIMESERIES_TARGET_FORM_DATA)
+    migrate_and_assert(MigrateAreaChart, SOURCE_FORM_DATA, TARGET_FORM_DATA)