fix: Explore long URL problem (apache#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
CybercentreCanada · Jan 28, 2022 · 4b61c76 · 4b61c76
1 parent a06e043
commit 4b61c76
Show file tree

Hide file tree

Showing 25 changed files with 375 additions and 398 deletions.
diff --git a/superset-frontend/cypress-base/cypress/integration/explore/advanced_analytics.test.ts b/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')

diff --git a/superset-frontend/cypress-base/cypress/integration/explore/link.test.ts b/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' });

diff --git a/superset-frontend/spec/helpers/ResizeObserver.ts b/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 };
diff --git a/superset-frontend/spec/helpers/shim.ts b/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();
 

diff --git a/superset-frontend/src/constants.ts b/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;
 
 /**

diff --git a/superset-frontend/src/dashboard/components/SliceHeader/SliceHeader.test.tsx b/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', () => {

diff --git a/superset-frontend/src/dashboard/components/SliceHeader/index.tsx b/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}

diff --git a/superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx b/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', () => {

diff --git a/superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx b/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;
-  exploreUrl?: string;
+  formData: { slice_id: number; datasource: 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}>
-            <a href={this.props.exploreUrl} rel="noopener noreferrer">
-              {t('View chart in Explore')}
-            </a>
+          <Menu.Item
+            key={MENU_KEYS.EXPLORE_CHART}
+            onClick={this.props.onExploreChart}
+          >
+            {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}
           />
         )}
 

diff --git a/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx b/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 {
-  exportChart,
-  getExploreUrlFromDashboard,
-} from 'src/explore/exploreUtils';
+import { exportChart, mountExploreUrl } 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}

diff --git a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/ShareMenuItems.test.tsx b/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.',
     );
   });
 });

diff --git a/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx b/superset-frontend/src/dashboard/components/menu/ShareMenuItems/index.tsx
@@ -19,24 +19,27 @@
 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;
   emailBody: string;
   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();
-      const shortUrl = await getShortUrl(copyUrl);
-      await copyTextToClipboard(shortUrl);
+      await copyTextToClipboard(await generateUrl());
       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 shortUrl = await getShortUrl(copyUrl);
-      const bodyWithLink = `${emailBody}${shortUrl}`;
+      const bodyWithLink = `${emailBody}${await generateUrl()}`;
       window.location.href = `mailto:?Subject=${emailSubject}%20&Body=${bodyWithLink}`;
     } catch (error) {
+      logging.error(error);
       addDangerToast(t('Sorry, something went wrong. Try again later.'));
     }
   }