codeant-ai-for-open-source[bot] commented on code in PR #40905:
URL: https://github.com/apache/superset/pull/40905#discussion_r3455111814
##########
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.tsx:
##########
@@ -249,6 +344,45 @@ 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}
+ />
+ }
+ >
Review Comment:
**Suggestion:** The new plugin column-picker control ignores
`controlItem.config.required`, so required plugin controls are not validated
before save. This allows saving an incomplete filter configuration and can lead
to runtime failures when the plugin expects a mandatory column value. Add form
validation rules for `isColumnSelect` controls the same way the main `groupby`
column control enforces required fields. [incomplete implementation]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Native filter plugins save without required column configured.
- ❌ Plugin filters may crash on missing column configuration.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open the "Manage native filters" modal for a dashboard, which renders
`FiltersConfigForm`
(`superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx`,
lines 40–55 in the shown snippet) and calls `getControlItemsMap` with the
current
`filterType` and `datasetId`.
2. Register a native filter plugin whose control panel registration (via
`getChartControlPanelRegistry` used in `getControlItemsMap` at line 187)
exposes a
`CustomControlItem` with `config.isColumnSelect === true` and
`config.required === true`
(e.g. a "Metric column" control needed by the plugin).
3. In the modal, add or edit a filter of that plugin type so that
`getControlItemsMap`
builds the plugin column control; it is rendered using `<StyledFormItem>`
with
`name={['filters', filterId, 'controlValues', controlItem.name]}` at lines
358–368, but
without any `rules` array or required validator wired to
`controlItem.config.required`.
4. Leave this plugin column control empty and save/apply the filter; because
there are no
validation rules for this field (unlike the main `groupby` column at lines
228–243 which
enforces `required: mainControlItem.config?.required && !removed`), the form
does not
block submission, and the filter configuration is saved with
`controlValues[controlItem.name]` unset, even though the plugin expects a
mandatory column
and may fail at runtime when consuming this configuration.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=ec9f9545a4be46c0b97e5bbbc55e7f21&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=ec9f9545a4be46c0b97e5bbbc55e7f21&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:** 358:368
**Comment:**
*Incomplete Implementation: The new plugin column-picker control
ignores `controlItem.config.required`, so required plugin controls are not
validated before save. This allows saving an incomplete filter configuration
and can lead to runtime failures when the plugin expects a mandatory column
value. Add form validation rules for `isColumnSelect` controls the same way the
main `groupby` column control enforces required fields.
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=aa54a77bf646efc8b859a7369ac5cff3ef88c9cadd78871be164aec8426337c7&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40905&comment_hash=aa54a77bf646efc8b859a7369ac5cff3ef88c9cadd78871be164aec8426337c7&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]
