Skip to content

Commit

Permalink
fix: Explore long URL problem
Browse files Browse the repository at this point in the history
  • Loading branch information
michael-s-molina committed Jan 26, 2022
1 parent fa11a97 commit 9d0ea9e
Show file tree
Hide file tree
Showing 19 changed files with 291 additions and 357 deletions.
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 @@ -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 @@ -100,7 +100,7 @@ export interface SliceHeaderControlsProps {
updatedDttm: number | null;
isFullSize?: boolean;
formData: object;
exploreUrl?: string;
onExploreChart: () => void;

forceRefresh: (sliceId: number, dashboardId: number) => void;
logExploreChart?: (sliceId: number) => void;
Expand Down Expand Up @@ -283,10 +283,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 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.slice.id,
this.props.formData,
);
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
30 changes: 1 addition & 29 deletions superset-frontend/src/explore/components/EmbedCodeButton.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,46 +17,23 @@
* under the License.
*/
import React from 'react';
import PropTypes from 'prop-types';
import { t } from '@superset-ui/core';

import Popover from 'src/components/Popover';
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) {
super(props);
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) {
Expand All @@ -67,9 +44,7 @@ export default class EmbedCodeButton extends React.Component {
}

generateEmbedHTML() {
const srcLink = `${window.location.origin + getURIDirectory()}?r=${
this.state.shortUrlId
}&${URL_PARAMS.standalone.name}=1&height=${this.state.height}`;
const srcLink = `${window.location.href}&${URL_PARAMS.standalone.name}=1&height=${this.state.height}`;
return (
'<iframe\n' +
` width="${this.state.width}"\n` +
Expand Down Expand Up @@ -150,7 +125,6 @@ export default class EmbedCodeButton extends React.Component {
<Popover
trigger="click"
placement="left"
onClick={this.getCopyUrl}
content={this.renderPopoverContent()}
>
<Tooltip
Expand All @@ -171,5 +145,3 @@ export default class EmbedCodeButton extends React.Component {
);
}
}

EmbedCodeButton.propTypes = propTypes;
72 changes: 8 additions & 64 deletions superset-frontend/src/explore/components/EmbedCodeButton.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,84 +17,29 @@
* under the License.
*/
import React from 'react';
import { shallow, mount } from 'enzyme';
import { supersetTheme, ThemeProvider } from '@superset-ui/core';
import { shallow } from 'enzyme';
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(
true,
);
expect(React.isValidElement(<EmbedCodeButton />)).toBe(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
.stub(exploreUtils, 'getURIDirectory')
.callsFake(() => 'endpoint_url');
const wrapper = mount(
<ThemeProvider theme={supersetTheme}>
<EmbedCodeButton {...defaultProps} />
</ThemeProvider>,
);
const href = 'http://localhost/explore?form_data_key=xxxxxxxxx';
Object.defineProperty(window, 'location', { value: { href } });
const wrapper = mount(<EmbedCodeButton />);
wrapper.find(EmbedCodeButton).setState({
height: '1000',
width: '2000',
shortUrlId: 100,
});
const embedHTML =
`${
Expand All @@ -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();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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();
await copyTextToClipboard(shortUrl);
const url = window.location.href;
await copyTextToClipboard(url);
setCopyTooltip(t('Copied to clipboard!'));
addSuccessToast(t('Copied to clipboard!'));
} catch (error) {
Expand All @@ -123,8 +120,8 @@ const ExploreActionButtons = (props: ExploreActionButtonsProps) => {
const doShareEmail = async () => {
try {
const subject = t('Superset Chart');
const shortUrl = await getShortUrl();
const body = t('%s%s', 'Check out this chart: ', shortUrl);
const url = window.location.href;
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.'));
Expand Down Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading

0 comments on commit 9d0ea9e

Please sign in to comment.