Skip to content

Commit

Permalink
feat: Enable new dataset creation flow II (apache#22835)
Browse files Browse the repository at this point in the history
  • Loading branch information
lyndsiWilliams authored and PawankumarES committed Feb 13, 2023
1 parent c84f2ce commit edac72e
Show file tree
Hide file tree
Showing 23 changed files with 236 additions and 375 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -186,27 +186,6 @@ test('should render a create dataset infobox', async () => {
expect(infoboxText).toBeVisible();
});

test('should render a save dataset modal when "Create a dataset" is clicked', async () => {
const newProps = {
...props,
datasource: {
...datasource,
type: DatasourceType.Query,
},
};
render(<DatasourcePanel {...newProps} />, { useRedux: true, useDnd: true });

const createButton = await screen.findByRole('button', {
name: /create a dataset/i,
});

userEvent.click(createButton);

const saveDatasetModalTitle = screen.getByText(/save or overwrite dataset/i);

expect(saveDatasetModalTitle).toBeVisible();
});

test('should not render a save dataset modal when datasource is not query or dataset', async () => {
const newProps = {
...props,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,9 @@ const ColumnSelectPopover = ({
}, []);

const setDatasetAndClose = () => {
if (setDatasetModal) setDatasetModal(true);
if (setDatasetModal) {
setDatasetModal(true);
}
onClose();
};

Expand Down
5 changes: 1 addition & 4 deletions superset-frontend/src/pages/ChartCreation/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -337,10 +337,7 @@ export class ChartCreation extends React.PureComponent<
const isButtonDisabled = this.isBtnDisabled();
const datasetHelpText = this.state.canCreateDataset ? (
<span data-test="dataset-write">
<Link
to="/tablemodelview/list/#create"
data-test="add-chart-new-dataset"
>
<Link to="/dataset/add/" data-test="add-chart-new-dataset">
{t('Add a dataset')}
</Link>
{` ${t('or')} `}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import React, { useState, useMemo, useEffect } from 'react';
import rison from 'rison';
import { useSelector } from 'react-redux';
import { useQueryParams, BooleanParam } from 'use-query-params';
import { LocalStorageKeys, setItem } from 'src/utils/localStorageHelpers';

import Loading from 'src/components/Loading';
import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags';
Expand Down Expand Up @@ -157,6 +158,9 @@ function DatabaseList({ addDangerToast, addSuccessToast }: DatabaseListProps) {
refreshData();
addSuccessToast(t('Deleted: %s', dbName));

// Delete user-selected db from local storage
setItem(LocalStorageKeys.db, null);

// Close delete modal
setDatabaseCurrentlyDeleting(null);
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,14 @@ jest.mock('src/components/Icons/Icon', () => ({
StyledIcon: () => <span />,
}));

const mockHistoryPush = jest.fn();
jest.mock('react-router-dom', () => ({
...jest.requireActual('react-router-dom'),
useHistory: () => ({
push: mockHistoryPush,
}),
}));

const dbProps = {
show: true,
database_name: 'my database',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ interface DatabaseModalProps {
show: boolean;
databaseId: number | undefined; // If included, will go into edit mode
dbEngine: string | undefined; // if included goto step 2 with engine already set
history?: any;
}

export enum ActionType {
Expand Down Expand Up @@ -521,6 +522,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
show,
databaseId,
dbEngine,
history,
}) => {
const [db, setDB] = useReducer<
Reducer<Partial<DatabaseObject> | null, DBReducerActionType>
Expand Down Expand Up @@ -653,6 +655,16 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
onHide();
};

const redirectURL = (url: string) => {
/* TODO (lyndsiWilliams): This check and passing history
as a prop can be removed once SQL Lab is in the SPA */
if (!isEmpty(history)) {
history?.push(url);
} else {
window.location.href = url;
}
};

// Database import logic
const {
state: {
Expand Down Expand Up @@ -1345,23 +1357,21 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
const renderCTABtns = () => (
<StyledBtns>
<Button
// eslint-disable-next-line no-return-assign
buttonStyle="secondary"
onClick={() => {
setLoading(true);
fetchAndSetDB();
window.location.href = '/tablemodelview/list#create';
redirectURL('/dataset/add/');
}}
>
{t('CREATE DATASET')}
</Button>
<Button
buttonStyle="secondary"
// eslint-disable-next-line no-return-assign
onClick={() => {
setLoading(true);
fetchAndSetDB();
window.location.href = `/superset/sqllab/?db=true`;
redirectURL(`/superset/sqllab/?db=true`);
}}
>
{t('QUERY DATA IN SQL LAB')}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,14 @@ import React from 'react';
import { render, screen } from 'spec/helpers/testing-library';
import AddDataset from 'src/views/CRUD/data/dataset/AddDataset';

const mockHistoryPush = jest.fn();
jest.mock('react-router-dom', () => ({
...jest.requireActual('react-router-dom'),
useHistory: () => ({
push: mockHistoryPush,
}),
}));

describe('AddDataset', () => {
it('renders a blank state AddDataset', async () => {
render(<AddDataset />, { useRedux: true });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export const exampleDataset: DatasetObject[] = [
id: 1,
database_name: 'test_database',
owners: [1],
backend: 'test_backend',
},
schema: 'test_schema',
dataset_name: 'example_dataset',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@
* under the License.
*/
import React, { useEffect, useState, useRef } from 'react';
import { SupersetClient } from '@superset-ui/core';
import { SupersetClient, logging, t } from '@superset-ui/core';
import { DatasetObject } from 'src/views/CRUD/data/dataset/AddDataset/types';
import { addDangerToast } from 'src/components/MessageToasts/actions';
import DatasetPanel from './DatasetPanel';
import { ITableColumn, IDatabaseTable, isIDatabaseTable } from './types';

Expand Down Expand Up @@ -94,9 +95,17 @@ const DatasetPanelWrapper = ({
setColumnList([]);
setHasColumns?.(false);
setHasError(true);
// eslint-disable-next-line no-console
console.error(
`The API response from ${path} does not match the IDatabaseTable interface.`,
addDangerToast(
t(
'The API response from %s does not match the IDatabaseTable interface.',
path,
),
);
logging.error(
t(
'The API response from %s does not match the IDatabaseTable interface.',
path,
),
);
}
} catch (error) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,14 @@ import React from 'react';
import { render, screen } from 'spec/helpers/testing-library';
import Footer from 'src/views/CRUD/data/dataset/AddDataset/Footer';

const mockHistoryPush = jest.fn();
jest.mock('react-router-dom', () => ({
...jest.requireActual('react-router-dom'),
useHistory: () => ({
push: mockHistoryPush,
}),
}));

const mockedProps = {
url: 'realwebsite.com',
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
* under the License.
*/
import React from 'react';
import { useHistory } from 'react-router-dom';
import Button from 'src/components/Button';
import { t } from '@superset-ui/core';
import { useSingleViewResource } from 'src/views/CRUD/hooks';
Expand Down Expand Up @@ -49,12 +50,12 @@ const LOG_ACTIONS = [
];

function Footer({
url,
datasetObject,
addDangerToast,
hasColumns = false,
datasets,
}: FooterProps) {
const history = useHistory();
const { createResource } = useSingleViewResource<Partial<DatasetObject>>(
'dataset',
t('dataset'),
Expand All @@ -72,11 +73,6 @@ function Footer({

return LOG_ACTIONS[value];
};
const goToPreviousUrl = () => {
// this is a placeholder url until the final feature gets implemented
// at that point we will be passing in the url of the previous location.
window.location.href = url;
};

const cancelButtonOnClick = () => {
if (!datasetObject) {
Expand All @@ -85,7 +81,7 @@ function Footer({
const logAction = createLogAction(datasetObject);
logEvent(logAction, datasetObject);
}
goToPreviousUrl();
history.goBack();
};

const tooltipText = t('Select a database table.');
Expand All @@ -104,13 +100,13 @@ function Footer({
if (typeof response === 'number') {
logEvent(LOG_ACTIONS_DATASET_CREATION_SUCCESS, datasetObject);
// When a dataset is created the response we get is its ID number
goToPreviousUrl();
history.push(`/chart/add/?dataset=${datasetObject.table_name}`);
}
});
}
};

const CREATE_DATASET_TEXT = t('Create Dataset');
const CREATE_DATASET_TEXT = t('Create dataset and create chart');
const disabledCheck =
!datasetObject?.table_name ||
!hasColumns ||
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import fetchMock from 'fetch-mock';
import userEvent from '@testing-library/user-event';
import { render, screen, waitFor } from 'spec/helpers/testing-library';
import LeftPanel from 'src/views/CRUD/data/dataset/AddDataset/LeftPanel';
import { exampleDataset } from 'src/views/CRUD/data/dataset/AddDataset/DatasetPanel/fixtures';

const databasesEndpoint = 'glob:*/api/v1/database/?q*';
const schemasEndpoint = 'glob:*/api/v1/database/*/schemas*';
Expand Down Expand Up @@ -136,8 +137,8 @@ fetchMock.get(schemasEndpoint, {
});

fetchMock.get(tablesEndpoint, {
count: 3,
result: [
tableLength: 3,
options: [
{ value: 'Sheet1', type: 'table', extra: null },
{ value: 'Sheet2', type: 'table', extra: null },
{ value: 'Sheet3', type: 'table', extra: null },
Expand Down Expand Up @@ -181,31 +182,29 @@ test('does not render blank state if there is nothing selected', async () => {
});

test('renders list of options when user clicks on schema', async () => {
render(<LeftPanel setDataset={mockFun} schema="schema_a" dbId={1} />, {
render(<LeftPanel setDataset={mockFun} dataset={exampleDataset[0]} />, {
useRedux: true,
});

// Click 'test-postgres' database to access schemas
const databaseSelect = screen.getByRole('combobox', {
name: 'Select database or type database name',
});
// Schema select should be disabled until database is selected
const schemaSelect = screen.getByRole('combobox', {
name: /select schema or type schema name/i,
});
userEvent.click(databaseSelect);
expect(await screen.findByText('test-postgres')).toBeInTheDocument();
expect(schemaSelect).toBeDisabled();
userEvent.click(screen.getByText('test-postgres'));

// Wait for schema field to be enabled
// Schema select will be automatically populated if there is only one schema
const schemaSelect = screen.getByRole('combobox', {
name: /select schema or type schema name/i,
});
await waitFor(() => {
expect(schemaSelect).toBeEnabled();
});
});

test('searches for a table name', async () => {
render(<LeftPanel setDataset={mockFun} schema="schema_a" dbId={1} />, {
render(<LeftPanel setDataset={mockFun} dataset={exampleDataset[0]} />, {
useRedux: true,
});

Expand Down Expand Up @@ -245,9 +244,8 @@ test('renders a warning icon when a table name has a pre-existing dataset', asyn
render(
<LeftPanel
setDataset={mockFun}
schema="schema_a"
dbId={1}
datasets={['Sheet2']}
dataset={exampleDataset[0]}
datasetNames={['Sheet2']}
/>,
{
useRedux: true,
Expand Down
Loading

0 comments on commit edac72e

Please sign in to comment.