From commits-return-827-archive-asf-public=cust-asf.ponee.io@superset.incubator.apache.org Thu Mar 29 02:41:40 2018 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by mx-eu-01.ponee.io (Postfix) with SMTP id B290B180652 for ; Thu, 29 Mar 2018 02:41:38 +0200 (CEST) Received: (qmail 81341 invoked by uid 500); 29 Mar 2018 00:41:37 -0000 Mailing-List: contact commits-help@superset.incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@superset.incubator.apache.org Delivered-To: mailing list commits@superset.incubator.apache.org Received: (qmail 81328 invoked by uid 99); 29 Mar 2018 00:41:37 -0000 Received: from ec2-52-202-80-70.compute-1.amazonaws.com (HELO gitbox.apache.org) (52.202.80.70) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 29 Mar 2018 00:41:37 +0000 Received: by gitbox.apache.org (ASF Mail Server at gitbox.apache.org, from userid 33) id 1A79D807EF; Thu, 29 Mar 2018 00:41:37 +0000 (UTC) Date: Thu, 29 Mar 2018 00:41:36 +0000 To: "commits@superset.apache.org" Subject: [incubator-superset] branch master updated: [Explore] Streamlined metric definitions for SQLA and Druid (#4663) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Message-ID: <152228409691.9929.11934021202225972482@gitbox.apache.org> From: maximebeauchemin@apache.org X-Git-Host: gitbox.apache.org X-Git-Repo: incubator-superset X-Git-Refname: refs/heads/master X-Git-Reftype: branch X-Git-Oldrev: 7e1b6b7363e1c5dcff92d7e74716192bc0ceaad8 X-Git-Newrev: 68dec245423a64808b6edb45ce2bedbbaf1a000d X-Git-Rev: 68dec245423a64808b6edb45ce2bedbbaf1a000d X-Git-NotificationType: ref_changed_plus_diff X-Git-Multimail-Version: 1.5.dev Auto-Submitted: auto-generated This is an automated email from the ASF dual-hosted git repository. maximebeauchemin pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/incubator-superset.git The following commit(s) were added to refs/heads/master by this push: new 68dec24 [Explore] Streamlined metric definitions for SQLA and Druid (#4663) 68dec24 is described below commit 68dec245423a64808b6edb45ce2bedbbaf1a000d Author: Gabe Lyons AuthorDate: Wed Mar 28 17:41:29 2018 -0700 [Explore] Streamlined metric definitions for SQLA and Druid (#4663) * adding streamlined metric editing * addressing lint issues on new metrics control * enabling druid --- superset/assets/.eslintrc | 3 + .../javascripts/components/ColumnTypeLabel.jsx | 10 +- .../javascripts/components/OnPasteSelect.jsx | 6 +- superset/assets/javascripts/dashboard/reducers.js | 1 + superset/assets/javascripts/explore/AdhocMetric.js | 32 +++ .../explore/components/AdhocMetricEditPopover.jsx | 141 ++++++++++++ .../components/AdhocMetricEditPopoverTitle.jsx | 79 +++++++ .../explore/components/AdhocMetricOption.jsx | 60 +++++ .../explore/components/AggregateOption.jsx | 22 ++ .../explore/components/MetricDefinitionOption.jsx | 36 +++ .../explore/components/MetricDefinitionValue.jsx | 47 ++++ .../explore/components/controls/MetricsControl.jsx | 256 +++++++++++++++++++++ .../explore/components/controls/index.js | 2 + superset/assets/javascripts/explore/constants.js | 9 + superset/assets/javascripts/explore/main.css | 4 + .../explore/propTypes/adhocMetricType.js | 10 + .../explore/propTypes/aggregateOptionType.js | 5 + .../javascripts/explore/propTypes/columnType.js | 6 + .../explore/propTypes/savedMetricType.js | 6 + .../assets/javascripts/explore/stores/controls.jsx | 20 +- .../assets/javascripts/explore/stores/visTypes.js | 3 +- superset/assets/package.json | 4 +- .../spec/javascripts/explore/AdhocMetric_spec.js | 85 +++++++ .../AdhocMetricEditPopoverTitle_spec.jsx | 48 ++++ .../components/AdhocMetricEditPopover_spec.jsx | 90 ++++++++ .../explore/components/AdhocMetricOption_spec.jsx | 42 ++++ .../explore/components/AggregateOption_spec.jsx | 14 ++ .../components/MetricDefinitionOption_spec.jsx | 27 +++ .../components/MetricDefinitionValue_spec.jsx | 30 +++ .../explore/components/MetricsControl_spec.jsx | 250 ++++++++++++++++++++ superset/assets/stylesheets/superset.less | 6 + superset/connectors/druid/models.py | 44 +++- superset/connectors/sqla/models.py | 23 +- superset/utils.py | 14 +- superset/viz.py | 13 +- tests/druid_func_tests.py | 114 ++++++++- 36 files changed, 1517 insertions(+), 45 deletions(-) diff --git a/superset/assets/.eslintrc b/superset/assets/.eslintrc index c8b0766..921e927 100644 --- a/superset/assets/.eslintrc +++ b/superset/assets/.eslintrc @@ -38,5 +38,8 @@ "react/no-unescaped-entities": 0, "react/no-unused-prop-types": 0, "react/no-string-refs": 0, + "indent": 0, + "no-multi-spaces": 0, + "padded-blocks": 0, } } diff --git a/superset/assets/javascripts/components/ColumnTypeLabel.jsx b/superset/assets/javascripts/components/ColumnTypeLabel.jsx index 719891e..33f8319 100644 --- a/superset/assets/javascripts/components/ColumnTypeLabel.jsx +++ b/superset/assets/javascripts/components/ColumnTypeLabel.jsx @@ -2,16 +2,20 @@ import React from 'react'; import PropTypes from 'prop-types'; const propTypes = { - type: PropTypes.string.isRequired, + type: PropTypes.string, }; export default function ColumnTypeLabel({ type }) { let stringIcon = ''; - if (type === '' || type === 'expression') { + if (typeof type !== 'string') { + stringIcon = '?'; + } else if (type === '' || type === 'expression') { stringIcon = 'ƒ'; + } else if (type === 'aggregate') { + stringIcon = 'AGG'; } else if (type.match(/.*char.*/i) || type.match(/string.*/i) || type.match(/.*text.*/i)) { stringIcon = 'ABC'; - } else if (type.match(/.*int.*/i) || type === 'LONG' || type === 'DOUBLE') { + } else if (type.match(/.*int.*/i) || type === 'LONG' || type === 'DOUBLE' || type === 'FLOAT') { stringIcon = '#'; } else if (type.match(/.*bool.*/i)) { stringIcon = 'T/F'; diff --git a/superset/assets/javascripts/components/OnPasteSelect.jsx b/superset/assets/javascripts/components/OnPasteSelect.jsx index b043bf3..40bbbd0 100644 --- a/superset/assets/javascripts/components/OnPasteSelect.jsx +++ b/superset/assets/javascripts/components/OnPasteSelect.jsx @@ -49,8 +49,8 @@ export default class OnPasteSelect extends React.Component { render() { const SelectComponent = this.props.selectWrap; const refFunc = (ref) => { - if (this.props.ref) { - this.props.ref(ref); + if (this.props.refFunc) { + this.props.refFunc(ref); } this.pasteInput = ref; }; @@ -68,7 +68,7 @@ export default class OnPasteSelect extends React.Component { OnPasteSelect.propTypes = { separator: PropTypes.string.isRequired, selectWrap: PropTypes.func.isRequired, - ref: PropTypes.func, + refFunc: PropTypes.func, onChange: PropTypes.func.isRequired, valueKey: PropTypes.string.isRequired, labelKey: PropTypes.string.isRequired, diff --git a/superset/assets/javascripts/dashboard/reducers.js b/superset/assets/javascripts/dashboard/reducers.js index bf42532..1cc3e76 100644 --- a/superset/assets/javascripts/dashboard/reducers.js +++ b/superset/assets/javascripts/dashboard/reducers.js @@ -1,3 +1,4 @@ +/* eslint-disable camelcase */ import { combineReducers } from 'redux'; import d3 from 'd3'; import shortid from 'shortid'; diff --git a/superset/assets/javascripts/explore/AdhocMetric.js b/superset/assets/javascripts/explore/AdhocMetric.js new file mode 100644 index 0000000..e123521 --- /dev/null +++ b/superset/assets/javascripts/explore/AdhocMetric.js @@ -0,0 +1,32 @@ +export default class AdhocMetric { + constructor(adhocMetric) { + this.column = adhocMetric.column; + this.aggregate = adhocMetric.aggregate; + this.hasCustomLabel = !!(adhocMetric.hasCustomLabel && adhocMetric.label); + this.fromFormData = !!adhocMetric.optionName; + this.label = this.hasCustomLabel ? adhocMetric.label : this.getDefaultLabel(); + + this.optionName = adhocMetric.optionName || + `metric_${Math.random().toString(36).substring(2, 15)}_${Math.random().toString(36).substring(2, 15)}`; + } + + getDefaultLabel() { + return `${this.aggregate || ''}(${(this.column && this.column.column_name) || ''})`; + } + + duplicateWith(nextFields) { + return new AdhocMetric({ + ...this, + ...nextFields, + }); + } + + equals(adhocMetric) { + return adhocMetric.label === this.label && + adhocMetric.aggregate === this.aggregate && + ( + (adhocMetric.column && adhocMetric.column.column_name) === + (this.column && this.column.column_name) + ); + } +} diff --git a/superset/assets/javascripts/explore/components/AdhocMetricEditPopover.jsx b/superset/assets/javascripts/explore/components/AdhocMetricEditPopover.jsx new file mode 100644 index 0000000..0964a51 --- /dev/null +++ b/superset/assets/javascripts/explore/components/AdhocMetricEditPopover.jsx @@ -0,0 +1,141 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import { Button, ControlLabel, FormGroup, Popover } from 'react-bootstrap'; +import VirtualizedSelect from 'react-virtualized-select'; + +import { AGGREGATES } from '../constants'; +import { t } from '../../locales'; +import VirtualizedRendererWrap from '../../components/VirtualizedRendererWrap'; +import OnPasteSelect from '../../components/OnPasteSelect'; +import AdhocMetricEditPopoverTitle from './AdhocMetricEditPopoverTitle'; +import columnType from '../propTypes/columnType'; +import AdhocMetric from '../AdhocMetric'; +import ColumnOption from '../../components/ColumnOption'; + +const propTypes = { + adhocMetric: PropTypes.instanceOf(AdhocMetric).isRequired, + onChange: PropTypes.func.isRequired, + onClose: PropTypes.func.isRequired, + columns: PropTypes.arrayOf(columnType), + datasourceType: PropTypes.string, +}; + +const defaultProps = { + columns: [], +}; + +export default class AdhocMetricEditPopover extends React.Component { + constructor(props) { + super(props); + this.onSave = this.onSave.bind(this); + this.onColumnChange = this.onColumnChange.bind(this); + this.onAggregateChange = this.onAggregateChange.bind(this); + this.onLabelChange = this.onLabelChange.bind(this); + this.state = { adhocMetric: this.props.adhocMetric }; + this.selectProps = { + multi: false, + name: 'select-column', + labelKey: 'label', + autosize: false, + clearable: true, + selectWrap: VirtualizedSelect, + }; + } + + onSave() { + this.props.onChange(this.state.adhocMetric); + this.props.onClose(); + } + + onColumnChange(column) { + this.setState({ adhocMetric: this.state.adhocMetric.duplicateWith({ column }) }); + } + + onAggregateChange(aggregate) { + // we construct this object explicitly to overwrite the value in the case aggregate is null + this.setState({ + adhocMetric: this.state.adhocMetric.duplicateWith({ + aggregate: aggregate && aggregate.aggregate, + }), + }); + } + + onLabelChange(e) { + this.setState({ + adhocMetric: this.state.adhocMetric.duplicateWith({ + label: e.target.value, hasCustomLabel: true, + }), + }); + } + + render() { + const { adhocMetric, columns, onChange, onClose, datasourceType, ...popoverProps } = this.props; + + const columnSelectProps = { + placeholder: t('%s column(s)', columns.length), + options: columns, + value: this.state.adhocMetric.column && this.state.adhocMetric.column.column_name, + onChange: this.onColumnChange, + optionRenderer: VirtualizedRendererWrap(option => ( + + )), + valueRenderer: column => column.column_name, + valueKey: 'column_name', + }; + + const aggregateSelectProps = { + placeholder: t('%s aggregates(s)', Object.keys(AGGREGATES).length), + options: Object.keys(AGGREGATES).map(aggregate => ({ aggregate })), + value: this.state.adhocMetric.aggregate, + onChange: this.onAggregateChange, + optionRenderer: VirtualizedRendererWrap(aggregate => aggregate.aggregate), + valueRenderer: aggregate => aggregate.aggregate, + valueKey: 'aggregate', + }; + + if (this.props.datasourceType === 'druid') { + aggregateSelectProps.options = aggregateSelectProps.options.filter(( + option => option.aggregate !== 'AVG' + )); + } + + const popoverTitle = ( + + ); + + const stateIsValid = this.state.adhocMetric.column && this.state.adhocMetric.aggregate; + const hasUnsavedChanges = this.state.adhocMetric.equals(this.props.adhocMetric); + + return ( + + + column + + + + aggregate + + + + + + ); + } +} +AdhocMetricEditPopover.propTypes = propTypes; +AdhocMetricEditPopover.defaultProps = defaultProps; diff --git a/superset/assets/javascripts/explore/components/AdhocMetricEditPopoverTitle.jsx b/superset/assets/javascripts/explore/components/AdhocMetricEditPopoverTitle.jsx new file mode 100644 index 0000000..d14b111 --- /dev/null +++ b/superset/assets/javascripts/explore/components/AdhocMetricEditPopoverTitle.jsx @@ -0,0 +1,79 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import { FormControl, OverlayTrigger, Tooltip } from 'react-bootstrap'; +import AdhocMetric from '../AdhocMetric'; + +const propTypes = { + adhocMetric: PropTypes.instanceOf(AdhocMetric), + onChange: PropTypes.func.isRequired, +}; + +export default class AdhocMetricEditPopoverTitle extends React.Component { + constructor(props) { + super(props); + this.onMouseOver = this.onMouseOver.bind(this); + this.onMouseOut = this.onMouseOut.bind(this); + this.onClick = this.onClick.bind(this); + this.onBlur = this.onBlur.bind(this); + this.state = { + isHovered: false, + isEditable: false, + }; + } + + onMouseOver() { + this.setState({ isHovered: true }); + } + + onMouseOut() { + this.setState({ isHovered: false }); + } + + onClick() { + this.setState({ isEditable: true }); + } + + onBlur() { + this.setState({ isEditable: false }); + } + + refFunc(ref) { + if (ref) { + ref.focus(); + } + } + + render() { + const { adhocMetric, onChange } = this.props; + + const editPrompt = Click to edit label; + + return ( + + {this.state.isEditable ? + : + + {adhocMetric.hasCustomLabel ? adhocMetric.label : 'My Metric'} +   + + + } + + ); + } +} +AdhocMetricEditPopoverTitle.propTypes = propTypes; diff --git a/superset/assets/javascripts/explore/components/AdhocMetricOption.jsx b/superset/assets/javascripts/explore/components/AdhocMetricOption.jsx new file mode 100644 index 0000000..88dd0d7 --- /dev/null +++ b/superset/assets/javascripts/explore/components/AdhocMetricOption.jsx @@ -0,0 +1,60 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import { Label, OverlayTrigger } from 'react-bootstrap'; + +import AdhocMetricEditPopover from './AdhocMetricEditPopover'; +import AdhocMetric from '../AdhocMetric'; +import columnType from '../propTypes/columnType'; + +const propTypes = { + adhocMetric: PropTypes.instanceOf(AdhocMetric), + onMetricEdit: PropTypes.func.isRequired, + columns: PropTypes.arrayOf(columnType), + multi: PropTypes.bool, + datasourceType: PropTypes.string, +}; + +export default class AdhocMetricOption extends React.PureComponent { + constructor(props) { + super(props); + this.closeMetricEditOverlay = this.closeMetricEditOverlay.bind(this); + } + + closeMetricEditOverlay() { + this.refs.overlay.hide(); + } + + render() { + const { adhocMetric } = this.props; + const overlay = ( + + ); + + return ( + + + + ); + } +} +AdhocMetricOption.propTypes = propTypes; diff --git a/superset/assets/javascripts/explore/components/AggregateOption.jsx b/superset/assets/javascripts/explore/components/AggregateOption.jsx new file mode 100644 index 0000000..e56ccf9 --- /dev/null +++ b/superset/assets/javascripts/explore/components/AggregateOption.jsx @@ -0,0 +1,22 @@ +import React from 'react'; +import PropTypes from 'prop-types'; + +import ColumnTypeLabel from '../../components/ColumnTypeLabel'; +import aggregateOptionType from '../propTypes/aggregateOptionType'; + +const propTypes = { + aggregate: aggregateOptionType, + showType: PropTypes.bool, +}; + +export default function AggregateOption({ aggregate, showType }) { + return ( +
+ {showType && } + + {aggregate.aggregate_name} + +
+ ); +} +AggregateOption.propTypes = propTypes; diff --git a/superset/assets/javascripts/explore/components/MetricDefinitionOption.jsx b/superset/assets/javascripts/explore/components/MetricDefinitionOption.jsx new file mode 100644 index 0000000..c275b9c --- /dev/null +++ b/superset/assets/javascripts/explore/components/MetricDefinitionOption.jsx @@ -0,0 +1,36 @@ +import React from 'react'; +import PropTypes from 'prop-types'; + +import MetricOption from '../../components/MetricOption'; +import ColumnOption from '../../components/ColumnOption'; +import AggregateOption from './AggregateOption'; +import columnType from '../propTypes/columnType'; +import savedMetricType from '../propTypes/savedMetricType'; +import aggregateOptionType from '../propTypes/aggregateOptionType'; + +const propTypes = { + option: PropTypes.oneOfType([ + columnType, + savedMetricType, + aggregateOptionType, + ]).isRequired, +}; + +export default function MetricDefinitionOption({ option }) { + if (option.metric_name) { + return ( + + ); + } else if (option.column_name) { + return ( + + ); + } else if (option.aggregate_name) { + return ( + + ); + } + notify.error('You must supply either a saved metric, column or aggregate to MetricDefinitionOption'); + return null; +} +MetricDefinitionOption.propTypes = propTypes; diff --git a/superset/assets/javascripts/explore/components/MetricDefinitionValue.jsx b/superset/assets/javascripts/explore/components/MetricDefinitionValue.jsx new file mode 100644 index 0000000..4997dce --- /dev/null +++ b/superset/assets/javascripts/explore/components/MetricDefinitionValue.jsx @@ -0,0 +1,47 @@ +import React from 'react'; +import PropTypes from 'prop-types'; + +import AdhocMetricOption from './AdhocMetricOption'; +import AdhocMetric from '../AdhocMetric'; +import columnType from '../propTypes/columnType'; +import MetricOption from '../../components/MetricOption'; +import savedMetricType from '../propTypes/savedMetricType'; +import adhocMetricType from '../propTypes/adhocMetricType'; + +const propTypes = { + option: PropTypes.oneOfType([ + savedMetricType, + adhocMetricType, + ]).isRequired, + onMetricEdit: PropTypes.func, + columns: PropTypes.arrayOf(columnType), + multi: PropTypes.bool, + datasourceType: PropTypes.string, +}; + +export default function MetricDefinitionValue({ + option, + onMetricEdit, + columns, + multi, + datasourceType, +}) { + if (option.metric_name) { + return ( + + ); + } else if (option instanceof AdhocMetric) { + return ( + + ); + } + notify.error('You must supply either a saved metric or adhoc metric to MetricDefinitionValue'); + return null; +} +MetricDefinitionValue.propTypes = propTypes; diff --git a/superset/assets/javascripts/explore/components/controls/MetricsControl.jsx b/superset/assets/javascripts/explore/components/controls/MetricsControl.jsx new file mode 100644 index 0000000..9a16f52 --- /dev/null +++ b/superset/assets/javascripts/explore/components/controls/MetricsControl.jsx @@ -0,0 +1,256 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import VirtualizedSelect from 'react-virtualized-select'; +import ControlHeader from '../ControlHeader'; +import { t } from '../../../locales'; +import VirtualizedRendererWrap from '../../../components/VirtualizedRendererWrap'; +import OnPasteSelect from '../../../components/OnPasteSelect'; +import MetricDefinitionOption from '../MetricDefinitionOption'; +import MetricDefinitionValue from '../MetricDefinitionValue'; +import AdhocMetric from '../../AdhocMetric'; +import columnType from '../../propTypes/columnType'; +import savedMetricType from '../../propTypes/savedMetricType'; +import adhocMetricType from '../../propTypes/adhocMetricType'; +import { AGGREGATES } from '../../constants'; + +const propTypes = { + name: PropTypes.string.isRequired, + onChange: PropTypes.func, + value: PropTypes.oneOfType([ + PropTypes.arrayOf(PropTypes.oneOfType([PropTypes.string, adhocMetricType])), + PropTypes.oneOfType([PropTypes.string, adhocMetricType]), + ]), + columns: PropTypes.arrayOf(columnType), + savedMetrics: PropTypes.arrayOf(savedMetricType), + multi: PropTypes.bool, + datasourceType: PropTypes.string, +}; + +const defaultProps = { + onChange: () => {}, +}; + +function isDictionaryForAdhocMetric(value) { + return value && !(value instanceof AdhocMetric) && value.column && value.aggregate && value.label; +} + +// adhoc metrics are stored as dictionaries in URL params. We convert them back into the +// AdhocMetric class for typechecking, consistency and instance method access. +function coerceAdhocMetrics(value) { + if (!value) { + return []; + } + if (!Array.isArray(value)) { + if (isDictionaryForAdhocMetric(value)) { + return [new AdhocMetric(value)]; + } + return [value]; + } + return value.map((val) => { + if (isDictionaryForAdhocMetric(val)) { + return new AdhocMetric(val); + } + return val; + }); +} + +function getDefaultAggregateForColumn(column) { + const type = column.type; + if (typeof type !== 'string') { + return AGGREGATES.COUNT; + } else if (type === '' || type === 'expression') { + return AGGREGATES.SUM; + } else if (type.match(/.*char.*/i) || type.match(/string.*/i) || type.match(/.*text.*/i)) { + return AGGREGATES.COUNT_DISTINCT; + } else if (type.match(/.*int.*/i) || type === 'LONG' || type === 'DOUBLE' || type === 'FLOAT') { + return AGGREGATES.SUM; + } else if (type.match(/.*bool.*/i)) { + return AGGREGATES.MAX; + } else if (type.match(/.*time.*/i)) { + return AGGREGATES.COUNT; + } else if (type.match(/unknown/i)) { + return AGGREGATES.COUNT; + } + return null; +} + +const autoGeneratedMetricRegex = /^(LONG|DOUBLE|FLOAT)?(SUM|AVG|MAX|MIN|COUNT)\([A-Z_][A-Z0-9_]*\)$/i; +function isAutoGeneratedMetric(savedMetric) { + return ( + autoGeneratedMetricRegex.test(savedMetric.expression) || + autoGeneratedMetricRegex.test(savedMetric.verbose_name) + ); +} + +export default class MetricsControl extends React.PureComponent { + constructor(props) { + super(props); + this.onChange = this.onChange.bind(this); + this.onMetricEdit = this.onMetricEdit.bind(this); + this.checkIfAggregateInInput = this.checkIfAggregateInInput.bind(this); + this.optionsForSelect = this.optionsForSelect.bind(this); + this.selectFilterOption = this.selectFilterOption.bind(this); + this.optionRenderer = VirtualizedRendererWrap(option => ( + + ), { ignoreAutogeneratedMetrics: true }); + this.valueRenderer = option => ( + + ); + this.refFunc = (ref) => { + if (ref) { + // eslint-disable-next-line no-underscore-dangle + this.select = ref._selectRef; + } + }; + this.state = { + aggregateInInput: null, + options: this.optionsForSelect(this.props), + value: coerceAdhocMetrics(this.props.value), + }; + } + + componentWillReceiveProps(nextProps) { + if ( + this.props.columns !== nextProps.columns || + this.props.savedMetrics !== nextProps.savedMetrics + ) { + this.setState({ options: this.optionsForSelect(nextProps) }); + this.props.onChange([]); + } + if (this.props.value !== nextProps.value) { + this.setState({ value: coerceAdhocMetrics(nextProps.value) }); + } + } + + onMetricEdit(changedMetric) { + let newValue = this.state.value.map((value) => { + if (value.optionName === changedMetric.optionName) { + return changedMetric; + } + return value; + }); + if (!this.props.multi) { + newValue = newValue[0]; + } + this.props.onChange(newValue); + } + + onChange(opts) { + let transformedOpts = opts; + if (!this.props.multi) { + transformedOpts = [opts].filter(option => option); + } + let optionValues = transformedOpts.map((option) => { + if (option.metric_name) { + return option.metric_name; + } else if (option.column_name) { + const clearedAggregate = this.clearedAggregateInInput; + this.clearedAggregateInInput = null; + return new AdhocMetric({ + column: option, + aggregate: clearedAggregate || getDefaultAggregateForColumn(option), + }); + } else if (option instanceof AdhocMetric) { + return option; + } else if (option.aggregate_name) { + const newValue = `${option.aggregate_name}()`; + this.select.setInputValue(newValue); + this.select.handleInputChange({ target: { value: newValue } }); + // we need to set a timeout here or the selectionWill be overwritten + // by some browsers (e.g. Chrome) + setTimeout(() => { + this.select.input.input.selectionStart = newValue.length - 1; + this.select.input.input.selectionEnd = newValue.length - 1; + }, 0); + return null; + } + return null; + }).filter(option => option); + if (!this.props.multi) { + optionValues = optionValues[0]; + } + this.props.onChange(optionValues); + } + + checkIfAggregateInInput(input) { + let nextState = { aggregateInInput: null }; + Object.keys(AGGREGATES).forEach((aggregate) => { + if (input.toLowerCase().startsWith(aggregate.toLowerCase() + '(')) { + nextState = { aggregateInInput: aggregate }; + } + }); + this.clearedAggregateInInput = this.state.aggregateInInput; + this.setState(nextState); + } + + optionsForSelect(props) { + const options = [ + ...props.columns, + ...Object.keys(AGGREGATES).map(aggregate => ({ aggregate_name: aggregate })), + ...props.savedMetrics, + ]; + + return options.map((option) => { + if (option.metric_name) { + return { ...option, optionName: option.metric_name }; + } else if (option.column_name) { + return { ...option, optionName: '_col_' + option.column_name }; + } else if (option.aggregate_name) { + return { ...option, optionName: '_aggregate_' + option.aggregate_name }; + } + notify.error(`provided invalid option to MetricsControl, ${option}`); + return null; + }); + } + + selectFilterOption(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.toLowerCase()) >= 0); + } + return option.optionName && + (!option.metric_name || !isAutoGeneratedMetric(option)) && + (option.optionName.toLowerCase().indexOf(filterValue.toLowerCase()) >= 0); + } + + render() { + // TODO figure out why the dropdown isnt appearing as soon as a metric is selected + return ( +
+ + +
+ ); + } +} + +MetricsControl.propTypes = propTypes; +MetricsControl.defaultProps = defaultProps; diff --git a/superset/assets/javascripts/explore/components/controls/index.js b/superset/assets/javascripts/explore/components/controls/index.js index 35aaeef..a7ca463 100644 --- a/superset/assets/javascripts/explore/components/controls/index.js +++ b/superset/assets/javascripts/explore/components/controls/index.js @@ -17,6 +17,7 @@ import TextControl from './TextControl'; import TimeSeriesColumnControl from './TimeSeriesColumnControl'; import ViewportControl from './ViewportControl'; import VizTypeControl from './VizTypeControl'; +import MetricsControl from './MetricsControl'; const controlMap = { AnnotationLayerControl, @@ -38,5 +39,6 @@ const controlMap = { TimeSeriesColumnControl, ViewportControl, VizTypeControl, + MetricsControl, }; export default controlMap; diff --git a/superset/assets/javascripts/explore/constants.js b/superset/assets/javascripts/explore/constants.js new file mode 100644 index 0000000..330841f --- /dev/null +++ b/superset/assets/javascripts/explore/constants.js @@ -0,0 +1,9 @@ +export const AGGREGATES = { + AVG: 'AVG', + COUNT: 'COUNT ', + COUNT_DISTINCT: 'COUNT_DISTINCT', + MAX: 'MAX', + MIN: 'MIN', + SUM: 'SUM', +}; + diff --git a/superset/assets/javascripts/explore/main.css b/superset/assets/javascripts/explore/main.css index 2add0ab..c6fede9 100644 --- a/superset/assets/javascripts/explore/main.css +++ b/superset/assets/javascripts/explore/main.css @@ -39,6 +39,10 @@ width: 100px; } +.control-panel-section .metrics-select .Select-multi-value-wrapper .Select-input > input { + width: 300px; +} + .background-transparent { background-color: transparent !important; } diff --git a/superset/assets/javascripts/explore/propTypes/adhocMetricType.js b/superset/assets/javascripts/explore/propTypes/adhocMetricType.js new file mode 100644 index 0000000..b11126e --- /dev/null +++ b/superset/assets/javascripts/explore/propTypes/adhocMetricType.js @@ -0,0 +1,10 @@ +import PropTypes from 'prop-types'; + +import { AGGREGATES } from '../constants'; +import columnType from './columnType'; + +export default PropTypes.shape({ + column: columnType.isRequired, + aggregate: PropTypes.oneOf(Object.keys(AGGREGATES)).isRequired, + label: PropTypes.string.isRequired, +}); diff --git a/superset/assets/javascripts/explore/propTypes/aggregateOptionType.js b/superset/assets/javascripts/explore/propTypes/aggregateOptionType.js new file mode 100644 index 0000000..0a5eebf --- /dev/null +++ b/superset/assets/javascripts/explore/propTypes/aggregateOptionType.js @@ -0,0 +1,5 @@ +import PropTypes from 'prop-types'; + +export default PropTypes.shape({ + aggregate_name: PropTypes.string.isRequired, +}); diff --git a/superset/assets/javascripts/explore/propTypes/columnType.js b/superset/assets/javascripts/explore/propTypes/columnType.js new file mode 100644 index 0000000..5ff33e5 --- /dev/null +++ b/superset/assets/javascripts/explore/propTypes/columnType.js @@ -0,0 +1,6 @@ +import PropTypes from 'prop-types'; + +export default PropTypes.shape({ + column_name: PropTypes.string.isRequired, + type: PropTypes.string, +}); diff --git a/superset/assets/javascripts/explore/propTypes/savedMetricType.js b/superset/assets/javascripts/explore/propTypes/savedMetricType.js new file mode 100644 index 0000000..3e713a8 --- /dev/null +++ b/superset/assets/javascripts/explore/propTypes/savedMetricType.js @@ -0,0 +1,6 @@ +import PropTypes from 'prop-types'; + +export default PropTypes.shape({ + metric_name: PropTypes.string.isRequired, + expression: PropTypes.string.isRequired, +}); diff --git a/superset/assets/javascripts/explore/stores/controls.jsx b/superset/assets/javascripts/explore/stores/controls.jsx index 617c717..f0d00bf 100644 --- a/superset/assets/javascripts/explore/stores/controls.jsx +++ b/superset/assets/javascripts/explore/stores/controls.jsx @@ -129,19 +129,18 @@ export const controls = { }, metrics: { - type: 'SelectControl', + type: 'MetricsControl', multi: true, label: t('Metrics'), validators: [v.nonEmpty], - valueKey: 'metric_name', - optionRenderer: m => , - valueRenderer: m => , default: (c) => { const metric = mainMetric(c.options); return metric ? [metric] : null; }, mapStateToProps: state => ({ - options: (state.datasource) ? state.datasource.metrics : [], + columns: state.datasource ? state.datasource.columns : [], + savedMetrics: state.datasource ? state.datasource.metrics : [], + datasourceType: state.datasource && state.datasource.type, }), description: t('One or many metrics to display'), }, @@ -219,17 +218,16 @@ export const controls = { }, metric: { - type: 'SelectControl', + type: 'MetricsControl', + multi: false, label: t('Metric'), clearable: false, - description: t('Choose the metric'), validators: [v.nonEmpty], - optionRenderer: m => , - valueRenderer: m => , default: c => mainMetric(c.options), - valueKey: 'metric_name', mapStateToProps: state => ({ - options: (state.datasource) ? state.datasource.metrics : [], + columns: state.datasource ? state.datasource.columns : [], + savedMetrics: state.datasource ? state.datasource.metrics : [], + datasourceType: state.datasource && state.datasource.type, }), }, diff --git a/superset/assets/javascripts/explore/stores/visTypes.js b/superset/assets/javascripts/explore/stores/visTypes.js index 7544346..d848f40 100644 --- a/superset/assets/javascripts/explore/stores/visTypes.js +++ b/superset/assets/javascripts/explore/stores/visTypes.js @@ -139,7 +139,8 @@ export const visTypes = { label: t('Query'), expanded: true, controlSetRows: [ - ['metrics', 'groupby'], + ['metrics'], + ['groupby'], ['limit'], ], }, diff --git a/superset/assets/package.json b/superset/assets/package.json index 7d41be3..5bf03c0 100644 --- a/superset/assets/package.json +++ b/superset/assets/package.json @@ -96,7 +96,7 @@ "react-map-gl": "^3.0.4", "react-redux": "^5.0.2", "react-resizable": "^1.3.3", - "react-select": "1.0.0-rc.10", + "react-select": "1.2.1", "react-select-fast-filter-options": "^0.2.1", "react-sortable-hoc": "^0.6.7", "react-split-pane": "^0.1.66", @@ -127,7 +127,7 @@ "clean-webpack-plugin": "^0.1.16", "css-loader": "^0.28.0", "enzyme": "^2.0.0", - "eslint": "^3.19.0", + "eslint": "^4.19.0", "eslint-config-airbnb": "^15.0.1", "eslint-plugin-import": "^2.2.0", "eslint-plugin-jsx-a11y": "^5.1.1", diff --git a/superset/assets/spec/javascripts/explore/AdhocMetric_spec.js b/superset/assets/spec/javascripts/explore/AdhocMetric_spec.js new file mode 100644 index 0000000..ad9ab47 --- /dev/null +++ b/superset/assets/spec/javascripts/explore/AdhocMetric_spec.js @@ -0,0 +1,85 @@ +import { expect } from 'chai'; +import { describe, it } from 'mocha'; + +import AdhocMetric from '../../../javascripts/explore/AdhocMetric'; +import { AGGREGATES } from '../../../javascripts/explore/constants'; + +const valueColumn = { type: 'DOUBLE', column_name: 'value' }; + +describe('AdhocMetric', () => { + it('sets label, hasCustomLabel and optionName in constructor', () => { + const adhocMetric = new AdhocMetric({ + column: valueColumn, + aggregate: AGGREGATES.SUM, + }); + expect(adhocMetric.optionName.length).to.be.above(10); + expect(adhocMetric).to.deep.equal({ + column: valueColumn, + aggregate: AGGREGATES.SUM, + fromFormData: false, + label: 'SUM(value)', + hasCustomLabel: false, + optionName: adhocMetric.optionName, + }); + }); + + it('can create altered duplicates', () => { + const adhocMetric1 = new AdhocMetric({ + column: valueColumn, + aggregate: AGGREGATES.SUM, + }); + const adhocMetric2 = adhocMetric1.duplicateWith({ aggregate: AGGREGATES.AVG }); + + expect(adhocMetric1.column).to.equal(adhocMetric2.column); + expect(adhocMetric1.column).to.equal(valueColumn); + + expect(adhocMetric1.aggregate).to.equal(AGGREGATES.SUM); + expect(adhocMetric2.aggregate).to.equal(AGGREGATES.AVG); + }); + + it('can verify equality', () => { + const adhocMetric1 = new AdhocMetric({ + column: valueColumn, + aggregate: AGGREGATES.SUM, + }); + const adhocMetric2 = adhocMetric1.duplicateWith({}); + + // eslint-disable-next-line no-unused-expressions + expect(adhocMetric1.equals(adhocMetric2)).to.be.true; + }); + + it('can verify inequality', () => { + const adhocMetric1 = new AdhocMetric({ + column: valueColumn, + aggregate: AGGREGATES.SUM, + label: 'old label', + hasCustomLabel: true, + }); + const adhocMetric2 = adhocMetric1.duplicateWith({ label: 'new label' }); + + // eslint-disable-next-line no-unused-expressions + expect(adhocMetric1.equals(adhocMetric2)).to.be.false; + }); + + it('updates label if hasCustomLabel is false', () => { + const adhocMetric1 = new AdhocMetric({ + column: valueColumn, + aggregate: AGGREGATES.SUM, + }); + const adhocMetric2 = adhocMetric1.duplicateWith({ aggregate: AGGREGATES.AVG }); + + expect(adhocMetric2.label).to.equal('AVG(value)'); + }); + + it('keeps label if hasCustomLabel is true', () => { + const adhocMetric1 = new AdhocMetric({ + column: valueColumn, + aggregate: AGGREGATES.SUM, + hasCustomLabel: true, + label: 'label1', + }); + const adhocMetric2 = adhocMetric1.duplicateWith({ aggregate: AGGREGATES.AVG }); + + expect(adhocMetric2.label).to.equal('label1'); + }); +}); diff --git a/superset/assets/spec/javascripts/explore/components/AdhocMetricEditPopoverTitle_spec.jsx b/superset/assets/spec/javascripts/explore/components/AdhocMetricEditPopoverTitle_spec.jsx new file mode 100644 index 0000000..b732a5a --- /dev/null +++ b/superset/assets/spec/javascripts/explore/components/AdhocMetricEditPopoverTitle_spec.jsx @@ -0,0 +1,48 @@ +/* eslint-disable no-unused-expressions */ +import React from 'react'; +import sinon from 'sinon'; +import { expect } from 'chai'; +import { describe, it } from 'mocha'; +import { shallow } from 'enzyme'; +import { OverlayTrigger } from 'react-bootstrap'; + +import AdhocMetric from '../../../../javascripts/explore/AdhocMetric'; +import AdhocMetricEditPopoverTitle from '../../../../javascripts/explore/components/AdhocMetricEditPopoverTitle'; +import { AGGREGATES } from '../../../../javascripts/explore/constants'; + +const columns = [ + { type: 'VARCHAR(255)', column_name: 'source' }, + { type: 'VARCHAR(255)', column_name: 'target' }, + { type: 'DOUBLE', column_name: 'value' }, +]; + +const sumValueAdhocMetric = new AdhocMetric({ + column: columns[2], + aggregate: AGGREGATES.SUM, +}); + +function setup(overrides) { + const onChange = sinon.spy(); + const props = { + adhocMetric: sumValueAdhocMetric, + onChange, + ...overrides, + }; + const wrapper = shallow(); + return { wrapper, onChange }; +} + +describe('AdhocMetricEditPopoverTitle', () => { + it('renders an OverlayTrigger wrapper with the title', () => { + const { wrapper } = setup(); + expect(wrapper.find(OverlayTrigger)).to.have.lengthOf(1); + expect(wrapper.find(OverlayTrigger).dive().text()).to.equal('My Metric\xa0'); + }); + + it('transfers to edit mode when clicked', () => { + const { wrapper } = setup(); + expect(wrapper.state('isEditable')).to.be.false; + wrapper.simulate('click'); + expect(wrapper.state('isEditable')).to.be.true; + }); +}); diff --git a/superset/assets/spec/javascripts/explore/components/AdhocMetricEditPopover_spec.jsx b/superset/assets/spec/javascripts/explore/components/AdhocMetricEditPopover_spec.jsx new file mode 100644 index 0000000..0ba69fb --- /dev/null +++ b/superset/assets/spec/javascripts/explore/components/AdhocMetricEditPopover_spec.jsx @@ -0,0 +1,90 @@ +/* eslint-disable no-unused-expressions */ +import React from 'react'; +import sinon from 'sinon'; +import { expect } from 'chai'; +import { describe, it } from 'mocha'; +import { shallow } from 'enzyme'; +import { Button, FormGroup, Popover } from 'react-bootstrap'; + +import AdhocMetric from '../../../../javascripts/explore/AdhocMetric'; +import AdhocMetricEditPopover from '../../../../javascripts/explore/components/AdhocMetricEditPopover'; +import { AGGREGATES } from '../../../../javascripts/explore/constants'; + +const columns = [ + { type: 'VARCHAR(255)', column_name: 'source' }, + { type: 'VARCHAR(255)', column_name: 'target' }, + { type: 'DOUBLE', column_name: 'value' }, +]; + +const sumValueAdhocMetric = new AdhocMetric({ + column: columns[2], + aggregate: AGGREGATES.SUM, +}); + +function setup(overrides) { + const onChange = sinon.spy(); + const onClose = sinon.spy(); + const props = { + adhocMetric: sumValueAdhocMetric, + onChange, + onClose, + columns, + ...overrides, + }; + const wrapper = shallow(); + return { wrapper, onChange, onClose }; +} + +describe('AdhocMetricEditPopover', () => { + it('renders a popover with edit metric form contents', () => { + const { wrapper } = setup(); + expect(wrapper.find(Popover)).to.have.lengthOf(1); + expect(wrapper.find(FormGroup)).to.have.lengthOf(2); + expect(wrapper.find(Button)).to.have.lengthOf(2); + }); + + it('overwrites the adhocMetric in state with onColumnChange', () => { + const { wrapper } = setup(); + wrapper.instance().onColumnChange(columns[0]); + expect(wrapper.state('adhocMetric')).to.deep.equal(sumValueAdhocMetric.duplicateWith({ column: columns[0] })); + }); + + it('overwrites the adhocMetric in state with onAggregateChange', () => { + const { wrapper } = setup(); + wrapper.instance().onAggregateChange({ aggregate: AGGREGATES.AVG }); + expect(wrapper.state('adhocMetric')).to.deep.equal(sumValueAdhocMetric.duplicateWith({ aggregate: AGGREGATES.AVG })); + }); + + it('overwrites the adhocMetric in state with onLabelChange', () => { + const { wrapper } = setup(); + wrapper.instance().onLabelChange({ target: { value: 'new label' } }); + expect(wrapper.state('adhocMetric').label).to.equal('new label'); + expect(wrapper.state('adhocMetric').hasCustomLabel).to.be.true; + }); + + it('returns to default labels when the custom label is cleared', () => { + const { wrapper } = setup(); + wrapper.instance().onLabelChange({ target: { value: 'new label' } }); + wrapper.instance().onLabelChange({ target: { value: '' } }); + expect(wrapper.state('adhocMetric').label).to.equal('SUM(value)'); + expect(wrapper.state('adhocMetric').hasCustomLabel).to.be.false; + }); + + it('prevents saving if no column or aggregate is chosen', () => { + const { wrapper } = setup(); + expect(wrapper.find(Button).find({ disabled: true })).to.have.lengthOf(0); + wrapper.instance().onColumnChange(null); + expect(wrapper.find(Button).find({ disabled: true })).to.have.lengthOf(1); + wrapper.instance().onColumnChange({ column: columns[0] }); + expect(wrapper.find(Button).find({ disabled: true })).to.have.lengthOf(0); + wrapper.instance().onAggregateChange(null); + expect(wrapper.find(Button).find({ disabled: true })).to.have.lengthOf(1); + }); + + it('highlights save if changes are present', () => { + const { wrapper } = setup(); + expect(wrapper.find(Button).find({ bsStyle: 'primary' })).to.have.lengthOf(0); + wrapper.instance().onColumnChange({ column: columns[1] }); + expect(wrapper.find(Button).find({ bsStyle: 'primary' })).to.have.lengthOf(1); + }); +}); diff --git a/superset/assets/spec/javascripts/explore/components/AdhocMetricOption_spec.jsx b/superset/assets/spec/javascripts/explore/components/AdhocMetricOption_spec.jsx new file mode 100644 index 0000000..ac36825 --- /dev/null +++ b/superset/assets/spec/javascripts/explore/components/AdhocMetricOption_spec.jsx @@ -0,0 +1,42 @@ +/* eslint-disable no-unused-expressions */ +import React from 'react'; +import sinon from 'sinon'; +import { expect } from 'chai'; +import { describe, it } from 'mocha'; +import { shallow } from 'enzyme'; +import { Label, OverlayTrigger } from 'react-bootstrap'; + +import AdhocMetric from '../../../../javascripts/explore/AdhocMetric'; +import AdhocMetricOption from '../../../../javascripts/explore/components/AdhocMetricOption'; +import { AGGREGATES } from '../../../../javascripts/explore/constants'; + +const columns = [ + { type: 'VARCHAR(255)', column_name: 'source' }, + { type: 'VARCHAR(255)', column_name: 'target' }, + { type: 'DOUBLE', column_name: 'value' }, +]; + +const sumValueAdhocMetric = new AdhocMetric({ + column: columns[2], + aggregate: AGGREGATES.SUM, +}); + +function setup(overrides) { + const onMetricEdit = sinon.spy(); + const props = { + adhocMetric: sumValueAdhocMetric, + onMetricEdit, + columns, + ...overrides, + }; + const wrapper = shallow(); + return { wrapper, onMetricEdit }; +} + +describe('AdhocMetricOption', () => { + it('renders an overlay trigger wrapper for the label', () => { + const { wrapper } = setup(); + expect(wrapper.find(OverlayTrigger)).to.have.lengthOf(1); + expect(wrapper.find(Label)).to.have.lengthOf(1); + }); +}); diff --git a/superset/assets/spec/javascripts/explore/components/AggregateOption_spec.jsx b/superset/assets/spec/javascripts/explore/components/AggregateOption_spec.jsx new file mode 100644 index 0000000..a1fb317 --- /dev/null +++ b/superset/assets/spec/javascripts/explore/components/AggregateOption_spec.jsx @@ -0,0 +1,14 @@ +/* eslint-disable no-unused-expressions */ +import React from 'react'; +import { expect } from 'chai'; +import { describe, it } from 'mocha'; +import { shallow } from 'enzyme'; + +import AggregateOption from '../../../../javascripts/explore/components/AggregateOption'; + +describe('AggregateOption', () => { + it('renders the aggregate', () => { + const wrapper = shallow(); + expect(wrapper.text()).to.equal('SUM'); + }); +}); diff --git a/superset/assets/spec/javascripts/explore/components/MetricDefinitionOption_spec.jsx b/superset/assets/spec/javascripts/explore/components/MetricDefinitionOption_spec.jsx new file mode 100644 index 0000000..e39c225 --- /dev/null +++ b/superset/assets/spec/javascripts/explore/components/MetricDefinitionOption_spec.jsx @@ -0,0 +1,27 @@ +/* eslint-disable no-unused-expressions */ +import React from 'react'; +import { expect } from 'chai'; +import { describe, it } from 'mocha'; +import { shallow } from 'enzyme'; + +import MetricDefinitionOption from '../../../../javascripts/explore/components/MetricDefinitionOption'; +import MetricOption from '../../../../javascripts/components/MetricOption'; +import ColumnOption from '../../../../javascripts/components/ColumnOption'; +import AggregateOption from '../../../../javascripts/explore/components/AggregateOption'; + +describe('MetricDefinitionOption', () => { + it('renders a MetricOption given a saved metric', () => { + const wrapper = shallow(); + expect(wrapper.find(MetricOption)).to.have.lengthOf(1); + }); + + it('renders a ColumnOption given a column', () => { + const wrapper = shallow(); + expect(wrapper.find(ColumnOption)).to.have.lengthOf(1); + }); + + it('renders an AggregateOption given an aggregate metric', () => { + const wrapper = shallow(); + expect(wrapper.find(AggregateOption)).to.have.lengthOf(1); + }); +}); diff --git a/superset/assets/spec/javascripts/explore/components/MetricDefinitionValue_spec.jsx b/superset/assets/spec/javascripts/explore/components/MetricDefinitionValue_spec.jsx new file mode 100644 index 0000000..5690c21 --- /dev/null +++ b/superset/assets/spec/javascripts/explore/components/MetricDefinitionValue_spec.jsx @@ -0,0 +1,30 @@ +/* eslint-disable no-unused-expressions */ +import React from 'react'; +import { expect } from 'chai'; +import { describe, it } from 'mocha'; +import { shallow } from 'enzyme'; + +import MetricDefinitionValue from '../../../../javascripts/explore/components/MetricDefinitionValue'; +import MetricOption from '../../../../javascripts/components/MetricOption'; +import AdhocMetricOption from '../../../../javascripts/explore/components/AdhocMetricOption'; +import AdhocMetric from '../../../../javascripts/explore/AdhocMetric'; +import { AGGREGATES } from '../../../../javascripts/explore/constants'; + +const sumValueAdhocMetric = new AdhocMetric({ + column: { type: 'DOUBLE', column_name: 'value' }, + aggregate: AGGREGATES.SUM, +}); + +describe('MetricDefinitionValue', () => { + it('renders a MetricOption given a saved metric', () => { + const wrapper = shallow(); + expect(wrapper.find(MetricOption)).to.have.lengthOf(1); + }); + + it('renders an AdhocMetricOption given an adhoc metric', () => { + const wrapper = shallow(( + {}} option={sumValueAdhocMetric} /> + )); + expect(wrapper.find(AdhocMetricOption)).to.have.lengthOf(1); + }); +}); diff --git a/superset/assets/spec/javascripts/explore/components/MetricsControl_spec.jsx b/superset/assets/spec/javascripts/explore/components/MetricsControl_spec.jsx new file mode 100644 index 0000000..346a287 --- /dev/null +++ b/superset/assets/spec/javascripts/explore/components/MetricsControl_spec.jsx @@ -0,0 +1,250 @@ +/* eslint-disable no-unused-expressions */ +import React from 'react'; +import sinon from 'sinon'; +import { expect } from 'chai'; +import { describe, it } from 'mocha'; +import { shallow } from 'enzyme'; + +import MetricsControl from '../../../../javascripts/explore/components/controls/MetricsControl'; +import { AGGREGATES } from '../../../../javascripts/explore/constants'; +import OnPasteSelect from '../../../../javascripts/components/OnPasteSelect'; +import AdhocMetric from '../../../../javascripts/explore/AdhocMetric'; + +const defaultProps = { + name: 'metrics', + label: 'Metrics', + value: undefined, + multi: true, + columns: [ + { type: 'VARCHAR(255)', column_name: 'source' }, + { type: 'VARCHAR(255)', column_name: 'target' }, + { type: 'DOUBLE', column_name: 'value' }, + ], + savedMetrics: [ + { metric_name: 'sum__value', expression: 'SUM(energy_usage.value)' }, + { metric_name: 'avg__value', expression: 'AVG(energy_usage.value)' }, + ], +}; + +function setup(overrides) { + const onChange = sinon.spy(); + const props = { + onChange, + ...defaultProps, + ...overrides, + }; + const wrapper = shallow(); + return { wrapper, onChange }; +} + +const valueColumn = { type: 'DOUBLE', column_name: 'value' }; + +const sumValueAdhocMetric = new AdhocMetric({ + column: valueColumn, + aggregate: AGGREGATES.SUM, + label: 'SUM(value)', +}); + +describe('MetricsControl', () => { + + it('renders an OnPasteSelect', () => { + const { wrapper } = setup(); + expect(wrapper.find(OnPasteSelect)).to.have.lengthOf(1); + }); + + describe('constructor', () => { + + it('unifies options for the dropdown select with aggregates', () => { + const { wrapper } = setup(); + expect(wrapper.state('options')).to.deep.equal([ + { 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('coerces Adhoc Metrics from form data into instances of the AdhocMetric class and leaves saved metrics', () => { + const { wrapper } = setup({ + value: [ + { + column: { type: 'double', column_name: 'value' }, + aggregate: AGGREGATES.SUM, + label: 'SUM(value)', + optionName: 'blahblahblah', + }, + 'avg__value', + ], + }); + + const adhocMetric = wrapper.state('value')[0]; + expect(adhocMetric instanceof AdhocMetric).to.be.true; + expect(adhocMetric.optionName.length).to.be.above(10); + expect(wrapper.state('value')).to.deep.equal([ + { + column: { type: 'double', column_name: 'value' }, + aggregate: AGGREGATES.SUM, + fromFormData: true, + label: 'SUM(value)', + hasCustomLabel: false, + optionName: 'blahblahblah', + }, + 'avg__value', + ]); + }); + + }); + + describe('onChange', () => { + + it('handles saved metrics being selected', () => { + const { wrapper, onChange } = setup(); + const select = wrapper.find(OnPasteSelect); + select.simulate('change', [{ metric_name: 'sum__value' }]); + expect(onChange.lastCall.args).to.deep.equal([['sum__value']]); + }); + + it('handles columns being selected', () => { + const { wrapper, onChange } = setup(); + const select = wrapper.find(OnPasteSelect); + select.simulate('change', [valueColumn]); + + const adhocMetric = onChange.lastCall.args[0][0]; + expect(adhocMetric instanceof AdhocMetric).to.be.true; + expect(onChange.lastCall.args).to.deep.equal([[{ + column: valueColumn, + aggregate: AGGREGATES.SUM, + label: 'SUM(value)', + fromFormData: false, + hasCustomLabel: false, + optionName: adhocMetric.optionName, + }]]); + }); + + it('handles aggregates being selected', () => { + const { wrapper, onChange } = setup(); + const select = wrapper.find(OnPasteSelect); + + // mock out the Select ref + const setInputSpy = sinon.spy(); + const handleInputSpy = sinon.spy(); + wrapper.instance().select = { + setInputValue: setInputSpy, + handleInputChange: handleInputSpy, + input: { input: {} }, + }; + + select.simulate('change', [{ aggregate_name: 'SUM', optionName: 'SUM' }]); + + expect(setInputSpy.calledWith('SUM()')).to.be.true; + expect(handleInputSpy.calledWith({ target: { value: 'SUM()' } })).to.be.true; + expect(onChange.lastCall.args).to.deep.equal([[]]); + }); + + it('preserves existing selected AdhocMetrics', () => { + const { wrapper, onChange } = setup(); + const select = wrapper.find(OnPasteSelect); + select.simulate('change', [{ metric_name: 'sum__value' }, sumValueAdhocMetric]); + expect(onChange.lastCall.args).to.deep.equal([['sum__value', sumValueAdhocMetric]]); + }); + }); + + describe('onMetricEdit', () => { + it('accepts an edited metric from an AdhocMetricEditPopover', () => { + const { wrapper, onChange } = setup({ + value: [sumValueAdhocMetric], + }); + + const editedMetric = sumValueAdhocMetric.duplicateWith({ aggregate: AGGREGATES.AVG }); + wrapper.instance().onMetricEdit(editedMetric); + + expect(onChange.lastCall.args).to.deep.equal([[ + editedMetric, + ]]); + }); + }); + + describe('checkIfAggregateInInput', () => { + it('handles an aggregate in the input', () => { + const { wrapper } = setup(); + + expect(wrapper.state('aggregateInInput')).to.be.null; + wrapper.instance().checkIfAggregateInInput('AVG('); + expect(wrapper.state('aggregateInInput')).to.equal(AGGREGATES.AVG); + }); + + it('handles no aggregate in the input', () => { + const { wrapper } = setup(); + + expect(wrapper.state('aggregateInInput')).to.be.null; + wrapper.instance().checkIfAggregateInInput('colu'); + expect(wrapper.state('aggregateInInput')).to.be.null; + }); + }); + + describe('option filter', () => { + it('includes user defined metrics', () => { + const { wrapper } = setup(); + + expect(!!wrapper.instance().selectFilterOption( + { + metric_name: 'a_metric', + optionName: 'a_metric', + expression: 'SUM(FANCY(metric))', + }, + 'a', + )).to.be.true; + }); + + it('includes columns and aggregates', () => { + const { wrapper } = setup(); + + expect(!!wrapper.instance().selectFilterOption( + { type: 'VARCHAR(255)', column_name: 'source', optionName: '_col_source' }, + 'Sou', + )).to.be.true; + + expect(!!wrapper.instance().selectFilterOption( + { aggregate_name: 'AVG', optionName: '_aggregate_AVG' }, + 'av', + )).to.be.true; + }); + + it('excludes auto generated metrics', () => { + const { wrapper } = setup(); + + expect(!!wrapper.instance().selectFilterOption( + { + metric_name: 'sum__value', + optionName: 'sum__value', + expression: 'SUM(value)', + }, + 'sum', + )).to.be.false; + }); + + it('filters out metrics if the input begins with an aggregate', () => { + const { wrapper } = setup(); + wrapper.setState({ aggregateInInput: true }); + + expect(!!wrapper.instance().selectFilterOption( + { metric_name: 'metric', expression: 'SUM(FANCY(metric))' }, + 'SUM(', + )).to.be.false; + }); + + it('includes columns if the input begins with an aggregate', () => { + const { wrapper } = setup(); + wrapper.setState({ aggregateInInput: true }); + + expect(!!wrapper.instance().selectFilterOption( + { type: 'DOUBLE', column_name: 'value' }, + 'SUM(', + )).to.be.true; + }); + }); +}); diff --git a/superset/assets/stylesheets/superset.less b/superset/assets/stylesheets/superset.less index 4ac5ba8..035acce 100644 --- a/superset/assets/stylesheets/superset.less +++ b/superset/assets/stylesheets/superset.less @@ -449,3 +449,9 @@ g.annotation-container { color: @brand-primary; border-color: @brand-primary; } + +.metric-edit-popover-label-input { + border-radius: 4px; + height: 30px; + padding-left: 10px; +} diff --git a/superset/connectors/druid/models.py b/superset/connectors/druid/models.py index 107e3c8..d514a2f 100644 --- a/superset/connectors/druid/models.py +++ b/superset/connectors/druid/models.py @@ -897,13 +897,16 @@ class DruidDatasource(Model, BaseDatasource): def metrics_and_post_aggs(metrics, metrics_dict): # Separate metrics into those that are aggregations # and those that are post aggregations - agg_names = set() + saved_agg_names = set() + adhoc_agg_configs = [] postagg_names = [] - for metric_name in metrics: - if metrics_dict[metric_name].metric_type != 'postagg': - agg_names.add(metric_name) + for metric in metrics: + if utils.is_adhoc_metric(metric): + adhoc_agg_configs.append(metric) + elif metrics_dict[metric].metric_type != 'postagg': + saved_agg_names.add(metric) else: - postagg_names.append(metric_name) + postagg_names.append(metric) # Create the post aggregations, maintain order since postaggs # may depend on previous ones post_aggs = OrderedDict() @@ -912,8 +915,8 @@ class DruidDatasource(Model, BaseDatasource): postagg = metrics_dict[postagg_name] visited_postaggs.add(postagg_name) DruidDatasource.resolve_postagg( - postagg, post_aggs, agg_names, visited_postaggs, metrics_dict) - return list(agg_names), post_aggs + postagg, post_aggs, saved_agg_names, visited_postaggs, metrics_dict) + return list(saved_agg_names), adhoc_agg_configs, post_aggs def values_for_column(self, column_name, @@ -968,11 +971,29 @@ class DruidDatasource(Model, BaseDatasource): ret = Filter(type='and', fields=[ff, dim_filter]) return ret - def get_aggregations(self, all_metrics): + @staticmethod + def druid_type_from_adhoc_metric(adhoc_metric): + column_type = adhoc_metric.get('column').get('type').lower() + aggregate = adhoc_metric.get('aggregate').lower() + if (aggregate == 'count'): + return 'count' + if (aggregate == 'count_distinct'): + return 'cardinality' + else: + return column_type + aggregate.capitalize() + + def get_aggregations(self, saved_metrics, adhoc_metrics=[]): aggregations = OrderedDict() for m in self.metrics: - if m.metric_name in all_metrics: + if m.metric_name in saved_metrics: aggregations[m.metric_name] = m.json_obj + for adhoc_metric in adhoc_metrics: + aggregations[adhoc_metric['label']] = { + 'fieldName': adhoc_metric['column']['column_name'], + 'fieldNames': [adhoc_metric['column']['column_name']], + 'type': self.druid_type_from_adhoc_metric(adhoc_metric), + 'name': adhoc_metric['label'], + } return aggregations def check_restricted_metrics(self, aggregations): @@ -1066,11 +1087,11 @@ class DruidDatasource(Model, BaseDatasource): metrics_dict = {m.metric_name: m for m in self.metrics} columns_dict = {c.column_name: c for c in self.columns} - all_metrics, post_aggs = DruidDatasource.metrics_and_post_aggs( + saved_metrics, adhoc_metrics, post_aggs = DruidDatasource.metrics_and_post_aggs( metrics, metrics_dict) - aggregations = self.get_aggregations(all_metrics) + aggregations = self.get_aggregations(saved_metrics, adhoc_metrics) self.check_restricted_metrics(aggregations) # the dimensions list with dimensionSpecs expanded @@ -1246,6 +1267,7 @@ class DruidDatasource(Model, BaseDatasource): cols += query_obj.get('columns') or [] cols += query_obj.get('metrics') or [] + cols = utils.get_metric_names(cols) cols = [col for col in cols if col in df.columns] df = df[cols] diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py index e2dca32..c8ee402 100644 --- a/superset/connectors/sqla/models.py +++ b/superset/connectors/sqla/models.py @@ -278,6 +278,15 @@ class SqlaTable(Model, BaseDatasource): export_parent = 'database' export_children = ['metrics', 'columns'] + sqla_aggregations = { + 'COUNT_DISTINCT': lambda column_name: sa.func.COUNT(sa.distinct(column_name)), + 'COUNT': sa.func.COUNT, + 'SUM': sa.func.SUM, + 'AVG': sa.func.AVG, + 'MIN': sa.func.MIN, + 'MAX': sa.func.MAX, + } + def __repr__(self): return self.name @@ -436,6 +445,12 @@ class SqlaTable(Model, BaseDatasource): return TextAsFrom(sa.text(from_sql), []).alias('expr_qry') return self.get_sqla_table() + def adhoc_metric_to_sa(self, metric): + column_name = metric.get('column').get('column_name') + sa_metric = self.sqla_aggregations[metric.get('aggregate')](column(column_name)) + sa_metric = sa_metric.label(metric.get('label')) + return sa_metric + def get_sqla_query( # sqla self, groupby, metrics, @@ -484,10 +499,14 @@ class SqlaTable(Model, BaseDatasource): 'and is required by this type of chart')) if not groupby and not metrics and not columns: raise Exception(_('Empty query?')) + metrics_exprs = [] for m in metrics: - if m not in metrics_dict: + if utils.is_adhoc_metric(m): + metrics_exprs.append(self.adhoc_metric_to_sa(m)) + elif m in metrics_dict: + metrics_exprs.append(metrics_dict.get(m).sqla_col) + else: raise Exception(_("Metric '{}' is not valid".format(m))) - metrics_exprs = [metrics_dict.get(m).sqla_col for m in metrics] if metrics_exprs: main_metric_expr = metrics_exprs[0] else: diff --git a/superset/utils.py b/superset/utils.py index 8e91e8d..4163739 100644 --- a/superset/utils.py +++ b/superset/utils.py @@ -832,8 +832,7 @@ def get_or_create_main_db(): dbobj = ( db.session.query(models.Database) .filter_by(database_name='main') - .first() - ) + .first()) if not dbobj: dbobj = models.Database(database_name='main') dbobj.set_sqlalchemy_uri(conf.get('SQLALCHEMY_DATABASE_URI')) @@ -842,3 +841,14 @@ def get_or_create_main_db(): db.session.add(dbobj) db.session.commit() return dbobj + + +def is_adhoc_metric(metric): + return (isinstance(metric, dict) and + metric['column'] and + metric['aggregate'] and + metric['label']) + + +def get_metric_names(metrics): + return [metric['label'] if is_adhoc_metric(metric) else metric for metric in metrics] diff --git a/superset/viz.py b/superset/viz.py index fc87430..5d98d5e 100644 --- a/superset/viz.py +++ b/superset/viz.py @@ -64,7 +64,14 @@ class BaseViz(object): self.query = '' self.token = self.form_data.get( 'token', 'token_' + uuid.uuid4().hex[:8]) - self.metrics = self.form_data.get('metrics') or [] + metrics = self.form_data.get('metrics') or [] + self.metrics = [] + for metric in metrics: + if isinstance(metric, dict): + self.metrics.append(metric['label']) + else: + self.metrics.append(metric) + self.groupby = self.form_data.get('groupby') or [] self.time_shift = timedelta() @@ -1058,12 +1065,12 @@ class NVD3TimeSeriesViz(NVD3Viz): df = df.pivot_table( index=DTTM_ALIAS, columns=fd.get('groupby'), - values=fd.get('metrics')) + values=utils.get_metric_names(fd.get('metrics'))) else: df = df.pivot_table( index=DTTM_ALIAS, columns=fd.get('groupby'), - values=fd.get('metrics'), + values=utils.get_metric_names(fd.get('metrics')), fill_value=0, aggfunc=sum) diff --git a/tests/druid_func_tests.py b/tests/druid_func_tests.py index 5b535e9..22c1f38 100644 --- a/tests/druid_func_tests.py +++ b/tests/druid_func_tests.py @@ -180,6 +180,51 @@ class DruidFuncTestCase(unittest.TestCase): self.assertIn('post_aggregations', called_args) # restore functions + def test_run_query_with_adhoc_metric(self): + client = Mock() + from_dttm = Mock() + to_dttm = Mock() + from_dttm.replace = Mock(return_value=from_dttm) + to_dttm.replace = Mock(return_value=to_dttm) + from_dttm.isoformat = Mock(return_value='from') + to_dttm.isoformat = Mock(return_value='to') + timezone = 'timezone' + from_dttm.tzname = Mock(return_value=timezone) + ds = DruidDatasource(datasource_name='datasource') + metric1 = DruidMetric(metric_name='metric1') + metric2 = DruidMetric(metric_name='metric2') + ds.metrics = [metric1, metric2] + col1 = DruidColumn(column_name='col1') + col2 = DruidColumn(column_name='col2') + ds.columns = [col1, col2] + all_metrics = [] + post_aggs = ['some_agg'] + ds._metrics_and_post_aggs = Mock(return_value=(all_metrics, post_aggs)) + groupby = [] + metrics = [{ + 'column': {'type': 'DOUBLE', 'column_name': 'col1'}, + 'aggregate': 'SUM', + 'label': 'My Adhoc Metric', + }] + + ds.get_having_filters = Mock(return_value=[]) + client.query_builder = Mock() + client.query_builder.last_query = Mock() + client.query_builder.last_query.query_dict = {'mock': 0} + # no groupby calls client.timeseries + ds.run_query( + groupby, metrics, None, from_dttm, + to_dttm, client=client, filter=[], row_limit=100, + ) + self.assertEqual(0, len(client.topn.call_args_list)) + self.assertEqual(0, len(client.groupby.call_args_list)) + self.assertEqual(1, len(client.timeseries.call_args_list)) + # check that there is no dimensions entry + called_args = client.timeseries.call_args_list[0][1] + self.assertNotIn('dimensions', called_args) + self.assertIn('post_aggregations', called_args) + # restore functions + def test_run_query_single_groupby(self): client = Mock() from_dttm = Mock() @@ -467,7 +512,7 @@ class DruidFuncTestCase(unittest.TestCase): depends_on('I', ['H', 'K']) depends_on('J', 'K') depends_on('K', ['m8', 'm9']) - all_metrics, postaggs = DruidDatasource.metrics_and_post_aggs( + all_metrics, saved_metrics, postaggs = DruidDatasource.metrics_and_post_aggs( metrics, metrics_dict) expected_metrics = set(all_metrics) self.assertEqual(9, len(all_metrics)) @@ -541,25 +586,80 @@ class DruidFuncTestCase(unittest.TestCase): ), } + adhoc_metric = { + 'column': {'type': 'DOUBLE', 'column_name': 'value'}, + 'aggregate': 'SUM', + 'label': 'My Adhoc Metric', + } + metrics = ['some_sum'] - all_metrics, post_aggs = DruidDatasource.metrics_and_post_aggs( + saved_metrics, adhoc_metrics, post_aggs = DruidDatasource.metrics_and_post_aggs( + metrics, metrics_dict) + + assert saved_metrics == ['some_sum'] + assert adhoc_metrics == [] + assert post_aggs == {} + + metrics = [adhoc_metric] + saved_metrics, adhoc_metrics, post_aggs = DruidDatasource.metrics_and_post_aggs( + metrics, metrics_dict) + + assert saved_metrics == [] + assert adhoc_metrics == [adhoc_metric] + assert post_aggs == {} + + metrics = ['some_sum', adhoc_metric] + saved_metrics, adhoc_metrics, post_aggs = DruidDatasource.metrics_and_post_aggs( metrics, metrics_dict) - assert all_metrics == ['some_sum'] + assert saved_metrics == ['some_sum'] + assert adhoc_metrics == [adhoc_metric] assert post_aggs == {} metrics = ['quantile_p95'] - all_metrics, post_aggs = DruidDatasource.metrics_and_post_aggs( + saved_metrics, adhoc_metrics, post_aggs = DruidDatasource.metrics_and_post_aggs( metrics, metrics_dict) result_postaggs = set(['quantile_p95']) - assert all_metrics == ['a_histogram'] + assert saved_metrics == ['a_histogram'] + assert adhoc_metrics == [] assert set(post_aggs.keys()) == result_postaggs metrics = ['aCustomPostAgg'] - all_metrics, post_aggs = DruidDatasource.metrics_and_post_aggs( + saved_metrics, adhoc_metrics, post_aggs = DruidDatasource.metrics_and_post_aggs( metrics, metrics_dict) result_postaggs = set(['aCustomPostAgg']) - assert all_metrics == ['aCustomMetric'] + assert saved_metrics == ['aCustomMetric'] + assert adhoc_metrics == [] assert set(post_aggs.keys()) == result_postaggs + + def test_druid_type_from_adhoc_metric(self): + + druid_type = DruidDatasource.druid_type_from_adhoc_metric({ + 'column': {'type': 'DOUBLE', 'column_name': 'value'}, + 'aggregate': 'SUM', + 'label': 'My Adhoc Metric', + }) + assert(druid_type == 'doubleSum') + + druid_type = DruidDatasource.druid_type_from_adhoc_metric({ + 'column': {'type': 'LONG', 'column_name': 'value'}, + 'aggregate': 'MAX', + 'label': 'My Adhoc Metric', + }) + assert(druid_type == 'longMax') + + druid_type = DruidDatasource.druid_type_from_adhoc_metric({ + 'column': {'type': 'VARCHAR(255)', 'column_name': 'value'}, + 'aggregate': 'COUNT', + 'label': 'My Adhoc Metric', + }) + assert(druid_type == 'count') + + druid_type = DruidDatasource.druid_type_from_adhoc_metric({ + 'column': {'type': 'VARCHAR(255)', 'column_name': 'value'}, + 'aggregate': 'COUNT_DISTINCT', + 'label': 'My Adhoc Metric', + }) + assert(druid_type == 'cardinality') -- To stop receiving notification emails like this one, please contact maximebeauchemin@apache.org.