Skip to content

Commit

Permalink
fix(embedded): CSV download for chart (#20261)
Browse files Browse the repository at this point in the history
* move postForm to superset client

* lint

* fix lint

* fix type

* update tests

* add tests

* add test for form submit

* add test for request form

* lint

* fix test

* fix tests

* more tests

* more tests

* test

* lint

* more test for postForm

* lint

* Update superset-frontend/packages/superset-ui-core/test/connection/SupersetClientClass.test.ts

Co-authored-by: David Aaron Suddjian <1858430+suddjian@users.noreply.github.com>

* update tests

* remove useless test

* make test cover happy

* make test cover happy

* make test cover happy

* make codecov happy

* make codecov happy

Co-authored-by: David Aaron Suddjian <1858430+suddjian@users.noreply.github.com>
  • Loading branch information
lilykuang and suddjian committed Jun 18, 2022
1 parent f53018c commit ab9f72f
Show file tree
Hide file tree
Showing 12 changed files with 190 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ const SupersetClient: SupersetClientInterface = {
init: force => getInstance().init(force),
isAuthenticated: () => getInstance().isAuthenticated(),
post: request => getInstance().post(request),
postForm: (...args) => getInstance().postForm(...args),
put: request => getInstance().put(request),
reAuthenticate: () => getInstance().reAuthenticate(),
request: request => getInstance().request(request),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,36 @@ export default class SupersetClientClass {
return this.getCSRFToken();
}

async postForm(url: string, payload: Record<string, any>, target = '_blank') {
if (url) {
await this.ensureAuth();
const hiddenForm = document.createElement('form');
hiddenForm.action = url;
hiddenForm.method = 'POST';
hiddenForm.target = target;
const payloadWithToken: Record<string, any> = {
...payload,
csrf_token: this.csrfToken!,
};

if (this.guestToken) {
payloadWithToken.guest_token = this.guestToken;
}

Object.entries(payloadWithToken).forEach(([key, value]) => {
const data = document.createElement('input');
data.type = 'hidden';
data.name = key;
data.value = value;
hiddenForm.appendChild(data);
});

document.body.appendChild(hiddenForm);
hiddenForm.submit();
document.body.removeChild(hiddenForm);
}
}

async reAuthenticate() {
return this.init(true);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ export interface SupersetClientInterface
| 'delete'
| 'get'
| 'post'
| 'postForm'
| 'put'
| 'request'
| 'init'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,21 +30,23 @@ describe('SupersetClient', () => {

afterEach(SupersetClient.reset);

it('exposes reset, configure, init, get, post, isAuthenticated, and reAuthenticate methods', () => {
it('exposes reset, configure, init, get, post, postForm, isAuthenticated, and reAuthenticate methods', () => {
expect(typeof SupersetClient.configure).toBe('function');
expect(typeof SupersetClient.init).toBe('function');
expect(typeof SupersetClient.get).toBe('function');
expect(typeof SupersetClient.post).toBe('function');
expect(typeof SupersetClient.postForm).toBe('function');
expect(typeof SupersetClient.isAuthenticated).toBe('function');
expect(typeof SupersetClient.reAuthenticate).toBe('function');
expect(typeof SupersetClient.request).toBe('function');
expect(typeof SupersetClient.reset).toBe('function');
});

it('throws if you call init, get, post, isAuthenticated, or reAuthenticate before configure', () => {
it('throws if you call init, get, post, postForm, isAuthenticated, or reAuthenticate before configure', () => {
expect(SupersetClient.init).toThrow();
expect(SupersetClient.get).toThrow();
expect(SupersetClient.post).toThrow();
expect(SupersetClient.postForm).toThrow();
expect(SupersetClient.isAuthenticated).toThrow();
expect(SupersetClient.reAuthenticate).toThrow();
expect(SupersetClient.request).toThrow();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -605,4 +605,107 @@ describe('SupersetClientClass', () => {
}
});
});

describe('.postForm()', () => {
const protocol = 'https:';
const host = 'host';
const mockPostFormEndpoint = '/post_form/url';
const mockPostFormUrl = `${protocol}//${host}${mockPostFormEndpoint}`;
const guestToken = 'test-guest-token';
const postFormPayload = { number: 123, array: [1, 2, 3] };

let authSpy: jest.SpyInstance;
let client: SupersetClientClass;
let appendChild: any;
let removeChild: any;
let submit: any;
let createElement: any;

beforeEach(async () => {
client = new SupersetClientClass({ protocol, host });
authSpy = jest.spyOn(SupersetClientClass.prototype, 'ensureAuth');
await client.init();
appendChild = jest.fn();
removeChild = jest.fn();
submit = jest.fn();
createElement = jest.fn(() => ({
appendChild: jest.fn(),
submit,
}));

document.createElement = createElement as any;
document.body.appendChild = appendChild;
document.body.removeChild = removeChild;
});

afterEach(() => {
jest.restoreAllMocks();
});

it('makes postForm request', async () => {
await client.postForm(mockPostFormUrl, {});

const hiddenForm = createElement.mock.results[0].value;
const csrfTokenInput = createElement.mock.results[1].value;

expect(createElement.mock.calls).toHaveLength(2);

expect(hiddenForm.action).toBe(mockPostFormUrl);
expect(hiddenForm.method).toBe('POST');
expect(hiddenForm.target).toBe('_blank');

expect(csrfTokenInput.type).toBe('hidden');
expect(csrfTokenInput.name).toBe('csrf_token');
expect(csrfTokenInput.value).toBe(1234);

expect(appendChild.mock.calls).toHaveLength(1);
expect(removeChild.mock.calls).toHaveLength(1);
expect(authSpy).toHaveBeenCalledTimes(1);
});

it('makes postForm request with guest token', async () => {
client = new SupersetClientClass({ protocol, host, guestToken });
await client.init();

await client.postForm(mockPostFormUrl, {});

const guestTokenInput = createElement.mock.results[2].value;

expect(createElement.mock.calls).toHaveLength(3);

expect(guestTokenInput.type).toBe('hidden');
expect(guestTokenInput.name).toBe('guest_token');
expect(guestTokenInput.value).toBe(guestToken);

expect(appendChild.mock.calls).toHaveLength(1);
expect(removeChild.mock.calls).toHaveLength(1);
expect(authSpy).toHaveBeenCalledTimes(1);
});

it('makes postForm request with payload', async () => {
await client.postForm(mockPostFormUrl, { form_data: postFormPayload });

const postFormPayloadInput = createElement.mock.results[1].value;

expect(createElement.mock.calls).toHaveLength(3);

expect(postFormPayloadInput.type).toBe('hidden');
expect(postFormPayloadInput.name).toBe('form_data');
expect(postFormPayloadInput.value).toBe(postFormPayload);

expect(appendChild.mock.calls).toHaveLength(1);
expect(removeChild.mock.calls).toHaveLength(1);
expect(submit.mock.calls).toHaveLength(1);
expect(authSpy).toHaveBeenCalledTimes(1);
});

it('should do nothing when url is empty string', async () => {
const result = await client.postForm('', {});
expect(result).toBeUndefined();
expect(createElement.mock.calls).toHaveLength(0);
expect(appendChild.mock.calls).toHaveLength(0);
expect(removeChild.mock.calls).toHaveLength(0);
expect(authSpy).toHaveBeenCalledTimes(0);
});
});
});
6 changes: 4 additions & 2 deletions superset-frontend/src/components/Chart/chartAction.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import {
getExploreUrl,
getLegacyEndpointType,
buildV1ChartDataPayload,
postForm,
shouldUseLegacyApi,
getChartDataUri,
} from 'src/explore/exploreUtils';
Expand All @@ -40,6 +39,7 @@ import { addDangerToast } from 'src/components/MessageToasts/actions';
import { logEvent } from 'src/logger/actions';
import { Logger, LOG_ACTIONS_LOAD_CHART } from 'src/logger/LogUtils';
import { getClientErrorObject } from 'src/utils/getClientErrorObject';
import { safeStringify } from 'src/utils/safeStringify';
import { allowCrossDomain as domainShardingEnabled } from 'src/utils/hostNamesConfig';
import { updateDataMask } from 'src/dataMask/actions';
import { waitForAsyncData } from 'src/middleware/asyncEvent';
Expand Down Expand Up @@ -563,7 +563,9 @@ export function redirectSQLLab(formData) {
datasourceKey: formData.datasource,
sql: json.result[0].query,
};
postForm(redirectUrl, payload);
SupersetClient.postForm(redirectUrl, {
form_data: safeStringify(payload),
});
})
.catch(() =>
dispatch(addDangerToast(t('An error occurred while loading the SQL'))),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import React from 'react';
import { render, screen, act } from 'spec/helpers/testing-library';
import userEvent from '@testing-library/user-event';
import { SupersetClient, DatasourceType } from '@superset-ui/core';
import * as Utils from 'src/explore/exploreUtils';
import DatasourceControl from '.';

const SupersetClientGet = jest.spyOn(SupersetClient, 'get');
Expand Down Expand Up @@ -142,7 +141,7 @@ test('Click on Edit dataset', async () => {

test('Click on View in SQL Lab', async () => {
const props = createProps();
const postFormSpy = jest.spyOn(Utils, 'postForm');
const postFormSpy = jest.spyOn(SupersetClient, 'postForm');
postFormSpy.mockImplementation(jest.fn());

render(<DatasourceControl {...props} />, {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,13 @@
*/
import React from 'react';
import PropTypes from 'prop-types';
import { t, styled, withTheme, DatasourceType } from '@superset-ui/core';
import {
DatasourceType,
SupersetClient,
styled,
t,
withTheme,
} from '@superset-ui/core';
import { getUrlParam } from 'src/utils/urlUtils';

import { AntdDropdown } from 'src/components';
Expand All @@ -30,13 +36,13 @@ import {
ChangeDatasourceModal,
DatasourceModal,
} from 'src/components/Datasource';
import { SaveDatasetModal } from 'src/SqlLab/components/SaveDatasetModal';
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';
import { isUserAdmin } from 'src/dashboard/util/findPermission';
import { SaveDatasetModal } from 'src/SqlLab/components/SaveDatasetModal';
import { safeStringify } from 'src/utils/safeStringify';

const propTypes = {
actions: PropTypes.object.isRequired,
Expand Down Expand Up @@ -193,7 +199,9 @@ class DatasourceControl extends React.PureComponent {
datasourceKey: `${datasource.id}__${datasource.type}`,
sql: datasource.sql,
};
postForm('/superset/sqllab/', payload);
SupersetClient.postForm('/superset/sqllab/', {
form_data: safeStringify(payload),
});
}
break;

Expand Down
17 changes: 15 additions & 2 deletions superset-frontend/src/explore/exploreUtils/exploreUtils.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,14 @@ import sinon from 'sinon';
import URI from 'urijs';
import {
buildV1ChartDataPayload,
exploreChart,
getExploreUrl,
shouldUseLegacyApi,
getSimpleSQLExpression,
shouldUseLegacyApi,
} from 'src/explore/exploreUtils';
import { DashboardStandaloneMode } from 'src/dashboard/util/constants';
import * as hostNamesConfig from 'src/utils/hostNamesConfig';
import { getChartMetadataRegistry } from '@superset-ui/core';
import { getChartMetadataRegistry, SupersetClient } from '@superset-ui/core';

describe('exploreUtils', () => {
const { location } = window;
Expand Down Expand Up @@ -275,4 +276,16 @@ describe('exploreUtils', () => {
);
});
});

describe('.exploreChart()', () => {
it('postForm', () => {
const postFormSpy = jest.spyOn(SupersetClient, 'postForm');
postFormSpy.mockImplementation(jest.fn());

exploreChart({
formData: { ...formData, viz_type: 'my_custom_viz' },
});
expect(postFormSpy).toBeCalledTimes(1);
});
});
});
31 changes: 4 additions & 27 deletions superset-frontend/src/explore/exploreUtils/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
ensureIsArray,
getChartBuildQueryRegistry,
getChartMetadataRegistry,
SupersetClient,
} from '@superset-ui/core';
import { availableDomains } from 'src/utils/hostNamesConfig';
import { safeStringify } from 'src/utils/safeStringify';
Expand Down Expand Up @@ -234,31 +235,6 @@ export const buildV1ChartDataPayload = ({
export const getLegacyEndpointType = ({ resultType, resultFormat }) =>
resultFormat === 'csv' ? resultFormat : resultType;

export function postForm(url, payload, target = '_blank') {
if (!url) {
return;
}

const hiddenForm = document.createElement('form');
hiddenForm.action = url;
hiddenForm.method = 'POST';
hiddenForm.target = target;
const token = document.createElement('input');
token.type = 'hidden';
token.name = 'csrf_token';
token.value = (document.getElementById('csrf_token') || {}).value;
hiddenForm.appendChild(token);
const data = document.createElement('input');
data.type = 'hidden';
data.name = 'form_data';
data.value = safeStringify(payload);
hiddenForm.appendChild(data);

document.body.appendChild(hiddenForm);
hiddenForm.submit();
document.body.removeChild(hiddenForm);
}

export const exportChart = ({
formData,
resultFormat = 'json',
Expand Down Expand Up @@ -286,7 +262,8 @@ export const exportChart = ({
ownState,
});
}
postForm(url, payload);

SupersetClient.postForm(url, { form_data: safeStringify(payload) });
};

export const exploreChart = formData => {
Expand All @@ -295,7 +272,7 @@ export const exploreChart = formData => {
endpointType: 'base',
allowDomainSharding: false,
});
postForm(url, formData);
SupersetClient.postForm(url, { form_data: safeStringify(formData) });
};

export const useDebouncedEffect = (effect, delay, deps) => {
Expand Down
4 changes: 3 additions & 1 deletion superset/security/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -1383,7 +1383,9 @@ def get_guest_user_from_request(self, req: Request) -> Optional[GuestUser]:
:return: A guest user object
"""
raw_token = req.headers.get(current_app.config["GUEST_TOKEN_HEADER_NAME"])
raw_token = req.headers.get(
current_app.config["GUEST_TOKEN_HEADER_NAME"]
) or req.form.get("guest_token")
if raw_token is None:
return None

Expand Down

0 comments on commit ab9f72f

Please sign in to comment.