From commits-return-2767-archive-asf-public=cust-asf.ponee.io@superset.incubator.apache.org Sat Jun 1 01:28:17 2019 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 [207.244.88.153]) by mx-eu-01.ponee.io (Postfix) with SMTP id 5595E1807BD for ; Sat, 1 Jun 2019 03:28:16 +0200 (CEST) Received: (qmail 68137 invoked by uid 500); 1 Jun 2019 01:28:15 -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 68114 invoked by uid 99); 1 Jun 2019 01:28:15 -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; Sat, 01 Jun 2019 01:28:15 +0000 Received: by gitbox.apache.org (ASF Mail Server at gitbox.apache.org, from userid 33) id A13EA8A577; Sat, 1 Jun 2019 01:28:15 +0000 (UTC) Date: Sat, 01 Jun 2019 01:28:21 +0000 To: "commits@superset.apache.org" Subject: [incubator-superset] 07/13: Adding controls for verifying options (#7468) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit From: michellet@apache.org In-Reply-To: <155935249403.2146.12212886507610984301@gitbox.apache.org> References: <155935249403.2146.12212886507610984301@gitbox.apache.org> X-Git-Host: gitbox.apache.org X-Git-Repo: incubator-superset X-Git-Refname: refs/heads/release--0.33 X-Git-Reftype: branch X-Git-Rev: c4dc38a34d6bddfc667fe2db5e44889c67380920 X-Git-NotificationType: diff X-Git-Multimail-Version: 1.5.dev Auto-Submitted: auto-generated Message-Id: <20190601012815.A13EA8A577@gitbox.apache.org> This is an automated email from the ASF dual-hosted git repository. michellet pushed a commit to branch release--0.33 in repository https://gitbox.apache.org/repos/asf/incubator-superset.git commit c4dc38a34d6bddfc667fe2db5e44889c67380920 Author: michellethomas AuthorDate: Wed May 22 16:39:01 2019 -0700 Adding controls for verifying options (#7468) * Creating withVerification HOC * Updating to use componentDidMount and componentDidUpdate and adding propTypes * Adding tests to withVerification * Adding documentation to withVerification (cherry picked from commit 421183d3f46c48215e88e9d7d285f2dc6c7ccfe6) --- .../explore/components/withVerification_spec.jsx | 106 +++++++++++++++++++++ .../src/explore/components/controls/index.js | 4 + .../components/controls/withVerification.jsx | 88 +++++++++++++++++ 3 files changed, 198 insertions(+) diff --git a/superset/assets/spec/javascripts/explore/components/withVerification_spec.jsx b/superset/assets/spec/javascripts/explore/components/withVerification_spec.jsx new file mode 100644 index 0000000..44377ea --- /dev/null +++ b/superset/assets/spec/javascripts/explore/components/withVerification_spec.jsx @@ -0,0 +1,106 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import React from 'react'; +import sinon from 'sinon'; +import { shallow } from 'enzyme'; +import fetchMock from 'fetch-mock'; + +import MetricsControl from '../../../../src/explore/components/controls/MetricsControl'; +import withVerification from '../../../../src/explore/components/controls/withVerification'; + +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)' }, + ], + datasourceType: 'sqla', + getEndpoint: controlValues => `valid_metrics?data=${controlValues}`, +}; + +const VALID_METRIC = { metric_name: 'sum__value', expression: 'SUM(energy_usage.value)' }; + +function setup(overrides) { + const onChange = sinon.spy(); + const props = { + onChange, + ...defaultProps, + ...overrides, + }; + const VerifiedControl = withVerification(MetricsControl, 'metric_name', 'savedMetrics'); + const wrapper = shallow(); + fetchMock.mock('glob:*/valid_metrics*', `["${VALID_METRIC.metric_name}"]`); + return { props, wrapper, onChange }; +} + +afterEach(fetchMock.restore); + +describe('VerifiedMetricsControl', () => { + + it('Gets valid options', () => { + const { wrapper } = setup(); + setTimeout(() => { + expect(fetchMock.calls(defaultProps.getEndpoint())).toHaveLength(1); + expect(wrapper.state('validOptions')).toEqual([VALID_METRIC]); + fetchMock.reset(); + }, 0); + }); + + it('Returns verified options', () => { + const { wrapper } = setup(); + setTimeout(() => { + expect(fetchMock.calls(defaultProps.getEndpoint())).toHaveLength(1); + const child = wrapper.find(MetricsControl); + expect(child.props().savedMetrics).toEqual([VALID_METRIC]); + fetchMock.reset(); + }, 0); + }); + + it('Makes no calls if endpoint is not set', () => { + const { wrapper } = setup({ + getEndpoint: () => null, + }); + setTimeout(() => { + expect(fetchMock.calls(defaultProps.getEndpoint())).toHaveLength(0); + expect(wrapper.state('validOptions')).toEqual(new Set()); + fetchMock.reset(); + }, 0); + }); + + it('Calls endpoint if control values change', () => { + const { props, wrapper } = setup({ controlValues: { metrics: 'sum__value' } }); + setTimeout(() => { + expect(fetchMock.calls(defaultProps.getEndpoint())).toHaveLength(1); + fetchMock.reset(); + }, 0); + wrapper.setProps({ ...props, controlValues: { metrics: 'avg__value' } }); + setTimeout(() => { + expect(fetchMock.calls(defaultProps.getEndpoint())).toHaveLength(1); + fetchMock.reset(); + }, 0); + }); +}); diff --git a/superset/assets/src/explore/components/controls/index.js b/superset/assets/src/explore/components/controls/index.js index d1a2f2c..f96d728 100644 --- a/superset/assets/src/explore/components/controls/index.js +++ b/superset/assets/src/explore/components/controls/index.js @@ -39,6 +39,7 @@ import MetricsControl from './MetricsControl'; import AdhocFilterControl from './AdhocFilterControl'; import FilterPanel from './FilterPanel'; import FilterBoxItemControl from './FilterBoxItemControl'; +import withVerification from './withVerification'; const controlMap = { AnnotationLayerControl, @@ -64,5 +65,8 @@ const controlMap = { AdhocFilterControl, FilterPanel, FilterBoxItemControl, + MetricsControlVerifiedOptions: withVerification(MetricsControl, 'metric_name', 'savedMetrics'), + SelectControlVerifiedOptions: withVerification(SelectControl, 'column_name', 'options'), + AdhocFilterControlVerifiedOptions: withVerification(AdhocFilterControl, 'column_name', 'columns'), }; export default controlMap; diff --git a/superset/assets/src/explore/components/controls/withVerification.jsx b/superset/assets/src/explore/components/controls/withVerification.jsx new file mode 100644 index 0000000..8f1e549 --- /dev/null +++ b/superset/assets/src/explore/components/controls/withVerification.jsx @@ -0,0 +1,88 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import React from 'react'; +import { SupersetClient } from '@superset-ui/connection'; + +import { isEqual } from 'lodash'; + +export default function withVerification(WrappedComponent, optionLabel, optionsName) { + /* + * This function will verify control options before passing them to the control by calling an + * endpoint on mount and when the controlValues change. controlValues should be set in + * mapStateToProps that can be added as a control override along with getEndpoint. + */ + class withVerificationComponent extends React.Component { + constructor(props) { + super(props); + this.state = { + validOptions: new Set(), + hasRunVerification: false, + }; + + this.getValidOptions = this.getValidOptions.bind(this); + } + + componentDidMount() { + this.getValidOptions(); + } + + componentDidUpdate(prevProps) { + const { hasRunVerification } = this.state; + if (!isEqual(this.props.controlValues, prevProps.controlValues) || !hasRunVerification) { + this.getValidOptions(); + } + } + + getValidOptions() { + const endpoint = this.props.getEndpoint(this.props.controlValues); + if (endpoint) { + SupersetClient.get({ + endpoint, + }).then(({ json }) => { + if (Array.isArray(json)) { + this.setState({ validOptions: new Set(json) || new Set() }); + } + }).catch(error => console.log(error)); + + if (!this.state.hasRunVerification) { + this.setState({ hasRunVerification: true }); + } + } + } + + render() { + const { validOptions } = this.state; + const options = this.props[optionsName]; + const verifiedOptions = validOptions.size ? + options.filter(o => (validOptions.has(o[optionLabel]))) : + options; + + const newProps = { ...this.props, [optionsName]: verifiedOptions }; + + return ( + + ); + } + } + withVerificationComponent.propTypes = WrappedComponent.propTypes; + return withVerificationComponent; +} +