diff --git a/superset-frontend/src/explore/components/ControlPanelAlert.tsx b/superset-frontend/src/explore/components/ControlPanelAlert.tsx new file mode 100644 index 0000000000000..142e074b559e6 --- /dev/null +++ b/superset-frontend/src/explore/components/ControlPanelAlert.tsx @@ -0,0 +1,98 @@ +/** + * 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 { styled } from '@superset-ui/core'; +import Button from 'src/components/Button'; + +interface ControlPanelAlertProps { + title: string; + bodyText: string; + primaryButtonAction: (e: React.MouseEvent) => void; + secondaryButtonAction?: (e: React.MouseEvent) => void; + primaryButtonText: string; + secondaryButtonText?: string; + type: 'info' | 'warning'; +} + +const AlertContainer = styled.div` + margin: ${({ theme }) => theme.gridUnit * 4}px; + padding: ${({ theme }) => theme.gridUnit * 4}px; + + border: ${({ theme }) => `1px solid ${theme.colors.info.base}`}; + background-color: ${({ theme }) => theme.colors.info.light2}; + border-radius: 2px; + + color: ${({ theme }) => theme.colors.info.dark2}; + font-size: ${({ theme }) => theme.typography.sizes.s}; + + &.alert-type-warning { + border-color: ${({ theme }) => theme.colors.alert.base}; + background-color: ${({ theme }) => theme.colors.alert.light2}; + + p { + color: ${({ theme }) => theme.colors.alert.dark2}; + } + } +`; + +const ButtonContainer = styled.div` + display: flex; + justify-content: flex-end; + button { + line-height: 1; + } +`; + +const Title = styled.p` + font-weight: ${({ theme }) => theme.typography.weights.bold}; +`; + +export const ControlPanelAlert = ({ + title, + bodyText, + primaryButtonAction, + secondaryButtonAction, + primaryButtonText, + secondaryButtonText, + type = 'info', +}: ControlPanelAlertProps) => ( + + {title} +

{bodyText}

+ + {secondaryButtonAction && secondaryButtonText && ( + + )} + + +
+); diff --git a/superset-frontend/src/explore/components/ControlPanelsContainer.test.tsx b/superset-frontend/src/explore/components/ControlPanelsContainer.test.tsx index f25793dd5f3c1..24101d9c17b9c 100644 --- a/superset-frontend/src/explore/components/ControlPanelsContainer.test.tsx +++ b/superset-frontend/src/explore/components/ControlPanelsContainer.test.tsx @@ -17,7 +17,7 @@ * under the License. */ import React from 'react'; -import { styledShallow as shallow } from 'spec/helpers/theming'; +import { render, screen } from 'spec/helpers/testing-library'; import { DatasourceType, getChartControlPanelRegistry, @@ -30,9 +30,7 @@ import { ControlPanelsContainerProps, } from 'src/explore/components/ControlPanelsContainer'; -describe('ControlPanelsContainer', () => { - let wrapper; - +describe('ControlPanelsContainer2', () => { beforeAll(() => { getChartControlPanelRegistry().registerValue('table', { controlPanelSections: [ @@ -96,9 +94,9 @@ describe('ControlPanelsContainer', () => { } it('renders ControlPanelSections', () => { - wrapper = shallow(); + render(); expect( - wrapper.find('[data-test="collapsible-control-panel"]'), - ).toHaveLength(5); + screen.getAllByTestId('collapsible-control-panel-header'), + ).toHaveLength(4); }); }); diff --git a/superset-frontend/src/explore/components/ControlPanelsContainer.tsx b/superset-frontend/src/explore/components/ControlPanelsContainer.tsx index ac48db4718b5d..ed98570881ad0 100644 --- a/superset-frontend/src/explore/components/ControlPanelsContainer.tsx +++ b/superset-frontend/src/explore/components/ControlPanelsContainer.tsx @@ -17,9 +17,14 @@ * under the License. */ /* eslint camelcase: 0 */ -import React from 'react'; -import { bindActionCreators } from 'redux'; -import { connect } from 'react-redux'; +import React, { + useCallback, + useContext, + useEffect, + useMemo, + useRef, + useState, +} from 'react'; import { ensureIsArray, t, @@ -44,21 +49,20 @@ import Tabs from 'src/components/Tabs'; import { PluginContext } from 'src/components/DynamicPlugins'; import Loading from 'src/components/Loading'; +import { usePrevious } from 'src/hooks/usePrevious'; import { getSectionsToRender } from 'src/explore/controlUtils'; -import { - ExploreActions, - exploreActions, -} from 'src/explore/actions/exploreActions'; +import { ExploreActions } from 'src/explore/actions/exploreActions'; import { ExplorePageState } from 'src/explore/reducers/getInitialState'; import { ChartState } from 'src/explore/types'; import ControlRow from './ControlRow'; import Control from './Control'; +import { ControlPanelAlert } from './ControlPanelAlert'; export type ControlPanelsContainerProps = { + exploreState: ExplorePageState['explore']; actions: ExploreActions; datasource_type: DatasourceType; - exploreState: ExplorePageState['explore']; chart: ChartState; controls: Record; form_data: QueryFormData; @@ -77,7 +81,6 @@ const Styles = styled.div` width: 100%; overflow: auto; overflow-x: visible; - overflow-y: auto; #controlSections { min-height: 100%; overflow: visible; @@ -113,14 +116,6 @@ const ControlPanelsTabs = styled(Tabs)` } `; -type ControlPanelsContainerState = { - expandedQuerySections: string[]; - expandedCustomizeSections: string[]; - querySections: ControlPanelSectionConfig[]; - customizeSections: ControlPanelSectionConfig[]; - loading: boolean; -}; - const isTimeSection = (section: ControlPanelSectionConfig): boolean => !!section.label && (sections.legacyRegularTime.label === section.label || @@ -144,39 +139,35 @@ const sectionsToExpand = ( ); function getState( - props: ControlPanelsContainerProps, -): ControlPanelsContainerState { - const { - exploreState: { datasource }, - } = props; - + vizType: string, + datasource: DatasourceMeta, + datasourceType: DatasourceType, +) { const querySections: ControlPanelSectionConfig[] = []; const customizeSections: ControlPanelSectionConfig[] = []; - getSectionsToRender(props.form_data.viz_type, props.datasource_type).forEach( - section => { - // if at least one control in the section is not `renderTrigger` - // or asks to be displayed at the Data tab - if ( - section.tabOverride === 'data' || - section.controlSetRows.some(rows => - rows.some( - control => - control && - typeof control === 'object' && - 'config' in control && - control.config && - (!control.config.renderTrigger || - control.config.tabOverride === 'data'), - ), - ) - ) { - querySections.push(section); - } else { - customizeSections.push(section); - } - }, - ); + getSectionsToRender(vizType, datasourceType).forEach(section => { + // if at least one control in the section is not `renderTrigger` + // or asks to be displayed at the Data tab + if ( + section.tabOverride === 'data' || + section.controlSetRows.some(rows => + rows.some( + control => + control && + typeof control === 'object' && + 'config' in control && + control.config && + (!control.config.renderTrigger || + control.config.tabOverride === 'data'), + ), + ) + ) { + querySections.push(section); + } else { + customizeSections.push(section); + } + }); const expandedQuerySections: string[] = sectionsToExpand( querySections, datasource, @@ -190,57 +181,72 @@ function getState( expandedCustomizeSections, querySections, customizeSections, - loading: false, }; } -export class ControlPanelsContainer extends React.Component< - ControlPanelsContainerProps, - ControlPanelsContainerState -> { - // trigger updates to the component when async plugins load - static contextType = PluginContext; - - constructor(props: ControlPanelsContainerProps) { - super(props); - this.state = { - expandedQuerySections: [], - expandedCustomizeSections: [], - querySections: [], - customizeSections: [], - loading: false, - }; - this.renderControl = this.renderControl.bind(this); - this.renderControlPanelSection = this.renderControlPanelSection.bind(this); - } +export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => { + const pluginContext = useContext(PluginContext); - componentDidUpdate(prevProps: ControlPanelsContainerProps) { - if ( - this.props.form_data.datasource !== prevProps.form_data.datasource || - this.props.form_data.viz_type !== prevProps.form_data.viz_type - ) { - // eslint-disable-next-line react/no-did-update-set-state - this.setState(getState(this.props)); - } - } + const prevDatasource = usePrevious(props.exploreState.datasource); - // required for an Antd bug that would otherwise malfunction re-rendering - // a collapsed panel after changing the datasource or viz type - UNSAFE_componentWillReceiveProps(nextProps: ControlPanelsContainerProps) { + const [showDatasourceAlert, setShowDatasourceAlert] = useState(false); + + const containerRef = useRef(null); + + useEffect(() => { if ( - this.props.form_data.datasource !== nextProps.form_data.datasource || - this.props.form_data.viz_type !== nextProps.form_data.viz_type + prevDatasource && + (props.exploreState.datasource?.id !== prevDatasource.id || + props.exploreState.datasource?.type !== prevDatasource.type) ) { - this.setState({ loading: true }); + setShowDatasourceAlert(true); + containerRef.current?.scrollTo(0, 0); } - } + }, [ + props.exploreState.datasource?.id, + props.exploreState.datasource?.type, + prevDatasource, + ]); - componentDidMount() { - this.setState(getState(this.props)); - } + const { + expandedQuerySections, + expandedCustomizeSections, + querySections, + customizeSections, + } = useMemo( + () => + getState( + props.form_data.viz_type, + props.exploreState.datasource, + props.datasource_type, + ), + [ + props.form_data.datasource, + props.form_data.viz_type, + props.datasource_type, + ], + ); - renderControl({ name, config }: CustomControlItem) { - const { actions, controls, chart, exploreState } = this.props; + const resetTransferredControls = useCallback(() => { + ensureIsArray(props.exploreState.controlsTransferred).forEach(controlName => + props.actions.setControlValue( + controlName, + props.controls[controlName].default, + ), + ); + }, [props.actions, props.exploreState.controlsTransferred, props.controls]); + + const handleClearFormClick = useCallback(() => { + resetTransferredControls(); + setShowDatasourceAlert(false); + }, [resetTransferredControls]); + + const handleContinueClick = useCallback(() => { + setShowDatasourceAlert(false); + }, []); + + const renderControl = ({ name, config }: CustomControlItem) => { + const { controls, chart, exploreState } = props; const { visibility } = config; // If the control item is not an object, we have to look up the control data from @@ -253,8 +259,7 @@ export class ControlPanelsContainer extends React.Component< // state, too. Since it's may be expensive to run mapStateToProps for every // re-render, we only run this when the chart plugin explicitly ask for this. ...(config.mapStateToProps?.length === 3 - ? // @ts-ignore /* The typing accuses of having an extra parameter. I didn't remove it because I believe it could be an error in the types and not in the code */ - config.mapStateToProps(exploreState, controls[name], chart) + ? config.mapStateToProps(exploreState, controls[name], chart) : // for other controls, `mapStateToProps` is already run in // controlUtils/getControlState.ts undefined), @@ -265,7 +270,7 @@ export class ControlPanelsContainer extends React.Component< }; // if visibility check says the config is not visible, don't render it - if (visibility && !visibility.call(config, this.props, controlData)) { + if (visibility && !visibility.call(config, props, controlData)) { return null; } return ( @@ -273,14 +278,16 @@ export class ControlPanelsContainer extends React.Component< key={`control-${name}`} name={name} validationErrors={validationErrors} - actions={actions} + actions={props.actions} {...restProps} /> ); - } + }; - renderControlPanelSection(section: ExpandedControlPanelSectionConfig) { - const { controls } = this.props; + const renderControlPanelSection = ( + section: ExpandedControlPanelSectionConfig, + ) => { + const { controls } = props; const { label, description } = section; // Section label can be a ReactNode but in some places we want to @@ -305,7 +312,7 @@ export class ControlPanelsContainer extends React.Component< }), ); const PanelHeader = () => ( - + {label}{' '} {description && ( // label is only used in tooltip id (should probably call this prop `id`) @@ -323,7 +330,6 @@ export class ControlPanelsContainer extends React.Component< return ( css` margin-bottom: 0; box-shadow: none; @@ -334,14 +340,14 @@ export class ControlPanelsContainer extends React.Component< .panel-body { margin-left: ${theme.gridUnit * 4}px; - padding-bottom: 0px; + padding-bottom: 0; } span.label { display: inline-block; } `} - header={PanelHeader()} + header={} key={sectionId} > {section.controlSetRows.map((controlSets, i) => { @@ -360,7 +366,7 @@ export class ControlPanelsContainer extends React.Component< controlItem.config && controlItem.name !== 'datasource' ) { - return this.renderControl(controlItem); + return renderControl(controlItem); } return null; }) @@ -378,81 +384,84 @@ export class ControlPanelsContainer extends React.Component< })} ); + }; + + const hasControlsTransferred = + ensureIsArray(props.exploreState.controlsTransferred).length > 0; + + const DatasourceAlert = useCallback( + () => + hasControlsTransferred ? ( + + ) : ( + + ), + [handleClearFormClick, handleContinueClick, hasControlsTransferred], + ); + + const controlPanelRegistry = getChartControlPanelRegistry(); + if ( + !controlPanelRegistry.has(props.form_data.viz_type) && + pluginContext.loading + ) { + return ; } - render() { - const controlPanelRegistry = getChartControlPanelRegistry(); - if ( - (!controlPanelRegistry.has(this.props.form_data.viz_type) && - this.context.loading) || - this.state.loading - ) { - return ; - } + const showCustomizeTab = customizeSections.length > 0; - const showCustomizeTab = this.state.customizeSections.length > 0; - return ( - - - + return ( + + + + + {showDatasourceAlert && } + {querySections.map(renderControlPanelSection)} + + + {showCustomizeTab && ( + { - this.setState({ - expandedQuerySections: ensureIsArray(selection), - }); - }} ghost + key={`customize-sections-${props.form_data.datasource}-${props.form_data.viz_type}`} > - {this.state.querySections.map(this.renderControlPanelSection)} + {customizeSections.map(renderControlPanelSection)} - {showCustomizeTab && ( - - { - this.setState({ - expandedCustomizeSections: ensureIsArray(selection), - }); - }} - ghost - > - {this.state.customizeSections.map( - this.renderControlPanelSection, - )} - - - )} - - - ); - } -} + )} + + + ); +}; -export default connect( - function mapStateToProps(state: ExplorePageState) { - const { explore, charts } = state; - const chartKey = Object.keys(charts)[0]; - const chart = charts[chartKey]; - return { - chart, - isDatasourceMetaLoading: explore.isDatasourceMetaLoading, - controls: explore.controls, - exploreState: explore, - }; - }, - function mapDispatchToProps(dispatch) { - return { - actions: bindActionCreators(exploreActions, dispatch), - }; - }, -)(ControlPanelsContainer); +export default ControlPanelsContainer; diff --git a/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx b/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx index dc212cebcdcc2..b3315688127e7 100644 --- a/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx +++ b/superset-frontend/src/explore/components/ExploreViewContainer/index.jsx @@ -609,9 +609,11 @@ function ExploreViewContainer(props) { datasourceType={props.datasource_type} /> @@ -673,6 +675,7 @@ function mapStateToProps(state) { ownState: dataMask[form_data.slice_id ?? 0]?.ownState, // 0 - unsaved chart impressionId, user: explore.user, + exploreState: explore, reports, }; } diff --git a/superset-frontend/src/explore/controlUtils/getControlValuesCompatibleWithDatasource.ts b/superset-frontend/src/explore/controlUtils/getControlValuesCompatibleWithDatasource.ts new file mode 100644 index 0000000000000..efd594cb7ff86 --- /dev/null +++ b/superset-frontend/src/explore/controlUtils/getControlValuesCompatibleWithDatasource.ts @@ -0,0 +1,99 @@ +/** + * 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 { + ControlState, + DatasourceMeta, + Metric, +} from '@superset-ui/chart-controls'; +import { + Column, + isAdhocMetricSimple, + isSavedMetric, + isSimpleAdhocFilter, + JsonValue, + SimpleAdhocFilter, +} from '@superset-ui/core'; +import AdhocMetric from 'src/explore/components/controls/MetricControl/AdhocMetric'; + +const isControlValueCompatibleWithDatasource = ( + datasource: DatasourceMeta, + controlState: ControlState, + value: any, +) => { + if (controlState.options && typeof value === 'string') { + if ( + (Array.isArray(controlState.options) && + controlState.options.some( + (option: [string | number, string]) => option[0] === value, + )) || + value in controlState.options + ) { + return datasource.columns.some(column => column.column_name === value); + } + } + if ( + controlState.savedMetrics && + isSavedMetric(value) && + controlState.savedMetrics.some( + (savedMetric: Metric) => savedMetric.metric_name === value, + ) + ) { + return datasource.metrics.some( + (metric: Metric) => metric.metric_name === value, + ); + } + if ( + controlState.columns && + (isAdhocMetricSimple(value) || isSimpleAdhocFilter(value)) && + controlState.columns.some( + (column: Column) => + column.column_name === (value as AdhocMetric).column?.column_name || + column.column_name === (value as SimpleAdhocFilter).subject, + ) + ) { + return datasource.columns.some( + (column: Column) => + column.column_name === (value as AdhocMetric).column?.column_name || + column.column_name === (value as SimpleAdhocFilter).subject, + ); + } + return false; +}; + +export const getControlValuesCompatibleWithDatasource = ( + datasource: DatasourceMeta, + controlState: ControlState, + value: JsonValue, +) => { + if (value === undefined || value === null) { + return undefined; + } + if (Array.isArray(value)) { + const compatibleValues = value.filter(val => + isControlValueCompatibleWithDatasource(datasource, controlState, val), + ); + return compatibleValues.length > 0 + ? compatibleValues + : controlState.default; + } + return isControlValueCompatibleWithDatasource(datasource, controlState, value) + ? value + : controlState.default; +}; diff --git a/superset-frontend/src/explore/controlUtils/index.ts b/superset-frontend/src/explore/controlUtils/index.ts index 0556cd2ab7405..d9cb1132f4f5c 100644 --- a/superset-frontend/src/explore/controlUtils/index.ts +++ b/superset-frontend/src/explore/controlUtils/index.ts @@ -20,3 +20,4 @@ export * from './getSectionsToRender'; export * from './getControlConfig'; export * from './getControlState'; export * from './getFormDataFromControls'; +export * from './getControlValuesCompatibleWithDatasource'; diff --git a/superset-frontend/src/explore/reducers/exploreReducer.js b/superset-frontend/src/explore/reducers/exploreReducer.js index 4c771971a0abf..a68530d870d53 100644 --- a/superset-frontend/src/explore/reducers/exploreReducer.js +++ b/superset-frontend/src/explore/reducers/exploreReducer.js @@ -17,6 +17,7 @@ * under the License. */ /* eslint camelcase: 0 */ +import { ensureIsArray } from '@superset-ui/core'; import { DYNAMIC_PLUGIN_CONTROLS_READY } from 'src/chart/chartAction'; import { DEFAULT_TIME_RANGE } from 'src/explore/constants'; import { getControlsState } from 'src/explore/store'; @@ -24,6 +25,7 @@ import { getControlConfig, getFormDataFromControls, getControlStateFromControlConfig, + getControlValuesCompatibleWithDatasource, } from 'src/explore/controlUtils'; import * as actions from 'src/explore/actions/exploreActions'; @@ -64,6 +66,7 @@ export default function exploreReducer(state = {}, action) { } const controls = { ...state.controls }; + const controlsTransferred = []; if ( action.datasource.id !== state.datasource.id || action.datasource.type !== state.datasource.type @@ -77,16 +80,24 @@ export default function exploreReducer(state = {}, action) { // for direct column select controls controlState.valueKey === 'column_name' || // for all other controls - 'columns' in controlState + 'savedMetrics' in controlState || + 'columns' in controlState || + ('options' in controlState && !Array.isArray(controlState.options)) ) { - // if a control use datasource columns, reset its value to `undefined`, - // then `getControlsState` will pick up the default. - // TODO: filter out only invalid columns and keep others controls[controlName] = { ...controlState, - value: undefined, }; - newFormData[controlName] = undefined; + newFormData[controlName] = getControlValuesCompatibleWithDatasource( + action.datasource, + controlState, + controlState.value, + ); + if ( + ensureIsArray(newFormData[controlName]).length > 0 && + newFormData[controlName] !== controls[controlName].default + ) { + controlsTransferred.push(controlName); + } } }); } @@ -102,6 +113,7 @@ export default function exploreReducer(state = {}, action) { ...newState, form_data: newFormData, controls: getControlsState(newState, newFormData), + controlsTransferred, }; }, [actions.FETCH_DATASOURCES_STARTED]() { diff --git a/superset-frontend/src/explore/reducers/getInitialState.ts b/superset-frontend/src/explore/reducers/getInitialState.ts index 08ed3d542a1a7..45440f6f5b4b9 100644 --- a/superset-frontend/src/explore/reducers/getInitialState.ts +++ b/superset-frontend/src/explore/reducers/getInitialState.ts @@ -36,7 +36,7 @@ import { applyMapStateToPropsToControl, } from 'src/explore/controlUtils'; -export interface ExlorePageBootstrapData extends JsonObject { +export interface ExplorePageBootstrapData extends JsonObject { can_add: boolean; can_download: boolean; can_overwrite: boolean; @@ -53,7 +53,7 @@ export interface ExlorePageBootstrapData extends JsonObject { } export default function getInitialState( - bootstrapData: ExlorePageBootstrapData, + bootstrapData: ExplorePageBootstrapData, ) { const { form_data: initialFormData } = bootstrapData; const { slice } = bootstrapData; @@ -76,6 +76,7 @@ export default function getInitialState( bootstrapData, initialFormData, ) as ControlStateMapping, + controlsTransferred: [], }; // apply initial mapStateToProps for all controls, must execute AFTER