You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by kg...@apache.org on 2023/04/21 06:50:54 UTC

[superset] branch master updated: fix: Further drilling by different groupby fields (#23754)

This is an automated email from the ASF dual-hosted git repository.

kgabryje pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git


The following commit(s) were added to refs/heads/master by this push:
     new 0b43112873 fix: Further drilling by different groupby fields (#23754)
0b43112873 is described below

commit 0b43112873f984500e7018a0e496cc9bd89bd477
Author: Kamil Gabryjelski <ka...@gmail.com>
AuthorDate: Fri Apr 21 08:50:44 2023 +0200

    fix: Further drilling by different groupby fields (#23754)
---
 .../Chart/ChartContextMenu/ChartContextMenu.tsx    |   4 +-
 .../Chart/DrillBy/DrillByMenuItems.test.tsx        |   9 +-
 .../components/Chart/DrillBy/DrillByMenuItems.tsx  |  30 ++--
 .../components/Chart/DrillBy/DrillByModal.test.tsx |  11 +-
 .../src/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 401c3fa992..6ab80aad9e 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(
       <DrillByMenuItems
-        filters={filters?.drillBy?.filters}
-        groupbyFieldName={filters?.drillBy?.groupbyFieldName}
-        adhocFilterFieldName={filters?.drillBy?.adhocFilterFieldName}
+        drillByConfig={filters?.drillBy}
         onSelection={onSelection}
         formData={formData}
         contextMenuY={clientY}
diff --git a/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.test.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.test.tsx
index b0e3bf62b5..7d411e050b 100644
--- a/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.test.tsx
+++ b/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.test.tsx
@@ -61,15 +61,14 @@ const defaultFilters = [
 
 const renderMenu = ({
   formData = defaultFormData,
-  filters = defaultFilters,
+  drillByConfig = { filters: defaultFilters, groupbyFieldName: 'groupby' },
   ...rest
 }: Partial<DrillByMenuItemsProps>) =>
   render(
     <Menu>
       <DrillByMenuItems
         formData={formData ?? defaultFormData}
-        filters={filters}
-        groupbyFieldName="groupby"
+        drillByConfig={drillByConfig}
         {...rest}
       />
     </Menu>,
@@ -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 b503b1eed5..17e013d29f 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 && (
         <DrillByModal
           column={currentColumn}
-          filters={filters}
+          drillByConfig={drillByConfig}
           formData={formData}
-          groupbyFieldName={groupbyFieldName}
-          adhocFilterFieldName={adhocFilterFieldName}
           onHideModal={closeModal}
           dataset={dataset!}
         />
diff --git a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx
index 66fd6a42ba..95e2f6027f 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<DrillByModalProps> = {}) => {
             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 0a05635645..3792a2f315 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<ContextMenuFilters>['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<DrillByConfigs>([
+    { ...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 | undefined>(
-    column,
-  );
   const [currentFormData, setCurrentFormData] = useState(formData);
-  const [currentFilters, setCurrentFilters] = useState(filters || []);
   const [usedGroupbyColumns, setUsedGroupbyColumns] = useState<Column[]>(
     [...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<string>(),
+          overridenAdhocFilterFields: new Set<string>(),
+        },
+      ),
+    [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<ContextMenuFilters>['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;
       });
     },