codeant-ai-for-open-source[bot] commented on code in PR #40905:
URL: https://github.com/apache/superset/pull/40905#discussion_r3455126417
##########
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/NativeFiltersModal.test.tsx:
##########
@@ -33,19 +33,23 @@ Object.defineProperty(window, 'matchMedia', {
})),
});
-jest.mock('@superset-ui/core', () => ({
- ...jest.requireActual('@superset-ui/core'),
- getChartMetadataRegistry: () => ({
- items: {
- filter_select: {
- value: {
- datasourceCount: 1,
- behaviors: ['NATIVE_FILTER'],
- },
+jest.mock('@superset-ui/core', () => {
+ const items: Record<string, { value: Record<string, unknown> }> = {
+ filter_select: {
+ value: {
+ datasourceCount: 1,
+ behaviors: ['NATIVE_FILTER'],
},
Review Comment:
**Suggestion:** This registry mock omits `supportsCascadeDependencies` for
`filter_select`, which makes the tested environment treat the select filter as
non-cascadable and diverges from production behavior. Include the new metadata
field in the mock so modal tests validate dependency UI/logic against realistic
plugin metadata. [incomplete implementation]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Native filter modal tests misrepresent cascade dependency capabilities.
- ⚠️ Cascading regression may pass tests due to incomplete metadata.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In
`superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/NativeFiltersModal.test.tsx:36-52`,
Jest mocks `@superset-ui/core` and defines a `filter_select` registry item
whose `value`
only includes `datasourceCount: 1` and `behaviors: ['NATIVE_FILTER']` (lines
39-42), with
no `supportsCascadeDependencies` field.
2. The test renders `<FiltersConfigModal>` (imported at line 20) via the
`setup()` helper
(lines 61-73), which internally uses `useFilterOperations` from
`superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/hooks/useFilterOperations.ts`
(imported in `FiltersConfigModal.tsx:8-14`).
3. Inside `useFilterOperations.ts:32-37`,
`filterSupportsDependencies(filterType)` calls
`getChartMetadataRegistry().get(filterType)` and returns
`metadata?.supportsCascadeDependencies ?? false`; with the mocked registry,
`metadata.supportsCascadeDependencies` is `undefined` for `filter_select`, so
`filterSupportsDependencies('filter_select')` always evaluates to `false`
during these
tests.
4. In production, the real `FilterSelectPlugin` in
`superset-frontend/src/filters/components/Select/index.ts:28-36` constructs
`ChartMetadata` with `supportsCascadeDependencies: true`, so
`filterSupportsDependencies('filter_select')` returns `true`; this mismatch
means the test
environment treats the select filter as non-cascadable and would hide the
dependency UI
where production shows it, allowing regressions in cascade dependency
behavior to go
untested.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=b03b6b8eb9a84b4daba18a0daf515e75&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=b03b6b8eb9a84b4daba18a0daf515e75&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/NativeFiltersModal.test.tsx
**Line:** 39:42
**Comment:**
*Incomplete Implementation: This registry mock omits
`supportsCascadeDependencies` for `filter_select`, which makes the tested
environment treat the select filter as non-cascadable and diverges from
production behavior. Include the new metadata field in the mock so modal tests
validate dependency UI/logic against realistic plugin metadata.
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=56d2ed626f9bad676ff706cd2fc1397f810cf710dbdacd7426a90c847c6d9d4f&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40905&comment_hash=56d2ed626f9bad676ff706cd2fc1397f810cf710dbdacd7426a90c847c6d9d4f&reaction=dislike'>👎</a>
##########
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/hooks/useFilterOperations.ts:
##########
@@ -18,17 +18,24 @@
*/
import { useCallback } from 'react';
import { t } from '@apache-superset/core/translation';
-import { Filter, Divider, NativeFilterType } from '@superset-ui/core';
+import {
+ 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?.supportsCascadeDependencies ?? false;
Review Comment:
**Suggestion:** The new dependency gate only checks
`supportsCascadeDependencies` and ignores `Behavior.NativeFilter`, so
third-party native filter plugins that register correctly but do not add this
new metadata flag will be silently excluded from cascade dependencies. Add a
fallback to native-filter behavior (or default this capability from behavior)
to preserve the intended registry-driven extensibility. [incomplete
implementation]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Custom native filter plugins lose cascade dependency configuration UI.
- ⚠️ Dashboard filter dependencies incomplete for third-party filter types.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. `ChartMetadataConfig` in
`superset-frontend/packages/superset-ui-core/src/chart/models/ChartMetadata.ts:59-64`
defines an optional `supportsCascadeDependencies?: boolean` field, and the
`ChartMetadata`
constructor defaults `supportsCascadeDependencies = false` when omitted
(lines 137 and
170), regardless of the `behaviors` array.
2. A filter plugin is implemented following the existing pattern in
`superset-frontend/src/filters/components/Select/index.ts:26-36`, using `new
ChartMetadata({ name, description, behaviors: [Behavior.InteractiveChart,
Behavior.NativeFilter], ... })` but, unlike the core plugins, omitting the
`supportsCascadeDependencies` field; this plugin still correctly registers
`Behavior.NativeFilter`.
3. In the dashboard, `FiltersConfigModal` (at
`superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.tsx:59-81`)
wires `useFilterOperations`, whose `filterSupportsDependencies(filterType)`
implementation
in `hooks/useFilterOperations.ts:32-37` fetches metadata via
`getChartMetadataRegistry().get(filterType)` and returns
`metadata?.supportsCascadeDependencies ?? false`, ignoring the
`Behavior.NativeFilter`
flag.
4. `FiltersConfigForm` computes `canDependOnOtherFilters =
filterSupportsDependencies(formFilter?.filterType)` at
`superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx:451-453`
and only renders the `<DependencyList>` ("Filter is dependent on…") block
when this is
true (lines 1117-1126); for a third-party plugin that correctly sets
`Behavior.NativeFilter` but omits `supportsCascadeDependencies`,
`canDependOnOtherFilters`
is false, so the cascade dependency UI never appears and the filter cannot
be used as a
dependency parent, contrary to the PR's stated goal that native-filter
behavior alone
should enable this feature.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=ee508b5e26eb4945b31154c91386b220&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=ee508b5e26eb4945b31154c91386b220&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:** 36:37
**Comment:**
*Incomplete Implementation: The new dependency gate only checks
`supportsCascadeDependencies` and ignores `Behavior.NativeFilter`, so
third-party native filter plugins that register correctly but do not add this
new metadata flag will be silently excluded from cascade dependencies. Add a
fallback to native-filter behavior (or default this capability from behavior)
to preserve the intended registry-driven extensibility.
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=69611f3f1427726bb7809a003ceb111a0577d6f2dc4fa8ce74fb547bcfece06e&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40905&comment_hash=69611f3f1427726bb7809a003ceb111a0577d6f2dc4fa8ce74fb547bcfece06e&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]
