Skip to content

Commit

Permalink
feat: Improves key expiration handling in Explore (#18624)
Browse files Browse the repository at this point in the history
* feat: Improves key expiration handling in Explore

* Sets use_slice_data equals true

* Shows toast when recovering
  • Loading branch information
michael-s-molina authored Feb 9, 2022
1 parent bcad1ac commit f03b4db
Show file tree
Hide file tree
Showing 8 changed files with 79 additions and 8 deletions.
8 changes: 8 additions & 0 deletions superset-frontend/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,14 @@ export const URL_PARAMS = {
name: 'form_data_key',
type: 'string',
},
sliceId: {
name: 'slice_id',
type: 'string',
},
datasetId: {
name: 'dataset_id',
type: 'string',
},
} as const;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,9 +245,12 @@ export default class Chart extends React.Component {
const key = await postFormData(
this.props.datasource.id,
this.props.formData,
this.props.slice.id,
this.props.slice.slice_id,
);
const url = mountExploreUrl(null, { [URL_PARAMS.formDataKey.name]: key });
const url = mountExploreUrl(null, {
[URL_PARAMS.formDataKey.name]: key,
[URL_PARAMS.sliceId.name]: this.props.slice.slice_id,
});
window.open(url, '_blank', 'noreferrer');
} catch (error) {
logging.error(error);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ test('Click on "Share dashboard by email" and succeed', async () => {
await waitFor(() => {
expect(props.addDangerToast).toBeCalledTimes(0);
expect(window.location.href).toBe(
'mailto:?Subject=Superset dashboard COVID Vaccine Dashboard%20&Body=Check out this dashboard: http://localhost:8088/r/3',
'mailto:?Subject=Superset%20dashboard%20COVID%20Vaccine%20Dashboard%20&Body=Check%20out%20this%20dashboard%3A%20http%3A%2F%2Flocalhost%3A8088%2Fr%2F3',
);
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ const ShareMenuItems = (props: ShareMenuItemProps) => {
);
return `${window.location.origin}${mountExploreUrl(null, {
[URL_PARAMS.formDataKey.name]: key,
[URL_PARAMS.sliceId.name]: formData.slice_id,
})}`;
}
const copyUrl = await getCopyUrl();
Expand All @@ -101,8 +102,11 @@ const ShareMenuItems = (props: ShareMenuItemProps) => {

async function onShareByEmail() {
try {
const bodyWithLink = `${emailBody}${await generateUrl()}`;
window.location.href = `mailto:?Subject=${emailSubject}%20&Body=${bodyWithLink}`;
const encodedBody = encodeURIComponent(
`${emailBody}${await generateUrl()}`,
);
const encodedSubject = encodeURIComponent(emailSubject);
window.location.href = `mailto:?Subject=${encodedSubject}%20&Body=${encodedBody}`;
} catch (error) {
logging.error(error);
addDangerToast(t('Sorry, something went wrong. Try again later.'));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const reduxState = {
common: { conf: { SUPERSET_WEBSERVER_TIMEOUT: 60 } },
controls: { datasource: { value: '1__table' } },
datasource: {
id: 1,
type: 'table',
columns: [{ is_dttm: false }],
metrics: [{ id: 1, metric_name: 'count' }],
Expand Down Expand Up @@ -65,7 +66,7 @@ fetchMock.get('glob:*/api/v1/explore/form_data*', {});

const renderWithRouter = (withKey?: boolean) => {
const path = '/superset/explore/';
const search = withKey ? `?form_data_key=${key}` : '';
const search = withKey ? `?form_data_key=${key}&dataset_id=1` : '';
return render(
<MemoryRouter initialEntries={[`${path}${search}`]}>
<Route path={path}>
Expand All @@ -82,7 +83,12 @@ test('generates a new form_data param when none is available', async () => {
expect(replaceState).toHaveBeenCalledWith(
expect.anything(),
undefined,
expect.stringMatching('form_data'),
expect.stringMatching('form_data_key'),
);
expect(replaceState).toHaveBeenCalledWith(
expect.anything(),
undefined,
expect.stringMatching('dataset_id'),
);
replaceState.mockRestore();
});
Expand All @@ -96,6 +102,11 @@ test('generates a different form_data param when one is provided and is mounting
undefined,
expect.stringMatching(key),
);
expect(replaceState).toHaveBeenCalledWith(
expect.anything(),
undefined,
expect.stringMatching('dataset_id'),
);
replaceState.mockRestore();
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,12 @@ const updateHistory = debounce(
async (formData, datasetId, isReplace, standalone, force, title) => {
const payload = { ...formData };
const chartId = formData.slice_id;
const additionalParam = {};
if (chartId) {
additionalParam[URL_PARAMS.sliceId.name] = chartId;
} else {
additionalParam[URL_PARAMS.datasetId.name] = datasetId;
}

try {
let key;
Expand All @@ -183,6 +189,7 @@ const updateHistory = debounce(
standalone ? URL_PARAMS.standalone.name : null,
{
[URL_PARAMS.formDataKey.name]: key,
...additionalParam,
},
force,
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import React from 'react';
import PropTypes from 'prop-types';
import { t, styled, supersetTheme } from '@superset-ui/core';
import { getUrlParam } from 'src/utils/urlUtils';

import { Dropdown, Menu } from 'src/common/components';
import { Tooltip } from 'src/components/Tooltip';
Expand All @@ -32,6 +33,7 @@ import { postForm } from 'src/explore/exploreUtils';
import Button from 'src/components/Button';
import ErrorAlert from 'src/components/ErrorMessage/ErrorAlert';
import WarningIconWithTooltip from 'src/components/WarningIconWithTooltip';
import { URL_PARAMS } from 'src/constants';

const propTypes = {
actions: PropTypes.object.isRequired,
Expand Down Expand Up @@ -182,6 +184,14 @@ class DatasourceControl extends React.PureComponent {
const { showChangeDatasourceModal, showEditDatasourceModal } = this.state;
const { datasource, onChange } = this.props;
const isMissingDatasource = datasource.id == null;
let isMissingParams = false;
if (isMissingDatasource) {
const datasetId = getUrlParam(URL_PARAMS.datasetId);
const sliceId = getUrlParam(URL_PARAMS.sliceId);
if (!datasetId && !sliceId) {
isMissingParams = true;
}
}

const isSqlSupported = datasource.type === 'table';

Expand Down Expand Up @@ -244,7 +254,25 @@ class DatasourceControl extends React.PureComponent {
</Dropdown>
</div>
{/* missing dataset */}
{isMissingDatasource && (
{isMissingDatasource && isMissingParams && (
<div className="error-alert">
<ErrorAlert
level="warning"
title={t('Missing URL parameters')}
source="explore"
subtitle={
<>
<p>
{t(
'The URL is missing the dataset_id or slice_id parameters.',
)}
</p>
</>
}
/>
</div>
)}
{isMissingDatasource && !isMissingParams && (
<div className="error-alert">
<ErrorAlert
level="warning"
Expand Down
10 changes: 10 additions & 0 deletions superset/views/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -747,6 +747,16 @@ def explore(
if value:
initial_form_data = json.loads(value)

if not initial_form_data:
slice_id = request.args.get("slice_id")
dataset_id = request.args.get("dataset_id")
if slice_id:
initial_form_data["slice_id"] = slice_id
flash(_("Form data not found in cache, reverting to chart metadata."))
elif dataset_id:
initial_form_data["datasource"] = f"{dataset_id}__table"
flash(_("Form data not found in cache, reverting to dataset metadata."))

form_data, slc = get_form_data(
use_slice_data=True, initial_form_data=initial_form_data
)
Expand Down

0 comments on commit f03b4db

Please sign in to comment.