codeant-ai-for-open-source[bot] commented on code in PR #41136:
URL: https://github.com/apache/superset/pull/41136#discussion_r3476836054
##########
superset-frontend/packages/superset-ui-core/src/components/Select/Select.tsx:
##########
@@ -693,17 +696,24 @@ const Select = forwardRef(
const array = token ? uniq(pastedText.split(token)) : [pastedText];
const newOptions: SelectOptionsType = [];
+ // When `allowNewOptionsOnPaste` is set, accept pasted values that are
+ // not in the loaded options even if `allowNewOptions` is false. The
+ // full option set is searched server-side and only partially loaded
+ // client-side, so a pasted value can legitimately exist in the dataset
+ // but fall outside the loaded page.
+ const keepUnknownValues = allowNewOptions || allowNewOptionsOnPaste;
const values = array
.map(item => {
const option = getOption(item, fullSelectOptions, true);
- if (!option && allowNewOptions) {
+ if (!option && keepUnknownValues) {
const newOption = {
label: item,
value: item,
isNewOption: true,
};
newOptions.push(newOption);
+ return labelInValue ? { label: item, value: item } : item;
Review Comment:
**Suggestion:** Pasted tokens are used verbatim when
`allowNewOptionsOnPaste` is enabled, so whitespace around comma-separated
values is preserved (for example, `"Liam, OutsideValue"` stores `"
OutsideValue"`). That produces mismatched filter values and can make valid
pasted values fail to match server-side results. Normalize each token (trim)
before lookup/creation so pasted values are compared and stored consistently.
[logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Multi-select searchAllOptions filters mis-match pasted values with
spaces.
- ⚠️ Dashboard queries exclude valid rows for trimmed filter values.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure a native Select filter with `multiSelect=true` and
`searchAllOptions=true`
via the filter config modal (`FiltersConfigForm.tsx:147-156` uses
`PluginFilterSelectCustomizeProps` keys including `multiSelect` and
`searchAllOptions`).
2. Render a dashboard using that filter so `PluginFilterSelect` runs; in
`SelectFilterPlugin.tsx:143-151` it destructures `multiSelect` and
`searchAllOptions` from
`formData`, and in the Select instantiation at
`SelectFilterPlugin.tsx:31-37` passes
`allowNewOptionsOnPaste={multiSelect && searchAllOptions}`, enabling the
special paste
path for multi-select search-all filters.
3. In the running dashboard, paste a comma-separated string that includes a
space after
the comma, e.g. `"Liam, OutsideValue"`, into the filter input; this triggers
`onPaste` in
`Select.tsx:687-691`, which splits `pastedText` on the token separator `,` at
`Select.tsx:695-697`, producing the array `["Liam", " OutsideValue"]` where
the second
token retains a leading space.
4. For the multi-select branch, the paste handler maps tokens at
`Select.tsx:698-71`; when
`getOption(" OutsideValue", fullSelectOptions, true)` finds no loaded option
and
`keepUnknownValues` is true (`allowNewOptionsOnPaste` is enabled) at
`Select.tsx:55-60`,
it creates a new option with `label: " OutsideValue"` and `value: "
OutsideValue"` at
`Select.tsx:61-65`, returns that untrimmed value to `handleChange`
(`SelectFilterPlugin.tsx:288-296`), and `getSelectExtraFormData`
(`filters/utils.ts:50-88`) sends an `IN` filter whose `val` includes `"
OutsideValue"`,
failing to match dataset rows whose actual value is `"OutsideValue"` without
the space.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=4632f44d960345ceb63051707931a5e4&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=4632f44d960345ceb63051707931a5e4&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:** 709:716
**Comment:**
*Logic Error: Pasted tokens are used verbatim when
`allowNewOptionsOnPaste` is enabled, so whitespace around comma-separated
values is preserved (for example, `"Liam, OutsideValue"` stores `"
OutsideValue"`). That produces mismatched filter values and can make valid
pasted values fail to match server-side results. Normalize each token (trim)
before lookup/creation so pasted values are compared and stored consistently.
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=6e153a5bfa8ce0b0153a674cbd0c5b64b89ed5769ad6f41d14ce34a58aa05c4d&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41136&comment_hash=6e153a5bfa8ce0b0153a674cbd0c5b64b89ed5769ad6f41d14ce34a58aa05c4d&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]
