villebro commented on code in PR #23905:
URL: https://github.com/apache/superset/pull/23905#discussion_r1182552084
##########
superset/migrations/shared/migrate_viz/base.py:
##########
@@ -68,22 +69,51 @@ def _migrate(self) -> None:
if key in self.rename_keys:
rv_data[self.rename_keys[key]] = value
+ continue
if key in self.remove_keys:
continue
rv_data[key] = value
+ generic_chart_axes = is_feature_enabled("GENERIC_CHART_AXES")
+ if generic_chart_axes:
+ self._migrate_temporal_filter(rv_data)
+
self.data = rv_data
def _post_action(self) -> None:
- """some actions after migrate"""
+ """Some actions after migrate"""
+
+ def _migrate_temporal_filter(self, rv_data: Dict[str, Any]) -> None:
+ """Adds a temporal filter."""
+ granularity_sqla = rv_data.pop("granularity_sqla", None)
+ time_range = rv_data.pop("time_range", None)
+
+ if not granularity_sqla or not time_range:
+ return
Review Comment:
Is this correct - shouldn't we create at least a "No filter" time filter as
long as `granularity_sqla` is defined? Or is `time_range` in practice always
defined?
##########
superset/migrations/shared/migrate_viz/base.py:
##########
@@ -68,22 +69,51 @@ def _migrate(self) -> None:
if key in self.rename_keys:
rv_data[self.rename_keys[key]] = value
+ continue
if key in self.remove_keys:
continue
rv_data[key] = value
+ generic_chart_axes = is_feature_enabled("GENERIC_CHART_AXES")
+ if generic_chart_axes:
Review Comment:
nit: as we're not reusing `generic_chart_axes, maybe this would be cleaner:
```suggestion
if is_feature_enabled("GENERIC_CHART_AXES"):
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
