Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(embedded): CSV download for chart #20261

Merged
merged 31 commits into from
Jun 18, 2022
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
77e8970
move postForm to superset client
lilykuang Jun 3, 2022
3748b98
lint
lilykuang Jun 3, 2022
90087c5
fix lint
lilykuang Jun 3, 2022
ebca313
fix type
lilykuang Jun 3, 2022
e2bc61c
Merge branch 'master' of https://github.com/preset-io/incubator-super…
lilykuang Jun 6, 2022
c79bcfa
update tests
lilykuang Jun 6, 2022
32409c9
Merge branch 'master' of https://github.com/preset-io/incubator-super…
lilykuang Jun 7, 2022
4c0a407
add tests
lilykuang Jun 7, 2022
4eaeb22
add test for form submit
lilykuang Jun 8, 2022
aa8a55e
add test for request form
lilykuang Jun 8, 2022
dc49040
lint
lilykuang Jun 8, 2022
44c4d5a
fix test
lilykuang Jun 8, 2022
3260fec
fix tests
lilykuang Jun 8, 2022
590c0e6
more tests
lilykuang Jun 8, 2022
63a47d1
more tests
lilykuang Jun 8, 2022
f3bf4bb
test
lilykuang Jun 9, 2022
5db8bec
Merge branch 'master' of https://github.com/preset-io/superset into f…
lilykuang Jun 9, 2022
143a588
lint
lilykuang Jun 9, 2022
e77f12e
more test for postForm
lilykuang Jun 9, 2022
41ca02a
Merge branch 'master' of https://github.com/preset-io/superset into f…
lilykuang Jun 9, 2022
4f916d1
Merge branch 'master' of https://github.com/preset-io/incubator-super…
lilykuang Jun 15, 2022
2acf6e8
lint
lilykuang Jun 15, 2022
019198b
Update superset-frontend/packages/superset-ui-core/test/connection/Su…
lilykuang Jun 17, 2022
6dbed55
update tests
lilykuang Jun 17, 2022
361bd6a
remove useless test
lilykuang Jun 17, 2022
661f3fc
make test cover happy
lilykuang Jun 18, 2022
8461938
Merge branch 'master' of https://github.com/preset-io/incubator-super…
lilykuang Jun 18, 2022
129c360
make test cover happy
lilykuang Jun 18, 2022
78bd39e
make test cover happy
lilykuang Jun 18, 2022
88a6dfa
make codecov happy
lilykuang Jun 18, 2022
d7974f1
make codecov happy
lilykuang Jun 18, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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,37 @@ export default class SupersetClientClass {
return this.getCSRFToken();
}

async postForm(url: string, payload: Record<string, any>, target = '_blank') {
if (!url) {
return;
}
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,114 @@ 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] };

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

const createElement = jest.fn(() => ({
appendChild: jest.fn(),
submit: jest.fn(),
}));

document.createElement = createElement as any;
const appendChild = jest.fn();
const removeChild = jest.fn();
document.body.appendChild = appendChild;
document.body.removeChild = removeChild;
lilykuang marked this conversation as resolved.
Show resolved Hide resolved

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);
});

it('makes postForm request with guesr token', async () => {
lilykuang marked this conversation as resolved.
Show resolved Hide resolved
const client = new SupersetClientClass({ protocol, host, guestToken });
await client.init();

const createElement = jest.fn(() => ({
appendChild: jest.fn(),
submit: jest.fn(),
}));

document.createElement = createElement as any;
const appendChild = jest.fn();
const removeChild = jest.fn();
document.body.appendChild = appendChild;
document.body.removeChild = removeChild;

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);
});

it('makes postForm request with payload', async () => {
const client = new SupersetClientClass({ protocol, host });
await client.init();
const submit = jest.fn();

const createElement = jest.fn(() => ({
appendChild: jest.fn(),
submit,
}));

document.createElement = createElement as any;
const appendChild = jest.fn();
const removeChild = jest.fn();
document.body.appendChild = appendChild;
document.body.removeChild = removeChild;

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);
});

it('should do nothing when url is empty string', async () => {
const client = new SupersetClientClass({ protocol, host });
await client.init();
const result = await client.postForm('', {});
expect(result).toBeUndefined();
lilykuang marked this conversation as resolved.
Show resolved Hide resolved
});
});
});
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
Loading