You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "rusackas (via GitHub)" <gi...@apache.org> on 2026/07/20 06:41:28 UTC
[PR] feat(bullet): rebuild the Bullet Chart on ECharts [superset]
rusackas opened a new pull request, #42225:
URL: https://github.com/apache/superset/pull/42225
### SUMMARY
Stacked on #41714. Rebuilds the Bullet Chart as an ECharts composition so the last-but-one nvd3 chart comes off that library, leaving Time-series Period Pivot as the only nvd3 renderer.
The composition mirrors d3-bullet semantics: nested qualitative range bands (markArea, largest threshold painted first so smaller bands nest on top, shaded with the theme fill tokens), the measure as a thin primary-color bar, triangle point markers as a scatter series, and marker lines via markLine with their labels. Control names and their comma-separated value semantics (`ranges`, `range_labels`, `markers`, `marker_labels`, `marker_lines`, `marker_line_labels`) match the legacy chart exactly and the `viz_type` stays `bullet`, so saved charts keep working with **no DB migration**. A shared `y_axis_format` control is added for number formatting (the legacy chart had no format control).
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Screenshots to follow from the ephemeral env.
### TESTING INSTRUCTIONS
- `npx jest plugins/plugin-chart-echarts/test/Bullet/` β buildQuery + transformProps coverage (bands ordering, markers, marker lines, legacy 110% default band, empty-result safety).
- Manual: open a saved Bullet chart or build one against `birth_names` (e.g. `sum__num` with ranges `100000,200000,300000`).
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [x] Changes UI
- [ ] Includes DB Migration
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
π€ Generated with [Claude Code](https://claude.com/claude-code)
--
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
Re: [PR] feat(bullet): rebuild the Bullet Chart on ECharts [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #42225:
URL: https://github.com/apache/superset/pull/42225#discussion_r3615748033
##########
superset-frontend/plugins/plugin-chart-echarts/src/Bullet/buildQuery.ts:
##########
@@ -0,0 +1,34 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { buildQueryContext, QueryFormData } from '@superset-ui/core';
+
+/**
+ * A single ungrouped aggregate of the selected metric, mirroring the query
+ * the legacy nvd3 bullet chart issued.
+ */
+export default function buildQuery(formData: QueryFormData) {
+ const { metric } = formData;
+ return buildQueryContext(formData, baseQueryObject => [
+ {
+ ...baseQueryObject,
+ columns: [],
+ metrics: metric ? [metric] : [],
Review Comment:
The shared `metric` control carries `validateNonEmpty`, so explore won't fire the query until a metric is picked. This also matches the legacy chart's buildQuery exactly, so keeping parity here.
##########
superset-frontend/plugins/plugin-chart-echarts/src/Bullet/transformProps.ts:
##########
@@ -0,0 +1,196 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import {
+ getMetricLabel,
+ getNumberFormatter,
+ JsonObject,
+ sanitizeHtml,
+} from '@superset-ui/core';
+import type { EChartsCoreOption } from 'echarts/core';
+import { Refs } from '../types';
+import { EchartsBulletChartProps, BulletChartTransformedProps } from './types';
+import { tokenizeToNumericArray, tokenizeToStringArray } from './utils';
+
+const MEASURE_BAR_FRACTION = 0.28;
+
+export default function transformProps(
+ chartProps: EchartsBulletChartProps,
+): BulletChartTransformedProps {
+ const { width, height, formData, queriesData, theme, hooks } = chartProps;
+ const {
+ metric,
+ ranges: rawRanges,
+ rangeLabels: rawRangeLabels,
+ markers: rawMarkers,
+ markerLabels: rawMarkerLabels,
+ markerLines: rawMarkerLines,
+ markerLineLabels: rawMarkerLineLabels,
+ yAxisFormat,
+ } = formData;
+ const refs: Refs = {};
+ const { onContextMenu, setDataMask = () => {} } = hooks;
+
+ const metricLabel = getMetricLabel(metric ?? '');
+ const records = (queriesData[0]?.data ?? []) as JsonObject[];
+ const measure = Number(records[0]?.[metricLabel] ?? 0);
+
+ // Match the legacy default: a single band up to 110% of the measure.
+ const ranges = tokenizeToNumericArray(rawRanges) ?? [0, measure * 1.1];
+ const rangeLabels = tokenizeToStringArray(rawRangeLabels) ?? [];
+ const markers = tokenizeToNumericArray(rawMarkers) ?? [];
+ const markerLabels = tokenizeToStringArray(rawMarkerLabels) ?? [];
+ const markerLines = tokenizeToNumericArray(rawMarkerLines) ?? [];
+ const markerLineLabels = tokenizeToStringArray(rawMarkerLineLabels) ?? [];
+
+ const formatter = getNumberFormatter(yAxisFormat);
+
+ const axisMax = Math.max(measure, ...ranges, ...markers, ...markerLines);
+ const axisMin = Math.min(0, measure, ...ranges, ...markers, ...markerLines);
Review Comment:
Good catch, fixed in 6518be3 with a test pinning the non-collapsed domain.
--
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
Re: [PR] feat(bullet): rebuild the Bullet Chart on ECharts [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #42225:
URL: https://github.com/apache/superset/pull/42225#issuecomment-5024572961
πͺ [Showtime](https://github.com/mistercrunch/superset-showtime) is building environment on [GHA](https://github.com/apache/superset/actions/runs/29758593182) for [6518be3](https://github.com/apache/superset/commit/6518be3)
--
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
Re: [PR] feat(bullet): rebuild the Bullet Chart on ECharts [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #42225:
URL: https://github.com/apache/superset/pull/42225#discussion_r3612425517
##########
superset-frontend/plugins/plugin-chart-echarts/test/Bullet/transformProps.test.ts:
##########
@@ -0,0 +1,99 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { ChartProps, SqlaFormData, VizType } from '@superset-ui/core';
+import { supersetTheme } from '@apache-superset/core/theme';
+import transformProps from '../../src/Bullet/transformProps';
+import { EchartsBulletChartProps } from '../../src/Bullet/types';
+
+const formData: SqlaFormData = {
+ datasource: '5__table',
+ viz_type: VizType.Bullet,
+ metric: 'sum__num',
+ ranges: '100,200,300',
+ rangeLabels: 'low,mid,high',
+ markers: '150',
+ markerLabels: 'goal',
+ markerLines: '250',
+ markerLineLabels: 'stretch',
+};
+
+const chartProps = (overrides: Partial<SqlaFormData> = {}) =>
+ new ChartProps({
+ width: 800,
+ height: 200,
+ formData: { ...formData, ...overrides },
+ theme: supersetTheme,
+ queriesData: [{ data: [{ sum__num: 120 }] }],
+ hooks: {},
+ }) as unknown as EchartsBulletChartProps;
+
+test('renders the measure bar with markers, marker lines and range bands', () => {
+ const { echartOptions } = transformProps(chartProps());
+ const series = (echartOptions as any).series;
Review Comment:
**Suggestion:** Replace the `any` cast with a concrete ECharts options type (or a narrowed local interface) before reading `series`. [custom_rule]
**Severity Level:** Minor π§Ή
<details>
<summary><b>Why it matters? β </b></summary>
The code uses `as any`, which directly violates the no-`any` rule. The suggestion correctly identifies a real instance of that violation.
</details>
<details>
<summary><b>Rule source π </b></summary>
=== .cursor/rules/dev-standard.mdc === (line 16)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=6ed2e7a568ea4f419a8e0a0746092545&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=6ed2e7a568ea4f419a8e0a0746092545&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/Bullet/transformProps.test.ts
**Line:** 48:48
**Comment:**
*Custom Rule: Replace the `any` cast with a concrete ECharts options type (or a narrowed local interface) before reading `series`.
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%2F42225&comment_hash=fe540a8bf2fe4f5d87dc1820bb92deb9c56b8b2b47c061cba76d9f671d120d50&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42225&comment_hash=fe540a8bf2fe4f5d87dc1820bb92deb9c56b8b2b47c061cba76d9f671d120d50&reaction=dislike'>π</a>
##########
superset-frontend/plugins/plugin-chart-echarts/test/Bullet/transformProps.test.ts:
##########
@@ -0,0 +1,99 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { ChartProps, SqlaFormData, VizType } from '@superset-ui/core';
+import { supersetTheme } from '@apache-superset/core/theme';
+import transformProps from '../../src/Bullet/transformProps';
+import { EchartsBulletChartProps } from '../../src/Bullet/types';
+
+const formData: SqlaFormData = {
+ datasource: '5__table',
+ viz_type: VizType.Bullet,
+ metric: 'sum__num',
+ ranges: '100,200,300',
+ rangeLabels: 'low,mid,high',
+ markers: '150',
+ markerLabels: 'goal',
+ markerLines: '250',
+ markerLineLabels: 'stretch',
+};
+
+const chartProps = (overrides: Partial<SqlaFormData> = {}) =>
+ new ChartProps({
+ width: 800,
+ height: 200,
+ formData: { ...formData, ...overrides },
+ theme: supersetTheme,
+ queriesData: [{ data: [{ sum__num: 120 }] }],
+ hooks: {},
+ }) as unknown as EchartsBulletChartProps;
+
+test('renders the measure bar with markers, marker lines and range bands', () => {
+ const { echartOptions } = transformProps(chartProps());
+ const series = (echartOptions as any).series;
+
+ // measure bar
+ expect(series[0].type).toBe('bar');
+ expect(series[0].data).toEqual([120]);
+
+ // marker lines on the bar series
+ expect(series[0].markLine.data).toHaveLength(1);
+ expect(series[0].markLine.data[0].xAxis).toBe(250);
+
+ // nested range bands, largest drawn first
+ const bandEnds = series[0].markArea.data.map((d: any) => d[1].xAxis);
+ expect(bandEnds).toEqual([300, 200, 100]);
+
+ // triangle markers
+ expect(series[1].type).toBe('scatter');
+ expect(series[1].data[0].value).toEqual([150, 0]);
+
+ // axis spans all values
+ expect((echartOptions as any).xAxis.max).toBe(300);
+});
+
+test('defaults the band to 110% of the measure when no ranges are set', () => {
+ const { echartOptions } = transformProps(
+ chartProps({
+ ranges: '',
+ rangeLabels: '',
+ markers: '',
+ markerLabels: '',
+ markerLines: '',
+ markerLineLabels: '',
+ }),
+ );
+ const series = (echartOptions as any).series;
+ expect(series[0].data).toEqual([120]);
+ const bandEnds = series[0].markArea.data.map((d: any) => d[1].xAxis);
+ expect(bandEnds).toEqual([120 * 1.1]);
+ expect(series[1].data).toHaveLength(0);
+});
+
+test('handles an empty query result without crashing', () => {
+ const props = new ChartProps({
+ width: 800,
+ height: 200,
+ formData,
+ theme: supersetTheme,
+ queriesData: [{ data: [] }],
+ hooks: {},
+ }) as unknown as EchartsBulletChartProps;
+ const { echartOptions } = transformProps(props);
+ expect((echartOptions as any).series[0].data).toEqual([0]);
Review Comment:
**Suggestion:** Replace the `any` assertion with an explicit type for `echartOptions` before checking nested `series` data. [custom_rule]
**Severity Level:** Minor π§Ή
<details>
<summary><b>Why it matters? β </b></summary>
The snippet uses `as any` to access nested series data, which is a direct violation of the no-`any` rule. The suggestion is accurate.
</details>
<details>
<summary><b>Rule source π </b></summary>
=== .cursor/rules/dev-standard.mdc === (line 16)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=c8ba32e5dd9d40b199a7c2429eac2a3e&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=c8ba32e5dd9d40b199a7c2429eac2a3e&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/Bullet/transformProps.test.ts
**Line:** 98:98
**Comment:**
*Custom Rule: Replace the `any` assertion with an explicit type for `echartOptions` before checking nested `series` data.
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%2F42225&comment_hash=d4ee3c7ea709023c1f2ac4ba72f742f987ae3d07b5fb1235fd3cf2ebe3bc9e49&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42225&comment_hash=d4ee3c7ea709023c1f2ac4ba72f742f987ae3d07b5fb1235fd3cf2ebe3bc9e49&reaction=dislike'>π</a>
##########
superset-frontend/plugins/plugin-chart-echarts/test/Bullet/transformProps.test.ts:
##########
@@ -0,0 +1,99 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { ChartProps, SqlaFormData, VizType } from '@superset-ui/core';
+import { supersetTheme } from '@apache-superset/core/theme';
+import transformProps from '../../src/Bullet/transformProps';
+import { EchartsBulletChartProps } from '../../src/Bullet/types';
+
+const formData: SqlaFormData = {
+ datasource: '5__table',
+ viz_type: VizType.Bullet,
+ metric: 'sum__num',
+ ranges: '100,200,300',
+ rangeLabels: 'low,mid,high',
+ markers: '150',
+ markerLabels: 'goal',
+ markerLines: '250',
+ markerLineLabels: 'stretch',
+};
+
+const chartProps = (overrides: Partial<SqlaFormData> = {}) =>
+ new ChartProps({
+ width: 800,
+ height: 200,
+ formData: { ...formData, ...overrides },
+ theme: supersetTheme,
+ queriesData: [{ data: [{ sum__num: 120 }] }],
+ hooks: {},
+ }) as unknown as EchartsBulletChartProps;
+
+test('renders the measure bar with markers, marker lines and range bands', () => {
+ const { echartOptions } = transformProps(chartProps());
+ const series = (echartOptions as any).series;
+
+ // measure bar
+ expect(series[0].type).toBe('bar');
+ expect(series[0].data).toEqual([120]);
+
+ // marker lines on the bar series
+ expect(series[0].markLine.data).toHaveLength(1);
+ expect(series[0].markLine.data[0].xAxis).toBe(250);
+
+ // nested range bands, largest drawn first
+ const bandEnds = series[0].markArea.data.map((d: any) => d[1].xAxis);
+ expect(bandEnds).toEqual([300, 200, 100]);
+
+ // triangle markers
+ expect(series[1].type).toBe('scatter');
+ expect(series[1].data[0].value).toEqual([150, 0]);
+
+ // axis spans all values
+ expect((echartOptions as any).xAxis.max).toBe(300);
Review Comment:
**Suggestion:** Use a typed options object (or a specific type assertion) rather than casting `echartOptions` to `any` for axis access. [custom_rule]
**Severity Level:** Minor π§Ή
<details>
<summary><b>Why it matters? β </b></summary>
The code uses `as any` to access `xAxis`, which violates the no-`any` rule. The suggestion matches an actual rule breach.
</details>
<details>
<summary><b>Rule source π </b></summary>
=== .cursor/rules/dev-standard.mdc === (line 16)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=87281a0bfb084c3d976528d79a573692&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=87281a0bfb084c3d976528d79a573692&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/Bullet/transformProps.test.ts
**Line:** 67:67
**Comment:**
*Custom Rule: Use a typed options object (or a specific type assertion) rather than casting `echartOptions` to `any` for axis access.
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%2F42225&comment_hash=da20e7c2a34d13f245b49c431d036b24e34dcabaf085145de1df642cabad8682&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42225&comment_hash=da20e7c2a34d13f245b49c431d036b24e34dcabaf085145de1df642cabad8682&reaction=dislike'>π</a>
##########
superset-frontend/plugins/plugin-chart-echarts/test/Bullet/transformProps.test.ts:
##########
@@ -0,0 +1,99 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { ChartProps, SqlaFormData, VizType } from '@superset-ui/core';
+import { supersetTheme } from '@apache-superset/core/theme';
+import transformProps from '../../src/Bullet/transformProps';
+import { EchartsBulletChartProps } from '../../src/Bullet/types';
+
+const formData: SqlaFormData = {
+ datasource: '5__table',
+ viz_type: VizType.Bullet,
+ metric: 'sum__num',
+ ranges: '100,200,300',
+ rangeLabels: 'low,mid,high',
+ markers: '150',
+ markerLabels: 'goal',
+ markerLines: '250',
+ markerLineLabels: 'stretch',
+};
+
+const chartProps = (overrides: Partial<SqlaFormData> = {}) =>
+ new ChartProps({
+ width: 800,
+ height: 200,
+ formData: { ...formData, ...overrides },
+ theme: supersetTheme,
+ queriesData: [{ data: [{ sum__num: 120 }] }],
+ hooks: {},
+ }) as unknown as EchartsBulletChartProps;
+
+test('renders the measure bar with markers, marker lines and range bands', () => {
+ const { echartOptions } = transformProps(chartProps());
+ const series = (echartOptions as any).series;
+
+ // measure bar
+ expect(series[0].type).toBe('bar');
+ expect(series[0].data).toEqual([120]);
+
+ // marker lines on the bar series
+ expect(series[0].markLine.data).toHaveLength(1);
+ expect(series[0].markLine.data[0].xAxis).toBe(250);
+
+ // nested range bands, largest drawn first
+ const bandEnds = series[0].markArea.data.map((d: any) => d[1].xAxis);
Review Comment:
**Suggestion:** Type the mapped callback parameter with the expected mark-area item shape instead of `any`. [custom_rule]
**Severity Level:** Minor π§Ή
<details>
<summary><b>Why it matters? β </b></summary>
The callback parameter is explicitly typed as `any`, which is a direct violation of the no-`any` rule. This is a real, present issue in the file.
</details>
<details>
<summary><b>Rule source π </b></summary>
=== .cursor/rules/dev-standard.mdc === (line 16)
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=a74032623fcb40bbb98bbd58f2b0c687&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=a74032623fcb40bbb98bbd58f2b0c687&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/Bullet/transformProps.test.ts
**Line:** 59:59
**Comment:**
*Custom Rule: Type the mapped callback parameter with the expected mark-area item shape instead of `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%2F42225&comment_hash=a2498295431259f2accf8ca8f602e85895fd29be4d8af43c1f2daa2da54051c6&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42225&comment_hash=a2498295431259f2accf8ca8f602e85895fd29be4d8af43c1f2daa2da54051c6&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
Re: [PR] feat(bullet): rebuild the Bullet Chart on ECharts [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #42225:
URL: https://github.com/apache/superset/pull/42225#issuecomment-5019482320
### <span aria-hidden="true">β
</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">π¨</span> Latest commit | 09c2102838374132bc71f77fc04b4e137bc4b491 |
|<span aria-hidden="true">π</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a5dc318da1fdb00086e8ebe |
|<span aria-hidden="true">π</span> Deploy Preview | [https://deploy-preview-42225--superset-docs-preview.netlify.app](https://deploy-preview-42225--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">π±</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
|<span aria-hidden="true">π€</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#q1YqKMrPLShRslJS0lFKKkrMS84AslOTMxKLSop1k0pzclJLgDIpqQU5-ZWeKUA5s0TTlGRjQ4uURMO0lCQDAwMLs1SL1KRUpVoA) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-42225--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
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
Re: [PR] feat(bullet): rebuild the Bullet Chart on ECharts [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #42225:
URL: https://github.com/apache/superset/pull/42225#discussion_r3612686210
##########
superset-frontend/plugins/plugin-chart-echarts/test/Bullet/transformProps.test.ts:
##########
@@ -0,0 +1,99 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { ChartProps, SqlaFormData, VizType } from '@superset-ui/core';
+import { supersetTheme } from '@apache-superset/core/theme';
+import transformProps from '../../src/Bullet/transformProps';
+import { EchartsBulletChartProps } from '../../src/Bullet/types';
+
+const formData: SqlaFormData = {
+ datasource: '5__table',
+ viz_type: VizType.Bullet,
+ metric: 'sum__num',
+ ranges: '100,200,300',
+ rangeLabels: 'low,mid,high',
+ markers: '150',
+ markerLabels: 'goal',
+ markerLines: '250',
+ markerLineLabels: 'stretch',
+};
+
+const chartProps = (overrides: Partial<SqlaFormData> = {}) =>
+ new ChartProps({
+ width: 800,
+ height: 200,
+ formData: { ...formData, ...overrides },
+ theme: supersetTheme,
+ queriesData: [{ data: [{ sum__num: 120 }] }],
+ hooks: {},
+ }) as unknown as EchartsBulletChartProps;
+
+test('renders the measure bar with markers, marker lines and range bands', () => {
+ const { echartOptions } = transformProps(chartProps());
+ const series = (echartOptions as any).series;
Review Comment:
Test file already follows this package's established convention β `as any` on `echartOptions`/`series` shows up in nearly every other Bullet-adjacent `transformProps.test.ts` (Timeseries, BigNumber, Funnel, Graph, etc.) since `EChartsCoreOption`'s series union isn't practical to narrow in test assertions. Not changing this one file in isolation.
##########
superset-frontend/plugins/plugin-chart-echarts/test/Bullet/transformProps.test.ts:
##########
@@ -0,0 +1,99 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { ChartProps, SqlaFormData, VizType } from '@superset-ui/core';
+import { supersetTheme } from '@apache-superset/core/theme';
+import transformProps from '../../src/Bullet/transformProps';
+import { EchartsBulletChartProps } from '../../src/Bullet/types';
+
+const formData: SqlaFormData = {
+ datasource: '5__table',
+ viz_type: VizType.Bullet,
+ metric: 'sum__num',
+ ranges: '100,200,300',
+ rangeLabels: 'low,mid,high',
+ markers: '150',
+ markerLabels: 'goal',
+ markerLines: '250',
+ markerLineLabels: 'stretch',
+};
+
+const chartProps = (overrides: Partial<SqlaFormData> = {}) =>
+ new ChartProps({
+ width: 800,
+ height: 200,
+ formData: { ...formData, ...overrides },
+ theme: supersetTheme,
+ queriesData: [{ data: [{ sum__num: 120 }] }],
+ hooks: {},
+ }) as unknown as EchartsBulletChartProps;
+
+test('renders the measure bar with markers, marker lines and range bands', () => {
+ const { echartOptions } = transformProps(chartProps());
+ const series = (echartOptions as any).series;
+
+ // measure bar
+ expect(series[0].type).toBe('bar');
+ expect(series[0].data).toEqual([120]);
+
+ // marker lines on the bar series
+ expect(series[0].markLine.data).toHaveLength(1);
+ expect(series[0].markLine.data[0].xAxis).toBe(250);
+
+ // nested range bands, largest drawn first
+ const bandEnds = series[0].markArea.data.map((d: any) => d[1].xAxis);
Review Comment:
Same as the other `any`-in-test-file comments on this file β matches this package's existing test convention, not fixing in isolation.
##########
superset-frontend/plugins/plugin-chart-echarts/test/Bullet/transformProps.test.ts:
##########
@@ -0,0 +1,99 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { ChartProps, SqlaFormData, VizType } from '@superset-ui/core';
+import { supersetTheme } from '@apache-superset/core/theme';
+import transformProps from '../../src/Bullet/transformProps';
+import { EchartsBulletChartProps } from '../../src/Bullet/types';
+
+const formData: SqlaFormData = {
+ datasource: '5__table',
+ viz_type: VizType.Bullet,
+ metric: 'sum__num',
+ ranges: '100,200,300',
+ rangeLabels: 'low,mid,high',
+ markers: '150',
+ markerLabels: 'goal',
+ markerLines: '250',
+ markerLineLabels: 'stretch',
+};
+
+const chartProps = (overrides: Partial<SqlaFormData> = {}) =>
+ new ChartProps({
+ width: 800,
+ height: 200,
+ formData: { ...formData, ...overrides },
+ theme: supersetTheme,
+ queriesData: [{ data: [{ sum__num: 120 }] }],
+ hooks: {},
+ }) as unknown as EchartsBulletChartProps;
+
+test('renders the measure bar with markers, marker lines and range bands', () => {
+ const { echartOptions } = transformProps(chartProps());
+ const series = (echartOptions as any).series;
+
+ // measure bar
+ expect(series[0].type).toBe('bar');
+ expect(series[0].data).toEqual([120]);
+
+ // marker lines on the bar series
+ expect(series[0].markLine.data).toHaveLength(1);
+ expect(series[0].markLine.data[0].xAxis).toBe(250);
+
+ // nested range bands, largest drawn first
+ const bandEnds = series[0].markArea.data.map((d: any) => d[1].xAxis);
+ expect(bandEnds).toEqual([300, 200, 100]);
+
+ // triangle markers
+ expect(series[1].type).toBe('scatter');
+ expect(series[1].data[0].value).toEqual([150, 0]);
+
+ // axis spans all values
+ expect((echartOptions as any).xAxis.max).toBe(300);
Review Comment:
Same as the other `any`-in-test-file comments on this file β matches this package's existing test convention, not fixing in isolation.
##########
superset-frontend/plugins/plugin-chart-echarts/test/Bullet/transformProps.test.ts:
##########
@@ -0,0 +1,99 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { ChartProps, SqlaFormData, VizType } from '@superset-ui/core';
+import { supersetTheme } from '@apache-superset/core/theme';
+import transformProps from '../../src/Bullet/transformProps';
+import { EchartsBulletChartProps } from '../../src/Bullet/types';
+
+const formData: SqlaFormData = {
+ datasource: '5__table',
+ viz_type: VizType.Bullet,
+ metric: 'sum__num',
+ ranges: '100,200,300',
+ rangeLabels: 'low,mid,high',
+ markers: '150',
+ markerLabels: 'goal',
+ markerLines: '250',
+ markerLineLabels: 'stretch',
+};
+
+const chartProps = (overrides: Partial<SqlaFormData> = {}) =>
+ new ChartProps({
+ width: 800,
+ height: 200,
+ formData: { ...formData, ...overrides },
+ theme: supersetTheme,
+ queriesData: [{ data: [{ sum__num: 120 }] }],
+ hooks: {},
+ }) as unknown as EchartsBulletChartProps;
+
+test('renders the measure bar with markers, marker lines and range bands', () => {
+ const { echartOptions } = transformProps(chartProps());
+ const series = (echartOptions as any).series;
+
+ // measure bar
+ expect(series[0].type).toBe('bar');
+ expect(series[0].data).toEqual([120]);
+
+ // marker lines on the bar series
+ expect(series[0].markLine.data).toHaveLength(1);
+ expect(series[0].markLine.data[0].xAxis).toBe(250);
+
+ // nested range bands, largest drawn first
+ const bandEnds = series[0].markArea.data.map((d: any) => d[1].xAxis);
+ expect(bandEnds).toEqual([300, 200, 100]);
+
+ // triangle markers
+ expect(series[1].type).toBe('scatter');
+ expect(series[1].data[0].value).toEqual([150, 0]);
+
+ // axis spans all values
+ expect((echartOptions as any).xAxis.max).toBe(300);
+});
+
+test('defaults the band to 110% of the measure when no ranges are set', () => {
+ const { echartOptions } = transformProps(
+ chartProps({
+ ranges: '',
+ rangeLabels: '',
+ markers: '',
+ markerLabels: '',
+ markerLines: '',
+ markerLineLabels: '',
+ }),
+ );
+ const series = (echartOptions as any).series;
+ expect(series[0].data).toEqual([120]);
+ const bandEnds = series[0].markArea.data.map((d: any) => d[1].xAxis);
+ expect(bandEnds).toEqual([120 * 1.1]);
+ expect(series[1].data).toHaveLength(0);
+});
+
+test('handles an empty query result without crashing', () => {
+ const props = new ChartProps({
+ width: 800,
+ height: 200,
+ formData,
+ theme: supersetTheme,
+ queriesData: [{ data: [] }],
+ hooks: {},
+ }) as unknown as EchartsBulletChartProps;
+ const { echartOptions } = transformProps(props);
+ expect((echartOptions as any).series[0].data).toEqual([0]);
Review Comment:
Same as the other `any`-in-test-file comments on this file β matches this package's existing test convention, not fixing in isolation.
--
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
Re: [PR] feat(bullet): rebuild the Bullet Chart on ECharts [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #42225:
URL: https://github.com/apache/superset/pull/42225#issuecomment-5028788820
πͺ [Showtime](https://github.com/mistercrunch/superset-showtime) deployed environment on [GHA](https://github.com/apache/superset/actions/runs/29789581169) for [38d59fb](https://github.com/apache/superset/commit/38d59fb)
β’ **Environment:** http://100.23.165.50:8080 (admin/admin)
β’ **Lifetime:** 48h auto-cleanup
β’ **Updates:** New commits create fresh environments automatically
--
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
Re: [PR] feat(bullet): rebuild the Bullet Chart on ECharts [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #42225:
URL: https://github.com/apache/superset/pull/42225#issuecomment-5024736002
πͺ [Showtime](https://github.com/mistercrunch/superset-showtime) deployed environment on [GHA](https://github.com/apache/superset/actions/runs/29758593182) for [6518be3](https://github.com/apache/superset/commit/6518be3)
β’ **Environment:** http://16.147.233.242:8080 (admin/admin)
β’ **Lifetime:** 48h auto-cleanup
β’ **Updates:** New commits create fresh environments automatically
--
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
Re: [PR] feat(bullet): rebuild the Bullet Chart on ECharts [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #42225:
URL: https://github.com/apache/superset/pull/42225
--
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
Re: [PR] feat(bullet): rebuild the Bullet Chart on ECharts [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #42225:
URL: https://github.com/apache/superset/pull/42225#issuecomment-5028356150
πͺ [Showtime](https://github.com/mistercrunch/superset-showtime) is building environment on [GHA](https://github.com/apache/superset/actions/runs/29787705462) for [684a97e](https://github.com/apache/superset/commit/684a97e)
--
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
Re: [PR] feat(bullet): rebuild the Bullet Chart on ECharts [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #42225:
URL: https://github.com/apache/superset/pull/42225#issuecomment-5025569439
πͺ [Showtime](https://github.com/mistercrunch/superset-showtime) deployed environment on [GHA](https://github.com/apache/superset/actions/runs/29765238173) for [7021004](https://github.com/apache/superset/commit/7021004)
β’ **Environment:** http://35.88.181.27:8080 (admin/admin)
β’ **Lifetime:** 48h auto-cleanup
β’ **Updates:** New commits create fresh environments automatically
--
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
Re: [PR] feat(bullet): rebuild the Bullet Chart on ECharts [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #42225:
URL: https://github.com/apache/superset/pull/42225#discussion_r3612687534
##########
superset-frontend/plugins/plugin-chart-echarts/src/Bullet/transformProps.ts:
##########
@@ -0,0 +1,191 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import {
+ getMetricLabel,
+ getNumberFormatter,
+ JsonObject,
+} from '@superset-ui/core';
+import type { EChartsCoreOption } from 'echarts/core';
+import { Refs } from '../types';
+import { EchartsBulletChartProps, BulletChartTransformedProps } from './types';
+import { tokenizeToNumericArray, tokenizeToStringArray } from './utils';
+
+const MEASURE_BAR_FRACTION = 0.28;
+
+export default function transformProps(
+ chartProps: EchartsBulletChartProps,
+): BulletChartTransformedProps {
+ const { width, height, formData, queriesData, theme, hooks } = chartProps;
+ const {
+ metric,
+ ranges: rawRanges,
+ rangeLabels: rawRangeLabels,
+ markers: rawMarkers,
+ markerLabels: rawMarkerLabels,
+ markerLines: rawMarkerLines,
+ markerLineLabels: rawMarkerLineLabels,
+ yAxisFormat,
+ } = formData;
+ const refs: Refs = {};
+ const { onContextMenu, setDataMask = () => {} } = hooks;
+
+ const metricLabel = getMetricLabel(metric ?? '');
+ const records = (queriesData[0]?.data ?? []) as JsonObject[];
+ const measure = Number(records[0]?.[metricLabel] ?? 0);
+
+ // Match the legacy default: a single band up to 110% of the measure.
+ const ranges = tokenizeToNumericArray(rawRanges) ?? [0, measure * 1.1];
+ const rangeLabels = tokenizeToStringArray(rawRangeLabels) ?? [];
+ const markers = tokenizeToNumericArray(rawMarkers) ?? [];
+ const markerLabels = tokenizeToStringArray(rawMarkerLabels) ?? [];
+ const markerLines = tokenizeToNumericArray(rawMarkerLines) ?? [];
+ const markerLineLabels = tokenizeToStringArray(rawMarkerLineLabels) ?? [];
+
+ const formatter = getNumberFormatter(yAxisFormat);
+
+ const axisMax = Math.max(measure, ...ranges, ...markers, ...markerLines);
+ const axisMin = Math.min(0, measure, ...ranges, ...markers, ...markerLines);
+
+ // Nested qualitative bands like d3-bullet: draw from the largest threshold
+ // down so smaller (darker) bands paint on top.
+ const bandFills = [
+ theme.colorFillQuaternary,
+ theme.colorFillTertiary,
+ theme.colorFillSecondary,
+ theme.colorFill,
+ ];
+ const sortedRanges = [...ranges]
+ .map((value, i) => ({ value, label: rangeLabels[i] }))
+ .filter(({ value }) => value > axisMin)
+ .sort((a, b) => b.value - a.value);
+ const bands = sortedRanges.map(({ value, label }, i) => ({
+ name: label,
+ itemStyle: {
+ color: bandFills[Math.min(i, bandFills.length - 1)],
+ },
+ label: { show: false },
+ // markArea item: [{start}, {end}]
+ coords: [{ xAxis: axisMin }, { xAxis: value }],
+ }));
+
+ const markerLabelOf = (value: number, labels: string[], values: number[]) => {
+ const index = values.indexOf(value);
+ return index >= 0 && labels[index] ? labels[index] : formatter(value);
+ };
Review Comment:
Good catch β fixed. `markerLabelOf` now resolves by array position instead of `indexOf(value)`, so duplicate marker/marker-line values each keep their own label. Added a regression test for this.
##########
superset-frontend/plugins/plugin-chart-echarts/src/Bullet/transformProps.ts:
##########
@@ -0,0 +1,191 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import {
+ getMetricLabel,
+ getNumberFormatter,
+ JsonObject,
+} from '@superset-ui/core';
+import type { EChartsCoreOption } from 'echarts/core';
+import { Refs } from '../types';
+import { EchartsBulletChartProps, BulletChartTransformedProps } from './types';
+import { tokenizeToNumericArray, tokenizeToStringArray } from './utils';
+
+const MEASURE_BAR_FRACTION = 0.28;
+
+export default function transformProps(
+ chartProps: EchartsBulletChartProps,
+): BulletChartTransformedProps {
+ const { width, height, formData, queriesData, theme, hooks } = chartProps;
+ const {
+ metric,
+ ranges: rawRanges,
+ rangeLabels: rawRangeLabels,
+ markers: rawMarkers,
+ markerLabels: rawMarkerLabels,
+ markerLines: rawMarkerLines,
+ markerLineLabels: rawMarkerLineLabels,
+ yAxisFormat,
+ } = formData;
+ const refs: Refs = {};
+ const { onContextMenu, setDataMask = () => {} } = hooks;
+
+ const metricLabel = getMetricLabel(metric ?? '');
+ const records = (queriesData[0]?.data ?? []) as JsonObject[];
+ const measure = Number(records[0]?.[metricLabel] ?? 0);
+
+ // Match the legacy default: a single band up to 110% of the measure.
+ const ranges = tokenizeToNumericArray(rawRanges) ?? [0, measure * 1.1];
+ const rangeLabels = tokenizeToStringArray(rawRangeLabels) ?? [];
+ const markers = tokenizeToNumericArray(rawMarkers) ?? [];
+ const markerLabels = tokenizeToStringArray(rawMarkerLabels) ?? [];
+ const markerLines = tokenizeToNumericArray(rawMarkerLines) ?? [];
+ const markerLineLabels = tokenizeToStringArray(rawMarkerLineLabels) ?? [];
+
+ const formatter = getNumberFormatter(yAxisFormat);
+
+ const axisMax = Math.max(measure, ...ranges, ...markers, ...markerLines);
+ const axisMin = Math.min(0, measure, ...ranges, ...markers, ...markerLines);
+
+ // Nested qualitative bands like d3-bullet: draw from the largest threshold
+ // down so smaller (darker) bands paint on top.
+ const bandFills = [
+ theme.colorFillQuaternary,
+ theme.colorFillTertiary,
+ theme.colorFillSecondary,
+ theme.colorFill,
+ ];
+ const sortedRanges = [...ranges]
+ .map((value, i) => ({ value, label: rangeLabels[i] }))
+ .filter(({ value }) => value > axisMin)
+ .sort((a, b) => b.value - a.value);
+ const bands = sortedRanges.map(({ value, label }, i) => ({
+ name: label,
+ itemStyle: {
+ color: bandFills[Math.min(i, bandFills.length - 1)],
+ },
+ label: { show: false },
+ // markArea item: [{start}, {end}]
+ coords: [{ xAxis: axisMin }, { xAxis: value }],
+ }));
+
+ const markerLabelOf = (value: number, labels: string[], values: number[]) => {
+ const index = values.indexOf(value);
+ return index >= 0 && labels[index] ? labels[index] : formatter(value);
+ };
+
+ const echartOptions: EChartsCoreOption = {
+ grid: {
+ top: theme.sizeUnit * 2,
+ bottom: theme.sizeUnit * 6,
+ left: theme.sizeUnit * 2,
+ right: theme.sizeUnit * 2,
+ containLabel: true,
+ },
+ xAxis: {
+ type: 'value',
+ min: axisMin,
+ max: axisMax,
+ axisLabel: {
+ color: theme.colorTextSecondary,
+ formatter: (value: number) => formatter(value),
+ },
+ splitLine: { show: false },
+ },
+ yAxis: {
+ type: 'category',
+ data: [''],
+ axisLine: { show: false },
+ axisTick: { show: false },
+ axisLabel: { show: false },
+ },
+ tooltip: {
+ confine: true,
+ formatter: () => `${metricLabel}: <b>${formatter(measure)}</b>`,
+ },
Review Comment:
Fixed β the measure tooltip's HTML string now goes through `sanitizeHtml` before being returned to the ECharts formatter, matching the sanitization convention this package already uses in `tooltipHtml()`.
##########
superset-frontend/plugins/plugin-chart-echarts/src/Bullet/transformProps.ts:
##########
@@ -0,0 +1,191 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import {
+ getMetricLabel,
+ getNumberFormatter,
+ JsonObject,
+} from '@superset-ui/core';
+import type { EChartsCoreOption } from 'echarts/core';
+import { Refs } from '../types';
+import { EchartsBulletChartProps, BulletChartTransformedProps } from './types';
+import { tokenizeToNumericArray, tokenizeToStringArray } from './utils';
+
+const MEASURE_BAR_FRACTION = 0.28;
+
+export default function transformProps(
+ chartProps: EchartsBulletChartProps,
+): BulletChartTransformedProps {
+ const { width, height, formData, queriesData, theme, hooks } = chartProps;
+ const {
+ metric,
+ ranges: rawRanges,
+ rangeLabels: rawRangeLabels,
+ markers: rawMarkers,
+ markerLabels: rawMarkerLabels,
+ markerLines: rawMarkerLines,
+ markerLineLabels: rawMarkerLineLabels,
+ yAxisFormat,
+ } = formData;
+ const refs: Refs = {};
+ const { onContextMenu, setDataMask = () => {} } = hooks;
+
+ const metricLabel = getMetricLabel(metric ?? '');
+ const records = (queriesData[0]?.data ?? []) as JsonObject[];
+ const measure = Number(records[0]?.[metricLabel] ?? 0);
+
+ // Match the legacy default: a single band up to 110% of the measure.
+ const ranges = tokenizeToNumericArray(rawRanges) ?? [0, measure * 1.1];
+ const rangeLabels = tokenizeToStringArray(rawRangeLabels) ?? [];
+ const markers = tokenizeToNumericArray(rawMarkers) ?? [];
+ const markerLabels = tokenizeToStringArray(rawMarkerLabels) ?? [];
+ const markerLines = tokenizeToNumericArray(rawMarkerLines) ?? [];
+ const markerLineLabels = tokenizeToStringArray(rawMarkerLineLabels) ?? [];
+
+ const formatter = getNumberFormatter(yAxisFormat);
+
+ const axisMax = Math.max(measure, ...ranges, ...markers, ...markerLines);
+ const axisMin = Math.min(0, measure, ...ranges, ...markers, ...markerLines);
+
+ // Nested qualitative bands like d3-bullet: draw from the largest threshold
+ // down so smaller (darker) bands paint on top.
+ const bandFills = [
+ theme.colorFillQuaternary,
+ theme.colorFillTertiary,
+ theme.colorFillSecondary,
+ theme.colorFill,
+ ];
+ const sortedRanges = [...ranges]
+ .map((value, i) => ({ value, label: rangeLabels[i] }))
+ .filter(({ value }) => value > axisMin)
+ .sort((a, b) => b.value - a.value);
+ const bands = sortedRanges.map(({ value, label }, i) => ({
+ name: label,
+ itemStyle: {
+ color: bandFills[Math.min(i, bandFills.length - 1)],
+ },
+ label: { show: false },
+ // markArea item: [{start}, {end}]
+ coords: [{ xAxis: axisMin }, { xAxis: value }],
+ }));
+
+ const markerLabelOf = (value: number, labels: string[], values: number[]) => {
+ const index = values.indexOf(value);
+ return index >= 0 && labels[index] ? labels[index] : formatter(value);
+ };
+
+ const echartOptions: EChartsCoreOption = {
+ grid: {
+ top: theme.sizeUnit * 2,
+ bottom: theme.sizeUnit * 6,
+ left: theme.sizeUnit * 2,
+ right: theme.sizeUnit * 2,
+ containLabel: true,
+ },
+ xAxis: {
+ type: 'value',
+ min: axisMin,
+ max: axisMax,
+ axisLabel: {
+ color: theme.colorTextSecondary,
+ formatter: (value: number) => formatter(value),
+ },
+ splitLine: { show: false },
+ },
+ yAxis: {
+ type: 'category',
+ data: [''],
+ axisLine: { show: false },
+ axisTick: { show: false },
+ axisLabel: { show: false },
+ },
+ tooltip: {
+ confine: true,
+ formatter: () => `${metricLabel}: <b>${formatter(measure)}</b>`,
+ },
+ series: [
+ {
+ name: metricLabel,
+ type: 'bar',
+ data: [measure],
+ barWidth: `${MEASURE_BAR_FRACTION * 100}%`,
+ itemStyle: { color: theme.colorPrimary },
+ z: 10,
+ markArea: {
+ silent: true,
+ data: bands.map(band => [
+ {
+ name: band.name,
+ itemStyle: band.itemStyle,
+ label: band.label,
+ ...band.coords[0],
+ },
+ band.coords[1],
+ ]),
+ },
+ markLine: {
+ silent: true,
+ symbol: 'none',
+ data: markerLines.map(value => ({
+ xAxis: value,
+ label: {
+ formatter: () =>
+ markerLabelOf(value, markerLineLabels, markerLines),
+ color: theme.colorTextSecondary,
+ },
+ lineStyle: {
+ color: theme.colorText,
+ type: 'solid',
+ width: 2,
+ },
+ })),
+ },
+ },
+ {
+ name: 'markers',
+ type: 'scatter',
+ data: markers.map(value => ({
+ value: [value, 0],
+ tooltip: {
+ formatter: () =>
+ `${markerLabelOf(value, markerLabels, markers)}: <b>${formatter(
+ value,
+ )}</b>`,
+ },
Review Comment:
Fixed the same way β the marker tooltip formatter now runs its output through `sanitizeHtml` too.
--
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
Re: [PR] feat(bullet): rebuild the Bullet Chart on ECharts [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #42225:
URL: https://github.com/apache/superset/pull/42225#issuecomment-5028827292
πͺ [Showtime](https://github.com/mistercrunch/superset-showtime) is building environment on [GHA](https://github.com/apache/superset/actions/runs/29790674925) for [caa31e3](https://github.com/apache/superset/commit/caa31e3)
--
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
Re: [PR] feat(bullet): rebuild the Bullet Chart on ECharts [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #42225:
URL: https://github.com/apache/superset/pull/42225#discussion_r3616413594
##########
superset-frontend/plugins/plugin-chart-echarts/src/Bullet/EchartsBullet.tsx:
##########
@@ -0,0 +1,38 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import Echart from '../components/Echart';
+import { allEventHandlers } from '../utils/eventHandlers';
+import { BulletChartTransformedProps } from './types';
+
+export default function EchartsBullet(props: BulletChartTransformedProps) {
+ const { height, width, echartOptions, refs, formData } = props;
+
+ const eventHandlers = allEventHandlers(props);
+
+ return (
+ <Echart
+ refs={refs}
+ height={height}
+ width={width}
+ echartOptions={echartOptions}
+ eventHandlers={eventHandlers}
+ vizType={formData.vizType}
Review Comment:
**Suggestion:** The form-data key for chart type is expected to be snake_case in Superset (`viz_type`), but this code reads `vizType`, which can evaluate to `undefined` and break chart-type-dependent behavior in the shared ECharts wrapper (such as event handling/metadata paths keyed by viz type). Use the correct form-data field name consistently. [logic error]
<details>
<summary><b>Severity Level:</b> Major β οΈ</summary>
```mdx
- β Bullet charts send undefined vizType into shared Echart.
- β οΈ Bullet-specific event handlers may not register correctly.
- β οΈ Chart-type-based metadata paths may be missing for bullet.
- β οΈ Analytics keyed by viz_type can misclassify bullet charts.
```
</details>
<details>
<summary><b>Steps of Reproduction β
</b></summary>
```mdx
1. Open any existing Bullet chart in Superset, which uses the `bullet` viz_type and the
new ECharts-based renderer wired through
`plugins/plugin-chart-echarts/src/Bullet/EchartsBullet.tsx:23-38`.
2. Observe that the `BulletChartTransformedProps` passed into `EchartsBullet` contains
`formData` originating from the server-side form data, which uses the snake_case key
`viz_type` (standard across Superset form data), not a camelCase `vizType`.
3. At `superset-frontend/plugins/plugin-chart-echarts/src/Bullet/EchartsBullet.tsx:35`,
the component passes `vizType={formData.vizType}` into the shared `<Echart>` wrapper;
since `formData` does not define `vizType`, the value passed is `undefined`.
4. In the shared ECharts wrapper at
`superset-frontend/plugins/plugin-chart-echarts/src/components/Echart.tsx` (which keys
certain behaviors such as event handling and metadata paths off the `vizType` prop), the
undefined `vizType` causes Bullet charts to miss chart-type-specific handling (for
example, event handler registration keyed by viz type), while other ECharts charts, which
correctly pass `formData.viz_type`, retain their type-specific behavior.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=8888e896e2ba4d41973bdb1e9ee464de&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=8888e896e2ba4d41973bdb1e9ee464de&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/src/Bullet/EchartsBullet.tsx
**Line:** 35:35
**Comment:**
*Logic Error: The form-data key for chart type is expected to be snake_case in Superset (`viz_type`), but this code reads `vizType`, which can evaluate to `undefined` and break chart-type-dependent behavior in the shared ECharts wrapper (such as event handling/metadata paths keyed by viz type). Use the correct form-data field name consistently.
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%2F42225&comment_hash=e415f555edb9e5935fcfa3d81b94377af3633c86623e11a558e3dafd87797e6d&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42225&comment_hash=e415f555edb9e5935fcfa3d81b94377af3633c86623e11a558e3dafd87797e6d&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
Re: [PR] feat(bullet): rebuild the Bullet Chart on ECharts [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #42225:
URL: https://github.com/apache/superset/pull/42225#discussion_r3615751476
##########
superset-frontend/plugins/plugin-chart-echarts/src/Bullet/buildQuery.ts:
##########
@@ -0,0 +1,34 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { buildQueryContext, QueryFormData } from '@superset-ui/core';
+
+/**
+ * A single ungrouped aggregate of the selected metric, mirroring the query
+ * the legacy nvd3 bullet chart issued.
+ */
+export default function buildQuery(formData: QueryFormData) {
+ const { metric } = formData;
+ return buildQueryContext(formData, baseQueryObject => [
+ {
+ ...baseQueryObject,
+ columns: [],
+ metrics: metric ? [metric] : [],
Review Comment:
β
**Customized review instruction saved!**
**Instruction:**
> Do not flag missing-metric handling in the Bullet chart buildQuery when the shared metric control already enforces a non-empty selection and the implementation intentionally matches the legacy chart query behavior.
**Applied to:**
- `superset-frontend/plugins/plugin-chart-echarts/src/Bullet/**`
---
π‘ *To manage or update this instruction, visit: [CodeAnt AI Settings](https://app.codeant.ai/org/settings/learnings)*
--
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
Re: [PR] feat(bullet): rebuild the Bullet Chart on ECharts [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #42225:
URL: https://github.com/apache/superset/pull/42225#issuecomment-5019526474
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/42225?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `78.94737%` with `12 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 65.43%. Comparing base ([`bcd320c`](https://app.codecov.io/gh/apache/superset/commit/bcd320cbc1c4ff8856a26a0ed023ce696105b455?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`09c2102`](https://app.codecov.io/gh/apache/superset/commit/09c2102838374132bc71f77fc04b4e137bc4b491?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/42225?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [.../plugin-chart-echarts/src/Bullet/transformProps.ts](https://app.codecov.io/gh/apache/superset/pull/42225?src=pr&el=tree&filepath=superset-frontend%2Fplugins%2Fplugin-chart-echarts%2Fsrc%2FBullet%2FtransformProps.ts&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvQnVsbGV0L3RyYW5zZm9ybVByb3BzLnRz) | 83.33% | [6 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/42225?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [.../plugin-chart-echarts/src/Bullet/EchartsBullet.tsx](https://app.codecov.io/gh/apache/superset/pull/42225?src=pr&el=tree&filepath=superset-frontend%2Fplugins%2Fplugin-chart-echarts%2Fsrc%2FBullet%2FEchartsBullet.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvQnVsbGV0L0VjaGFydHNCdWxsZXQudHN4) | 0.00% | [4 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/42225?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...d/plugins/plugin-chart-echarts/src/Bullet/index.ts](https://app.codecov.io/gh/apache/superset/pull/42225?src=pr&el=tree&filepath=superset-frontend%2Fplugins%2Fplugin-chart-echarts%2Fsrc%2FBullet%2Findex.ts&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvQnVsbGV0L2luZGV4LnRz) | 66.66% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/42225?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...d/plugins/plugin-chart-echarts/src/Bullet/utils.ts](https://app.codecov.io/gh/apache/superset/pull/42225?src=pr&el=tree&filepath=superset-frontend%2Fplugins%2Fplugin-chart-echarts%2Fsrc%2FBullet%2Futils.ts&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvQnVsbGV0L3V0aWxzLnRz) | 88.88% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/42225?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## remove-legacy-viz-pipeline #42225 +/- ##
==============================================================
- Coverage 65.43% 65.43% -0.01%
==============================================================
Files 2786 2792 +6
Lines 154933 154990 +57
Branches 35623 35638 +15
==============================================================
+ Hits 101378 101414 +36
- Misses 51671 51692 +21
Partials 1884 1884
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/42225/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Ξ | |
|---|---|---|
| [javascript](https://app.codecov.io/gh/apache/superset/pull/42225/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `71.12% <78.94%> (-0.01%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/42225?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</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: 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
Re: [PR] feat(bullet): rebuild the Bullet Chart on ECharts [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #42225:
URL: https://github.com/apache/superset/pull/42225#discussion_r3612689169
##########
superset-frontend/plugins/plugin-chart-echarts/test/Bullet/transformProps.test.ts:
##########
@@ -0,0 +1,99 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { ChartProps, SqlaFormData, VizType } from '@superset-ui/core';
+import { supersetTheme } from '@apache-superset/core/theme';
+import transformProps from '../../src/Bullet/transformProps';
+import { EchartsBulletChartProps } from '../../src/Bullet/types';
+
+const formData: SqlaFormData = {
+ datasource: '5__table',
+ viz_type: VizType.Bullet,
+ metric: 'sum__num',
+ ranges: '100,200,300',
+ rangeLabels: 'low,mid,high',
+ markers: '150',
+ markerLabels: 'goal',
+ markerLines: '250',
+ markerLineLabels: 'stretch',
+};
+
+const chartProps = (overrides: Partial<SqlaFormData> = {}) =>
+ new ChartProps({
+ width: 800,
+ height: 200,
+ formData: { ...formData, ...overrides },
+ theme: supersetTheme,
+ queriesData: [{ data: [{ sum__num: 120 }] }],
+ hooks: {},
+ }) as unknown as EchartsBulletChartProps;
+
+test('renders the measure bar with markers, marker lines and range bands', () => {
+ const { echartOptions } = transformProps(chartProps());
+ const series = (echartOptions as any).series;
+
+ // measure bar
+ expect(series[0].type).toBe('bar');
+ expect(series[0].data).toEqual([120]);
+
+ // marker lines on the bar series
+ expect(series[0].markLine.data).toHaveLength(1);
+ expect(series[0].markLine.data[0].xAxis).toBe(250);
+
+ // nested range bands, largest drawn first
+ const bandEnds = series[0].markArea.data.map((d: any) => d[1].xAxis);
+ expect(bandEnds).toEqual([300, 200, 100]);
+
+ // triangle markers
+ expect(series[1].type).toBe('scatter');
+ expect(series[1].data[0].value).toEqual([150, 0]);
+
+ // axis spans all values
+ expect((echartOptions as any).xAxis.max).toBe(300);
+});
+
+test('defaults the band to 110% of the measure when no ranges are set', () => {
+ const { echartOptions } = transformProps(
+ chartProps({
+ ranges: '',
+ rangeLabels: '',
+ markers: '',
+ markerLabels: '',
+ markerLines: '',
+ markerLineLabels: '',
+ }),
+ );
+ const series = (echartOptions as any).series;
+ expect(series[0].data).toEqual([120]);
+ const bandEnds = series[0].markArea.data.map((d: any) => d[1].xAxis);
+ expect(bandEnds).toEqual([120 * 1.1]);
+ expect(series[1].data).toHaveLength(0);
+});
+
+test('handles an empty query result without crashing', () => {
+ const props = new ChartProps({
+ width: 800,
+ height: 200,
+ formData,
+ theme: supersetTheme,
+ queriesData: [{ data: [] }],
+ hooks: {},
+ }) as unknown as EchartsBulletChartProps;
+ const { echartOptions } = transformProps(props);
+ expect((echartOptions as any).series[0].data).toEqual([0]);
Review Comment:
β
**Customized review instruction saved!**
**Instruction:**
> Do not flag `any` assertions in test files when they match the package's existing test conventions; allow them in test-only code.
**Applied to:**
- `**/test/**`
- `**/tests/**`
- `**/*test*.ts`
---
π‘ *To manage or update this instruction, visit: [CodeAnt AI Settings](https://app.codeant.ai/org/settings/learnings)*
--
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
Re: [PR] feat(bullet): rebuild the Bullet Chart on ECharts [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #42225:
URL: https://github.com/apache/superset/pull/42225#issuecomment-5028053898
πͺ [Showtime](https://github.com/mistercrunch/superset-showtime) deployed environment on [GHA](https://github.com/apache/superset/actions/runs/29784580015) for [fbf2126](https://github.com/apache/superset/commit/fbf2126)
β’ **Environment:** http://52.13.51.86:8080 (admin/admin)
β’ **Lifetime:** 48h auto-cleanup
β’ **Updates:** New commits create fresh environments automatically
--
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
Re: [PR] feat(bullet): rebuild the Bullet Chart on ECharts [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #42225:
URL: https://github.com/apache/superset/pull/42225#discussion_r3612438086
##########
superset-frontend/plugins/plugin-chart-echarts/src/Bullet/transformProps.ts:
##########
@@ -0,0 +1,191 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import {
+ getMetricLabel,
+ getNumberFormatter,
+ JsonObject,
+} from '@superset-ui/core';
+import type { EChartsCoreOption } from 'echarts/core';
+import { Refs } from '../types';
+import { EchartsBulletChartProps, BulletChartTransformedProps } from './types';
+import { tokenizeToNumericArray, tokenizeToStringArray } from './utils';
+
+const MEASURE_BAR_FRACTION = 0.28;
+
+export default function transformProps(
+ chartProps: EchartsBulletChartProps,
+): BulletChartTransformedProps {
+ const { width, height, formData, queriesData, theme, hooks } = chartProps;
+ const {
+ metric,
+ ranges: rawRanges,
+ rangeLabels: rawRangeLabels,
+ markers: rawMarkers,
+ markerLabels: rawMarkerLabels,
+ markerLines: rawMarkerLines,
+ markerLineLabels: rawMarkerLineLabels,
+ yAxisFormat,
+ } = formData;
+ const refs: Refs = {};
+ const { onContextMenu, setDataMask = () => {} } = hooks;
+
+ const metricLabel = getMetricLabel(metric ?? '');
+ const records = (queriesData[0]?.data ?? []) as JsonObject[];
+ const measure = Number(records[0]?.[metricLabel] ?? 0);
+
+ // Match the legacy default: a single band up to 110% of the measure.
+ const ranges = tokenizeToNumericArray(rawRanges) ?? [0, measure * 1.1];
+ const rangeLabels = tokenizeToStringArray(rawRangeLabels) ?? [];
+ const markers = tokenizeToNumericArray(rawMarkers) ?? [];
+ const markerLabels = tokenizeToStringArray(rawMarkerLabels) ?? [];
+ const markerLines = tokenizeToNumericArray(rawMarkerLines) ?? [];
+ const markerLineLabels = tokenizeToStringArray(rawMarkerLineLabels) ?? [];
+
+ const formatter = getNumberFormatter(yAxisFormat);
+
+ const axisMax = Math.max(measure, ...ranges, ...markers, ...markerLines);
+ const axisMin = Math.min(0, measure, ...ranges, ...markers, ...markerLines);
+
+ // Nested qualitative bands like d3-bullet: draw from the largest threshold
+ // down so smaller (darker) bands paint on top.
+ const bandFills = [
+ theme.colorFillQuaternary,
+ theme.colorFillTertiary,
+ theme.colorFillSecondary,
+ theme.colorFill,
+ ];
+ const sortedRanges = [...ranges]
+ .map((value, i) => ({ value, label: rangeLabels[i] }))
+ .filter(({ value }) => value > axisMin)
+ .sort((a, b) => b.value - a.value);
+ const bands = sortedRanges.map(({ value, label }, i) => ({
+ name: label,
+ itemStyle: {
+ color: bandFills[Math.min(i, bandFills.length - 1)],
+ },
+ label: { show: false },
+ // markArea item: [{start}, {end}]
+ coords: [{ xAxis: axisMin }, { xAxis: value }],
+ }));
+
+ const markerLabelOf = (value: number, labels: string[], values: number[]) => {
+ const index = values.indexOf(value);
+ return index >= 0 && labels[index] ? labels[index] : formatter(value);
+ };
Review Comment:
**Suggestion:** Label lookup is based on `indexOf(value)`, so when duplicate marker or marker-line values exist, all duplicates resolve to the first label and later labels are never used. Preserve positional mapping by passing the array index through the map and using that index for label selection. [logic error]
<details>
<summary><b>Severity Level:</b> Major β οΈ</summary>
```mdx
- β οΈ Duplicate-value markers/lines display incorrect labels in chart.
- β οΈ Tooltips misrepresent configured semantics for repeated thresholds.
```
</details>
<details>
<summary><b>Steps of Reproduction β
</b></summary>
```mdx
1. A Bullet chart is configured with duplicate marker values and distinct labels, e.g.
`markers = "50,50"` and `markerLabels = "Target,Forecast"`, which are read from `formData`
and tokenized at lines 55β56 into `markers` and `markerLabels`.
2. When the chart is rendered, `transformProps()` at
`superset-frontend/plugins/plugin-chart-echarts/src/Bullet/transformProps.ts:31`
constructs `markerLines` and `markers` series; both use `markerLabelOf()` (defined at
lines 87β90) to resolve labels for tooltip and markLine text.
3. Inside `markerLabelOf()`, `const index = values.indexOf(value);` (line 88) always
returns the index of the first occurrence of `value` in `values`, even when there are
later duplicates.
4. For the second marker (and any subsequent duplicates), `markerLabelOf()` still returns
`labels[0]` (line 89), so the second marker/marker-line at value 50 incorrectly shows
label "Target" instead of "Forecast"; all duplicates share the first label, and later
labels are never used.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=c3d8c46a3e9c4a48a58ce44087dd80d1&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=c3d8c46a3e9c4a48a58ce44087dd80d1&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/src/Bullet/transformProps.ts
**Line:** 87:90
**Comment:**
*Logic Error: Label lookup is based on `indexOf(value)`, so when duplicate marker or marker-line values exist, all duplicates resolve to the first label and later labels are never used. Preserve positional mapping by passing the array index through the map and using that index for label selection.
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%2F42225&comment_hash=70530cd70ef047b0e8d8406804632d82aca92f527d0b27f6351383c85a7a7b1a&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42225&comment_hash=70530cd70ef047b0e8d8406804632d82aca92f527d0b27f6351383c85a7a7b1a&reaction=dislike'>π</a>
##########
superset-frontend/plugins/plugin-chart-echarts/src/Bullet/transformProps.ts:
##########
@@ -0,0 +1,191 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import {
+ getMetricLabel,
+ getNumberFormatter,
+ JsonObject,
+} from '@superset-ui/core';
+import type { EChartsCoreOption } from 'echarts/core';
+import { Refs } from '../types';
+import { EchartsBulletChartProps, BulletChartTransformedProps } from './types';
+import { tokenizeToNumericArray, tokenizeToStringArray } from './utils';
+
+const MEASURE_BAR_FRACTION = 0.28;
+
+export default function transformProps(
+ chartProps: EchartsBulletChartProps,
+): BulletChartTransformedProps {
+ const { width, height, formData, queriesData, theme, hooks } = chartProps;
+ const {
+ metric,
+ ranges: rawRanges,
+ rangeLabels: rawRangeLabels,
+ markers: rawMarkers,
+ markerLabels: rawMarkerLabels,
+ markerLines: rawMarkerLines,
+ markerLineLabels: rawMarkerLineLabels,
+ yAxisFormat,
+ } = formData;
+ const refs: Refs = {};
+ const { onContextMenu, setDataMask = () => {} } = hooks;
+
+ const metricLabel = getMetricLabel(metric ?? '');
+ const records = (queriesData[0]?.data ?? []) as JsonObject[];
+ const measure = Number(records[0]?.[metricLabel] ?? 0);
+
+ // Match the legacy default: a single band up to 110% of the measure.
+ const ranges = tokenizeToNumericArray(rawRanges) ?? [0, measure * 1.1];
+ const rangeLabels = tokenizeToStringArray(rawRangeLabels) ?? [];
+ const markers = tokenizeToNumericArray(rawMarkers) ?? [];
+ const markerLabels = tokenizeToStringArray(rawMarkerLabels) ?? [];
+ const markerLines = tokenizeToNumericArray(rawMarkerLines) ?? [];
+ const markerLineLabels = tokenizeToStringArray(rawMarkerLineLabels) ?? [];
+
+ const formatter = getNumberFormatter(yAxisFormat);
+
+ const axisMax = Math.max(measure, ...ranges, ...markers, ...markerLines);
+ const axisMin = Math.min(0, measure, ...ranges, ...markers, ...markerLines);
+
+ // Nested qualitative bands like d3-bullet: draw from the largest threshold
+ // down so smaller (darker) bands paint on top.
+ const bandFills = [
+ theme.colorFillQuaternary,
+ theme.colorFillTertiary,
+ theme.colorFillSecondary,
+ theme.colorFill,
+ ];
+ const sortedRanges = [...ranges]
+ .map((value, i) => ({ value, label: rangeLabels[i] }))
+ .filter(({ value }) => value > axisMin)
+ .sort((a, b) => b.value - a.value);
+ const bands = sortedRanges.map(({ value, label }, i) => ({
+ name: label,
+ itemStyle: {
+ color: bandFills[Math.min(i, bandFills.length - 1)],
+ },
+ label: { show: false },
+ // markArea item: [{start}, {end}]
+ coords: [{ xAxis: axisMin }, { xAxis: value }],
+ }));
+
+ const markerLabelOf = (value: number, labels: string[], values: number[]) => {
+ const index = values.indexOf(value);
+ return index >= 0 && labels[index] ? labels[index] : formatter(value);
+ };
+
+ const echartOptions: EChartsCoreOption = {
+ grid: {
+ top: theme.sizeUnit * 2,
+ bottom: theme.sizeUnit * 6,
+ left: theme.sizeUnit * 2,
+ right: theme.sizeUnit * 2,
+ containLabel: true,
+ },
+ xAxis: {
+ type: 'value',
+ min: axisMin,
+ max: axisMax,
+ axisLabel: {
+ color: theme.colorTextSecondary,
+ formatter: (value: number) => formatter(value),
+ },
+ splitLine: { show: false },
+ },
+ yAxis: {
+ type: 'category',
+ data: [''],
+ axisLine: { show: false },
+ axisTick: { show: false },
+ axisLabel: { show: false },
+ },
+ tooltip: {
+ confine: true,
+ formatter: () => `${metricLabel}: <b>${formatter(measure)}</b>`,
+ },
+ series: [
+ {
+ name: metricLabel,
+ type: 'bar',
+ data: [measure],
+ barWidth: `${MEASURE_BAR_FRACTION * 100}%`,
+ itemStyle: { color: theme.colorPrimary },
+ z: 10,
+ markArea: {
+ silent: true,
+ data: bands.map(band => [
+ {
+ name: band.name,
+ itemStyle: band.itemStyle,
+ label: band.label,
+ ...band.coords[0],
+ },
+ band.coords[1],
+ ]),
+ },
+ markLine: {
+ silent: true,
+ symbol: 'none',
+ data: markerLines.map(value => ({
+ xAxis: value,
+ label: {
+ formatter: () =>
+ markerLabelOf(value, markerLineLabels, markerLines),
+ color: theme.colorTextSecondary,
+ },
+ lineStyle: {
+ color: theme.colorText,
+ type: 'solid',
+ width: 2,
+ },
+ })),
+ },
+ },
+ {
+ name: 'markers',
+ type: 'scatter',
+ data: markers.map(value => ({
+ value: [value, 0],
+ tooltip: {
+ formatter: () =>
+ `${markerLabelOf(value, markerLabels, markers)}: <b>${formatter(
+ value,
+ )}</b>`,
+ },
Review Comment:
**Suggestion:** The marker tooltip formatter injects label text into HTML without sanitization, so user-provided marker labels can introduce XSS payloads. Escape or sanitize marker label text before building the tooltip string. [security]
<details>
<summary><b>Severity Level:</b> Critical π¨</summary>
```mdx
- β Marker tooltips allow stored XSS via marker labels.
- β οΈ Compromised charts can attack all viewers on hover.
```
</details>
<details>
<summary><b>Steps of Reproduction β
</b></summary>
```mdx
1. A Bullet chart with markers is viewed, so Superset builds `chartProps` and calls
`transformProps()` at
`superset-frontend/plugins/plugin-chart-echarts/src/Bullet/transformProps.ts:31`.
2. `transformProps()` reads `formData.markers` and `formData.markerLabels` (lines 55β56)
and tokenizes them into numeric `markers` and string `markerLabels`, both directly
controlled by the chart author via the comma-separated controls mentioned in the PR
description.
3. For the marker scatter series, `data` is constructed at lines 162β170; each marker
assigns a `tooltip.formatter` that returns ``${markerLabelOf(value, markerLabels,
markers)}: <b>${formatter(value)}</b>`` (lines 164β168), interpolating the marker label as
raw HTML.
4. When a viewer hovers a marker, ECharts renders the tooltip using HTML and injects this
string into the tooltip DOM; any HTML/JS placed in a marker label (e.g., a script tag or
onerror handler) will be executed in the viewerβs browser as stored XSS.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=40177c04d3104145beaee0376ca42677&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=40177c04d3104145beaee0376ca42677&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/src/Bullet/transformProps.ts
**Line:** 164:169
**Comment:**
*Security: The marker tooltip formatter injects label text into HTML without sanitization, so user-provided marker labels can introduce XSS payloads. Escape or sanitize marker label text before building the tooltip string.
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%2F42225&comment_hash=08c84205f12accbce0b42808fa37f2b44105fe8f20d5004f65f7cd7c26c2ae22&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42225&comment_hash=08c84205f12accbce0b42808fa37f2b44105fe8f20d5004f65f7cd7c26c2ae22&reaction=dislike'>π</a>
##########
superset-frontend/plugins/plugin-chart-echarts/src/Bullet/transformProps.ts:
##########
@@ -0,0 +1,191 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import {
+ getMetricLabel,
+ getNumberFormatter,
+ JsonObject,
+} from '@superset-ui/core';
+import type { EChartsCoreOption } from 'echarts/core';
+import { Refs } from '../types';
+import { EchartsBulletChartProps, BulletChartTransformedProps } from './types';
+import { tokenizeToNumericArray, tokenizeToStringArray } from './utils';
+
+const MEASURE_BAR_FRACTION = 0.28;
+
+export default function transformProps(
+ chartProps: EchartsBulletChartProps,
+): BulletChartTransformedProps {
+ const { width, height, formData, queriesData, theme, hooks } = chartProps;
+ const {
+ metric,
+ ranges: rawRanges,
+ rangeLabels: rawRangeLabels,
+ markers: rawMarkers,
+ markerLabels: rawMarkerLabels,
+ markerLines: rawMarkerLines,
+ markerLineLabels: rawMarkerLineLabels,
+ yAxisFormat,
+ } = formData;
+ const refs: Refs = {};
+ const { onContextMenu, setDataMask = () => {} } = hooks;
+
+ const metricLabel = getMetricLabel(metric ?? '');
+ const records = (queriesData[0]?.data ?? []) as JsonObject[];
+ const measure = Number(records[0]?.[metricLabel] ?? 0);
+
+ // Match the legacy default: a single band up to 110% of the measure.
+ const ranges = tokenizeToNumericArray(rawRanges) ?? [0, measure * 1.1];
+ const rangeLabels = tokenizeToStringArray(rawRangeLabels) ?? [];
+ const markers = tokenizeToNumericArray(rawMarkers) ?? [];
+ const markerLabels = tokenizeToStringArray(rawMarkerLabels) ?? [];
+ const markerLines = tokenizeToNumericArray(rawMarkerLines) ?? [];
+ const markerLineLabels = tokenizeToStringArray(rawMarkerLineLabels) ?? [];
+
+ const formatter = getNumberFormatter(yAxisFormat);
+
+ const axisMax = Math.max(measure, ...ranges, ...markers, ...markerLines);
+ const axisMin = Math.min(0, measure, ...ranges, ...markers, ...markerLines);
+
+ // Nested qualitative bands like d3-bullet: draw from the largest threshold
+ // down so smaller (darker) bands paint on top.
+ const bandFills = [
+ theme.colorFillQuaternary,
+ theme.colorFillTertiary,
+ theme.colorFillSecondary,
+ theme.colorFill,
+ ];
+ const sortedRanges = [...ranges]
+ .map((value, i) => ({ value, label: rangeLabels[i] }))
+ .filter(({ value }) => value > axisMin)
+ .sort((a, b) => b.value - a.value);
+ const bands = sortedRanges.map(({ value, label }, i) => ({
+ name: label,
+ itemStyle: {
+ color: bandFills[Math.min(i, bandFills.length - 1)],
+ },
+ label: { show: false },
+ // markArea item: [{start}, {end}]
+ coords: [{ xAxis: axisMin }, { xAxis: value }],
+ }));
+
+ const markerLabelOf = (value: number, labels: string[], values: number[]) => {
+ const index = values.indexOf(value);
+ return index >= 0 && labels[index] ? labels[index] : formatter(value);
+ };
+
+ const echartOptions: EChartsCoreOption = {
+ grid: {
+ top: theme.sizeUnit * 2,
+ bottom: theme.sizeUnit * 6,
+ left: theme.sizeUnit * 2,
+ right: theme.sizeUnit * 2,
+ containLabel: true,
+ },
+ xAxis: {
+ type: 'value',
+ min: axisMin,
+ max: axisMax,
+ axisLabel: {
+ color: theme.colorTextSecondary,
+ formatter: (value: number) => formatter(value),
+ },
+ splitLine: { show: false },
+ },
+ yAxis: {
+ type: 'category',
+ data: [''],
+ axisLine: { show: false },
+ axisTick: { show: false },
+ axisLabel: { show: false },
+ },
+ tooltip: {
+ confine: true,
+ formatter: () => `${metricLabel}: <b>${formatter(measure)}</b>`,
+ },
Review Comment:
**Suggestion:** The tooltip HTML interpolates `metricLabel` directly into an HTML string without sanitization, which can allow stored XSS if a metric label contains HTML/JS payloads. Sanitize or escape dynamic label content before returning tooltip HTML. [security]
<details>
<summary><b>Severity Level:</b> Critical π¨</summary>
```mdx
- β Bullet chart tooltip enables stored XSS via metric labels.
- β οΈ Viewers of malicious charts can run attacker JavaScript.
```
</details>
<details>
<summary><b>Steps of Reproduction β
</b></summary>
```mdx
1. A Bullet chart is viewed in Superset, causing the chart engine to build `chartProps`
and call `transformProps()` at
`superset-frontend/plugins/plugin-chart-echarts/src/Bullet/transformProps.ts:31`.
2. `transformProps()` reads `formData.metric` (line 35) and derives a display label via
`getMetricLabel(metric ?? '')` into `metricLabel` at line 48; this label is
user-configurable in the chartβs metric settings.
3. At lines 117β120, the `tooltip.formatter` returns the string ``${metricLabel}:
<b>${formatter(measure)}</b>`` which is an HTML snippet that interpolates `metricLabel`
directly without escaping.
4. EChartsβ tooltip component uses HTML rendering by default; when a user hovers the
measure bar, this string is injected into the tooltip DOM as innerHTML, so any HTML/JS
embedded in the metric label (e.g., an <img onerror> payload) will execute in the viewerβs
browser.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=e09de1a0a7fd4c918eac2f909a1a53f7&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=e09de1a0a7fd4c918eac2f909a1a53f7&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/src/Bullet/transformProps.ts
**Line:** 117:120
**Comment:**
*Security: The tooltip HTML interpolates `metricLabel` directly into an HTML string without sanitization, which can allow stored XSS if a metric label contains HTML/JS payloads. Sanitize or escape dynamic label content before returning tooltip HTML.
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%2F42225&comment_hash=6f69b4600788a25d0e0a5e6a7f81617431ec8a94efaea5ce366f0d67056d1eb7&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42225&comment_hash=6f69b4600788a25d0e0a5e6a7f81617431ec8a94efaea5ce366f0d67056d1eb7&reaction=dislike'>π</a>
##########
superset-frontend/plugins/plugin-chart-echarts/src/Bullet/types.ts:
##########
@@ -0,0 +1,45 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { QueryFormData, QueryFormMetric } from '@superset-ui/core';
+import {
+ BaseChartProps,
+ BaseTransformedProps,
+ CrossFilterTransformedProps,
+} from '../types';
+
+export type EchartsBulletFormData = QueryFormData & {
+ metric?: QueryFormMetric;
+ /** Comma-separated threshold values shaded as background bands */
+ ranges?: string;
+ rangeLabels?: string;
+ /** Comma-separated values drawn as point markers */
+ markers?: string;
+ markerLabels?: string;
+ /** Comma-separated values drawn as vertical lines */
+ markerLines?: string;
+ markerLineLabels?: string;
+ yAxisFormat?: string;
+};
Review Comment:
**Suggestion:** The form-data type uses camelCase fields for controls that are defined in snake_case (`range_labels`, `marker_labels`, `marker_lines`, `marker_line_labels`, `y_axis_format`). This creates a contract mismatch that can drop values as `undefined` in any path that consumes raw form data (including saved legacy charts), so align the type with actual control keys or explicitly support both shapes at the boundary where form data is normalized. [logic error]
<details>
<summary><b>Severity Level:</b> Critical π¨</summary>
```mdx
- β Bullet charts lose configured range and marker labels.
- β Bullet marker lines not rendered from legacy configs.
- β οΈ y_axis_format control has no visible effect.
- β οΈ Saved Bullet charts render differently than before.
```
</details>
<details>
<summary><b>Steps of Reproduction β
</b></summary>
```mdx
1. In the Superset UI, create or open a Bullet chart (viz_type `bullet`) that uses the
legacy controls `range_labels`, `marker_labels`, `marker_lines`, `marker_line_labels`, and
`y_axis_format` defined in the Bullet control panel (controls follow the snake_case names
described in the PR text).
2. When the chart is rendered on the frontend, the `Bullet` plugin constructs chart props
typed as `EchartsBulletChartProps` (importing `EchartsBulletFormData` from
`superset-frontend/plugins/plugin-chart-echarts/src/Bullet/types.ts:26-38`), so downstream
code treats `formData` as having camelCase fields `rangeLabels`, `markerLabels`,
`markerLines`, `markerLineLabels`, and `yAxisFormat`.
3. At runtime, the actual `formData` object coming from the backend still uses the legacy
snake_case keys (`range_labels`, `marker_labels`, `marker_lines`, `marker_line_labels`,
`y_axis_format`), causing any destructuring or property access that relies on the
TypeScript shape (`formData.rangeLabels`, `formData.markerLines`, etc.) to read
`undefined` instead of the real values.
4. Observe in the rendered Bullet chart that qualitative range labels, marker labels,
marker lines, and the configured y-axis number format are missing or defaulted, even
though the corresponding controls are populated, demonstrating that the contract mismatch
in `EchartsBulletFormData` (types.ts:26-38) causes values from saved configs to be
dropped.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=04e39d7f3b2040638b992e2b1b7d809f&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=04e39d7f3b2040638b992e2b1b7d809f&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/src/Bullet/types.ts
**Line:** 26:38
**Comment:**
*Logic Error: The form-data type uses camelCase fields for controls that are defined in snake_case (`range_labels`, `marker_labels`, `marker_lines`, `marker_line_labels`, `y_axis_format`). This creates a contract mismatch that can drop values as `undefined` in any path that consumes raw form data (including saved legacy charts), so align the type with actual control keys or explicitly support both shapes at the boundary where form data is normalized.
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%2F42225&comment_hash=d67d281ea9a4e5cdf17a4ed6d6d0b6b252ee8c1f5c707d95ca42816efedbfced&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42225&comment_hash=d67d281ea9a4e5cdf17a4ed6d6d0b6b252ee8c1f5c707d95ca42816efedbfced&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
Re: [PR] feat(bullet): rebuild the Bullet Chart on ECharts [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #42225:
URL: https://github.com/apache/superset/pull/42225#issuecomment-5027952374
πͺ [Showtime](https://github.com/mistercrunch/superset-showtime) is building environment on [GHA](https://github.com/apache/superset/actions/runs/29784580015) for [fbf2126](https://github.com/apache/superset/commit/fbf2126)
--
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
Re: [PR] feat(bullet): rebuild the Bullet Chart on ECharts [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #42225:
URL: https://github.com/apache/superset/pull/42225#issuecomment-5028683008
πͺ [Showtime](https://github.com/mistercrunch/superset-showtime) is building environment on [GHA](https://github.com/apache/superset/actions/runs/29789581169) for [38d59fb](https://github.com/apache/superset/commit/38d59fb)
--
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
Re: [PR] feat(bullet): rebuild the Bullet Chart on ECharts [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #42225:
URL: https://github.com/apache/superset/pull/42225#issuecomment-5025397682
πͺ [Showtime](https://github.com/mistercrunch/superset-showtime) is building environment on [GHA](https://github.com/apache/superset/actions/runs/29765238173) for [7021004](https://github.com/apache/superset/commit/7021004)
--
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
Re: [PR] feat(bullet): rebuild the Bullet Chart on ECharts [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #42225:
URL: https://github.com/apache/superset/pull/42225#discussion_r3615626877
##########
superset-frontend/plugins/plugin-chart-echarts/src/Bullet/buildQuery.ts:
##########
@@ -0,0 +1,34 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { buildQueryContext, QueryFormData } from '@superset-ui/core';
+
+/**
+ * A single ungrouped aggregate of the selected metric, mirroring the query
+ * the legacy nvd3 bullet chart issued.
+ */
+export default function buildQuery(formData: QueryFormData) {
+ const { metric } = formData;
+ return buildQueryContext(formData, baseQueryObject => [
+ {
+ ...baseQueryObject,
+ columns: [],
+ metrics: metric ? [metric] : [],
Review Comment:
**Suggestion:** When `metric` is missing, this builds a query with both `columns` and `metrics` empty, which backend query validation can reject and return a 400 instead of rendering safely. Make `metric` required for this chart or short-circuit with a controlled empty-state path before issuing the query. [logic error]
<details>
<summary><b>Severity Level:</b> Major β οΈ</summary>
```mdx
- β Bullet chart request fails when metric control unset.
- β οΈ Users see backend error instead of empty visualization.
```
</details>
<details>
<summary><b>Steps of Reproduction β
</b></summary>
```mdx
1. Open the Explore UI and select the Bullet chart type (`viz_type: 'bullet'`), which is
registered via `EchartsBulletChartPlugin` in
`superset-frontend/src/visualizations/presets/MainPreset.ts:88-89` and wired to this
plugin's `buildQuery` function in
`superset-frontend/plugins/plugin-chart-echarts/src/Bullet/index.ts` (per repo graph).
2. In the Bullet control panel
(`superset-frontend/plugins/plugin-chart-echarts/src/Bullet/controlPanel.tsx`), leave the
metric control empty so that the `metric` field in the `QueryFormData` passed into the
plugin is `undefined`.
3. Click "Run" so the frontend constructs `formData` and calls the Bullet plugin's
`buildQuery(formData)` function defined in
`superset-frontend/plugins/plugin-chart-echarts/src/Bullet/buildQuery.ts:25-33`.
4. Inside `buildQuery`, line 31 sets `metrics: metric ? [metric] : []`, so with no metric
selected the generated query object has `columns: []` and `metrics: []`; when this reaches
the backend query processor (which requires at least one metric or column) the request is
rejected with a validation error (HTTP 400), and the Bullet chart fails to render instead
of showing a controlled empty state.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=1c11d40c2aa4472499fdeed3d4f734da&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=1c11d40c2aa4472499fdeed3d4f734da&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/src/Bullet/buildQuery.ts
**Line:** 31:31
**Comment:**
*Logic Error: When `metric` is missing, this builds a query with both `columns` and `metrics` empty, which backend query validation can reject and return a 400 instead of rendering safely. Make `metric` required for this chart or short-circuit with a controlled empty-state path before issuing the query.
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%2F42225&comment_hash=f6ca0ef4b4efa7bd126b5e2cff974117c7686d90221039c6da236d9233023602&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42225&comment_hash=f6ca0ef4b4efa7bd126b5e2cff974117c7686d90221039c6da236d9233023602&reaction=dislike'>π</a>
##########
superset-frontend/plugins/plugin-chart-echarts/src/Bullet/transformProps.ts:
##########
@@ -0,0 +1,196 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import {
+ getMetricLabel,
+ getNumberFormatter,
+ JsonObject,
+ sanitizeHtml,
+} from '@superset-ui/core';
+import type { EChartsCoreOption } from 'echarts/core';
+import { Refs } from '../types';
+import { EchartsBulletChartProps, BulletChartTransformedProps } from './types';
+import { tokenizeToNumericArray, tokenizeToStringArray } from './utils';
+
+const MEASURE_BAR_FRACTION = 0.28;
+
+export default function transformProps(
+ chartProps: EchartsBulletChartProps,
+): BulletChartTransformedProps {
+ const { width, height, formData, queriesData, theme, hooks } = chartProps;
+ const {
+ metric,
+ ranges: rawRanges,
+ rangeLabels: rawRangeLabels,
+ markers: rawMarkers,
+ markerLabels: rawMarkerLabels,
+ markerLines: rawMarkerLines,
+ markerLineLabels: rawMarkerLineLabels,
+ yAxisFormat,
+ } = formData;
+ const refs: Refs = {};
+ const { onContextMenu, setDataMask = () => {} } = hooks;
+
+ const metricLabel = getMetricLabel(metric ?? '');
+ const records = (queriesData[0]?.data ?? []) as JsonObject[];
+ const measure = Number(records[0]?.[metricLabel] ?? 0);
+
+ // Match the legacy default: a single band up to 110% of the measure.
+ const ranges = tokenizeToNumericArray(rawRanges) ?? [0, measure * 1.1];
+ const rangeLabels = tokenizeToStringArray(rawRangeLabels) ?? [];
+ const markers = tokenizeToNumericArray(rawMarkers) ?? [];
+ const markerLabels = tokenizeToStringArray(rawMarkerLabels) ?? [];
+ const markerLines = tokenizeToNumericArray(rawMarkerLines) ?? [];
+ const markerLineLabels = tokenizeToStringArray(rawMarkerLineLabels) ?? [];
+
+ const formatter = getNumberFormatter(yAxisFormat);
+
+ const axisMax = Math.max(measure, ...ranges, ...markers, ...markerLines);
+ const axisMin = Math.min(0, measure, ...ranges, ...markers, ...markerLines);
Review Comment:
**Suggestion:** If all plotted values are identical (for example empty results producing `0`), `axisMin` and `axisMax` become equal, which creates a zero-width x-axis domain and can make the bullet chart render incorrectly or appear blank. Add a guard to expand the domain when min and max are the same. [logic error]
<details>
<summary><b>Severity Level:</b> Major β οΈ</summary>
```mdx
- β Bullet chart with empty result renders with degenerate x-axis.
- β οΈ Users see blank chart despite valid bullet configuration.
```
</details>
<details>
<summary><b>Steps of Reproduction β
</b></summary>
```mdx
1. In a unit test or runtime, call `transformProps` from
`superset-frontend/plugins/plugin-chart-echarts/src/Bullet/transformProps.ts:32-35` with
`queriesData` set to `[ { data: [] } ]` so that `records` becomes an empty array at line
50.
2. Pass `formData.metric` to match an existing metric label, but leave `formData.ranges`,
`formData.markers`, and `formData.markerLines` `undefined` so `rawRanges`, `rawMarkers`,
and `rawMarkerLines` are `undefined` at lines 38β43.
3. Inside `transformProps`, observe at line 51 that `measure` is computed as
`Number(records[0]?.[metricLabel] ?? 0)`, which becomes `0` when `records[0]` is
`undefined` due to the empty result set.
4. At lines 53β59 and 63β64, `ranges` falls back to `[0, measure * 1.1]` which becomes
`[0, 0]`, while `markers` and `markerLines` fall back to empty arrays; `axisMax` and
`axisMin` are both computed as `0`, producing an x-axis in `echartOptions.xAxis` (lines
102β111) with `min = 0` and `max = 0`, resulting in a zero-length axis domain and a
degenerate or blank bullet chart when rendered.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=c62e64b388924eb782a0ca0d0823cb72&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=c62e64b388924eb782a0ca0d0823cb72&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/src/Bullet/transformProps.ts
**Line:** 63:64
**Comment:**
*Logic Error: If all plotted values are identical (for example empty results producing `0`), `axisMin` and `axisMax` become equal, which creates a zero-width x-axis domain and can make the bullet chart render incorrectly or appear blank. Add a guard to expand the domain when min and max are the same.
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%2F42225&comment_hash=f7463b2db24d0d4ed144a9ddff950780ba2edde46504f983e281e4eb4deee371&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42225&comment_hash=f7463b2db24d0d4ed144a9ddff950780ba2edde46504f983e281e4eb4deee371&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
Re: [PR] feat(bullet): rebuild the Bullet Chart on ECharts [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42225:
URL: https://github.com/apache/superset/pull/42225#issuecomment-5019472599
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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
Re: [PR] feat(bullet): rebuild the Bullet Chart on ECharts [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #42225:
URL: https://github.com/apache/superset/pull/42225#issuecomment-5028694023
πͺ [Showtime](https://github.com/mistercrunch/superset-showtime) is building environment on [GHA](https://github.com/apache/superset/actions/runs/29789670256) for [26a72de](https://github.com/apache/superset/commit/26a72de)
--
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
Re: [PR] feat(bullet): rebuild the Bullet Chart on ECharts [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #42225:
URL: https://github.com/apache/superset/pull/42225#issuecomment-5028736219
πͺ [Showtime](https://github.com/mistercrunch/superset-showtime) is building environment on [GHA](https://github.com/apache/superset/actions/runs/29790010038) for [26340c0](https://github.com/apache/superset/commit/26340c0)
--
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
Re: [PR] feat(bullet): rebuild the Bullet Chart on ECharts [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #42225:
URL: https://github.com/apache/superset/pull/42225#discussion_r3618098812
##########
superset-frontend/plugins/plugin-chart-echarts/src/Bullet/EchartsBullet.tsx:
##########
@@ -0,0 +1,38 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import Echart from '../components/Echart';
+import { allEventHandlers } from '../utils/eventHandlers';
+import { BulletChartTransformedProps } from './types';
+
+export default function EchartsBullet(props: BulletChartTransformedProps) {
+ const { height, width, echartOptions, refs, formData } = props;
+
+ const eventHandlers = allEventHandlers(props);
+
+ return (
+ <Echart
+ refs={refs}
+ height={height}
+ width={width}
+ echartOptions={echartOptions}
+ eventHandlers={eventHandlers}
+ vizType={formData.vizType}
Review Comment:
`formData.vizType` is correct here β that'\''s the camelCase field on the client-side `FormData` type, and every other ECharts plugin (Timeseries, Pie, BoxPlot, etc.) passes it into `Echart` the same way.
--
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
Re: [PR] feat(bullet): rebuild the Bullet Chart on ECharts [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #42225:
URL: https://github.com/apache/superset/pull/42225#issuecomment-5028551117
πͺ [Showtime](https://github.com/mistercrunch/superset-showtime) deployed environment on [GHA](https://github.com/apache/superset/actions/runs/29787705462) for [684a97e](https://github.com/apache/superset/commit/684a97e)
β’ **Environment:** http://18.237.87.147:8080 (admin/admin)
β’ **Lifetime:** 48h auto-cleanup
β’ **Updates:** New commits create fresh environments automatically
--
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
Re: [PR] feat(bullet): rebuild the Bullet Chart on ECharts [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #42225:
URL: https://github.com/apache/superset/pull/42225#discussion_r3612688335
##########
superset-frontend/plugins/plugin-chart-echarts/test/Bullet/transformProps.test.ts:
##########
@@ -0,0 +1,99 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { ChartProps, SqlaFormData, VizType } from '@superset-ui/core';
+import { supersetTheme } from '@apache-superset/core/theme';
+import transformProps from '../../src/Bullet/transformProps';
+import { EchartsBulletChartProps } from '../../src/Bullet/types';
+
+const formData: SqlaFormData = {
+ datasource: '5__table',
+ viz_type: VizType.Bullet,
+ metric: 'sum__num',
+ ranges: '100,200,300',
+ rangeLabels: 'low,mid,high',
+ markers: '150',
+ markerLabels: 'goal',
+ markerLines: '250',
+ markerLineLabels: 'stretch',
+};
+
+const chartProps = (overrides: Partial<SqlaFormData> = {}) =>
+ new ChartProps({
+ width: 800,
+ height: 200,
+ formData: { ...formData, ...overrides },
+ theme: supersetTheme,
+ queriesData: [{ data: [{ sum__num: 120 }] }],
+ hooks: {},
+ }) as unknown as EchartsBulletChartProps;
+
+test('renders the measure bar with markers, marker lines and range bands', () => {
+ const { echartOptions } = transformProps(chartProps());
+ const series = (echartOptions as any).series;
Review Comment:
β
**Customized review instruction saved!**
**Instruction:**
> Avoid flagging `as any` casts used to inspect ECharts options in test-only `transformProps` assertions, where narrowing the `EChartsCoreOption` series union is impractical.
**Applied to:**
- `**/test/**`
- `**/tests/**`
- `**/*test*.ts`
---
π‘ *To manage or update this instruction, visit: [CodeAnt AI Settings](https://app.codeant.ai/org/settings/learnings)*
##########
superset-frontend/plugins/plugin-chart-echarts/test/Bullet/transformProps.test.ts:
##########
@@ -0,0 +1,99 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { ChartProps, SqlaFormData, VizType } from '@superset-ui/core';
+import { supersetTheme } from '@apache-superset/core/theme';
+import transformProps from '../../src/Bullet/transformProps';
+import { EchartsBulletChartProps } from '../../src/Bullet/types';
+
+const formData: SqlaFormData = {
+ datasource: '5__table',
+ viz_type: VizType.Bullet,
+ metric: 'sum__num',
+ ranges: '100,200,300',
+ rangeLabels: 'low,mid,high',
+ markers: '150',
+ markerLabels: 'goal',
+ markerLines: '250',
+ markerLineLabels: 'stretch',
+};
+
+const chartProps = (overrides: Partial<SqlaFormData> = {}) =>
+ new ChartProps({
+ width: 800,
+ height: 200,
+ formData: { ...formData, ...overrides },
+ theme: supersetTheme,
+ queriesData: [{ data: [{ sum__num: 120 }] }],
+ hooks: {},
+ }) as unknown as EchartsBulletChartProps;
+
+test('renders the measure bar with markers, marker lines and range bands', () => {
+ const { echartOptions } = transformProps(chartProps());
+ const series = (echartOptions as any).series;
+
+ // measure bar
+ expect(series[0].type).toBe('bar');
+ expect(series[0].data).toEqual([120]);
+
+ // marker lines on the bar series
+ expect(series[0].markLine.data).toHaveLength(1);
+ expect(series[0].markLine.data[0].xAxis).toBe(250);
+
+ // nested range bands, largest drawn first
+ const bandEnds = series[0].markArea.data.map((d: any) => d[1].xAxis);
Review Comment:
β
**Customized review instruction saved!**
**Instruction:**
> Do not flag `any` usage in test files; this package follows an existing test convention that allows `any` in tests for brevity.
**Applied to:**
- `**/test/**`
- `**/tests/**`
- `**/*test*.ts`
---
π‘ *To manage or update this instruction, visit: [CodeAnt AI Settings](https://app.codeant.ai/org/settings/learnings)*
##########
superset-frontend/plugins/plugin-chart-echarts/test/Bullet/transformProps.test.ts:
##########
@@ -0,0 +1,99 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { ChartProps, SqlaFormData, VizType } from '@superset-ui/core';
+import { supersetTheme } from '@apache-superset/core/theme';
+import transformProps from '../../src/Bullet/transformProps';
+import { EchartsBulletChartProps } from '../../src/Bullet/types';
+
+const formData: SqlaFormData = {
+ datasource: '5__table',
+ viz_type: VizType.Bullet,
+ metric: 'sum__num',
+ ranges: '100,200,300',
+ rangeLabels: 'low,mid,high',
+ markers: '150',
+ markerLabels: 'goal',
+ markerLines: '250',
+ markerLineLabels: 'stretch',
+};
+
+const chartProps = (overrides: Partial<SqlaFormData> = {}) =>
+ new ChartProps({
+ width: 800,
+ height: 200,
+ formData: { ...formData, ...overrides },
+ theme: supersetTheme,
+ queriesData: [{ data: [{ sum__num: 120 }] }],
+ hooks: {},
+ }) as unknown as EchartsBulletChartProps;
+
+test('renders the measure bar with markers, marker lines and range bands', () => {
+ const { echartOptions } = transformProps(chartProps());
+ const series = (echartOptions as any).series;
+
+ // measure bar
+ expect(series[0].type).toBe('bar');
+ expect(series[0].data).toEqual([120]);
+
+ // marker lines on the bar series
+ expect(series[0].markLine.data).toHaveLength(1);
+ expect(series[0].markLine.data[0].xAxis).toBe(250);
+
+ // nested range bands, largest drawn first
+ const bandEnds = series[0].markArea.data.map((d: any) => d[1].xAxis);
+ expect(bandEnds).toEqual([300, 200, 100]);
+
+ // triangle markers
+ expect(series[1].type).toBe('scatter');
+ expect(series[1].data[0].value).toEqual([150, 0]);
+
+ // axis spans all values
+ expect((echartOptions as any).xAxis.max).toBe(300);
Review Comment:
β
**Customized review instruction saved!**
**Instruction:**
> Do not flag `any` casts in test files; they follow the package's existing test convention.
**Applied to:**
- `**/test/**`
- `**/tests/**`
- `**/*test*.ts`
---
π‘ *To manage or update this instruction, visit: [CodeAnt AI Settings](https://app.codeant.ai/org/settings/learnings)*
--
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
Re: [PR] feat(bullet): rebuild the Bullet Chart on ECharts [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #42225:
URL: https://github.com/apache/superset/pull/42225#discussion_r3612688673
##########
superset-frontend/plugins/plugin-chart-echarts/src/Bullet/types.ts:
##########
@@ -0,0 +1,45 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { QueryFormData, QueryFormMetric } from '@superset-ui/core';
+import {
+ BaseChartProps,
+ BaseTransformedProps,
+ CrossFilterTransformedProps,
+} from '../types';
+
+export type EchartsBulletFormData = QueryFormData & {
+ metric?: QueryFormMetric;
+ /** Comma-separated threshold values shaded as background bands */
+ ranges?: string;
+ rangeLabels?: string;
+ /** Comma-separated values drawn as point markers */
+ markers?: string;
+ markerLabels?: string;
+ /** Comma-separated values drawn as vertical lines */
+ markerLines?: string;
+ markerLineLabels?: string;
+ yAxisFormat?: string;
+};
Review Comment:
Not a mismatch β `ChartProps` runs `formData` through `convertKeysToCamelCase()` before handing it to `transformProps` (packages/superset-ui-core/src/chart/models/ChartProps.ts), so the snake_case control names (`range_labels`, `marker_labels`, etc.) land as `rangeLabels`/`markerLabels`/... by the time this code sees them. Same convention Histogram and other echarts plugins in this package rely on for `y_axis_format` -> `yAxisFormat`.
--
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
Re: [PR] feat(bullet): rebuild the Bullet Chart on ECharts [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #42225:
URL: https://github.com/apache/superset/pull/42225#issuecomment-5028948187
πͺ [Showtime](https://github.com/mistercrunch/superset-showtime) deployed environment on [GHA](https://github.com/apache/superset/actions/runs/29790674925) for [caa31e3](https://github.com/apache/superset/commit/caa31e3)
β’ **Environment:** http://35.90.254.226:8080 (admin/admin)
β’ **Lifetime:** 48h auto-cleanup
β’ **Updates:** New commits create fresh environments automatically
--
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