codeant-ai-for-open-source[bot] commented on code in PR #40905:
URL: https://github.com/apache/superset/pull/40905#discussion_r3444122694
##########
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/hooks/useFilterOperations.ts:
##########
@@ -18,17 +18,25 @@
*/
import { useCallback } from 'react';
import { t } from '@apache-superset/core/translation';
-import { Filter, Divider, NativeFilterType } from '@superset-ui/core';
+import {
+ Behavior,
+ Filter,
+ Divider,
+ NativeFilterType,
+ getChartMetadataRegistry,
+} from '@superset-ui/core';
import type { FormInstance } from '@superset-ui/core/components';
import { NativeFiltersForm } from '../types';
import { generateFilterId, hasCircularDependency } from '../utils';
import type { ItemStateManager } from './useItemStateManager';
-export const ALLOW_DEPENDENCIES = [
- 'filter_range',
- 'filter_select',
- 'filter_time',
-];
+export function filterSupportsDependencies(
+ filterType: string | undefined,
+): boolean {
+ if (!filterType) return false;
+ const metadata = getChartMetadataRegistry().get(filterType);
+ return metadata?.behaviors?.includes(Behavior.NativeFilter) ?? false;
Review Comment:
**Suggestion:** The new registry check treats every plugin with
`Behavior.NativeFilter` as dependency-capable, which now allows
`filter_timegrain` and `filter_timecolumn` to be selected as cascade parents.
Those parents emit `extraFormData` keys like
`time_grain_sqla`/`granularity_sqla`, and `FiltersConfigForm` blindly merges
that into child filter queries; for children on different datasets this can
produce invalid query params and backend errors when loading options. Restrict
this check to plugins that explicitly support cascade dependencies (e.g., via a
dedicated metadata flag) instead of using `Behavior.NativeFilter` as the sole
gate. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Child filter default options request can fail with errors.
- ❌ Cascades from time grain/column can break cross-dataset filters.
- ⚠️ Native filter config modal becomes unreliable for some setups.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In the frontend, `filterSupportsDependencies()` at
`useFilterOperations.ts:33-39`
returns `true` for any viz whose metadata behaviors include
`Behavior.NativeFilter`, based
on `getChartMetadataRegistry().get(filterType)`.
2. The Time Grain and Time Column filter plugins
(`src/filters/components/TimeGrain/index.ts:26-32` and
`src/filters/components/TimeColumn/index.ts:26-32`) declare `behaviors:
[Behavior.InteractiveChart, Behavior.NativeFilter]`, so
`filterSupportsDependencies('filter_timegrain')` and
`filterSupportsDependencies('filter_timecolumn')` both evaluate to `true`.
3. In the native filter config modal,
`useFilterOperations().canBeUsedAsDependency()` at
`useFilterOperations.ts:149-161` and `getAvailableFilters()` at
`useFilterOperations.ts:184-203` rely on `filterSupportsDependencies()`, so
Time Grain /
Time Column filters are now offered as valid cascade parents in the "Filter
is dependent
on..." UI (`FiltersConfigForm.tsx:192-195` uses `filterSupportsDependencies`
to gate
dependency support).
4. Configure a dashboard so that (a) you have a `filter_timegrain` or
`filter_timecolumn`
native filter on dataset A, with a default selection persisted (its
`defaultDataMask.extraFormData` contains keys like `time_grain_sqla` or
`granularity_sqla`, as shown in
`docs/static/resources/openapi.json:19596-19605` and
`19628-19635`), and (b) you create a second native filter (e.g.
`filter_select`) on
dataset B and set its dependency to the Time Grain/Time Column filter in the
modal.
5. When editing the dependent filter on dataset B, `FiltersConfigForm`
computes
`dependencies` at `FiltersConfigForm.tsx:77-79`, then builds
`dependenciesDefaultValues`
by merging `filters[dependency].defaultDataMask.extraFormData` for each
parent at
`FiltersConfigForm.tsx:212-220`, which now includes the parent's
`time_grain_sqla` /
`granularity_sqla` keys from dataset A.
6. Still in `FiltersConfigForm`, `refreshHandler` at
`FiltersConfigForm.tsx:7-20` builds a
`formData` payload for fetching default-value options via
`getFormData(...)`, then
overwrites `formData.extra_form_data = dependenciesDefaultValues` at
`FiltersConfigForm.tsx:19` and calls `getChartDataRequest({ formData, ...
})` to query
dataset B.
7. Because `formData.datasource` now points to dataset B (set inside
`getFormData` in
`nativeFilters/utils.ts:47-111`), but `formData.extra_form_data` still
carries
parent-specific `time_grain_sqla` / `granularity_sqla` from dataset A, the
backend
receives inconsistent query parameters (e.g., a `granularity_sqla` column
that does not
exist on dataset B) and can respond with an error; in the UI this manifests
as the child
filter's default-value options failing to load and `setErrorWrapper` /
`setError` being
triggered instead of `defaultValueQueriesData` being populated.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=22a69e44ff1c41d4ac58e3797d6ddcb2&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=22a69e44ff1c41d4ac58e3797d6ddcb2&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/hooks/useFilterOperations.ts
**Line:** 37:38
**Comment:**
*Logic Error: The new registry check treats every plugin with
`Behavior.NativeFilter` as dependency-capable, which now allows
`filter_timegrain` and `filter_timecolumn` to be selected as cascade parents.
Those parents emit `extraFormData` keys like
`time_grain_sqla`/`granularity_sqla`, and `FiltersConfigForm` blindly merges
that into child filter queries; for children on different datasets this can
produce invalid query params and backend errors when loading options. Restrict
this check to plugins that explicitly support cascade dependencies (e.g., via a
dedicated metadata flag) instead of using `Behavior.NativeFilter` as the sole
gate.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40905&comment_hash=b89e13df807a666260c41364e497a58bbed247eb1b6508bc4d12f2ffc35ab786&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40905&comment_hash=b89e13df807a666260c41364e497a58bbed247eb1b6508bc4d12f2ffc35ab786&reaction=dislike'>👎</a>
--
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]
