From 1fc5005d0d26e99a19a5c1ccdb5738d380df455c Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Thu, 14 Oct 2021 17:10:20 -0700 Subject: [PATCH 1/2] remove unnecessary parameters from the explore url --- .../dashboard/components/gridComponents/Chart.jsx | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx b/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx index 60cbab7295303..4f6066e03a890 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx @@ -230,7 +230,18 @@ export default class Chart extends React.Component { }); }; - getChartUrl = () => getExploreLongUrl(this.props.formData); + getChartUrl = () => { + const filters = this.props.formData?.extra_form_data?.filters; + // remove formData params that we don't need in the explore url + // this should be superseded by some sort of "exploration context" system + const formData = { + ...this.props.formData, + extra_form_data: { filters }, + native_filters: undefined, + dataMask: undefined, + }; + return getExploreLongUrl(formData, null, false); + }; exportCSV(isFullCSV = false) { this.props.logEvent(LOG_ACTIONS_EXPORT_CSV_DASHBOARD_CHART, { From ee336880978f65ddedd6b3b54b059660141c1bd2 Mon Sep 17 00:00:00 2001 From: David Aaron Suddjian Date: Fri, 15 Oct 2021 11:43:57 -0700 Subject: [PATCH 2/2] refactor, test --- .../components/gridComponents/Chart.jsx | 13 +---------- .../exploreUtils/getExploreLongUrl.test.ts | 23 +++++++++++++++++++ .../src/explore/exploreUtils/index.js | 9 +++++++- 3 files changed, 32 insertions(+), 13 deletions(-) diff --git a/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx b/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx index 4f6066e03a890..8801cf3a21e62 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx @@ -230,18 +230,7 @@ export default class Chart extends React.Component { }); }; - getChartUrl = () => { - const filters = this.props.formData?.extra_form_data?.filters; - // remove formData params that we don't need in the explore url - // this should be superseded by some sort of "exploration context" system - const formData = { - ...this.props.formData, - extra_form_data: { filters }, - native_filters: undefined, - dataMask: undefined, - }; - return getExploreLongUrl(formData, null, false); - }; + getChartUrl = () => getExploreLongUrl(this.props.formData, null, false); exportCSV(isFullCSV = false) { this.props.logEvent(LOG_ACTIONS_EXPORT_CSV_DASHBOARD_CHART, { diff --git a/superset-frontend/src/explore/exploreUtils/getExploreLongUrl.test.ts b/superset-frontend/src/explore/exploreUtils/getExploreLongUrl.test.ts index 6c69e469bec9c..c8d78b901408a 100644 --- a/superset-frontend/src/explore/exploreUtils/getExploreLongUrl.test.ts +++ b/superset-frontend/src/explore/exploreUtils/getExploreLongUrl.test.ts @@ -90,3 +90,26 @@ test('Get url when endpointType:results and allowOverflow:false', () => { '/superset/explore_json/?same=any-string&form_data=%7B%22datasource%22%3A%22datasource%22%2C%22viz_type%22%3A%22viz_type%22%7D', ); }); + +test('Get url from a dashboard', () => { + const formData = { + ...createParams().formData, + // these params should get filtered out + extra_form_data: { + filters: { + col: 'foo', + op: 'IN', + val: ['bar'], + }, + }, + dataMask: { + 'NATIVE_FILTER-bqEoUsEPe': { + id: 'NATIVE_FILTER-bqEoUsEPe', + lots: 'of other stuff here too', + }, + }, + }; + expect(getExploreLongUrl(formData, null, false)).toBe( + '/superset/explore/?form_data=%7B%22datasource%22%3A%22datasource%22%2C%22viz_type%22%3A%22viz_type%22%2C%22extra_form_data%22%3A%7B%22filters%22%3A%7B%22col%22%3A%22foo%22%2C%22op%22%3A%22IN%22%2C%22val%22%3A%5B%22bar%22%5D%7D%7D%7D', + ); +}); diff --git a/superset-frontend/src/explore/exploreUtils/index.js b/superset-frontend/src/explore/exploreUtils/index.js index ddcd95b82a437..87720a7e3eb46 100644 --- a/superset-frontend/src/explore/exploreUtils/index.js +++ b/superset-frontend/src/explore/exploreUtils/index.js @@ -18,6 +18,7 @@ */ import { useCallback, useEffect } from 'react'; +import { omit } from 'lodash'; /* eslint camelcase: 0 */ import URI from 'urijs'; import { @@ -103,13 +104,19 @@ export function getExploreLongUrl( return null; } + // remove formData params that we don't need in the explore url. + // These are present when generating explore urls from the dashboard page. + // This should be superseded by some sort of "exploration context" system + // where form data and other context is referenced by id. + const trimmedFormData = omit(formData, ['dataMask', 'url_params']); + const uri = new URI('/'); const directory = getURIDirectory(endpointType); const search = uri.search(true); Object.keys(extraSearch).forEach(key => { search[key] = extraSearch[key]; }); - search.form_data = safeStringify(formData); + search.form_data = safeStringify(trimmedFormData); if (endpointType === URL_PARAMS.standalone.name) { search.standalone = DashboardStandaloneMode.HIDE_NAV; }