fix: Further drilling by different groupby fields (#23754)

apache · Apr 21, 2023 · 0b43112 · 0b43112
1 parent b31efba
commit 0b43112
Show file tree

Hide file tree

Showing 5 changed files with 129 additions and 77 deletions.
diff --git 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
@@ -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
@@ -29,8 +29,8 @@ import { Menu } from 'src/components/Menu';
 import {
   BaseFormData,
   Behavior,
-  BinaryQueryObjectFilterClause,
   Column,
+  ContextMenuFilters,
   css,
   ensureIsArray,
   getChartMetadataRegistry,
@@ -55,22 +55,18 @@ 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;
   excludedColumns?: Column[];
 }
 
 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
@@ -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');