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

chore: Migrate /superset/tables/* to API v1 #22501

Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1,221 changes: 810 additions & 411 deletions docs/static/resources/openapi.json

Large diffs are not rendered by default.

Expand Up @@ -82,7 +82,7 @@ describe('SqlLab query panel', () => {
});

it.skip('successfully saves a query', () => {
cy.intercept('superset/tables/**').as('getTables');
cy.intercept('api/v1/database/**/tables/**').as('getTables');
cy.intercept('savedqueryviewapi/**').as('getSavedQuery');

const query =
Expand Down
Expand Up @@ -54,7 +54,7 @@ jest.mock('src/SqlLab/components/SqlEditorLeftBar', () => () => (
const MOCKED_SQL_EDITOR_HEIGHT = 500;

fetchMock.get('glob:*/api/v1/database/*', { result: [] });
fetchMock.get('glob:*/superset/tables/*', { options: [] });
fetchMock.get('glob:*/api/v1/database/*/tables/*', { options: [] });
fetchMock.post('glob:*/sql_json/*', { result: [] });

const middlewares = [thunk];
Expand Down
Expand Up @@ -42,14 +42,16 @@ const mockStore = configureStore(middlewares);
const store = mockStore(initialState);

fetchMock.get('glob:*/api/v1/database/*/schemas/?*', { result: [] });
fetchMock.get('glob:*/superset/tables/**', {
options: [
{
label: 'ab_user',
value: 'ab_user',
},
],
tableLength: 1,
fetchMock.get('glob:*/api/v1/database/*/tables/*', {
result: {
options: [
{
label: 'ab_user',
value: 'ab_user',
},
],
tableLength: 1,
},
});

const renderAndWait = (props, store) =>
Expand Down
Expand Up @@ -51,12 +51,14 @@ const getSchemaMockFunction = async () =>
const getTableMockFunction = async () =>
({
json: {
options: [
{ label: 'table_a', value: 'table_a' },
{ label: 'table_b', value: 'table_b' },
{ label: 'table_c', value: 'table_c' },
{ label: 'table_d', value: 'table_d' },
],
result: {
options: [
{ label: 'table_a', value: 'table_a' },
{ label: 'table_b', value: 'table_b' },
{ label: 'table_c', value: 'table_c' },
{ label: 'table_d', value: 'table_d' },
],
},
},
} as any);

Expand Down
66 changes: 36 additions & 30 deletions superset-frontend/src/hooks/apiResources/tables.test.ts
Expand Up @@ -23,47 +23,51 @@ import { useTables } from './tables';

const fakeApiResult = {
json: {
options: [
{
id: 1,
name: 'fake api result1',
label: 'fake api label1',
},
{
id: 2,
name: 'fake api result2',
label: 'fake api label2',
},
],
tableLength: 2,
result: {
options: [
{
id: 1,
name: 'fake api result1',
label: 'fake api label1',
},
{
id: 2,
name: 'fake api result2',
label: 'fake api label2',
},
],
tableLength: 2,
},
},
};

const fakeHasMoreApiResult = {
json: {
options: [
{
id: 1,
name: 'fake api result1',
label: 'fake api label1',
},
{
id: 2,
name: 'fake api result2',
label: 'fake api label2',
},
],
tableLength: 4,
result: {
options: [
{
id: 1,
name: 'fake api result1',
label: 'fake api label1',
},
{
id: 2,
name: 'fake api result2',
label: 'fake api label2',
},
],
tableLength: 4,
},
},
};

const expectedData = {
...fakeApiResult.json,
...fakeApiResult.json.result,
hasMore: false,
};

const expectedHasMoreData = {
...fakeHasMoreApiResult.json,
...fakeHasMoreApiResult.json.result,
hasMore: true,
};

Expand Down Expand Up @@ -103,15 +107,17 @@ describe('useTables hook', () => {
});
expect(SupersetClient.get).toHaveBeenCalledTimes(1);
expect(SupersetClient.get).toHaveBeenCalledWith({
endpoint: `/superset/tables/${expectDbId}/${expectedSchema}/${forceRefresh}/`,
endpoint: `/api/v1/database/${expectDbId}/tables/?q=(force:!${
forceRefresh ? 't' : 'f'
},schema_name:${expectedSchema})`,
});
expect(result.current.data).toEqual(expectedData);
await act(async () => {
result.current.refetch();
});
expect(SupersetClient.get).toHaveBeenCalledTimes(2);
expect(SupersetClient.get).toHaveBeenCalledWith({
endpoint: `/superset/tables/${expectDbId}/${expectedSchema}/true/`,
endpoint: `/api/v1/database/${expectDbId}/tables/?q=(force:!t,schema_name:${expectedSchema})`,
});
expect(result.current.data).toEqual(expectedData);
});
Expand Down
23 changes: 17 additions & 6 deletions superset-frontend/src/hooks/apiResources/tables.ts
Expand Up @@ -18,6 +18,7 @@
*/
import { useRef } from 'react';
import { useQuery, UseQueryOptions } from 'react-query';
import rison from 'rison';
import { SupersetClient } from '@superset-ui/core';

export type FetchTablesQueryParams = {
Expand All @@ -39,11 +40,16 @@ export interface Table {
}

type QueryData = {
json: { options: Table[]; tableLength: number };
json: {
result: {
options: Table[];
tableLength: number;
};
};
Copy link
Member

Choose a reason for hiding this comment

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

isn't this:

type QueryData = {
  json: {
    count: number;
    result: {
      options: Table[];
    };
  };

response: Response;
};

export type Data = QueryData['json'] & {
export type Data = QueryData['json']['result'] & {
hasMore: boolean;
};

Expand All @@ -53,10 +59,15 @@ export function fetchTables({
forceRefresh,
}: FetchTablesQueryParams) {
const encodedSchema = schema ? encodeURIComponent(schema) : '';
const params = rison.encode({
force: forceRefresh,
schema_name: encodedSchema,
});

// TODO: Would be nice to add pagination in a follow-up. Needs endpoint changes.
const endpoint = `/superset/tables/${
const endpoint = `/api/v1/database/${
dbId ?? 'undefined'
}/${encodedSchema}/${forceRefresh}/`;
}/tables/?q=${params}`;
return SupersetClient.get({ endpoint }) as Promise<QueryData>;
}

Expand All @@ -72,8 +83,8 @@ export function useTables(options: Params) {
() => fetchTables({ ...params, forceRefresh: forceRefreshRef.current }),
{
select: ({ json }) => ({
...json,
hasMore: json.tableLength > json.options.length,
...json.result,
hasMore: json.result.tableLength > json.result.options.length,
}),
enabled: Boolean(dbId && schema),
onSuccess,
Expand Down
Expand Up @@ -24,7 +24,7 @@ import LeftPanel from 'src/views/CRUD/data/dataset/AddDataset/LeftPanel';

const databasesEndpoint = 'glob:*/api/v1/database/?q*';
const schemasEndpoint = 'glob:*/api/v1/database/*/schemas*';
const tablesEndpoint = 'glob:*/superset/tables*';
const tablesEndpoint = 'glob:*/api/v1/database/*/tables/?q*';

fetchMock.get(databasesEndpoint, {
count: 2,
Expand Down Expand Up @@ -136,12 +136,14 @@ fetchMock.get(schemasEndpoint, {
});

fetchMock.get(tablesEndpoint, {
tableLength: 3,
options: [
{ value: 'Sheet1', type: 'table', extra: null },
{ value: 'Sheet2', type: 'table', extra: null },
{ value: 'Sheet3', type: 'table', extra: null },
],
result: {
tableLength: 3,
options: [
{ value: 'Sheet1', type: 'table', extra: null },
{ value: 'Sheet2', type: 'table', extra: null },
{ value: 'Sheet3', type: 'table', extra: null },
],
},
});

const mockFun = jest.fn();
Expand Down
Expand Up @@ -17,6 +17,7 @@
* under the License.
*/
import React, { useEffect, useState, SetStateAction, Dispatch } from 'react';
import rison from 'rison';
import {
SupersetClient,
t,
Expand Down Expand Up @@ -177,15 +178,17 @@ export default function LeftPanel({
const getTablesList = (url: string) => {
SupersetClient.get({ url })
.then(({ json }) => {
const options: TableOption[] = json.options.map((table: Table) => {
const option: TableOption = {
value: table.value,
label: <TableOption table={table} />,
text: table.label,
};
const options: TableOption[] = json.result.options.map(
(table: Table) => {
const option: TableOption = {
value: table.value,
label: <TableOption table={table} />,
text: table.label,
};

return option;
});
return option;
},
);

setTableOptions(options);
setLoadTables(false);
Expand Down Expand Up @@ -213,9 +216,12 @@ export default function LeftPanel({

useEffect(() => {
if (loadTables) {
const endpoint = encodeURI(
`/superset/tables/${dbId}/${encodedSchema}/${refresh}/`,
);
const params = rison.encode({
force: refresh,
schema_name: encodedSchema,
});

const endpoint = `/api/v1/database/${dbId}/tables/?q=${params}`;
getTablesList(endpoint);
}
}, [loadTables]);
Expand Down
1 change: 1 addition & 0 deletions superset/constants.py
Expand Up @@ -115,6 +115,7 @@ class RouteMethod: # pylint: disable=too-few-public-methods
"put": "write",
"related": "read",
"related_objects": "read",
"tables": "read",
"schemas": "read",
"select_star": "read",
"table_metadata": "read",
Expand Down