You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by pk...@apache.org on 2021/10/27 19:43:59 UTC
[superset] branch master updated: chore: remove supersetTheme with
withTheme (#17069)
This is an automated email from the ASF dual-hosted git repository.
pkdotson pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 93f59e0 chore: remove supersetTheme with withTheme (#17069)
93f59e0 is described below
commit 93f59e055e8312fb28687bc9fc22342b4be68d0e
Author: Phillip Kelley-Dotson <pk...@yahoo.com>
AuthorDate: Wed Oct 27 12:42:56 2021 -0700
chore: remove supersetTheme with withTheme (#17069)
* remove supersettheme
* migrate Datasourceeditor to rtl
* lint fix
* remove file
* fix import
* fix test
* fix lint
* spread tests and remove unused code
* fix lint
---
.../src/components/Datasource/DatasourceEditor.jsx | 22 +-
.../Datasource/DatasourceEditor.test.jsx | 243 ++++++++++-----------
2 files changed, 123 insertions(+), 142 deletions(-)
diff --git a/superset-frontend/src/components/Datasource/DatasourceEditor.jsx b/superset-frontend/src/components/Datasource/DatasourceEditor.jsx
index e80e402..ebd2ebf 100644
--- a/superset-frontend/src/components/Datasource/DatasourceEditor.jsx
+++ b/superset-frontend/src/components/Datasource/DatasourceEditor.jsx
@@ -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';
@@ -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 {
@@ -828,7 +828,7 @@ class DatasourceEditor extends React.PureComponent {
);
}
- renderSourceFieldset() {
+ renderSourceFieldset(theme) {
const { datasource } = this.state;
return (
<div>
@@ -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 && (
@@ -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()}
@@ -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={
@@ -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);
diff --git a/superset-frontend/src/components/Datasource/DatasourceEditor.test.jsx b/superset-frontend/src/components/Datasource/DatasourceEditor.test.jsx
index 10f3d52..a0d46a6 100644
--- a/superset-frontend/src/components/Datasource/DatasourceEditor.test.jsx
+++ b/superset-frontend/src/components/Datasource/DatasourceEditor.test.jsx
@@ -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'],
@@ -43,23 +33,14 @@ 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', () => {
@@ -67,15 +48,18 @@ describe('DatasourceEditor', () => {
});
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);
@@ -84,76 +68,77 @@ 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', () => {
@@ -161,7 +146,6 @@ describe('DatasourceEditor', () => {
isFeatureEnabledMock = jest
.spyOn(featureFlags, 'isFeatureEnabled')
.mockImplementation(() => false);
- wrapper = shallow(el, { context: { store } }).dive();
});
afterAll(() => {
@@ -169,50 +153,49 @@ describe('DatasourceEditor', () => {
});
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();
+ });
});
});
@@ -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 () => {