aminghadersohi commented on code in PR #44096:
URL: https://github.com/apache/superset/pull/44096#discussion_r4036595420
##########
superset/constants.py:
##########
@@ -40,6 +40,10 @@
NO_TIME_RANGE = "No filter"
+# Keep in sync with superset-frontend/src/utils/common.ts; select-filter masks
+# and dataMask hydration use this exact predicate to recognize an explicit
clear.
+EMPTY_FILTER_SQL_EXPRESSION = "1 = 0"
Review Comment:
Good catch, and thanks for tracing it to the consequence rather than just
the duplicate literal. Folded: `_EMPTY_FILTER_SENTINEL =
EMPTY_FILTER_SQL_EXPRESSION`, using the import that was already there.
I also added the guest-token coverage you suggested, in
`tests/unit_tests/security/test_guest_token_empty_filter.py`. The tests are
written against the constant rather than its value, so they lock the coupling
instead of the literal:
- `test_unselected_required_select_filter_passes_extras_validation` — the
sanitized predicate alone passes `extras.where` validation.
- `test_empty_filter_predicate_is_allowed_alongside_stored_sql` — it
composes with the chart's own adhoc SQL.
- `test_other_injected_sql_is_still_rejected` — allowing the sentinel does
not open the vector it guards (`1 = 1` is still rejected).
I checked they actually catch the drift you described rather than just
passing: with `EMPTY_FILTER_SQL_EXPRESSION` changed to `"0 = 1"` and the
sentinel folded, all three pass; restore the sentinel to the old `"1 = 0"`
literal and the first two fail, which is exactly the embedded-chart rejection
you predicted.
##########
docs/admin_docs/configuration/mcp-server.mdx:
##########
@@ -41,6 +41,74 @@ flowchart LR
C --> D[("Database<br/>(Postgres)")]
```
+## Dashboard filter notifications
+
+`apply_dashboard_filters` supports exact-match select filters without inverse
+selection, and time filters. Inverse-selection configurations are rejected.
+Select filters configured for contains, starts-with, or ends-with matching are
+rejected rather than silently applying exact matching. A single-select filter
+accepts at most one value; a longer list is rejected rather than stored as a
+selection the control cannot render. Clearing a required select
+filter with `values: []` matches no rows, including when opening the returned
+permalink; an explicit clear does not restore its saved default.
+
+Returned dashboard permalink URLs use `WEBDRIVER_BASEURL_USER_FRIENDLY` as
+the public origin and respect `APPLICATION_ROOT` (or the request's WSGI
+`SCRIPT_NAME` when mounted behind a proxy). Set the public URL to the
+browser-accessible Superset host; the MCP service's own host may be different.
+
+To stack filters across chat turns, pass the previous
+`apply_dashboard_filters` response's `permalink_key` as `base_permalink_key`
+in the next request for the same dashboard. For example, apply Region = EMEA,
+then pass that key with a Time Range = `2024-01-01 : 2025-01-01` request to
+keep EMEA and add the year constraint. Each new value replaces that filter's
+entire prior entry; unmentioned filters persist. Omit the base key to start
+from dashboard defaults. `applied_filters` summarizes only the new request,
+not the inherited selections. Inherited entries for filters the dashboard no
+longer defines are dropped, so a filter deleted or recreated between turns
+does not carry a stale predicate forward.
+
+Base state is resolved server-side under the calling user's dashboard access
+and merged without display redaction; the raw base mask is not returned.
+Missing or expired keys, resolution failures, access denial, and dashboard
+mismatches fail explicitly without creating a replacement link. Missing and
+expired keys share an error because the permalink store does not distinguish
+them.
+
+After creating a dashboard permalink, `apply_dashboard_filters` attempts a
+best-effort `dashboard.filters_applied` realtime notification scoped to the
+calling principal. Its payload contains only `dashboard_id` and
`permalink_key`;
+filter values and resolved state are not included. Consumers must resolve the
+key through the dashboard's authorized permalink load path.
+
+Live browser updates require `WEBSOCKET_ENABLE = True`, a configured
+`DISTRIBUTED_COORDINATION_CONFIG` backend, a running `superset-websocket`
+server, and a strong, non-default `WEBSOCKET_JWT_SECRET` of at least 32 bytes.
+`WEBSOCKET_URL` must also point at a `ws://`/`wss://` endpoint the browser can
+reach. Its default, `ws://127.0.0.1:8080/`, resolves to the browser's own
+loopback, so a remote deployment that leaves it unchanged has every
+server-side piece configured correctly while live updates never connect.
+The browser user also needs
+`can_read` on `Realtime`. With the default configuration the realtime half
+is inert, but the tool still creates and returns a permalink URL.
+The MCP process and websocket transport must use the same
+`REALTIME_CHANNEL_PREFIX`. This publisher does not require the Global Task
+Framework feature flag.
+
+The response's `live_update_pushed` field reports publication success, not
+browser receipt or application. Without a backend, an identifiable principal,
+or a successful publish, it is `false`; the created `dashboard_url` remains
+the fallback. An open dashboard resolves matching notifications through the
+existing dashboard permalink API and applies the returned filters without a
+reload. A “Filters applied from chat” toast offers **Undo** for eight seconds,
Review Comment:
You're right on both counts, thanks.
Docs updated: the toast now reads as carrying an action and therefore having
no auto-dismiss timer, staying up until Undo or its close button is used. I
also folded in the other half of `10196a117d` that the old wording missed —
that Undo restores only the filters whose current state still matches what was
applied, so an edit made in the meantime survives.
`duration: 8000` in `useDashboardFilterSync.ts` is removed rather than
adjusted. It was inert twice over: `Toast.tsx` skips the timer whenever
`toast.action` is set, and 8000 is `addToast`'s default anyway. A comment now
says why there is no override, so it does not get "helpfully" added back.
The PR body is updated too, including a note on the screenshots — see the
reply to your non-blocking ask below.
--
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]
