From 7dc1f655ca7afa55a65a36956ce10bfe6adf4c6e Mon Sep 17 00:00:00 2001 From: Enzo Martellucci Date: Tue, 6 Feb 2024 18:57:31 +0100 Subject: [PATCH 01/15] chore: file migration from jsx to tsx --- .../src/components/AlteredSliceTag/index.tsx | 258 ++++++++++++++++++ 1 file changed, 258 insertions(+) create mode 100644 superset-frontend/src/components/AlteredSliceTag/index.tsx diff --git a/superset-frontend/src/components/AlteredSliceTag/index.tsx b/superset-frontend/src/components/AlteredSliceTag/index.tsx new file mode 100644 index 0000000000000..6866153280384 --- /dev/null +++ b/superset-frontend/src/components/AlteredSliceTag/index.tsx @@ -0,0 +1,258 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import React from 'react'; +import { isEqual, isEmpty } from 'lodash'; +import { styled, t } from '@superset-ui/core'; +import { sanitizeFormData } from 'src/explore/exploreUtils/formData'; +import getControlsForVizType from 'src/utils/getControlsForVizType'; +import { safeStringify } from 'src/utils/safeStringify'; +import { Tooltip } from 'src/components/Tooltip'; +import ModalTrigger from '../ModalTrigger'; +import TableView from '../TableView'; + +// Define interfaces for props and state +interface FormData { + [key: string]: any; // Use a more specific type if possible for your form data +} + +interface AlteredSliceTagProps { + origFormData: FormData; + currentFormData: FormData; +} + +interface ControlMap { + [key: string]: { + label?: string; + type?: string; + }; +} + +interface Diff { + before: any; // Specify a more precise type if possible + after: any; // Specify a more precise type if possible +} + +interface Row { + control: string; + before: string; + after: string; +} + +interface AlteredSliceTagState { + rows: Row[]; + hasDiffs: boolean; + controlsMap: ControlMap; +} + +const StyledLabel = styled.span` + ${({ theme }) => ` + font-size: ${theme.typography.sizes.s}px; + color: ${theme.colors.grayscale.dark1}; + background-color: ${theme.colors.alert.base}; + + &:hover { + background-color: ${theme.colors.alert.dark1}; + } + `} +`; + +function alterForComparison(value: any): any { + // Treat `null`, `undefined`, and empty strings as equivalent + if (value === undefined || value === null || value === '') { + return null; + } + // Treat empty arrays and objects as equivalent to null + if (Array.isArray(value) && value.length === 0) { + return null; + } + if (typeof value === 'object' && Object.keys(value).length === 0) { + return null; + } + // Return the value unchanged if it doesn't meet the above conditions + return value; +} + +class AlteredSliceTag extends React.Component< + AlteredSliceTagProps, + AlteredSliceTagState +> { + constructor(props: AlteredSliceTagProps) { + super(props); + const diffs = this.getDiffs(props); + const controlsMap: ControlMap = getControlsForVizType( + props.origFormData.viz_type, + ); + const rows = this.getRowsFromDiffs(diffs, controlsMap); + + this.state = { rows, hasDiffs: !isEmpty(diffs), controlsMap }; + } + + UNSAFE_componentWillReceiveProps(newProps: AlteredSliceTagProps): void { + if (isEqual(this.props, newProps)) { + return; + } + const diffs = this.getDiffs(newProps); + this.setState(prevState => ({ + rows: this.getRowsFromDiffs(diffs, prevState.controlsMap), + hasDiffs: !isEmpty(diffs), + })); + } + + getRowsFromDiffs( + diffs: { [key: string]: Diff }, + controlsMap: ControlMap, + ): Row[] { + return Object.entries(diffs).map(([key, diff]) => ({ + control: (controlsMap[key] && controlsMap[key].label) || key, + before: this.formatValue(diff.before, key, controlsMap), + after: this.formatValue(diff.after, key, controlsMap), + })); + } + + getDiffs(props: AlteredSliceTagProps): { [key: string]: Diff } { + const ofd = sanitizeFormData(props.origFormData); + const cfd = sanitizeFormData(props.currentFormData); + const fdKeys = Object.keys(cfd); + const diffs: { [key: string]: Diff } = {}; + fdKeys.forEach(fdKey => { + if (!ofd[fdKey] && !cfd[fdKey]) { + return; + } + if (['filters', 'having', 'where'].includes(fdKey)) { + return; + } + if (!this.isEqualish(ofd[fdKey], cfd[fdKey])) { + diffs[fdKey] = { before: ofd[fdKey], after: cfd[fdKey] }; + } + }); + return diffs; + } + + isEqualish(val1: any, val2: any): boolean { + // Consider refining 'any' types + return isEqual(alterForComparison(val1), alterForComparison(val2)); + } + + formatValue(value: any, key: string, controlsMap: ControlMap): string { + // Consider refining 'any' types + // Format display value based on the control type + // or the value type + if (value === undefined) { + return 'N/A'; + } + if (value === null) { + return 'null'; + } + if (controlsMap[key]?.type === 'AdhocFilterControl') { + if (!value.length) { + return '[]'; + } + return value + .map(v => { + const filterVal = + v.comparator && v.comparator.constructor === Array + ? `[${v.comparator.join(', ')}]` + : v.comparator; + return `${v.subject} ${v.operator} ${filterVal}`; + }) + .join(', '); + } + if (controlsMap[key]?.type === 'BoundsControl') { + return `Min: ${value[0]}, Max: ${value[1]}`; + } + if (controlsMap[key]?.type === 'CollectionControl') { + return value.map(v => safeStringify(v)).join(', '); + } + if ( + controlsMap[key]?.type === 'MetricsControl' && + value.constructor === Array + ) { + const formattedValue = value.map(v => v?.label ?? v); + return formattedValue.length ? formattedValue.join(', ') : '[]'; + } + if (typeof value === 'boolean') { + return value ? 'true' : 'false'; + } + if (value.constructor === Array) { + const formattedValue = value.map(v => v?.label ?? v); + return formattedValue.length ? formattedValue.join(', ') : '[]'; + } + if (typeof value === 'string' || typeof value === 'number') { + return value; + } + return safeStringify(value); + } + + renderModalBody(): JSX.Element { + const columns = [ + { + accessor: 'control', + Header: t('Control'), + }, + { + accessor: 'before', + Header: t('Before'), + }, + { + accessor: 'after', + Header: t('After'), + }, + ]; + // set the wrap text in the specific columns. + const columnsForWrapText = ['Control', 'Before', 'After']; + + return ( + + ); + } + + renderTriggerNode(): JSX.Element { + return ( + + {t('Altered')} + + ); + } + + render(): JSX.Element | null { + // Return nothing if there are no differences + if (!this.state.hasDiffs) { + return null; + } + // Render the label-warning 'Altered' tag which the user may + // click to open a modal containing a table summarizing the + // differences in the slice + return ( + + ); + } +} + +export default AlteredSliceTag; From a660b16b4f78612937e7aa27ec4ea41ee2d89279 Mon Sep 17 00:00:00 2001 From: Enzo Martellucci Date: Tue, 6 Feb 2024 19:17:46 +0100 Subject: [PATCH 02/15] chore: Migration --- .../src/components/AlteredSliceTag/index.jsx | 226 ------------------ 1 file changed, 226 deletions(-) delete mode 100644 superset-frontend/src/components/AlteredSliceTag/index.jsx diff --git a/superset-frontend/src/components/AlteredSliceTag/index.jsx b/superset-frontend/src/components/AlteredSliceTag/index.jsx deleted file mode 100644 index 83458fc0a4091..0000000000000 --- a/superset-frontend/src/components/AlteredSliceTag/index.jsx +++ /dev/null @@ -1,226 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -import React from 'react'; -import PropTypes from 'prop-types'; -import { isEqual, isEmpty } from 'lodash'; -import { styled, t } from '@superset-ui/core'; -import { sanitizeFormData } from 'src/explore/exploreUtils/formData'; -import getControlsForVizType from 'src/utils/getControlsForVizType'; -import { safeStringify } from 'src/utils/safeStringify'; -import { Tooltip } from 'src/components/Tooltip'; -import ModalTrigger from '../ModalTrigger'; -import TableView from '../TableView'; - -const propTypes = { - origFormData: PropTypes.object.isRequired, - currentFormData: PropTypes.object.isRequired, -}; - -const StyledLabel = styled.span` - ${({ theme }) => ` - font-size: ${theme.typography.sizes.s}px; - color: ${theme.colors.grayscale.dark1}; - background-color: ${theme.colors.alert.base}; - - &: hover { - background-color: ${theme.colors.alert.dark1}; - } - `} -`; - -function alterForComparison(value) { - // Considering `[]`, `{}`, `null` and `undefined` as identical - // for this purpose - if (value === undefined || value === null || value === '') { - return null; - } - if (typeof value === 'object') { - if (Array.isArray(value) && value.length === 0) { - return null; - } - const keys = Object.keys(value); - if (keys && keys.length === 0) { - return null; - } - } - return value; -} - -export default class AlteredSliceTag extends React.Component { - constructor(props) { - super(props); - const diffs = this.getDiffs(props); - const controlsMap = getControlsForVizType(this.props.origFormData.viz_type); - const rows = this.getRowsFromDiffs(diffs, controlsMap); - - this.state = { rows, hasDiffs: !isEmpty(diffs), controlsMap }; - } - - UNSAFE_componentWillReceiveProps(newProps) { - // Update differences if need be - if (isEqual(this.props, newProps)) { - return; - } - const diffs = this.getDiffs(newProps); - this.setState(prevState => ({ - rows: this.getRowsFromDiffs(diffs, prevState.controlsMap), - hasDiffs: !isEmpty(diffs), - })); - } - - getRowsFromDiffs(diffs, controlsMap) { - return Object.entries(diffs).map(([key, diff]) => ({ - control: (controlsMap[key] && controlsMap[key].label) || key, - before: this.formatValue(diff.before, key, controlsMap), - after: this.formatValue(diff.after, key, controlsMap), - })); - } - - getDiffs(props) { - // Returns all properties that differ in the - // current form data and the saved form data - const ofd = sanitizeFormData(props.origFormData); - const cfd = sanitizeFormData(props.currentFormData); - - const fdKeys = Object.keys(cfd); - const diffs = {}; - fdKeys.forEach(fdKey => { - if (!ofd[fdKey] && !cfd[fdKey]) { - return; - } - if (['filters', 'having', 'where'].includes(fdKey)) { - return; - } - if (!this.isEqualish(ofd[fdKey], cfd[fdKey])) { - diffs[fdKey] = { before: ofd[fdKey], after: cfd[fdKey] }; - } - }); - return diffs; - } - - isEqualish(val1, val2) { - return isEqual(alterForComparison(val1), alterForComparison(val2)); - } - - formatValue(value, key, controlsMap) { - // Format display value based on the control type - // or the value type - if (value === undefined) { - return 'N/A'; - } - if (value === null) { - return 'null'; - } - if (controlsMap[key]?.type === 'AdhocFilterControl') { - if (!value.length) { - return '[]'; - } - return value - .map(v => { - const filterVal = - v.comparator && v.comparator.constructor === Array - ? `[${v.comparator.join(', ')}]` - : v.comparator; - return `${v.subject} ${v.operator} ${filterVal}`; - }) - .join(', '); - } - if (controlsMap[key]?.type === 'BoundsControl') { - return `Min: ${value[0]}, Max: ${value[1]}`; - } - if (controlsMap[key]?.type === 'CollectionControl') { - return value.map(v => safeStringify(v)).join(', '); - } - if ( - controlsMap[key]?.type === 'MetricsControl' && - value.constructor === Array - ) { - const formattedValue = value.map(v => v?.label ?? v); - return formattedValue.length ? formattedValue.join(', ') : '[]'; - } - if (typeof value === 'boolean') { - return value ? 'true' : 'false'; - } - if (value.constructor === Array) { - const formattedValue = value.map(v => v?.label ?? v); - return formattedValue.length ? formattedValue.join(', ') : '[]'; - } - if (typeof value === 'string' || typeof value === 'number') { - return value; - } - return safeStringify(value); - } - - renderModalBody() { - const columns = [ - { - accessor: 'control', - Header: t('Control'), - }, - { - accessor: 'before', - Header: t('Before'), - }, - { - accessor: 'after', - Header: t('After'), - }, - ]; - // set the wrap text in the specific columns. - const columnsForWrapText = ['Control', 'Before', 'After']; - - return ( - - ); - } - - renderTriggerNode() { - return ( - - {t('Altered')} - - ); - } - - render() { - // Return nothing if there are no differences - if (!this.state.hasDiffs) { - return null; - } - // Render the label-warning 'Altered' tag which the user may - // click to open a modal containing a table summarizing the - // differences in the slice - return ( - - ); - } -} - -AlteredSliceTag.propTypes = propTypes; From 5f45b87bda135d2bbd144607fb2bb08763498f3c Mon Sep 17 00:00:00 2001 From: Enzo Martellucci Date: Thu, 8 Feb 2024 09:40:16 +0100 Subject: [PATCH 03/15] chore: Removed all types any --- .../src/components/AlteredSliceTag/index.tsx | 43 +++++++++---------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/superset-frontend/src/components/AlteredSliceTag/index.tsx b/superset-frontend/src/components/AlteredSliceTag/index.tsx index 6866153280384..a88ba4369af84 100644 --- a/superset-frontend/src/components/AlteredSliceTag/index.tsx +++ b/superset-frontend/src/components/AlteredSliceTag/index.tsx @@ -18,7 +18,7 @@ */ import React from 'react'; import { isEqual, isEmpty } from 'lodash'; -import { styled, t } from '@superset-ui/core'; +import { QueryFormData, styled, t } from '@superset-ui/core'; import { sanitizeFormData } from 'src/explore/exploreUtils/formData'; import getControlsForVizType from 'src/utils/getControlsForVizType'; import { safeStringify } from 'src/utils/safeStringify'; @@ -26,17 +26,12 @@ import { Tooltip } from 'src/components/Tooltip'; import ModalTrigger from '../ModalTrigger'; import TableView from '../TableView'; -// Define interfaces for props and state -interface FormData { - [key: string]: any; // Use a more specific type if possible for your form data -} - interface AlteredSliceTagProps { - origFormData: FormData; - currentFormData: FormData; + origFormData: QueryFormData; + currentFormData: QueryFormData; } -interface ControlMap { +export interface ControlMap { [key: string]: { label?: string; type?: string; @@ -44,8 +39,8 @@ interface ControlMap { } interface Diff { - before: any; // Specify a more precise type if possible - after: any; // Specify a more precise type if possible + before: []; + after: []; } interface Row { @@ -53,7 +48,12 @@ interface Row { before: string; after: string; } - +interface FilterItem { + comparator?: string | string[]; + subject: string; + operator: string; + label?: string; +} interface AlteredSliceTagState { rows: Row[]; hasDiffs: boolean; @@ -72,7 +72,7 @@ const StyledLabel = styled.span` `} `; -function alterForComparison(value: any): any { +function alterForComparison(value: string): [] | Object | null { // Treat `null`, `undefined`, and empty strings as equivalent if (value === undefined || value === null || value === '') { return null; @@ -84,7 +84,6 @@ function alterForComparison(value: any): any { if (typeof value === 'object' && Object.keys(value).length === 0) { return null; } - // Return the value unchanged if it doesn't meet the above conditions return value; } @@ -97,7 +96,7 @@ class AlteredSliceTag extends React.Component< const diffs = this.getDiffs(props); const controlsMap: ControlMap = getControlsForVizType( props.origFormData.viz_type, - ); + ) as ControlMap; const rows = this.getRowsFromDiffs(diffs, controlsMap); this.state = { rows, hasDiffs: !isEmpty(diffs), controlsMap }; @@ -119,7 +118,7 @@ class AlteredSliceTag extends React.Component< controlsMap: ControlMap, ): Row[] { return Object.entries(diffs).map(([key, diff]) => ({ - control: (controlsMap[key] && controlsMap[key].label) || key, + control: controlsMap[key]?.label || key, before: this.formatValue(diff.before, key, controlsMap), after: this.formatValue(diff.after, key, controlsMap), })); @@ -144,15 +143,15 @@ class AlteredSliceTag extends React.Component< return diffs; } - isEqualish(val1: any, val2: any): boolean { - // Consider refining 'any' types + isEqualish(val1: string, val2: string): boolean { return isEqual(alterForComparison(val1), alterForComparison(val2)); } - formatValue(value: any, key: string, controlsMap: ControlMap): string { - // Consider refining 'any' types - // Format display value based on the control type - // or the value type + formatValue( + value: FilterItem[], + key: string, + controlsMap: ControlMap, + ): string { if (value === undefined) { return 'N/A'; } From 63a4e4a55d78d50dee82f787113e36a8206ebf7a Mon Sep 17 00:00:00 2001 From: Enzo Martellucci <52219496+EnxDev@users.noreply.github.com> Date: Thu, 8 Feb 2024 11:09:15 +0100 Subject: [PATCH 04/15] Update superset-frontend/src/components/AlteredSliceTag/index.tsx Co-authored-by: Geido <60598000+geido@users.noreply.github.com> --- superset-frontend/src/components/AlteredSliceTag/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/src/components/AlteredSliceTag/index.tsx b/superset-frontend/src/components/AlteredSliceTag/index.tsx index a88ba4369af84..2770c568dd73a 100644 --- a/superset-frontend/src/components/AlteredSliceTag/index.tsx +++ b/superset-frontend/src/components/AlteredSliceTag/index.tsx @@ -235,7 +235,7 @@ class AlteredSliceTag extends React.Component< ); } - render(): JSX.Element | null { + render() { // Return nothing if there are no differences if (!this.state.hasDiffs) { return null; From 0864107007157a9fce849efd9d83443d3dbeec02 Mon Sep 17 00:00:00 2001 From: Enzo Martellucci <52219496+EnxDev@users.noreply.github.com> Date: Thu, 8 Feb 2024 11:09:24 +0100 Subject: [PATCH 05/15] Update superset-frontend/src/components/AlteredSliceTag/index.tsx Co-authored-by: Geido <60598000+geido@users.noreply.github.com> --- superset-frontend/src/components/AlteredSliceTag/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/src/components/AlteredSliceTag/index.tsx b/superset-frontend/src/components/AlteredSliceTag/index.tsx index 2770c568dd73a..b9fecd9bd218e 100644 --- a/superset-frontend/src/components/AlteredSliceTag/index.tsx +++ b/superset-frontend/src/components/AlteredSliceTag/index.tsx @@ -198,7 +198,7 @@ class AlteredSliceTag extends React.Component< return safeStringify(value); } - renderModalBody(): JSX.Element { + renderModalBody(): React.ReactNode { const columns = [ { accessor: 'control', From b093434e4973037d3dd07ac2e5422eb159ce5038 Mon Sep 17 00:00:00 2001 From: Enzo Martellucci <52219496+EnxDev@users.noreply.github.com> Date: Thu, 8 Feb 2024 11:09:42 +0100 Subject: [PATCH 06/15] Update superset-frontend/src/components/AlteredSliceTag/index.tsx Co-authored-by: Geido <60598000+geido@users.noreply.github.com> --- superset-frontend/src/components/AlteredSliceTag/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/src/components/AlteredSliceTag/index.tsx b/superset-frontend/src/components/AlteredSliceTag/index.tsx index b9fecd9bd218e..400c73b92cd0e 100644 --- a/superset-frontend/src/components/AlteredSliceTag/index.tsx +++ b/superset-frontend/src/components/AlteredSliceTag/index.tsx @@ -227,7 +227,7 @@ class AlteredSliceTag extends React.Component< ); } - renderTriggerNode(): JSX.Element { + renderTriggerNode(): React.ReactNode { return ( {t('Altered')} From 532b67956ca58ccf7d3cd492612822e0f5556d39 Mon Sep 17 00:00:00 2001 From: Enzo Martellucci Date: Thu, 8 Feb 2024 21:50:21 +0100 Subject: [PATCH 07/15] chore: Migration js file to ts file --- ...lteredSliceTagMocks.js => AlteredSliceTagMocks.ts} | 11 +++++++---- .../src/components/AlteredSliceTag/index.tsx | 6 +++--- 2 files changed, 10 insertions(+), 7 deletions(-) rename superset-frontend/src/components/AlteredSliceTag/{AlteredSliceTagMocks.js => AlteredSliceTagMocks.ts} (90%) diff --git a/superset-frontend/src/components/AlteredSliceTag/AlteredSliceTagMocks.js b/superset-frontend/src/components/AlteredSliceTag/AlteredSliceTagMocks.ts similarity index 90% rename from superset-frontend/src/components/AlteredSliceTag/AlteredSliceTagMocks.js rename to superset-frontend/src/components/AlteredSliceTag/AlteredSliceTagMocks.ts index 90b243f9914f3..843f70d9ea156 100644 --- a/superset-frontend/src/components/AlteredSliceTag/AlteredSliceTagMocks.js +++ b/superset-frontend/src/components/AlteredSliceTag/AlteredSliceTagMocks.ts @@ -16,8 +16,11 @@ * specific language governing permissions and limitations * under the License. */ +import { QueryFormData } from '@superset-ui/core'; +import { ControlPanelConfig } from 'packages/superset-ui-chart-controls/src/types'; +import { Row } from './index'; -export const defaultProps = { +export const defaultProps: Record> = { origFormData: { viz_type: 'altered_slice_tag_spec', adhoc_filters: [ @@ -57,7 +60,7 @@ export const defaultProps = { }, }; -export const expectedDiffs = { +export const expectedDiffs: Record> = { adhoc_filters: { before: [ { @@ -103,7 +106,7 @@ export const expectedDiffs = { after: { x: 'y', z: 'z' }, }, }; -export const expectedRows = [ +export const expectedRows: Partial[] = [ { control: 'Fake Filters', before: 'a == hello', @@ -128,7 +131,7 @@ export const expectedRows = [ after: '{"x":"y","z":"z"}', }, ]; -export const fakePluginControls = { +export const fakePluginControls: ControlPanelConfig = { controlPanelSections: [ { label: 'Fake Control Panel Sections', diff --git a/superset-frontend/src/components/AlteredSliceTag/index.tsx b/superset-frontend/src/components/AlteredSliceTag/index.tsx index 400c73b92cd0e..d85ab29695d1f 100644 --- a/superset-frontend/src/components/AlteredSliceTag/index.tsx +++ b/superset-frontend/src/components/AlteredSliceTag/index.tsx @@ -43,10 +43,10 @@ interface Diff { after: []; } -interface Row { +export interface Row { control: string; - before: string; - after: string; + before: string | number; + after: string | number; } interface FilterItem { comparator?: string | string[]; From 1d99a25d0425cfa340ca1cfd195475b083b77b4a Mon Sep 17 00:00:00 2001 From: Enzo Martellucci <52219496+EnxDev@users.noreply.github.com> Date: Fri, 9 Feb 2024 22:04:00 +0100 Subject: [PATCH 08/15] Update superset-frontend/src/components/AlteredSliceTag/index.tsx Co-authored-by: Geido <60598000+geido@users.noreply.github.com> --- superset-frontend/src/components/AlteredSliceTag/index.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/superset-frontend/src/components/AlteredSliceTag/index.tsx b/superset-frontend/src/components/AlteredSliceTag/index.tsx index d85ab29695d1f..36a1bd37ef929 100644 --- a/superset-frontend/src/components/AlteredSliceTag/index.tsx +++ b/superset-frontend/src/components/AlteredSliceTag/index.tsx @@ -48,6 +48,7 @@ export interface Row { before: string | number; after: string | number; } + interface FilterItem { comparator?: string | string[]; subject: string; From ea3d6e88014cf04479f987a5818e288a023f95b3 Mon Sep 17 00:00:00 2001 From: Enzo Martellucci Date: Mon, 12 Feb 2024 22:28:43 +0100 Subject: [PATCH 09/15] chore: Enhance types/interfaces --- .../src/components/AlteredSliceTag/index.tsx | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/superset-frontend/src/components/AlteredSliceTag/index.tsx b/superset-frontend/src/components/AlteredSliceTag/index.tsx index d85ab29695d1f..3f341f63c7d2e 100644 --- a/superset-frontend/src/components/AlteredSliceTag/index.tsx +++ b/superset-frontend/src/components/AlteredSliceTag/index.tsx @@ -38,22 +38,24 @@ export interface ControlMap { }; } -interface Diff { +type Diff = { before: []; after: []; -} +}; -export interface Row { +export type Row = { control: string; before: string | number; after: string | number; -} -interface FilterItem { +}; + +type FilterItem = { comparator?: string | string[]; subject: string; operator: string; label?: string; -} +}; + interface AlteredSliceTagState { rows: Row[]; hasDiffs: boolean; From b27196e78ac8801ffc0c5c90b346da2ff699ca77 Mon Sep 17 00:00:00 2001 From: Enzo Martellucci Date: Mon, 12 Feb 2024 22:39:07 +0100 Subject: [PATCH 10/15] chore: Enhance types/interfaces --- superset-frontend/src/components/AlteredSliceTag/index.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/superset-frontend/src/components/AlteredSliceTag/index.tsx b/superset-frontend/src/components/AlteredSliceTag/index.tsx index 51f66afa24480..3f341f63c7d2e 100644 --- a/superset-frontend/src/components/AlteredSliceTag/index.tsx +++ b/superset-frontend/src/components/AlteredSliceTag/index.tsx @@ -49,12 +49,12 @@ export type Row = { after: string | number; }; -interface FilterItem { +type FilterItem = { comparator?: string | string[]; subject: string; operator: string; label?: string; -} +}; interface AlteredSliceTagState { rows: Row[]; From b9305d055b0a5dee9213d24bf16c83d61ea7ad76 Mon Sep 17 00:00:00 2001 From: geido Date: Tue, 13 Feb 2024 13:44:37 +0200 Subject: [PATCH 11/15] Enhance types --- .../AlteredSliceTag/AlteredSliceTagMocks.ts | 6 +- .../src/components/AlteredSliceTag/index.tsx | 68 +++++++++++-------- 2 files changed, 44 insertions(+), 30 deletions(-) diff --git a/superset-frontend/src/components/AlteredSliceTag/AlteredSliceTagMocks.ts b/superset-frontend/src/components/AlteredSliceTag/AlteredSliceTagMocks.ts index 843f70d9ea156..233f519446d70 100644 --- a/superset-frontend/src/components/AlteredSliceTag/AlteredSliceTagMocks.ts +++ b/superset-frontend/src/components/AlteredSliceTag/AlteredSliceTagMocks.ts @@ -18,7 +18,7 @@ */ import { QueryFormData } from '@superset-ui/core'; import { ControlPanelConfig } from 'packages/superset-ui-chart-controls/src/types'; -import { Row } from './index'; +import { DiffType, RowType } from './index'; export const defaultProps: Record> = { origFormData: { @@ -60,7 +60,7 @@ export const defaultProps: Record> = { }, }; -export const expectedDiffs: Record> = { +export const expectedDiffs: Record = { adhoc_filters: { before: [ { @@ -106,7 +106,7 @@ export const expectedDiffs: Record> = { after: { x: 'y', z: 'z' }, }, }; -export const expectedRows: Partial[] = [ +export const expectedRows: RowType[] = [ { control: 'Fake Filters', before: 'a == hello', diff --git a/superset-frontend/src/components/AlteredSliceTag/index.tsx b/superset-frontend/src/components/AlteredSliceTag/index.tsx index 3f341f63c7d2e..dfedc9f5b651f 100644 --- a/superset-frontend/src/components/AlteredSliceTag/index.tsx +++ b/superset-frontend/src/components/AlteredSliceTag/index.tsx @@ -38,26 +38,37 @@ export interface ControlMap { }; } -type Diff = { - before: []; - after: []; -}; - -export type Row = { - control: string; - before: string | number; - after: string | number; -}; - -type FilterItem = { +type FilterItemType = { comparator?: string | string[]; subject: string; operator: string; label?: string; }; +export type DiffItemType< + T = FilterItemType | number | string | Record, +> = + | T[] + | boolean + | number + | string + | Record + | null + | undefined; + +export type DiffType = { + before: DiffItemType; + after: DiffItemType; +}; + +export type RowType = { + before: string | number; + after: string | number; + control: string; +}; + interface AlteredSliceTagState { - rows: Row[]; + rows: RowType[]; hasDiffs: boolean; controlsMap: ControlMap; } @@ -74,7 +85,7 @@ const StyledLabel = styled.span` `} `; -function alterForComparison(value: string): [] | Object | null { +function alterForComparison(value?: string | null | []): string | null { // Treat `null`, `undefined`, and empty strings as equivalent if (value === undefined || value === null || value === '') { return null; @@ -116,9 +127,9 @@ class AlteredSliceTag extends React.Component< } getRowsFromDiffs( - diffs: { [key: string]: Diff }, + diffs: { [key: string]: DiffType }, controlsMap: ControlMap, - ): Row[] { + ): RowType[] { return Object.entries(diffs).map(([key, diff]) => ({ control: controlsMap[key]?.label || key, before: this.formatValue(diff.before, key, controlsMap), @@ -126,11 +137,11 @@ class AlteredSliceTag extends React.Component< })); } - getDiffs(props: AlteredSliceTagProps): { [key: string]: Diff } { + getDiffs(props: AlteredSliceTagProps): { [key: string]: DiffType } { const ofd = sanitizeFormData(props.origFormData); const cfd = sanitizeFormData(props.currentFormData); const fdKeys = Object.keys(cfd); - const diffs: { [key: string]: Diff } = {}; + const diffs: { [key: string]: DiffType } = {}; fdKeys.forEach(fdKey => { if (!ofd[fdKey] && !cfd[fdKey]) { return; @@ -150,17 +161,20 @@ class AlteredSliceTag extends React.Component< } formatValue( - value: FilterItem[], + value: DiffItemType, key: string, controlsMap: ControlMap, - ): string { + ): string | number { if (value === undefined) { return 'N/A'; } if (value === null) { return 'null'; } - if (controlsMap[key]?.type === 'AdhocFilterControl') { + if ( + controlsMap[key]?.type === 'AdhocFilterControl' && + Array.isArray(value) + ) { if (!value.length) { return '[]'; } @@ -177,20 +191,20 @@ class AlteredSliceTag extends React.Component< if (controlsMap[key]?.type === 'BoundsControl') { return `Min: ${value[0]}, Max: ${value[1]}`; } - if (controlsMap[key]?.type === 'CollectionControl') { - return value.map(v => safeStringify(v)).join(', '); - } if ( - controlsMap[key]?.type === 'MetricsControl' && - value.constructor === Array + controlsMap[key]?.type === 'CollectionControl' && + Array.isArray(value) ) { + return value.map(v => safeStringify(v)).join(', '); + } + if (controlsMap[key]?.type === 'MetricsControl' && Array.isArray(value)) { const formattedValue = value.map(v => v?.label ?? v); return formattedValue.length ? formattedValue.join(', ') : '[]'; } if (typeof value === 'boolean') { return value ? 'true' : 'false'; } - if (value.constructor === Array) { + if (Array.isArray(value)) { const formattedValue = value.map(v => v?.label ?? v); return formattedValue.length ? formattedValue.join(', ') : '[]'; } From 33b6cfd374290295d5fc5853cdbd98ad616ade59 Mon Sep 17 00:00:00 2001 From: Enzo Martellucci Date: Thu, 15 Feb 2024 18:35:29 +0100 Subject: [PATCH 12/15] chore: Migration jsx file to tsx file --- .../ErrorBoundary/ErrorBoundary.test.tsx | 8 +-- .../ErrorBoundary/{index.jsx => index.tsx} | 53 +++++++++++-------- 2 files changed, 36 insertions(+), 25 deletions(-) rename superset-frontend/src/components/ErrorBoundary/{index.jsx => index.tsx} (61%) diff --git a/superset-frontend/src/components/ErrorBoundary/ErrorBoundary.test.tsx b/superset-frontend/src/components/ErrorBoundary/ErrorBoundary.test.tsx index 75ddc1c6fa9dd..0226bd731c4d6 100644 --- a/superset-frontend/src/components/ErrorBoundary/ErrorBoundary.test.tsx +++ b/superset-frontend/src/components/ErrorBoundary/ErrorBoundary.test.tsx @@ -18,15 +18,15 @@ */ import React from 'react'; import { render, screen } from 'spec/helpers/testing-library'; -import ErrorBoundary from '.'; +import ErrorBoundary, { ErrorBoundaryProps } from '.'; -const mockedProps = { +const mockedProps: Partial = { children: Error children, - onError: () => null, + onError: jest.fn(), showMessage: false, }; -const Child = () => { +const Child = (): React.ReactElement => { throw new Error('Thrown error'); }; diff --git a/superset-frontend/src/components/ErrorBoundary/index.jsx b/superset-frontend/src/components/ErrorBoundary/index.tsx similarity index 61% rename from superset-frontend/src/components/ErrorBoundary/index.jsx rename to superset-frontend/src/components/ErrorBoundary/index.tsx index 0a1d0c7c46510..9ca156ce17a8a 100644 --- a/superset-frontend/src/components/ErrorBoundary/index.jsx +++ b/superset-frontend/src/components/ErrorBoundary/index.tsx @@ -17,47 +17,60 @@ * under the License. */ import React from 'react'; -import PropTypes from 'prop-types'; import { t } from '@superset-ui/core'; import ErrorMessageWithStackTrace from 'src/components/ErrorMessage/ErrorMessageWithStackTrace'; -const propTypes = { - children: PropTypes.node.isRequired, - onError: PropTypes.func, - showMessage: PropTypes.bool, -}; -const defaultProps = { - onError: () => {}, - showMessage: true, -}; +export interface ErrorBoundaryProps { + children: React.ReactNode; + onError?: (error: Error, info: React.ErrorInfo) => void; + showMessage?: boolean; +} + +interface ErrorBoundaryState { + error: Error | null; + info: React.ErrorInfo | null; +} -export default class ErrorBoundary extends React.Component { - constructor(props) { +export default class ErrorBoundary extends React.Component< + ErrorBoundaryProps, + ErrorBoundaryState +> { + static defaultProps: Partial = { + showMessage: true, + }; + + constructor(props: ErrorBoundaryProps) { super(props); this.state = { error: null, info: null }; } - componentDidCatch(error, info) { - if (this.props.onError) this.props.onError(error, info); + componentDidCatch(error: Error, info: React.ErrorInfo): void { + this.props.onError?.(error, info); this.setState({ error, info }); } render() { - const { error, info } = this.state; + const { error } = this.state; if (error) { const firstLine = error.toString(); - const message = ( + const messageString = `${t('Unexpected error')}${ + firstLine ? `: ${firstLine}` : '' + }`; + const messageElement = ( {t('Unexpected error')} {firstLine ? `: ${firstLine}` : ''} ); + if (this.props.showMessage) { return ( ); } @@ -66,5 +79,3 @@ export default class ErrorBoundary extends React.Component { return this.props.children; } } -ErrorBoundary.propTypes = propTypes; -ErrorBoundary.defaultProps = defaultProps; From 60793a5aa8e294f47f0d6d4b6f5fcf15c19174c1 Mon Sep 17 00:00:00 2001 From: Enzo Martellucci <52219496+EnxDev@users.noreply.github.com> Date: Fri, 16 Feb 2024 20:23:25 +0100 Subject: [PATCH 13/15] Update index.tsx Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> --- superset-frontend/src/components/ErrorBoundary/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/src/components/ErrorBoundary/index.tsx b/superset-frontend/src/components/ErrorBoundary/index.tsx index 9ca156ce17a8a..22636a8ad8d45 100644 --- a/superset-frontend/src/components/ErrorBoundary/index.tsx +++ b/superset-frontend/src/components/ErrorBoundary/index.tsx @@ -50,7 +50,7 @@ export default class ErrorBoundary extends React.Component< } render() { - const { error } = this.state; + const { error, info } = this.state; if (error) { const firstLine = error.toString(); const messageString = `${t('Unexpected error')}${ From 47141c1777699b2fe25fc8a741e0c3075f42b551 Mon Sep 17 00:00:00 2001 From: Enzo Martellucci <52219496+EnxDev@users.noreply.github.com> Date: Fri, 16 Feb 2024 20:23:41 +0100 Subject: [PATCH 14/15] Update index.tsx Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> --- superset-frontend/src/components/ErrorBoundary/index.tsx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/superset-frontend/src/components/ErrorBoundary/index.tsx b/superset-frontend/src/components/ErrorBoundary/index.tsx index 22636a8ad8d45..0c4efe1b50998 100644 --- a/superset-frontend/src/components/ErrorBoundary/index.tsx +++ b/superset-frontend/src/components/ErrorBoundary/index.tsx @@ -68,9 +68,7 @@ export default class ErrorBoundary extends React.Component< ); } From 9948196a02ed6285945b3807c5067a10f37024e4 Mon Sep 17 00:00:00 2001 From: Enzo Martellucci Date: Wed, 28 Feb 2024 14:41:12 +0100 Subject: [PATCH 15/15] chore: Migration jsx file to tsx file --- .../CopyToClipboard/{index.jsx => index.tsx} | 54 +++++++++---------- 1 file changed, 25 insertions(+), 29 deletions(-) rename superset-frontend/src/components/CopyToClipboard/{index.jsx => index.tsx} (74%) diff --git a/superset-frontend/src/components/CopyToClipboard/index.jsx b/superset-frontend/src/components/CopyToClipboard/index.tsx similarity index 74% rename from superset-frontend/src/components/CopyToClipboard/index.jsx rename to superset-frontend/src/components/CopyToClipboard/index.tsx index d4f74904dbca4..6cdc5c084aba7 100644 --- a/superset-frontend/src/components/CopyToClipboard/index.jsx +++ b/superset-frontend/src/components/CopyToClipboard/index.tsx @@ -17,26 +17,25 @@ * under the License. */ import React from 'react'; -import PropTypes from 'prop-types'; import { t } from '@superset-ui/core'; import { Tooltip } from 'src/components/Tooltip'; import withToasts from 'src/components/MessageToasts/withToasts'; import copyTextToClipboard from 'src/utils/copy'; -const propTypes = { - copyNode: PropTypes.node, - getText: PropTypes.func, - onCopyEnd: PropTypes.func, - shouldShowText: PropTypes.bool, - text: PropTypes.string, - wrapped: PropTypes.bool, - tooltipText: PropTypes.string, - addDangerToast: PropTypes.func.isRequired, - addSuccessToast: PropTypes.func.isRequired, - hideTooltip: PropTypes.bool, -}; +export interface CopyToClipboardProps { + copyNode?: React.ReactNode; + getText?: (callback: (data: string) => void) => void; + onCopyEnd?: () => void; + shouldShowText?: boolean; + text?: string; + wrapped?: boolean; + tooltipText?: string; + addDangerToast: (msg: string) => void; + addSuccessToast: (msg: string) => void; + hideTooltip?: boolean; +} -const defaultProps = { +const defaultProps: Partial = { copyNode: {t('Copy')}, onCopyEnd: () => {}, shouldShowText: true, @@ -45,33 +44,33 @@ const defaultProps = { hideTooltip: false, }; -class CopyToClipboard extends React.Component { - constructor(props) { - super(props); +class CopyToClipboard extends React.Component { + static defaultProps = defaultProps; - // bindings + constructor(props: CopyToClipboardProps) { + super(props); this.copyToClipboard = this.copyToClipboard.bind(this); this.onClick = this.onClick.bind(this); } onClick() { if (this.props.getText) { - this.props.getText(d => { + this.props.getText((d: string) => { this.copyToClipboard(Promise.resolve(d)); }); } else { - this.copyToClipboard(Promise.resolve(this.props.text)); + this.copyToClipboard(Promise.resolve(this.props.text || '')); } } getDecoratedCopyNode() { - return React.cloneElement(this.props.copyNode, { + return React.cloneElement(this.props.copyNode as React.ReactElement, { style: { cursor: 'pointer' }, onClick: this.onClick, }); } - copyToClipboard(textToCopy) { + copyToClipboard(textToCopy: Promise) { copyTextToClipboard(() => textToCopy) .then(() => { this.props.addSuccessToast(t('Copied to clipboard!')); @@ -84,11 +83,11 @@ class CopyToClipboard extends React.Component { ); }) .finally(() => { - this.props.onCopyEnd(); + if (this.props.onCopyEnd) this.props.onCopyEnd(); }); } - renderTooltip(cursor) { + renderTooltip(cursor: string) { return ( <> {!this.props.hideTooltip ? ( @@ -96,7 +95,7 @@ class CopyToClipboard extends React.Component { id="copy-to-clipboard-tooltip" placement="topRight" style={{ cursor }} - title={this.props.tooltipText} + title={this.props.tooltipText || ''} trigger={['hover']} arrowPointAtCenter > @@ -121,7 +120,7 @@ class CopyToClipboard extends React.Component { {this.props.text} )} - {this.renderTooltip()} + {this.renderTooltip('pointer')} ); } @@ -136,6 +135,3 @@ class CopyToClipboard extends React.Component { } export default withToasts(CopyToClipboard); - -CopyToClipboard.propTypes = propTypes; -CopyToClipboard.defaultProps = defaultProps;