From 61c2be2c32315958c0da7a091b80c09aa14cd38f Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Fri, 7 Apr 2023 16:16:54 +0200 Subject: [PATCH 1/5] feat: Implement breadcrumbs in drill by modal --- .../ChartContextMenu/ChartContextMenu.tsx | 1 + .../Chart/DrillBy/DrillByMenuItems.tsx | 30 ++- .../components/Chart/DrillBy/DrillByModal.tsx | 173 ++++++++++++------ .../Chart/DrillBy/useDisplayModeToggle.tsx | 64 +++++++ .../Chart/DrillBy/useDrillByBreadcrumbs.tsx | 89 +++++++++ superset-frontend/src/components/index.ts | 1 + 6 files changed, 288 insertions(+), 70 deletions(-) create mode 100644 superset-frontend/src/components/Chart/DrillBy/useDisplayModeToggle.tsx create mode 100644 superset-frontend/src/components/Chart/DrillBy/useDrillByBreadcrumbs.tsx diff --git a/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx b/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx index 063ed787b13a8..401c3fa992723 100644 --- a/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx +++ b/superset-frontend/src/components/Chart/ChartContextMenu/ChartContextMenu.tsx @@ -236,6 +236,7 @@ const ChartContextMenu = ( 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) => { 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.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx index 6a9a82b5733ec..1280a1afc2104 100644 --- a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx +++ b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx @@ -31,6 +31,7 @@ import { QueryData, css, ensureIsArray, + isDefined, t, useTheme, } from '@superset-ui/core'; @@ -39,7 +40,6 @@ import { Link } from 'react-router-dom'; import Modal from 'src/components/Modal'; import Loading from 'src/components/Loading'; import Button from 'src/components/Button'; -import { Radio } from 'src/components/Radio'; import { RootState } from 'src/dashboard/types'; import { DashboardPageIdContext } from 'src/dashboard/containers/DashboardPage'; import { postFormData } from 'src/explore/exploreUtils/formData'; @@ -52,6 +52,11 @@ import DrillByChart from './DrillByChart'; import { ContextMenuItem } from '../ChartContextMenu/ChartContextMenu'; import { useContextMenu } from '../ChartContextMenu/useContextMenu'; import { getChartDataRequest } from '../chartAction'; +import { useDisplayModeToggle } from './useDisplayModeToggle'; +import { + DrillByBreadcrumb, + useDrillByBreadcrumbs, +} from './useDrillByBreadcrumbs'; const DATA_SIZE = 15; interface ModalFooterProps { @@ -107,6 +112,7 @@ interface DrillByModalProps { filters?: BinaryQueryObjectFilterClause[]; formData: BaseFormData & { [key: string]: any }; groupbyFieldName?: string; + adhocFilterFieldName?: string; onHideModal: () => void; } @@ -116,36 +122,87 @@ export default function DrillByModal({ filters, formData, groupbyFieldName = 'groupby', + adhocFilterFieldName = 'adhoc_filters', onHideModal, }: DrillByModalProps) { const theme = useTheme(); - const [chartDataResult, setChartDataResult] = useState(); - const [drillByDisplayMode, setDrillByDisplayMode] = useState( - DrillByType.Chart, + + const initialGroupbyColumns = useMemo( + () => + ensureIsArray(formData[groupbyFieldName]) + .map(colName => + dataset.columns?.find(col => col.column_name === colName), + ) + .filter(isDefined), + [dataset.columns, formData, groupbyFieldName], ); + + const { displayModeToggle, drillByDisplayMode } = useDisplayModeToggle(); + const [chartDataResult, setChartDataResult] = useState(); const [datasourceId] = useMemo( () => formData.datasource.split('__'), [formData.datasource], ); - const [currentColumn, setCurrentColumn] = useState(column); + // currentColumn can be an array when the original chart is grouped by multiple + // columns and user navigates back to the original chart by clicking the first + // breadcrumb + const [currentColumn, setCurrentColumn] = useState< + Column | Column[] | undefined + >(column); const [currentFormData, setCurrentFormData] = useState(formData); - const [currentFilters, setCurrentFilters] = useState(filters); - const [usedGroupbyColumns, setUsedGroupbyColumns] = useState([ - ...ensureIsArray(formData[groupbyFieldName]).map(colName => - dataset.columns?.find(col => col.column_name === colName), - ), - column, + const [currentFilters, setCurrentFilters] = useState(filters || []); + const [usedGroupbyColumns, setUsedGroupbyColumns] = useState( + [...initialGroupbyColumns, column].filter(isDefined), + ); + const [breadcrumbsData, setBreadcrumbsData] = useState([ + { groupby: initialGroupbyColumns, filters }, + { groupby: column || [] }, ]); - const updatedFormData = useMemo(() => { - let updatedFormData = { ...currentFormData }; + const getNewGroupby = useCallback( + (groupbyCol: Column | Column[]) => { + const columnNames = ensureIsArray(groupbyCol).map(col => col.column_name); + return Array.isArray(formData[groupbyFieldName]) + ? columnNames + : columnNames[0]; + }, + [formData, groupbyFieldName], + ); + + const onBreadcrumbClick = useCallback( + (breadcrumb: DrillByBreadcrumb, index: number) => { + setCurrentColumn(breadcrumb.groupby); + setCurrentFilters(filters => filters.slice(0, index)); + setBreadcrumbsData(prevBreadcrumbs => { + const newBreadcrumbs = prevBreadcrumbs.slice(0, index + 1); + delete newBreadcrumbs[newBreadcrumbs.length - 1].filters; + return newBreadcrumbs; + }); + setUsedGroupbyColumns(prevUsedGroupbyColumns => + prevUsedGroupbyColumns.slice(0, index), + ); + setCurrentFormData(prevFormData => ({ + ...prevFormData, + [groupbyFieldName]: getNewGroupby(breadcrumb.groupby), + [adhocFilterFieldName]: [ + ...formData[adhocFilterFieldName], + ...prevFormData[adhocFilterFieldName].slice( + formData[adhocFilterFieldName].length, + formData[adhocFilterFieldName].length + index, + ), + ], + })); + }, + [adhocFilterFieldName, formData, getNewGroupby, groupbyFieldName], + ); + + const breadcrumbs = useDrillByBreadcrumbs(breadcrumbsData, onBreadcrumbClick); + + const drilledFormData = useMemo(() => { + let updatedFormData = { ...formData }; if (currentColumn) { - updatedFormData[groupbyFieldName] = Array.isArray( - currentFormData[groupbyFieldName], - ) - ? [currentColumn.column_name] - : currentColumn.column_name; + updatedFormData[groupbyFieldName] = getNewGroupby(currentColumn); } if (currentFilters) { @@ -154,8 +211,8 @@ export default function DrillByModal({ ); updatedFormData = { ...updatedFormData, - adhoc_filters: [ - ...ensureIsArray(currentFormData.adhoc_filters), + [adhocFilterFieldName]: [ + ...ensureIsArray(formData[adhocFilterFieldName]), ...adhocFilters, ], }; @@ -164,23 +221,44 @@ export default function DrillByModal({ delete updatedFormData.slice_name; delete updatedFormData.dashboards; return updatedFormData; - }, [currentColumn, currentFormData, currentFilters, groupbyFieldName]); + }, [ + formData, + currentColumn, + currentFilters, + groupbyFieldName, + getNewGroupby, + adhocFilterFieldName, + ]); useEffect(() => { - setUsedGroupbyColumns(cols => - cols.includes(currentColumn) ? cols : [...cols, currentColumn], - ); + setUsedGroupbyColumns(usedCols => { + const currentColumns = ensureIsArray(currentColumn); + return !currentColumn || + usedCols.some(usedCol => + currentColumns.some( + currentCol => currentCol?.column_name === usedCol.column_name, + ), + ) + ? usedCols + : [...usedCols, ...currentColumns]; + }); }, [currentColumn]); const onSelection = useCallback( (newColumn: Column, filters: BinaryQueryObjectFilterClause[]) => { setCurrentColumn(newColumn); - setCurrentFormData(updatedFormData); - setCurrentFilters(filters); + setCurrentFormData(drilledFormData); + setCurrentFilters(prevFilters => [...prevFilters, ...filters]); + setBreadcrumbsData(prevBreadcrumbs => { + const newBreadcrumbs = [...prevBreadcrumbs, { groupby: newColumn }]; + newBreadcrumbs[newBreadcrumbs.length - 2].filters = filters; + return newBreadcrumbs; + }); }, - [updatedFormData], + [drilledFormData], ); + console.log(currentFormData); const additionalConfig = useMemo( () => ({ drillBy: { excludedColumns: usedGroupbyColumns, openNewModal: false }, @@ -206,14 +284,15 @@ export default function DrillByModal({ }); useEffect(() => { - if (updatedFormData) { + if (drilledFormData) { + setChartDataResult(undefined); getChartDataRequest({ - formData: updatedFormData, + formData: drilledFormData, }).then(({ json }) => { setChartDataResult(json.result); }); } - }, [updatedFormData]); + }, [drilledFormData]); const { metadataBar } = useDatasetMetadataBar({ dataset }); return ( @@ -226,7 +305,7 @@ export default function DrillByModal({ show onHide={onHideModal ?? (() => null)} title={t('Drill by: %s', chartName)} - footer={} + footer={} responsive resizable resizableConfig={{ @@ -249,38 +328,12 @@ export default function DrillByModal({ `} > {metadataBar} -
- { - setDrillByDisplayMode(value); - }} - defaultValue={DrillByType.Chart} - > - - {t('Chart')} - - - {t('Table')} - - -
+ {breadcrumbs} + {displayModeToggle} {!chartDataResult && } {drillByDisplayMode === DrillByType.Chart && chartDataResult && ( { + const [drillByDisplayMode, setDrillByDisplayMode] = useState( + DrillByType.Chart, + ); + + const displayModeToggle = useMemo( + () => ( +
css` + margin-bottom: ${theme.gridUnit * 6}px; + .ant-radio-button-wrapper-checked:not(.ant-radio-button-wrapper-disabled):focus-within { + box-shadow: none; + } + `} + > + { + setDrillByDisplayMode(value); + }} + defaultValue={DrillByType.Chart} + > + + {t('Chart')} + + + {t('Table')} + + +
+ ), + [], + ); + return { displayModeToggle, drillByDisplayMode }; +}; diff --git a/superset-frontend/src/components/Chart/DrillBy/useDrillByBreadcrumbs.tsx b/superset-frontend/src/components/Chart/DrillBy/useDrillByBreadcrumbs.tsx new file mode 100644 index 0000000000000..4559cbf0ba335 --- /dev/null +++ b/superset-frontend/src/components/Chart/DrillBy/useDrillByBreadcrumbs.tsx @@ -0,0 +1,89 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import React, { useMemo } from 'react'; +import { + BinaryQueryObjectFilterClause, + Column, + css, + ensureIsArray, + styled, + SupersetTheme, +} from '@superset-ui/core'; +import { AntdBreadcrumb } from 'src/components/index'; +import { noOp } from 'src/utils/common'; + +export interface DrillByBreadcrumb { + groupby: Column | Column[]; + filters?: BinaryQueryObjectFilterClause[]; +} + +const BreadcrumbItem = styled(AntdBreadcrumb.Item)<{ isClickable: boolean }>` + ${({ theme, isClickable }) => css` + cursor: ${isClickable ? 'pointer' : 'auto'}; + color: ${theme.colors.grayscale.light1}; + transition: color ease-in ${theme.transitionTiming}s; + .ant-breadcrumb > span:last-child > & { + color: ${theme.colors.grayscale.dark1}; + } + &:hover { + color: ${isClickable ? theme.colors.grayscale.dark1 : 'inherit'}; + } + `} +`; + +export const useDrillByBreadcrumbs = ( + breadcrumbsData: DrillByBreadcrumb[], + onBreadcrumbClick: ( + breadcrumb: DrillByBreadcrumb, + index: number, + ) => void = noOp, +) => + useMemo(() => { + // the last breadcrumb is not clickable + const isClickable = (index: number) => index < breadcrumbsData.length - 1; + return ( + css` + margin: ${theme.gridUnit * 2}px 0 ${theme.gridUnit * 4}px; + `} + > + {breadcrumbsData.map((breadcrumb, index) => ( + onBreadcrumbClick(breadcrumb, index) + : noOp + } + > + {ensureIsArray(breadcrumb.groupby) + .map(column => column.verbose_name || column.column_name) + .join(', ')}{' '} + {breadcrumb.filters + ? `(${breadcrumb.filters + .map(filter => filter.formattedVal || filter.val) + .join(', ')})` + : ''} + + ))} + + ); + }, [breadcrumbsData, onBreadcrumbClick]); diff --git a/superset-frontend/src/components/index.ts b/superset-frontend/src/components/index.ts index bfa341a9ddd18..4c425b053464e 100644 --- a/superset-frontend/src/components/index.ts +++ b/superset-frontend/src/components/index.ts @@ -55,6 +55,7 @@ export { * or extending the components in src/components. */ export { + Breadcrumb as AntdBreadcrumb, Button as AntdButton, Card as AntdCard, Checkbox as AntdCheckbox, From 52d7f4be1f92e0d50ddab87ab2497069c47368a6 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Wed, 12 Apr 2023 18:06:49 +0200 Subject: [PATCH 2/5] Remove console log --- superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx index 1280a1afc2104..82aa73a513cd7 100644 --- a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx +++ b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx @@ -258,7 +258,6 @@ export default function DrillByModal({ [drilledFormData], ); - console.log(currentFormData); const additionalConfig = useMemo( () => ({ drillBy: { excludedColumns: usedGroupbyColumns, openNewModal: false }, From b226125c137bedf35b97a53cb2e0564024aae72e Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Thu, 13 Apr 2023 13:31:27 +0200 Subject: [PATCH 3/5] Refactor currentColumn logic --- .../components/Chart/DrillBy/DrillByModal.tsx | 66 +++++++++---------- 1 file changed, 30 insertions(+), 36 deletions(-) diff --git a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx index 82aa73a513cd7..03f101b2bd6c0 100644 --- a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx +++ b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx @@ -144,12 +144,9 @@ export default function DrillByModal({ [formData.datasource], ); - // currentColumn can be an array when the original chart is grouped by multiple - // columns and user navigates back to the original chart by clicking the first - // breadcrumb - const [currentColumn, setCurrentColumn] = useState< - Column | Column[] | undefined - >(column); + const [currentColumn, setCurrentColumn] = useState( + column, + ); const [currentFormData, setCurrentFormData] = useState(formData); const [currentFilters, setCurrentFilters] = useState(filters || []); const [usedGroupbyColumns, setUsedGroupbyColumns] = useState( @@ -161,18 +158,18 @@ export default function DrillByModal({ ]); const getNewGroupby = useCallback( - (groupbyCol: Column | Column[]) => { - const columnNames = ensureIsArray(groupbyCol).map(col => col.column_name); - return Array.isArray(formData[groupbyFieldName]) - ? columnNames - : columnNames[0]; - }, + (groupbyCol: Column) => + Array.isArray(formData[groupbyFieldName]) + ? [groupbyCol.column_name] + : groupbyCol.column_name, [formData, groupbyFieldName], ); const onBreadcrumbClick = useCallback( (breadcrumb: DrillByBreadcrumb, index: number) => { - setCurrentColumn(breadcrumb.groupby); + const newGroupbyCol = + index === 0 ? undefined : (breadcrumb.groupby as Column); + setCurrentColumn(newGroupbyCol); setCurrentFilters(filters => filters.slice(0, index)); setBreadcrumbsData(prevBreadcrumbs => { const newBreadcrumbs = prevBreadcrumbs.slice(0, index + 1); @@ -184,7 +181,9 @@ export default function DrillByModal({ ); setCurrentFormData(prevFormData => ({ ...prevFormData, - [groupbyFieldName]: getNewGroupby(breadcrumb.groupby), + [groupbyFieldName]: newGroupbyCol + ? getNewGroupby(newGroupbyCol) + : formData[groupbyFieldName], [adhocFilterFieldName]: [ ...formData[adhocFilterFieldName], ...prevFormData[adhocFilterFieldName].slice( @@ -205,18 +204,16 @@ export default function DrillByModal({ updatedFormData[groupbyFieldName] = getNewGroupby(currentColumn); } - if (currentFilters) { - const adhocFilters = currentFilters.map(filter => - simpleFilterToAdhoc(filter), - ); - updatedFormData = { - ...updatedFormData, - [adhocFilterFieldName]: [ - ...ensureIsArray(formData[adhocFilterFieldName]), - ...adhocFilters, - ], - }; - } + const adhocFilters = currentFilters.map(filter => + simpleFilterToAdhoc(filter), + ); + updatedFormData = { + ...updatedFormData, + [adhocFilterFieldName]: [ + ...ensureIsArray(formData[adhocFilterFieldName]), + ...adhocFilters, + ], + }; updatedFormData.slice_id = 0; delete updatedFormData.slice_name; delete updatedFormData.dashboards; @@ -231,17 +228,14 @@ export default function DrillByModal({ ]); useEffect(() => { - setUsedGroupbyColumns(usedCols => { - const currentColumns = ensureIsArray(currentColumn); - return !currentColumn || - usedCols.some(usedCol => - currentColumns.some( - currentCol => currentCol?.column_name === usedCol.column_name, - ), - ) + setUsedGroupbyColumns(usedCols => + !currentColumn || + usedCols.some( + usedCol => usedCol.column_name === currentColumn.column_name, + ) ? usedCols - : [...usedCols, ...currentColumns]; - }); + : [...usedCols, currentColumn], + ); }, [currentColumn]); const onSelection = useCallback( From 6910d2786fc5b39b8927f60a286e12b11bc76b08 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Thu, 13 Apr 2023 15:17:21 +0200 Subject: [PATCH 4/5] Add ut --- .../Chart/DrillBy/DrillByModal.test.tsx | 52 ++++++++++++-- .../components/Chart/DrillBy/DrillByModal.tsx | 3 +- .../DrillBy/useDrillByBreadcrumbs.test.ts | 72 +++++++++++++++++++ .../Chart/DrillBy/useDrillByBreadcrumbs.tsx | 20 +++--- 4 files changed, 134 insertions(+), 13 deletions(-) create mode 100644 superset-frontend/src/components/Chart/DrillBy/useDrillByBreadcrumbs.test.ts diff --git a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx index f08a9701a85a2..f0768253e0adf 100644 --- 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 = {}) => { const DrillByModalWrapper = () => { const [showModal, setShowModal] = useState(false); @@ -76,6 +82,7 @@ const renderModal = async () => { formData={formData} onHideModal={() => setShowModal(false)} dataset={dataset} + {...modalProps} /> )} @@ -127,7 +134,10 @@ 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: { @@ -135,6 +145,18 @@ test('should generate Explore url', async () => { 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(); +}); diff --git a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx index 03f101b2bd6c0..55ba43e3d8894 100644 --- a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx +++ b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx @@ -106,7 +106,7 @@ const ModalFooter = ({ formData, closeModal }: ModalFooterProps) => { ); }; -interface DrillByModalProps { +export interface DrillByModalProps { column?: Column; dataset: Dataset; filters?: BinaryQueryObjectFilterClause[]; @@ -127,6 +127,7 @@ export default function DrillByModal({ }: DrillByModalProps) { const theme = useTheme(); + console.log('DUPA', formData); const initialGroupbyColumns = useMemo( () => ensureIsArray(formData[groupbyFieldName]) diff --git a/superset-frontend/src/components/Chart/DrillBy/useDrillByBreadcrumbs.test.ts b/superset-frontend/src/components/Chart/DrillBy/useDrillByBreadcrumbs.test.ts new file mode 100644 index 0000000000000..48cc328f7a6df --- /dev/null +++ b/superset-frontend/src/components/Chart/DrillBy/useDrillByBreadcrumbs.test.ts @@ -0,0 +1,72 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { renderHook } from '@testing-library/react-hooks'; +import userEvent from '@testing-library/user-event'; +import { render, screen } from 'spec/helpers/testing-library'; +import { + DrillByBreadcrumb, + useDrillByBreadcrumbs, +} from './useDrillByBreadcrumbs'; + +const BREADCRUMBS_DATA: DrillByBreadcrumb[] = [ + { + groupby: [{ column_name: 'col1' }, { column_name: 'col2' }], + filters: [ + { col: 'col1', op: '==', val: 'col1 filter' }, + { col: 'col2', op: '==', val: 'col2 filter' }, + ], + }, + { + groupby: [{ column_name: 'col3', verbose_name: 'Column 3' }], + filters: [{ col: 'col3', op: '==', val: 'col3 filter' }], + }, + { groupby: [{ column_name: 'col4' }] }, +]; + +test('Render breadcrumbs', () => { + const { result } = renderHook(() => useDrillByBreadcrumbs(BREADCRUMBS_DATA)); + render(result.current); + expect(screen.getAllByTestId('drill-by-breadcrumb-item')).toHaveLength(3); + expect( + screen.getByText('col1, col2 (col1 filter, col2 filter)'), + ).toBeInTheDocument(); + expect(screen.getByText('Column 3 (col3 filter)')).toBeInTheDocument(); + expect(screen.getByText('col4')).toBeInTheDocument(); +}); + +test('Call click handler with correct arguments when breadcrumb is clicked', () => { + const onClick = jest.fn(); + const { result } = renderHook(() => + useDrillByBreadcrumbs(BREADCRUMBS_DATA, onClick), + ); + render(result.current); + + userEvent.click(screen.getByText('col1, col2 (col1 filter, col2 filter)')); + expect(onClick).toHaveBeenCalledWith(BREADCRUMBS_DATA[0], 0); + onClick.mockClear(); + + userEvent.click(screen.getByText('Column 3 (col3 filter)')); + expect(onClick).toHaveBeenCalledWith(BREADCRUMBS_DATA[1], 1); + onClick.mockClear(); + + userEvent.click(screen.getByText('col4')); + expect(onClick).not.toHaveBeenCalled(); + onClick.mockClear(); +}); diff --git a/superset-frontend/src/components/Chart/DrillBy/useDrillByBreadcrumbs.tsx b/superset-frontend/src/components/Chart/DrillBy/useDrillByBreadcrumbs.tsx index 4559cbf0ba335..fc7c0b2bf5507 100644 --- a/superset-frontend/src/components/Chart/DrillBy/useDrillByBreadcrumbs.tsx +++ b/superset-frontend/src/components/Chart/DrillBy/useDrillByBreadcrumbs.tsx @@ -58,6 +58,16 @@ export const useDrillByBreadcrumbs = ( useMemo(() => { // the last breadcrumb is not clickable const isClickable = (index: number) => index < breadcrumbsData.length - 1; + const getBreadcrumbText = (breadcrumb: DrillByBreadcrumb) => + `${ensureIsArray(breadcrumb.groupby) + .map(column => column.verbose_name || column.column_name) + .join(', ')} ${ + breadcrumb.filters + ? `(${breadcrumb.filters + .map(filter => filter.formattedVal || filter.val) + .join(', ')})` + : '' + }`; return ( css` @@ -73,15 +83,9 @@ export const useDrillByBreadcrumbs = ( ? () => onBreadcrumbClick(breadcrumb, index) : noOp } + data-test="drill-by-breadcrumb-item" > - {ensureIsArray(breadcrumb.groupby) - .map(column => column.verbose_name || column.column_name) - .join(', ')}{' '} - {breadcrumb.filters - ? `(${breadcrumb.filters - .map(filter => filter.formattedVal || filter.val) - .join(', ')})` - : ''} + {getBreadcrumbText(breadcrumb)} ))} From 44c8073068e5c684303484968f9e661ea1db25d1 Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Thu, 13 Apr 2023 16:07:38 +0200 Subject: [PATCH 5/5] Remove console log --- superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx index 55ba43e3d8894..7f65576854873 100644 --- a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx +++ b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx @@ -127,7 +127,6 @@ export default function DrillByModal({ }: DrillByModalProps) { const theme = useTheme(); - console.log('DUPA', formData); const initialGroupbyColumns = useMemo( () => ensureIsArray(formData[groupbyFieldName])