feat(native-filters): Hide time filters if loaded datasets don't have temporal columns (#15225)

* feat(native-filters): Hide time filters if loaded datasets don't have temporal columns

* Remove "px" suffixes to fix warnings

* Disable an option instead of hiding filter types

* Fix tests

* Add 2 more tests
This commit is contained in:
Kamil Gabryjelski 2021-06-17 16:51:11 +02:00 committed by GitHub
parent ea8507b4e2
commit c7c63751e9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 119 additions and 43 deletions

View File

@ -164,6 +164,7 @@ export default {
column_name: 'num_girls',
},
],
column_types: [0, 1, 2],
id,
granularity_sqla: [['ds', 'ds']],
name: 'birth_names',

View File

@ -26,6 +26,7 @@ import {
styled,
SupersetApiError,
t,
GenericDataType,
} from '@superset-ui/core';
import {
ColumnMeta,
@ -102,7 +103,7 @@ export const StyledFormItem = styled(FormItem)`
margin-bottom: ${({ theme }) => theme.gridUnit * 4}px;
& .ant-form-item-label {
padding-bottom: 0px;
padding-bottom: 0;
}
& .ant-form-item-control-input {
@ -111,12 +112,12 @@ export const StyledFormItem = styled(FormItem)`
`;
export const StyledRowFormItem = styled(FormItem)`
margin-bottom: 0px;
padding-bottom: 0px;
margin-bottom: 0;
padding-bottom: 0;
min-width: 50%;
& .ant-form-item-label {
padding-bottom: 0px;
padding-bottom: 0;
}
.ant-form-item-control-input-content > div > div {
@ -154,17 +155,17 @@ const StyledCollapse = styled(Collapse)`
margin-right: ${({ theme }) => theme.gridUnit * -4}px;
border-left: 1px solid ${({ theme }) => theme.colors.grayscale.light2};
border-top: 1px solid ${({ theme }) => theme.colors.grayscale.light2};
border-radius: 0px;
border-radius: 0;
.ant-collapse-header {
border-bottom: 1px solid ${({ theme }) => theme.colors.grayscale.light2};
border-top: 1px solid ${({ theme }) => theme.colors.grayscale.light2};
margin-top: -1px;
border-radius: 0px;
border-radius: 0;
}
.ant-collapse-content {
border: 0px;
border: 0;
}
.ant-collapse-content-box {
@ -172,8 +173,8 @@ const StyledCollapse = styled(Collapse)`
}
&.ant-collapse > .ant-collapse-item {
border: 0px;
border-radius: 0px;
border: 0;
border-radius: 0;
}
`;
@ -182,17 +183,17 @@ const StyledTabs = styled(Tabs)`
position: sticky;
margin-left: ${({ theme }) => theme.gridUnit * -4}px;
margin-right: ${({ theme }) => theme.gridUnit * -4}px;
top: 0px;
top: 0;
background: white;
z-index: 1;
}
.ant-tabs-nav-list {
padding: 0px;
padding: 0;
}
.ant-form-item-label {
padding-bottom: 0px;
padding-bottom: 0;
}
`;
@ -245,6 +246,8 @@ const FILTERS_WITHOUT_COLUMN = [
const FILTERS_WITH_ADHOC_FILTERS = ['filter_select', 'filter_range'];
const TIME_FILTERS = ['filter_time', 'filter_timegrain', 'filter_timecolumn'];
const BASIC_CONTROL_ITEMS = ['enableEmptyFilter', 'multiSelect'];
// TODO: Rename the filter plugins and remove this mapping
@ -257,6 +260,11 @@ const FILTER_TYPE_NAME_MAPPING = {
[t('Group By')]: t('Group by'),
};
// TODO: add column_types field to DatasourceMeta
const hasTemporalColumns = (
dataset: DatasourceMeta & { column_types: GenericDataType[] },
) => dataset?.column_types?.includes(GenericDataType.TEMPORAL);
/**
* The configuration form for a specific filter.
* Assigns field values to `filters[filterId]` in the form.
@ -283,7 +291,7 @@ const FiltersConfigForm = (
const forceUpdate = useForceUpdate();
const [datasetDetails, setDatasetDetails] = useState<Record<string, any>>();
const defaultFormFilter = useMemo(() => {}, []);
const defaultFormFilter = useMemo(() => ({}), []);
const formFilter =
form.getFieldValue('filters')?.[filterId] || defaultFormFilter;
@ -299,6 +307,22 @@ const FiltersConfigForm = (
({ datasources }) => datasources,
);
const doLoadedDatasetsHaveTemporalColumns = useMemo(
() =>
Object.values(loadedDatasets).some(dataset =>
hasTemporalColumns(dataset),
),
[loadedDatasets],
);
const showTimeRangePicker = useMemo(() => {
const currentDataset = Object.values(loadedDatasets).find(
dataset => dataset.id === formFilter.dataset?.value,
);
return currentDataset ? hasTemporalColumns(currentDataset) : true;
}, [formFilter.dataset?.value, loadedDatasets]);
// @ts-ignore
const hasDataset = !!nativeFilterItems[formFilter?.filterType]?.value
?.datasourceCount;
@ -576,9 +600,21 @@ const FiltersConfigForm = (
const mappedName = name
? FILTER_TYPE_NAME_MAPPING[name]
: undefined;
const isDisabled =
TIME_FILTERS.includes(filterType) &&
!doLoadedDatasetsHaveTemporalColumns;
return {
value: filterType,
label: mappedName || name,
label: isDisabled ? (
<Tooltip
title={t('Datasets do not contain a temporal column')}
>
{mappedName || name}
</Tooltip>
) : (
mappedName || name
),
isDisabled,
};
})}
onChange={({ value }: { value: string }) => {
@ -845,28 +881,30 @@ const FiltersConfigForm = (
}
/>
</StyledRowFormItem>
<StyledRowFormItem
name={['filters', filterId, 'time_range']}
label={<StyledLabel>{t('Time range')}</StyledLabel>}
initialValue={filterToEdit?.time_range || 'No filter'}
required={!hasAdhoc}
rules={[
{
validator: preFilterValidator,
},
]}
>
<DateFilterControl
name="time_range"
onChange={timeRange => {
setNativeFilterFieldValues(form, filterId, {
time_range: timeRange,
});
forceUpdate();
validatePreFilter();
}}
/>
</StyledRowFormItem>
{showTimeRangePicker && (
<StyledRowFormItem
name={['filters', filterId, 'time_range']}
label={<StyledLabel>{t('Time range')}</StyledLabel>}
initialValue={filterToEdit?.time_range || 'No filter'}
required={!hasAdhoc}
rules={[
{
validator: preFilterValidator,
},
]}
>
<DateFilterControl
name="time_range"
onChange={timeRange => {
setNativeFilterFieldValues(form, filterId, {
time_range: timeRange,
});
forceUpdate();
validatePreFilter();
}}
/>
</StyledRowFormItem>
)}
{hasTimeRange && (
<StyledRowFormItem
name={['filters', filterId, 'granularity_sqla']}

View File

@ -19,7 +19,7 @@
import { flatMapDeep } from 'lodash';
import { FormInstance } from 'antd/lib/form';
import React from 'react';
import { CustomControlItem } from '@superset-ui/chart-controls';
import { CustomControlItem, DatasourceMeta } from '@superset-ui/chart-controls';
const FILTERS_FIELD_NAME = 'filters';
@ -64,7 +64,9 @@ type DatasetSelectValue = {
label: string;
};
export const datasetToSelectOption = (item: any): DatasetSelectValue => ({
export const datasetToSelectOption = (
item: DatasourceMeta & { table_name: string },
): DatasetSelectValue => ({
value: item.id,
label: item.table_name,
});

View File

@ -16,7 +16,10 @@
* specific language governing permissions and limitations
* under the License.
*/
import React from 'react';
import { Preset } from '@superset-ui/core';
import fetchMock from 'fetch-mock';
import userEvent, { specialChars } from '@testing-library/user-event';
import {
SelectFilterPlugin,
RangeFilterPlugin,
@ -25,9 +28,7 @@ import {
TimeGrainFilterPlugin,
} from 'src/filters/components';
import { render, screen, waitFor } from 'spec/helpers/testing-library';
import fetchMock from 'fetch-mock';
import React from 'react';
import userEvent, { specialChars } from '@testing-library/user-event';
import mockDatasource, { datasourceId } from 'spec/fixtures/mockDatasource';
import {
FiltersConfigModal,
FiltersConfigModalProps,
@ -49,7 +50,18 @@ class MainPreset extends Preset {
}
const initialStoreState = {
datasources: [{ id: 1, table_name: 'Datasource 1' }],
datasources: mockDatasource,
};
const storeWithDatasourceWithoutTemporalColumns = {
...initialStoreState,
datasources: {
...initialStoreState.datasources,
[datasourceId]: {
...initialStoreState.datasources[datasourceId],
column_types: [0, 1],
},
},
};
fetchMock.get('glob:*/api/v1/dataset/1', {
@ -114,6 +126,7 @@ const DEFAULT_VALUE_REQUIRED_REGEX = /^default value is required$/i;
const PARENT_REQUIRED_REGEX = /^parent filter is required$/i;
const PRE_FILTER_REQUIRED_REGEX = /^pre-filter is required$/i;
const FILL_REQUIRED_FIELDS_REGEX = /fill all required fields to enable/;
const TIME_RANGE_PREFILTER_REGEX = /^time range$/i;
const props: FiltersConfigModalProps = {
isOpen: true,
@ -128,7 +141,7 @@ beforeAll(() => {
function defaultRender(
overrides?: Partial<FiltersConfigModalProps>,
initialState?: {},
initialState = initialStoreState,
) {
return render(<FiltersConfigModal {...props} {...overrides} />, {
useRedux: true,
@ -243,6 +256,20 @@ test('renders a time grain filter type', () => {
expect(screen.queryByText(ADVANCED_REGEX)).not.toBeInTheDocument();
});
test('render time filter types as disabled if there are no temporal columns in the dataset', () => {
defaultRender(undefined, storeWithDatasourceWithoutTemporalColumns);
userEvent.click(screen.getByText(VALUE_REGEX));
expect(screen.getByText(TIME_RANGE_REGEX).closest('div')).toHaveClass(
'Select__option--is-disabled',
);
expect(screen.getByText(TIME_GRAIN_REGEX).closest('div')).toHaveClass(
'Select__option--is-disabled',
);
expect(screen.getByText(TIME_COLUMN_REGEX).closest('div')).toHaveClass(
'Select__option--is-disabled',
);
});
test('validates the name', async () => {
defaultRender();
userEvent.click(screen.getByRole('button', { name: SAVE_REGEX }));
@ -287,6 +314,14 @@ test('validates the pre-filter value', async () => {
).toBeInTheDocument();
});
test("doesn't render time range pre-filter if there are no temporal columns in datasource", async () => {
defaultRender(undefined, storeWithDatasourceWithoutTemporalColumns);
userEvent.click(screen.getByText(ADVANCED_REGEX));
userEvent.click(getCheckbox(PRE_FILTER_REGEX));
expect(
screen.queryByText(TIME_RANGE_PREFILTER_REGEX),
).not.toBeInTheDocument();
});
/*
TODO
adds a new value filter type with all fields filled