codeant-ai-for-open-source[bot] commented on code in PR #42599:
URL: https://github.com/apache/superset/pull/42599#discussion_r3679137798
##########
superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/Bar/transformProps.test.ts:
##########
@@ -994,4 +994,56 @@ describe('Bar Chart X-axis Time Formatting', () => {
expect(grid.bottom).not.toBe(expandedPadding.bottom);
});
});
+
+ describe('Regression test for Issue #42560', () => {
+ test('custom X Axis Title is preserved verbatim, not overwritten by the
axis number/currency format ("unit")', () => {
+ const formData = {
+ ...baseFormData,
+ orientation: 'vertical',
+ xAxisTitle: 'My X Axis',
+ xAxisNumberFormat: 'SMART_NUMBER',
+ yAxisFormat: '$,.2f',
+ };
+
+ const chartProps = new ChartProps({
+ ...baseChartPropsConfig,
+ formData,
Review Comment:
**Suggestion:** The fixture uses temporal `__timestamp` data inherited from
`baseChartPropsConfig`, so `transformProps` selects the time formatter and
ignores `xAxisNumberFormat`. The test therefore only verifies direct title
assignment and cannot detect an overwrite caused by numeric or currency
formatting. Use numeric X-axis query data and assert the title alongside the
numeric formatter path. [incomplete implementation]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Numeric X-axis title overwrite regressions remain undetected.
- ⚠️ `SMART_NUMBER` is unused on the temporal formatter path.
- ⚠️ Bar chart formatting coverage does not match the test claim.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=a0a71a3d7e2f4bb1b7066bb4c2870704&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=a0a71a3d7e2f4bb1b7066bb4c2870704&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/plugin-chart-echarts/test/Timeseries/Bar/transformProps.test.ts
**Line:** 1003:1010
**Comment:**
*Incomplete Implementation: The fixture uses temporal `__timestamp`
data inherited from `baseChartPropsConfig`, so `transformProps` selects the
time formatter and ignores `xAxisNumberFormat`. The test therefore only
verifies direct title assignment and cannot detect an overwrite caused by
numeric or currency formatting. Use numeric X-axis query data and assert the
title alongside the numeric formatter path.
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%2F42599&comment_hash=0976658bd2bedc3281566b88a332dc290dcb4b322166785ee746a110d35c3dff&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42599&comment_hash=0976658bd2bedc3281566b88a332dc290dcb4b322166785ee746a110d35c3dff&reaction=dislike'>👎</a>
##########
superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/Bar/transformProps.test.ts:
##########
@@ -994,4 +994,56 @@ describe('Bar Chart X-axis Time Formatting', () => {
expect(grid.bottom).not.toBe(expandedPadding.bottom);
});
});
+
+ describe('Regression test for Issue #42560', () => {
+ test('custom X Axis Title is preserved verbatim, not overwritten by the
axis number/currency format ("unit")', () => {
+ const formData = {
+ ...baseFormData,
+ orientation: 'vertical',
+ xAxisTitle: 'My X Axis',
+ xAxisNumberFormat: 'SMART_NUMBER',
+ yAxisFormat: '$,.2f',
+ };
+
+ const chartProps = new ChartProps({
+ ...baseChartPropsConfig,
+ formData,
+ });
+
+ const transformedProps = transformProps(
+ chartProps as EchartsTimeseriesChartProps,
+ );
+ const xAxis = transformedProps.echartOptions.xAxis as any;
+
+ expect(xAxis.name).toBe('My X Axis');
+ });
+
+ test('X Axis Title control maps onto the rendered category (left) axis in
horizontal orientation, not the bottom axis', () => {
+ // Documents the axis swap for horizontal bar charts: `xAxisTitle` ends
+ // up on `echartOptions.yAxis.name` (the vertical category axis) and
+ // `yAxisTitle` ends up on `echartOptions.xAxis.name` (the horizontal
+ // value axis). This is existing, intentional swap behavior, not the
+ // "unit" overwrite described in the issue.
+ const formData = {
+ ...baseFormData,
+ orientation: 'horizontal',
+ xAxisTitle: 'My X Axis',
+ yAxisTitle: 'My Y Axis',
+ };
+
+ const chartProps = new ChartProps({
+ ...baseChartPropsConfig,
+ formData,
Review Comment:
**Suggestion:** This test claims to validate mapping to the rendered
category axis, but it also inherits temporal timestamp data from
`baseChartPropsConfig`, so the rendered left axis is a temporal axis rather
than a category axis. The assertion can pass without validating the
category-axis behavior described by the test; provide category-based query data
for this horizontal-bar case. [incomplete implementation]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Category-axis title mapping remains untested.
- ⚠️ Horizontal Bar coverage exercises temporal data instead.
- ⚠️ Axis-swap assertions may miss category-specific behavior.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=912bd48444ff4c2f8e9afde2753e0e06&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=912bd48444ff4c2f8e9afde2753e0e06&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/plugin-chart-echarts/test/Timeseries/Bar/transformProps.test.ts
**Line:** 1030:1036
**Comment:**
*Incomplete Implementation: This test claims to validate mapping to the
rendered category axis, but it also inherits temporal timestamp data from
`baseChartPropsConfig`, so the rendered left axis is a temporal axis rather
than a category axis. The assertion can pass without validating the
category-axis behavior described by the test; provide category-based query data
for this horizontal-bar case.
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%2F42599&comment_hash=d0eda96009509d70772f80532f5f16f31c0b1c15384bcd67e88efe42ff41b260&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42599&comment_hash=d0eda96009509d70772f80532f5f16f31c0b1c15384bcd67e88efe42ff41b260&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]
