codeant-ai-for-open-source[bot] commented on code in PR #41341:
URL: https://github.com/apache/superset/pull/41341#discussion_r3460594801
##########
superset-frontend/plugins/preset-chart-deckgl/src/utilities/tooltipUtils.tsx:
##########
@@ -170,6 +176,36 @@ function buildFieldBasedTooltipItems(
}
});
+ // In the geojson polygcon chart, where users can only pick columns as
tooltip_contents,
+ // we still include the configured metric, so hover also shows the
aggregation value.
+ const hasSelectedMetricItem = formData.tooltip_contents.some(
+ (item: any) =>
+ item && typeof item === 'object' && item.item_type === 'metric',
+ );
+ if (!hasSelectedMetricItem && formData.metric) {
+ let metricFieldName = '';
+ if (typeof formData.metric === 'string') {
+ metricFieldName = formData.metric;
+ } else {
+ const metric = formData.metric as { label?: string; value?: string };
+ metricFieldName = metric.label || metric.value || '';
+ }
+
+ if (metricFieldName) {
+ const { value } = extractValue(o, metricFieldName, false);
+ const metricValue = value === '' ? o.object?.metric || '' : value;
Review Comment:
**Suggestion:** The fallback uses `|| ''`, which treats `0` as missing and
drops valid zero-valued metrics from the appended configured-metric tooltip
row. Use a nullish check instead so `0` is preserved and rendered. [falsy zero
check]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Zero-valued metrics missing from deck.gl polygon tooltips.
- ⚠️ Misleads users interpreting polygons with zero aggregation.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Create a DeckGL Polygon chart (or other deck.gl chart using
`createTooltipContent`)
where the configured aggregation metric can legitimately be zero for some
polygons;
tooltip content is wired via `createTooltipContent` in
`plugins/preset-chart-deckgl/src/layers/Polygon/Polygon.tsx:98-100` and
`utilities/tooltipUtils.tsx:392-399`.
2. Ensure "Tooltip contents" contains only column entries (strings or `{
item_type:
'column' }` objects) and no metric entries, so `hasSelectedMetricItem` at
`tooltipUtils.tsx:181-184` evaluates to false and the appended metric logic
runs when
hovering.
3. Hover a polygon whose aggregated metric value is exactly `0`;
`metricFieldName` is
derived from `formData.metric` at `tooltipUtils.tsx:185-192`, and
`extractValue(o,
metricFieldName, false)` at `tooltipUtils.tsx:194-195` returns `value ===
''` when the
metric field is not present in properties/extraProps/metrics/data.
4. At `tooltipUtils.tsx:196`, `const metricValue = value === '' ?
o.object?.metric || '' :
value;` is executed; because `o.object.metric` is `0`, the `|| ''` fallback
treats `0` as
falsy and yields `''`, so `metricValue` becomes an empty string and the
subsequent guard
`if (metricValue !== '')` at `tooltipUtils.tsx:197` prevents pushing a
`TooltipRow`,
causing the tooltip to omit the configured metric for polygons with
zero-valued metrics.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=41b96ff562a74e78984e0f062eb56b80&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=41b96ff562a74e78984e0f062eb56b80&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/plugins/preset-chart-deckgl/src/utilities/tooltipUtils.tsx
**Line:** 196:196
**Comment:**
*Falsy Zero Check: The fallback uses `|| ''`, which treats `0` as
missing and drops valid zero-valued metrics from the appended configured-metric
tooltip row. Use a nullish check instead so `0` is preserved and rendered.
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%2F41341&comment_hash=26a2a1346e91671076ef60a6f0923c7ac99c6e6db8604be810030ffa2dd3e569&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41341&comment_hash=26a2a1346e91671076ef60a6f0923c7ac99c6e6db8604be810030ffa2dd3e569&reaction=dislike'>👎</a>
##########
superset-frontend/plugins/preset-chart-deckgl/src/utilities/tooltipUtils.tsx:
##########
@@ -170,6 +176,36 @@ function buildFieldBasedTooltipItems(
}
});
+ // In the geojson polygcon chart, where users can only pick columns as
tooltip_contents,
+ // we still include the configured metric, so hover also shows the
aggregation value.
+ const hasSelectedMetricItem = formData.tooltip_contents.some(
+ (item: any) =>
+ item && typeof item === 'object' && item.item_type === 'metric',
+ );
Review Comment:
**Suggestion:** The metric-presence check only treats object-shaped tooltip
items as metrics, so legacy/free-form metric entries stored as strings are
ignored and the configured metric gets appended again, producing duplicated
metric rows in the tooltip. Update the detection to also recognize string
entries that match the configured metric field name. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Deck.gl polygon tooltips show duplicate metric rows.
- ⚠️ Other DeckGL charts may duplicate configured metric tooltip.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open a DeckGL Polygon chart in Explore; the front-end uses `getLayer` and
`createTooltipContent` from
`plugins/preset-chart-deckgl/src/layers/Polygon/Polygon.tsx:98-100` to build
tooltip
content for hover events.
2. Configure a saved metric (e.g. `metric_a`) as the chart metric; this
becomes
`fd.metric` in `PolygonFormData` and is later read by
`buildFieldBasedTooltipItems` in
`utilities/tooltipUtils.tsx:126-177`.
3. In the "Tooltip contents" control (configured with
`DndColumnMetricSelect` in
`utilities/Shared_DeckGL.tsx:37-69`), drag the same saved metric `metric_a`
into the
control; `DndColumnMetricSelect.onDrop` in
`src/explore/components/controls/DndColumnSelectControl/DndColumnMetricSelect.tsx:210-229`
pushes the metric's `metric_name` as a string into
`formData.tooltip_contents`.
4. Hover a polygon: `createTooltipContent` in `tooltipUtils.tsx:392-399`
calls
`buildFieldBasedTooltipItems`, which renders one `TooltipRow` for the string
`metric_a`,
but `hasSelectedMetricItem` at `tooltipUtils.tsx:181-184` only treats object
entries with
`item_type === 'metric'` as metrics, so it ignores the string entry; since
`formData.metric` is set and `hasSelectedMetricItem` is false, the block at
`tooltipUtils.tsx:185-207` appends another `TooltipRow` for the configured
metric,
producing duplicated metric rows in the tooltip.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=1cb54604ce554d548cc04e78dff123e0&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=1cb54604ce554d548cc04e78dff123e0&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/plugins/preset-chart-deckgl/src/utilities/tooltipUtils.tsx
**Line:** 181:184
**Comment:**
*Logic Error: The metric-presence check only treats object-shaped
tooltip items as metrics, so legacy/free-form metric entries stored as strings
are ignored and the configured metric gets appended again, producing duplicated
metric rows in the tooltip. Update the detection to also recognize string
entries that match the configured metric field 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.
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%2F41341&comment_hash=8938315e90f7ce9d92ab0890a55a581d7924cf9d1a38efdd8574486cdce19e53&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41341&comment_hash=8938315e90f7ce9d92ab0890a55a581d7924cf9d1a38efdd8574486cdce19e53&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]
