Skip to content

Commit

Permalink
feat(explore): Clear temporal filter value (apache#27788)
Browse files Browse the repository at this point in the history
  • Loading branch information
kgabryje authored Apr 9, 2024
1 parent e80d194 commit 4ecfce9
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -283,7 +283,7 @@ export const ControlPanelsContainer = (props: ControlPanelsContainerProps) => {
>(state => state.explore.controlsTransferred);

const defaultTimeFilter = useSelector<ExplorePageState>(
state => state.common?.conf?.DEFAULT_TIME_FILTER,
state => state.common?.conf?.DEFAULT_TIME_FILTER || NO_TIME_RANGE,
);

const { form_data, actions } = props;
Expand Down Expand Up @@ -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',
},
]);
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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<DndFilterSelectProps>;
} = {}) {
return (
<DndFilterSelect
Expand All @@ -91,10 +94,15 @@ function setup({
value={ensureIsArray(value)}
formData={formData}
columns={columns}
{...additionalProps}
/>
);
}

beforeEach(() => {
jest.clearAllMocks();
});

test('renders with default props', async () => {
render(setup(), { useDnd: true, store });
expect(
Expand Down Expand Up @@ -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({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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],
);
Expand Down

0 comments on commit 4ecfce9

Please sign in to comment.