codeant-ai-for-open-source[bot] commented on code in PR #39005:
URL: https://github.com/apache/superset/pull/39005#discussion_r3021753916
##########
superset-frontend/src/components/AlteredSliceTag/utils/index.ts:
##########
@@ -52,7 +52,9 @@ export const formatValueHandler = (
v.comparator && v.comparator.constructor === Array
? `[${v.comparator.join(', ')}]`
: v.comparator;
- return filterVal ? `${v.subject} ${v.operator} ${filterVal}` :
`${v.subject} ${v.operator}`;
+ return filterVal
Review Comment:
**Suggestion:** The truthy check on `filterVal` drops valid empty-string
comparators, so filters like `= ''` are rendered as if they had no comparator
at all. Check explicitly for `undefined` instead of truthiness so empty strings
are preserved. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Chart changes modal misrepresents filters comparing to empty strings.
- ⚠️ Explore header diff summary less clear for seeded new filters.
- ⚠️ Users may misread saved vs current filter conditions in Explore.
```
</details>
```suggestion
return filterVal !== undefined
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open a saved chart in Explore so that `ExploreChartHeader` renders with an
`AlteredSliceTag` using `formDiffs` computed by `getChartFormDiffs`
(`superset-frontend/src/explore/components/ExploreChartHeader/index.tsx:239-241`
for
`formDiffs` and `:303-308` for the `<AlteredSliceTag ... diffs={formDiffs}
/>` usage).
2. In the Explore controls panel, add a new simple adhoc filter on a column
via the
regular filter control: selecting a column in `AdhocFilterControl` creates an
`AdhocFilter` with `comparator: ''` by default
(`superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterControl/index.tsx:22-29`,
specifically line `366: comparator: ''`, as shown by Grep), or via
drag-and-drop using
`DndFilterSelect` which likewise seeds `comparator: ''`
(`superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.tsx:309-315`
with `312: comparator: ''`).
3. Without entering any value into the filter comparator input (so the
comparator remains
the empty string), observe that the Explore form now differs from the
original saved form,
causing `getChartFormDiffs` to emit a diff entry for the `adhoc_filters` key
(`superset-frontend/src/utils/getChartFormDiffs/index.ts:38-63`), which is
then converted
into rows via `getRowsFromDiffs` calling `formatValueHandler`
(`superset-frontend/src/components/AlteredSliceTag/utils/index.ts:104-108`).
4. Click the "Altered" label in the chart header to open the "Chart changes"
modal
(`superset-frontend/src/components/AlteredSliceTag/index.tsx:33-47`); in
`formatValueHandler` for `AdhocFilterControl`
(`superset-frontend/src/components/AlteredSliceTag/utils/index.ts:12-25`),
the filter's
`comparator: ''` is first assigned to `filterVal` (lines 17-20), then the
ternary at lines
55-57 (`return filterVal ? \`\${v.subject} \${v.operator} \${filterVal}\` :
\`\${v.subject} \${v.operator}\`;`) treats the empty string as falsy and
drops it, so the
modal renders the condition as `subject operator` with no visible comparator,
misrepresenting a real `= ''` filter as if it had no comparator.
```
</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/AlteredSliceTag/utils/index.ts
**Line:** 55:55
**Comment:**
*Logic Error: The truthy check on `filterVal` drops valid empty-string
comparators, so filters like `= ''` are rendered as if they had no comparator
at all. Check explicitly for `undefined` instead of truthiness so empty strings
are preserved.
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%2F39005&comment_hash=981e79e849501f75c3a9d8d39f15807ed74c01d25806fe4226d7468fdb4e1b20&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39005&comment_hash=981e79e849501f75c3a9d8d39f15807ed74c01d25806fe4226d7468fdb4e1b20&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]
