From 117360cd57bdbf9fd60fc479c6fe64dc077dbfee Mon Sep 17 00:00:00 2001 From: Kamil Gabryjelski Date: Wed, 5 Apr 2023 11:20:45 +0200 Subject: [PATCH] feat: Drill by open in Explore (#23575) --- .../src/query/types/Filter.ts | 12 +- .../Chart/DrillBy/DrillByChart.test.tsx | 28 +---- .../components/Chart/DrillBy/DrillByChart.tsx | 42 +------ .../Chart/DrillBy/DrillByModal.test.tsx | 69 ++++++++--- .../components/Chart/DrillBy/DrillByModal.tsx | 108 +++++++++++++----- .../getFormDataWithDashboardContext.ts | 29 +++-- 6 files changed, 171 insertions(+), 117 deletions(-) diff --git a/superset-frontend/packages/superset-ui-core/src/query/types/Filter.ts b/superset-frontend/packages/superset-ui-core/src/query/types/Filter.ts index e113a843d4..bd7983f736 100644 --- a/superset-frontend/packages/superset-ui-core/src/query/types/Filter.ts +++ b/superset-frontend/packages/superset-ui-core/src/query/types/Filter.ts @@ -27,14 +27,17 @@ import { } from './Operator'; import { TimeGranularity } from '../../time-format'; -interface BaseSimpleAdhocFilter { - expressionType: 'SIMPLE'; +interface BaseAdhocFilter { clause: 'WHERE' | 'HAVING'; - subject: string; timeGrain?: TimeGranularity; isExtra?: boolean; } +interface BaseSimpleAdhocFilter extends BaseAdhocFilter { + expressionType: 'SIMPLE'; + subject: string; +} + export type UnaryAdhocFilter = BaseSimpleAdhocFilter & { operator: UnaryOperator; }; @@ -54,9 +57,8 @@ export type SimpleAdhocFilter = | BinaryAdhocFilter | SetAdhocFilter; -export interface FreeFormAdhocFilter { +export interface FreeFormAdhocFilter extends BaseAdhocFilter { expressionType: 'SQL'; - clause: 'WHERE' | 'HAVING'; sqlExpression: string; } diff --git a/superset-frontend/src/components/Chart/DrillBy/DrillByChart.test.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillByChart.test.tsx index d9d85566dc..55dbd3dee3 100644 --- a/superset-frontend/src/components/Chart/DrillBy/DrillByChart.test.tsx +++ b/superset-frontend/src/components/Chart/DrillBy/DrillByChart.test.tsx @@ -40,30 +40,10 @@ const fetchWithNoData = () => { }); }; -const setup = (overrides: Record = {}) => { - const props = { - column: { column_name: 'state' }, - formData: { ...chart.form_data, viz_type: 'pie' }, - groupbyFieldName: 'groupby', - ...overrides, - }; - return render( - , - { - useRedux: true, - }, - ); -}; +const setup = (overrides: Record = {}) => + render(, { + useRedux: true, + }); const waitForRender = (overrides: Record = {}) => waitFor(() => setup(overrides)); diff --git a/superset-frontend/src/components/Chart/DrillBy/DrillByChart.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillByChart.tsx index 58a19996b9..5d1a3dec23 100644 --- a/superset-frontend/src/components/Chart/DrillBy/DrillByChart.tsx +++ b/superset-frontend/src/components/Chart/DrillBy/DrillByChart.tsx @@ -17,52 +17,20 @@ * under the License. */ import React, { useEffect, useState } from 'react'; -import { - Behavior, - BinaryQueryObjectFilterClause, - Column, - css, - SuperChart, -} from '@superset-ui/core'; -import { simpleFilterToAdhoc } from 'src/utils/simpleFilterToAdhoc'; +import { BaseFormData, Behavior, css, SuperChart } from '@superset-ui/core'; import { getChartDataRequest } from 'src/components/Chart/chartAction'; import Loading from 'src/components/Loading'; interface DrillByChartProps { - column?: Column; - filters?: BinaryQueryObjectFilterClause[]; - formData: { [key: string]: any; viz_type: string }; - groupbyFieldName?: string; + formData: BaseFormData & { [key: string]: any }; } -export default function DrillByChart({ - column, - filters, - formData, - groupbyFieldName = 'groupby', -}: DrillByChartProps) { - let updatedFormData = formData; - let groupbyField: any = []; +export default function DrillByChart({ formData }: DrillByChartProps) { const [chartDataResult, setChartDataResult] = useState(); - if (column) { - groupbyField = Array.isArray(formData[groupbyFieldName]) - ? [column.column_name] - : column.column_name; - } - - if (filters) { - const adhocFilters = filters.map(filter => simpleFilterToAdhoc(filter)); - updatedFormData = { - ...formData, - adhoc_filters: [...formData.adhoc_filters, ...adhocFilters], - [groupbyFieldName]: groupbyField, - }; - } - useEffect(() => { getChartDataRequest({ - formData: updatedFormData, + formData, }).then(({ json }) => { setChartDataResult(json.result); }); @@ -81,7 +49,7 @@ export default function DrillByChart({ behaviors={[Behavior.INTERACTIVE_CHART]} chartType={formData.viz_type} enableNoResults - formData={updatedFormData} + formData={formData} queriesData={chartDataResult} height="100%" width="100%" diff --git a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx index dd3cddec71..54eb653028 100644 --- a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx +++ b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.test.tsx @@ -18,30 +18,35 @@ */ 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 { render, screen } from 'spec/helpers/testing-library'; import chartQueries, { sliceId } from 'spec/fixtures/mockChartQueries'; import mockState from 'spec/fixtures/mockState'; -import fetchMock from 'fetch-mock'; +import { DashboardPageIdContext } from 'src/dashboard/containers/DashboardPage'; import DrillByModal from './DrillByModal'; -const CHART_DATA_ENDPOINT = - 'glob:*api/v1/chart/data?form_data=%7B%22slice_id%22%3A18%7D'; - -fetchMock.post(CHART_DATA_ENDPOINT, { body: {} }, {}); +const CHART_DATA_ENDPOINT = 'glob:*/api/v1/chart/data*'; +const FORM_DATA_KEY_ENDPOINT = 'glob:*/api/v1/explore/form_data'; const { form_data: formData } = chartQueries[sliceId]; const { slice_name: chartName } = formData; const drillByModalState = { ...mockState, dashboardLayout: { - CHART_ID: { - id: 'CHART_ID', - meta: { - chartId: formData.slice_id, - sliceName: chartName, + past: [], + present: { + CHART_ID: { + id: 'CHART_ID', + meta: { + chartId: formData.slice_id, + sliceName: chartName, + }, }, }, + future: [], }, }; const dataset = { @@ -56,12 +61,13 @@ const dataset = { }, ], }; -const renderModal = async (state?: object) => { + +const renderModal = async () => { const DrillByModalWrapper = () => { const [showModal, setShowModal] = useState(false); return ( - <> + @@ -71,23 +77,29 @@ const renderModal = async (state?: object) => { onHideModal={() => setShowModal(false)} dataset={dataset} /> - + ); }; render(, { useDnd: true, useRedux: true, useRouter: true, - initialState: state, + initialState: drillByModalState, }); userEvent.click(screen.getByRole('button', { name: 'Show modal' })); await screen.findByRole('dialog', { name: `Drill by: ${chartName}` }); }; + +beforeEach(() => { + fetchMock + .post(CHART_DATA_ENDPOINT, { body: {} }, {}) + .post(FORM_DATA_KEY_ENDPOINT, { key: '123' }); +}); afterEach(fetchMock.restore); test('should render the title', async () => { - await renderModal(drillByModalState); + await renderModal(); expect(screen.getByText(`Drill by: ${chartName}`)).toBeInTheDocument(); }); @@ -105,3 +117,30 @@ test('should close the modal', async () => { userEvent.click(screen.getAllByRole('button', { name: 'Close' })[1]); expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); }); + +test('should generate Explore url', async () => { + await renderModal(); + await waitFor(() => fetchMock.called(FORM_DATA_KEY_ENDPOINT)); + const expectedRequestPayload = { + form_data: { + ...omitBy( + omit(formData, ['slice_id', 'slice_name', 'dashboards']), + isUndefined, + ), + slice_id: 0, + }, + datasource_id: Number(formData.datasource.split('__')[0]), + datasource_type: formData.datasource.split('__')[1], + }; + + const parsedRequestPayload = JSON.parse( + fetchMock.lastCall()?.[1]?.body as string, + ); + parsedRequestPayload.form_data = JSON.parse(parsedRequestPayload.form_data); + + expect(parsedRequestPayload).toEqual(expectedRequestPayload); + + expect( + await screen.findByRole('link', { name: 'Edit chart' }), + ).toHaveAttribute('href', '/explore/?form_data_key=123&dashboard_page_id=1'); +}); diff --git a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx index aab89f6acc..2c87ee0cab 100644 --- a/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx +++ b/superset-frontend/src/components/Chart/DrillBy/DrillByModal.tsx @@ -17,47 +17,79 @@ * under the License. */ -import React from 'react'; +import React, { useContext, useEffect, useMemo, useState } from 'react'; import { + BaseFormData, BinaryQueryObjectFilterClause, Column, css, + ensureIsArray, t, useTheme, } from '@superset-ui/core'; +import { useSelector } from 'react-redux'; +import { Link } from 'react-router-dom'; import Modal from 'src/components/Modal'; import Button from 'src/components/Button'; -import { useSelector } from 'react-redux'; import { DashboardLayout, RootState } from 'src/dashboard/types'; +import { DashboardPageIdContext } from 'src/dashboard/containers/DashboardPage'; +import { postFormData } from 'src/explore/exploreUtils/formData'; +import { noOp } from 'src/utils/common'; +import { simpleFilterToAdhoc } from 'src/utils/simpleFilterToAdhoc'; import { useDatasetMetadataBar } from 'src/features/datasets/metadataBar/useDatasetMetadataBar'; import { Dataset } from '../types'; import DrillByChart from './DrillByChart'; interface ModalFooterProps { - exploreChart: () => void; + formData: BaseFormData; closeModal?: () => void; } -const ModalFooter = ({ exploreChart, closeModal }: ModalFooterProps) => ( - <> - - - -); +const ModalFooter = ({ formData, closeModal }: ModalFooterProps) => { + const [url, setUrl] = useState(''); + const dashboardPageId = useContext(DashboardPageIdContext); + const [datasource_id, datasource_type] = formData.datasource.split('__'); + useEffect(() => { + postFormData(Number(datasource_id), datasource_type, formData, 0) + .then(key => { + setUrl( + `/explore/?form_data_key=${key}&dashboard_page_id=${dashboardPageId}`, + ); + }) + .catch(e => { + console.log(e); + }); + }, [dashboardPageId, datasource_id, datasource_type, formData]); + return ( + <> + + + + ); +}; interface DrillByModalProps { column?: Column; filters?: BinaryQueryObjectFilterClause[]; - formData: { [key: string]: any; viz_type: string }; + formData: BaseFormData & { [key: string]: any }; groupbyFieldName?: string; onHideModal: () => void; showModal: boolean; @@ -68,7 +100,7 @@ export default function DrillByModal({ column, filters, formData, - groupbyFieldName, + groupbyFieldName = 'groupby', onHideModal, showModal, dataset, @@ -82,7 +114,32 @@ export default function DrillByModal({ ); const chartName = chartLayoutItem?.meta.sliceNameOverride || chartLayoutItem?.meta.sliceName; - const exploreChart = () => {}; + + const updatedFormData = useMemo(() => { + let updatedFormData = { ...formData }; + if (column) { + updatedFormData[groupbyFieldName] = Array.isArray( + formData[groupbyFieldName], + ) + ? [column.column_name] + : column.column_name; + } + + if (filters) { + const adhocFilters = filters.map(filter => simpleFilterToAdhoc(filter)); + updatedFormData = { + ...updatedFormData, + adhoc_filters: [ + ...ensureIsArray(formData.adhoc_filters), + ...adhocFilters, + ], + }; + } + updatedFormData.slice_id = 0; + delete updatedFormData.slice_name; + delete updatedFormData.dashboards; + return updatedFormData; + }, [column, filters, formData, groupbyFieldName]); const { metadataBar } = useDatasetMetadataBar({ dataset }); return ( @@ -95,7 +152,7 @@ export default function DrillByModal({ show={showModal} onHide={onHideModal ?? (() => null)} title={t('Drill by: %s', chartName)} - footer={} + footer={} responsive resizable resizableConfig={{ @@ -118,12 +175,7 @@ export default function DrillByModal({ `} > {metadataBar} - + ); diff --git a/superset-frontend/src/explore/controlUtils/getFormDataWithDashboardContext.ts b/superset-frontend/src/explore/controlUtils/getFormDataWithDashboardContext.ts index 5ea6788485..1f12d3f6f7 100644 --- a/superset-frontend/src/explore/controlUtils/getFormDataWithDashboardContext.ts +++ b/superset-frontend/src/explore/controlUtils/getFormDataWithDashboardContext.ts @@ -33,6 +33,17 @@ import { } from '@superset-ui/core'; import { simpleFilterToAdhoc } from 'src/utils/simpleFilterToAdhoc'; +const removeExtraFieldForNewCharts = ( + filters: AdhocFilter[], + isNewChart: boolean, +) => + filters.map(filter => { + if (filter.isExtra) { + return { ...filter, isExtra: !isNewChart }; + } + return filter; + }); + const removeAdhocFilterDuplicates = (filters: AdhocFilter[]) => { const isDuplicate = ( adhocFilter: AdhocFilter, @@ -103,7 +114,6 @@ const mergeNativeFiltersToFormData = ( ) => { const nativeFiltersData: JsonObject = {}; const extraFormData = dashboardFormData.extra_form_data || {}; - Object.entries(EXTRA_FORM_DATA_OVERRIDE_REGULAR_MAPPINGS).forEach( ([srcKey, targetKey]) => { const val = extraFormData[srcKey]; @@ -193,13 +203,16 @@ export const getFormDataWithDashboardContext = ( .reduce( (acc, key) => ({ ...acc, - [key]: applyTimeRangeFilters( - dashboardContextFormData, - removeAdhocFilterDuplicates([ - ...ensureIsArray(exploreFormData[key]), - ...ensureIsArray(filterBoxData[key]), - ...ensureIsArray(nativeFiltersData[key]), - ]), + [key]: removeExtraFieldForNewCharts( + applyTimeRangeFilters( + dashboardContextFormData, + removeAdhocFilterDuplicates([ + ...ensureIsArray(exploreFormData[key]), + ...ensureIsArray(filterBoxData[key]), + ...ensureIsArray(nativeFiltersData[key]), + ]), + ), + exploreFormData.slice_id === 0, ), }), {},