codeant-ai-for-open-source[bot] commented on code in PR #40984:
URL: https://github.com/apache/superset/pull/40984#discussion_r3399161922
##########
superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx:
##########
@@ -315,23 +315,33 @@ export default function PluginFilterSelect(props:
PluginFilterSelectProps) {
const uniqueOptions = useMemo(() => {
const allOptions = new Set(data.map(el => el[col]));
- return [...allOptions].map((value: string) => ({
+ const baseOptions = [...allOptions].map((value: string) => ({
label: labelFormatter(value, datatype),
value,
isNewOption: false,
}));
- }, [data, datatype, col, labelFormatter]);
+ if (creatable !== false && filterState.value) {
+ const existing = new Set(allOptions);
+ ensureIsArray(filterState.value)
+ .filter(v => v !== null && v !== undefined && !existing.has(v))
+ .forEach(v => {
+ baseOptions.push({
+ label: String(v),
+ value: String(v),
+ isNewOption: true,
+ });
+ existing.add(v);
+ });
+ }
+ return baseOptions;
+ }, [data, datatype, col, labelFormatter, creatable, filterState.value]);
const options = useMemo(() => {
- if (search && !multiSelect && !hasOption(search, uniqueOptions, true)) {
- uniqueOptions.unshift({
- label: search,
- value: search,
- isNewOption: true,
- });
+ if (search && creatable !== false && !hasOption(search, uniqueOptions,
true)) {
+ return [{ label: search, value: search, isNewOption: true },
...uniqueOptions];
}
Review Comment:
**Suggestion:** The new-option injection condition does not check
`searchAllOptions`, so a "create new" option is still shown when remote search
mode disables creation (`allowNewOptions={!searchAllOptions && creatable !==
false}`). In that mode users can pick a synthetic value that should not be
creatable, causing inconsistent filter state and incorrect queries. Add the
same `!searchAllOptions` guard here so option creation behavior matches the
Select props. [incorrect condition logic]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Remote-search value filters still allow synthetic created options.
- ⚠️ Dashboards may apply filters on non-existent, user-typed values.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In the native filter configuration UI implemented by
`FiltersConfigForm.tsx` (see
`controlsOrder` including `'creatable'` and `'searchAllOptions'` at
`superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx:149-154`),
create a `filter_select` filter, leave `creatable` at its default `true`,
and enable
`searchAllOptions` so that remote search is active.
2. Load a dashboard with this filter, which renders the `PluginFilterSelect`
component
(`superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx:119-151`)
and
passes `formData.searchAllOptions` and `formData.creatable` into the
component; note that
the Select control is configured with `allowNewOptions={!searchAllOptions &&
creatable !==
false}` at lines 600-605, disabling built-in new-option creation when
`searchAllOptions`
is true.
3. Type a value into the filter's search box that is not present in the
current option
list while `searchAllOptions` remains true; `onSearch` at lines 265-279
updates the local
`search` state and, because `searchAllOptions` is true, dispatches
`ownState.search` for
remote querying, but regardless of remote results the component's local
`search` variable
now contains the typed text.
4. Because `options` is computed at lines 339-344 without checking
`searchAllOptions`
(only `search && creatable !== false && !hasOption(search, uniqueOptions,
true)`), the
typed term is injected as `{ label: search, value: search, isNewOption: true
}` even when
`searchAllOptions` is true; the `Select` at lines 600-633 receives this
synthetic option
in its `options` prop despite `allowNewOptions` being false, so the user can
select a
"created" value in a mode where creation should be disabled, leading to
inconsistent
filter state and queries built via `updateDataMask` at lines 216-245
containing that
synthetic value.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=2c5a7ba401a14bd5a87aeb21bce5825c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=2c5a7ba401a14bd5a87aeb21bce5825c&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/filters/components/Select/SelectFilterPlugin.tsx
**Line:** 340:342
**Comment:**
*Incorrect Condition Logic: The new-option injection condition does not
check `searchAllOptions`, so a "create new" option is still shown when remote
search mode disables creation (`allowNewOptions={!searchAllOptions && creatable
!== false}`). In that mode users can pick a synthetic value that should not be
creatable, causing inconsistent filter state and incorrect queries. Add the
same `!searchAllOptions` guard here so option creation behavior matches the
Select props.
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%2F40984&comment_hash=a9aa1d8e97aca6a08e28702d5f21cd996a5f999788125bad7bf84341336e5d8b&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40984&comment_hash=a9aa1d8e97aca6a08e28702d5f21cd996a5f999788125bad7bf84341336e5d8b&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]
