diff --git a/superset-frontend/src/explore/components/ControlPanelsContainer.tsx b/superset-frontend/src/explore/components/ControlPanelsContainer.tsx index cdc9259c0..d47c1abaf 100644 --- a/superset-frontend/src/explore/components/ControlPanelsContainer.tsx +++ b/superset-frontend/src/explore/components/ControlPanelsContainer.tsx @@ -52,7 +52,7 @@ import { } from '@superset-ui/chart-controls'; import { useSelector } from 'react-redux'; import { rgba } from 'emotion-rgba'; -import { kebabCase } from 'lodash'; +import { kebabCase, isEqual } from 'lodash'; import Collapse from 'src/components/Collapse'; import Tabs from 'src/components/Tabs'; @@ -283,7 +283,7 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => { >(state => state.explore.controlsTransferred); const defaultTimeFilter = useSelector( - state => state.common?.conf?.DEFAULT_TIME_FILTER, + state => state.common?.conf?.DEFAULT_TIME_FILTER || NO_TIME_RANGE, ); const { form_data, actions } = props; @@ -317,7 +317,7 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => { clause: Clauses.Where, subject: x_axis, operator: Operators.TemporalRange, - comparator: defaultTimeFilter || NO_TIME_RANGE, + comparator: defaultTimeFilter, expressionType: 'SIMPLE', }, ]); @@ -494,9 +494,26 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => { if (!controls?.time_range?.value && 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.`, + // if temporal filter's value is "No filter", prevent deletion + // otherwise reset the value to "No filter" + if (valueToBeDeleted.comparator === defaultTimeFilter) { + return t( + `You cannot delete the last temporal filter as it's used for time range filters in dashboards.`, + ); + } + props.actions.setControlValue( + name, + values.map(val => { + if (isEqual(val, valueToBeDeleted)) { + return { + ...val, + comparator: defaultTimeFilter, + }; + } + return val; + }), ); + return false; } } return true; diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.test.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.test.tsx index 6be82472d..035cacd81 100644 --- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.test.tsx +++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.test.tsx @@ -37,6 +37,7 @@ import { import type { AsyncAceEditorProps } from 'src/components/AsyncAceEditor'; import AdhocMetric from 'src/explore/components/controls/MetricControl/AdhocMetric'; import AdhocFilter from 'src/explore/components/controls/FilterControl/AdhocFilter'; +import { Operators } from 'src/explore/constants'; import { DndFilterSelect, DndFilterSelectProps, @@ -78,11 +79,13 @@ function setup({ formData = baseFormData, columns = [], datasource = PLACEHOLDER_DATASOURCE, + additionalProps = {}, }: { - value?: AdhocFilter; + value?: AdhocFilter | AdhocFilter[]; formData?: QueryFormData; columns?: ColumnMeta[]; datasource?: Datasource; + additionalProps?: Partial; } = {}) { return ( ); } +beforeEach(() => { + jest.clearAllMocks(); +}); + test('renders with default props', async () => { render(setup(), { useDnd: true, store }); expect( @@ -248,6 +256,73 @@ test('cannot drop a column that is not part of the simple column selection', () ).toHaveTextContent('AGG(metric_a)'); }); +test('calls onChange when close is clicked and canDelete is true', () => { + const value1 = new AdhocFilter({ + sqlExpression: 'COUNT(*)', + expressionType: ExpressionTypes.Sql, + }); + const value2 = new AdhocFilter({ + expressionType: ExpressionTypes.Simple, + subject: 'col', + comparator: 'val', + operator: Operators.Equals, + }); + const canDelete = jest.fn(); + canDelete.mockReturnValue(true); + render(setup({ value: [value1, value2], additionalProps: { canDelete } }), { + useDnd: true, + store, + }); + fireEvent.click(screen.getAllByTestId('remove-control-button')[0]); + expect(canDelete).toHaveBeenCalled(); + expect(defaultProps.onChange).toHaveBeenCalledWith([value2]); +}); + +test('onChange is not called when close is clicked and canDelete is false', () => { + const value1 = new AdhocFilter({ + sqlExpression: 'COUNT(*)', + expressionType: ExpressionTypes.Sql, + }); + const value2 = new AdhocFilter({ + expressionType: ExpressionTypes.Simple, + subject: 'col', + comparator: 'val', + operator: Operators.Equals, + }); + const canDelete = jest.fn(); + canDelete.mockReturnValue(false); + render(setup({ value: [value1, value2], additionalProps: { canDelete } }), { + useDnd: true, + store, + }); + fireEvent.click(screen.getAllByTestId('remove-control-button')[0]); + expect(canDelete).toHaveBeenCalled(); + expect(defaultProps.onChange).not.toHaveBeenCalled(); +}); + +test('onChange is not called when close is clicked and canDelete is string, warning is displayed', async () => { + const value1 = new AdhocFilter({ + sqlExpression: 'COUNT(*)', + expressionType: ExpressionTypes.Sql, + }); + const value2 = new AdhocFilter({ + expressionType: ExpressionTypes.Simple, + subject: 'col', + comparator: 'val', + operator: Operators.Equals, + }); + const canDelete = jest.fn(); + canDelete.mockReturnValue('Test warning'); + render(setup({ value: [value1, value2], additionalProps: { canDelete } }), { + useDnd: true, + store, + }); + fireEvent.click(screen.getAllByTestId('remove-control-button')[0]); + expect(canDelete).toHaveBeenCalled(); + expect(defaultProps.onChange).not.toHaveBeenCalled(); + expect(await screen.findByText('Test warning')).toBeInTheDocument(); +}); + describe('when disallow_adhoc_metrics is set', () => { test('can drop a column type from the simple column selection', () => { const adhocMetric = new AdhocMetric({ diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.tsx index 955c48072..b479ce4c8 100644 --- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.tsx +++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.tsx @@ -232,7 +232,9 @@ const DndFilterSelect = (props: DndFilterSelectProps) => { warning({ title: t('Warning'), content: result }); return; } - removeValue(index); + if (result === true) { + removeValue(index); + } }, [canDelete, removeValue, values], );