Skip to content
This repository has been archived by the owner on Dec 10, 2021. It is now read-only.

feat(query): add functions to wrap api calls with typings #555

Merged
merged 6 commits into from
Jun 3, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 8 additions & 0 deletions packages/superset-ui-query/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,13 @@
"homepage": "https://github.com/apache-superset/superset-ui#readme",
"publishConfig": {
"access": "public"
},
"devDependencies": {
"@types/fetch-mock": "^6.0.4",
"fetch-mock": "^7.2.5",
"node-fetch": "^2.2.0"
},
"peerDependencies": {
"@superset-ui/connection": "^0.13.25"
}
}
28 changes: 28 additions & 0 deletions packages/superset-ui-query/src/api/legacy/fetchExploreJson.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { SupersetClientInterface, SupersetClient, RequestConfig } from '@superset-ui/connection';
import { QueryFormData } from '../../types/QueryFormData';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank god for this custom type... i have mild ptsd from formData lol

import { LegacyChartDataResponse } from './types';

export interface Params {
client?: SupersetClientInterface;
method?: 'GET' | 'POST';
requestConfig?: Partial<RequestConfig>;
url?: string;
formData: QueryFormData;
}

export default function fetchExploreJson({
client = SupersetClient,
method = 'POST',
requestConfig,
url = '/superset/explore_json/',
formData,
}: Params) {
const fetchFunc = method === 'GET' ? client.get : client.post;

return fetchFunc({
...requestConfig,
// TODO: Have to transform formData as query string for GET

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have an example for what a query string could look like?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a utility function in incubator-superset that has to be ported here.

url,
postPayload: { form_data: formData },
} as RequestConfig).then(({ json }) => json as LegacyChartDataResponse);
}
5 changes: 5 additions & 0 deletions packages/superset-ui-query/src/api/legacy/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { V1ChartDataResponseResult } from '../v1/types';

export interface LegacyChartDataResponse extends Omit<V1ChartDataResponseResult, 'data'> {
data: Record<string, unknown>[] | Record<string, unknown>;
}
24 changes: 24 additions & 0 deletions packages/superset-ui-query/src/api/v1/postChartData.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { SupersetClientInterface, SupersetClient, RequestConfig } from '@superset-ui/connection';
import { QueryContext } from '../../types/Query';
import { V1ChartDataResponse } from './types';

export interface Params {
client?: SupersetClientInterface;
requestConfig?: Partial<RequestConfig>;
queryContext: QueryContext;
}

export default function postChartData({
client = SupersetClient,
requestConfig,
queryContext,
}: Params) {
return client
.post({
...requestConfig,
endpoint: '/api/v1/chart/data',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify(queryContext),
} as RequestConfig)
.then(({ json }) => json as V1ChartDataResponse);
}
17 changes: 17 additions & 0 deletions packages/superset-ui-query/src/api/v1/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/* eslint-disable camelcase */
export interface V1ChartDataResponseResult {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about the V1 prefix. Having a prefix makes it look temporary or deprecated. Was ChartDataResponseResult used elsewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with the doubts about variable names. This was the hardest part about this PR.
I had trouble with naming and went back and forth whether to version or not. Still not fully satisfied TBH. On one hand I want to avoid naming collision if there is v2. On the other hand the V1xxx looks a bit weird.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @rusackas @suddjian if you have thoughts about naming.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think marking it as v1 is useful in cases where the interface specifically relates to behavior of the v1 api.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still don't feel strongly either way so I will merge and adjust later if anybody has more comments.

cache_key: string | null;
cache_timeout: number | null;
cache_dttm: string | null;
data: Record<string, unknown>[];
error: string | null;
is_cached: boolean;
query: string;
rowcount: number;
stacktrace: string | null;
status: 'stopped' | 'failed' | 'pending' | 'running' | 'scheduled' | 'success' | 'timed_out';
}

export interface V1ChartDataResponse {
result: V1ChartDataResponseResult[];
}
7 changes: 7 additions & 0 deletions packages/superset-ui-query/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,10 @@ export * from './types/Column';
export * from './types/Datasource';
export * from './types/Metric';
export * from './types/Query';

// API Calls
export { default as fetchExploreJson } from './api/legacy/fetchExploreJson';
export { default as postChartData } from './api/v1/postChartData';

export * from './api/legacy/types';
export * from './api/v1/types';
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const LOGIN_GLOB = 'glob:*superset/csrf_token/*'; // eslint-disable-line import/prefer-default-export
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import fetchMock from 'fetch-mock';
import { SupersetClient } from '@superset-ui/connection';
import { LOGIN_GLOB } from '../fixtures/constants';
import { fetchExploreJson } from '../../../src';

describe('fetchExploreJson()', () => {
beforeAll(() => {
fetchMock.get(LOGIN_GLOB, { csrf_token: '1234' });
SupersetClient.reset();
SupersetClient.configure().init();
});

afterEach(fetchMock.restore);

it('returns a promise of LegacyChartDataResponse', () => {
fetchMock.post('glob:*/superset/explore_json/', {
field1: 'abc',
field2: 'def',
});

return expect(
fetchExploreJson({
formData: {
granularity: 'minute',
viz_type: 'word_cloud',
datasource: '1__table',
},
}),
).resolves.toEqual({
field1: 'abc',
field2: 'def',
});
});
it('uses GET when specified', () => {
fetchMock.get('glob:*/superset/explore_json/', {
field1: 'abc',
field2: 'def',
});

return expect(
fetchExploreJson({
method: 'GET',
formData: {
granularity: 'minute',
viz_type: 'word_cloud',
datasource: '1__table',
},
}),
).resolves.toEqual({
field1: 'abc',
field2: 'def',
});
});
});
42 changes: 42 additions & 0 deletions packages/superset-ui-query/test/api/v1/postChartData.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import fetchMock from 'fetch-mock';
import { SupersetClient } from '@superset-ui/connection';
import { LOGIN_GLOB } from '../fixtures/constants';
import { postChartData, buildQueryContext } from '../../../src';

describe('postChartData()', () => {
beforeAll(() => {
fetchMock.get(LOGIN_GLOB, { csrf_token: '1234' });
SupersetClient.reset();
SupersetClient.configure().init();
});

afterEach(fetchMock.restore);

it('returns a promise of ChartDataResponse', () => {
fetchMock.post('glob:*/api/v1/chart/data', {
result: [
{
field1: 'abc',
field2: 'def',
},
],
});

return expect(
postChartData({
queryContext: buildQueryContext({
granularity: 'minute',
viz_type: 'word_cloud',
datasource: '1__table',
}),
}),
).resolves.toEqual({
result: [
{
field1: 'abc',
field2: 'def',
},
],
});
});
});