From 0b43112873f984500e7018a0e496cc9bd89bd477 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Fri, 21 Apr 2023 08:50:44 +0200 Subject: [PATCH] fix: Further drilling by different groupby fields (#23754) --- .../ChartContextMenu/ChartContextMenu.tsx | 4 +- .../Chart/DrillBy/DrillByMenuItems.test.tsx | 9 +- .../Chart/DrillBy/DrillByMenuItems.tsx | 30 ++-- .../Chart/DrillBy/DrillByModal.test.tsx | 11 +- .../components/Chart/DrillBy/DrillByModal.tsx | 152 ++++++++++++------ 5 files changed, 129 insertions(+), 77 deletions(-) diff --git a/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx b/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx index 401c3fa992723..6ab80aad9e7ee 100644 --- a/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx +++ b/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx @@ -234,9 +234,7 @@ const ChartContextMenu = ( } menuItems.push( ) => render( , @@ -133,7 +132,7 @@ test('render disabled menu item for unsupported chart', async () => { }); test('render disabled menu item for supported chart, no filters', async () => { - renderMenu({ filters: [] }); + renderMenu({ drillByConfig: { filters: [], groupbyFieldName: 'groupby' } }); await expectDrillByDisabled('Drill by is not available for this data point'); }); @@ -237,6 +236,6 @@ test('When menu item is clicked, call onSelection with clicked column and drill column_name: 'col1', groupby: true, }, - defaultFilters, + { filters: defaultFilters, groupbyFieldName: 'groupby' }, ); }); diff --git a/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.tsx index b503b1eed5db5..17e013d29f1ff 100644 --- a/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.tsx +++ b/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.tsx @@ -29,8 +29,8 @@ import { Menu } from 'src/components/Menu'; import { BaseFormData, Behavior, - BinaryQueryObjectFilterClause, Column, + ContextMenuFilters, css, ensureIsArray, getChartMetadataRegistry, @@ -55,12 +55,10 @@ const SHOW_COLUMNS_SEARCH_THRESHOLD = 10; const SEARCH_INPUT_HEIGHT = 48; export interface DrillByMenuItemsProps { - filters?: BinaryQueryObjectFilterClause[]; + drillByConfig?: ContextMenuFilters['drillBy']; formData: BaseFormData & { [key: string]: any }; contextMenuY?: number; submenuIndex?: number; - groupbyFieldName?: string; - adhocFilterFieldName?: string; onSelection?: (...args: any) => void; onClick?: (event: MouseEvent) => void; openNewModal?: boolean; @@ -68,9 +66,7 @@ export interface DrillByMenuItemsProps { } export const DrillByMenuItems = ({ - filters, - groupbyFieldName, - adhocFilterFieldName, + drillByConfig, formData, contextMenuY = 0, submenuIndex = 0, @@ -90,13 +86,13 @@ export const DrillByMenuItems = ({ const handleSelection = useCallback( (event, column) => { onClick(event); - onSelection(column, filters); + onSelection(column, drillByConfig); setCurrentColumn(column); if (openNewModal) { setShowModal(true); } }, - [filters, onClick, onSelection, openNewModal], + [drillByConfig, onClick, onSelection, openNewModal], ); const closeModal = useCallback(() => { setShowModal(false); @@ -108,7 +104,9 @@ export const DrillByMenuItems = ({ setSearchInput(''); }, [columns.length]); - const hasDrillBy = ensureIsArray(filters).length && groupbyFieldName; + const hasDrillBy = + ensureIsArray(drillByConfig?.filters).length && + drillByConfig?.groupbyFieldName; const handlesDimensionContextMenu = useMemo( () => @@ -131,9 +129,9 @@ export const DrillByMenuItems = ({ .filter(column => column.groupby) .filter( column => - !ensureIsArray(formData[groupbyFieldName]).includes( - column.column_name, - ) && + !ensureIsArray( + formData[drillByConfig.groupbyFieldName ?? ''], + ).includes(column.column_name) && column.column_name !== formData.x_axis && ensureIsArray(excludedColumns)?.every( excludedCol => @@ -151,7 +149,7 @@ export const DrillByMenuItems = ({ addDangerToast, excludedColumns, formData, - groupbyFieldName, + drillByConfig?.groupbyFieldName, handlesDimensionContextMenu, hasDrillBy, ]); @@ -269,10 +267,8 @@ export const DrillByMenuItems = ({ {showModal && ( diff --git a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx index 66fd6a42ba63c..95e2f6027f211 100644 --- a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx +++ b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx @@ -82,6 +82,7 @@ const renderModal = async (modalProps: Partial = {}) => { formData={formData} onHideModal={() => setShowModal(false)} dataset={dataset} + drillByConfig={{ groupbyFieldName: 'groupby', filters: [] }} {...modalProps} /> )} @@ -148,7 +149,10 @@ test('should render alert banner when results fail to load', async () => { test('should generate Explore url', async () => { await renderModal({ column: { column_name: 'name' }, - filters: [{ col: 'gender', op: '==', val: 'boy' }], + drillByConfig: { + filters: [{ col: 'gender', op: '==', val: 'boy' }], + groupbyFieldName: 'groupby', + }, }); await waitFor(() => fetchMock.called(CHART_DATA_ENDPOINT)); const expectedRequestPayload = { @@ -208,7 +212,10 @@ test('should render radio buttons', async () => { test('render breadcrumbs', async () => { await renderModal({ column: { column_name: 'name' }, - filters: [{ col: 'gender', op: '==', val: 'boy' }], + drillByConfig: { + filters: [{ col: 'gender', op: '==', val: 'boy' }], + groupbyFieldName: 'groupby', + }, }); const breadcrumbItems = screen.getAllByTestId('drill-by-breadcrumb-item'); diff --git a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx index 0a05635645f12..3792a2f315cc3 100644 --- a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx +++ b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx @@ -26,7 +26,6 @@ import React, { } from 'react'; import { BaseFormData, - BinaryQueryObjectFilterClause, Column, QueryData, css, @@ -34,6 +33,7 @@ import { isDefined, t, useTheme, + ContextMenuFilters, } from '@superset-ui/core'; import { useSelector } from 'react-redux'; import { Link } from 'react-router-dom'; @@ -61,6 +61,8 @@ import { } from './useDrillByBreadcrumbs'; const DATA_SIZE = 15; + +const DEFAULT_ADHOC_FILTER_FIELD_NAME = 'adhoc_filters'; interface ModalFooterProps { closeModal?: () => void; formData: BaseFormData; @@ -123,26 +125,36 @@ const ModalFooter = ({ formData, closeModal }: ModalFooterProps) => { export interface DrillByModalProps { column?: Column; dataset: Dataset; - filters?: BinaryQueryObjectFilterClause[]; + drillByConfig: Required['drillBy']; formData: BaseFormData & { [key: string]: any }; - groupbyFieldName?: string; - adhocFilterFieldName?: string; onHideModal: () => void; } +type DrillByConfigs = (ContextMenuFilters['drillBy'] & { column?: Column })[]; + export default function DrillByModal({ column, dataset, - filters, + drillByConfig, formData, - groupbyFieldName = 'groupby', - adhocFilterFieldName = 'adhoc_filters', onHideModal, }: DrillByModalProps) { const theme = useTheme(); const { addDangerToast } = useToasts(); const [isChartDataLoading, setIsChartDataLoading] = useState(true); + const [drillByConfigs, setDrillByConfigs] = useState([ + { ...drillByConfig, column }, + ]); + + const { + column: currentColumn, + filters, + groupbyFieldName = drillByConfig.groupbyFieldName, + adhocFilterFieldName = drillByConfig.adhocFilterFieldName || + DEFAULT_ADHOC_FILTER_FIELD_NAME, + } = drillByConfigs[drillByConfigs.length - 1] || {}; + const initialGroupbyColumns = useMemo( () => ensureIsArray(formData[groupbyFieldName]) @@ -160,11 +172,7 @@ export default function DrillByModal({ [formData.datasource], ); - const [currentColumn, setCurrentColumn] = useState( - column, - ); const [currentFormData, setCurrentFormData] = useState(formData); - const [currentFilters, setCurrentFilters] = useState(filters || []); const [usedGroupbyColumns, setUsedGroupbyColumns] = useState( [...initialGroupbyColumns, column].filter(isDefined), ); @@ -174,19 +182,48 @@ export default function DrillByModal({ ]); const getNewGroupby = useCallback( - (groupbyCol: Column) => - Array.isArray(formData[groupbyFieldName]) + (groupbyCol: Column, fieldName = groupbyFieldName) => + Array.isArray(formData[fieldName]) ? [groupbyCol.column_name] : groupbyCol.column_name, [formData, groupbyFieldName], ); + const getFormDataChangesFromConfigs = useCallback( + (configs: DrillByConfigs) => + configs.reduce( + (acc, config) => { + if (config?.groupbyFieldName && config.column) { + acc.formData[config.groupbyFieldName] = getNewGroupby( + config.column, + config.groupbyFieldName, + ); + acc.overridenGroupbyFields.add(config.groupbyFieldName); + } + const adhocFilterFieldName = + config?.adhocFilterFieldName || DEFAULT_ADHOC_FILTER_FIELD_NAME; + acc.formData[adhocFilterFieldName] = [ + ...ensureIsArray(acc[adhocFilterFieldName]), + ...ensureIsArray(config.filters).map(filter => + simpleFilterToAdhoc(filter), + ), + ]; + acc.overridenAdhocFilterFields.add(adhocFilterFieldName); + + return acc; + }, + { + formData: {}, + overridenGroupbyFields: new Set(), + overridenAdhocFilterFields: new Set(), + }, + ), + [getNewGroupby], + ); + const onBreadcrumbClick = useCallback( (breadcrumb: DrillByBreadcrumb, index: number) => { - const newGroupbyCol = - index === 0 ? undefined : (breadcrumb.groupby as Column); - setCurrentColumn(newGroupbyCol); - setCurrentFilters(filters => filters.slice(0, index)); + setDrillByConfigs(prevConfigs => prevConfigs.slice(0, index)); setBreadcrumbsData(prevBreadcrumbs => { const newBreadcrumbs = prevBreadcrumbs.slice(0, index + 1); delete newBreadcrumbs[newBreadcrumbs.length - 1].filters; @@ -195,52 +232,61 @@ export default function DrillByModal({ setUsedGroupbyColumns(prevUsedGroupbyColumns => prevUsedGroupbyColumns.slice(0, index), ); - setCurrentFormData(prevFormData => ({ - ...prevFormData, - [groupbyFieldName]: newGroupbyCol - ? getNewGroupby(newGroupbyCol) - : formData[groupbyFieldName], - [adhocFilterFieldName]: [ - ...formData[adhocFilterFieldName], - ...prevFormData[adhocFilterFieldName].slice( - formData[adhocFilterFieldName].length, - formData[adhocFilterFieldName].length + index, - ), - ], - })); + setCurrentFormData(() => { + if (index === 0) { + return formData; + } + const { formData: overrideFormData, overridenAdhocFilterFields } = + getFormDataChangesFromConfigs(drillByConfigs.slice(0, index)); + + const newFormData = { + ...formData, + ...overrideFormData, + }; + overridenAdhocFilterFields.forEach(adhocFilterField => ({ + ...newFormData, + [adhocFilterField]: [ + ...formData[adhocFilterField], + ...overrideFormData[adhocFilterField], + ], + })); + return newFormData; + }); }, - [adhocFilterFieldName, formData, getNewGroupby, groupbyFieldName], + [drillByConfigs, formData, getFormDataChangesFromConfigs], ); const breadcrumbs = useDrillByBreadcrumbs(breadcrumbsData, onBreadcrumbClick); const drilledFormData = useMemo(() => { - let updatedFormData = { ...formData }; - if (currentColumn) { + let updatedFormData = { ...currentFormData }; + if (currentColumn && groupbyFieldName) { updatedFormData[groupbyFieldName] = getNewGroupby(currentColumn); } - const adhocFilters = currentFilters.map(filter => - simpleFilterToAdhoc(filter), - ); - updatedFormData = { - ...updatedFormData, - [adhocFilterFieldName]: [ - ...ensureIsArray(formData[adhocFilterFieldName]), - ...adhocFilters, - ], - }; + if (adhocFilterFieldName && Array.isArray(filters)) { + const adhocFilters = filters.map(filter => simpleFilterToAdhoc(filter)); + updatedFormData = { + ...updatedFormData, + [adhocFilterFieldName]: [ + ...ensureIsArray(formData[adhocFilterFieldName]), + ...adhocFilters, + ], + }; + } + updatedFormData.slice_id = 0; delete updatedFormData.slice_name; delete updatedFormData.dashboards; return updatedFormData; }, [ - formData, + currentFormData, currentColumn, - currentFilters, + filters, + adhocFilterFieldName, + formData, groupbyFieldName, getNewGroupby, - adhocFilterFieldName, ]); useEffect(() => { @@ -255,13 +301,19 @@ export default function DrillByModal({ }, [currentColumn]); const onSelection = useCallback( - (newColumn: Column, filters: BinaryQueryObjectFilterClause[]) => { - setCurrentColumn(newColumn); + ( + newColumn: Column, + drillByConfig: Required['drillBy'], + ) => { setCurrentFormData(drilledFormData); - setCurrentFilters(prevFilters => [...prevFilters, ...filters]); + setDrillByConfigs(prevConfigs => [ + ...prevConfigs, + { ...drillByConfig, column: newColumn }, + ]); setBreadcrumbsData(prevBreadcrumbs => { const newBreadcrumbs = [...prevBreadcrumbs, { groupby: newColumn }]; - newBreadcrumbs[newBreadcrumbs.length - 2].filters = filters; + newBreadcrumbs[newBreadcrumbs.length - 2].filters = + drillByConfig.filters; return newBreadcrumbs; }); },