codeant-ai-for-open-source[bot] commented on code in PR #40905:
URL: https://github.com/apache/superset/pull/40905#discussion_r3444123697
##########
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.tsx:
##########
@@ -249,6 +344,36 @@ export default function getControlItemsMap({
);
mapControlItems[controlItem.name] = { element, checked: initialValue };
});
+
+ // Render plugin-declared column-picker controls using config hooks
+ controlItems
+ .filter((item: CustomControlItem) => item?.config?.isColumnSelect === true)
+ .forEach(controlItem => {
+ const initialValue = resolveInitialValue(
+ controlItem,
+ filterToEdit,
+ customizationToEdit,
+ );
+ const element = (
+ <StyledFormItem
+ expanded={expanded}
+ name={['filters', filterId, 'controlValues', controlItem.name]}
+ initialValue={initialValue}
+ label={
+ <ControlLabel
+ label={controlItem.config?.label}
+ description={controlItem.config?.description}
+ />
+ }
+ >
+ <DatasetColumnSelect datasetId={datasetId} onChange={notifyChange} />
+ </StyledFormItem>
Review Comment:
**Suggestion:** The new plugin column-picker path does not clear
`defaultDataMask` when the selected dataset column changes, unlike the existing
`groupby` column handler. This leaves stale default values bound to the
previous column and can save invalid defaults for the new column. Reset the
default mask in this on-change path before triggering re-render/form-change.
[incomplete implementation]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Plugin filters may persist defaults for wrong column.
- ⚠️ Dashboards may load with mismatched filter defaults.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open the native filter configuration flow, which renders
`FiltersConfigForm` (see
`FiltersConfigForm.tsx:62-80`) and note that it builds `controlItems` by
calling
`getControlItemsMap` (call site at `FiltersConfigForm.tsx:200-215`).
2. Register or use a native filter plugin whose controlPanel declares a
`CustomControlItem` with `config.isColumnSelect === true`;
`getControlItemsMap` will pick
this up via the `isColumnSelect` filter at `getControlItemsMap.tsx:349-352`
and render it
as a form field bound to `['filters', filterId, 'controlValues',
controlItem.name]`.
3. In the Manage native filters modal, for that plugin filter, configure a
non-empty
default value so that `filters[filterId].defaultDataMask` is populated (see
`Filter`
including `defaultDataMask` in `getControlItemsMap.test.tsx:78-82` and
dashboard filter
fixtures using `defaultDataMask` in `DashboardContainer.test.tsx:67-70`),
then change the
plugin-declared column via the new column picker rendered from
`<DatasetColumnSelect
datasetId={datasetId} onChange={notifyChange} />` at
`getControlItemsMap.tsx:369`.
4. Observe in code that this plugin column-change path only invokes
`notifyChange` (which
calls `forceUpdate` and `formChanged` at `getControlItemsMap.tsx:190-193`)
and never calls
`setNativeFilterFieldValues` to clear or adjust `defaultDataMask`, unlike
the built-in
`groupby` column handler at `getControlItemsMap.tsx:257-262` which
explicitly sets `{
defaultDataMask: null }` when the main column changes; as a result, the
filter retains a
stale `defaultDataMask` built for the previous column, which will be used
when computing
dependency defaults (`FiltersConfigForm.tsx:471-477`) and applying the
filter, allowing
invalid defaults to be saved for the new column.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=4f6f58456a6749c882b59a7931c23c6d&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=4f6f58456a6749c882b59a7931c23c6d&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/FiltersConfigForm/getControlItemsMap.tsx
**Line:** 370:370
**Comment:**
*Incomplete Implementation: The new plugin column-picker path does not
clear `defaultDataMask` when the selected dataset column changes, unlike the
existing `groupby` column handler. This leaves stale default values bound to
the previous column and can save invalid defaults for the new column. Reset the
default mask in this on-change path before triggering re-render/form-change.
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=39ad17f509d9b799776aab50db9fb90189f27ae6bc0221005ee6b35ef4b6d22b&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40905&comment_hash=39ad17f509d9b799776aab50db9fb90189f27ae6bc0221005ee6b35ef4b6d22b&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]
