fix(explore): adhoc metrics popover resets label after hovering outside (#16196)

* fix(explore): adhoc metrics popover resets label after hovering outside

* Remove irrelevant tests and skip rest

* Use ensureIsArray
This commit is contained in:
Kamil Gabryjelski 2021-08-11 18:05:08 +02:00 committed by GitHub
parent 6c304b83a9
commit ccfc95fbe6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 218 additions and 525 deletions

View File

@ -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: [

View File

@ -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 => <MetricDefinitionOption option={option} />;
this.valueRenderer = (option, index) => (
const emptySavedMetric = { metric_name: '', expression: '' };
const MetricsControl = ({
onChange,
multi,
value: propsValue,
columns,
savedMetrics,
datasource,
datasourceType,
...props
}) => {
const [value, setValue] = useState(coerceAdhocMetrics(propsValue));
const theme = useTheme();
const handleChange = useCallback(
opts => {
// if clear out options
if (opts === null) {
onChange(null);
return;
}
const transformedOpts = ensureIsArray(opts);
const optionValues = transformedOpts
.map(option => {
// pre-defined metric
if (option.metric_name) {
return option.metric_name;
}
return option;
})
.filter(option => option);
onChange(multi ? optionValues : optionValues[0]);
},
[multi, onChange],
);
const onNewMetric = useCallback(
newMetric => {
const newValue = [...value, newMetric];
setValue(newValue);
handleChange(newValue);
},
[handleChange, value],
);
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 val;
});
setValue(newValue);
handleChange(newValue);
},
[handleChange, value],
);
const onRemoveMetric = useCallback(
index => {
if (!Array.isArray(value)) {
return;
}
const valuesCopy = [...value];
valuesCopy.splice(index, 1);
setValue(valuesCopy);
handleChange(valuesCopy);
},
[handleChange, value],
);
const moveLabel = useCallback(
(dragIndex, hoverIndex) => {
const newValues = [...value];
[newValues[hoverIndex], newValues[dragIndex]] = [
newValues[dragIndex],
newValues[hoverIndex],
];
setValue(newValues);
},
[value],
);
const isAddNewMetricDisabled = useCallback(() => !multi && value.length > 0, [
multi,
value.length,
]);
const savedMetricOptions = useMemo(
() => getOptionsForSavedMetrics(savedMetrics, propsValue, null),
[propsValue, savedMetrics],
);
const newAdhocMetric = useMemo(() => new AdhocMetric({ isNew: true }), [
value,
]);
const addNewMetricPopoverTrigger = useCallback(
trigger => {
if (isAddNewMetricDisabled()) {
return trigger;
}
return (
<AdhocMetricPopoverTrigger
adhocMetric={newAdhocMetric}
onMetricEdit={onNewMetric}
columns={columns}
savedMetricsOptions={savedMetricOptions}
datasource={datasource}
savedMetric={emptySavedMetric}
datasourceType={datasourceType}
createNew
>
{trigger}
</AdhocMetricPopoverTrigger>
);
},
[
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([]);
}
}, [columns, savedMetrics]);
useEffect(() => {
setValue(coerceAdhocMetrics(propsValue));
}, [propsValue]);
const onDropLabel = useCallback(() => handleChange(value), [
handleChange,
value,
]);
const valueRenderer = useCallback(
(option, index) => (
<MetricDefinitionValue
key={index}
index={index}
option={option}
onMetricEdit={this.onMetricEdit}
onRemoveMetric={this.onRemoveMetric}
columns={this.props.columns}
datasource={this.props.datasource}
savedMetrics={this.props.savedMetrics}
onMetricEdit={onMetricEdit}
onRemoveMetric={onRemoveMetric}
columns={columns}
datasource={datasource}
savedMetrics={savedMetrics}
savedMetricsOptions={getOptionsForSavedMetrics(
this.props.savedMetrics,
this.props.value,
this.props.value?.[index],
savedMetrics,
value,
value?.[index],
)}
datasourceType={this.props.datasourceType}
onMoveLabel={this.moveLabel}
onDropLabel={() => this.props.onChange(this.state.value)}
multi={this.props.multi}
datasourceType={datasourceType}
onMoveLabel={moveLabel}
onDropLabel={onDropLabel}
multi={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),
};
}
),
[
columns,
datasource,
datasourceType,
moveLabel,
multi,
onDropLabel,
onMetricEdit,
onRemoveMetric,
savedMetrics,
value,
],
);
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) });
// 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([]);
}
}
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;
}
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);
}
onChange(opts) {
// if clear out options
if (opts === null) {
this.props.onChange(null);
return;
}
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;
}
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 (
<AdhocMetricPopoverTrigger
adhocMetric={new AdhocMetric({ isNew: true })}
onMetricEdit={this.onNewMetric}
columns={this.props.columns}
savedMetricsOptions={getOptionsForSavedMetrics(
this.props.savedMetrics,
this.props.value,
null,
return (
<div className="metrics-select">
<HeaderContainer>
<ControlHeader {...props} />
{addNewMetricPopoverTrigger(
<AddIconButton
disabled={isAddNewMetricDisabled()}
data-test="add-metric-button"
>
<Icons.PlusLarge
iconSize="s"
iconColor={theme.colors.grayscale.light5}
/>
</AddIconButton>,
)}
datasource={this.props.datasource}
savedMetric={{ metric_name: '', expression: '' }}
datasourceType={this.props.datasourceType}
createNew
>
{trigger}
</AdhocMetricPopoverTrigger>
);
}
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 });
}
optionsForSelect(props) {
const { columns, savedMetrics } = props;
const aggregates =
columns && columns.length
? AGGREGATES_OPTIONS.map(aggregate => ({
aggregate_name: aggregate,
}))
: [];
const options = [
...(columns || []),
...aggregates,
...(savedMetrics || []),
];
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;
}, []);
}
isAutoGeneratedMetric(savedMetric) {
if (this.props.datasourceType === 'druid') {
return druidAutoGeneratedMetricRegex.test(savedMetric.verbose_name);
}
return sqlaAutoGeneratedMetricNameRegex.test(savedMetric.metric_name);
}
selectFilterOption({ data: option }, filterValue) {
if (this.state.aggregateInInput) {
let endIndex = filterValue.length;
if (filterValue.endsWith(')')) {
endIndex = filterValue.length - 1;
}
const valueAfterAggregate = filterValue.substring(
filterValue.indexOf('(') + 1,
endIndex,
);
return (
option.column_name &&
option.column_name.toLowerCase().indexOf(valueAfterAggregate) >= 0
);
}
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))
);
}
render() {
const { theme } = this.props;
return (
<div className="metrics-select">
<HeaderContainer>
<ControlHeader {...this.props} />
{this.addNewMetricPopoverTrigger(
<AddIconButton
disabled={this.isAddNewMetricDisabled()}
data-test="add-metric-button"
>
<Icons.PlusLarge
iconSize="s"
iconColor={theme.colors.grayscale.light5}
/>
</AddIconButton>,
)}
</HeaderContainer>
<LabelsContainer>
{this.state.value.length > 0
? this.state.value.map((value, index) =>
this.valueRenderer(value, index),
)
: this.addNewMetricPopoverTrigger(
<AddControlLabel>
<Icons.PlusSmall iconColor={theme.colors.grayscale.light1} />
{t('Add metric')}
</AddControlLabel>,
)}
</LabelsContainer>
</div>
);
}
}
</HeaderContainer>
<LabelsContainer>
{value.length > 0
? value.map((value, index) => valueRenderer(value, index))
: addNewMetricPopoverTrigger(
<AddControlLabel>
<Icons.PlusSmall iconColor={theme.colors.grayscale.light1} />
{t('Add metric')}
</AddControlLabel>,
)}
</LabelsContainer>
</div>
);
};
MetricsControl.propTypes = propTypes;
MetricsControl.defaultProps = defaultProps;
export default withTheme(MetricsControl);
export default MetricsControl;