iop
/
superset
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(explore): Disable saved metric name edit in Metric popover (#12582)

This commit is contained in:
Kamil Gabryjelski 2021-01-19 21:28:55 +01:00 committed by GitHub
parent 862ddd9cb4
commit 853d34beba
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 99 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
superset-frontend/cypress-base/cypress/integration/explore/AdhocMetrics.test.ts
View File

@ -37,6 +37,9 @@ describe('AdhocMetrics', () => {
.find('[data-test="add-metric-button"]')
.click();
// Title edit for saved metrics is disabled - switch to Simple
cy.get('[id="adhoc-metric-edit-tabs-tab-SIMPLE"]').click();
cy.get('[data-test="AdhocMetricEditTitle#trigger"]').click();
cy.get('[data-test="AdhocMetricEditTitle#input"]').type(metricName);

4
superset-frontend/cypress-base/cypress/integration/explore/visualizations/line.test.ts
View File

@ -50,6 +50,10 @@ describe('Visualization > Line', () => {
cy.get('[data-test=metrics]')
.find('[data-test="add-metric-button"]')
.click();
// Title edit for saved metrics is disabled - switch to Simple
cy.get('[id="adhoc-metric-edit-tabs-tab-SIMPLE"]').click();
cy.get('[name="select-column"]').click().type('num{enter}');
cy.get('[name="select-aggregate"]').click().type('sum{enter}');
cy.get('[data-test="AdhocMetricEdit#save"]').contains('Save').click();

16
superset-frontend/spec/javascripts/explore/components/AdhocMetricEditPopoverTitle_spec.jsx
View File

@ -46,15 +46,25 @@ describe('AdhocMetricEditPopoverTitle', () => {
expect(wrapper.find(Tooltip)).toExist();
expect(
wrapper.find('[data-test="AdhocMetricEditTitle#trigger"]').text(),
).toBe('My Metric\xa0');
).toBe(`${title.label}\xa0`);
});
it('transfers to edit mode when clicked', () => {
const { wrapper } = setup();
expect(wrapper.state('isEditable')).toBe(false);
expect(wrapper.state('isEditMode')).toBe(false);
wrapper
.find('[data-test="AdhocMetricEditTitle#trigger"]')
.simulate('click');
expect(wrapper.state('isEditable')).toBe(true);
expect(wrapper.state('isEditMode')).toBe(true);
});
it('Render non-interactive span with title when edit is disabled', () => {
const { wrapper } = setup({ isEditDisabled: true });
expect(
wrapper.find('[data-test="AdhocMetricTitle"]').exists(),
).toBeTruthy();
expect(
wrapper.find('[data-test="AdhocMetricEditTitle#trigger"]').exists(),
).toBeFalsy();
});
});

73
superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover.jsx
View File

@ -29,6 +29,7 @@ import { ColumnOption } from '@superset-ui/chart-controls';
import FormLabel from 'src/components/FormLabel';
import { SQLEditor } from 'src/components/AsyncAceEditor';
import sqlKeywords from 'src/SqlLab/utils/sqlKeywords';
import { noOp } from 'src/utils/common';
import { AGGREGATES_OPTIONS } from 'src/explore/constants';
import columnType from 'src/explore/propTypes/columnType';
@ -40,6 +41,7 @@ const propTypes = {
onChange: PropTypes.func.isRequired,
onClose: PropTypes.func.isRequired,
onResize: PropTypes.func.isRequired,
getCurrentTab: PropTypes.func,
columns: PropTypes.arrayOf(columnType),
savedMetrics: PropTypes.arrayOf(savedMetricType),
savedMetric: savedMetricType,
@ -52,6 +54,7 @@ const propTypes = {
const defaultProps = {
columns: [],
getCurrentTab: noOp,
};
const ResizeIcon = styled.i`
@ -64,12 +67,20 @@ const ColumnOptionStyle = styled.span`
}
`;
const SAVED_TAB_KEY = 'SAVED';
export const SAVED_TAB_KEY = 'SAVED';
const startingWidth = 320;
const startingHeight = 240;
export default class AdhocMetricEditPopover extends React.Component {
// "Saved" is a default tab unless there are no saved metrics for dataset
defaultActiveTabKey =
(this.props.savedMetric.metric_name || this.props.adhocMetric.isNew) &&
Array.isArray(this.props.savedMetrics) &&
this.props.savedMetrics.length > 0
? SAVED_TAB_KEY
: this.props.adhocMetric.expressionType;
constructor(props) {
super(props);
this.onSave = this.onSave.bind(this);
@ -81,6 +92,7 @@ export default class AdhocMetricEditPopover extends React.Component {
this.onDragDown = this.onDragDown.bind(this);
this.onMouseMove = this.onMouseMove.bind(this);
this.onMouseUp = this.onMouseUp.bind(this);
this.onTabChange = this.onTabChange.bind(this);
this.handleAceEditorRef = this.handleAceEditorRef.bind(this);
this.refreshAceEditor = this.refreshAceEditor.bind(this);
@ -94,6 +106,10 @@ export default class AdhocMetricEditPopover extends React.Component {
document.addEventListener('mouseup', this.onMouseUp);
}
componentDidMount() {
this.props.getCurrentTab(this.defaultActiveTabKey);
}
componentWillUnmount() {
document.removeEventListener('mouseup', this.onMouseUp);
document.removeEventListener('mousemove', this.onMouseMove);
@ -207,6 +223,11 @@ export default class AdhocMetricEditPopover extends React.Component {
document.removeEventListener('mousemove', this.onMouseMove);
}
onTabChange(tab) {
this.refreshAceEditor();
this.props.getCurrentTab(tab);
}
handleAceEditorRef(ref) {
if (ref) {
this.aceEditorRef = ref;
@ -316,16 +337,33 @@ export default class AdhocMetricEditPopover extends React.Component {
<Tabs
id="adhoc-metric-edit-tabs"
data-test="adhoc-metric-edit-tabs"
defaultActiveKey={
propsSavedMetric.metric_name
? SAVED_TAB_KEY
: adhocMetric.expressionType
}
defaultActiveKey={this.defaultActiveTabKey}
className="adhoc-metric-edit-tabs"
style={{ height: this.state.height, width: this.state.width }}
onChange={this.refreshAceEditor}
onChange={this.onTabChange}
allowOverflow
>
<Tabs.TabPane key={SAVED_TAB_KEY} tab={t('Saved')}>
<FormGroup>
<FormLabel>
<strong>{t('Saved metric')}</strong>
</FormLabel>
<Select name="select-saved" {...savedSelectProps}>
{Array.isArray(savedMetrics) &&
savedMetrics.map(savedMetric => (
<Select.Option
value={savedMetric.id}
filterBy={
savedMetric.verbose_name || savedMetric.metric_name
}
key={savedMetric.id}
>
{this.renderColumnOption(savedMetric)}
</Select.Option>
))}
</Select>
</FormGroup>
</Tabs.TabPane>
<Tabs.TabPane key={EXPRESSION_TYPES.SIMPLE} tab={t('Simple')}>
<FormGroup>
<FormLabel>
@ -356,27 +394,6 @@ export default class AdhocMetricEditPopover extends React.Component {
</Select>
</FormGroup>
</Tabs.TabPane>
<Tabs.TabPane key={SAVED_TAB_KEY} tab={t('Saved')}>
<FormGroup>
<FormLabel>
<strong>{t('Saved metric')}</strong>
</FormLabel>
<Select name="select-saved" {...savedSelectProps}>
{Array.isArray(savedMetrics) &&
savedMetrics.map(savedMetric => (
<Select.Option
value={savedMetric.id}
filterBy={
savedMetric.verbose_name || savedMetric.metric_name
}
key={savedMetric.id}
>
{this.renderColumnOption(savedMetric)}
</Select.Option>
))}
</Select>
</FormGroup>
</Tabs.TabPane>
<Tabs.TabPane
key={EXPRESSION_TYPES.SQL}
tab={t('Custom SQL')}

21
superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopoverTitle.jsx
View File

@ -17,6 +17,7 @@
* under the License.
*/
import React from 'react';
import { t } from '@superset-ui/core';
import PropTypes from 'prop-types';
import { FormControl } from 'react-bootstrap';
import { Tooltip } from 'src/common/components/Tooltip';
@ -27,6 +28,7 @@ const propTypes = {
hasCustomLabel: PropTypes.bool,
}),
onChange: PropTypes.func.isRequired,
isEditDisabled: PropTypes.bool,
};
export default class AdhocMetricEditPopoverTitle extends React.Component {
@ -39,7 +41,7 @@ export default class AdhocMetricEditPopoverTitle extends React.Component {
this.onInputBlur = this.onInputBlur.bind(this);
this.state = {
isHovered: false,
isEditable: false,
isEditMode: false,
};
}
@ -52,11 +54,11 @@ export default class AdhocMetricEditPopoverTitle extends React.Component {
}
onClick() {
this.setState({ isEditable: true });
this.setState({ isEditMode: true });
}
onBlur() {
this.setState({ isEditable: false });
this.setState({ isEditMode: false });
}
onInputBlur(e) {
@ -67,9 +69,16 @@ export default class AdhocMetricEditPopoverTitle extends React.Component {
}
render() {
const { title, onChange } = this.props;
const { title, onChange, isEditDisabled } = this.props;
const defaultLabel = t('My Metric');
return this.state.isEditable ? (
if (isEditDisabled) {
return (
<span data-test="AdhocMetricTitle">{title.label || defaultLabel}</span>
);
}
return this.state.isEditMode ? (
<FormControl
className="metric-edit-popover-label-input"
type="text"
@ -92,7 +101,7 @@ export default class AdhocMetricEditPopoverTitle extends React.Component {
role="button"
tabIndex={0}
>
{title.hasCustomLabel ? title.label : 'My Metric'}
{title.label || defaultLabel}
&nbsp;
<i
className="fa fa-pencil"

17
superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricPopoverTrigger.tsx
View File

@ -19,7 +19,9 @@
import React, { ReactNode } from 'react';
import Popover from 'src/common/components/Popover';
import AdhocMetricEditPopoverTitle from 'src/explore/components/controls/MetricControl/AdhocMetricEditPopoverTitle';
import AdhocMetricEditPopover from './AdhocMetricEditPopover';
import AdhocMetricEditPopover, {
SAVED_TAB_KEY,
} from './AdhocMetricEditPopover';
import AdhocMetric from './AdhocMetric';
import { savedMetricType } from './types';
@ -38,6 +40,7 @@ export type AdhocMetricPopoverTriggerState = {
popoverVisible: boolean;
title: { label: string; hasCustomLabel: boolean };
labelModified: boolean;
isTitleEditDisabled: boolean;
};
class AdhocMetricPopoverTrigger extends React.PureComponent<
@ -57,6 +60,7 @@ class AdhocMetricPopoverTrigger extends React.PureComponent<
hasCustomLabel: props.adhocMetric.hasCustomLabel,
},
labelModified: false,
isTitleEditDisabled: false,
};
}
@ -89,11 +93,12 @@ class AdhocMetricPopoverTrigger extends React.PureComponent<
}
render() {
const { adhocMetric } = this.props;
const { adhocMetric, savedMetric } = this.props;
const { verbose_name, metric_name } = savedMetric;
const { label, hasCustomLabel } = adhocMetric;
const title = this.state.labelModified
? this.state.title
: { label, hasCustomLabel };
: { label: verbose_name || metric_name || label, hasCustomLabel };
const overlayContent = (
<AdhocMetricEditPopover
@ -106,6 +111,11 @@ class AdhocMetricPopoverTrigger extends React.PureComponent<
onResize={this.onPopoverResize}
onClose={this.closePopover}
onChange={this.props.onMetricEdit}
getCurrentTab={(tab: string) =>
this.setState({
isTitleEditDisabled: tab === SAVED_TAB_KEY,
})
}
/>
);
@ -113,6 +123,7 @@ class AdhocMetricPopoverTrigger extends React.PureComponent<
<AdhocMetricEditPopoverTitle
title={title}
onChange={this.onLabelChange}
isEditDisabled={this.state.isTitleEditDisabled}
/>
);

2
superset-frontend/src/explore/components/controls/MetricControl/MetricDefinitionValue.jsx
View File

@ -62,7 +62,7 @@ export default function MetricDefinitionValue({
if (option instanceof AdhocMetric || savedMetric) {
const adhocMetric =
option instanceof AdhocMetric ? option : new AdhocMetric({});
option instanceof AdhocMetric ? option : new AdhocMetric({ isNew: true });
const metricOptionProps = {
onMetricEdit,

6
superset-frontend/src/explore/components/controls/MetricControl/MetricsControl.jsx
View File

@ -191,7 +191,9 @@ class MetricsControl extends React.PureComponent {
// compare saved metrics
value === oldMetric.metric_name ||
// compare adhoc metrics
value.optionName === oldMetric.optionName
typeof value.optionName !== 'undefined'
? value.optionName === oldMetric.optionName
: false
) {
return changedMetric;
}
@ -263,7 +265,7 @@ class MetricsControl extends React.PureComponent {
}
return (
<AdhocMetricPopoverTrigger
adhocMetric={new AdhocMetric({})}
adhocMetric={new AdhocMetric({ isNew: true })}
onMetricEdit={this.onNewMetric}
columns={this.props.columns}
savedMetrics={this.props.savedMetrics}