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 all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
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,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();
lilykuang marked this conversation as resolved.
Show resolved Hide resolved
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
Loading