codeant-ai-for-open-source[bot] commented on code in PR #37165:
URL: https://github.com/apache/superset/pull/37165#discussion_r2704656932
##########
superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.jsx:
##########
@@ -316,6 +316,7 @@ function ColumnCollectionTable({
control={
<Select
ariaLabel={t('Data type')}
+ header={<FormLabel>{t('Data type')}</FormLabel>}
Review Comment:
**Suggestion:** The added JSX uses the generic `Select` component which
doesn't integrate with the form Field API and likely won't wire the
value/onChange into the dataset column item; replace the `Select` usage with
`SelectControl` and provide the `controlId` so the Field/Fieldset will
correctly bind this input to the `type` field (this ensures the value is saved
and the label is handled by the form control). [possible bug]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Data type may not persist on calculated columns.
- ⚠️ Dataset editor form binding inconsistent.
- ⚠️ Users may lose edits when saving dataset.
```
</details>
```suggestion
<SelectControl
controlId="type"
ariaLabel={t('Data type')}
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open
superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.jsx
and find the Field for "type" in the calculated-columns expandFieldset. The
control JSX is
at hunk lines 317-324 (317 <Select ... 324 />).
2. Note the pattern used elsewhere in this file: controls wired to fields
typically use
form-aware controls (e.g., TextControl has controlId set at lines ~296-300
for
"verbose_name"). Those controls integrate with Field/Fieldset binding.
3. Start the web app, go to dataset list, click Edit on a dataset, switch to
Calculated
columns tab (the render for this tab mounts ColumnCollectionTable with
allowEditDataType
enabled — see the render of the Calculated columns tab where
ColumnCollectionTable is
rendered).
4. Add or edit a calculated column and change the Data type using the
rendered Select (the
JSX at lines 317-324). Because the code uses the generic Select without
controlId/form-binding semantics, the Field/Fieldset machinery is not
receiving input
through the same API other controls use. This can be observed by comparing
other Field
controls in the same file that supply controlId (e.g., TextControl at lines
~296-300).
Replacing the JSX with <SelectControl controlId="type" .../> at the same
location binds
this input to the Field API, ensuring values are persisted when the dataset
is saved.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.jsx
**Line:** 317:319
**Comment:**
*Possible Bug: The added JSX uses the generic `Select` component which
doesn't integrate with the form Field API and likely won't wire the
value/onChange into the dataset column item; replace the `Select` usage with
`SelectControl` and provide the `controlId` so the Field/Fieldset will
correctly bind this input to the `type` field (this ensures the value is saved
and the label is handled by the form control).
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.
```
</details>
--
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]
