aminghadersohi commented on code in PR #44096: URL: https://github.com/apache/superset/pull/44096#discussion_r4019878253
########## superset/mcp_service/dashboard/tool/apply_dashboard_filters.py: ########## @@ -0,0 +1,459 @@ +# 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. + +""" +MCP tool: apply_dashboard_filters + +Applies native filter VALUES to a dashboard for the calling user by +storing a ``dataMask`` in a dashboard permalink. The dashboard's saved +native filter configuration is untouched, so nothing another viewer sees +changes -- which is what separates this tool from manage_native_filters, +which DEFINES the filters and writes to the shared dashboard. +""" + +import logging +from typing import Any + +from fastmcp import Context +from superset_core.mcp.decorators import tool, ToolAnnotations + +from superset.constants import EMPTY_FILTER_SQL_EXPRESSION, NO_TIME_RANGE +from superset.extensions import event_logger +from superset.mcp_service.dashboard.permalink import ( + build_dashboard_permalink_url, + create_dashboard_permalink, + get_dashboard_permalink_data_mask, +) +from superset.mcp_service.dashboard.schemas import ( + AppliedFilterSummary, + ApplyDashboardFiltersRequest, + ApplyDashboardFiltersResponse, + ApplyFilterValueSpec, + FilterSelectValue, +) +from superset.mcp_service.dashboard.tool.manage_native_filters import ( + current_native_filter_config, +) + +logger = logging.getLogger(__name__) + +# Filter types this tool knows how to apply a value to. Kept in step with the +# types manage_native_filters can create. +SUPPORTED_FILTER_TYPES: frozenset[str] = frozenset({"filter_select", "filter_time"}) + +# Display strings the frontend uses when labelling a selected value; mirrored +# here so a permalink's label reads the same as a UI-applied one. +_NULL_LABEL = "<NULL>" +_TRUE_LABEL = "TRUE" +_FALSE_LABEL = "FALSE" + + +class _FilterApplyError(Exception): + """Raised internally when a requested filter value cannot be applied.""" + + +def _publish_filters_applied(dashboard_id: int, permalink_key: str) -> bool: + """Best-effort principal nudge; filter state stays in the authorized permalink.""" + from superset.coordination.base import CoordinationService + from superset.realtime.publish import publish_realtime + from superset.websocket.channel import get_realtime_principal + + try: + if not CoordinationService.is_backend_defined(): + return False + principal = get_realtime_principal() + if principal is None: + return False + return publish_realtime( + topic="dashboard.filters_applied", + scope="principal", + payload={"dashboard_id": dashboard_id, "permalink_key": permalink_key}, + routes=[principal["channel"]], + ) + except Exception: # noqa: BLE001 pylint: disable=broad-except + logger.warning( + "Failed to publish filters applied for dashboard %s", dashboard_id + ) + return False + + +def _describe_filters(configs: list[dict[str, Any]]) -> str: + """Render the dashboard's filters as a name/ID list for error messages.""" + if not configs: + return "This dashboard has no native filters." + described = ", ".join( + f"{conf.get('name') or '(unnamed)'} (id={conf.get('id')}, " + f"type={conf.get('filterType')})" + for conf in configs + ) + return f"Filters on this dashboard: {described}." + + +def _resolve_filter(reference: str, configs: list[dict[str, Any]]) -> dict[str, Any]: + """Resolve a filter name or ID to its configuration. + + An exact ID match wins over a name match, so a filter whose display name + happens to equal another filter's ID cannot shadow that filter. + """ + for conf in configs: + if conf.get("id") == reference: + return conf + + wanted = reference.strip().casefold() + matches = [ + conf + for conf in configs + if isinstance(conf.get("name"), str) + and conf["name"].strip().casefold() == wanted + ] + if len(matches) == 1: + return matches[0] + if matches: + raise _FilterApplyError( + f"'{reference}' matches more than one filter on this dashboard " + f"({', '.join(str(conf.get('id')) for conf in matches)}). " + "Pass the filter ID instead of the name." + ) + raise _FilterApplyError( + f"No filter named '{reference}' was found on this dashboard. " + f"{_describe_filters(configs)}" + ) + + +def _value_label(value: FilterSelectValue) -> str: + """Format one selected value the way the dashboard UI labels it.""" + if value is None: + return _NULL_LABEL + if isinstance(value, bool): + return _TRUE_LABEL if value else _FALSE_LABEL + return str(value) + + +def _select_data_mask( + conf: dict[str, Any], values: list[FilterSelectValue] +) -> dict[str, Any]: + """Build the data mask a filter_select filter produces for ``values``. + + Mirrors the frontend's ``getSelectExtraFormData``: a non-empty selection + becomes an ``IN`` predicate on the filter's target column, and an empty + selection on a filter marked ``enableEmptyFilter`` becomes an impossible + predicate (the "required filter, nothing chosen" state) rather than no + filtering at all. + """ + targets = [target for target in (conf.get("targets") or []) if target] + column = (targets[0].get("column") or {}).get("name") if targets else None + if not column: + raise _FilterApplyError( + f"Filter '{conf.get('name') or conf.get('id')}' has no target " + "column, so a value cannot be applied to it." + ) + + control_values = conf.get("controlValues") or {} + if control_values.get("inverseSelection"): + raise _FilterApplyError( + f"Filter '{conf.get('name') or conf.get('id')}' enables inverse " + "selection, which this tool does not support." + ) + if (operator := control_values.get("operatorType", "exact")) != "exact": + raise _FilterApplyError( + f"Filter '{conf.get('name') or conf.get('id')}' uses matching " + f"operator '{operator}', which this tool does not support. " + "Only exact-match select filters are supported." + ) + + if values: + extra_form_data: dict[str, Any] = { + "filters": [{"col": column, "op": "IN", "val": list(values)}] + } + filter_state: dict[str, Any] = { + "value": list(values), + "label": ", ".join(_value_label(value) for value in values), + } + else: + extra_form_data = ( + { + "adhoc_filters": [ + { + "expressionType": "SQL", + "clause": "WHERE", + "sqlExpression": EMPTY_FILTER_SQL_EXPRESSION, + } + ] + } + if control_values.get("enableEmptyFilter") + else {} + ) + filter_state = {"value": None} + + return {"extraFormData": extra_form_data, "filterState": filter_state} + + +def _time_data_mask(time_range: str) -> dict[str, Any]: + """Build the data mask a filter_time filter produces for ``time_range``.""" + is_set = bool(time_range) and time_range != NO_TIME_RANGE Review Comment: I chose to reject clears for required time filters. Unlike required select filters, the dashboard has no established empty-value sentinel for time filters, so inventing one here would broaden frontend hydration semantics and risk changing query behavior. The tool now returns a clear validation error before creating a permalink when a required time filter is given `No filter`; optional time filters still clear normally. This keeps the live and permalink paths consistent, and the new backend regression test covers a required filter with a saved default. Fixed in 10196a117d. ########## superset-frontend/src/dashboard/useDashboardFilterSync.ts: ########## @@ -0,0 +1,112 @@ +/** + * 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. + */ +import { useEffect } from 'react'; +import { useDispatch, useStore } from 'react-redux'; +import { t } from '@apache-superset/core/translation'; +import { + addSuccessToast, + removeToast, +} from 'src/components/MessageToasts/actions'; +import { removeDataMask, updateDataMask } from 'src/dataMask/actions'; +import { subscribeRealtime } from 'src/middleware/realtime'; +import { RootState } from './types'; +import { getPermalinkValue } from './components/nativeFilters/FilterBar/keyValue'; + +/** Apply chat permalink state to the mounted dashboard, with one-shot undo. */ +export default function useDashboardFilterSync(dashboardId?: number) { + const dispatch = useDispatch(); + const store = useStore<RootState>(); + + useEffect(() => { + if (!dashboardId) return undefined; + + let active = true; + let request = 0; + let appliedRequest = 0; + let toastId: string | undefined; + const dismissToast = () => { + if (toastId) dispatch(removeToast(toastId)); + toastId = undefined; + }; + const unsubscribe = subscribeRealtime( + 'dashboard.filters_applied', + async payload => { + if ( + !active || + typeof payload !== 'object' || + payload === null || + !('dashboard_id' in payload) || + payload.dashboard_id !== dashboardId || + !('permalink_key' in payload) || + typeof payload.permalink_key !== 'string' || + !payload.permalink_key + ) { + return; + } + request += 1; + const version = request; + try { + const value = await getPermalinkValue(payload.permalink_key); + const mask = value?.state?.dataMask; + if (!active || version !== request || !mask) return; + const entries = Object.entries(mask); + if (!entries.length) return; + + // Read at application time, including edits made during the fetch. + const previous = store.getState().dataMask; + appliedRequest = version; + dismissToast(); + entries.forEach(([id, dataMask]) => { + dispatch(updateDataMask(id, dataMask)); + }); + let undone = false; + const toast = addSuccessToast(t('Filters applied from chat'), { + duration: 8000, + action: { + label: t('Undo'), + onClick: () => { + if (!active || undone || version !== appliedRequest) return; + undone = true; + entries.forEach(([id]) => { Review Comment: Fixed in 10196a117d. After dispatching the notification mask, the hook snapshots the actual applied reducer state. Undo restores an entry only when its current state still deep-equals that applied snapshot, so later user edits are preserved while untouched entries are reverted. The new regression test edits Region after notification delivery, then verifies Undo preserves Region while reverting the untouched time filter. ########## superset-frontend/src/components/MessageToasts/Toast.tsx: ########## @@ -152,6 +153,17 @@ export default function Toast({ toast, onCloseToast }: ToastPresenterProps) { {icon} <Interweave content={toast.text} noHtml={!toast.allowHtml} /> </div> + {toast.action && ( Review Comment: Fixed in 10196a117d using the existing toast action rather than introducing a second control pattern. Toasts with an interactive action no longer auto-dismiss; they remain until the action or close button is activated, avoiding a timer race for keyboard and assistive-technology users. A fake-timer regression test verifies an actionable toast remains past its configured duration. -- 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]
