codeant-ai-for-open-source[bot] commented on code in PR #37482:
URL: https://github.com/apache/superset/pull/37482#discussion_r3481253023
##########
superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx:
##########
@@ -380,16 +380,20 @@ const SliceHeaderControls = (
const updatedWhen = updatedDttm
? (extendedDayjs.utc(updatedDttm) as any).fromNow()
: '';
- const getCachedTitle = (itemCached: boolean) => {
+ const getCachedTitle = (itemCached: boolean, index: number) => {
if (itemCached) {
- return t('Cached %s', cachedWhen);
+ return t('Cached %s', cachedWhen[index]);
}
if (updatedWhen) {
return t('Fetched %s', updatedWhen);
}
return '';
};
- const refreshTooltipData = [...new Set(isCached.map(getCachedTitle) || '')];
+ const refreshTooltipData = [
+ ...new Set(
+ isCached.map((itemCached, index) => getCachedTitle(itemCached, index)),
+ ),
+ ];
Review Comment:
**Suggestion:** Deduplicating tooltip rows with `Set` drops per-query
entries whenever only some queries share the same cache/fetch text, which
breaks the query-to-tooltip alignment (for example, query 3 can disappear and
query numbering becomes incorrect). Only collapse to a single row when all
queries have the same message, otherwise keep the full per-query list. [logic
error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Dashboard slice refresh tooltip mislabels per-query cache states.
- ⚠️ Some queries' cache/fetch rows disappear from tooltip.
- ⚠️ Users may misinterpret which query was refreshed or cached.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. The dashboard chart grid component at
`superset-frontend/src/dashboard/components/gridComponents/Chart/Chart.tsx:244-259`
builds
an `isCached: boolean[]` array from `chart.queriesResponse` using `useMemo`,
and later at
`Chart.tsx:64-69` builds a parallel `cachedDttm: string[]` array by mapping
each
`q.cached_dttm` value from `queriesResponse`.
2. The slice header component at
`superset-frontend/src/dashboard/components/SliceHeader/index.tsx:112-120`
passes these
arrays (`isCached`, `cachedDttm`, plus `updatedDttm`) through to
`SliceHeaderControls`, as
seen in the JSX `<SliceHeaderControls ... isCached={isCached}
cachedDttm={cachedDttm}
updatedDttm={updatedDttm} />`.
3. Inside `SliceHeaderControls` at
`superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx:44-57`
(diff
lines 383-396), each query index produces a label via
`getCachedTitle(itemCached, index)`,
which returns `'Cached %s'` or `'Fetched %s'` based on `isCached[index]` and
`cachedWhen[index]` / `updatedWhen`. The code then computes
`refreshTooltipData = [...new
Set(isCached.map((itemCached, index) => getCachedTitle(itemCached,
index)))]` (diff lines
392-396), deduplicating these per-query labels before building
`refreshTooltip` with
`refreshTooltipData.map((item, index) => t('Query %s: %s', index + 1,
item))`.
4. When the backend returns a `queriesResponse` with at least three entries
such that
`getCachedTitle` yields something like `['Cached 5 minutes ago', 'Cached 5
minutes ago',
'Fetched a minute ago']` (for example, two cached queries sharing the same
`cached_dttm`
and a third freshly fetched query), the `Set` in `refreshTooltipData`
collapses this to
`['Cached 5 minutes ago', 'Fetched a minute ago']`. As a result, the tooltip
rendered from
`refreshTooltip` shows only two rows: `Query 1: Cached 5 minutes ago` and
`Query 2:
Fetched a minute ago`, completely omitting the second query's row and
assigning the
`'Query 2'` label to the third query instead of the actual second query.
This misalignment
is directly caused by the deduplication in `refreshTooltipData` (lines
392-396) combined
with per-query numbering in `refreshTooltip`.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=2c43135865774bb5bc65ed87ad069076&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=2c43135865774bb5bc65ed87ad069076&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/src/dashboard/components/SliceHeaderControls/index.tsx
**Line:** 392:396
**Comment:**
*Logic Error: Deduplicating tooltip rows with `Set` drops per-query
entries whenever only some queries share the same cache/fetch text, which
breaks the query-to-tooltip alignment (for example, query 3 can disappear and
query numbering becomes incorrect). Only collapse to a single row when all
queries have the same message, otherwise keep the full per-query list.
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%2F37482&comment_hash=8c4a3c49a41c2f8ff00af38a86acf037225882ddbe20dea6bd5073b37d440ec8&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37482&comment_hash=8c4a3c49a41c2f8ff00af38a86acf037225882ddbe20dea6bd5073b37d440ec8&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]
