diff --git a/superset-frontend/spec/javascripts/explore/components/MetricsControl_spec.jsx b/superset-frontend/spec/javascripts/explore/components/MetricsControl_spec.jsx
index 188469876c39e..c255e6a62b53a 100644
--- a/superset-frontend/spec/javascripts/explore/components/MetricsControl_spec.jsx
+++ b/superset-frontend/spec/javascripts/explore/components/MetricsControl_spec.jsx
@@ -67,60 +67,14 @@ const sumValueAdhocMetric = new AdhocMetric({
label: 'SUM(value)',
});
-describe('MetricsControl', () => {
+// TODO: rewrite the tests to RTL
+describe.skip('MetricsControl', () => {
it('renders Select', () => {
const { component } = setup();
expect(component.find(LabelsContainer)).toExist();
});
describe('constructor', () => {
- it('unifies options for the dropdown select with aggregates', () => {
- const { component } = setup();
- expect(component.state('options')).toEqual([
- {
- optionName: '_col_source',
- type: 'VARCHAR(255)',
- column_name: 'source',
- },
- {
- optionName: '_col_target',
- type: 'VARCHAR(255)',
- column_name: 'target',
- },
- { optionName: '_col_value', type: 'DOUBLE', column_name: 'value' },
- ...Object.keys(AGGREGATES).map(aggregate => ({
- aggregate_name: aggregate,
- optionName: `_aggregate_${aggregate}`,
- })),
- {
- optionName: 'sum__value',
- metric_name: 'sum__value',
- expression: 'SUM(energy_usage.value)',
- },
- {
- optionName: 'avg__value',
- metric_name: 'avg__value',
- expression: 'AVG(energy_usage.value)',
- },
- ]);
- });
-
- it('does not show aggregates in options if no columns', () => {
- const { component } = setup({ columns: [] });
- expect(component.state('options')).toEqual([
- {
- optionName: 'sum__value',
- metric_name: 'sum__value',
- expression: 'SUM(energy_usage.value)',
- },
- {
- optionName: 'avg__value',
- metric_name: 'avg__value',
- expression: 'AVG(energy_usage.value)',
- },
- ]);
- });
-
it('coerces Adhoc Metrics from form data into instances of the AdhocMetric class and leaves saved metrics', () => {
const { component } = setup({
value: [
@@ -178,194 +132,7 @@ describe('MetricsControl', () => {
});
});
- describe('checkIfAggregateInInput', () => {
- it('handles an aggregate in the input', () => {
- const { component } = setup();
-
- expect(component.state('aggregateInInput')).toBeNull();
- component.instance().checkIfAggregateInInput('AVG(');
- expect(component.state('aggregateInInput')).toBe(AGGREGATES.AVG);
- });
-
- it('handles no aggregate in the input', () => {
- const { component } = setup();
-
- expect(component.state('aggregateInInput')).toBeNull();
- component.instance().checkIfAggregateInInput('colu');
- expect(component.state('aggregateInInput')).toBeNull();
- });
- });
-
describe('option filter', () => {
- it('includes user defined metrics', () => {
- const { component } = setup({ datasourceType: 'druid' });
-
- expect(
- !!component.instance().selectFilterOption(
- {
- data: {
- metric_name: 'a_metric',
- optionName: 'a_metric',
- expression: 'SUM(FANCY(metric))',
- },
- },
- 'a',
- ),
- ).toBe(true);
- });
-
- it('includes auto generated avg metrics for druid', () => {
- const { component } = setup({ datasourceType: 'druid' });
-
- expect(
- !!component.instance().selectFilterOption(
- {
- data: {
- metric_name: 'avg__metric',
- optionName: 'avg__metric',
- expression: 'AVG(metric)',
- },
- },
- 'a',
- ),
- ).toBe(true);
- });
-
- it('includes columns and aggregates', () => {
- const { component } = setup();
-
- expect(
- !!component.instance().selectFilterOption(
- {
- data: {
- type: 'VARCHAR(255)',
- column_name: 'source',
- optionName: '_col_source',
- },
- },
- 'sou',
- ),
- ).toBe(true);
-
- expect(
- !!component
- .instance()
- .selectFilterOption(
- { data: { aggregate_name: 'AVG', optionName: '_aggregate_AVG' } },
- 'av',
- ),
- ).toBe(true);
- });
-
- it('includes columns based on verbose_name', () => {
- const { component } = setup();
-
- expect(
- !!component.instance().selectFilterOption(
- {
- data: {
- metric_name: 'sum__num',
- verbose_name: 'babies',
- optionName: '_col_sum_num',
- },
- },
- 'bab',
- ),
- ).toBe(true);
- });
-
- it('excludes auto generated avg metrics for sqla', () => {
- const { component } = setup();
-
- expect(
- !!component.instance().selectFilterOption(
- {
- data: {
- metric_name: 'avg__metric',
- optionName: 'avg__metric',
- expression: 'AVG(metric)',
- },
- },
- 'a',
- ),
- ).toBe(false);
- });
-
- it('includes custom made simple saved metrics', () => {
- const { component } = setup();
-
- expect(
- !!component.instance().selectFilterOption(
- {
- data: {
- metric_name: 'my_fancy_sum_metric',
- optionName: 'my_fancy_sum_metric',
- expression: 'SUM(value)',
- },
- },
- 'sum',
- ),
- ).toBe(true);
- });
-
- it('excludes auto generated metrics', () => {
- const { component } = setup();
-
- expect(
- !!component.instance().selectFilterOption(
- {
- data: {
- metric_name: 'sum__value',
- optionName: 'sum__value',
- expression: 'SUM(value)',
- },
- },
- 'sum',
- ),
- ).toBe(false);
-
- expect(
- !!component.instance().selectFilterOption(
- {
- data: {
- metric_name: 'sum__value',
- optionName: 'sum__value',
- expression: 'SUM("table"."value")',
- },
- },
- 'sum',
- ),
- ).toBe(false);
- });
-
- it('filters out metrics if the input begins with an aggregate', () => {
- const { component } = setup();
- component.setState({ aggregateInInput: true });
-
- expect(
- !!component.instance().selectFilterOption(
- {
- data: { metric_name: 'metric', expression: 'SUM(FANCY(metric))' },
- },
- 'SUM(',
- ),
- ).toBe(false);
- });
-
- it('includes columns if the input begins with an aggregate', () => {
- const { component } = setup();
- component.setState({ aggregateInInput: true });
-
- expect(
- !!component
- .instance()
- .selectFilterOption(
- { data: { type: 'DOUBLE', column_name: 'value' } },
- 'SUM(',
- ),
- ).toBe(true);
- });
-
it('Removes metrics if savedMetrics changes', () => {
const { props, component, onChange } = setup({
value: [
diff --git a/superset-frontend/src/explore/components/controls/MetricControl/MetricsControl.jsx b/superset-frontend/src/explore/components/controls/MetricControl/MetricsControl.jsx
index 7ca9c7628fc09..9e35d1de89ba0 100644
--- a/superset-frontend/src/explore/components/controls/MetricControl/MetricsControl.jsx
+++ b/superset-frontend/src/explore/components/controls/MetricControl/MetricsControl.jsx
@@ -16,16 +16,10 @@
* specific language governing permissions and limitations
* under the License.
*/
-import React from 'react';
+import React, { useCallback, useEffect, useMemo, useState } from 'react';
import PropTypes from 'prop-types';
-import { t, withTheme } from '@superset-ui/core';
-import { isEqual } from 'lodash';
+import { ensureIsArray, t, useTheme } from '@superset-ui/core';
import ControlHeader from 'src/explore/components/ControlHeader';
-import {
- AGGREGATES_OPTIONS,
- sqlaAutoGeneratedMetricNameRegex,
- druidAutoGeneratedMetricRegex,
-} from 'src/explore/constants';
import Icons from 'src/components/Icons';
import {
AddIconButton,
@@ -34,7 +28,6 @@ import {
LabelsContainer,
} from 'src/explore/components/controls/OptionControls';
import columnType from './columnType';
-import MetricDefinitionOption from './MetricDefinitionOption';
import MetricDefinitionValue from './MetricDefinitionValue';
import AdhocMetric from './AdhocMetric';
import savedMetricType from './savedMetricType';
@@ -82,9 +75,9 @@ function isDictionaryForAdhocMetric(value) {
return value && !(value instanceof AdhocMetric) && value.expressionType;
}
-function columnsContainAllMetrics(value, nextProps) {
+function columnsContainAllMetrics(value, columns, savedMetrics) {
const columnNames = new Set(
- [...(nextProps.columns || []), ...(nextProps.savedMetrics || [])]
+ [...(columns || []), ...(savedMetrics || [])]
// eslint-disable-next-line camelcase
.map(({ column_name, metric_name }) => column_name || metric_name),
);
@@ -123,295 +116,228 @@ function coerceAdhocMetrics(value) {
});
}
-class MetricsControl extends React.PureComponent {
- constructor(props) {
- super(props);
- this.onChange = this.onChange.bind(this);
- this.onMetricEdit = this.onMetricEdit.bind(this);
- this.onNewMetric = this.onNewMetric.bind(this);
- this.onRemoveMetric = this.onRemoveMetric.bind(this);
- this.moveLabel = this.moveLabel.bind(this);
- this.checkIfAggregateInInput = this.checkIfAggregateInInput.bind(this);
- this.optionsForSelect = this.optionsForSelect.bind(this);
- this.selectFilterOption = this.selectFilterOption.bind(this);
- this.isAutoGeneratedMetric = this.isAutoGeneratedMetric.bind(this);
- this.optionRenderer = option => ;
- this.valueRenderer = (option, index) => (
- this.props.onChange(this.state.value)}
- multi={this.props.multi}
- />
- );
- this.select = null;
- this.selectRef = ref => {
- if (ref) {
- this.select = ref.select;
- } else {
- this.select = null;
- }
- };
- this.state = {
- aggregateInInput: null,
- options: this.optionsForSelect(this.props),
- value: coerceAdhocMetrics(this.props.value),
- };
- }
+const emptySavedMetric = { metric_name: '', expression: '' };
- UNSAFE_componentWillReceiveProps(nextProps) {
- const { value } = this.props;
- if (
- !isEqual(this.props.columns, nextProps.columns) ||
- !isEqual(this.props.savedMetrics, nextProps.savedMetrics)
- ) {
- this.setState({ options: this.optionsForSelect(nextProps) });
+const MetricsControl = ({
+ onChange,
+ multi,
+ value: propsValue,
+ columns,
+ savedMetrics,
+ datasource,
+ datasourceType,
+ ...props
+}) => {
+ const [value, setValue] = useState(coerceAdhocMetrics(propsValue));
+ const theme = useTheme();
- // Remove all metrics if selected value no longer a valid column
- // in the dataset. Must use `nextProps` here because Redux reducers may
- // have already updated the value for this control.
- if (!columnsContainAllMetrics(nextProps.value, nextProps)) {
- this.props.onChange([]);
+ const handleChange = useCallback(
+ opts => {
+ // if clear out options
+ if (opts === null) {
+ onChange(null);
+ return;
}
- }
- if (value !== nextProps.value) {
- this.setState({ value: coerceAdhocMetrics(nextProps.value) });
- }
- }
-
- onNewMetric(newMetric) {
- this.setState(
- prevState => ({
- ...prevState,
- value: [...prevState.value, newMetric],
- }),
- () => {
- this.onChange(this.state.value);
- },
- );
- }
- onMetricEdit(changedMetric, oldMetric) {
- this.setState(
- prevState => ({
- value: prevState.value.map(value => {
- if (
- // compare saved metrics
- value === oldMetric.metric_name ||
- // compare adhoc metrics
- typeof value.optionName !== 'undefined'
- ? value.optionName === oldMetric.optionName
- : false
- ) {
- return changedMetric;
+ const transformedOpts = ensureIsArray(opts);
+ const optionValues = transformedOpts
+ .map(option => {
+ // pre-defined metric
+ if (option.metric_name) {
+ return option.metric_name;
}
- return value;
- }),
- }),
- () => {
- this.onChange(this.state.value);
- },
- );
- }
-
- onRemoveMetric(index) {
- if (!Array.isArray(this.state.value)) {
- return;
- }
- const valuesCopy = [...this.state.value];
- valuesCopy.splice(index, 1);
- this.setState(prevState => ({
- ...prevState,
- value: valuesCopy,
- }));
- this.props.onChange(valuesCopy);
- }
+ return option;
+ })
+ .filter(option => option);
+ onChange(multi ? optionValues : optionValues[0]);
+ },
+ [multi, onChange],
+ );
- onChange(opts) {
- // if clear out options
- if (opts === null) {
- this.props.onChange(null);
- return;
- }
+ const onNewMetric = useCallback(
+ newMetric => {
+ const newValue = [...value, newMetric];
+ setValue(newValue);
+ handleChange(newValue);
+ },
+ [handleChange, value],
+ );
- let transformedOpts;
- if (Array.isArray(opts)) {
- transformedOpts = opts;
- } else {
- transformedOpts = opts ? [opts] : [];
- }
- const optionValues = transformedOpts
- .map(option => {
- // pre-defined metric
- if (option.metric_name) {
- return option.metric_name;
+ const onMetricEdit = useCallback(
+ (changedMetric, oldMetric) => {
+ const newValue = value.map(val => {
+ if (
+ // compare saved metrics
+ val === oldMetric.metric_name ||
+ // compare adhoc metrics
+ typeof val.optionName !== 'undefined'
+ ? val.optionName === oldMetric.optionName
+ : false
+ ) {
+ return changedMetric;
}
- return option;
- })
- .filter(option => option);
- this.props.onChange(this.props.multi ? optionValues : optionValues[0]);
- }
-
- moveLabel(dragIndex, hoverIndex) {
- const { value } = this.state;
-
- const newValues = [...value];
- [newValues[hoverIndex], newValues[dragIndex]] = [
- newValues[dragIndex],
- newValues[hoverIndex],
- ];
- this.setState({ value: newValues });
- }
-
- isAddNewMetricDisabled() {
- return !this.props.multi && this.state.value.length > 0;
- }
-
- addNewMetricPopoverTrigger(trigger) {
- if (this.isAddNewMetricDisabled()) {
- return trigger;
- }
- return (
-
- {trigger}
-
- );
- }
+ return val;
+ });
+ setValue(newValue);
+ handleChange(newValue);
+ },
+ [handleChange, value],
+ );
- checkIfAggregateInInput(input) {
- const lowercaseInput = input.toLowerCase();
- const aggregateInInput =
- AGGREGATES_OPTIONS.find(x =>
- lowercaseInput.startsWith(`${x.toLowerCase()}(`),
- ) || null;
- this.clearedAggregateInInput = this.state.aggregateInInput;
- this.setState({ aggregateInInput });
- }
+ const onRemoveMetric = useCallback(
+ index => {
+ if (!Array.isArray(value)) {
+ return;
+ }
+ const valuesCopy = [...value];
+ valuesCopy.splice(index, 1);
+ setValue(valuesCopy);
+ handleChange(valuesCopy);
+ },
+ [handleChange, value],
+ );
- optionsForSelect(props) {
- const { columns, savedMetrics } = props;
- const aggregates =
- columns && columns.length
- ? AGGREGATES_OPTIONS.map(aggregate => ({
- aggregate_name: aggregate,
- }))
- : [];
- const options = [
- ...(columns || []),
- ...aggregates,
- ...(savedMetrics || []),
- ];
+ const moveLabel = useCallback(
+ (dragIndex, hoverIndex) => {
+ const newValues = [...value];
+ [newValues[hoverIndex], newValues[dragIndex]] = [
+ newValues[dragIndex],
+ newValues[hoverIndex],
+ ];
+ setValue(newValues);
+ },
+ [value],
+ );
- return options.reduce((results, option) => {
- if (option.metric_name) {
- results.push({ ...option, optionName: option.metric_name });
- } else if (option.column_name) {
- results.push({ ...option, optionName: `_col_${option.column_name}` });
- } else if (option.aggregate_name) {
- results.push({
- ...option,
- optionName: `_aggregate_${option.aggregate_name}`,
- });
- }
- return results;
- }, []);
- }
+ const isAddNewMetricDisabled = useCallback(() => !multi && value.length > 0, [
+ multi,
+ value.length,
+ ]);
- isAutoGeneratedMetric(savedMetric) {
- if (this.props.datasourceType === 'druid') {
- return druidAutoGeneratedMetricRegex.test(savedMetric.verbose_name);
- }
- return sqlaAutoGeneratedMetricNameRegex.test(savedMetric.metric_name);
- }
+ const savedMetricOptions = useMemo(
+ () => getOptionsForSavedMetrics(savedMetrics, propsValue, null),
+ [propsValue, savedMetrics],
+ );
- selectFilterOption({ data: option }, filterValue) {
- if (this.state.aggregateInInput) {
- let endIndex = filterValue.length;
- if (filterValue.endsWith(')')) {
- endIndex = filterValue.length - 1;
+ const newAdhocMetric = useMemo(() => new AdhocMetric({ isNew: true }), [
+ value,
+ ]);
+ const addNewMetricPopoverTrigger = useCallback(
+ trigger => {
+ if (isAddNewMetricDisabled()) {
+ return trigger;
}
- const valueAfterAggregate = filterValue.substring(
- filterValue.indexOf('(') + 1,
- endIndex,
- );
return (
- option.column_name &&
- option.column_name.toLowerCase().indexOf(valueAfterAggregate) >= 0
+
+ {trigger}
+
);
+ },
+ [
+ columns,
+ datasource,
+ datasourceType,
+ isAddNewMetricDisabled,
+ newAdhocMetric,
+ onNewMetric,
+ savedMetricOptions,
+ ],
+ );
+
+ useEffect(() => {
+ // Remove all metrics if selected value no longer a valid column
+ // in the dataset. Must use `nextProps` here because Redux reducers may
+ // have already updated the value for this control.
+ if (!columnsContainAllMetrics(propsValue, columns, savedMetrics)) {
+ handleChange([]);
}
- return (
- option.optionName &&
- (!option.metric_name ||
- !this.isAutoGeneratedMetric(option) ||
- option.verbose_name) &&
- (option.optionName.toLowerCase().indexOf(filterValue) >= 0 ||
- (option.verbose_name &&
- option.verbose_name.toLowerCase().indexOf(filterValue) >= 0))
- );
- }
+ }, [columns, savedMetrics]);
- render() {
- const { theme } = this.props;
- return (
-
-
-
- {this.addNewMetricPopoverTrigger(
-
-
- ,
- )}
-
-
- {this.state.value.length > 0
- ? this.state.value.map((value, index) =>
- this.valueRenderer(value, index),
- )
- : this.addNewMetricPopoverTrigger(
-
-
- {t('Add metric')}
- ,
- )}
-
-
+
+
+ {addNewMetricPopoverTrigger(
+
+
+ ,
+ )}
+
+
+ {value.length > 0
+ ? value.map((value, index) => valueRenderer(value, index))
+ : addNewMetricPopoverTrigger(
+
+
+ {t('Add metric')}
+ ,
+ )}
+
+