msyavuz commented on code in PR #37669:
URL: https://github.com/apache/superset/pull/37669#discussion_r2773261560
##########
superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Scatter/buildQuery.ts:
##########
@@ -82,26 +86,41 @@ export default function buildQuery(formData:
DeckScatterFormData) {
// Only add metric if point_radius_fixed is a metric type
const isMetric = isMetricValue(point_radius_fixed);
- const metricValue = isMetric
- ? extractMetricKey(point_radius_fixed?.value)
- : null;
+ // Extract metric value, handling both legacy string format and object
format
+ let metricValue: QueryFormMetric | null = null;
+ if (isMetric) {
+ if (typeof point_radius_fixed === 'string') {
+ // Legacy string format: treat the string itself as the metric
+ metricValue = point_radius_fixed;
+ } else if (
+ point_radius_fixed?.value !== undefined &&
+ typeof point_radius_fixed.value !== 'number'
+ ) {
+ // Metric object format: use the metric value (string or adhoc
metric object)
+ metricValue = point_radius_fixed.value as QueryFormMetric;
+ }
+ }
// Preserve existing metrics and only add radius metric if it's
metric-based
const existingMetrics = baseQueryObject.metrics || [];
- const radiusMetrics = processMetricsArray(
- metricValue ? [metricValue] : [],
+ // Deduplicate metrics using getMetricLabel for comparison
+ const existingLabels = new Set(
+ existingMetrics.map(m => getMetricLabel(m)),
);
- // Deduplicate metrics to avoid adding the same metric twice
- const metricsSet = new Set([...existingMetrics, ...radiusMetrics]);
- const metrics = Array.from(metricsSet);
+ const metrics: QueryFormMetric[] =
+ metricValue && !existingLabels.has(getMetricLabel(metricValue))
+ ? [...existingMetrics, metricValue]
+ : [...existingMetrics];
Review Comment:
```suggestion
: existingMetrics;
```
##########
superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Scatter/buildQuery.ts:
##########
@@ -82,26 +86,41 @@ export default function buildQuery(formData:
DeckScatterFormData) {
// Only add metric if point_radius_fixed is a metric type
const isMetric = isMetricValue(point_radius_fixed);
- const metricValue = isMetric
- ? extractMetricKey(point_radius_fixed?.value)
- : null;
+ // Extract metric value, handling both legacy string format and object
format
+ let metricValue: QueryFormMetric | null = null;
+ if (isMetric) {
+ if (typeof point_radius_fixed === 'string') {
+ // Legacy string format: treat the string itself as the metric
+ metricValue = point_radius_fixed;
+ } else if (
+ point_radius_fixed?.value !== undefined &&
+ typeof point_radius_fixed.value !== 'number'
+ ) {
+ // Metric object format: use the metric value (string or adhoc
metric object)
+ metricValue = point_radius_fixed.value as QueryFormMetric;
+ }
+ }
Review Comment:
Maybe we can flatten this a bit more? It's just a nit though
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
