bito-code-review[bot] commented on code in PR #37712:
URL: https://github.com/apache/superset/pull/37712#discussion_r2792689783
##########
superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/Bar/transformProps.test.ts:
##########
@@ -351,4 +351,55 @@ describe('Bar Chart X-axis Time Formatting', () => {
expect(chartProps.formData.xAxisTimeFormat).toBe('smart_date');
});
});
+
+ describe('X-Axis sorting with dimensions (series ordering)', () => {
+ it('reorders series correctly when xAxisSort is set to sum', () => {
+ const sortingFormData = {
+ ...baseFormData,
+ groupby: ['category'],
+ metrics: ['Sales', 'Marketing'],
+ xAxisSort: 'sum',
+ xAxisSortAsc: false,
+ };
+
+ const sortingData = [
+ {
+ data: [
+ {
+ Sales: 10,
+ Marketing: 5,
+ category: 'A',
+ __timestamp: 1609459200000,
+ },
+ {
+ Sales: 2,
+ Marketing: 12,
+ category: 'B',
+ __timestamp: 1609459200000,
+ },
+ ],
+ colnames: ['Sales', 'Marketing', 'category', '__timestamp'],
+ coltypes: ['BIGINT', 'BIGINT', 'STRING', 'TIMESTAMP'],
+ },
+ ];
+
+ const chartProps = new ChartProps({
+ width: 800,
+ height: 600,
+ formData: sortingFormData,
+ queriesData: sortingData,
+ theme: supersetTheme,
+ }) as unknown as EchartsTimeseriesChartProps;
+
+ const transformedProps = transformProps(chartProps);
+
+ const series = transformedProps.echartOptions.series as any[];
+
+ // The sum of Sales across points:
+ // A: 10, B: 2
+ expect(series[0].name).toContain('Marketing');
+ expect(series[1].name).toContain('Sales');
+ // Next lower sum metric
+ });
+ });
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Test expects unimplemented series sorting by
xAxisSort</b></div>
<div id="fix">
The test sets xAxisSort to 'sum' and expects series to be ordered by their
total sum descending (Marketing first, then Sales), but the current code sorts
series alphabetically by name. This test will fail with the existing
implementation. The intended behavior appears to be that xAxisSort controls
series ordering in multi-series charts, so transformProps needs to override
sortSeriesType with the mapped xAxisSort value when multiple series are present.
</div>
</div>
<small><i>Code Review Run #df9ed9</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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]
