Skip to content
New issue

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

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

Already on GitHub? # to your account

fix: Explore long URL problem #18181

Merged
merged 12 commits into from
Jan 28, 2022
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -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')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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' });
Expand Down
33 changes: 33 additions & 0 deletions superset-frontend/spec/helpers/ResizeObserver.ts
Original file line number Diff line number Diff line change
@@ -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 };
2 changes: 2 additions & 0 deletions superset-frontend/spec/helpers/shim.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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();

Expand Down
4 changes: 4 additions & 0 deletions superset-frontend/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ export const URL_PARAMS = {
name: 'show_filters',
type: 'boolean',
},
formDataKey: {
name: 'form_data_key',
type: 'string',
},
} as const;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ const SliceHeader: FC<SliceHeaderProps> = ({
updateSliceName = () => ({}),
toggleExpandSlice = () => ({}),
logExploreChart = () => ({}),
exploreUrl = '#',
onExploreChart,
exportCSV = () => ({}),
editMode = false,
annotationQuery = {},
Expand Down Expand Up @@ -171,7 +171,7 @@ const SliceHeader: FC<SliceHeaderProps> = ({
toggleExpandSlice={toggleExpandSlice}
forceRefresh={forceRefresh}
logExploreChart={logExploreChart}
exploreUrl={exploreUrl}
onExploreChart={onExploreChart}
exportCSV={exportCSV}
exportFullCSV={exportFullCSV}
supersetCanExplore={supersetCanExplore}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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';
Expand Down Expand Up @@ -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>
)}

Expand All @@ -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}
/>
)}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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';
Expand Down Expand Up @@ -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, {
Expand Down Expand Up @@ -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}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.',
);
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand All @@ -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);
Expand All @@ -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.'));
}
}
Expand Down
Loading