codeant-ai-for-open-source[bot] commented on code in PR #37208:
URL: https://github.com/apache/superset/pull/37208#discussion_r2698188623
##########
superset-frontend/plugins/plugin-chart-echarts/test/Heatmap/transformProps.test.ts:
##########
@@ -291,4 +291,56 @@ describe('Heatmap transformProps', () => {
// Y-axis: numbers sorted numerically (1, 2, 10 NOT 1, 10, 2)
expect(yAxisData).toEqual([1, 2, 10]);
});
+
+ test('should include rank as 4th dimension when normalized is true', () => {
+ const dataWithRank = [
+ { day_of_week: 'Monday', hour: 9, count: 10, rank: 0.33 },
+ { day_of_week: 'Monday', hour: 14, count: 15, rank: 0.67 },
+ { day_of_week: 'Wednesday', hour: 11, count: 8, rank: 0.17 },
+ { day_of_week: 'Friday', hour: 16, count: 20, rank: 1.0 },
+ ];
+
+ const chartProps = createChartProps({ normalized: true }, dataWithRank);
+
Review Comment:
**Suggestion:** The test enabling `normalized: true` constructs `chartProps`
with `dataWithRank` but does not update `queriesData[0].colnames` (and
`coltypes`) to include the newly added `rank` column. As a result, the
transform logic may not detect the `rank` field and the produced series points
will have 3 dimensions instead of 4, causing the test to fail. Fix by
explicitly adding the `rank` column name (and matching coltypes length) to
`queriesData` on the created `chartProps` before calling `transformProps`.
[logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Heatmap transformProps unit test fails in CI.
- ⚠️ Blocks PR merge until tests are fixed.
- ❌ Normalized heatmap rendering validation not tested.
```
</details>
```suggestion
// Ensure queriesData metadata reflects the added `rank` column so
transformProps
// maps values into a 4th dimension for normalized charts.
(chartProps as any).queriesData[0].colnames = [
'day_of_week',
'hour',
'count',
'rank',
];
(chartProps as any).queriesData[0].coltypes = [0, 0, 0, 0];
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open test file at
superset-frontend/plugins/plugin-chart-echarts/test/Heatmap/transformProps.test.ts
and
locate the normalized test starting at line 295 (test name at line 295). The
test defines
dataWithRank at lines 296-301 and creates chartProps at line 303 using
createChartProps().
2. Inspect createChartProps implementation at the same file around lines
57-73; it sets
queriesData[0].colnames = ['day_of_week', 'hour', 'count'] by default (no
'rank' column).
3. When running the test (e.g., yarn test
superset-frontend/plugins/plugin-chart-echarts/test/Heatmap/transformProps.test.ts),
the
test calls transformProps at line 305. transformProps reads queriesData
metadata to map
input columns into series tuples.
4. Because queriesData.colnames was not updated to include 'rank' (step 2),
transformProps
will not map the 'rank' field into a fourth dimension; the produced series
points remain
[xIndex, yIndex, metricValue]. The assertion at line 329 expecting
visualMap.dimension ===
3 (4th element) fails, causing the test to fail in CI.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/plugins/plugin-chart-echarts/test/Heatmap/transformProps.test.ts
**Line:** 304:304
**Comment:**
*Logic Error: The test enabling `normalized: true` constructs
`chartProps` with `dataWithRank` but does not update `queriesData[0].colnames`
(and `coltypes`) to include the newly added `rank` column. As a result, the
transform logic may not detect the `rank` field and the produced series points
will have 3 dimensions instead of 4, causing the test to fail. Fix by
explicitly adding the `rank` column name (and matching coltypes length) to
`queriesData` on the created `chartProps` before calling `transformProps`.
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]
