feat: Improves key expiration handling in Explore (#18624)

* feat: Improves key expiration handling in Explore

* Sets use_slice_data equals true

* Shows toast when recovering
This commit is contained in:
Michael S. Molina 2022-02-09 13:20:21 -03:00 committed by GitHub
parent bcad1acec2
commit f03b4dbedb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 79 additions and 8 deletions

View File

@ -59,6 +59,14 @@ export const URL_PARAMS = {
name: 'form_data_key', name: 'form_data_key',
type: 'string', type: 'string',
}, },
sliceId: {
name: 'slice_id',
type: 'string',
},
datasetId: {
name: 'dataset_id',
type: 'string',
},
} as const; } as const;
/** /**

View File

@ -245,9 +245,12 @@ export default class Chart extends React.Component {
const key = await postFormData( const key = await postFormData(
this.props.datasource.id, this.props.datasource.id,
this.props.formData, 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'); window.open(url, '_blank', 'noreferrer');
} catch (error) { } catch (error) {
logging.error(error); logging.error(error);

View File

@ -159,7 +159,7 @@ test('Click on "Share dashboard by email" and succeed', async () => {
await waitFor(() => { await waitFor(() => {
expect(props.addDangerToast).toBeCalledTimes(0); expect(props.addDangerToast).toBeCalledTimes(0);
expect(window.location.href).toBe( 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',
); );
}); });
}); });

View File

@ -83,6 +83,7 @@ const ShareMenuItems = (props: ShareMenuItemProps) => {
); );
return `${window.location.origin}${mountExploreUrl(null, { return `${window.location.origin}${mountExploreUrl(null, {
[URL_PARAMS.formDataKey.name]: key, [URL_PARAMS.formDataKey.name]: key,
[URL_PARAMS.sliceId.name]: formData.slice_id,
})}`; })}`;
} }
const copyUrl = await getCopyUrl(); const copyUrl = await getCopyUrl();
@ -101,8 +102,11 @@ const ShareMenuItems = (props: ShareMenuItemProps) => {
async function onShareByEmail() { async function onShareByEmail() {
try { try {
const bodyWithLink = `${emailBody}${await generateUrl()}`; const encodedBody = encodeURIComponent(
window.location.href = `mailto:?Subject=${emailSubject}%20&Body=${bodyWithLink}`; `${emailBody}${await generateUrl()}`,
);
const encodedSubject = encodeURIComponent(emailSubject);
window.location.href = `mailto:?Subject=${encodedSubject}%20&Body=${encodedBody}`;
} catch (error) { } catch (error) {
logging.error(error); logging.error(error);
addDangerToast(t('Sorry, something went wrong. Try again later.')); addDangerToast(t('Sorry, something went wrong. Try again later.'));

View File

@ -28,6 +28,7 @@ const reduxState = {
common: { conf: { SUPERSET_WEBSERVER_TIMEOUT: 60 } }, common: { conf: { SUPERSET_WEBSERVER_TIMEOUT: 60 } },
controls: { datasource: { value: '1__table' } }, controls: { datasource: { value: '1__table' } },
datasource: { datasource: {
id: 1,
type: 'table', type: 'table',
columns: [{ is_dttm: false }], columns: [{ is_dttm: false }],
metrics: [{ id: 1, metric_name: 'count' }], metrics: [{ id: 1, metric_name: 'count' }],
@ -65,7 +66,7 @@ fetchMock.get('glob:*/api/v1/explore/form_data*', {});
const renderWithRouter = (withKey?: boolean) => { const renderWithRouter = (withKey?: boolean) => {
const path = '/superset/explore/'; const path = '/superset/explore/';
const search = withKey ? `?form_data_key=${key}` : ''; const search = withKey ? `?form_data_key=${key}&dataset_id=1` : '';
return render( return render(
<MemoryRouter initialEntries={[`${path}${search}`]}> <MemoryRouter initialEntries={[`${path}${search}`]}>
<Route path={path}> <Route path={path}>
@ -82,7 +83,12 @@ test('generates a new form_data param when none is available', async () => {
expect(replaceState).toHaveBeenCalledWith( expect(replaceState).toHaveBeenCalledWith(
expect.anything(), expect.anything(),
undefined, undefined,
expect.stringMatching('form_data'), expect.stringMatching('form_data_key'),
);
expect(replaceState).toHaveBeenCalledWith(
expect.anything(),
undefined,
expect.stringMatching('dataset_id'),
); );
replaceState.mockRestore(); replaceState.mockRestore();
}); });
@ -96,6 +102,11 @@ test('generates a different form_data param when one is provided and is mounting
undefined, undefined,
expect.stringMatching(key), expect.stringMatching(key),
); );
expect(replaceState).toHaveBeenCalledWith(
expect.anything(),
undefined,
expect.stringMatching('dataset_id'),
);
replaceState.mockRestore(); replaceState.mockRestore();
}); });

View File

@ -167,6 +167,12 @@ const updateHistory = debounce(
async (formData, datasetId, isReplace, standalone, force, title) => { async (formData, datasetId, isReplace, standalone, force, title) => {
const payload = { ...formData }; const payload = { ...formData };
const chartId = formData.slice_id; const chartId = formData.slice_id;
const additionalParam = {};
if (chartId) {
additionalParam[URL_PARAMS.sliceId.name] = chartId;
} else {
additionalParam[URL_PARAMS.datasetId.name] = datasetId;
}
try { try {
let key; let key;
@ -183,6 +189,7 @@ const updateHistory = debounce(
standalone ? URL_PARAMS.standalone.name : null, standalone ? URL_PARAMS.standalone.name : null,
{ {
[URL_PARAMS.formDataKey.name]: key, [URL_PARAMS.formDataKey.name]: key,
...additionalParam,
}, },
force, force,
); );

View File

@ -20,6 +20,7 @@
import React from 'react'; import React from 'react';
import PropTypes from 'prop-types'; import PropTypes from 'prop-types';
import { t, styled, supersetTheme } from '@superset-ui/core'; import { t, styled, supersetTheme } from '@superset-ui/core';
import { getUrlParam } from 'src/utils/urlUtils';
import { Dropdown, Menu } from 'src/common/components'; import { Dropdown, Menu } from 'src/common/components';
import { Tooltip } from 'src/components/Tooltip'; import { Tooltip } from 'src/components/Tooltip';
@ -32,6 +33,7 @@ import { postForm } from 'src/explore/exploreUtils';
import Button from 'src/components/Button'; import Button from 'src/components/Button';
import ErrorAlert from 'src/components/ErrorMessage/ErrorAlert'; import ErrorAlert from 'src/components/ErrorMessage/ErrorAlert';
import WarningIconWithTooltip from 'src/components/WarningIconWithTooltip'; import WarningIconWithTooltip from 'src/components/WarningIconWithTooltip';
import { URL_PARAMS } from 'src/constants';
const propTypes = { const propTypes = {
actions: PropTypes.object.isRequired, actions: PropTypes.object.isRequired,
@ -182,6 +184,14 @@ class DatasourceControl extends React.PureComponent {
const { showChangeDatasourceModal, showEditDatasourceModal } = this.state; const { showChangeDatasourceModal, showEditDatasourceModal } = this.state;
const { datasource, onChange } = this.props; const { datasource, onChange } = this.props;
const isMissingDatasource = datasource.id == null; 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'; const isSqlSupported = datasource.type === 'table';
@ -244,7 +254,25 @@ class DatasourceControl extends React.PureComponent {
</Dropdown> </Dropdown>
</div> </div>
{/* missing dataset */} {/* 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"> <div className="error-alert">
<ErrorAlert <ErrorAlert
level="warning" level="warning"

View File

@ -747,6 +747,16 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods
if value: if value:
initial_form_data = json.loads(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( form_data, slc = get_form_data(
use_slice_data=True, initial_form_data=initial_form_data use_slice_data=True, initial_form_data=initial_form_data
) )