Skip to content

Commit

Permalink
chore(api v1): Deprecate datasource/save and datasource/get endpoints (
Browse files Browse the repository at this point in the history
  • Loading branch information
jfrag1 committed Apr 19, 2023
1 parent 818a1d4 commit 44557f5
Show file tree
Hide file tree
Showing 23 changed files with 242 additions and 70 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { DatasourceType } from '@superset-ui/core';
import { Dataset } from './types';

export const TestDataset: Dataset = {
column_format: {},
column_formats: {},
columns: [
{
advanced_data_type: undefined,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export interface Dataset {
type: DatasourceType;
columns: ColumnMeta[];
metrics: Metric[];
column_format: Record<string, string>;
column_formats: Record<string, string>;
verbose_map: Record<string, string>;
main_dttm_col: string;
// eg. ['["ds", true]', 'ds [asc]']
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ describe('columnChoices()', () => {
},
],
verbose_map: {},
column_format: { fiz: 'NUMERIC', about: 'STRING', foo: 'DATE' },
column_formats: { fiz: 'NUMERIC', about: 'STRING', foo: 'DATE' },
datasource_name: 'my_datasource',
description: 'this is my datasource',
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ describe('defineSavedMetrics', () => {
time_grain_sqla: 'P1D',
columns: [],
verbose_map: {},
column_format: {},
column_formats: {},
datasource_name: 'my_datasource',
description: 'this is my datasource',
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ const datasourceData = {

const DATASOURCES_ENDPOINT =
'glob:*/api/v1/dataset/?q=(order_column:changed_on_delta_humanized,order_direction:desc,page:0,page_size:25)';
const DATASOURCE_ENDPOINT = `glob:*/datasource/get/${datasourceData.type}/${datasourceData.id}`;
const DATASOURCE_ENDPOINT = `glob:*/api/v1/dataset/${datasourceData.id}`;
const DATASOURCE_PAYLOAD = { new: 'data' };

const INFO_ENDPOINT = 'glob:*/api/v1/dataset/_info?*';
Expand Down Expand Up @@ -112,6 +112,6 @@ describe('ChangeDatasourceModal', () => {
});
await waitForComponentToPaint(wrapper);

expect(fetchMock.calls(/datasource\/get\/table\/7/)).toHaveLength(1);
expect(fetchMock.calls(/api\/v1\/dataset\/7/)).toHaveLength(1);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -173,10 +173,12 @@ const ChangeDatasourceModal: FunctionComponent<ChangeDatasourceModalProps> = ({

const handleChangeConfirm = () => {
SupersetClient.get({
endpoint: `/datasource/get/${confirmedDataset?.type}/${confirmedDataset?.id}/`,
endpoint: `/api/v1/dataset/${confirmedDataset?.id}`,
})
.then(({ json }) => {
onDatasourceSave(json);
// eslint-disable-next-line no-param-reassign
json.result.type = 'table';
onDatasourceSave(json.result);
onChange(`${confirmedDataset?.id}__table`);
})
.catch(response => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ const datasource = mockDatasource['7__table'];

const SAVE_ENDPOINT = 'glob:*/api/v1/dataset/7';
const SAVE_PAYLOAD = { new: 'data' };
const SAVE_DATASOURCE_ENDPOINT = 'glob:*/datasource/save/';
const SAVE_DATASOURCE_ENDPOINT = 'glob:*/api/v1/dataset/7';
const GET_DATASOURCE_ENDPOINT = SAVE_DATASOURCE_ENDPOINT;

const mockedProps = {
datasource,
Expand Down Expand Up @@ -96,7 +97,8 @@ describe('DatasourceModal', () => {

it('saves on confirm', async () => {
const callsP = fetchMock.post(SAVE_ENDPOINT, SAVE_PAYLOAD);
fetchMock.post(SAVE_DATASOURCE_ENDPOINT, {});
fetchMock.put(SAVE_DATASOURCE_ENDPOINT, {});
fetchMock.get(GET_DATASOURCE_ENDPOINT, {});
act(() => {
wrapper
.find('button[data-test="datasource-modal-save"]')
Expand All @@ -111,7 +113,11 @@ describe('DatasourceModal', () => {
okButton.simulate('click');
});
await waitForComponentToPaint(wrapper);
const expected = ['http://localhost/datasource/save/'];
// one call to PUT, then one to GET
const expected = [
'http://localhost/api/v1/dataset/7',
'http://localhost/api/v1/dataset/7',
];
expect(callsP._calls.map(call => call[0])).toEqual(
expected,
); /* eslint no-underscore-dangle: 0 */
Expand Down
98 changes: 70 additions & 28 deletions superset-frontend/src/components/Datasource/DatasourceModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -96,39 +96,81 @@ const DatasourceModal: FunctionComponent<DatasourceModalProps> = ({
currentDatasource.schema;

setIsSaving(true);
SupersetClient.post({
endpoint: '/datasource/save/',
postPayload: {
data: {
...currentDatasource,
cache_timeout:
currentDatasource.cache_timeout === ''
? null
: currentDatasource.cache_timeout,
schema,
metrics: currentDatasource?.metrics?.map(
(metric: Record<string, unknown>) => ({
...metric,
SupersetClient.put({
endpoint: `/api/v1/dataset/${currentDatasource.id}`,
jsonPayload: {
table_name: currentDatasource.table_name,
database_id: currentDatasource.database?.id,
sql: currentDatasource.sql,
filter_select_enabled: currentDatasource.filter_select_enabled,
fetch_values_predicate: currentDatasource.fetch_values_predicate,
schema,
description: currentDatasource.description,
main_dttm_col: currentDatasource.main_dttm_col,
offset: currentDatasource.offset,
default_endpoint: currentDatasource.default_endpoint,
cache_timeout:
currentDatasource.cache_timeout === ''
? null
: currentDatasource.cache_timeout,
is_sqllab_view: currentDatasource.is_sqllab_view,
template_params: currentDatasource.template_params,
extra: currentDatasource.extra,
is_managed_externally: currentDatasource.is_managed_externally,
external_url: currentDatasource.external_url,
metrics: currentDatasource?.metrics?.map(
(metric: Record<string, unknown>) => {
const metricBody: any = {
expression: metric.expression,
description: metric.description,
metric_name: metric.metric_name,
metric_type: metric.metric_type,
d3format: metric.d3format,
verbose_name: metric.verbose_name,
warning_text: metric.warning_text,
uuid: metric.uuid,
extra: buildExtraJsonObject(metric),
}),
),
columns: currentDatasource?.columns?.map(
(column: Record<string, unknown>) => ({
...column,
extra: buildExtraJsonObject(column),
}),
),
type: currentDatasource.type || currentDatasource.datasource_type,
owners: currentDatasource.owners.map(
(o: Record<string, number>) => o.value || o.id,
),
},
};
if (!Number.isNaN(Number(metric.id))) {
metricBody.id = metric.id;
}
return metricBody;
},
),
columns: currentDatasource?.columns?.map(
(column: Record<string, unknown>) => ({
id: column.id,
column_name: column.column_name,
type: column.type,
advanced_data_type: column.advanced_data_type,
verbose_name: column.verbose_name,
description: column.description,
expression: column.expression,
filterable: column.filterable,
groupby: column.groupby,
is_active: column.is_active,
is_dttm: column.is_dttm,
python_date_format: column.python_date_format,
uuid: column.uuid,
extra: buildExtraJsonObject(column),
}),
),
owners: currentDatasource.owners.map(
(o: Record<string, number>) => o.value || o.id,
),
},
})
.then(({ json }) => {
.then(() => {
addSuccessToast(t('The dataset has been saved'));
return SupersetClient.get({
endpoint: `/api/v1/dataset/${currentDatasource?.id}`,
});
})
.then(({ json }) => {
// eslint-disable-next-line no-param-reassign
json.result.type = 'table';
onDatasourceSave({
...json,
...json.result,
owners: currentDatasource.owners,
});
onHide();
Expand Down
2 changes: 1 addition & 1 deletion superset-frontend/src/dashboard/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export const PLACEHOLDER_DATASOURCE: Datasource = {
columns: [],
column_types: [],
metrics: [],
column_format: {},
column_formats: {},
verbose_map: {},
main_dttm_col: '',
description: '',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ const CURRENT_DATASOURCE = {
type: DatasourceType.Table,
columns: [],
metrics: [],
column_format: {},
column_formats: {},
verbose_map: {},
main_dttm_col: '__timestamp',
// eg. ['["ds", true]', 'ds [asc]']
Expand All @@ -47,7 +47,7 @@ const NEW_DATASOURCE = {
type: DatasourceType.Table,
columns: [],
metrics: [],
column_format: {},
column_formats: {},
verbose_map: {},
main_dttm_col: '__timestamp',
// eg. ['["ds", true]', 'ds [asc]']
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,20 @@ const createProps = (overrides: JsonObject = {}) => ({
});

async function openAndSaveChanges(datasource: any) {
fetchMock.post('glob:*/datasource/save/', datasource, {
overwriteRoutes: true,
});
fetchMock.put(
'glob:*/api/v1/dataset/*',
{},
{
overwriteRoutes: true,
},
);
fetchMock.get(
'glob:*/api/v1/dataset/*',
{ result: datasource },
{
overwriteRoutes: true,
},
);
userEvent.click(screen.getByTestId('datasource-menu-trigger'));
userEvent.click(await screen.findByTestId('edit-dataset'));
userEvent.click(await screen.findByTestId('datasource-modal-save'));
Expand Down Expand Up @@ -154,7 +165,7 @@ test('Should show SQL Lab for sql_lab role', async () => {

test('Click on Swap dataset option', async () => {
const props = createProps();
SupersetClientGet.mockImplementation(
SupersetClientGet.mockImplementationOnce(
async ({ endpoint }: { endpoint: string }) => {
if (endpoint.includes('_info')) {
return {
Expand Down Expand Up @@ -182,7 +193,7 @@ test('Click on Swap dataset option', async () => {

test('Click on Edit dataset', async () => {
const props = createProps();
SupersetClientGet.mockImplementation(
SupersetClientGet.mockImplementationOnce(
async () => ({ json: { result: [] } } as any),
);
render(<DatasourceControl {...props} />, {
Expand All @@ -206,7 +217,7 @@ test('Edit dataset should be disabled when user is not admin', async () => {
// @ts-expect-error
props.user.roles = {};
props.datasource.owners = [];
SupersetClientGet.mockImplementation(
SupersetClientGet.mockImplementationOnce(
async () => ({ json: { result: [] } } as any),
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ describe('controlUtils', () => {
type: DatasourceType.Table,
columns: [{ column_name: 'a' }],
metrics: [{ metric_name: 'first' }, { metric_name: 'second' }],
column_format: {},
column_formats: {},
verbose_map: {},
main_dttm_col: '',
datasource_name: '1__table',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ const sampleDatasource: Dataset = {
{ column_name: 'sample_column_4' },
],
metrics: [{ metric_name: 'saved_metric_2' }],
column_format: {},
column_formats: {},
verbose_map: {},
main_dttm_col: '',
datasource_name: 'Sample Dataset',
Expand Down
4 changes: 2 additions & 2 deletions superset-frontend/src/explore/fixtures.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ export const exploreInitialData: ExplorePageInitialData = {
type: DatasourceType.Table,
columns: [{ column_name: 'a' }],
metrics: [{ metric_name: 'first' }, { metric_name: 'second' }],
column_format: {},
column_formats: {},
verbose_map: {},
main_dttm_col: '',
datasource_name: '8__table',
Expand All @@ -153,7 +153,7 @@ export const fallbackExploreInitialData: ExplorePageInitialData = {
type: DatasourceType.Table,
columns: [],
metrics: [],
column_format: {},
column_formats: {},
verbose_map: {},
main_dttm_col: '',
owners: [],
Expand Down
2 changes: 1 addition & 1 deletion superset-frontend/src/utils/getDatasourceUid.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const TEST_DATASOURCE = {
type: DatasourceType.Table,
columns: [],
metrics: [],
column_format: {},
column_formats: {},
verbose_map: {},
main_dttm_col: '__timestamp',
// eg. ['["ds", true]', 'ds [asc]']
Expand Down
Loading

0 comments on commit 44557f5

Please sign in to comment.