fix: Explore long URL problem (#18181)

* fix: Explore long URL problem

* Fixes lint problems

* Fixes default value

* Removes duplicated test

* Fixes share menu items

* Fixes tests

* Debounces form_data updates

* Rewrites debounce function

* Moves history update outside the functional component

* Mocks lodash function in tests

* Fixes Cypress test

* Fixes Cypress test #2

superset-frontend/cypress-base/cypress/integration/explore/advanced_analytics.test.ts

@@ -21,6 +21,8 @@ describe('Advanced analytics', () => {
     cy.login();
     cy.intercept('POST', '/superset/explore_json/**').as('postJson');
     cy.intercept('GET', '/superset/explore_json/**').as('getJson');
+    cy.intercept('PUT', '/api/v1/explore/**').as('putExplore');
+    cy.intercept('GET', '/superset/explore/**').as('getExplore');
   });
 
   it('Create custom time compare', () => {
@@ -40,12 +42,13 @@ describe('Advanced analytics', () => {
 
     cy.get('button[data-test="run-query-button"]').click();
     cy.wait('@postJson');
+    cy.wait('@putExplore');
     cy.reload();
     cy.verifySliceSuccess({
       waitAlias: '@postJson',
       chartSelector: 'svg',
     });
-
+    cy.wait('@getExplore');
     cy.get('.ant-collapse-header').contains('Advanced Analytics').click();
     cy.get('[data-test=time_compare]')
       .find('.ant-select-selector')

superset-frontend/cypress-base/cypress/integration/explore/link.test.ts

@@ -48,18 +48,6 @@ describe('Test explore links', () => {
     });
   });
 
-  it('Test if short link is generated', () => {
-    cy.intercept('POST', 'r/shortner/').as('getShortUrl');
-
-    cy.visitChartByName('Growth Rate');
-    cy.verifySliceSuccess({ waitAlias: '@chartData' });
-
-    cy.get('[data-test=short-link-button]').click();
-
-    // explicitly wait for the url response
-    cy.wait('@getShortUrl');
-  });
-
   it('Test iframe link', () => {
     cy.visitChartByName('Growth Rate');
     cy.verifySliceSuccess({ waitAlias: '@chartData' });

superset-frontend/spec/helpers/ResizeObserver.ts

@@ -0,0 +1,33 @@
+/**
+ * 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.
+ */
+class ResizeObserver {
+  disconnect() {
+    return null;
+  }
+
+  observe() {
+    return null;
+  }
+
+  unobserve() {
+    return null;
+  }
+}
+
+export { ResizeObserver };

superset-frontend/spec/helpers/shim.ts

@@ -28,6 +28,7 @@ import Adapter from 'enzyme-adapter-react-16';
 import { configure as configureTranslation } from '../../packages/superset-ui-core/src/translation';
 import { Worker } from './Worker';
 import { IntersectionObserver } from './IntersectionObserver';
+import { ResizeObserver } from './ResizeObserver';
 import setupSupersetClient from './setupSupersetClient';
 import CacheStorage from './CacheStorage';
 
@@ -51,6 +52,7 @@ g.window.location = { href: 'about:blank' };
 g.window.performance = { now: () => new Date().getTime() };
 g.window.Worker = Worker;
 g.window.IntersectionObserver = IntersectionObserver;
+g.window.ResizeObserver = ResizeObserver;
 g.URL.createObjectURL = () => '';
 g.caches = new CacheStorage();
 

superset-frontend/src/constants.ts

@@ -55,6 +55,10 @@ export const URL_PARAMS = {
     name: 'show_filters',
     type: 'boolean',
   },
+  formDataKey: {
+    name: 'form_data_key',
+    type: 'string',
+  },
 } as const;
 
 /**

superset-frontend/src/dashboard/components/SliceHeader/SliceHeader.test.tsx

@@ -157,7 +157,8 @@ const createProps = () => ({
   forceRefresh: jest.fn(),
   logExploreChart: jest.fn(),
   exportCSV: jest.fn(),
-  formData: {},
+  onExploreChart: jest.fn(),
+  formData: { slice_id: 1, datasource: '58__table' },
 });
 
 test('Should render', () => {

superset-frontend/src/dashboard/components/SliceHeader/index.tsx

@@ -58,7 +58,7 @@ const SliceHeader: FC<SliceHeaderProps> = ({
   updateSliceName = () => ({}),
   toggleExpandSlice = () => ({}),
   logExploreChart = () => ({}),
-  exploreUrl = '#',
+  onExploreChart,
   exportCSV = () => ({}),
   editMode = false,
   annotationQuery = {},
@@ -171,7 +171,7 @@ const SliceHeader: FC<SliceHeaderProps> = ({
                 toggleExpandSlice={toggleExpandSlice}
                 forceRefresh={forceRefresh}
                 logExploreChart={logExploreChart}
-                exploreUrl={exploreUrl}
+                onExploreChart={onExploreChart}
                 exportCSV={exportCSV}
                 exportFullCSV={exportFullCSV}
                 supersetCanExplore={supersetCanExplore}

superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx

@@ -45,6 +45,7 @@ const createProps = (viz_type = 'sunburst') => ({
   forceRefresh: jest.fn(),
   handleToggleFullSize: jest.fn(),
   toggleExpandSlice: jest.fn(),
+  onExploreChart: jest.fn(),
   slice: {
     slice_id: 371,
     slice_url: '/superset/explore/?form_data=%7B%22slice_id%22%3A%20371%7D',
@@ -90,7 +91,7 @@ const createProps = (viz_type = 'sunburst') => ({
   chartStatus: 'rendered',
   showControls: true,
   supersetCanShare: true,
-  formData: {},
+  formData: { slice_id: 1, datasource: '58__table' },
 });
 
 test('Should render', () => {

superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx

@@ -27,8 +27,6 @@ import {
 import { Menu, NoAnimationDropdown } from 'src/common/components';
 import ShareMenuItems from 'src/dashboard/components/menu/ShareMenuItems';
 import downloadAsImage from 'src/utils/downloadAsImage';
-import getDashboardUrl from 'src/dashboard/util/getDashboardUrl';
-import { getActiveFilters } from 'src/dashboard/util/activeDashboardFilters';
 import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags';
 import CrossFilterScopingModal from 'src/dashboard/components/CrossFilterScopingModal/CrossFilterScopingModal';
 import Icons from 'src/components/Icons';
@@ -99,8 +97,8 @@ export interface SliceHeaderControlsProps {
   isExpanded?: boolean;
   updatedDttm: number | null;
   isFullSize?: boolean;
-  formData: object;
+  formData: { slice_id: number; datasource: string };
-  exploreUrl?: string;
+  onExploreChart: () => void;
 
   forceRefresh: (sliceId: number, dashboardId: number) => void;
   logExploreChart?: (sliceId: number) => void;
@@ -215,13 +213,13 @@ class SliceHeaderControls extends React.PureComponent<
     const {
       slice,
       isFullSize,
-      componentId,
       cachedDttm = [],
       updatedDttm = null,
       addSuccessToast = () => {},
       addDangerToast = () => {},
       supersetCanShare = false,
       isCached = [],
+      formData,
     } = this.props;
     const crossFilterItems = getChartMetadataRegistry().items;
     const isTable = slice.viz_type === 'table';
@@ -283,10 +281,11 @@ class SliceHeaderControls extends React.PureComponent<
         )}
 
         {this.props.supersetCanExplore && (
-          <Menu.Item key={MENU_KEYS.EXPLORE_CHART}>
+          <Menu.Item
-            <a href={this.props.exploreUrl} rel="noopener noreferrer">
+            key={MENU_KEYS.EXPLORE_CHART}
-              {t('View chart in Explore')}
+            onClick={this.props.onExploreChart}
-            </a>
+          >
+            {t('View chart in Explore')}
           </Menu.Item>
         )}
 
@@ -309,17 +308,13 @@ class SliceHeaderControls extends React.PureComponent<
 
         {supersetCanShare && (
           <ShareMenuItems
-            url={getDashboardUrl({
-              pathname: window.location.pathname,
-              filters: getActiveFilters(),
-              hash: componentId,
-            })}
             copyMenuItemTitle={t('Copy chart URL')}
             emailMenuItemTitle={t('Share chart by email')}
             emailSubject={t('Superset chart')}
             emailBody={t('Check out this chart: ')}
             addSuccessToast={addSuccessToast}
             addDangerToast={addDangerToast}
+            formData={formData}
           />
         )}
 

superset-frontend/src/dashboard/components/gridComponents/Chart.jsx

@@ -19,13 +19,10 @@
 import cx from 'classnames';
 import React from 'react';
 import PropTypes from 'prop-types';
-import { styled } from '@superset-ui/core';
+import { styled, t, logging } from '@superset-ui/core';
 import { isEqual } from 'lodash';
 
-import {
+import { exportChart, mountExploreUrl } from 'src/explore/exploreUtils';
-  exportChart,
-  getExploreUrlFromDashboard,
-} from 'src/explore/exploreUtils';
 import ChartContainer from 'src/chart/ChartContainer';
 import {
   LOG_ACTIONS_CHANGE_DASHBOARD_FILTER,
@@ -35,6 +32,8 @@ import {
 } from 'src/logger/LogUtils';
 import { areObjectsEqual } from 'src/reduxUtils';
 import { FILTER_BOX_MIGRATION_STATES } from 'src/explore/constants';
+import { postFormData } from 'src/explore/exploreUtils/formData';
+import { URL_PARAMS } from 'src/constants';
 
 import SliceHeader from '../SliceHeader';
 import MissingChart from '../MissingChart';
@@ -241,7 +240,20 @@ export default class Chart extends React.Component {
     });
   };
 
-  getChartUrl = () => getExploreUrlFromDashboard(this.props.formData);
+  onExploreChart = async () => {
+    try {
+      const key = await postFormData(
+        this.props.datasource.id,
+        this.props.formData,
+        this.props.slice.id,
+      );
+      const url = mountExploreUrl(null, { [URL_PARAMS.formDataKey.name]: key });
+      window.open(url, '_blank', 'noreferrer');
+    } catch (error) {
+      logging.error(error);
+      this.props.addDangerToast(t('An error occurred while opening Explore'));
+    }
+  };
 
   exportCSV(isFullCSV = false) {
     this.props.logEvent(LOG_ACTIONS_EXPORT_CSV_DASHBOARD_CHART, {
@@ -350,7 +362,7 @@ export default class Chart extends React.Component {
           editMode={editMode}
           annotationQuery={chart.annotationQuery}
           logExploreChart={this.logExploreChart}
-          exploreUrl={this.getChartUrl()}
+          onExploreChart={this.onExploreChart}
           exportCSV={this.exportCSV}
           exportFullCSV={this.exportFullCSV}
           updateSliceName={updateSliceName}

superset-frontend/src/dashboard/components/menu/ShareMenuItems/ShareMenuItems.test.tsx

@@ -134,7 +134,7 @@ test('Click on "Copy dashboard URL" and fail', async () => {
     expect(props.addSuccessToast).toBeCalledTimes(0);
     expect(props.addDangerToast).toBeCalledTimes(1);
     expect(props.addDangerToast).toBeCalledWith(
-      'Sorry, your browser does not support copying.',
+      'Sorry, something went wrong. Try again later.',
     );
   });
 });

superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx

@@ -19,17 +19,19 @@
 import React from 'react';
 import { useUrlShortener } from 'src/hooks/useUrlShortener';
 import copyTextToClipboard from 'src/utils/copy';
-import { t } from '@superset-ui/core';
+import { t, logging } from '@superset-ui/core';
 import { Menu } from 'src/common/components';
 import { getUrlParam } from 'src/utils/urlUtils';
+import { postFormData } from 'src/explore/exploreUtils/formData';
 import { URL_PARAMS } from 'src/constants';
+import { mountExploreUrl } from 'src/explore/exploreUtils';
 import {
   createFilterKey,
   getFilterValue,
 } from 'src/dashboard/components/nativeFilters/FilterBar/keyValue';
 
 interface ShareMenuItemProps {
-  url: string;
+  url?: string;
   copyMenuItemTitle: string;
   emailMenuItemTitle: string;
   emailSubject: string;
@@ -37,6 +39,7 @@ interface ShareMenuItemProps {
   addDangerToast: Function;
   addSuccessToast: Function;
   dashboardId?: string;
+  formData?: { slice_id: number; datasource: string };
 }
 
 const ShareMenuItems = (props: ShareMenuItemProps) => {
@@ -49,10 +52,11 @@ const ShareMenuItems = (props: ShareMenuItemProps) => {
     addDangerToast,
     addSuccessToast,
     dashboardId,
+    formData,
     ...rest
   } = props;
 
-  const getShortUrl = useUrlShortener(url);
+  const getShortUrl = useUrlShortener(url || '');
 
   async function getCopyUrl() {
     const risonObj = getUrlParam(URL_PARAMS.nativeFilters);
@@ -70,24 +74,37 @@ const ShareMenuItems = (props: ShareMenuItemProps) => {
     return `${newUrl.pathname}${newUrl.search}`;
   }
 
+  async function generateUrl() {
+    if (formData) {
+      const key = await postFormData(
+        parseInt(formData.datasource.split('_')[0], 10),
+        formData,
+        formData.slice_id,
+      );
+      return `${window.location.origin}${mountExploreUrl(null, {
+        [URL_PARAMS.formDataKey.name]: key,
+      })}`;
+    }
+    const copyUrl = await getCopyUrl();
+    return getShortUrl(copyUrl);
+  }
+
   async function onCopyLink() {
     try {
-      const copyUrl = await getCopyUrl();
+      await copyTextToClipboard(await generateUrl());
-      const shortUrl = await getShortUrl(copyUrl);
-      await copyTextToClipboard(shortUrl);
       addSuccessToast(t('Copied to clipboard!'));
     } catch (error) {
-      addDangerToast(t('Sorry, your browser does not support copying.'));
+      logging.error(error);
+      addDangerToast(t('Sorry, something went wrong. Try again later.'));
     }
   }
 
   async function onShareByEmail() {
     try {
-      const copyUrl = await getCopyUrl();
+      const bodyWithLink = `${emailBody}${await generateUrl()}`;
-      const shortUrl = await getShortUrl(copyUrl);
-      const bodyWithLink = `${emailBody}${shortUrl}`;
       window.location.href = `mailto:?Subject=${emailSubject}%20&Body=${bodyWithLink}`;
     } catch (error) {
+      logging.error(error);
       addDangerToast(t('Sorry, something went wrong. Try again later.'));
     }
   }

superset-frontend/src/explore/components/EmbedCodeButton.jsx

@@ -17,7 +17,6 @@
  * under the License.
  */
 import React from 'react';
-import PropTypes from 'prop-types';
 import { t } from '@superset-ui/core';
 
 import Popover from 'src/components/Popover';
@@ -25,13 +24,7 @@ import { FormLabel } from 'src/components/Form';
 import Icons from 'src/components/Icons';
 import { Tooltip } from 'src/components/Tooltip';
 import CopyToClipboard from 'src/components/CopyToClipboard';
-import { getShortUrl } from 'src/utils/urlUtils';
 import { URL_PARAMS } from 'src/constants';
-import { getExploreLongUrl, getURIDirectory } from '../exploreUtils';
-
-const propTypes = {
-  latestQueryFormData: PropTypes.object.isRequired,
-};
 
 export default class EmbedCodeButton extends React.Component {
   constructor(props) {
@@ -39,24 +32,8 @@ export default class EmbedCodeButton extends React.Component {
     this.state = {
       height: '400',
       width: '600',
-      shortUrlId: 0,
     };
     this.handleInputChange = this.handleInputChange.bind(this);
-    this.getCopyUrl = this.getCopyUrl.bind(this);
-    this.onShortUrlSuccess = this.onShortUrlSuccess.bind(this);
-  }
-
-  onShortUrlSuccess(shortUrl) {
-    const shortUrlId = shortUrl.substring(shortUrl.indexOf('/r/') + 3);
-    this.setState(() => ({
-      shortUrlId,
-    }));
-  }
-
-  getCopyUrl() {
-    return getShortUrl(getExploreLongUrl(this.props.latestQueryFormData))
-      .then(this.onShortUrlSuccess)
-      .catch(this.props.addDangerToast);
   }
 
   handleInputChange(e) {
@@ -67,9 +44,7 @@ export default class EmbedCodeButton extends React.Component {
   }
 
   generateEmbedHTML() {
-    const srcLink = `${window.location.origin + getURIDirectory()}?r=${
+    const srcLink = `${window.location.href}&${URL_PARAMS.standalone.name}=1&height=${this.state.height}`;
-      this.state.shortUrlId
-    }&${URL_PARAMS.standalone.name}=1&height=${this.state.height}`;
     return (
       '<iframe\n' +
       `  width="${this.state.width}"\n` +
@@ -150,7 +125,6 @@ export default class EmbedCodeButton extends React.Component {
       <Popover
         trigger="click"
         placement="left"
-        onClick={this.getCopyUrl}
         content={this.renderPopoverContent()}
       >
         <Tooltip
@@ -171,5 +145,3 @@ export default class EmbedCodeButton extends React.Component {
     );
   }
 }
-
-EmbedCodeButton.propTypes = propTypes;

superset-frontend/src/explore/components/EmbedCodeButton.test.jsx

@@ -17,84 +17,29 @@
  * under the License.
  */
 import React from 'react';
-import { shallow, mount } from 'enzyme';
+import { shallow } from 'enzyme';
-import { supersetTheme, ThemeProvider } from '@superset-ui/core';
+import { styledMount as mount } from 'spec/helpers/theming';
 import Popover from 'src/components/Popover';
-import sinon from 'sinon';
-import { Provider } from 'react-redux';
-import configureStore from 'redux-mock-store';
-import fetchMock from 'fetch-mock';
 import EmbedCodeButton from 'src/explore/components/EmbedCodeButton';
-import * as exploreUtils from 'src/explore/exploreUtils';
-import * as urlUtils from 'src/utils/urlUtils';
 import { DashboardStandaloneMode } from 'src/dashboard/util/constants';
 
-const ENDPOINT = 'glob:*/r/shortner/';
-
-fetchMock.post(ENDPOINT, {});
-
 describe('EmbedCodeButton', () => {
-  const mockStore = configureStore([]);
-  const store = mockStore({});
-
-  const defaultProps = {
-    latestQueryFormData: { datasource: '107__table' },
-  };
-
   it('renders', () => {
-    expect(React.isValidElement(<EmbedCodeButton {...defaultProps} />)).toBe(
+    expect(React.isValidElement(<EmbedCodeButton />)).toBe(true);
-      true,
-    );
   });
 
   it('renders overlay trigger', () => {
-    const wrapper = shallow(<EmbedCodeButton {...defaultProps} />);
+    const wrapper = shallow(<EmbedCodeButton />);
     expect(wrapper.find(Popover)).toExist();
   });
 
-  it('should create a short, standalone, explore url', () => {
-    const spy1 = sinon.spy(exploreUtils, 'getExploreLongUrl');
-    const spy2 = sinon.spy(urlUtils, 'getShortUrl');
-
-    const wrapper = mount(
-      <ThemeProvider theme={supersetTheme}>
-        <EmbedCodeButton {...defaultProps} />
-      </ThemeProvider>,
-      {
-        wrappingComponent: Provider,
-        wrappingComponentProps: {
-          store,
-        },
-      },
-    ).find(EmbedCodeButton);
-    wrapper.setState({
-      height: '1000',
-      width: '2000',
-      shortUrlId: 100,
-    });
-
-    const trigger = wrapper.find(Popover);
-    trigger.simulate('click');
-    expect(spy1.callCount).toBe(1);
-    expect(spy2.callCount).toBe(1);
-
-    spy1.restore();
-    spy2.restore();
-  });
-
   it('returns correct embed code', () => {
-    const stub = sinon
+    const href = 'http://localhost/explore?form_data_key=xxxxxxxxx';
-      .stub(exploreUtils, 'getURIDirectory')
+    Object.defineProperty(window, 'location', { value: { href } });
-      .callsFake(() => 'endpoint_url');
+    const wrapper = mount(<EmbedCodeButton />);
-    const wrapper = mount(
-      <ThemeProvider theme={supersetTheme}>
-        <EmbedCodeButton {...defaultProps} />
-      </ThemeProvider>,
-    );
     wrapper.find(EmbedCodeButton).setState({
       height: '1000',
       width: '2000',
-      shortUrlId: 100,
     });
     const embedHTML =
       `${
@@ -104,13 +49,12 @@ describe('EmbedCodeButton', () => {
         '  seamless\n' +
         '  frameBorder="0"\n' +
         '  scrolling="no"\n' +
-        '  src="http://localhostendpoint_url?r=100&standalone='
+        `  src="${href}&standalone=`
       }${DashboardStandaloneMode.HIDE_NAV}&height=1000"\n` +
       `>\n` +
       `</iframe>`;
     expect(wrapper.find(EmbedCodeButton).instance().generateEmbedHTML()).toBe(
       embedHTML,
     );
-    stub.restore();
   });
 });

superset-frontend/src/explore/components/ExploreActionButtons.tsx

@@ -23,9 +23,8 @@ import Icons from 'src/components/Icons';
 import { Tooltip } from 'src/components/Tooltip';
 import copyTextToClipboard from 'src/utils/copy';
 import withToasts from 'src/components/MessageToasts/withToasts';
-import { useUrlShortener } from 'src/hooks/useUrlShortener';
 import EmbedCodeButton from './EmbedCodeButton';
-import { exportChart, getExploreLongUrl } from '../exploreUtils';
+import { exportChart } from '../exploreUtils';
 import ExploreAdditionalActionsMenu from './ExploreAdditionalActionsMenu';
 import { ExportToCSVDropdown } from './ExportToCSVDropdown';
 
@@ -104,14 +103,12 @@ const ExploreActionButtons = (props: ExploreActionButtonsProps) => {
 
   const copyTooltipText = t('Copy chart URL to clipboard');
   const [copyTooltip, setCopyTooltip] = useState(copyTooltipText);
-  const longUrl = getExploreLongUrl(latestQueryFormData);
-  const getShortUrl = useUrlShortener(longUrl);
 
   const doCopyLink = async () => {
     try {
       setCopyTooltip(t('Loading...'));
-      const shortUrl = await getShortUrl();
+      const url = window.location.href;
-      await copyTextToClipboard(shortUrl);
+      await copyTextToClipboard(url);
       setCopyTooltip(t('Copied to clipboard!'));
       addSuccessToast(t('Copied to clipboard!'));
     } catch (error) {
@@ -123,8 +120,8 @@ const ExploreActionButtons = (props: ExploreActionButtonsProps) => {
   const doShareEmail = async () => {
     try {
       const subject = t('Superset Chart');
-      const shortUrl = await getShortUrl();
+      const url = window.location.href;
-      const body = t('%s%s', 'Check out this chart: ', shortUrl);
+      const body = encodeURIComponent(t('%s%s', 'Check out this chart: ', url));
       window.location.href = `mailto:?Subject=${subject}%20&Body=${body}`;
     } catch (error) {
       addDangerToast(t('Sorry, something went wrong. Try again later.'));
@@ -179,7 +176,7 @@ const ExploreActionButtons = (props: ExploreActionButtonsProps) => {
             tooltip={t('Share chart by email')}
             onClick={doShareEmail}
           />
-          <EmbedCodeButton latestQueryFormData={latestQueryFormData} />
+          <EmbedCodeButton />
           <ActionButton
             prefixIcon={<Icons.FileTextOutlined iconSize="m" />}
             text=".JSON"

superset-frontend/src/explore/components/ExploreChartHeader/index.jsx

@@ -56,7 +56,6 @@ const CHART_STATUS_MAP = {
 
 const propTypes = {
   actions: PropTypes.object.isRequired,
-  addHistory: PropTypes.func,
   can_overwrite: PropTypes.bool.isRequired,
   can_download: PropTypes.bool.isRequired,
   dashboardId: PropTypes.number,

superset-frontend/src/explore/components/ExploreChartPanel.jsx

@@ -34,7 +34,6 @@ import { buildV1ChartDataPayload } from '../exploreUtils';
 
 const propTypes = {
   actions: PropTypes.object.isRequired,
-  addHistory: PropTypes.func,
   onQuery: PropTypes.func,
   can_overwrite: PropTypes.bool.isRequired,
   can_download: PropTypes.bool.isRequired,
@@ -288,7 +287,6 @@ const ExploreChartPanel = props => {
     <ConnectedExploreChartHeader
       ownState={props.ownState}
       actions={props.actions}
-      addHistory={props.addHistory}
       can_overwrite={props.can_overwrite}
       can_download={props.can_download}
       dashboardId={props.dashboardId}

superset-frontend/src/explore/components/ExploreViewContainer/ExploreViewContainer.test.tsx

@@ -0,0 +1,112 @@
+/**
+ * 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 from 'react';
+import fetchMock from 'fetch-mock';
+import { MemoryRouter, Route } from 'react-router-dom';
+import { render, screen, waitFor } from 'spec/helpers/testing-library';
+import userEvent from '@testing-library/user-event';
+import ExploreViewContainer from '.';
+
+const reduxState = {
+  explore: {
+    common: { conf: { SUPERSET_WEBSERVER_TIMEOUT: 60 } },
+    controls: { datasource: { value: '1__table' } },
+    datasource: {
+      type: 'table',
+      columns: [{ is_dttm: false }],
+      metrics: [{ id: 1, metric_name: 'count' }],
+    },
+    user: {
+      userId: 1,
+    },
+    isStarred: false,
+  },
+  charts: {
+    1: {
+      id: 1,
+      latestQueryFormData: {
+        datasource: '1__table',
+      },
+    },
+  },
+};
+
+const key = 'aWrs7w29sd';
+
+jest.mock('react-resize-detector', () => ({
+  __esModule: true,
+  useResizeDetector: () => ({ height: 100, width: 100 }),
+}));
+
+jest.mock('lodash/debounce', () => ({
+  __esModule: true,
+  default: (fuc: Function) => fuc,
+}));
+
+fetchMock.post('glob:*/api/v1/explore/form_data*', { key });
+fetchMock.put('glob:*/api/v1/explore/form_data*', { key });
+fetchMock.get('glob:*/api/v1/explore/form_data*', {});
+
+const renderWithRouter = (withKey?: boolean) => {
+  const path = '/superset/explore/';
+  const search = withKey ? `?form_data_key=${key}` : '';
+  return render(
+    <MemoryRouter initialEntries={[`${path}${search}`]}>
+      <Route path={path}>
+        <ExploreViewContainer />
+      </Route>
+    </MemoryRouter>,
+    { useRedux: true, initialState: reduxState },
+  );
+};
+
+test('generates a new form_data param when none is available', async () => {
+  const replaceState = jest.spyOn(window.history, 'replaceState');
+  await waitFor(() => renderWithRouter());
+  expect(replaceState).toHaveBeenCalledWith(
+    expect.anything(),
+    undefined,
+    expect.stringMatching('form_data'),
+  );
+  replaceState.mockRestore();
+});
+
+test('generates a different form_data param when one is provided and is mounting', async () => {
+  const replaceState = jest.spyOn(window.history, 'replaceState');
+  await waitFor(() => renderWithRouter(true));
+  expect(replaceState).not.toHaveBeenLastCalledWith(
+    0,
+    expect.anything(),
+    undefined,
+    expect.stringMatching(key),
+  );
+  replaceState.mockRestore();
+});
+
+test('reuses the same form_data param when updating', async () => {
+  const replaceState = jest.spyOn(window.history, 'replaceState');
+  const pushState = jest.spyOn(window.history, 'pushState');
+  await waitFor(() => renderWithRouter());
+  expect(replaceState.mock.calls.length).toBe(1);
+  userEvent.click(screen.getByText('Run'));
+  await waitFor(() => expect(pushState.mock.calls.length).toBe(1));
+  expect(replaceState.mock.calls[0]).toEqual(pushState.mock.calls[0]);
+  replaceState.mockRestore();
+  pushState.mockRestore();
+});

superset-frontend/src/explore/components/ExploreViewContainer/index.jsx

@@ -21,7 +21,7 @@ import React, { useCallback, useEffect, useMemo, useState } from 'react';
 import PropTypes from 'prop-types';
 import { bindActionCreators } from 'redux';
 import { connect } from 'react-redux';
-import { styled, t, css, useTheme } from '@superset-ui/core';
+import { styled, t, css, useTheme, logging } from '@superset-ui/core';
 import { debounce } from 'lodash';
 import { Resizable } from 're-resizable';
 
@@ -37,26 +37,28 @@ import {
   LocalStorageKeys,
 } from 'src/utils/localStorageHelpers';
 import { URL_PARAMS } from 'src/constants';
+import { getUrlParam } from 'src/utils/urlUtils';
 import cx from 'classnames';
 import * as chartActions from 'src/chart/chartAction';
 import { fetchDatasourceMetadata } from 'src/dashboard/actions/datasources';
 import { chartPropShape } from 'src/dashboard/util/propShapes';
 import { mergeExtraFormData } from 'src/dashboard/components/nativeFilters/utils';
-import ExploreChartPanel from './ExploreChartPanel';
+import { postFormData, putFormData } from 'src/explore/exploreUtils/formData';
-import ConnectedControlPanelsContainer from './ControlPanelsContainer';
+import ExploreChartPanel from '../ExploreChartPanel';
-import SaveModal from './SaveModal';
+import ConnectedControlPanelsContainer from '../ControlPanelsContainer';
-import QueryAndSaveBtns from './QueryAndSaveBtns';
+import SaveModal from '../SaveModal';
-import DataSourcePanel from './DatasourcePanel';
+import QueryAndSaveBtns from '../QueryAndSaveBtns';
-import { getExploreLongUrl } from '../exploreUtils';
+import DataSourcePanel from '../DatasourcePanel';
-import { areObjectsEqual } from '../../reduxUtils';
+import { mountExploreUrl } from '../../exploreUtils';
-import { getFormDataFromControls } from '../controlUtils';
+import { areObjectsEqual } from '../../../reduxUtils';
-import * as exploreActions from '../actions/exploreActions';
+import { getFormDataFromControls } from '../../controlUtils';
-import * as saveModalActions from '../actions/saveModalActions';
+import * as exploreActions from '../../actions/exploreActions';
-import * as logActions from '../../logger/actions';
+import * as saveModalActions from '../../actions/saveModalActions';
+import * as logActions from '../../../logger/actions';
 import {
   LOG_ACTIONS_MOUNT_EXPLORER,
   LOG_ACTIONS_CHANGE_EXPLORE_CONTROLS,
-} from '../../logger/LogUtils';
+} from '../../../logger/LogUtils';
 
 const propTypes = {
   ...ExploreChartPanel.propTypes,
@@ -161,6 +163,37 @@ function useWindowSize({ delayMs = 250 } = {}) {
   return size;
 }
 
+const updateHistory = debounce(
+  async (formData, datasetId, isReplace, standalone, force, title) => {
+    const payload = { ...formData };
+    const chartId = formData.slice_id;
+
+    try {
+      let key;
+      let stateModifier;
+      if (isReplace) {
+        key = await postFormData(datasetId, formData, chartId);
+        stateModifier = 'replaceState';
+      } else {
+        key = getUrlParam(URL_PARAMS.formDataKey);
+        await putFormData(datasetId, key, formData, chartId);
+        stateModifier = 'pushState';
+      }
+      const url = mountExploreUrl(
+        standalone ? URL_PARAMS.standalone.name : null,
+        {
+          [URL_PARAMS.formDataKey.name]: key,
+        },
+        force,
+      );
+      window.history[stateModifier](payload, title, url);
+    } catch (e) {
+      logging.warn('Failed at altering browser history', e);
+    }
+  },
+  1000,
+);
+
 function ExploreViewContainer(props) {
   const dynamicPluginContext = usePluginContext();
   const dynamicPlugin = dynamicPluginContext.dynamicPlugins[props.vizType];
@@ -191,39 +224,31 @@ function ExploreViewContainer(props) {
   };
 
   const addHistory = useCallback(
-    ({ isReplace = false, title } = {}) => {
+    async ({ isReplace = false, title } = {}) => {
       const formData = props.dashboardId
         ? {
             ...props.form_data,
             dashboardId: props.dashboardId,
           }
         : props.form_data;
-      const payload = { ...formData };
+      const datasetId = props.datasource.id;
-      const longUrl = getExploreLongUrl(
-        formData,
-        props.standalone ? URL_PARAMS.standalone.name : null,
-        false,
-        {},
-        props.force,
-      );
 
-      try {
+      updateHistory(
-        if (isReplace) {
+        formData,
-          window.history.replaceState(payload, title, longUrl);
+        datasetId,
-        } else {
+        isReplace,
-          window.history.pushState(payload, title, longUrl);
+        props.standalone,
-        }
+        props.force,
-      } catch (e) {
+        title,
-        // eslint-disable-next-line no-console
+      );
-        console.warn(
-          'Failed at altering browser history',
-          payload,
-          title,
-          longUrl,
-        );
-      }
     },
-    [props.form_data, props.standalone, props.force],
+    [
+      props.dashboardId,
+      props.form_data,
+      props.datasource.id,
+      props.standalone,
+      props.force,
+    ],
   );
 
   const handlePopstate = useCallback(() => {
@@ -447,7 +472,6 @@ function ExploreViewContainer(props) {
         {...props}
         errorMessage={renderErrorMessage()}
         refreshOverlayVisible={chartIsStale}
-        addHistory={addHistory}
         onQuery={onQuery}
       />
     );

superset-frontend/src/explore/exploreUtils/exploreUtils.test.jsx

@@ -22,7 +22,6 @@ import URI from 'urijs';
 import {
   buildV1ChartDataPayload,
   getExploreUrl,
-  getExploreLongUrl,
   shouldUseLegacyApi,
   getSimpleSQLExpression,
 } from 'src/explore/exploreUtils';
@@ -35,7 +34,6 @@ describe('exploreUtils', () => {
   const formData = {
     datasource: '1__table',
   };
-  const sFormData = JSON.stringify(formData);
   function compareURI(uri1, uri2) {
     expect(uri1.toString()).toBe(uri2.toString());
   }
@@ -191,25 +189,6 @@ describe('exploreUtils', () => {
     });
   });
 
-  describe('getExploreLongUrl', () => {
-    it('generates proper base url with form_data', () => {
-      compareURI(
-        URI(getExploreLongUrl(formData, 'base')),
-        URI('/superset/explore/').search({ form_data: sFormData }),
-      );
-    });
-
-    it('generates url with standalone', () => {
-      compareURI(
-        URI(getExploreLongUrl(formData, 'standalone')),
-        URI('/superset/explore/').search({
-          form_data: sFormData,
-          standalone: DashboardStandaloneMode.HIDE_NAV,
-        }),
-      );
-    });
-  });
-
   describe('buildV1ChartDataPayload', () => {
     it('generate valid request payload despite no registered buildQuery', () => {
       const v1RequestPayload = buildV1ChartDataPayload({

superset-frontend/src/explore/exploreUtils/formData.ts

@@ -0,0 +1,61 @@
+/**
+ * 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 { SupersetClient, JsonObject } from '@superset-ui/core';
+
+type Payload = {
+  dataset_id: number;
+  form_data: string;
+  chart_id?: number;
+};
+
+const assemblePayload = (
+  datasetId: number,
+  form_data: JsonObject,
+  chartId?: number,
+) => {
+  const payload: Payload = {
+    dataset_id: datasetId,
+    form_data: JSON.stringify(form_data),
+  };
+  if (chartId) {
+    payload.chart_id = chartId;
+  }
+  return payload;
+};
+
+export const postFormData = (
+  datasetId: number,
+  form_data: JsonObject,
+  chartId?: number,
+): Promise<string> =>
+  SupersetClient.post({
+    endpoint: 'api/v1/explore/form_data',
+    jsonPayload: assemblePayload(datasetId, form_data, chartId),
+  }).then(r => r.json.key);
+
+export const putFormData = (
+  datasetId: number,
+  key: string,
+  form_data: JsonObject,
+  chartId?: number,
+): Promise<string> =>
+  SupersetClient.put({
+    endpoint: `api/v1/explore/form_data/${key}`,
+    jsonPayload: assemblePayload(datasetId, form_data, chartId),
+  }).then(r => r.json.message);

superset-frontend/src/explore/exploreUtils/getExploreLongUrl.test.ts

@@ -1,134 +0,0 @@
-/**
- * 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 { getExploreLongUrl, getExploreUrlFromDashboard } from '.';
-
-const createParams = () => ({
-  formData: {
-    datasource: 'datasource',
-    viz_type: 'viz_type',
-  },
-  endpointType: 'endpointType',
-  allowOverflow: true,
-  extraSearch: { same: 'any-string' },
-});
-
-test('Should return null if formData.datasource is falsy', () => {
-  const params = createParams();
-  expect(
-    getExploreLongUrl(
-      {},
-      params.endpointType,
-      params.allowOverflow,
-      params.extraSearch,
-    ),
-  ).toBeNull();
-});
-
-test('Get url when endpointType:standalone', () => {
-  const params = createParams();
-  expect(
-    getExploreLongUrl(
-      params.formData,
-      'standalone',
-      params.allowOverflow,
-      params.extraSearch,
-    ),
-  ).toBe(
-    '/superset/explore/?same=any-string&form_data=%7B%22datasource%22%3A%22datasource%22%2C%22viz_type%22%3A%22viz_type%22%7D&standalone=1',
-  );
-});
-
-test('Get url when endpointType:standalone and force:true', () => {
-  const params = createParams();
-  expect(
-    getExploreLongUrl(
-      params.formData,
-      'standalone',
-      params.allowOverflow,
-      params.extraSearch,
-      true,
-    ),
-  ).toBe(
-    '/superset/explore/?same=any-string&form_data=%7B%22datasource%22%3A%22datasource%22%2C%22viz_type%22%3A%22viz_type%22%7D&force=1&standalone=1',
-  );
-});
-
-test('Get url when endpointType:standalone and allowOverflow:false', () => {
-  const params = createParams();
-  expect(
-    getExploreLongUrl(
-      params.formData,
-      params.endpointType,
-      false,
-      params.extraSearch,
-    ),
-  ).toBe(
-    '/superset/explore/?same=any-string&form_data=%7B%22datasource%22%3A%22datasource%22%2C%22viz_type%22%3A%22viz_type%22%7D',
-  );
-});
-
-test('Get url when endpointType:results', () => {
-  const params = createParams();
-  expect(
-    getExploreLongUrl(
-      params.formData,
-      'results',
-      params.allowOverflow,
-      params.extraSearch,
-    ),
-  ).toBe(
-    '/superset/explore_json/?same=any-string&form_data=%7B%22datasource%22%3A%22datasource%22%2C%22viz_type%22%3A%22viz_type%22%7D',
-  );
-});
-
-test('Get url when endpointType:results and allowOverflow:false', () => {
-  const params = createParams();
-  expect(
-    getExploreLongUrl(params.formData, 'results', false, params.extraSearch),
-  ).toBe(
-    '/superset/explore_json/?same=any-string&form_data=%7B%22datasource%22%3A%22datasource%22%2C%22viz_type%22%3A%22viz_type%22%7D',
-  );
-});
-
-test('Get url from a dashboard', () => {
-  const formData = {
-    ...createParams().formData,
-    // these params should get filtered out
-    extra_form_data: {
-      filters: {
-        col: 'foo',
-        op: 'IN',
-        val: ['bar'],
-      },
-    },
-    dataMask: {
-      'NATIVE_FILTER-bqEoUsEPe': {
-        id: 'NATIVE_FILTER-bqEoUsEPe',
-        lots: 'of other stuff here too',
-      },
-    },
-    url_params: {
-      native_filters: '(blah)',
-      standalone: true,
-    },
-  };
-  expect(getExploreUrlFromDashboard(formData)).toBe(
-    '/superset/explore/?form_data=%7B%22datasource%22%3A%22datasource%22%2C%22viz_type%22%3A%22viz_type%22%2C%22extra_form_data%22%3A%7B%22filters%22%3A%7B%22col%22%3A%22foo%22%2C%22op%22%3A%22IN%22%2C%22val%22%3A%5B%22bar%22%5D%7D%7D%7D',
-  );
-});

superset-frontend/src/explore/exploreUtils/index.js

@@ -18,7 +18,6 @@
  */
 
 import { useCallback, useEffect } from 'react';
-import { omit } from 'lodash';
 /* eslint camelcase: 0 */
 import URI from 'urijs';
 import {
@@ -37,8 +36,6 @@ import {
 import { DashboardStandaloneMode } from 'src/dashboard/util/constants';
 import { optionLabel } from '../../utils/common';
 
-const MAX_URL_LENGTH = 8000;
-
 export function getChartKey(explore) {
   const { slice } = explore;
   return slice ? slice.slice_id : 0;
@@ -91,64 +88,20 @@ export function getURIDirectory(endpointType = 'base') {
   return '/superset/explore/';
 }
 
-/**
+export function mountExploreUrl(endpointType, extraSearch = {}, force = false) {
- * This gets the url of the explore page, with all the form data included explicitly.
- * This includes any form data overrides from the dashboard.
- */
-export function getExploreLongUrl(
-  formData,
-  endpointType,
-  allowOverflow = true,
-  extraSearch = {},
-  force = false,
-) {
-  if (!formData.datasource) {
-    return null;
-  }
-
   const uri = new URI('/');
   const directory = getURIDirectory(endpointType);
   const search = uri.search(true);
   Object.keys(extraSearch).forEach(key => {
     search[key] = extraSearch[key];
   });
-  search.form_data = safeStringify(formData);
   if (endpointType === URL_PARAMS.standalone.name) {
     if (force) {
       search.force = '1';
     }
     search.standalone = DashboardStandaloneMode.HIDE_NAV;
   }
-  const url = uri.directory(directory).search(search).toString();
+  return uri.directory(directory).search(search).toString();
-  if (!allowOverflow && url.length > MAX_URL_LENGTH) {
-    const minimalFormData = {
-      datasource: formData.datasource,
-      viz_type: formData.viz_type,
-    };
-    return getExploreLongUrl(
-      minimalFormData,
-      endpointType,
-      false,
-      {
-        URL_IS_TOO_LONG_TO_SHARE: null,
-      },
-      force,
-    );
-  }
-  return url;
-}
-
-export function getExploreUrlFromDashboard(formData) {
-  // remove formData params that we don't need in the explore url.
-  // These are present when generating explore urls from the dashboard page.
-  // This should be superseded by some sort of "exploration context" system
-  // where form data and other context is referenced by id.
-  const trimmedFormData = omit(formData, [
-    'dataMask',
-    'url_params',
-    'label_colors',
-  ]);
-  return getExploreLongUrl(trimmedFormData, null, false);
 }
 
 export function getChartDataUri({ path, qs, allowDomainSharding = false }) {

superset/views/core.py

@@ -87,6 +87,8 @@ from superset.exceptions import (
     SupersetSecurityException,
     SupersetTimeoutException,
 )
+from superset.explore.form_data.commands.get import GetFormDataCommand
+from superset.explore.form_data.commands.parameters import CommandParameters
 from superset.extensions import async_query_manager, cache_manager
 from superset.jinja_context import get_template_processor
 from superset.models.core import Database, FavStar, Log
@@ -730,7 +732,19 @@ class Superset(BaseSupersetView):  # pylint: disable=too-many-public-methods
     def explore(
         self, datasource_type: Optional[str] = None, datasource_id: Optional[int] = None
     ) -> FlaskResponse:
-        form_data, slc = get_form_data(use_slice_data=True)
+        initial_form_data = {}
+
+        form_data_key = request.args.get("form_data_key")
+        if form_data_key:
+            parameters = CommandParameters(actor=g.user, key=form_data_key,)
+            value = GetFormDataCommand(parameters).run()
+            if value:
+                initial_form_data = json.loads(value)
+
+        form_data, slc = get_form_data(
+            use_slice_data=True, initial_form_data=initial_form_data
+        )
+
         query_context = request.form.get("query_context")
         # Flash the SIP-15 message if the slice is owned by the current user and has not
         # been updated, i.e., is not using the [start, end) interval.

superset/views/utils.py

@@ -137,9 +137,11 @@ def loads_request_json(request_json_data: str) -> Dict[Any, Any]:
 
 
 def get_form_data(  # pylint: disable=too-many-locals
-    slice_id: Optional[int] = None, use_slice_data: bool = False
+    slice_id: Optional[int] = None,
+    use_slice_data: bool = False,
+    initial_form_data: Optional[Dict[str, Any]] = None,
 ) -> Tuple[Dict[str, Any], Optional[Slice]]:
-    form_data: Dict[str, Any] = {}
+    form_data: Dict[str, Any] = initial_form_data or {}
 
     if has_request_context():  # type: ignore
         # chart data API requests are JSON