Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Implement breadcrumbs in Drill By modal #23664

Merged
merged 5 commits into from
Apr 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
30 changes: 20 additions & 10 deletions superset-frontend/src/components/Chart/DrillBy/DrillByMenuItems.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -68,6 +69,7 @@ export interface DrillByMenuItemsProps {
export const DrillByMenuItems = ({
filters,
groupbyFieldName,
adhocFilterFieldName,
formData,
contextMenuY = 0,
submenuIndex = 0,
Expand Down Expand Up @@ -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,
),
),
);
Expand All @@ -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();
Expand All @@ -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(
Expand Down Expand Up @@ -260,6 +269,7 @@ export const DrillByMenuItems = ({
filters={filters}
formData={formData}
groupbyFieldName={groupbyFieldName}
adhocFilterFieldName={adhocFilterFieldName}
onHideModal={closeModal}
dataset={dataset!}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);

Expand All @@ -76,6 +82,7 @@ const renderModal = async () => {
formData={formData}
onHideModal={() => setShowModal(false)}
dataset={dataset}
{...modalProps}
/>
)}
</DashboardPageIdContext.Provider>
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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();
});
Loading