diff --git a/superset-frontend/spec/javascripts/chart/chartActions_spec.js b/superset-frontend/spec/javascripts/chart/chartActions_spec.js index c33c4b9cc01f..5c67509409d4 100644 --- a/superset-frontend/spec/javascripts/chart/chartActions_spec.js +++ b/superset-frontend/spec/javascripts/chart/chartActions_spec.js @@ -47,8 +47,8 @@ describe('chart actions', () => { beforeEach(() => { dispatch = sinon.spy(); urlStub = sinon - .stub(exploreUtils, 'getExploreUrlAndPayload') - .callsFake(() => ({ url: MOCK_URL, payload: {} })); + .stub(exploreUtils, 'getExploreUrl') + .callsFake(() => MOCK_URL); fakeMetadata = { useLegacyApi: true }; metadataRegistryStub = sinon .stub(chartlib, 'getChartMetadataRegistry') diff --git a/superset-frontend/spec/javascripts/explore/components/SaveModal_spec.jsx b/superset-frontend/spec/javascripts/explore/components/SaveModal_spec.jsx index 0d1c4a63c446..55e685bab06e 100644 --- a/superset-frontend/spec/javascripts/explore/components/SaveModal_spec.jsx +++ b/superset-frontend/spec/javascripts/explore/components/SaveModal_spec.jsx @@ -140,9 +140,7 @@ describe('SaveModal', () => { describe('saveOrOverwrite', () => { beforeEach(() => { - sinon - .stub(exploreUtils, 'getExploreUrlAndPayload') - .callsFake(() => ({ url: 'mockURL', payload: defaultProps.form_data })); + sinon.stub(exploreUtils, 'getExploreUrl').callsFake(() => 'mockURL'); sinon.stub(defaultProps.actions, 'saveSlice').callsFake(() => Promise.resolve({ @@ -155,7 +153,7 @@ describe('SaveModal', () => { }); afterEach(() => { - exploreUtils.getExploreUrlAndPayload.restore(); + exploreUtils.getExploreUrl.restore(); defaultProps.actions.saveSlice.restore(); }); diff --git a/superset-frontend/spec/javascripts/explore/utils_spec.jsx b/superset-frontend/spec/javascripts/explore/utils_spec.jsx index f8bf7fa7dde1..2318d6e51ea9 100644 --- a/superset-frontend/spec/javascripts/explore/utils_spec.jsx +++ b/superset-frontend/spec/javascripts/explore/utils_spec.jsx @@ -19,10 +19,7 @@ import sinon from 'sinon'; import URI from 'urijs'; -import { - getExploreUrlAndPayload, - getExploreLongUrl, -} from 'src/explore/exploreUtils'; +import { getExploreUrl, getExploreLongUrl } from 'src/explore/exploreUtils'; import * as hostNamesConfig from 'src/utils/hostNamesConfig'; describe('exploreUtils', () => { @@ -35,33 +32,31 @@ describe('exploreUtils', () => { expect(uri1.toString()).toBe(uri2.toString()); } - describe('getExploreUrlAndPayload', () => { + describe('getExploreUrl', () => { it('generates proper base url', () => { // This assertion is to show clearly the value of location.href // in the context of unit tests. expect(location.href).toBe('http://localhost/'); - const { url, payload } = getExploreUrlAndPayload({ + const url = getExploreUrl({ formData, endpointType: 'base', force: false, curUrl: 'http://superset.com', }); compareURI(URI(url), URI('/superset/explore/')); - expect(payload).toEqual(formData); }); it('generates proper json url', () => { - const { url, payload } = getExploreUrlAndPayload({ + const url = getExploreUrl({ formData, endpointType: 'json', force: false, curUrl: 'http://superset.com', }); compareURI(URI(url), URI('/superset/explore_json/')); - expect(payload).toEqual(formData); }); it('generates proper json forced url', () => { - const { url, payload } = getExploreUrlAndPayload({ + const url = getExploreUrl({ formData, endpointType: 'json', force: true, @@ -71,10 +66,9 @@ describe('exploreUtils', () => { URI(url), URI('/superset/explore_json/').search({ force: 'true' }), ); - expect(payload).toEqual(formData); }); it('generates proper csv URL', () => { - const { url, payload } = getExploreUrlAndPayload({ + const url = getExploreUrl({ formData, endpointType: 'csv', force: false, @@ -84,10 +78,9 @@ describe('exploreUtils', () => { URI(url), URI('/superset/explore_json/').search({ csv: 'true' }), ); - expect(payload).toEqual(formData); }); it('generates proper standalone URL', () => { - const { url, payload } = getExploreUrlAndPayload({ + const url = getExploreUrl({ formData, endpointType: 'standalone', force: false, @@ -97,10 +90,9 @@ describe('exploreUtils', () => { URI(url), URI('/superset/explore/').search({ standalone: 'true' }), ); - expect(payload).toEqual(formData); }); it('preserves main URLs params', () => { - const { url, payload } = getExploreUrlAndPayload({ + const url = getExploreUrl({ formData, endpointType: 'json', force: false, @@ -110,10 +102,9 @@ describe('exploreUtils', () => { URI(url), URI('/superset/explore_json/').search({ foo: 'bar' }), ); - expect(payload).toEqual(formData); }); it('generate proper save slice url', () => { - const { url, payload } = getExploreUrlAndPayload({ + const url = getExploreUrl({ formData, endpointType: 'json', force: false, @@ -123,20 +114,6 @@ describe('exploreUtils', () => { URI(url), URI('/superset/explore_json/').search({ foo: 'bar' }), ); - expect(payload).toEqual(formData); - }); - it('generate proper saveas slice url', () => { - const { url, payload } = getExploreUrlAndPayload({ - formData, - endpointType: 'json', - force: false, - curUrl: 'superset.com?foo=bar', - }); - compareURI( - URI(url), - URI('/superset/explore_json/').search({ foo: 'bar' }), - ); - expect(payload).toEqual(formData); }); }); @@ -158,48 +135,48 @@ describe('exploreUtils', () => { }); it('generate url to different domains', () => { - let url = getExploreUrlAndPayload({ + let url = getExploreUrl({ formData, endpointType: 'json', allowDomainSharding: true, - }).url; + }); // skip main domain for fetching chart if domain sharding is enabled // to leave main domain free for other calls like fav star, save change, etc. expect(url).toMatch(availableDomains[1]); - url = getExploreUrlAndPayload({ + url = getExploreUrl({ formData, endpointType: 'json', allowDomainSharding: true, - }).url; + }); expect(url).toMatch(availableDomains[2]); - url = getExploreUrlAndPayload({ + url = getExploreUrl({ formData, endpointType: 'json', allowDomainSharding: true, - }).url; + }); expect(url).toMatch(availableDomains[3]); // circle back to first available domain - url = getExploreUrlAndPayload({ + url = getExploreUrl({ formData, endpointType: 'json', allowDomainSharding: true, - }).url; + }); expect(url).toMatch(availableDomains[1]); }); it('not generate url to different domains without flag', () => { - let csvURL = getExploreUrlAndPayload({ + let csvURL = getExploreUrl({ formData, endpointType: 'csv', - }).url; + }); expect(csvURL).toMatch(availableDomains[0]); - csvURL = getExploreUrlAndPayload({ + csvURL = getExploreUrl({ formData, endpointType: 'csv', - }).url; + }); expect(csvURL).toMatch(availableDomains[0]); }); }); diff --git a/superset-frontend/spec/javascripts/sqllab/ExploreResultsButton_spec.jsx b/superset-frontend/spec/javascripts/sqllab/ExploreResultsButton_spec.jsx index 6bd0165715ea..fee2ba560e30 100644 --- a/superset-frontend/spec/javascripts/sqllab/ExploreResultsButton_spec.jsx +++ b/superset-frontend/spec/javascripts/sqllab/ExploreResultsButton_spec.jsx @@ -177,17 +177,14 @@ describe('ExploreResultsButton', () => { fetchMock.post(visualizeEndpoint, visualizationPayload); beforeEach(() => { - sinon.stub(exploreUtils, 'getExploreUrlAndPayload').callsFake(() => ({ - url: 'mockURL', - payload: { datasource: '107__table' }, - })); + sinon.stub(exploreUtils, 'getExploreUrl').callsFake(() => 'mockURL'); sinon.spy(exploreUtils, 'exportChart'); sinon .stub(wrapper.instance(), 'buildVizOptions') .callsFake(() => mockOptions); }); afterEach(() => { - exploreUtils.getExploreUrlAndPayload.restore(); + exploreUtils.getExploreUrl.restore(); exploreUtils.exportChart.restore(); wrapper.instance().buildVizOptions.restore(); fetchMock.reset(); diff --git a/superset-frontend/src/chart/chartAction.js b/superset-frontend/src/chart/chartAction.js index a26d67011347..3b46ae1498e6 100644 --- a/superset-frontend/src/chart/chartAction.js +++ b/superset-frontend/src/chart/chartAction.js @@ -20,14 +20,12 @@ /* eslint no-param-reassign: ["error", { "props": false }] */ import moment from 'moment'; import { t } from '@superset-ui/translation'; -import { - getChartBuildQueryRegistry, - getChartMetadataRegistry, -} from '@superset-ui/chart'; +import { getChartBuildQueryRegistry } from '@superset-ui/chart'; import { SupersetClient } from '@superset-ui/connection'; import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags'; import { - getExploreUrlAndPayload, + shouldUseLegacyApi, + getExploreUrl, getAnnotationJsonUrl, postForm, } from '../explore/exploreUtils'; @@ -213,7 +211,7 @@ function legacyChartDataRequest( dashboardId, requestParams, ) { - const { url, payload } = getExploreUrlAndPayload({ + const url = getExploreUrl({ formData, endpointType: 'json', force, @@ -224,7 +222,7 @@ function legacyChartDataRequest( const querySettings = { ...requestParams, url, - postPayload: { form_data: payload }, + postPayload: { form_data: formData }, }; const clientMethod = @@ -260,7 +258,7 @@ export function exploreJSON( const logStart = Logger.getTimestamp(); const controller = new AbortController(); - const { useLegacyApi } = getChartMetadataRegistry().get(formData.viz_type); + const useLegacyApi = shouldUseLegacyApi(formData); let requestParams = { signal: controller.signal, @@ -400,7 +398,7 @@ export function postChartFormData( export function redirectSQLLab(formData) { return dispatch => { - const { url } = getExploreUrlAndPayload({ + const url = getExploreUrl({ formData, endpointType: 'query', }); diff --git a/superset-frontend/src/explore/actions/saveModalActions.js b/superset-frontend/src/explore/actions/saveModalActions.js index 01797ee8ec06..a32f9563b87a 100644 --- a/superset-frontend/src/explore/actions/saveModalActions.js +++ b/superset-frontend/src/explore/actions/saveModalActions.js @@ -17,7 +17,7 @@ * under the License. */ import { SupersetClient } from '@superset-ui/connection'; -import { getExploreUrlAndPayload } from '../exploreUtils'; +import { getExploreUrl } from '../exploreUtils'; export const FETCH_DASHBOARDS_SUCCEEDED = 'FETCH_DASHBOARDS_SUCCEEDED'; export function fetchDashboardsSucceeded(choices) { @@ -62,7 +62,7 @@ export function removeSaveModalAlert() { export function saveSlice(formData, requestParams) { return dispatch => { - const { url, payload } = getExploreUrlAndPayload({ + const url = getExploreUrl({ formData, endpointType: 'base', force: false, @@ -70,7 +70,7 @@ export function saveSlice(formData, requestParams) { requestParams, }); - return SupersetClient.post({ url, postPayload: { form_data: payload } }) + return SupersetClient.post({ url, postPayload: { form_data: formData } }) .then(({ json }) => dispatch(saveSliceSuccess(json))) .catch(() => dispatch(saveSliceFailed())); }; diff --git a/superset-frontend/src/explore/components/DisplayQueryButton.jsx b/superset-frontend/src/explore/components/DisplayQueryButton.jsx index 72759b16cdc1..32d5f7630b2e 100644 --- a/superset-frontend/src/explore/components/DisplayQueryButton.jsx +++ b/superset-frontend/src/explore/components/DisplayQueryButton.jsx @@ -41,7 +41,7 @@ import { SupersetClient } from '@superset-ui/connection'; import getClientErrorObject from '../../utils/getClientErrorObject'; import CopyToClipboard from './../../components/CopyToClipboard'; -import { getExploreUrlAndPayload } from '../exploreUtils'; +import { getExploreUrl } from '../exploreUtils'; import Loading from '../../components/Loading'; import ModalTrigger from './../../components/ModalTrigger'; @@ -89,13 +89,13 @@ export class DisplayQueryButton extends React.PureComponent { } beforeOpen(endpointType) { this.setState({ isLoading: true }); - const { url, payload } = getExploreUrlAndPayload({ + const url = getExploreUrl({ formData: this.props.latestQueryFormData, endpointType, }); SupersetClient.post({ url, - postPayload: { form_data: payload }, + postPayload: { form_data: this.props.latestQueryFormData }, }) .then(({ json }) => { this.setState({ diff --git a/superset-frontend/src/explore/components/ExploreViewContainer.jsx b/superset-frontend/src/explore/components/ExploreViewContainer.jsx index 88c121c9503c..044372168bc9 100644 --- a/superset-frontend/src/explore/components/ExploreViewContainer.jsx +++ b/superset-frontend/src/explore/components/ExploreViewContainer.jsx @@ -27,7 +27,7 @@ import ExploreChartPanel from './ExploreChartPanel'; import ControlPanelsContainer from './ControlPanelsContainer'; import SaveModal from './SaveModal'; import QueryAndSaveBtns from './QueryAndSaveBtns'; -import { getExploreUrlAndPayload, getExploreLongUrl } from '../exploreUtils'; +import { getExploreUrl, getExploreLongUrl } from '../exploreUtils'; import { areObjectsEqual } from '../../reduxUtils'; import { getFormDataFromControls } from '../controlUtils'; import { chartPropShape } from '../../dashboard/util/propShapes'; @@ -231,9 +231,7 @@ class ExploreViewContainer extends React.Component { } addHistory({ isReplace = false, title }) { - const { payload } = getExploreUrlAndPayload({ - formData: this.props.form_data, - }); + const payload = { ...this.props.form_data }; const longUrl = getExploreLongUrl(this.props.form_data, null, false); try { if (isReplace) { diff --git a/superset-frontend/src/explore/exploreUtils.js b/superset-frontend/src/explore/exploreUtils.js index 42261f62e059..63706e851562 100644 --- a/superset-frontend/src/explore/exploreUtils.js +++ b/superset-frontend/src/explore/exploreUtils.js @@ -18,6 +18,7 @@ */ /* eslint camelcase: 0 */ import URI from 'urijs'; +import { getChartMetadataRegistry } from '@superset-ui/chart'; import { availableDomains } from '../utils/hostNamesConfig'; import { safeStringify } from '../utils/safeStringify'; @@ -63,15 +64,14 @@ export function getAnnotationJsonUrl(slice_id, form_data, isNative) { .toString(); } -export function getURIDirectory(formData, endpointType = 'base') { +export function getURIDirectory(endpointType = 'base') { // Building the directory part of the URI - let directory = '/superset/explore/'; if ( ['json', 'csv', 'query', 'results', 'samples'].indexOf(endpointType) >= 0 ) { - directory = '/superset/explore_json/'; + return '/superset/explore_json/'; } - return directory; + return '/superset/explore/'; } export function getExploreLongUrl( @@ -85,7 +85,7 @@ export function getExploreLongUrl( } const uri = new URI('/'); - const directory = getURIDirectory(formData, endpointType); + const directory = getURIDirectory(endpointType); const search = uri.search(true); Object.keys(extraSearch).forEach(key => { search[key] = extraSearch[key]; @@ -107,7 +107,12 @@ export function getExploreLongUrl( return url; } -export function getExploreUrlAndPayload({ +export function shouldUseLegacyApi(formData) { + const { useLegacyApi } = getChartMetadataRegistry().get(formData.viz_type); + return useLegacyApi || false; +} + +export function getExploreUrl({ formData, endpointType = 'base', force = false, @@ -134,7 +139,7 @@ export function getExploreUrlAndPayload({ uri = URI(URI(curUrl).search()); } - const directory = getURIDirectory(formData, endpointType); + const directory = getURIDirectory(endpointType); // Building the querystring (search) part of the URI const search = uri.search(true); @@ -178,13 +183,7 @@ export function getExploreUrlAndPayload({ } }); } - uri = uri.search(search).directory(directory); - const payload = { ...formData }; - - return { - url: uri.toString(), - payload, - }; + return uri.search(search).directory(directory).toString(); } export function postForm(url, payload, target = '_blank') { @@ -213,10 +212,10 @@ export function postForm(url, payload, target = '_blank') { } export function exportChart(formData, endpointType) { - const { url, payload } = getExploreUrlAndPayload({ + const url = getExploreUrl({ formData, endpointType, allowDomainSharding: false, }); - postForm(url, payload); + postForm(url, formData); }