codeant-ai-for-open-source[bot] commented on code in PR #41136:
URL: https://github.com/apache/superset/pull/41136#discussion_r3477134416
##########
superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx:
##########
@@ -592,6 +592,7 @@ export default function PluginFilterSelect(props:
PluginFilterSelectProps) {
allowClear
autoClearSearchValue
allowNewOptions={!searchAllOptions && creatable !== false}
+ allowNewOptionsOnPaste={multiSelect && searchAllOptions}
Review Comment:
**Suggestion:** This enables `allowNewOptionsOnPaste` for all multi-select
dynamic filters, including non-string columns. Unknown pasted values are
created as strings in `Select`, so numeric/temporal filters can receive string
values and fail to match correctly (or behave inconsistently across databases).
Gate this behavior to compatible datatypes (e.g., string) or add type-aware
coercion before updating the filter state. [type error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Numeric dynamic filters pass pasted values as strings.
- ⚠️ Temporal filters may mis-compare pasted values across databases.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure a native Select filter on a non-string column (e.g., numeric or
temporal) so
that `coltypeMap[col]` is `GenericDataType.Numeric` or
`GenericDataType.Temporal`
(`SelectFilterPlugin.tsx:153-166`), and set `formData.multiSelect = true`,
`formData.searchAllOptions = true` in the filter configuration (controls
wired via
`PluginFilterSelectCustomizeProps` in `FiltersConfigForm.tsx:8-17`).
2. Render the dashboard; `PluginFilterSelect` builds `options` from the
query data
(`SelectFilterPlugin.tsx:57-64`) using `data.map(el => el[col])`. For
numeric/temporal
columns these `value` fields are numbers or temporal objects, but the
`Select` component
is invoked with `allowNewOptionsOnPaste={multiSelect && searchAllOptions}`
(`SelectFilterPlugin.tsx:331-338`), which evaluates to `true` regardless of
datatype.
3. Paste a comma-separated list into the filter where at least one
numeric/temporal value
is not currently in the loaded options page but does exist in the underlying
dataset
(e.g., `"10,20"` where `10` is not in the first page). In
`Select.tsx:48-72`, `onPaste`
tokenizes the text and, for each item, calls `getOption(item,
fullSelectOptions, true)`;
for values not in the loaded options this returns `undefined`.
4. Because `allowNewOptionsOnPaste` is `true`, `keepUnknownValues` is `true`
(`Select.tsx:66-72`) and the unknown pasted value is turned into a new
option with `label:
item` and `value: item` (`Select.tsx:74-84`), where `item` is a string
(e.g., `'10'`).
`setSelectValue` appends these string values, `handleChange` in
`SelectFilterPlugin.tsx:28-37` passes them to `updateDataMask`, and
`getSelectExtraFormData` (`superset-frontend/src/filters/utils.ts:50-86`)
builds an `IN`
filter with `val` containing string representations of numeric/temporal
values. This
introduces a type mismatch between the column's actual type and the filter
values, which
can lead to incorrect matching or DB-dependent coercion behavior.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=c5b3b2b7886a4049afaf537c05d4cf89&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=c5b3b2b7886a4049afaf537c05d4cf89&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:** 595:595
**Comment:**
*Type Error: This enables `allowNewOptionsOnPaste` for all multi-select
dynamic filters, including non-string columns. Unknown pasted values are
created as strings in `Select`, so numeric/temporal filters can receive string
values and fail to match correctly (or behave inconsistently across databases).
Gate this behavior to compatible datatypes (e.g., string) or add type-aware
coercion before updating the filter state.
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%2F41136&comment_hash=5284860e81d9de8e96ef254e096a8260dcd358a59a83e2a14b1f6c471abbb76b&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41136&comment_hash=5284860e81d9de8e96ef254e096a8260dcd358a59a83e2a14b1f6c471abbb76b&reaction=dislike'>👎</a>
##########
superset-frontend/packages/superset-ui-core/src/components/Select/Select.tsx:
##########
@@ -690,20 +693,34 @@ const Select = forwardRef(
}
} else {
const token = tokenSeparators.find(token =>
pastedText.includes(token));
- const array = token ? uniq(pastedText.split(token)) : [pastedText];
+ const array = token
+ ? uniq(
+ pastedText
+ .split(token)
+ .map(item => item.trim())
+ .filter(Boolean),
+ )
+ : [pastedText.trim()];
Review Comment:
**Suggestion:** The paste parser can create an empty selected value when
`allowNewOptionsOnPaste` is enabled and the pasted text is blank/whitespace (or
trims to empty). In the no-separator path, `[pastedText.trim()]` keeps `''`,
and the unknown-value branch then turns that into a new option with empty
`label`/`value`. This can apply an unintended empty filter value. Normalize the
single-value branch the same way as the tokenized branch (drop empty strings
before mapping). [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Whitespace paste selects empty option in Select multi-filters.
- ⚠️ Dashboard filters apply unexpected empty value to queries.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure a native Select filter using `PluginFilterSelect`
(`superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx:119-151`)
on a
column, with `formData.multiSelect = true` and `formData.searchAllOptions =
true` so that
dynamic search-all is enabled.
2. Render a dashboard where this filter plugin is used; in the non-like
operator path,
`PluginFilterSelect` renders the core `Select` component
(`SelectFilterPlugin.tsx:331-338`) with `mode="multiple"` and
`allowNewOptionsOnPaste={multiSelect && searchAllOptions}` which evaluates
to `true`.
3. With the filter's combobox focused, paste a clipboard value that is all
whitespace or
blank (e.g. `" "`). In `Select.tsx:48-64`, `onPaste` reads `pastedText` and
finds no token
separator, so `array` is constructed as `[pastedText.trim()]`, which becomes
`['']` and is
not filtered for empties.
4. Still in `Select.tsx:74-88`, `keepUnknownValues` is `true` because
`allowNewOptionsOnPaste` is `true`; `getOption('', fullSelectOptions, true)`
returns no
match, so an `isNewOption` with `label: ''` and `value: ''` is pushed into
`newOptions`,
`values` includes `''`, and `setSelectValue` appends this empty value.
`handleChange` in
`SelectFilterPlugin.tsx:28-37` then forwards this array (including `''`) into
`updateDataMask`, which propagates an unintended empty filter value into
`extraFormData`.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=f67d34579b70484f98eb617c3da4966a&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=f67d34579b70484f98eb617c3da4966a&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/packages/superset-ui-core/src/components/Select/Select.tsx
**Line:** 696:703
**Comment:**
*Logic Error: The paste parser can create an empty selected value when
`allowNewOptionsOnPaste` is enabled and the pasted text is blank/whitespace (or
trims to empty). In the no-separator path, `[pastedText.trim()]` keeps `''`,
and the unknown-value branch then turns that into a new option with empty
`label`/`value`. This can apply an unintended empty filter value. Normalize the
single-value branch the same way as the tokenized branch (drop empty strings
before mapping).
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%2F41136&comment_hash=d2345ebb1a7096f59c3a2b825656954d99d5014462feed9f555dafc7aa2fc271&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41136&comment_hash=d2345ebb1a7096f59c3a2b825656954d99d5014462feed9f555dafc7aa2fc271&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]
