You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by kg...@apache.org on 2023/04/05 09:20:55 UTC

[superset] branch master updated: feat: Drill by open in Explore (#23575)

This is an automated email from the ASF dual-hosted git repository.

kgabryje pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git


The following commit(s) were added to refs/heads/master by this push:
     new 117360cd57 feat: Drill by open in Explore (#23575)
117360cd57 is described below

commit 117360cd57bdbf9fd60fc479c6fe64dc077dbfee
Author: Kamil Gabryjelski <ka...@gmail.com>
AuthorDate: Wed Apr 5 11:20:45 2023 +0200

    feat: Drill by open in Explore (#23575)
---
 .../superset-ui-core/src/query/types/Filter.ts     |  12 ++-
 .../components/Chart/DrillBy/DrillByChart.test.tsx |  28 +-----
 .../src/components/Chart/DrillBy/DrillByChart.tsx  |  42 +-------
 .../components/Chart/DrillBy/DrillByModal.test.tsx |  69 ++++++++++---
 .../src/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<string, any> = {}) => {
-  const props = {
-    column: { column_name: 'state' },
-    formData: { ...chart.form_data, viz_type: 'pie' },
-    groupbyFieldName: 'groupby',
-    ...overrides,
-  };
-  return render(
-    <DrillByChart
-      filters={[
-        {
-          col: 'gender',
-          op: '==',
-          val: 'boy',
-          formattedVal: 'boy',
-        },
-      ]}
-      {...props}
-    />,
-    {
-      useRedux: true,
-    },
-  );
-};
+const setup = (overrides: Record<string, any> = {}) =>
+  render(<DrillByChart formData={{ ...chart.form_data, ...overrides }} />, {
+    useRedux: true,
+  });
 
 const waitForRender = (overrides: Record<string, any> = {}) =>
   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 (
-      <>
+      <DashboardPageIdContext.Provider value="1">
         <button type="button" onClick={() => setShowModal(true)}>
           Show modal
         </button>
@@ -71,23 +77,29 @@ const renderModal = async (state?: object) => {
           onHideModal={() => setShowModal(false)}
           dataset={dataset}
         />
-      </>
+      </DashboardPageIdContext.Provider>
     );
   };
   render(<DrillByModalWrapper />, {
     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) => (
-  <>
-    <Button buttonStyle="secondary" buttonSize="small" onClick={exploreChart}>
-      {t('Edit chart')}
-    </Button>
-    <Button
-      buttonStyle="primary"
-      buttonSize="small"
-      onClick={closeModal}
-      data-test="close-drill-by-modal"
-    >
-      {t('Close')}
-    </Button>
-  </>
-);
+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 (
+    <>
+      <Button buttonStyle="secondary" buttonSize="small" onClick={noOp}>
+        <Link
+          css={css`
+            &:hover {
+              text-decoration: none;
+            }
+          `}
+          to={url}
+        >
+          {t('Edit chart')}
+        </Link>
+      </Button>
+      <Button
+        buttonStyle="primary"
+        buttonSize="small"
+        onClick={closeModal}
+        data-test="close-drill-by-modal"
+      >
+        {t('Close')}
+      </Button>
+    </>
+  );
+};
 
 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={<ModalFooter exploreChart={exploreChart} />}
+      footer={<ModalFooter formData={updatedFormData} />}
       responsive
       resizable
       resizableConfig={{
@@ -118,12 +175,7 @@ export default function DrillByModal({
         `}
       >
         {metadataBar}
-        <DrillByChart
-          column={column}
-          filters={filters}
-          formData={formData}
-          groupbyFieldName={groupbyFieldName}
-        />
+        <DrillByChart formData={updatedFormData} />
       </div>
     </Modal>
   );
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,
         ),
       }),
       {},