From 7ddc022b34653708a6b38465f688b0365db4b235 Mon Sep 17 00:00:00 2001 From: "Michael S. Molina" <70410625+michael-s-molina@users.noreply.github.com> Date: Fri, 3 Feb 2023 10:50:12 -0500 Subject: [PATCH] fix: Prevents last temporal filter removal (#22982) (cherry picked from commit b63ea6386fedecbacce68bfd552b144b79845cca) --- .../components/ControlPanelsContainer.tsx | 47 +++++++++---------- .../DndFilterSelect.tsx | 35 +++++--------- .../AdhocFilterControl/index.jsx | 28 +++-------- 3 files changed, 41 insertions(+), 69 deletions(-) diff --git a/superset-frontend/src/explore/components/ControlPanelsContainer.tsx b/superset-frontend/src/explore/components/ControlPanelsContainer.tsx index 497ed2733685b..60aac1948bf4e 100644 --- a/superset-frontend/src/explore/components/ControlPanelsContainer.tsx +++ b/superset-frontend/src/explore/components/ControlPanelsContainer.tsx @@ -46,6 +46,7 @@ import { CustomControlItem, Dataset, ExpandedControlItem, + isTemporalColumn, sections, } from '@superset-ui/chart-controls'; import { useSelector } from 'react-redux'; @@ -293,13 +294,17 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => { const previousXAxis = usePrevious(x_axis); useEffect(() => { - if (x_axis && x_axis !== previousXAxis) { + if ( + x_axis && + x_axis !== previousXAxis && + isTemporalColumn(x_axis, props.exploreState.datasource) + ) { const noFilter = !adhoc_filters || !adhoc_filters.find( filter => filter.expressionType === 'SIMPLE' && - filter.operator === 'TEMPORAL_RANGE' && + filter.operator === Operators.TEMPORAL_RANGE && filter.subject === x_axis, ); if (noFilter) { @@ -314,7 +319,7 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => { { clause: 'WHERE', subject: x_axis, - operator: 'TEMPORAL_RANGE', + operator: Operators.TEMPORAL_RANGE, comparator: defaultTimeFilter || NO_TIME_RANGE, expressionType: 'SIMPLE', }, @@ -329,6 +334,7 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => { setControlValue, defaultTimeFilter, previousXAxis, + props.exploreState.datasource, ]); useEffect(() => { @@ -482,28 +488,21 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => { : baseDescription; if (name === 'adhoc_filters') { - restProps.confirmDeletion = { - triggerCondition: ( - valueToBeDeleted: Record, - values: Record[], - ) => { - const isTemporalRange = (filter: Record) => - filter.operator === Operators.TEMPORAL_RANGE; - if (isTemporalRange(valueToBeDeleted)) { - const count = values.filter(isTemporalRange).length; - if (count < 2) { - return true; - } + restProps.canDelete = ( + valueToBeDeleted: Record, + values: Record[], + ) => { + const isTemporalRange = (filter: Record) => + filter.operator === Operators.TEMPORAL_RANGE; + if (isTemporalRange(valueToBeDeleted)) { + const count = values.filter(isTemporalRange).length; + if (count === 1) { + return t( + `You cannot delete the last temporal filter as it's used for time range filters in dashboards.`, + ); } - return false; - }, - confirmationTitle: t( - 'Are you sure you want to remove the last temporal filter?', - ), - confirmationText: t( - `This filter is the last temporal filter. If you proceed, - this chart won't be affected by time range filters in dashboards.`, - ), + } + return true; }; } diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.tsx index 90cf156c118f9..134678aa6fad9 100644 --- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.tsx +++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.tsx @@ -59,7 +59,7 @@ import AdhocFilterControl from '../FilterControl/AdhocFilterControl'; import DndAdhocFilterOption from './DndAdhocFilterOption'; import { useDefaultTimeFilter } from '../DateFilterControl/utils'; -const { confirm } = Modal; +const { warning } = Modal; const EMPTY_OBJECT = {}; const DND_ACCEPTED_TYPES = [ @@ -78,14 +78,10 @@ export interface DndFilterSelectProps savedMetrics: Metric[]; selectedMetrics: QueryFormMetric[]; datasource: Datasource; - confirmDeletion?: { - triggerCondition: ( - valueToBeDeleted: OptionValueType, - values: OptionValueType[], - ) => boolean; - confirmationTitle: string; - confirmationText: string; - }; + canDelete?: ( + valueToBeDeleted: OptionValueType, + values: OptionValueType[], + ) => true | string; } const DndFilterSelect = (props: DndFilterSelectProps) => { @@ -93,7 +89,7 @@ const DndFilterSelect = (props: DndFilterSelectProps) => { datasource, onChange = () => {}, name: controlName, - confirmDeletion, + canDelete, } = props; const propsValues = Array.from(props.value ?? []); @@ -217,23 +213,14 @@ const DndFilterSelect = (props: DndFilterSelectProps) => { const onClickClose = useCallback( (index: number) => { - if (confirmDeletion) { - const { confirmationText, confirmationTitle, triggerCondition } = - confirmDeletion; - if (triggerCondition(values[index], values)) { - confirm({ - title: confirmationTitle, - content: confirmationText, - onOk() { - removeValue(index); - }, - }); - return; - } + const result = canDelete?.(values[index], values); + if (typeof result === 'string') { + warning({ title: t('Warning'), content: result }); + return; } removeValue(index); }, - [confirmDeletion, removeValue, values], + [canDelete, removeValue, values], ); const onShiftOptions = useCallback( diff --git a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterControl/index.jsx b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterControl/index.jsx index 44f4c469e6b23..c0750c7805d00 100644 --- a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterControl/index.jsx +++ b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterControl/index.jsx @@ -52,7 +52,7 @@ import AdhocFilter, { import adhocFilterType from 'src/explore/components/controls/FilterControl/adhocFilterType'; import columnType from 'src/explore/components/controls/FilterControl/columnType'; -const { confirm } = Modal; +const { warning } = Modal; const selectedMetricType = PropTypes.oneOfType([ PropTypes.string, @@ -74,11 +74,7 @@ const propTypes = { PropTypes.arrayOf(selectedMetricType), ]), isLoading: PropTypes.bool, - confirmDeletion: PropTypes.shape({ - triggerCondition: PropTypes.func, - confirmationTitle: PropTypes.string, - confirmationText: PropTypes.string, - }), + canDelete: PropTypes.func, }; const defaultProps = { @@ -196,22 +192,12 @@ class AdhocFilterControl extends React.Component { } onRemoveFilter(index) { - const { confirmDeletion } = this.props; + const { canDelete } = this.props; const { values } = this.state; - const { removeFilter } = this; - if (confirmDeletion) { - const { confirmationText, confirmationTitle, triggerCondition } = - confirmDeletion; - if (triggerCondition(values[index], values)) { - confirm({ - title: confirmationTitle, - content: confirmationText, - onOk() { - removeFilter(index); - }, - }); - return; - } + const result = canDelete?.(values[index], values); + if (typeof result === 'string') { + warning({ title: t('Warning'), content: result }); + return; } this.removeFilter(index); }