diff --git a/superset-frontend/spec/fixtures/mockDatasource.js b/superset-frontend/spec/fixtures/mockDatasource.js index e434ad73b0..55d95ebde2 100644 --- a/superset-frontend/spec/fixtures/mockDatasource.js +++ b/superset-frontend/spec/fixtures/mockDatasource.js @@ -164,6 +164,7 @@ export default { column_name: 'num_girls', }, ], + column_types: [0, 1, 2], id, granularity_sqla: [['ds', 'ds']], name: 'birth_names', diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx index b8d64f45a4..aa83b8bbb8 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx @@ -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>(); - 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 ? ( + + {mappedName || name} + + ) : ( + mappedName || name + ), + isDisabled, }; })} onChange={({ value }: { value: string }) => { @@ -845,28 +881,30 @@ const FiltersConfigForm = ( } /> - {t('Time range')}} - initialValue={filterToEdit?.time_range || 'No filter'} - required={!hasAdhoc} - rules={[ - { - validator: preFilterValidator, - }, - ]} - > - { - setNativeFilterFieldValues(form, filterId, { - time_range: timeRange, - }); - forceUpdate(); - validatePreFilter(); - }} - /> - + {showTimeRangePicker && ( + {t('Time range')}} + initialValue={filterToEdit?.time_range || 'No filter'} + required={!hasAdhoc} + rules={[ + { + validator: preFilterValidator, + }, + ]} + > + { + setNativeFilterFieldValues(form, filterId, { + time_range: timeRange, + }); + forceUpdate(); + validatePreFilter(); + }} + /> + + )} {hasTimeRange && ( ({ +export const datasetToSelectOption = ( + item: DatasourceMeta & { table_name: string }, +): DatasetSelectValue => ({ value: item.id, label: item.table_name, }); diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx index 3ee4a9d884..80729cb46a 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigModal.test.tsx @@ -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, - initialState?: {}, + initialState = initialStoreState, ) { return render(, { 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