Skip to content

Commit

Permalink
chore: remove supersetTheme with withTheme (#17069)
Browse files Browse the repository at this point in the history
* remove supersettheme

* migrate Datasourceeditor to rtl

* lint fix

* remove file

* fix import

* fix test

* fix lint

* spread tests and remove unused code

* fix lint
  • Loading branch information
pkdotson committed Oct 27, 2021
1 parent 65f1644 commit 93f59e0
Show file tree
Hide file tree
Showing 2 changed files with 123 additions and 142 deletions.
22 changes: 11 additions & 11 deletions superset-frontend/src/components/Datasource/DatasourceEditor.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import Card from 'src/components/Card';
import Alert from 'src/components/Alert';
import Badge from 'src/components/Badge';
import shortid from 'shortid';
import { styled, SupersetClient, t, supersetTheme } from '@superset-ui/core';
import { styled, SupersetClient, t, withTheme } from '@superset-ui/core';
import { Select } from 'src/components';
import { FormLabel } from 'src/components/Form';
import Button from 'src/components/Button';
Expand Down Expand Up @@ -97,7 +97,7 @@ const StyledBadge = styled(Badge)`
`;

const EditLockContainer = styled.div`
font-size: ${supersetTheme.typography.sizes.s}px;
font-size: ${({ theme }) => theme.typography.sizes.s}px;
display: flex;
align-items: center;
a {
Expand Down Expand Up @@ -828,7 +828,7 @@ class DatasourceEditor extends React.PureComponent {
);
}

renderSourceFieldset() {
renderSourceFieldset(theme) {
const { datasource } = this.state;
return (
<div>
Expand Down Expand Up @@ -989,13 +989,9 @@ class DatasourceEditor extends React.PureComponent {
<EditLockContainer>
<span role="button" tabIndex={0} onClick={this.onChangeEditMode}>
{this.state.isEditMode ? (
<Icons.LockUnlocked
iconColor={supersetTheme.colors.grayscale.base}
/>
<Icons.LockUnlocked iconColor={theme.colors.grayscale.base} />
) : (
<Icons.LockLocked
iconColor={supersetTheme.colors.grayscale.base}
/>
<Icons.LockLocked iconColor={theme.colors.grayscale.base} />
)}
</span>
{!this.state.isEditMode && (
Expand Down Expand Up @@ -1166,6 +1162,8 @@ class DatasourceEditor extends React.PureComponent {
const { datasource, activeTabKey } = this.state;
const { metrics } = datasource;
const sortedMetrics = metrics?.length ? this.sortMetrics(metrics) : [];
const { theme } = this.props;

return (
<DatasourceContainer>
{this.renderErrors()}
Expand All @@ -1190,7 +1188,7 @@ class DatasourceEditor extends React.PureComponent {
defaultActiveKey={activeTabKey}
>
<Tabs.TabPane key={0} tab={t('Source')}>
{this.renderSourceFieldset()}
{this.renderSourceFieldset(theme)}
</Tabs.TabPane>
<Tabs.TabPane
tab={
Expand Down Expand Up @@ -1283,4 +1281,6 @@ class DatasourceEditor extends React.PureComponent {
DatasourceEditor.defaultProps = defaultProps;
DatasourceEditor.propTypes = propTypes;

export default withToasts(DatasourceEditor);
const DataSourceComponent = withTheme(DatasourceEditor);

export default withToasts(DataSourceComponent);
243 changes: 112 additions & 131 deletions superset-frontend/src/components/Datasource/DatasourceEditor.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,12 @@
* under the License.
*/
import React from 'react';
import { shallow } from 'enzyme';
import configureStore from 'redux-mock-store';
import fetchMock from 'fetch-mock';
import thunk from 'redux-thunk';
import userEvent from '@testing-library/user-event';
import { render, screen } from 'spec/helpers/testing-library';

import { Radio } from 'src/components/Radio';

import Icons from 'src/components/Icons';
import Tabs from 'src/components/Tabs';
import DatasourceEditor from 'src/components/Datasource/DatasourceEditor';
import Field from 'src/CRUD/Field';
import mockDatasource from 'spec/fixtures/mockDatasource';
import * as featureFlags from 'src/featureFlags';
import TableSelector from 'src/components/TableSelector';

const props = {
datasource: mockDatasource['7__table'],
Expand All @@ -43,39 +33,33 @@ const props = {
const DATASOURCE_ENDPOINT = 'glob:*/datasource/external_metadata_by_name/*';

describe('DatasourceEditor', () => {
const mockStore = configureStore([thunk]);
const store = mockStore({});
fetchMock.get(DATASOURCE_ENDPOINT, []);

let wrapper;
let el;
let inst;
let isFeatureEnabledMock;

beforeEach(() => {
el = <DatasourceEditor {...props} store={store} />;
wrapper = shallow(el).dive();
inst = wrapper.instance();
});

afterEach(() => {
wrapper.unmount();
el = <DatasourceEditor {...props} />;
render(el, { useRedux: true });
});

it('is valid', () => {
expect(React.isValidElement(el)).toBe(true);
});

it('renders Tabs', () => {
expect(wrapper.find('#table-tabs')).toExist();
expect(screen.getByTestId('edit-dataset-tabs')).toBeInTheDocument();
});

it('makes an async request', () =>
new Promise(done => {
wrapper.setState({ activeTabKey: 2 });
const syncButton = wrapper.find('.sync-from-source');
expect(syncButton).toHaveLength(1);
syncButton.simulate('click');
const columnsTab = screen.getByTestId('collection-tab-Columns');

userEvent.click(columnsTab);
const syncButton = screen.getByText(/sync columns from source/i);
expect(syncButton).toBeInTheDocument();

userEvent.click(syncButton);

setTimeout(() => {
expect(fetchMock.calls(DATASOURCE_ENDPOINT)).toHaveLength(1);
Expand All @@ -84,135 +68,134 @@ describe('DatasourceEditor', () => {
}, 0);
}));

it('to add, remove and modify columns accordingly', () => {
const columns = [
{
name: 'ds',
type: 'DATETIME',
nullable: true,
default: '',
primary_key: false,
is_dttm: true,
},
{
name: 'gender',
type: 'VARCHAR(32)',
nullable: true,
default: '',
primary_key: false,
is_dttm: false,
},
{
name: 'new_column',
type: 'VARCHAR(10)',
nullable: true,
default: '',
primary_key: false,
is_dttm: false,
},
];

const numCols = props.datasource.columns.length;
expect(inst.state.databaseColumns).toHaveLength(numCols);
inst.updateColumns(columns);
expect(inst.state.databaseColumns).toEqual(
expect.arrayContaining([
{
type: 'DATETIME',
description: null,
filterable: false,
verbose_name: null,
is_dttm: true,
expression: '',
groupby: false,
column_name: 'ds',
},
{
type: 'VARCHAR(32)',
description: null,
filterable: true,
verbose_name: null,
is_dttm: false,
expression: '',
groupby: true,
column_name: 'gender',
},
expect.objectContaining({
column_name: 'new_column',
type: 'VARCHAR(10)',
}),
]),
);
expect(inst.state.databaseColumns).not.toEqual(
expect.arrayContaining([expect.objectContaining({ name: 'name' })]),
// to add, remove and modify columns accordingly
it('can modify columns', async () => {
const columnsTab = screen.getByTestId('collection-tab-Columns');
userEvent.click(columnsTab);

const getToggles = screen.getAllByRole('button', {
name: /toggle expand/i,
});
userEvent.click(getToggles[0]);
const getTextboxes = screen.getAllByRole('textbox');
expect(getTextboxes.length).toEqual(5);

const inputLabel = screen.getByPlaceholderText('Label');
const inputDescription = screen.getByPlaceholderText('Description');
const inputDtmFormat = screen.getByPlaceholderText('%Y/%m/%d');
const inputCertifiedBy = screen.getByPlaceholderText('Certified by');
const inputCertDetails = screen.getByPlaceholderText(
'Certification details',
);

userEvent.type(await inputLabel, 'test_lable');
userEvent.type(await inputDescription, 'test');
userEvent.type(await inputDtmFormat, 'test');
userEvent.type(await inputCertifiedBy, 'test');
userEvent.type(await inputCertDetails, 'test');
});

it('can delete columns', async () => {
const columnsTab = screen.getByTestId('collection-tab-Columns');
userEvent.click(columnsTab);

const getToggles = screen.getAllByRole('button', {
name: /toggle expand/i,
});

userEvent.click(getToggles[0]);
screen.logTestingPlaygroundURL();
const deleteButtons = screen.getAllByRole('button', {
name: /delete item/i,
});
expect(deleteButtons.length).toEqual(7);
userEvent.click(deleteButtons[0]);
const countRows = screen.getAllByRole('button', { name: /delete item/i });
expect(countRows.length).toEqual(6);
});

it('can add new columns', async () => {
const calcColsTab = screen.getByTestId('collection-tab-Calculated columns');
userEvent.click(calcColsTab);
const addBtn = screen.getByRole('button', {
name: /add item/i,
});
expect(addBtn).toBeInTheDocument();
userEvent.click(addBtn);
const newColumn = screen.getByRole('button', {
name: /<new column>/i,
});
expect(newColumn).toBeInTheDocument();
});

it('renders isSqla fields', () => {
wrapper.setState({ activeTabKey: 4 });
expect(wrapper.state('isSqla')).toBe(true);
const columnsTab = screen.getByRole('tab', {
name: /settings/i,
});
userEvent.click(columnsTab);
const extraField = screen.getAllByText(/extra/i);
expect(extraField.length).toEqual(2);
expect(
wrapper.find(Field).find({ fieldKey: 'fetch_values_predicate' }).exists(),
).toBe(true);
screen.getByText(/autocomplete query predicate/i),
).toBeInTheDocument();
expect(screen.getByText(/template parameters/i)).toBeInTheDocument();
});

describe('enable edit Source tab', () => {
beforeAll(() => {
isFeatureEnabledMock = jest
.spyOn(featureFlags, 'isFeatureEnabled')
.mockImplementation(() => false);
wrapper = shallow(el, { context: { store } }).dive();
});

afterAll(() => {
isFeatureEnabledMock.mockRestore();
});

it('Source Tab: edit mode', () => {
wrapper.setState({ activeTabKey: 0, isEditMode: true });
const sourceTab = wrapper.find(Tabs.TabPane).first();
expect(sourceTab.find(Radio).first().prop('disabled')).toBe(false);

const icon = wrapper.find(Icons.LockUnlocked);
expect(icon).toExist();

const tableSelector = sourceTab.find(Field).shallow().find(TableSelector);
expect(tableSelector.length).toBe(1);
expect(tableSelector.prop('readOnly')).toBe(false);
const getLockBtn = screen.getByRole('img', { name: /lock-locked/i });
userEvent.click(getLockBtn);
const physicalRadioBtn = screen.getByRole('radio', {
name: /physical \(table or view\)/i,
});
const vituralRadioBtn = screen.getByRole('radio', {
name: /virtual \(sql\)/i,
});
expect(physicalRadioBtn).toBeEnabled();
expect(vituralRadioBtn).toBeEnabled();
});

it('Source Tab: readOnly mode', () => {
const sourceTab = wrapper.find(Tabs.TabPane).first();
expect(sourceTab.find(Radio).length).toBe(2);
expect(sourceTab.find(Radio).first().prop('disabled')).toBe(true);

const icon = wrapper.find(Icons.LockLocked);
expect(icon).toExist();
icon.parent().simulate('click');
expect(wrapper.state('isEditMode')).toBe(true);

const tableSelector = sourceTab.find(Field).shallow().find(TableSelector);
expect(tableSelector.length).toBe(1);
expect(tableSelector.prop('readOnly')).toBe(true);
const getLockBtn = screen.getByRole('img', { name: /lock-locked/i });
expect(getLockBtn).toBeInTheDocument();
const physicalRadioBtn = screen.getByRole('radio', {
name: /physical \(table or view\)/i,
});
const vituralRadioBtn = screen.getByRole('radio', {
name: /virtual \(sql\)/i,
});
expect(physicalRadioBtn).toBeDisabled();
expect(vituralRadioBtn).toBeDisabled();
});
});

it('disable edit Source tab', () => {
// when edit is disabled, show readOnly controls and no padlock
isFeatureEnabledMock = jest
.spyOn(featureFlags, 'isFeatureEnabled')
.mockImplementation(() => true);
wrapper = shallow(el, { context: { store } }).dive();
wrapper.setState({ activeTabKey: 0 });

const sourceTab = wrapper.find(Tabs.TabPane).first();
expect(sourceTab.find(Radio).length).toBe(2);
expect(sourceTab.find(Radio).first().prop('disabled')).toBe(true);
describe('render editor with feature flag false', () => {
beforeAll(() => {
isFeatureEnabledMock = jest
.spyOn(featureFlags, 'isFeatureEnabled')
.mockImplementation(() => true);
});

const icon = sourceTab.find(Icons.LockLocked);
expect(icon).toHaveLength(0);
beforeEach(() => {
render(el, { useRedux: true });
});

isFeatureEnabledMock.mockRestore();
it('disable edit Source tab', () => {
expect(
screen.queryByRole('img', { name: /lock-locked/i }),
).not.toBeInTheDocument();
isFeatureEnabledMock.mockRestore();
});
});
});

Expand All @@ -227,9 +210,7 @@ describe('DatasourceEditor RTL', () => {
/certification details/i,
);
expect(certificationDetails.value).toEqual('foo');
const warningMarkdown = await await screen.findByPlaceholderText(
/certified by/i,
);
const warningMarkdown = await screen.findByPlaceholderText(/certified by/i);
expect(warningMarkdown.value).toEqual('someone');
});
it('properly updates the metric information', async () => {
Expand Down

0 comments on commit 93f59e0

Please sign in to comment.