codeant-ai-for-open-source[bot] commented on code in PR #37136:
URL: https://github.com/apache/superset/pull/37136#discussion_r2694118439
##########
superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx:
##########
@@ -428,8 +428,12 @@ const ColumnSelectPopover = ({
/>
),
key: calculatedColumn.column_name,
+ column_name: calculatedColumn.column_name,
+ verbose_name:
+ calculatedColumn.verbose_name ?? '',
}),
)}
+ optionFilterProps={['column_name', 'verbose_name']}
Review Comment:
**Suggestion:** The Select prop `optionFilterProps` (plural) is not the Ant
Design prop used to customize filtering behavior — AntD expects
`optionFilterProp` (singular) or a custom `filterOption` function; using
`optionFilterProps` will likely be ignored and the new searchable fields won't
be used. Replace this with a custom `filterOption` implementation that checks
the option object's `column_name` and `verbose_name` so both fields are
searchable. [logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Saved expressions search ignores verbose names.
- ❌ Users cannot find saved columns by display label.
- ⚠️ UX regression in Explore column selection.
```
</details>
```suggestion
filterOption={(input, option) => {
const opt = option as any;
const search = String(input).toLowerCase();
return (
String(opt.column_name ||
'').toLowerCase().includes(search) ||
String(opt.verbose_name ||
'').toLowerCase().includes(search)
);
}}
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Start the frontend and open the Explore UI that mounts
ColumnSelectPopover.
Inspect the Saved tab Select in file
superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx
— see the Select options mapping at lines ~421-436 and the added prop at
line 436.
2. Ensure there is a calculated (saved) column whose verbose_name differs
from its
column_name.
The option value is set at line ~423: "value:
calculatedColumn.column_name" and the
label node is at lines ~425-429.
3. Open the Saved tab, focus the Select search box and type a string present
only in the
column's verbose_name (not column_name).
4. Observe that the list does not filter to the verbose_name match because
the Select prop
used in the PR is optionFilterProps (plural) at line 436,
which Ant Design does not recognize — Select continues to use the option
value
(column_name) by default.
Note: The existing code attempts to make verbose_name searchable but uses a
nonstandard
prop name; this is an actual regression in the saved expressions search UX.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx
**Line:** 436:436
**Comment:**
*Logic Error: The Select prop `optionFilterProps` (plural) is not the
Ant Design prop used to customize filtering behavior — AntD expects
`optionFilterProp` (singular) or a custom `filterOption` function; using
`optionFilterProps` will likely be ignored and the new searchable fields won't
be used. Replace this with a custom `filterOption` implementation that checks
the option object's `column_name` and `verbose_name` so both fields are
searchable.
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>
##########
superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx:
##########
@@ -557,8 +563,15 @@ const ColumnSelectPopover = ({
</MetricOptionContainer>
),
key: `metric-${metric.metric_name}`,
+ metric_name: metric.metric_name,
+ verbose_name: metric.verbose_name ?? '',
})),
]}
+ optionFilterProps={[
+ 'column_name',
+ 'verbose_name',
+ 'metric_name',
+ ]}
Review Comment:
**Suggestion:** The Simple tab also uses `optionFilterProps` (plural) which
will likely be ignored; to search across multiple fields (column name, verbose
name, and metric name) provide a `filterOption` function that checks each
option property rather than relying on a nonstandard prop name. [logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ "Columns and metrics" search ignores display labels.
- ❌ Users struggle finding metrics by verbose name.
- ⚠️ Search UX degraded in Explore controls.
```
</details>
```suggestion
filterOption={(input, option) => {
const opt = option as any;
const search = String(input).toLowerCase();
return (
String(opt.column_name ||
'').toLowerCase().includes(search) ||
String(opt.verbose_name ||
'').toLowerCase().includes(search) ||
String(opt.metric_name ||
'').toLowerCase().includes(search)
);
}}
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Launch the frontend and open the Explore UI that shows the "Simple" tab of
ColumnSelectPopover.
The Simple tab Select is defined in
superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx
(see the Select block in the Simple tab around lines 539-575).
2. Verify a metric exists where metric.verbose_name differs from
metric.metric_name.
In the options array the option value is set (value: metric.metric_name)
at ~line 556
and the displayed label node is at ~lines 558-563.
3. In the Simple tab Select, type a search term that matches a metric's
verbose_name but
not its metric_name.
4. Observe no filtering occurs for that verbose_name because the PR added
optionFilterProps (plural) at lines 570-574,
which the underlying Select implementation does not use — multi-field
searching
therefore fails in the Simple tab.
Note: Replacing this nonstandard prop with an explicit filterOption allows
reliable
multi-field searches.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/src/explore/components/controls/DndColumnSelectControl/ColumnSelectPopover.tsx
**Line:** 570:574
**Comment:**
*Logic Error: The Simple tab also uses `optionFilterProps` (plural)
which will likely be ignored; to search across multiple fields (column name,
verbose name, and metric name) provide a `filterOption` function that checks
each option property rather than relying on a nonstandard prop name.
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]
