codeant-ai-for-open-source[bot] commented on code in PR #38470:
URL: https://github.com/apache/superset/pull/38470#discussion_r2911575079
##########
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx:
##########
@@ -1698,6 +1718,57 @@ const FiltersConfigForm = (
)}
</CollapsibleControl>
</FormItem>
+ {!isChartCustomization && (
+ <StyledRowFormItem
+ expanded={expanded}
+ name={[
+ 'filters',
+ filterId,
+ 'controlValues',
+ 'operatorType',
+ ]}
+ initialValue={currentOperatorType}
+ label={
+ <>
+ <StyledLabel>{t('Match type')}</StyledLabel>
+
+ <InfoTooltip
+ placement="top"
+ tooltip={t(
+ 'Warning: ILIKE queries may be slow on
large datasets as they cannot use indexes effectively.',
+ )}
+ />
+ </>
+ }
+ >
+ <Select
+ ariaLabel={t('Match type')}
+ options={[
+ {
+ value: SelectFilterOperatorType.Exact,
+ label: t('Exact match (IN)'),
+ },
+ {
+ value: SelectFilterOperatorType.Contains,
+ label: t('Contains text (ILIKE %x%)'),
+ },
+ {
+ value: SelectFilterOperatorType.StartsWith,
+ label: t('Starts with (ILIKE x%)'),
+ },
+ {
+ value: SelectFilterOperatorType.EndsWith,
+ label: t('Ends with (ILIKE %x)'),
+ },
+ ]}
+ onChange={value => {
+ onOperatorTypeChanged(
+ value as SelectFilterOperatorType,
+ );
+ }}
+ />
+ </StyledRowFormItem>
+ )}
Review Comment:
**Suggestion:** The new "Match type" selector is rendered for all native
filter types (time, range, group-by, etc.), but `operatorType` is only honored
by the Select (Value) filter; this results in a configuration control that
appears to affect other filter types but is silently ignored, which is
misleading and can lead to incorrect assumptions about how those filters work.
[logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Non-select filters show unused "Match type" configuration.
- ⚠️ Dashboard authors may assume ILIKE works on range filters.
- ⚠️ Misleading UX in native filter settings modal for many filters.
```
</details>
```suggestion
{!isChartCustomization && itemTypeField ===
'filter_select' && (
<StyledRowFormItem
expanded={expanded}
name={[
'filters',
filterId,
'controlValues',
'operatorType',
]}
initialValue={currentOperatorType}
label={
<>
<StyledLabel>{t('Match
type')}</StyledLabel>
<InfoTooltip
placement="top"
tooltip={t(
'Warning: ILIKE queries may be slow on
large datasets as they cannot use indexes effectively.',
)}
/>
</>
}
>
<Select
ariaLabel={t('Match type')}
options={[
{
value: SelectFilterOperatorType.Exact,
label: t('Exact match (IN)'),
},
{
value: SelectFilterOperatorType.Contains,
label: t('Contains text (ILIKE %x%)'),
},
{
value:
SelectFilterOperatorType.StartsWith,
label: t('Starts with (ILIKE x%)'),
},
{
value: SelectFilterOperatorType.EndsWith,
label: t('Ends with (ILIKE %x)'),
},
]}
onChange={value => {
onOperatorTypeChanged(
value as SelectFilterOperatorType,
);
}}
/>
</StyledRowFormItem>
)}
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open any dashboard and edit an existing native filter in the Filters
Config Modal
(implemented in `FiltersConfigForm` at
`superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx:274-810`).
2. In the "Filter Type" dropdown rendered at
`FiltersConfigForm.tsx:924-979`, select a
non-select filter type such as the numerical range filter (`filter_range`),
so
`itemTypeField` (computed at `FiltersConfigForm.tsx:366-370`) becomes
`'filter_range'`
instead of `'filter_select'`.
3. Switch to the "Filter Settings" panel for this filter; observe that the
"Match type"
form item is rendered unconditionally for all non-customization filters at
`FiltersConfigForm.tsx:1721-1770` under the condition
`!isChartCustomization` (no check on
`itemTypeField`), so the Match type dropdown appears for the range filter.
4. Change the "Match type" value and save the filter; this writes
`controlValues.operatorType` via `onOperatorTypeChanged` at
`FiltersConfigForm.tsx:635-645`, but at runtime `operatorType` is only
consumed by the
Select filter implementation (`SelectFilterPlugin.tsx:150-155, 223-229,
485-490` and
`filters/utils.ts:50-88`), and a repo-wide search for `operatorType` shows
no usage in
other filter plugins, so for non-Select filters the Match type setting is
silently ignored
despite being configurable in the UI.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx
**Line:** 1721:1771
**Comment:**
*Logic Error: The new "Match type" selector is rendered for all native
filter types (time, range, group-by, etc.), but `operatorType` is only honored
by the Select (Value) filter; this results in a configuration control that
appears to affect other filter types but is silently ignored, which is
misleading and can lead to incorrect assumptions about how those filters work.
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>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38470&comment_hash=3f8c35ac05920de768e5054ddf312aa564acccf32b2b656c18b12dd955fd5ce0&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38470&comment_hash=3f8c35ac05920de768e5054ddf312aa564acccf32b2b656c18b12dd955fd5ce0&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]
