Skip to content

Commit 19cde09

Browse files
LevisNgigiLevisNgigi
committed
use common util
1 parent b29b00a commit 19cde09

File tree

5 files changed

+301
-32
lines changed

5 files changed

+301
-32
lines changed

‎superset-frontend/packages/superset-ui-core/src/utils/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,3 +33,4 @@ export * from './random';
3333
export * from './typedMemo';
3434
export * from './html';
3535
export * from './tooltip';
36+
export * from './metricColumnFilter';

‎superset-frontend/packages/superset-ui-core/src/utils/metricColumnFilter.test.ts‎

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
/**
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
import { QueryFormMetric, SqlaFormData } from '@superset-ui/core';
21+
import {
22+
shouldSkipMetricColumn,
23+
isRegularMetric,
24+
isPercentMetric,
25+
isMetric,
26+
} from './metricColumnFilter';
27+
28+
const createMetric = (label: string): QueryFormMetric =>
29+
({
30+
label,
31+
expressionType: 'SIMPLE',
32+
column: { column_name: label },
33+
aggregate: 'SUM',
34+
}) as QueryFormMetric;
35+
36+
describe('metricColumnFilter', () => {
37+
const createFormData = (
38+
metrics: string[],
39+
percentMetrics: string[],
40+
): SqlaFormData =>
41+
({
42+
datasource: 'test_datasource',
43+
viz_type: 'table',
44+
metrics: metrics.map(createMetric),
45+
percent_metrics: percentMetrics.map(createMetric),
46+
}) as SqlaFormData;
47+
48+
describe('shouldSkipMetricColumn', () => {
49+
it('should skip unprefixed percent metric columns if prefixed version exists', () => {
50+
const colnames = ['metric1', '%metric1'];
51+
const formData = createFormData([], ['metric1']);
52+
53+
const result = shouldSkipMetricColumn({
54+
colname: 'metric1',
55+
colnames,
56+
formData,
57+
});
58+
59+
expect(result).toBe(true);
60+
});
61+
62+
it('should not skip if column is also a regular metric', () => {
63+
const colnames = ['metric1', '%metric1'];
64+
const formData = createFormData(['metric1'], ['metric1']);
65+
66+
const result = shouldSkipMetricColumn({
67+
colname: 'metric1',
68+
colnames,
69+
formData,
70+
});
71+
72+
expect(result).toBe(false);
73+
});
74+
75+
it('should not skip if column starts with %', () => {
76+
const colnames = ['%metric1'];
77+
const formData = createFormData(['metric1'], []);
78+
79+
const result = shouldSkipMetricColumn({
80+
colname: '%metric1',
81+
colnames,
82+
formData,
83+
});
84+
85+
expect(result).toBe(false);
86+
});
87+
88+
it('should not skip if no prefixed version exists', () => {
89+
const colnames = ['metric1'];
90+
const formData = createFormData([], ['metric1']);
91+
92+
const result = shouldSkipMetricColumn({
93+
colname: 'metric1',
94+
colnames,
95+
formData,
96+
});
97+
98+
expect(result).toBe(false);
99+
});
100+
});
101+
102+
describe('isRegularMetric', () => {
103+
it('should return true for regular metrics', () => {
104+
const formData = createFormData(['metric1', 'metric2'], []);
105+
expect(isRegularMetric('metric1', formData)).toBe(true);
106+
expect(isRegularMetric('metric2', formData)).toBe(true);
107+
});
108+
109+
it('should return false for non-metrics', () => {
110+
const formData = createFormData(['metric1'], []);
111+
expect(isRegularMetric('non_metric', formData)).toBe(false);
112+
});
113+
114+
it('should return false for percentage metrics', () => {
115+
const formData = createFormData([], ['percent_metric1']);
116+
expect(isRegularMetric('percent_metric1', formData)).toBe(false);
117+
});
118+
});
119+
120+
describe('isPercentMetric', () => {
121+
it('should return true for percentage metrics', () => {
122+
const formData = createFormData([], ['percent_metric1']);
123+
expect(isPercentMetric('%percent_metric1', formData)).toBe(true);
124+
});
125+
126+
it('should return false for non-percentage metrics', () => {
127+
const formData = createFormData(['regular_metric'], []);
128+
expect(isPercentMetric('regular_metric', formData)).toBe(false);
129+
});
130+
131+
it('should return false for regular metrics', () => {
132+
const formData = createFormData(['metric1'], []);
133+
expect(isPercentMetric('metric1', formData)).toBe(false);
134+
});
135+
});
136+
137+
describe('isMetric', () => {
138+
it('should return true for regular metrics', () => {
139+
const formData = createFormData(['metric1'], []);
140+
expect(isMetric('metric1', formData)).toBe(true);
141+
});
142+
143+
it('should return true for percentage metrics', () => {
144+
const formData = createFormData([], ['percent_metric1']);
145+
expect(isMetric('%percent_metric1', formData)).toBe(true);
146+
});
147+
148+
it('should return false for non-metrics', () => {
149+
const formData = createFormData(['metric1'], []);
150+
expect(isMetric('non_metric', formData)).toBe(false);
151+
});
152+
153+
it('should handle mixed metrics and percentage metrics', () => {
154+
const formData = createFormData(['metric1'], ['percent_metric1']);
155+
expect(isMetric('metric1', formData)).toBe(true);
156+
expect(isMetric('%percent_metric1', formData)).toBe(true);
157+
expect(isMetric('non_metric', formData)).toBe(false);
158+
});
159+
});
160+
});

‎superset-frontend/packages/superset-ui-core/src/utils/metricColumnFilter.ts‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
/**
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
import {
21+
QueryFormMetric,
22+
getMetricLabel,
23+
SqlaFormData,
24+
} from '@superset-ui/core';
25+
26+
export interface MetricColumnFilterParams {
27+
colname: string;
28+
colnames: string[];
29+
formData: SqlaFormData;
30+
}
31+
32+
/**
33+
* Determines if a column should be skipped based on metric filtering logic.
34+
*
35+
* This function implements the logic to skip unprefixed percent metric columns
36+
* if a prefixed version exists, but doesn't skip if it's also a regular metric.
37+
*
38+
* @param params - The parameters for metric column filtering
39+
* @returns true if the column should be skipped, false otherwise
40+
*/
41+
export function shouldSkipMetricColumn({
42+
colname,
43+
colnames,
44+
formData,
45+
}: MetricColumnFilterParams): boolean {
46+
// Skip unprefixed percent metric columns if a prefixed version exists
47+
// But don't skip if it's also a regular metric
48+
return !!(
49+
colname &&
50+
!colname.startsWith('%') &&
51+
colnames.includes(`%${colname}`) &&
52+
formData.percent_metrics?.some(
53+
(metric: QueryFormMetric) => getMetricLabel(metric) === colname,
54+
) &&
55+
!formData.metrics?.some(
56+
(metric: QueryFormMetric) => getMetricLabel(metric) === colname,
57+
)
58+
);
59+
}
60+
61+
/**
62+
* Determines if a column is a regular metric.
63+
*
64+
* @param colname - The column name to check
65+
* @param formData - The form data containing metrics
66+
* @returns true if the column is a regular metric, false otherwise
67+
*/
68+
export function isRegularMetric(
69+
colname: string,
70+
formData: SqlaFormData,
71+
): boolean {
72+
return !!formData.metrics?.some(metric => getMetricLabel(metric) === colname);
73+
}
74+
75+
/**
76+
* Determines if a column is a percentage metric.
77+
*
78+
* @param colname: string,
79+
* @param formData - The form data containing percent_metrics
80+
* @returns true if the column is a percentage metric, false otherwise
81+
*/
82+
export function isPercentMetric(
83+
colname: string,
84+
formData: SqlaFormData,
85+
): boolean {
86+
return !!formData.percent_metrics?.some(
87+
(metric: QueryFormMetric) => `%${getMetricLabel(metric)}` === colname,
88+
);
89+
}
90+
91+
/**
92+
* Checks if a column is either a regular metric or percentage metric.
93+
*
94+
* @param colname - The column name to check
95+
* @param formData - The form data containing metrics and percent_metrics
96+
* @returns true if the column is a metric (regular or percentage), false otherwise
97+
*/
98+
export function isMetric(colname: string, formData: SqlaFormData): boolean {
99+
return (
100+
isRegularMetric(colname, formData) || isPercentMetric(colname, formData)
101+
);
102+
}

‎superset-frontend/plugins/plugin-chart-ag-grid-table/src/controlPanel.tsx‎

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -41,18 +41,19 @@ import {
4141
ensureIsArray,
4242
FeatureFlag,
4343
GenericDataType,
44-
getMetricLabel,
4544
isAdhocColumn,
4645
isFeatureEnabled,
4746
isPhysicalColumn,
4847
legacyValidateInteger,
4948
QueryFormColumn,
50-
QueryFormMetric,
5149
QueryMode,
5250
SMART_DATE_ID,
5351
t,
5452
validateMaxValue,
5553
validateServerPagination,
54+
shouldSkipMetricColumn,
55+
isRegularMetric,
56+
isPercentMetric,
5657
} from '@superset-ui/core';
5758

5859
import { isEmpty, last } from 'lodash';
@@ -533,14 +534,25 @@ const config: ControlPanelConfig = {
533534
)
534535
.forEach((colname, index) => {
535536
if (
536-
explore.form_data.metrics?.some(
537-
metric => getMetricLabel(metric) === colname,
538-
) ||
539-
explore.form_data.percent_metrics?.some(
540-
(metric: QueryFormMetric) =>
541-
getMetricLabel(metric) === colname,
542-
)
537+
shouldSkipMetricColumn({
538+
colname,
539+
colnames,
540+
formData: explore.form_data,
541+
})
543542
) {
543+
return;
544+
}
545+
546+
const isMetric = isRegularMetric(
547+
colname,
548+
explore.form_data,
549+
);
550+
const isPercentMetricValue = isPercentMetric(
551+
colname,
552+
explore.form_data,
553+
);
554+
555+
if (isMetric || isPercentMetricValue) {
544556
const comparisonColumns =
545557
generateComparisonColumns(colname);
546558
comparisonColumns.forEach((name, idx) => {

‎superset-frontend/plugins/plugin-chart-table/src/controlPanel.tsx‎

Lines changed: 17 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -40,17 +40,18 @@ import {
4040
import {
4141
ensureIsArray,
4242
GenericDataType,
43-
getMetricLabel,
4443
isAdhocColumn,
4544
isPhysicalColumn,
4645
legacyValidateInteger,
4746
QueryFormColumn,
48-
QueryFormMetric,
4947
QueryMode,
5048
SMART_DATE_ID,
5149
t,
5250
validateMaxValue,
5351
validateServerPagination,
52+
shouldSkipMetricColumn,
53+
isRegularMetric,
54+
isPercentMetric,
5455
} from '@superset-ui/core';
5556

5657
import { isEmpty, last } from 'lodash';
@@ -592,33 +593,26 @@ const config: ControlPanelConfig = {
592593
// Skip unprefixed percent metric columns if a prefixed version exists
593594
// But don't skip if it's also a regular metric
594595
if (
595-
colname &&
596-
!colname.startsWith('%') &&
597-
colnames.includes(`%${colname}`) &&
598-
explore.form_data.percent_metrics?.some(
599-
(metric: QueryFormMetric) =>
600-
getMetricLabel(metric) === colname,
601-
) &&
602-
!explore.form_data.metrics?.some(
603-
(metric: QueryFormMetric) =>
604-
getMetricLabel(metric) === colname,
605-
)
596+
shouldSkipMetricColumn({
597+
colname,
598+
colnames,
599+
formData: explore.form_data,
600+
})
606601
) {
607-
return; // skip this column
602+
return;
608603
}
609604

610-
// Check if column is a regular metric or percentage metric
611-
const isMetric = explore.form_data.metrics?.some(
612-
metric => getMetricLabel(metric) === colname,
605+
const isMetric = isRegularMetric(
606+
colname,
607+
explore.form_data,
608+
);
609+
const isPercentMetricValue = isPercentMetric(
610+
colname,
611+
explore.form_data,
613612
);
614-
const isPercentMetric =
615-
explore.form_data.percent_metrics?.some(
616-
(metric: QueryFormMetric) =>
617-
`%${getMetricLabel(metric)}` === colname,
618-
);
619613

620614
// Generate comparison columns for metrics (time comparison feature)
621-
if (isMetric || isPercentMetric) {
615+
if (isMetric || isPercentMetricValue) {
622616
const comparisonColumns =
623617
generateComparisonColumns(colname);
624618
comparisonColumns.forEach((name, idx) => {

0 commit comments

Comments
 (0)