Copilot commented on code in PR #40003:
URL: https://github.com/apache/superset/pull/40003#discussion_r3236461830
##########
superset/viz.py:
##########
@@ -1697,6 +1729,19 @@ def get_data(self, df: pd.DataFrame) -> VizData:
"slices": [slc.data for slc in slices if slc.data is not None],
}
+ @deprecated(deprecated_in="3.0")
+ def get_payload(self, query_obj: QueryObjectDict | None = None) ->
VizPayload:
+ payload = super().get_payload(query_obj)
+ payload["applied_filters"] = self._merge_filter_metadata(
+ payload.get("applied_filters"),
+ self.applied_filters,
+ )
+ payload["rejected_filters"] = self._merge_filter_metadata(
+ payload.get("rejected_filters"),
+ self.rejected_filters,
Review Comment:
`get_payload` reads `self.applied_filters` / `self.rejected_filters`, but
these attributes are only created in `get_data`. If `super().get_payload()`
returns without invoking `get_data` (e.g., due to caching/short-circuit paths),
this can raise `AttributeError`. Consider initializing these attributes on the
instance (preferred) or using `getattr(self, 'applied_filters', [])` /
`getattr(self, 'rejected_filters', [])` here.
##########
superset/viz.py:
##########
@@ -1578,6 +1578,27 @@ class DeckGLMultiLayer(BaseViz):
is_timeseries = False
credits = '<a href="https://uber.github.io/deck.gl/";>deck.gl</a>'
+ @staticmethod
+ def _merge_filter_metadata(
+ *filter_groups: list[dict[str, Any]] | None,
+ ) -> list[dict[str, Any]]:
+ merged_filters: list[dict[str, Any]] = []
+ seen_filters: set[str] = set()
+
+ for filters in filter_groups:
+ for filter_metadata in filters or []:
+ if not isinstance(filter_metadata, dict):
+ continue
+
+ cache_key = json.dumps(filter_metadata, sort_keys=True)
Review Comment:
Using `json.dumps` as a dedupe key can throw `TypeError` if filter metadata
contains non-JSON-serializable values (e.g., enums, datetimes, Decimal). This
would break payload generation for multi-layer charts. Consider making the
serialization robust (e.g., providing a `default=` handler) or using a
non-JSON-based stable keying approach.
##########
superset-frontend/src/dashboard/components/nativeFilters/selectors.ts:
##########
@@ -141,9 +141,20 @@ const selectIndicatorsForChartFromFilter = (
}));
};
+const getQueryFilterMetadata = (
+ chart: any,
+ metadataKey: 'applied_filters' | 'rejected_filters',
+) =>
+ ensureIsArray(chart?.queriesResponse).flatMap(
+ queryResponse =>
+ (metadataKey === 'applied_filters'
+ ? queryResponse?.applied_filters
+ : queryResponse?.rejected_filters) || [],
Review Comment:
This conditional can be simplified by indexing with the key
(`queryResponse?.[metadataKey] || []`), which reduces branching and makes it
easier to extend if more metadata keys are added later. This is optional, but
it improves readability and reduces duplication.
--
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]
