You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org> on 2026/07/05 21:45:15 UTC
Re: [PR] fix(mixed chart): respect truncate_metric setting for series names (#38190) [superset]
codeant-ai-for-open-source[bot] commented on code in PR #38451:
URL: https://github.com/apache/superset/pull/38451#discussion_r3525680797
##########
superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/transformProps.test.ts:
##########
@@ -433,3 +472,98 @@ test('should add a formula annotation when X-axis column has dataset-level label
expect(Array.isArray(formulaSeries?.data)).toBe(true);
expect((formulaSeries!.data as unknown[]).length).toBeGreaterThan(0);
});
+
+describe('truncateMetric behavior', () => {
+ const getChartProps = (overrides = {}) =>
+ createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: queriesData,
+ formData: {
+ ...formData,
+ ...overrides,
+ },
+ queriesData,
+ datasource: {
+ verboseMap: { sum__num: 'sum__num' },
+ columnFormats: {},
+ currencyFormats: {},
+ },
+ });
+
+ test('should show only group‑by values when truncateMetric=true and groupby exists (both queries truncated)', () => {
+ const chartProps = getChartProps({
+ truncateMetric: true,
+ truncateMetricB: true, // also truncate Query B to avoid full names
+ groupby: ['gender'],
+ groupbyB: ['gender'],
+ showQueryIdentifiers: false,
+ });
+ const transformed = transformProps(chartProps);
+ const series = transformed.echartOptions.series as any[];
Review Comment:
**Suggestion:** Replace the `any[]` assertion with a specific series type (for example `SeriesOption[]`) so the test does not introduce `any`. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
The new test code introduces `any[]`, which directly violates the no-`any` TypeScript rule. A specific series type should be used instead.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=0e0c9971dcb24e43bf9966124dfd779f&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=0e0c9971dcb24e43bf9966124dfd779f&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/MixedTimeseries/transformProps.test.ts
**Line:** 505:505
**Comment:**
*Custom Rule: Replace the `any[]` assertion with a specific series type (for example `SeriesOption[]`) so the test does not introduce `any`.
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%2F38451&comment_hash=71a0125c0c2097c3da1b995d101db80e64da493b242385b74477f1cb3287981e&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38451&comment_hash=71a0125c0c2097c3da1b995d101db80e64da493b242385b74477f1cb3287981e&reaction=dislike'>👎</a>
##########
superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/transformProps.test.ts:
##########
@@ -433,3 +472,98 @@ test('should add a formula annotation when X-axis column has dataset-level label
expect(Array.isArray(formulaSeries?.data)).toBe(true);
expect((formulaSeries!.data as unknown[]).length).toBeGreaterThan(0);
});
+
+describe('truncateMetric behavior', () => {
+ const getChartProps = (overrides = {}) =>
+ createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: queriesData,
+ formData: {
+ ...formData,
+ ...overrides,
+ },
+ queriesData,
+ datasource: {
+ verboseMap: { sum__num: 'sum__num' },
+ columnFormats: {},
+ currencyFormats: {},
+ },
+ });
+
+ test('should show only group‑by values when truncateMetric=true and groupby exists (both queries truncated)', () => {
+ const chartProps = getChartProps({
+ truncateMetric: true,
+ truncateMetricB: true, // also truncate Query B to avoid full names
+ groupby: ['gender'],
+ groupbyB: ['gender'],
+ showQueryIdentifiers: false,
+ });
+ const transformed = transformProps(chartProps);
+ const series = transformed.echartOptions.series as any[];
+
+ // No series should contain the metric name
+ expect(series.map(s => s.name)).not.toContain('sum__num, boy');
+ expect(series.map(s => s.name)).not.toContain('sum__num, girl');
+ // All series should be truncated to group‑by values
+ expect(series.map(s => s.name)).toContain('boy');
+ expect(series.map(s => s.name)).toContain('girl');
+ });
+
+ test('should keep full name when truncateMetric=true but no groupby', () => {
+ const chartProps = getChartProps({
+ truncateMetric: true,
+ groupby: [], // no groupby for Query A
+ groupbyB: ['gender'], // Query B still has groupby
+ showQueryIdentifiers: false,
+ });
+ const transformed = transformProps(chartProps);
+ const series = transformed.echartOptions.series as any[];
+ const seriesNames = series.map(s => s.name);
+
+ // Query A (no groupby) should show metric name only
+ expect(seriesNames).toContain('sum__num');
+ // Query B (with groupby) should show full names
+ expect(seriesNames).toContain('sum__num, boy');
+ expect(seriesNames).toContain('sum__num, girl');
+ // No truncated names from Query A because no groupby
+ expect(seriesNames).not.toContain('boy');
+ expect(seriesNames).not.toContain('girl');
+ });
+
+ test('should show full name when truncateMetric=false', () => {
+ const chartProps = getChartProps({
+ truncateMetric: false,
+ groupby: ['gender'],
+ groupbyB: ['gender'],
+ showQueryIdentifiers: false,
+ });
+ const transformed = transformProps(chartProps);
+ const series = transformed.echartOptions.series as any[];
+
+ expect(series.map(s => s.name)).toContain('sum__num, boy');
+ expect(series.map(s => s.name)).toContain('sum__num, girl');
+ });
+
+ test('truncateMetricB should work independently for Query B', () => {
+ const chartProps = getChartProps({
+ truncateMetricB: true, // only Query B truncated
+ groupby: ['gender'],
+ groupbyB: ['gender'],
+ showQueryIdentifiers: false,
+ });
+ const transformed = transformProps(chartProps);
+ const series = transformed.echartOptions.series as any[];
Review Comment:
**Suggestion:** Change this `any[]` cast to a concrete series type so the query-B truncation test remains type-safe. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This newly added line uses `any[]`, which is disallowed by the no-`any` TypeScript rule and should be replaced with a concrete series type.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=6d68c7b35a2047a7834cb77c11c13844&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=6d68c7b35a2047a7834cb77c11c13844&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/MixedTimeseries/transformProps.test.ts
**Line:** 558:558
**Comment:**
*Custom Rule: Change this `any[]` cast to a concrete series type so the query-B truncation test remains type-safe.
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%2F38451&comment_hash=c5a639ac519b6b3c2cdb3f173fad64b17dde591d3bd19a8264b3142210885fd6&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38451&comment_hash=c5a639ac519b6b3c2cdb3f173fad64b17dde591d3bd19a8264b3142210885fd6&reaction=dislike'>👎</a>
##########
superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/transformProps.test.ts:
##########
@@ -433,3 +472,98 @@ test('should add a formula annotation when X-axis column has dataset-level label
expect(Array.isArray(formulaSeries?.data)).toBe(true);
expect((formulaSeries!.data as unknown[]).length).toBeGreaterThan(0);
});
+
+describe('truncateMetric behavior', () => {
+ const getChartProps = (overrides = {}) =>
+ createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: queriesData,
+ formData: {
+ ...formData,
+ ...overrides,
+ },
+ queriesData,
+ datasource: {
+ verboseMap: { sum__num: 'sum__num' },
+ columnFormats: {},
+ currencyFormats: {},
+ },
+ });
+
+ test('should show only group‑by values when truncateMetric=true and groupby exists (both queries truncated)', () => {
+ const chartProps = getChartProps({
+ truncateMetric: true,
+ truncateMetricB: true, // also truncate Query B to avoid full names
+ groupby: ['gender'],
+ groupbyB: ['gender'],
+ showQueryIdentifiers: false,
+ });
+ const transformed = transformProps(chartProps);
+ const series = transformed.echartOptions.series as any[];
+
+ // No series should contain the metric name
+ expect(series.map(s => s.name)).not.toContain('sum__num, boy');
+ expect(series.map(s => s.name)).not.toContain('sum__num, girl');
+ // All series should be truncated to group‑by values
+ expect(series.map(s => s.name)).toContain('boy');
+ expect(series.map(s => s.name)).toContain('girl');
+ });
+
+ test('should keep full name when truncateMetric=true but no groupby', () => {
+ const chartProps = getChartProps({
+ truncateMetric: true,
+ groupby: [], // no groupby for Query A
+ groupbyB: ['gender'], // Query B still has groupby
+ showQueryIdentifiers: false,
+ });
+ const transformed = transformProps(chartProps);
+ const series = transformed.echartOptions.series as any[];
Review Comment:
**Suggestion:** Replace the `any[]` assertion with a strongly typed series array to comply with the no-`any` rule. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
This added test casts the series array to `any[]`, which is exactly the prohibited pattern under the no-`any` rule.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=c56520d65c70470fa56c4ecdb8752f63&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=c56520d65c70470fa56c4ecdb8752f63&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/MixedTimeseries/transformProps.test.ts
**Line:** 523:523
**Comment:**
*Custom Rule: Replace the `any[]` assertion with a strongly typed series array to comply with the no-`any` rule.
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%2F38451&comment_hash=b9fbe8d3f55cc8e411e4c8c35e4d86aa0fc6b0ed54c9196e106dc5883731e108&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38451&comment_hash=b9fbe8d3f55cc8e411e4c8c35e4d86aa0fc6b0ed54c9196e106dc5883731e108&reaction=dislike'>👎</a>
##########
superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/transformProps.test.ts:
##########
@@ -433,3 +472,98 @@ test('should add a formula annotation when X-axis column has dataset-level label
expect(Array.isArray(formulaSeries?.data)).toBe(true);
expect((formulaSeries!.data as unknown[]).length).toBeGreaterThan(0);
});
+
+describe('truncateMetric behavior', () => {
+ const getChartProps = (overrides = {}) =>
+ createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: queriesData,
+ formData: {
+ ...formData,
+ ...overrides,
+ },
+ queriesData,
+ datasource: {
+ verboseMap: { sum__num: 'sum__num' },
+ columnFormats: {},
+ currencyFormats: {},
+ },
+ });
+
+ test('should show only group‑by values when truncateMetric=true and groupby exists (both queries truncated)', () => {
+ const chartProps = getChartProps({
+ truncateMetric: true,
+ truncateMetricB: true, // also truncate Query B to avoid full names
+ groupby: ['gender'],
+ groupbyB: ['gender'],
+ showQueryIdentifiers: false,
+ });
+ const transformed = transformProps(chartProps);
+ const series = transformed.echartOptions.series as any[];
+
+ // No series should contain the metric name
+ expect(series.map(s => s.name)).not.toContain('sum__num, boy');
+ expect(series.map(s => s.name)).not.toContain('sum__num, girl');
+ // All series should be truncated to group‑by values
+ expect(series.map(s => s.name)).toContain('boy');
+ expect(series.map(s => s.name)).toContain('girl');
+ });
+
+ test('should keep full name when truncateMetric=true but no groupby', () => {
+ const chartProps = getChartProps({
+ truncateMetric: true,
+ groupby: [], // no groupby for Query A
+ groupbyB: ['gender'], // Query B still has groupby
+ showQueryIdentifiers: false,
+ });
+ const transformed = transformProps(chartProps);
+ const series = transformed.echartOptions.series as any[];
+ const seriesNames = series.map(s => s.name);
+
+ // Query A (no groupby) should show metric name only
+ expect(seriesNames).toContain('sum__num');
+ // Query B (with groupby) should show full names
+ expect(seriesNames).toContain('sum__num, boy');
+ expect(seriesNames).toContain('sum__num, girl');
+ // No truncated names from Query A because no groupby
+ expect(seriesNames).not.toContain('boy');
+ expect(seriesNames).not.toContain('girl');
+ });
+
+ test('should show full name when truncateMetric=false', () => {
+ const chartProps = getChartProps({
+ truncateMetric: false,
+ groupby: ['gender'],
+ groupbyB: ['gender'],
+ showQueryIdentifiers: false,
+ });
+ const transformed = transformProps(chartProps);
+ const series = transformed.echartOptions.series as any[];
Review Comment:
**Suggestion:** Use a specific series type for this cast rather than `any[]` to keep new TypeScript code strictly typed. [custom_rule]
**Severity Level:** Minor ⚠️
<details>
<summary><b>Why it matters? 🤔 </b></summary>
The test introduces a new `any[]` assertion, so the suggestion correctly identifies a real violation of the no-`any` rule.
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=47575b598ff74e6b9f42f6784a76b5ac&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=47575b598ff74e6b9f42f6784a76b5ac&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/MixedTimeseries/transformProps.test.ts
**Line:** 544:544
**Comment:**
*Custom Rule: Use a specific series type for this cast rather than `any[]` to keep new TypeScript code strictly typed.
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%2F38451&comment_hash=878bb5f1bca0b4afdbf9aa8e220464eb9a81c6993a290064a725135345df5e1a&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38451&comment_hash=878bb5f1bca0b4afdbf9aa8e220464eb9a81c6993a290064a725135345df5e1a&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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org