From b6e3b3e639c07f355620adca0af6cd32d4f41803 Mon Sep 17 00:00:00 2001 From: lyndsiWilliams Date: Mon, 1 Feb 2021 19:06:14 -0600 Subject: [PATCH 1/4] Class converted to component --- .../src/SqlLab/components/TableElement.jsx | 207 ++++++++---------- 1 file changed, 88 insertions(+), 119 deletions(-) diff --git a/superset-frontend/src/SqlLab/components/TableElement.jsx b/superset-frontend/src/SqlLab/components/TableElement.jsx index 7abdf7ef7eea..ea12bfe29152 100644 --- a/superset-frontend/src/SqlLab/components/TableElement.jsx +++ b/superset-frontend/src/SqlLab/components/TableElement.jsx @@ -16,11 +16,10 @@ * specific language governing permissions and limitations * under the License. */ -import React from 'react'; +import React, { useState } from 'react'; import PropTypes from 'prop-types'; import { Collapse, Well } from 'react-bootstrap'; import ButtonGroup from 'src/components/ButtonGroup'; -import shortid from 'shortid'; import { t, styled } from '@superset-ui/core'; import Fade from 'src/common/components/Fade'; @@ -52,59 +51,40 @@ const StyledSpan = styled.span` cursor: pointer; `; -class TableElement extends React.PureComponent { - constructor(props) { - super(props); - this.state = { - sortColumns: false, - expanded: true, - hovered: false, - }; - this.removeFromStore = this.removeFromStore.bind(this); - this.toggleSortColumns = this.toggleSortColumns.bind(this); - this.removeTable = this.removeTable.bind(this); - this.setHover = this.setHover.bind(this); - } - - setHover(hovered) { - this.setState({ hovered }); - } +function TableElement({ actions, table, timeout }) { + const [state, setState] = useState({ + sortColumns: false, + expanded: true, + hovered: false, + }); - popSelectStar() { - const qe = { - id: shortid.generate(), - title: this.props.table.name, - dbId: this.props.table.dbId, - autorun: true, - sql: this.props.table.selectStar, - }; - this.props.actions.addQueryEditor(qe); - } + const setHover = hovered => { + setState({ hovered }); + }; - toggleTable(e) { + const toggleTable = e => { e.preventDefault(); - if (this.props.table.expanded) { - this.props.actions.collapseTable(this.props.table); + if (table.expanded) { + actions.collapseTable(table); } else { - this.props.actions.expandTable(this.props.table); + actions.expandTable(table); } - } + }; - removeTable() { - this.setState({ expanded: false }); - this.props.actions.removeDataPreview(this.props.table); - } + const removeTable = () => { + setState({ expanded: false }); + actions.removeDataPreview(table); + }; - toggleSortColumns() { - this.setState(prevState => ({ sortColumns: !prevState.sortColumns })); - } + const toggleSortColumns = () => { + setState(prevState => ({ sortColumns: !prevState.sortColumns })); + }; - removeFromStore() { - this.props.actions.removeTable(this.props.table); - } + const removeFromStore = () => { + actions.removeTable(table); + }; - renderWell() { - const { table } = this.props; + const renderWell = () => { let header; if (table.partitions) { let partitionQuery; @@ -137,11 +117,10 @@ class TableElement extends React.PureComponent { ); } return header; - } + }; - renderControls() { + const renderControls = () => { let keyLink; - const { table } = this.props; if (table.indexes && table.indexes.length > 0) { keyLink = ( ); - } + }; - renderHeader() { - const { table } = this.props; - return ( -
- + + { + toggleTable(e); + }} > - { - this.toggleTable(e); - }} - > - {table.name} - - + {table.name} + + -
- {table.isMetadataLoading || table.isExtraMetadataLoading ? ( - - ) : ( - {this.renderControls()} - )} - { - this.toggleTable(e); - }} - className={ - 'text-primary pointer m-l-10 ' + - 'fa fa-lg ' + - `fa-angle-${table.expanded ? 'up' : 'down'}` - } - /> -
+
+ {table.isMetadataLoading || table.isExtraMetadataLoading ? ( + + ) : ( + {renderControls()} + )} + { + toggleTable(e); + }} + className={ + 'text-primary pointer m-l-10 ' + + 'fa fa-lg ' + + `fa-angle-${table.expanded ? 'up' : 'down'}` + } + />
- ); - } +
+ ); - renderBody() { - const { table } = this.props; + const renderBody = () => { let cols; if (table.columns) { cols = table.columns.slice(); - if (this.state.sortColumns) { + if (state.sortColumns) { cols.sort((a, b) => { const colA = a.name.toUpperCase(); const colB = b.name.toUpperCase(); @@ -271,9 +246,9 @@ class TableElement extends React.PureComponent { } } const metadata = ( - +
- {this.renderWell()} + {renderWell()}
{cols && cols.map(col => )} @@ -282,26 +257,20 @@ class TableElement extends React.PureComponent { ); return metadata; - } + }; - render() { - return ( - +
setHover(true)} + onMouseLeave={() => setHover(false)} > -
this.setHover(true)} - onMouseLeave={() => this.setHover(false)} - > - {this.renderHeader()} -
{this.renderBody()}
-
- - ); - } + {header} +
{renderBody()}
+
+ + ); } TableElement.propTypes = propTypes; TableElement.defaultProps = defaultProps; From 7963529139d075a480629bfb8aefc9617dab33f1 Mon Sep 17 00:00:00 2001 From: lyndsiWilliams Date: Wed, 3 Feb 2021 15:38:16 -0600 Subject: [PATCH 2/4] Trying to repro useState in test --- .../javascripts/sqllab/TableElement_spec.jsx | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/superset-frontend/spec/javascripts/sqllab/TableElement_spec.jsx b/superset-frontend/spec/javascripts/sqllab/TableElement_spec.jsx index e1426e20b851..22b8cc8d3106 100644 --- a/superset-frontend/spec/javascripts/sqllab/TableElement_spec.jsx +++ b/superset-frontend/spec/javascripts/sqllab/TableElement_spec.jsx @@ -16,8 +16,9 @@ * specific language governing permissions and limitations * under the License. */ -import React from 'react'; -import { mount, shallow } from 'enzyme'; +import React, { useState } from 'react'; +import { mount, shallow, configure } from 'enzyme'; +import Adapter from 'enzyme-adapter-react-16'; import { Provider } from 'react-redux'; import configureStore from 'redux-mock-store'; import { supersetTheme, ThemeProvider } from '@superset-ui/core'; @@ -29,6 +30,8 @@ import ColumnElement from 'src/SqlLab/components/ColumnElement'; import { mockedActions, table } from './fixtures'; +configure({ adapter: new Adapter() }); + describe('TableElement', () => { const mockStore = configureStore([]); const store = mockStore({}); @@ -65,8 +68,13 @@ describe('TableElement', () => { ); }); it('fades table', () => { - const wrapper = shallow(); - expect(wrapper.state().hovered).toBe(false); + const setHover = jest.fn(); + const wrapper = shallow(); + const onMouseEnter = jest.spyOn(React, "useState"); + onMouseEnter.mockImplementation(hover => [hover, setHover]); + console.log(wrapper.props()); + + expect(wrapper.props().hovered).toBe(false); expect(wrapper.find(Fade).props().hovered).toBe(false); wrapper.find('div.TableElement').simulate('mouseEnter'); expect(wrapper.state().hovered).toBe(true); From 4067237b9d038890de05e5f50555ad1aa76963e3 Mon Sep 17 00:00:00 2001 From: lyndsiWilliams Date: Thu, 4 Feb 2021 11:04:10 -0600 Subject: [PATCH 3/4] Working with tests --- .../javascripts/sqllab/TableElement_spec.jsx | 94 +++++++++++-------- 1 file changed, 53 insertions(+), 41 deletions(-) diff --git a/superset-frontend/spec/javascripts/sqllab/TableElement_spec.jsx b/superset-frontend/spec/javascripts/sqllab/TableElement_spec.jsx index 22b8cc8d3106..ad097329e224 100644 --- a/superset-frontend/spec/javascripts/sqllab/TableElement_spec.jsx +++ b/superset-frontend/spec/javascripts/sqllab/TableElement_spec.jsx @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import React, { useState } from 'react'; +import React from 'react'; import { mount, shallow, configure } from 'enzyme'; import Adapter from 'enzyme-adapter-react-16'; import { Provider } from 'react-redux'; @@ -40,6 +40,11 @@ describe('TableElement', () => { table, timeout: 0, }; + // Mocking useState + const setState = jest.fn(); + const useStateMock = (initState) => [initState, setState]; + afterEach(() => jest.clearAllMocks()); + it('renders', () => { expect(React.isValidElement()).toBe(true); }); @@ -68,49 +73,56 @@ describe('TableElement', () => { ); }); it('fades table', () => { - const setHover = jest.fn(); - const wrapper = shallow(); - const onMouseEnter = jest.spyOn(React, "useState"); - onMouseEnter.mockImplementation(hover => [hover, setHover]); - console.log(wrapper.props()); + const wrapper = shallow(); + // jest.spyOn(React, 'useState').mockImplementation(useStateMock); + // setState({ + // sortColumns: false, + // expanded: true, + // hovered: false, + // }); + + // expect(setState).toHaveBeenCalledTimes(1); - expect(wrapper.props().hovered).toBe(false); + console.log('WRAPPER PROPS BEFORE', wrapper.props().mountOnEnter); + + expect(wrapper.props().mountOnEnter).toBe(false); expect(wrapper.find(Fade).props().hovered).toBe(false); wrapper.find('div.TableElement').simulate('mouseEnter'); - expect(wrapper.state().hovered).toBe(true); + console.log('WRAPPER PROPS AFTER', wrapper.props().mountOnEnter); + expect(wrapper.props().mountOnEnter).toBe(true); expect(wrapper.find(Fade).props().hovered).toBe(true); }); - it('sorts columns', () => { - const wrapper = shallow(); - expect(wrapper.state().sortColumns).toBe(false); - expect(wrapper.find(ColumnElement).first().props().column.name).toBe('id'); - wrapper.find('.sort-cols').simulate('click'); - expect(wrapper.state().sortColumns).toBe(true); - expect(wrapper.find(ColumnElement).first().props().column.name).toBe( - 'active', - ); - }); - it('calls the collapseTable action', () => { - const wrapper = mount( - - - , - { - wrappingComponent: ThemeProvider, - wrappingComponentProps: { - theme: supersetTheme, - }, - }, - ); - expect(mockedActions.collapseTable.called).toBe(false); - wrapper.find('[data-test="collapse"]').hostNodes().simulate('click'); - expect(mockedActions.collapseTable.called).toBe(true); - }); - it('removes the table', () => { - const wrapper = shallow(); - expect(wrapper.state().expanded).toBe(true); - wrapper.find('.table-remove').simulate('click'); - expect(wrapper.state().expanded).toBe(false); - expect(mockedActions.removeDataPreview.called).toBe(true); - }); + // it('sorts columns', () => { + // const wrapper = shallow(); + // expect(wrapper.state().sortColumns).toBe(false); + // expect(wrapper.find(ColumnElement).first().props().column.name).toBe('id'); + // wrapper.find('.sort-cols').simulate('click'); + // expect(wrapper.state().sortColumns).toBe(true); + // expect(wrapper.find(ColumnElement).first().props().column.name).toBe( + // 'active', + // ); + // }); + // it('calls the collapseTable action', () => { + // const wrapper = mount( + // + // + // , + // { + // wrappingComponent: ThemeProvider, + // wrappingComponentProps: { + // theme: supersetTheme, + // }, + // }, + // ); + // expect(mockedActions.collapseTable.called).toBe(false); + // wrapper.find('[data-test="collapse"]').hostNodes().simulate('click'); + // expect(mockedActions.collapseTable.called).toBe(true); + // }); + // it('removes the table', () => { + // const wrapper = shallow(); + // expect(wrapper.state().expanded).toBe(true); + // wrapper.find('.table-remove').simulate('click'); + // expect(wrapper.state().expanded).toBe(false); + // expect(mockedActions.removeDataPreview.called).toBe(true); + // }); }); From 28bb86ab8231a8c03490d6e69c269287fde472d2 Mon Sep 17 00:00:00 2001 From: lyndsiWilliams Date: Fri, 5 Feb 2021 10:20:35 -0600 Subject: [PATCH 4/4] Broke state into individual hooks --- .../src/SqlLab/components/TableElement.jsx | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/superset-frontend/src/SqlLab/components/TableElement.jsx b/superset-frontend/src/SqlLab/components/TableElement.jsx index ea12bfe29152..ad4f4f78a44d 100644 --- a/superset-frontend/src/SqlLab/components/TableElement.jsx +++ b/superset-frontend/src/SqlLab/components/TableElement.jsx @@ -52,14 +52,12 @@ const StyledSpan = styled.span` `; function TableElement({ actions, table, timeout }) { - const [state, setState] = useState({ - sortColumns: false, - expanded: true, - hovered: false, - }); + const [sortColumns, setSortColumns] = useState(false); + const [expanded, setExpanded] = useState(true); + const [hovered, setHovered] = useState(false); const setHover = hovered => { - setState({ hovered }); + setHovered(hovered); }; const toggleTable = e => { @@ -72,12 +70,12 @@ function TableElement({ actions, table, timeout }) { }; const removeTable = () => { - setState({ expanded: false }); + setExpanded(false); actions.removeDataPreview(table); }; const toggleSortColumns = () => { - setState(prevState => ({ sortColumns: !prevState.sortColumns })); + setSortColumns(!sortColumns); }; const removeFromStore = () => { @@ -146,12 +144,12 @@ function TableElement({ actions, table, timeout }) { {keyLink} ) : ( - {renderControls()} + {renderControls()} )} { const colA = a.name.toUpperCase(); const colB = b.name.toUpperCase(); @@ -260,7 +258,7 @@ function TableElement({ actions, table, timeout }) { }; return ( - +
setHover(true)}