aminghadersohi commented on code in PR #44107:
URL: https://github.com/apache/superset/pull/44107#discussion_r3990520944
##########
superset/mcp_service/dashboard/schemas.py:
##########
@@ -1770,15 +1777,122 @@ def serialize_chart_summary(
)
+def _native_filter_value_is_valid(filter_type: str, value: Any) -> bool:
+ """Validate display-value shapes without interpreting them as
predicates."""
+ if value is None:
+ return True
+ if filter_type == "filter_time":
+ return isinstance(value, str)
+ if filter_type == "filter_range":
+ return (
+ isinstance(value, list)
+ and len(value) == 2
+ and all(
+ item is None
+ or (isinstance(item, (int, float)) and not isinstance(item,
bool))
+ for item in value
+ )
+ )
+ if filter_type == "filter_timegrain":
+ return (
+ isinstance(value, list)
+ and len(value) <= 1
+ and all(isinstance(item, str) for item in value)
+ )
+ # Select values may be JSON scalars or flat scalar lists, never nested
metadata.
+ values = value if isinstance(value, list) else [value]
+ return all(
+ item is None or isinstance(item, (str, int, float, bool)) for item in
values
+ )
+
+
def redact_filter_state_data_model_metadata(
filter_state: Dict[str, Any],
+ native_filters: list[NativeFilterSummary] | None = None,
) -> Dict[str, Any]:
- """Remove permalink filter state fields that expose data-model metadata."""
- return {
+ """Hide raw metadata, retaining known native filters' display values.
+
+ Match IDs and types against dashboard configuration, not caller-supplied
+ mask metadata. Time-column and custom filters can carry column names even
+ in their value or label, and therefore are not projected.
+ """
+ result = {
key: value
for key, value in filter_state.items()
- if key not in {"dataMask", "chartStates"}
+ if key
+ not in {
+ "dataMask",
+ "chartStates",
+ "native_filter_values",
+ "native_filter_values_incomplete",
+ }
}
+ if native_filters is None or not (
+ {"dataMask", "chartStates"} & filter_state.keys()
+ ):
+ return result
+
+ summaries: list[dict[str, Any]] = []
+ mask = filter_state.get("dataMask", {})
+ incomplete = bool(filter_state.get("chartStates")) or not isinstance(mask,
dict)
+ known_filters = {item.id: item for item in native_filters}
+ for filter_id, entry in mask.items() if isinstance(mask, dict) else []:
+ native_filter = known_filters.get(filter_id)
+ if (
+ native_filter is None
+ or native_filter.filter_type
+ not in {
+ "filter_select",
+ "filter_range",
+ "filter_time",
+ "filter_timegrain",
+ }
+ or not isinstance(entry, dict)
+ or not isinstance(entry.get("filterState"), dict)
+ or "value" not in entry["filterState"]
+ ):
+ incomplete = True
+ continue
+ extra = entry.get("extraFormData", {})
+ # Filter IDs survive type changes. A saved time-column mask can contain
+ # column names even when the dashboard config describes a supported
type.
+ if not isinstance(extra, dict) or "granularity_sqla" in extra:
+ incomplete = True
+ continue
Review Comment:
Addressed in b17d9977d6. Empty extraFormData is not sufficient to prove that
a filter was cleared: caller-supplied display-only state is also supported.
Retained values with empty or missing extraFormData are now marked incomplete,
without guessing whether a predicate is active. Explicitly cleared
null/empty-list values remain supported. Added regression coverage for all four
supported types and documented this distinction.
##########
superset/mcp_service/dashboard/schemas.py:
##########
@@ -1770,15 +1777,122 @@ def serialize_chart_summary(
)
+def _native_filter_value_is_valid(filter_type: str, value: Any) -> bool:
+ """Validate display-value shapes without interpreting them as
predicates."""
+ if value is None:
+ return True
+ if filter_type == "filter_time":
+ return isinstance(value, str)
+ if filter_type == "filter_range":
+ return (
+ isinstance(value, list)
+ and len(value) == 2
+ and all(
+ item is None
+ or (isinstance(item, (int, float)) and not isinstance(item,
bool))
+ for item in value
+ )
+ )
+ if filter_type == "filter_timegrain":
+ return (
+ isinstance(value, list)
+ and len(value) <= 1
+ and all(isinstance(item, str) for item in value)
+ )
+ # Select values may be JSON scalars or flat scalar lists, never nested
metadata.
+ values = value if isinstance(value, list) else [value]
+ return all(
+ item is None or isinstance(item, (str, int, float, bool)) for item in
values
+ )
+
+
def redact_filter_state_data_model_metadata(
filter_state: Dict[str, Any],
+ native_filters: list[NativeFilterSummary] | None = None,
) -> Dict[str, Any]:
- """Remove permalink filter state fields that expose data-model metadata."""
- return {
+ """Hide raw metadata, retaining known native filters' display values.
+
+ Match IDs and types against dashboard configuration, not caller-supplied
+ mask metadata. Time-column and custom filters can carry column names even
+ in their value or label, and therefore are not projected.
+ """
+ result = {
key: value
for key, value in filter_state.items()
- if key not in {"dataMask", "chartStates"}
+ if key
+ not in {
+ "dataMask",
+ "chartStates",
+ "native_filter_values",
+ "native_filter_values_incomplete",
+ }
}
+ if native_filters is None or not (
+ {"dataMask", "chartStates"} & filter_state.keys()
+ ):
+ return result
+
+ summaries: list[dict[str, Any]] = []
+ mask = filter_state.get("dataMask", {})
+ incomplete = bool(filter_state.get("chartStates")) or not isinstance(mask,
dict)
+ known_filters = {item.id: item for item in native_filters}
+ for filter_id, entry in mask.items() if isinstance(mask, dict) else []:
+ native_filter = known_filters.get(filter_id)
+ if (
+ native_filter is None
+ or native_filter.filter_type
+ not in {
+ "filter_select",
+ "filter_range",
+ "filter_time",
+ "filter_timegrain",
+ }
+ or not isinstance(entry, dict)
+ or not isinstance(entry.get("filterState"), dict)
+ or "value" not in entry["filterState"]
+ ):
+ incomplete = True
+ continue
+ extra = entry.get("extraFormData", {})
+ # Filter IDs survive type changes. A saved time-column mask can contain
+ # column names even when the dashboard config describes a supported
type.
+ if not isinstance(extra, dict) or "granularity_sqla" in extra:
+ incomplete = True
+ continue
+ # A display value does not describe SQL predicates or wildcard
+ # matching. Signal that the summary cannot express these semantics.
+ predicates = extra.get("filters", [])
+ if extra.get("adhoc_filters") or (
+ native_filter.filter_type == "filter_select"
+ and (
+ not isinstance(predicates, list)
+ or any(
+ not isinstance(predicate, dict)
+ or predicate.get("op") not in ("IN", "NOT IN")
+ for predicate in predicates
+ )
+ )
+ ):
Review Comment:
Fixed in b17d9977d6. Predicate validation now covers every supported type:
select allows IN/NOT IN, range allows >=/<=/==, and time/time-grain filters do
not accept column-filter predicates as complete context. Malformed predicate
lists and unsupported operators mark the summary incomplete. Added negative
coverage across all four types and positive coverage for built-in predicates.
All 536 dashboard tests and changed-file pre-commit hooks pass.
--
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]
