This is an automated email from the ASF dual-hosted git repository. amitmiran pushed a commit to branch 1.3 in repository https://gitbox.apache.org/repos/asf/superset.git
commit 598397c51cb8841f68d3beec32eaf54cd6dcc269 Author: Einat Bertenthal <[email protected]> AuthorDate: Thu Jul 1 13:28:07 2021 +0300 feat(dashboard-groupby): group by - add ability to exclude columns (#15454) * feat: group by - add ability to exclude columns * fix: create column select in a more generic way * fix: MR comments * fix: remove description * fix: multiple value bug in column select * fix: initial value bug * fix: lint * fix: unit tests * fix: MR comments * fix: MR comment Co-authored-by: einatnielsen <[email protected]> (cherry picked from commit e5d4765986b90076a4a369423c8b744a756f68df) --- .../FiltersConfigForm/ColumnSelect.tsx | 17 ++-- .../FiltersConfigForm/FiltersConfigForm.tsx | 58 +++----------- .../FiltersConfigForm/getControlItemsMap.test.tsx | 6 +- .../FiltersConfigForm/getControlItemsMap.tsx | 92 ++++++++++++++++++++-- .../FiltersConfigModal/FiltersConfigForm/utils.ts | 2 +- .../components/GroupBy/GroupByFilterPlugin.tsx | 14 +++- .../src/filters/components/GroupBy/controlPanel.ts | 18 +++++ .../src/filters/components/Range/controlPanel.ts | 3 +- .../src/filters/components/Select/controlPanel.ts | 14 +++- .../src/filters/components/Time/controlPanel.ts | 17 ++++ 10 files changed, 177 insertions(+), 64 deletions(-) diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/ColumnSelect.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/ColumnSelect.tsx index 5ee4095..5a05119 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/ColumnSelect.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/ColumnSelect.tsx @@ -34,8 +34,9 @@ interface ColumnSelectProps { formField?: string; filterId: string; datasetId?: number; - value?: string; + value?: string | string[]; onChange?: (value: string) => void; + mode?: 'multiple' | 'tags'; } const localCache = new Map<string, any>(); @@ -57,6 +58,7 @@ export function ColumnSelect({ datasetId, value, onChange, + mode, }: ColumnSelectProps) { const [columns, setColumns] = useState<Column[]>(); const { addDangerToast } = useToasts(); @@ -101,11 +103,11 @@ export function ColumnSelect({ endpoint: `/api/v1/dataset/${datasetId}`, }).then( ({ json: { result } }) => { - if ( - !result.columns.some( - (column: Column) => column.column_name === value, - ) - ) { + const lookupValue = Array.isArray(value) ? value : [value]; + const valueExists = result.columns.some((column: Column) => + lookupValue?.includes(column.column_name), + ); + if (!valueExists) { resetColumnField(); } setColumns(result.columns); @@ -124,7 +126,8 @@ export function ColumnSelect({ return ( <Select - value={value} + mode={mode} + value={mode === 'multiple' ? value || [] : value} onChange={onChange} options={options} placeholder={t('Select a column')} 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 1ed9881..3f45ded 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx @@ -72,7 +72,6 @@ import { ColumnSelect } from './ColumnSelect'; import { NativeFiltersForm } from '../types'; import { datasetToSelectOption, - doesColumnMatchFilterType, FILTER_SUPPORTED_TYPES, hasTemporalColumns, setNativeFilterFieldValues, @@ -269,13 +268,6 @@ export interface FiltersConfigFormProps { parentFilters: { id: string; title: string }[]; } -// TODO: Need to do with it something -const FILTERS_WITHOUT_COLUMN = [ - 'filter_timegrain', - 'filter_timecolumn', - 'filter_groupby', -]; - const FILTERS_WITH_ADHOC_FILTERS = ['filter_select', 'filter_range']; const BASIC_CONTROL_ITEMS = ['enableEmptyFilter', 'multiSelect']; @@ -352,15 +344,14 @@ const FiltersConfigForm = ( // @ts-ignore const hasDataset = !!nativeFilterItems[formFilter?.filterType]?.value ?.datasourceCount; - const hasColumn = - hasDataset && !FILTERS_WITHOUT_COLUMN.includes(formFilter?.filterType); + const nativeFilterItem = nativeFilterItems[formFilter?.filterType] ?? {}; // @ts-ignore const enableNoResults = !!nativeFilterItem.value?.enableNoResults; const datasetId = formFilter?.dataset?.value; useEffect(() => { - if (datasetId && hasColumn) { + if (datasetId && hasDataset) { cachedSupersetGet({ endpoint: `/api/v1/dataset/${datasetId}`, }) @@ -376,7 +367,7 @@ const FiltersConfigForm = ( addDangerToast(response.message); }); } - }, [datasetId, hasColumn]); + }, [datasetId, hasDataset]); useImperativeHandle(ref, () => ({ changeTab(tab: 'configuration' | 'scoping') { @@ -384,10 +375,10 @@ const FiltersConfigForm = ( }, })); - const hasMetrics = hasColumn && !!metrics.length; + const hasMetrics = hasDataset && !!metrics.length; const hasFilledDataset = - !hasDataset || (datasetId && (formFilter?.column || !hasColumn)); + !hasDataset || (datasetId && (formFilter?.column || !hasDataset)); const hasAdditionalFilters = FILTERS_WITH_ADHOC_FILTERS.includes( formFilter?.filterType, @@ -484,10 +475,9 @@ const FiltersConfigForm = ( (defaultDatasetSelectOptions.length === 1 ? defaultDatasetSelectOptions[0].value : undefined); - const initColumn = filterToEdit?.targets[0]?.column?.name; const newFormData = getFormData({ datasetId, - groupby: hasColumn ? formFilter?.column : undefined, + groupby: hasDataset ? formFilter?.column : undefined, ...formFilter, }); @@ -546,7 +536,7 @@ const FiltersConfigForm = ( const showDefaultValue = !hasDataset || (!isDataDirty && hasFilledDataset); - const controlItems = formFilter + const { controlItems = {}, mainControlItems = {} } = formFilter ? getControlItemsMap({ disabled: false, forceUpdate, @@ -555,6 +545,7 @@ const FiltersConfigForm = ( filterType: formFilter.filterType, filterToEdit, formFilter, + removed, }) : {}; @@ -731,35 +722,10 @@ const FiltersConfigForm = ( }} /> </StyledFormItem> - {hasColumn && ( - <StyledFormItem - // don't show the column select unless we have a dataset - // style={{ display: datasetId == null ? undefined : 'none' }} - name={['filters', filterId, 'column']} - initialValue={initColumn} - label={<StyledLabel>{t('Column')}</StyledLabel>} - rules={[ - { required: !removed, message: t('Column is required') }, - ]} - data-test="field-input" - > - <ColumnSelect - form={form} - filterId={filterId} - datasetId={datasetId} - filterValues={column => - doesColumnMatchFilterType(formFilter?.filterType, column) - } - onChange={() => { - // We need reset default value when when column changed - setNativeFilterFieldValues(form, filterId, { - defaultDataMask: null, - }); - forceUpdate(); - }} - /> - </StyledFormItem> - )} + {hasDataset && + Object.keys(mainControlItems).map( + key => mainControlItems[key].element, + )} </StyledRowContainer> )} <StyledCollapse diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.test.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.test.tsx index fcff22d..b641a58 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.test.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.test.tsx @@ -77,6 +77,7 @@ const createControlItems = () => [ false, {}, { name: 'name_1', config: { renderTrigger: true, resetConfig: true } }, + { name: 'groupby', config: { multiple: true, required: false } }, ]; beforeEach(() => { @@ -87,7 +88,10 @@ function renderControlItems( controlItemsMap: ReturnType<typeof getControlItemsMap>, ) { return render( - <>{Object.values(controlItemsMap).map(value => value.element)}</>, + // @ts-ignore + <> + {Object.values(controlItemsMap.controlItems).map(value => value.element)} + </>, ); } diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.tsx index fd220e4..45c685d 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/getControlItemsMap.tsx @@ -26,10 +26,19 @@ import { FormInstance } from 'antd/lib/form'; import { getChartControlPanelRegistry, styled, t } from '@superset-ui/core'; import { Tooltip } from 'src/components/Tooltip'; import { FormItem } from 'src/components/Form'; -import { getControlItems, setNativeFilterFieldValues } from './utils'; +import { + doesColumnMatchFilterType, + getControlItems, + setNativeFilterFieldValues, +} from './utils'; import { NativeFiltersForm, NativeFiltersFormItem } from '../types'; -import { StyledRowFormItem } from './FiltersConfigForm'; +import { + StyledFormItem, + StyledLabel, + StyledRowFormItem, +} from './FiltersConfigForm'; import { Filter } from '../../types'; +import { ColumnSelect } from './ColumnSelect'; export interface ControlItemsProps { disabled: boolean; @@ -39,6 +48,7 @@ export interface ControlItemsProps { filterType: string; filterToEdit?: Filter; formFilter?: NativeFiltersFormItem; + removed?: boolean; } const CleanFormItem = styled(FormItem)` @@ -53,17 +63,86 @@ export default function getControlItemsMap({ filterType, filterToEdit, formFilter, + removed, }: ControlItemsProps) { const controlPanelRegistry = getChartControlPanelRegistry(); const controlItems = getControlItems(controlPanelRegistry.get(filterType)) ?? []; - const map: Record< + const mapControlItems: Record< + string, + { element: React.ReactNode; checked: boolean } + > = {}; + const mapMainControlItems: Record< string, { element: React.ReactNode; checked: boolean } > = {}; controlItems .filter( + (mainControlItem: CustomControlItem) => + mainControlItem?.name === 'groupby', + ) + .forEach(mainControlItem => { + const initialValue = + filterToEdit?.controlValues?.[mainControlItem.name] ?? + mainControlItem?.config?.default; + const initColumn = filterToEdit?.targets[0]?.column?.name; + const datasetId = formFilter?.dataset?.value; + + const element = ( + <> + <CleanFormItem + name={['filters', filterId, 'requiredFirst', mainControlItem.name]} + hidden + initialValue={ + mainControlItem?.config?.requiredFirst && + filterToEdit?.requiredFirst + } + /> + <StyledFormItem + // don't show the column select unless we have a dataset + // style={{ display: datasetId == null ? undefined : 'none' }} + name={['filters', filterId, 'column']} + initialValue={initColumn} + label={ + <StyledLabel> + {t(`${mainControlItem.config?.label}`) || t('Column')} + </StyledLabel> + } + rules={[ + { + required: mainControlItem.config?.required && !removed, // TODO: need to move ColumnSelect settings to controlPanel for all filters + message: t('Column is required'), + }, + ]} + data-test="field-input" + > + <ColumnSelect + mode={mainControlItem.config?.multiple && 'multiple'} + form={form} + filterId={filterId} + datasetId={datasetId} + filterValues={column => + doesColumnMatchFilterType(formFilter?.filterType || '', column) + } + onChange={() => { + // We need reset default value when when column changed + setNativeFilterFieldValues(form, filterId, { + defaultDataMask: null, + }); + forceUpdate(); + }} + /> + </StyledFormItem> + </> + ); + mapMainControlItems[mainControlItem.name] = { + element, + checked: initialValue, + }; + }); + controlItems + .filter( (controlItem: CustomControlItem) => controlItem?.config?.renderTrigger && controlItem.name !== 'sortAscending', @@ -129,7 +208,10 @@ export default function getControlItemsMap({ </Tooltip> </> ); - map[controlItem.name] = { element, checked: initialValue }; + mapControlItems[controlItem.name] = { element, checked: initialValue }; }); - return map; + return { + controlItems: mapControlItems, + mainControlItems: mapMainControlItems, + }; } diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/utils.ts b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/utils.ts index 609f7ed..d5d97a8 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/utils.ts +++ b/superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/utils.ts @@ -98,4 +98,4 @@ export const hasTemporalColumns = ( export const doesColumnMatchFilterType = (filterType: string, column: Column) => !column.type_generic || !(filterType in FILTER_SUPPORTED_TYPES) || - FILTER_SUPPORTED_TYPES[filterType].includes(column.type_generic); + FILTER_SUPPORTED_TYPES[filterType]?.includes(column.type_generic); diff --git a/superset-frontend/src/filters/components/GroupBy/GroupByFilterPlugin.tsx b/superset-frontend/src/filters/components/GroupBy/GroupByFilterPlugin.tsx index 6e8a3bf..11739c8 100644 --- a/superset-frontend/src/filters/components/GroupBy/GroupByFilterPlugin.tsx +++ b/superset-frontend/src/filters/components/GroupBy/GroupByFilterPlugin.tsx @@ -68,7 +68,19 @@ export default function PluginFilterGroupBy(props: PluginFilterGroupByProps) { // so we can process it like this `JSON.stringify` or start to use `Immer` }, [JSON.stringify(defaultValue), multiSelect]); - const columns = data || []; + const groupby = formData?.groupby?.[0]?.length + ? formData?.groupby?.[0] + : null; + + const withData = groupby + ? data.filter(dataItem => + // @ts-ignore + groupby.includes(dataItem.column_name), + ) + : data; + + const columns = data ? withData : []; + const placeholderText = columns.length === 0 ? t('No columns') diff --git a/superset-frontend/src/filters/components/GroupBy/controlPanel.ts b/superset-frontend/src/filters/components/GroupBy/controlPanel.ts index 970683e..e6a6add 100644 --- a/superset-frontend/src/filters/components/GroupBy/controlPanel.ts +++ b/superset-frontend/src/filters/components/GroupBy/controlPanel.ts @@ -18,6 +18,7 @@ */ import { ControlPanelConfig, sections } from '@superset-ui/chart-controls'; import { t } from '@superset-ui/core'; +import { sharedControls } from '@superset-ui/chart-controls/lib'; import { DEFAULT_FORM_DATA } from './types'; const { multiSelect } = DEFAULT_FORM_DATA; @@ -27,6 +28,23 @@ const config: ControlPanelConfig = { // @ts-ignore sections.legacyRegularTime, { + label: t('Query'), + expanded: true, + controlSetRows: [ + [ + { + name: 'groupby', + config: { + ...sharedControls.groupby, + label: 'Columns to show', + multiple: true, + required: false, + }, + }, + ], + ], + }, + { label: t('UI Configuration'), expanded: true, controlSetRows: [ diff --git a/superset-frontend/src/filters/components/Range/controlPanel.ts b/superset-frontend/src/filters/components/Range/controlPanel.ts index ad2ecfc..555b019 100644 --- a/superset-frontend/src/filters/components/Range/controlPanel.ts +++ b/superset-frontend/src/filters/components/Range/controlPanel.ts @@ -36,8 +36,7 @@ const config: ControlPanelConfig = { config: { ...sharedControls.groupby, label: 'Column', - description: - 'The numeric column based on which to calculate the range', + required: true, }, }, ], diff --git a/superset-frontend/src/filters/components/Select/controlPanel.ts b/superset-frontend/src/filters/components/Select/controlPanel.ts index 80aad2c..c06d73c 100644 --- a/superset-frontend/src/filters/components/Select/controlPanel.ts +++ b/superset-frontend/src/filters/components/Select/controlPanel.ts @@ -18,6 +18,7 @@ */ import { t, validateNonEmpty } from '@superset-ui/core'; import { ControlPanelConfig, sections } from '@superset-ui/chart-controls'; +import { sharedControls } from '@superset-ui/chart-controls/lib'; import { DEFAULT_FORM_DATA } from './types'; const { @@ -36,7 +37,18 @@ const config: ControlPanelConfig = { { label: t('Query'), expanded: true, - controlSetRows: [['groupby']], + controlSetRows: [ + [ + { + name: 'groupby', + config: { + ...sharedControls.groupby, + label: 'Column', + required: true, + }, + }, + ], + ], }, { label: t('UI Configuration'), diff --git a/superset-frontend/src/filters/components/Time/controlPanel.ts b/superset-frontend/src/filters/components/Time/controlPanel.ts index 466991f..9fcbe06 100644 --- a/superset-frontend/src/filters/components/Time/controlPanel.ts +++ b/superset-frontend/src/filters/components/Time/controlPanel.ts @@ -18,11 +18,28 @@ */ import { ControlPanelConfig } from '@superset-ui/chart-controls'; import { t } from '@superset-ui/core'; +import { sharedControls } from '@superset-ui/chart-controls/lib'; const config: ControlPanelConfig = { // For control input types, see: superset-frontend/src/explore/components/controls/index.js controlPanelSections: [ { + label: t('Query'), + expanded: true, + controlSetRows: [ + [ + { + name: 'groupby', + config: { + ...sharedControls.groupby, + label: 'Column', + required: true, + }, + }, + ], + ], + }, + { label: t('UI Configuration'), expanded: true, controlSetRows: [
