feat: Implement breadcrumbs in Drill By modal (apache#23664)

sebastianliebscher · Apr 28, 2023 · b33650f · b33650f
1 parent 971bf9f
commit b33650f
Show file tree

Hide file tree

Showing 8 changed files with 414 additions and 83 deletions.
diff --git a/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx b/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx
@@ -236,6 +236,7 @@ const ChartContextMenu = (
       <DrillByMenuItems
         filters={filters?.drillBy?.filters}
         groupbyFieldName={filters?.drillBy?.groupbyFieldName}
+        adhocFilterFieldName={filters?.drillBy?.adhocFilterFieldName}
         onSelection={onSelection}
         formData={formData}
         contextMenuY={clientY}

diff --git a/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.tsx
@@ -59,6 +59,7 @@ export interface DrillByMenuItemsProps {
   contextMenuY?: number;
   submenuIndex?: number;
   groupbyFieldName?: string;
+  adhocFilterFieldName?: string;
   onSelection?: (...args: any) => void;
   onClick?: (event: MouseEvent) => void;
   openNewModal?: boolean;
@@ -68,6 +69,7 @@ export interface DrillByMenuItemsProps {
 export const DrillByMenuItems = ({
   filters,
   groupbyFieldName,
+  adhocFilterFieldName,
   formData,
   contextMenuY = 0,
   submenuIndex = 0,
@@ -130,6 +132,11 @@ export const DrillByMenuItems = ({
                 column =>
                   !ensureIsArray(formData[groupbyFieldName]).includes(
                     column.column_name,
+                  ) &&
+                  column.column_name !== formData.x_axis &&
+                  ensureIsArray(excludedColumns)?.every(
+                    excludedCol =>
+                      excludedCol.column_name !== column.column_name,
                   ),
               ),
           );
@@ -138,7 +145,13 @@ export const DrillByMenuItems = ({
           supersetGetCache.delete(`/api/v1/dataset/${datasetId}`);
         });
     }
-  }, [formData, groupbyFieldName, handlesDimensionContextMenu, hasDrillBy]);
+  }, [
+    excludedColumns,
+    formData,
+    groupbyFieldName,
+    handlesDimensionContextMenu,
+    hasDrillBy,
+  ]);
 
   const handleInput = useCallback((e: ChangeEvent<HTMLInputElement>) => {
     e.stopPropagation();
@@ -148,16 +161,12 @@ export const DrillByMenuItems = ({
 
   const filteredColumns = useMemo(
     () =>
-      columns.filter(
-        column =>
-          (column.verbose_name || column.column_name)
-            .toLowerCase()
-            .includes(searchInput.toLowerCase()) &&
-          !ensureIsArray(excludedColumns)?.some(
-            col => col.column_name === column.column_name,
-          ),
+      columns.filter(column =>
+        (column.verbose_name || column.column_name)
+          .toLowerCase()
+          .includes(searchInput.toLowerCase()),
       ),
-    [columns, excludedColumns, searchInput],
+    [columns, searchInput],
   );
 
   const submenuYOffset = useMemo(
@@ -260,6 +269,7 @@ export const DrillByMenuItems = ({
           filters={filters}
           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
@@ -21,12 +21,12 @@ import React, { useState } from 'react';
 import fetchMock from 'fetch-mock';
 import { omit, isUndefined, omitBy } from 'lodash';
 import userEvent from '@testing-library/user-event';
-import { waitFor } from '@testing-library/react';
+import { waitFor, within } from '@testing-library/react';
 import { render, screen } from 'spec/helpers/testing-library';
 import chartQueries, { sliceId } from 'spec/fixtures/mockChartQueries';
 import mockState from 'spec/fixtures/mockState';
 import { DashboardPageIdContext } from 'src/dashboard/containers/DashboardPage';
-import DrillByModal from './DrillByModal';
+import DrillByModal, { DrillByModalProps } from './DrillByModal';
 
 const CHART_DATA_ENDPOINT = 'glob:*/api/v1/chart/data*';
 const FORM_DATA_KEY_ENDPOINT = 'glob:*/api/v1/explore/form_data';
@@ -60,9 +60,15 @@ const dataset = {
       last_name: 'Connor',
     },
   ],
+  columns: [
+    {
+      column_name: 'gender',
+    },
+    { column_name: 'name' },
+  ],
 };
 
-const renderModal = async () => {
+const renderModal = async (modalProps: Partial<DrillByModalProps> = {}) => {
   const DrillByModalWrapper = () => {
     const [showModal, setShowModal] = useState(false);
 
@@ -76,6 +82,7 @@ const renderModal = async () => {
             formData={formData}
             onHideModal={() => setShowModal(false)}
             dataset={dataset}
+            {...modalProps}
           />
         )}
       </DashboardPageIdContext.Provider>
@@ -127,14 +134,29 @@ test('should render loading indicator', async () => {
 });
 
 test('should generate Explore url', async () => {
-  await renderModal();
+  await renderModal({
+    column: { column_name: 'name' },
+    filters: [{ col: 'gender', op: '==', val: 'boy' }],
+  });
   await waitFor(() => fetchMock.called(CHART_DATA_ENDPOINT));
   const expectedRequestPayload = {
     form_data: {
       ...omitBy(
         omit(formData, ['slice_id', 'slice_name', 'dashboards']),
         isUndefined,
       ),
+      groupby: ['name'],
+      adhoc_filters: [
+        ...formData.adhoc_filters,
+        {
+          clause: 'WHERE',
+          comparator: 'boy',
+          expressionType: 'SIMPLE',
+          operator: '==',
+          operatorId: 'EQUALS',
+          subject: 'gender',
+        },
+      ],
       slice_id: 0,
       result_format: 'json',
       result_type: 'full',
@@ -170,3 +192,25 @@ test('should render radio buttons', async () => {
   expect(chartRadio).not.toBeChecked();
   expect(tableRadio).toBeChecked();
 });
+
+test('render breadcrumbs', async () => {
+  await renderModal({
+    column: { column_name: 'name' },
+    filters: [{ col: 'gender', op: '==', val: 'boy' }],
+  });
+
+  const breadcrumbItems = screen.getAllByTestId('drill-by-breadcrumb-item');
+  expect(breadcrumbItems).toHaveLength(2);
+  expect(
+    within(breadcrumbItems[0]).getByText('gender (boy)'),
+  ).toBeInTheDocument();
+  expect(within(breadcrumbItems[1]).getByText('name')).toBeInTheDocument();
+
+  userEvent.click(screen.getByText('gender (boy)'));
+
+  const newBreadcrumbItems = screen.getAllByTestId('drill-by-breadcrumb-item');
+  // we need to assert that there is only 1 element now
+  // eslint-disable-next-line jest-dom/prefer-in-document
+  expect(newBreadcrumbItems).toHaveLength(1);
+  expect(within(breadcrumbItems[0]).getByText('gender')).toBeInTheDocument();
+});