bito-code-review[bot] commented on code in PR #43176:
URL: https://github.com/apache/superset/pull/43176#discussion_r4174200043
##########
superset/charts/data/form_data.py:
##########
@@ -31,33 +31,69 @@ def set_form_data(form_data: dict[str, Any]) -> None:
g.form_data = form_data
+def _as_form_data_dict(value: Any) -> dict[str, Any]:
+ return value if isinstance(value, dict) else {}
+
+
+def _as_query_list(value: Any) -> list[Any]:
+ if isinstance(value, (list, tuple)):
+ return list(value)
+ return []
+
+
def _serialize_query(
query: QueryObject,
form_data: dict[str, Any],
-) -> dict[str, Any]:
- """Serialize query fields consumed by the Jinja form-data fallback."""
- query_data = dict(query.to_dict())
- query_data["filters"] = query.filter
- if query.time_range is not None:
- query_data["time_range"] = query.time_range
+ time_range: str | None = None,
+) -> dict[str, Any] | None:
+ """Serialize query fields consumed by the Jinja form-data fallback.
+
+ Incomplete stubs (unit-test doubles without ``to_dict``) are skipped so
+ callers can still publish datasource context for Jinja without requiring a
+ full ``QueryObject``.
+
+ ``time_range`` is an optional overlay for callers that deliberately leave
+ ``QueryObject.time_range`` unset (tabular queries, so relative ranges keep
+ ``from_dttm``/``to_dttm`` in the cache key). Chart and async callers omit
+ it so ``get_time_filter()`` matches the chart-data API: a TEMPORAL_RANGE
+ filter alone is not a published time range.
+ """
+ to_dict = getattr(query, "to_dict", None)
+ if not callable(to_dict):
+ return None
+
+ query_data = dict(to_dict())
+ filters = getattr(query, "filter", None)
+ query_data["filters"] = filters
Review Comment:
<!-- Bito Reply -->
The change is appropriate as it removes defensive branching that was only
necessary for test stubs, aligning the production code with the expected
`QueryObject` interface. By using direct access to `to_dict()`, `filter`, and
`time_range`, the implementation is now cleaner and avoids masking potential
attribute errors.
**superset/charts/data/form_data.py**
```
query_data = dict(query.to_dict())
query_data["filters"] = query.filter
if query.time_range is not None:
query_data["time_range"] = query.time_range
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
