From 31aad28a31c117ad9c5c5a1957b6043a75e93020 Mon Sep 17 00:00:00 2001 From: Enzo Martellucci <52219496+EnxDev@users.noreply.github.com> Date: Thu, 31 Oct 2024 15:57:49 +0100 Subject: [PATCH] refactor: Migrate SliceAdder to typescript (#30697) --- .../spec/fixtures/mockSliceEntities.js | 133 +++++++++++++++--- ...liceAdder.test.jsx => SliceAdder.test.tsx} | 100 ++++++++----- .../{SliceAdder.jsx => SliceAdder.tsx} | 115 +++++++++------ ...ragPreview.jsx => AddSliceDragPreview.tsx} | 53 ++++--- 4 files changed, 269 insertions(+), 132 deletions(-) rename superset-frontend/src/dashboard/components/{SliceAdder.test.jsx => SliceAdder.test.tsx} (67%) rename superset-frontend/src/dashboard/components/{SliceAdder.jsx => SliceAdder.tsx} (82%) rename superset-frontend/src/dashboard/components/dnd/{AddSliceDragPreview.jsx => AddSliceDragPreview.tsx} (72%) diff --git a/superset-frontend/spec/fixtures/mockSliceEntities.js b/superset-frontend/spec/fixtures/mockSliceEntities.js index 2737d35d01d40..1a7bdad02429f 100644 --- a/superset-frontend/spec/fixtures/mockSliceEntities.js +++ b/superset-frontend/spec/fixtures/mockSliceEntities.js @@ -17,6 +17,7 @@ * under the License. */ import { datasourceId } from 'spec/fixtures/mockDatasource'; +import { DatasourceType } from '@superset-ui/core'; import { sliceId } from './mockChartQueries'; export const filterId = 127; @@ -47,8 +48,8 @@ export const sliceEntitiesForChart = { }, viz_type: 'pie', datasource: datasourceId, - description: null, - description_markeddown: '', + description: '', + description_markdown: '', modified: '23 hours ago', changed_on: 1529453332615, }, @@ -79,10 +80,18 @@ export const sliceEntitiesForDashboard = { }, viz_type: 'filter_box', datasource: '2__table', - description: null, - description_markeddown: '', + description: '', + description_markdown: '', modified: '23 hours ago', changed_on: 1529453332615, + changed_on_humanized: '', + datasource_id: 0, + datasource_type: DatasourceType.Query, + datasource_url: '', + datasource_name: '', + owners: [{ id: 0 }], + created_by: { id: 0 }, + thumbnail_url: '', }, 128: { slice_id: 128, @@ -91,10 +100,18 @@ export const sliceEntitiesForDashboard = { form_data: {}, viz_type: 'big_number', datasource: '2__table', - description: null, - description_markeddown: '', + description: '', + description_markdown: '', modified: '23 hours ago', changed_on: 1529453332628, + changed_on_humanized: '', + datasource_id: 0, + datasource_type: DatasourceType.Query, + datasource_url: '', + datasource_name: '', + owners: [{ id: 0 }], + created_by: { id: 0 }, + thumbnail_url: '', }, 129: { slice_id: 129, @@ -103,10 +120,19 @@ export const sliceEntitiesForDashboard = { form_data: {}, viz_type: 'table', datasource: '2__table', - description: null, - description_markeddown: '', + description: '', + description_markdown: 'dd', modified: '23 hours ago', changed_on: 1529453332637, + changed_on_humanized: '', + + datasource_id: 0, + datasource_type: DatasourceType.Query, + datasource_url: '', + datasource_name: '', + owners: [{ id: 0 }], + created_by: { id: 0 }, + thumbnail_url: '', }, 130: { slice_id: 130, @@ -115,10 +141,19 @@ export const sliceEntitiesForDashboard = { form_data: {}, viz_type: 'line', datasource: '2__table', - description: null, - description_markeddown: '', + description: '', + description_markdown: '', modified: '23 hours ago', changed_on: 1529453332645, + changed_on_humanized: '', + + datasource_id: 0, + datasource_type: DatasourceType.SlTable, + datasource_url: '', + datasource_name: '', + owners: [{ id: 0 }], + created_by: { id: 0 }, + thumbnail_url: '', }, 131: { slice_id: 131, @@ -127,10 +162,19 @@ export const sliceEntitiesForDashboard = { form_data: {}, viz_type: 'world_map', datasource: '2__table', - description: null, - description_markeddown: '', + description: '', + description_markdown: '', modified: '23 hours ago', changed_on: 1529453332654, + changed_on_humanized: '', + + datasource_id: 0, + datasource_type: DatasourceType.Table, + datasource_url: '', + datasource_name: '', + owners: [{ id: 0 }], + created_by: { id: 0 }, + thumbnail_url: '', }, 132: { slice_id: 132, @@ -139,10 +183,19 @@ export const sliceEntitiesForDashboard = { form_data: {}, viz_type: 'bubble', datasource: '2__table', - description: null, - description_markeddown: '', + description: '', + description_markdown: '', modified: '23 hours ago', changed_on: 1529453332663, + changed_on_humanized: '', + + datasource_id: 0, + datasource_type: DatasourceType.Query, + datasource_url: '', + datasource_name: '', + owners: [{ id: 0 }], + created_by: { id: 0 }, + thumbnail_url: '', }, 133: { slice_id: 133, @@ -151,10 +204,19 @@ export const sliceEntitiesForDashboard = { form_data: {}, viz_type: 'sunburst_v2', datasource: '2__table', - description: null, - description_markeddown: '', + description: '', + description_markdown: '', modified: '23 hours ago', changed_on: 1529453332673, + changed_on_humanized: '', + + datasource_id: 0, + datasource_type: DatasourceType.Query, + datasource_url: '', + datasource_name: '', + owners: [{ id: 0 }], + created_by: { id: 0 }, + thumbnail_url: '', }, 134: { slice_id: 134, @@ -163,10 +225,19 @@ export const sliceEntitiesForDashboard = { form_data: {}, viz_type: 'area', datasource: '2__table', - description: null, - description_markeddown: '', + description: '', + description_markdown: '', modified: '23 hours ago', changed_on: 1529453332680, + changed_on_humanized: '', + + datasource_id: 0, + datasource_type: DatasourceType.Dataset, + datasource_url: '', + datasource_name: '', + owners: [{ id: 0 }], + created_by: { id: 0 }, + thumbnail_url: '', }, 135: { slice_id: 135, @@ -175,10 +246,19 @@ export const sliceEntitiesForDashboard = { form_data: {}, viz_type: 'box_plot', datasource: '2__table', - description: null, - description_markeddown: '', + description: '', + description_markdown: '', modified: '23 hours ago', changed_on: 1529453332688, + changed_on_humanized: '', + + datasource_id: 0, + datasource_type: DatasourceType.Table, + datasource_url: '', + datasource_name: '', + owners: [{ id: 0 }], + created_by: { id: 0 }, + thumbnail_url: '', }, 136: { slice_id: 136, @@ -187,10 +267,19 @@ export const sliceEntitiesForDashboard = { form_data: {}, viz_type: 'treemap_v2', datasource: '2__table', - description: null, - description_markeddown: '', + description: '', + description_markdown: '', modified: '23 hours ago', changed_on: 1529453332700, + changed_on_humanized: '', + + datasource_id: 0, + datasource_type: DatasourceType.Table, + datasource_url: '', + datasource_name: '', + owners: [{ id: 0 }], + created_by: { id: 0 }, + thumbnail_url: '', }, }, isLoading: false, diff --git a/superset-frontend/src/dashboard/components/SliceAdder.test.jsx b/superset-frontend/src/dashboard/components/SliceAdder.test.tsx similarity index 67% rename from superset-frontend/src/dashboard/components/SliceAdder.test.jsx rename to superset-frontend/src/dashboard/components/SliceAdder.test.tsx index 6ae0346edb43e..a0a6acb568684 100644 --- a/superset-frontend/src/dashboard/components/SliceAdder.test.jsx +++ b/superset-frontend/src/dashboard/components/SliceAdder.test.tsx @@ -16,35 +16,45 @@ * specific language governing permissions and limitations * under the License. */ -import { shallow } from 'enzyme'; +import { shallow, ShallowWrapper } from 'enzyme'; import sinon from 'sinon'; import SliceAdder, { ChartList, DEFAULT_SORT_KEY, + SliceAdderProps, } from 'src/dashboard/components/SliceAdder'; import { sliceEntitiesForDashboard as mockSliceEntities } from 'spec/fixtures/mockSliceEntities'; import { styledShallow } from 'spec/helpers/theming'; -jest.mock('lodash/debounce', () => fn => { - // eslint-disable-next-line no-param-reassign - fn.throttle = jest.fn(); - return fn; -}); +jest.mock( + 'lodash/debounce', + () => (fn: { throttle: jest.Mock }) => { + // eslint-disable-next-line no-param-reassign + fn.throttle = jest.fn(); + return fn; + }, +); describe('SliceAdder', () => { - const props = { - ...mockSliceEntities, + const props: SliceAdderProps = { + slices: { + ...mockSliceEntities.slices, + }, fetchSlices: jest.fn(), updateSlices: jest.fn(), selectedSliceIds: [127, 128], userId: 1, + dashboardId: 0, + editMode: false, + errorMessage: '', + isLoading: false, + lastUpdated: 0, }; const errorProps = { ...props, errorMessage: 'this is error', }; - describe('SliceAdder.sortByComparator', () => { it('should sort by timestamp descending', () => { const sortedTimestamps = Object.values(props.slices) @@ -84,72 +94,88 @@ describe('SliceAdder', () => { }); it('componentDidMount', () => { - sinon.spy(SliceAdder.prototype, 'componentDidMount'); - sinon.spy(props, 'fetchSlices'); - + const componentDidMountSpy = sinon.spy( + SliceAdder.prototype, + 'componentDidMount', + ); + const fetchSlicesSpy = sinon.spy(props, 'fetchSlices'); shallow(, { lifecycleExperimental: true, }); - expect(SliceAdder.prototype.componentDidMount.calledOnce).toBe(true); - expect(props.fetchSlices.calledOnce).toBe(true); - SliceAdder.prototype.componentDidMount.restore(); - props.fetchSlices.restore(); + expect(componentDidMountSpy.calledOnce).toBe(true); + + expect(fetchSlicesSpy.calledOnce).toBe(true); + + componentDidMountSpy.restore(); + fetchSlicesSpy.restore(); }); describe('UNSAFE_componentWillReceiveProps', () => { - let wrapper; + let wrapper: ShallowWrapper; + let setStateSpy: sinon.SinonSpy; + beforeEach(() => { wrapper = shallow(); wrapper.setState({ filteredSlices: Object.values(props.slices) }); - sinon.spy(wrapper.instance(), 'setState'); + setStateSpy = sinon.spy(wrapper.instance() as SliceAdder, 'setState'); }); afterEach(() => { - wrapper.instance().setState.restore(); + setStateSpy.restore(); }); it('fetch slices should update state', () => { - wrapper.instance().UNSAFE_componentWillReceiveProps({ + const instance = wrapper.instance() as SliceAdder; + instance.UNSAFE_componentWillReceiveProps({ ...props, lastUpdated: new Date().getTime(), }); - expect(wrapper.instance().setState.calledOnce).toBe(true); + expect(setStateSpy.calledOnce).toBe(true); - const stateKeys = Object.keys( - wrapper.instance().setState.lastCall.args[0], - ); + const stateKeys = Object.keys(setStateSpy.lastCall.args[0]); expect(stateKeys).toContain('filteredSlices'); }); it('select slices should update state', () => { - wrapper.instance().UNSAFE_componentWillReceiveProps({ + const instance = wrapper.instance() as SliceAdder; + + instance.UNSAFE_componentWillReceiveProps({ ...props, selectedSliceIds: [127], }); - expect(wrapper.instance().setState.calledOnce).toBe(true); - const stateKeys = Object.keys( - wrapper.instance().setState.lastCall.args[0], - ); + expect(setStateSpy.calledOnce).toBe(true); + + const stateKeys = Object.keys(setStateSpy.lastCall.args[0]); expect(stateKeys).toContain('selectedSliceIdsSet'); }); }); describe('should rerun filter and sort', () => { - let wrapper; - let spy; + let wrapper: ShallowWrapper; + let spy: jest.Mock; + beforeEach(() => { - spy = props.fetchSlices; - wrapper = shallow(); - wrapper.setState({ filteredSlices: Object.values(props.slices) }); + spy = jest.fn(); + const fetchSlicesProps: SliceAdderProps = { + ...props, + fetchSlices: spy, + }; + wrapper = shallow(); + wrapper.setState({ + filteredSlices: Object.values(fetchSlicesProps.slices), + }); }); + afterEach(() => { spy.mockReset(); }); it('searchUpdated', () => { const newSearchTerm = 'new search term'; - wrapper.instance().handleChange(newSearchTerm); + + (wrapper.instance() as SliceAdder).handleChange(newSearchTerm); + expect(spy).toHaveBeenCalled(); expect(spy).toHaveBeenCalledWith( props.userId, @@ -160,7 +186,9 @@ describe('SliceAdder', () => { it('handleSelect', () => { const newSortBy = 'viz_type'; - wrapper.instance().handleSelect(newSortBy); + + (wrapper.instance() as SliceAdder).handleSelect(newSortBy); + expect(spy).toHaveBeenCalled(); expect(spy).toHaveBeenCalledWith(props.userId, '', newSortBy); }); diff --git a/superset-frontend/src/dashboard/components/SliceAdder.jsx b/superset-frontend/src/dashboard/components/SliceAdder.tsx similarity index 82% rename from superset-frontend/src/dashboard/components/SliceAdder.jsx rename to superset-frontend/src/dashboard/components/SliceAdder.tsx index b610fe152ce13..09f6270f83088 100644 --- a/superset-frontend/src/dashboard/components/SliceAdder.jsx +++ b/superset-frontend/src/dashboard/components/SliceAdder.tsx @@ -18,9 +18,9 @@ */ /* eslint-env browser */ import { Component } from 'react'; -import PropTypes from 'prop-types'; import AutoSizer from 'react-virtualized-auto-sizer'; import { FixedSizeList as List } from 'react-window'; +// @ts-ignore import { createFilter } from 'react-search-input'; import { t, styled, css } from '@superset-ui/core'; import { Input } from 'src/components/Input'; @@ -41,31 +41,40 @@ import { NEW_CHART_ID, NEW_COMPONENTS_SOURCE_ID, } from 'src/dashboard/util/constants'; -import { slicePropShape } from 'src/dashboard/util/propShapes'; import { debounce, pickBy } from 'lodash'; import Checkbox from 'src/components/Checkbox'; import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls'; +import { Dispatch } from 'redux'; +import { Slice } from 'src/dashboard/types'; import AddSliceCard from './AddSliceCard'; import AddSliceDragPreview from './dnd/AddSliceDragPreview'; import DragDroppable from './dnd/DragDroppable'; -const propTypes = { - fetchSlices: PropTypes.func.isRequired, - updateSlices: PropTypes.func.isRequired, - isLoading: PropTypes.bool.isRequired, - slices: PropTypes.objectOf(slicePropShape).isRequired, - lastUpdated: PropTypes.number.isRequired, - errorMessage: PropTypes.string, - userId: PropTypes.number.isRequired, - selectedSliceIds: PropTypes.arrayOf(PropTypes.number), - editMode: PropTypes.bool, - dashboardId: PropTypes.number, +export type SliceAdderProps = { + fetchSlices: ( + userId?: number, + filter_value?: string, + sortColumn?: string, + ) => Promise; + updateSlices: (slices: { + [id: number]: Slice; + }) => (dispatch: Dispatch) => void; + isLoading: boolean; + slices: Record; + lastUpdated: number; + errorMessage?: string; + userId: number; + selectedSliceIds?: number[]; + editMode?: boolean; + dashboardId: number; }; -const defaultProps = { - selectedSliceIds: [], - editMode: false, - errorMessage: '', +type SliceAdderState = { + filteredSlices: Slice[]; + searchTerm: string; + sortBy: keyof Slice; + selectedSliceIdsSet: Set; + showOnlyMyCharts: boolean; }; const KEYS_TO_FILTERS = ['slice_name', 'viz_type', 'datasource_name']; @@ -92,7 +101,7 @@ const Controls = styled.div` `} `; -const StyledSelect = styled(Select)` +const StyledSelect = styled(Select)<{ id?: string }>` margin-left: ${({ theme }) => theme.gridUnit * 2}px; min-width: 150px; `; @@ -124,22 +133,33 @@ export const ChartList = styled.div` min-height: 0; `; -class SliceAdder extends Component { - static sortByComparator(attr) { +class SliceAdder extends Component { + private slicesRequest?: AbortController | Promise; + + static sortByComparator(attr: keyof Slice) { const desc = attr === 'changed_on' ? -1 : 1; - return (a, b) => { - if (a[attr] < b[attr]) { + return (a: Slice, b: Slice) => { + const aValue = a[attr] ?? Number.MIN_SAFE_INTEGER; + const bValue = b[attr] ?? Number.MIN_SAFE_INTEGER; + + if (aValue < bValue) { return -1 * desc; } - if (a[attr] > b[attr]) { + if (aValue > bValue) { return 1 * desc; } return 0; }; } - constructor(props) { + static defaultProps = { + selectedSliceIds: [], + editMode: false, + errorMessage: '', + }; + + constructor(props: SliceAdderProps) { super(props); this.state = { filteredSlices: [], @@ -163,11 +183,15 @@ class SliceAdder extends Component { } componentDidMount() { - this.slicesRequest = this.props.fetchSlices(this.userIdForFetch()); + this.slicesRequest = this.props.fetchSlices( + this.userIdForFetch(), + '', + this.state.sortBy, + ); } - UNSAFE_componentWillReceiveProps(nextProps) { - const nextState = {}; + UNSAFE_componentWillReceiveProps(nextProps: SliceAdderProps) { + const nextState: SliceAdderState = {} as SliceAdderState; if (nextProps.lastUpdated !== this.props.lastUpdated) { nextState.filteredSlices = this.getFilteredSortedSlices( nextProps.slices, @@ -188,22 +212,27 @@ class SliceAdder extends Component { componentWillUnmount() { // Clears the redux store keeping only selected items - const selectedSlices = pickBy(this.props.slices, value => + const selectedSlices = pickBy(this.props.slices, (value: Slice) => this.state.selectedSliceIdsSet.has(value.slice_id), ); + this.props.updateSlices(selectedSlices); - if (this.slicesRequest && this.slicesRequest.abort) { + if (this.slicesRequest instanceof AbortController) { this.slicesRequest.abort(); } } - getFilteredSortedSlices(slices, searchTerm, sortBy, showOnlyMyCharts) { + getFilteredSortedSlices( + slices: SliceAdderProps['slices'], + searchTerm: string, + sortBy: keyof Slice, + showOnlyMyCharts: boolean, + ) { return Object.values(slices) .filter(slice => showOnlyMyCharts - ? (slice.owners && - slice.owners.find(owner => owner.id === this.props.userId)) || - (slice.created_by && slice.created_by.id === this.props.userId) + ? slice?.owners?.find(owner => owner.id === this.props.userId) || + slice?.created_by?.id === this.props.userId : true, ) .filter(createFilter(searchTerm, KEYS_TO_FILTERS)) @@ -219,7 +248,7 @@ class SliceAdder extends Component { ); }, 300); - searchUpdated(searchTerm) { + searchUpdated(searchTerm: string) { this.setState(prevState => ({ searchTerm, filteredSlices: this.getFilteredSortedSlices( @@ -231,7 +260,7 @@ class SliceAdder extends Component { })); } - handleSelect(sortBy) { + handleSelect(sortBy: keyof Slice) { this.setState(prevState => ({ sortBy, filteredSlices: this.getFilteredSortedSlices( @@ -248,9 +277,10 @@ class SliceAdder extends Component { ); } - rowRenderer({ key, index, style }) { + rowRenderer({ index, style }: { index: number; style: React.CSSProperties }) { const { filteredSlices, selectedSliceIdsSet } = this.state; const cellData = filteredSlices[index]; + const isSelected = selectedSliceIdsSet.has(cellData.slice_id); const type = CHART_TYPE; const id = NEW_CHART_ID; @@ -261,7 +291,7 @@ class SliceAdder extends Component { }; return ( 0 && ( - {({ height, width }) => ( + {({ height, width }: { height: number; width: number }) => ( this.state.filteredSlices[index].slice_id} > {this.rowRenderer} @@ -422,7 +450,4 @@ class SliceAdder extends Component { } } -SliceAdder.propTypes = propTypes; -SliceAdder.defaultProps = defaultProps; - export default SliceAdder; diff --git a/superset-frontend/src/dashboard/components/dnd/AddSliceDragPreview.jsx b/superset-frontend/src/dashboard/components/dnd/AddSliceDragPreview.tsx similarity index 72% rename from superset-frontend/src/dashboard/components/dnd/AddSliceDragPreview.jsx rename to superset-frontend/src/dashboard/components/dnd/AddSliceDragPreview.tsx index 375e2cdc3639a..dfd2d036880cd 100644 --- a/superset-frontend/src/dashboard/components/dnd/AddSliceDragPreview.jsx +++ b/superset-frontend/src/dashboard/components/dnd/AddSliceDragPreview.tsx @@ -16,17 +16,28 @@ * specific language governing permissions and limitations * under the License. */ -import PropTypes from 'prop-types'; -import { DragLayer } from 'react-dnd'; - +import { DragLayer, XYCoord } from 'react-dnd'; +import { Slice } from 'src/dashboard/types'; import AddSliceCard from '../AddSliceCard'; -import { slicePropShape } from '../../util/propShapes'; import { NEW_COMPONENT_SOURCE_TYPE, CHART_TYPE, } from '../../util/componentTypes'; -const staticCardStyles = { +interface DragItem { + index: number; + parentType: string; + type: string; +} + +interface AddSliceDragPreviewProps { + dragItem: DragItem | null; + slices: Slice[] | null; + isDragging: boolean; + currentOffset: XYCoord | null; +} + +const staticCardStyles: React.CSSProperties = { position: 'fixed', pointerEvents: 'none', top: 0, @@ -35,25 +46,12 @@ const staticCardStyles = { width: 376 - 2 * 16, }; -const propTypes = { - dragItem: PropTypes.shape({ - index: PropTypes.number.isRequired, - }), - slices: PropTypes.arrayOf(slicePropShape), - isDragging: PropTypes.bool.isRequired, - currentOffset: PropTypes.shape({ - x: PropTypes.number.isRequired, - y: PropTypes.number.isRequired, - }), -}; - -const defaultProps = { - currentOffset: null, - dragItem: null, - slices: null, -}; - -function AddSliceDragPreview({ dragItem, slices, isDragging, currentOffset }) { +const AddSliceDragPreview: React.FC = ({ + dragItem, + slices, + isDragging, + currentOffset, +}) => { if (!isDragging || !currentOffset || !dragItem || !slices) return null; const slice = slices[dragItem.index]; @@ -77,14 +75,11 @@ function AddSliceDragPreview({ dragItem, slices, isDragging, currentOffset }) { datasourceName={slice.datasource_name} /> ); -} - -AddSliceDragPreview.propTypes = propTypes; -AddSliceDragPreview.defaultProps = defaultProps; +}; // This injects these props into the component export default DragLayer(monitor => ({ - dragItem: monitor.getItem(), + dragItem: monitor.getItem() as DragItem | null, currentOffset: monitor.getSourceClientOffset(), isDragging: monitor.isDragging(), }))(AddSliceDragPreview);