codeant-ai-for-open-source[bot] commented on code in PR #39068:
URL: https://github.com/apache/superset/pull/39068#discussion_r3407206342
##########
superset-frontend/packages/superset-ui-core/src/components/Select/Select.tsx:
##########
@@ -689,8 +756,8 @@ const Select = forwardRef(
setSelectValue(value);
}
} else {
- const token = tokenSeparators.find(token =>
pastedText.includes(token));
- const array = token ? uniq(pastedText.split(token)) : [pastedText];
+ e.preventDefault();
+ const array = uniq(splitWithQuoteEscaping(pastedText,
tokenSeparators));
Review Comment:
**Suggestion:** Unconditionally calling `preventDefault()` in multi-select
paste handling drops user input when pasted text does not resolve to selectable
values (for example with `allowNewOptions` disabled). In that case the paste is
neither inserted into the search box nor converted to chips, so the user action
is lost. Only prevent default when tokenization will actually consume the
paste, or explicitly push the pasted text into search input. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ AG Grid Table "Ordering" control drops unmatched pasted text.
- ⚠️ Multi-select chart controls cannot use paste for searching.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In the AG Grid Table visualization, the "Ordering" control is configured
in
`superset-frontend/plugins/plugin-chart-ag-grid-table/src/controlPanel.tsx:181–195`
with
`type: 'SelectControl'` and `multi: true` (line 186), so it uses
`SelectControl` in
multi-select mode with default `freeForm`/`allowNewOptions` false.
2. `SelectControl` in
`superset-frontend/src/explore/components/controls/SelectControl.tsx`
renders the core
`Select` component at lines 97–113, building `selectProps` at lines 65–95.
For this
control, it sets `mode` to `'multiple'` when `multi` is true (line 78) and
passes
`allowNewOptions: freeForm` (line 66), which is `false` by default.
3. In the Explore UI for an AG Grid Table chart, focus the "Ordering"
multi-select input
and paste text that does not exactly match existing order-by choices (e.g. a
copied column
label with additional context, or multiple values separated by
commas/newlines).
4. The paste event triggers `onPaste` in `Select.tsx:751–797`. With
`mode='multiple'`, it
takes the `else` branch at lines 759–795, calling `e.preventDefault()` (line
759), then
`splitWithQuoteEscaping(pastedText, tokenSeparators)` (line 760) and
`getPastedTextValue`
for each token (lines 764–776). When no token matches an option and
`allowNewOptions` is
false (line 732), `getPastedTextValue` returns `undefined`, resulting in no
new values
being added while the default paste was blocked. The user sees neither new
chips nor
search text, so the pasted input is effectively lost.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=5e578fcee1d648899f360f57b9706a5c&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=5e578fcee1d648899f360f57b9706a5c&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:** 759:760
**Comment:**
*Logic Error: Unconditionally calling `preventDefault()` in
multi-select paste handling drops user input when pasted text does not resolve
to selectable values (for example with `allowNewOptions` disabled). In that
case the paste is neither inserted into the search box nor converted to chips,
so the user action is lost. Only prevent default when tokenization will
actually consume the paste, or explicitly push the pasted text into search
input.
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%2F39068&comment_hash=f06f8444d9aa926e0590f60d86e3523a0f6e9d57228f563511d703c85db3a710&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39068&comment_hash=f06f8444d9aa926e0590f60d86e3523a0f6e9d57228f563511d703c85db3a710&reaction=dislike'>👎</a>
##########
superset-frontend/packages/superset-ui-core/src/components/Select/AsyncSelect.tsx:
##########
@@ -669,8 +728,8 @@ const AsyncSelect = forwardRef(
setSelectValue(value);
}
} else {
- const token = tokenSeparators.find(token =>
pastedText.includes(token));
- const array = token ? uniq(pastedText.split(token)) : [pastedText];
+ e.preventDefault();
+ const array = uniq(splitWithQuoteEscaping(pastedText,
tokenSeparators));
const values = (
Review Comment:
**Suggestion:** Unconditionally preventing default paste behavior in
multi-select causes pasted text to be lost when it cannot be mapped to existing
options and new options are not allowed. This makes paste-based searching fail
because nothing is inserted into the input. Gate `preventDefault()` behind
successful token consumption or preserve pasted text in `inputValue`. [logic
error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Dashboard owners/roles/tags selectors drop unmatched pasted text.
- ⚠️ Users cannot paste names/emails to search async options.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open a dashboard and click the "Edit dashboard" properties modal, which
renders
`AccessSection` from
`superset-frontend/src/dashboard/components/PropertiesModal/sections/AccessSection.tsx:57–176`.
The "Owners" field uses `<AsyncSelect mode="multiple" ...>` at lines
110–124, relying on
the core `AsyncSelect` component in multi-select mode with default
`allowNewOptions=false`.
2. Focus the Owners `AsyncSelect` input and paste a string that does not
exactly match any
current owner label, e.g. `"nonexistent owner"` or a partial name. Because no
`allowNewOptions` prop is passed, `AsyncSelect` uses the default
`allowNewOptions = false`
(see `AsyncSelect` props destructuring at `AsyncSelect.tsx:115–150`).
3. The paste event triggers `onPaste` in `AsyncSelect.tsx:323–341`. Since
`mode !==
'single'`, execution enters the `else` branch at lines 331–339 where
`e.preventDefault()`
is called (line 331) and `splitWithQuoteEscaping(pastedText,
tokenSeparators)` builds
`array` (line 332). For each token, `getPastedTextValue` at lines 297–320
calls
`getOption(text, fullSelectOptions, true)` and, when no option is found and
`allowNewOptions` is false (line 307), returns `undefined`.
4. Because all tokens resolve to `undefined`, the `values` array at lines
333–335 is empty
and `setSelectValue(previous => [...previous, ...values])` leaves state
unchanged, while
the earlier `e.preventDefault()` prevents the browser from inserting the
pasted text into
the search input. The user sees no new chips and no search text, so the
pasted input is
silently discarded instead of being usable for searching.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=1ea10f46a1a2486b9aa9133718d2c07f&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=1ea10f46a1a2486b9aa9133718d2c07f&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/AsyncSelect.tsx
**Line:** 732:733
**Comment:**
*Logic Error: Unconditionally preventing default paste behavior in
multi-select causes pasted text to be lost when it cannot be mapped to existing
options and new options are not allowed. This makes paste-based searching fail
because nothing is inserted into the input. Gate `preventDefault()` behind
successful token consumption or preserve pasted text in `inputValue`.
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%2F39068&comment_hash=4b1673573ece59629dc1cc0cc24d1837f22cac1ca8145a57012223a73fc7b24c&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39068&comment_hash=4b1673573ece59629dc1cc0cc24d1837f22cac1ca8145a57012223a73fc7b24c&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]
