From 1956c5f5a3fed88cf8a8927494dc5dcf64f876a2 Mon Sep 17 00:00:00 2001 From: Antonio Rivero Date: Tue, 30 Jan 2024 18:12:37 +0100 Subject: [PATCH] Period Over Period Plugin: - Fix format and lint - Rewrite some functions to help with type checks - Use css when possible in the Plugin component --- superset-frontend/package.json | 2 +- .../README.md | 2 + .../src/PopKPI.tsx | 123 ++++++++---------- .../src/plugin/buildQuery.ts | 104 ++++++++------- .../src/plugin/transformProps.ts | 27 +++- .../src/types.ts | 4 + .../tsconfig.json | 2 +- .../src/visualizations/presets/MainPreset.js | 4 +- 8 files changed, 147 insertions(+), 121 deletions(-) diff --git a/superset-frontend/package.json b/superset-frontend/package.json index d1a127b5289b0..5e0d2dca3d89c 100644 --- a/superset-frontend/package.json +++ b/superset-frontend/package.json @@ -109,10 +109,10 @@ "@superset-ui/legacy-preset-chart-nvd3": "file:./plugins/legacy-preset-chart-nvd3", "@superset-ui/plugin-chart-echarts": "file:./plugins/plugin-chart-echarts", "@superset-ui/plugin-chart-handlebars": "file:./plugins/plugin-chart-handlebars", + "@superset-ui/plugin-chart-period-over-period-kpi": "file:./plugins/plugin-chart-period-over-period-kpi", "@superset-ui/plugin-chart-pivot-table": "file:./plugins/plugin-chart-pivot-table", "@superset-ui/plugin-chart-table": "file:./plugins/plugin-chart-table", "@superset-ui/plugin-chart-word-cloud": "file:./plugins/plugin-chart-word-cloud", - "@superset-ui/plugin-chart-period-over-period-kpi": "file:./plugins/plugin-chart-period-over-period-kpi", "@superset-ui/switchboard": "file:./packages/superset-ui-switchboard", "@types/d3-format": "^3.0.1", "@visx/axis": "^3.0.1", diff --git a/superset-frontend/plugins/plugin-chart-period-over-period-kpi/README.md b/superset-frontend/plugins/plugin-chart-period-over-period-kpi/README.md index fad228dfd4591..2a049731da43c 100644 --- a/superset-frontend/plugins/plugin-chart-period-over-period-kpi/README.md +++ b/superset-frontend/plugins/plugin-chart-period-over-period-kpi/README.md @@ -18,6 +18,7 @@ npm run dev ``` To add the package to Superset, go to the `superset-frontend` subdirectory in your Superset source folder (assuming both the `custom-viz` plugin and `superset` repos are in the same root directory) and run + ``` npm i -S ../../custom-viz ``` @@ -54,6 +55,7 @@ import { CustomViz } from 'custom-viz'; ``` to import the plugin and later add the following to the array that's passed to the `plugins` property: + ```js new CustomViz().configure({ key: 'custom-viz' }), ``` diff --git a/superset-frontend/plugins/plugin-chart-period-over-period-kpi/src/PopKPI.tsx b/superset-frontend/plugins/plugin-chart-period-over-period-kpi/src/PopKPI.tsx index d38426494056b..e780e93ca4efb 100644 --- a/superset-frontend/plugins/plugin-chart-period-over-period-kpi/src/PopKPI.tsx +++ b/superset-frontend/plugins/plugin-chart-period-over-period-kpi/src/PopKPI.tsx @@ -16,53 +16,18 @@ * specific language governing permissions and limitations * under the License. */ -import React, { useEffect, createRef } from 'react'; -import { - styled, - } from '@superset-ui/core'; -import { PopKPIProps, PopKPIStylesProps } from './types'; +import React, { createRef } from 'react'; +import { css, styled, useTheme } from '@superset-ui/core'; +import { PopKPIComparisonValueStyleProps, PopKPIProps } from './types'; -// The following Styles component is a
element, which has been styled using Emotion -// For docs, visit https://emotion.sh/docs/styled - -// Theming variables are provided for your use via a ThemeProvider -// imported from @superset-ui/core. For variables available, please visit -// https://github.com/apache-superset/superset-ui/blob/master/packages/superset-ui-core/src/style/index.ts - -const Styles = styled.div` - - font-family: ${({ theme }) => theme.typography.families.sansSerif}; - position: relative; - display: flex; - flex-direction: column; - justify-content: center; - padding: ${({ theme }) => theme.tdUnit * 4}px; - border-radius: ${({ theme }) => theme.tdUnit * 2}px; - height: ${({ height }) => height}px; - width: ${({ width }) => width}px; -`; - -const BigValueContainer = styled.div` - font-size: ${props=> props.headerFontSize ? props.headerFontSize : 60}px; - font-weight: ${({ theme }) => theme.typography.weights.normal}; - text-align: center; -`; - -const TableContainer = styled.div` - width: 100%; - display: table; -` - -const ComparisonContainer = styled.div` - display: table-row; -`; - -const ComparisonValue = styled.div` - font-weight: ${({ theme }) => theme.typography.weights.light}; - width: 33%; - display: table-cell; - font-size: ${props=> props.subheaderFontSize ? props.subheaderFontSize : 20}px; - text-align: center; +const ComparisonValue = styled.div` + ${({ theme, subheaderFontSize }) => ` + font-weight: ${theme.typography.weights.light}; + width: 33%; + display: table-cell; + font-size: ${subheaderFontSize || 20}px; + text-align: center; + `} `; export default function PopKPI(props: PopKPIProps) { @@ -78,28 +43,54 @@ export default function PopKPI(props: PopKPIProps) { } = props; const rootElem = createRef(); + const theme = useTheme(); - useEffect(() => { - const root = rootElem.current as HTMLElement; - }); + const wrapperDivStyles = css` + font-family: ${theme.typography.families.sansSerif}; + position: relative; + display: flex; + flex-direction: column; + justify-content: center; + padding: ${theme.gridUnit * 4}px; + border-radius: ${theme.gridUnit * 2}px; + height: ${height}px; + width: ${width}px; + `; - return ( - + const bigValueContainerStyles = css` + font-size: ${headerFontSize || 60}px; + font-weight: ${theme.typography.weights.normal}; + text-align: center; + `; - {bigNumber} - - - #: {prevNumber} - Δ: {valueDifference} - %: {percentDifference} - - - + return ( +
+
{bigNumber}
+
+
+ + {' '} + #: {prevNumber} + + + {' '} + Δ: {valueDifference} + + + {' '} + %: {percentDifference} + +
+
+
); } diff --git a/superset-frontend/plugins/plugin-chart-period-over-period-kpi/src/plugin/buildQuery.ts b/superset-frontend/plugins/plugin-chart-period-over-period-kpi/src/plugin/buildQuery.ts index b009bf90383f6..202063c13c5f8 100644 --- a/superset-frontend/plugins/plugin-chart-period-over-period-kpi/src/plugin/buildQuery.ts +++ b/superset-frontend/plugins/plugin-chart-period-over-period-kpi/src/plugin/buildQuery.ts @@ -16,7 +16,11 @@ * specific language governing permissions and limitations * under the License. */ -import { buildQueryContext, QueryFormData } from '@superset-ui/core'; +import { + AdhocFilter, + buildQueryContext, + QueryFormData, +} from '@superset-ui/core'; import moment, { Moment } from 'moment'; /** @@ -42,8 +46,8 @@ function getSinceUntil( relativeEnd: string | null = null, ): MomentTuple { const separator = ' : '; - const _relativeStart = relativeStart || "today"; - const _relativeEnd = relativeEnd || "today"; + const effectiveRelativeStart = relativeStart || 'today'; + const effectiveRelativeEnd = relativeEnd || 'today'; if (!timeRange) { return [null, null]; @@ -51,16 +55,16 @@ function getSinceUntil( let modTimeRange: string | null = timeRange; - if (timeRange === 'NO_TIME_RANGE' || timeRange === '_(NO_TIME_RANGE)'){ + if (timeRange === 'NO_TIME_RANGE' || timeRange === '_(NO_TIME_RANGE)') { return [null, null]; } if (timeRange?.startsWith('last') && !timeRange.includes(separator)) { - modTimeRange = timeRange + separator + _relativeEnd; + modTimeRange = timeRange + separator + effectiveRelativeEnd; } if (timeRange?.startsWith('next') && !timeRange.includes(separator)) { - modTimeRange = _relativeStart + separator + timeRange; + modTimeRange = effectiveRelativeStart + separator + timeRange; } if ( @@ -179,12 +183,15 @@ function getSinceUntil( return [_since, _until]; } -function calculatePrev(startDate: Moment | null, endDate: Moment | null, calcType: String) { - - if (!startDate || !endDate){ - return [null, null] +function calculatePrev( + startDate: Moment | null, + endDate: Moment | null, + calcType: String, +) { + if (!startDate || !endDate) { + return [null, null]; } - + const daysBetween = endDate.diff(startDate, 'days'); let startDatePrev = moment(); @@ -219,53 +226,63 @@ export default function buildQuery(formData: QueryFormData) { }, ]); + const timeFilterIndex: number = + formData.adhoc_filters?.findIndex( + filter => 'operator' in filter && filter.operator === 'TEMPORAL_RANGE', + ) ?? -1; - const timeFilter: any = formData.adhoc_filters?.find( - ({ operator }: { operator: string }) => operator === 'TEMPORAL_RANGE', - ); + const timeFilter: AdhocFilter | null = + timeFilterIndex !== -1 && formData.adhoc_filters + ? formData.adhoc_filters[timeFilterIndex] + : null; - const timeFilterIndex: any = formData.adhoc_filters?.findIndex( - ({ operator }: { operator: string }) => operator === 'TEMPORAL_RANGE', - ); + let testSince = null; + let testUntil = null; - const [testSince, testUntil] = getSinceUntil( - timeFilter.comparator.toLowerCase(), - ); - - let formDataB: QueryFormData; + if ( + timeFilter && + 'comparator' in timeFilter && + typeof timeFilter.comparator === 'string' + ) { + [testSince, testUntil] = getSinceUntil( + timeFilter.comparator.toLocaleLowerCase(), + ); + } - if (timeComparison!='c'){ + let formDataB: QueryFormData; - const [prevStartDateMoment, prevEndDateMoment] = calculatePrev( - testSince, - testUntil, - timeComparison, - ); + if (timeComparison !== 'c') { + const [prevStartDateMoment, prevEndDateMoment] = calculatePrev( + testSince, + testUntil, + timeComparison, + ); const queryBComparator = `${prevStartDateMoment?.format( - 'YYYY-MM-DDTHH:mm:ss', - )} : ${prevEndDateMoment?.format('YYYY-MM-DDTHH:mm:ss')}`; + 'YYYY-MM-DDTHH:mm:ss', + )} : ${prevEndDateMoment?.format('YYYY-MM-DDTHH:mm:ss')}`; - const queryBFilter = { + const queryBFilter: any = { ...timeFilter, - comparator: queryBComparator.replace(/Z/g, '') - } + comparator: queryBComparator.replace(/Z/g, ''), + }; - const otherFilters = formData.adhoc_filters?.filter((_value: any, index: number) => timeFilterIndex !== index); - const queryBFilters = otherFilters ? [queryBFilter, ...otherFilters] : [queryBFilter]; - - formDataB= { + const otherFilters = formData.adhoc_filters?.filter( + (_value: any, index: number) => timeFilterIndex !== index, + ); + const queryBFilters = otherFilters + ? [queryBFilter, ...otherFilters] + : [queryBFilter]; + + formDataB = { ...formData, adhoc_filters: queryBFilters, - } - + }; } else { - - formDataB= { + formDataB = { ...formData, adhoc_filters: formData.adhoc_custom, - } - + }; } const queryContextB = buildQueryContext(formDataB, baseQueryObject => [ @@ -275,7 +292,6 @@ export default function buildQuery(formData: QueryFormData) { }, ]); - return { ...queryContextA, queries: [...queryContextA.queries, ...queryContextB.queries], diff --git a/superset-frontend/plugins/plugin-chart-period-over-period-kpi/src/plugin/transformProps.ts b/superset-frontend/plugins/plugin-chart-period-over-period-kpi/src/plugin/transformProps.ts index 0f5723264cdbf..437641143cd3d 100644 --- a/superset-frontend/plugins/plugin-chart-period-over-period-kpi/src/plugin/transformProps.ts +++ b/superset-frontend/plugins/plugin-chart-period-over-period-kpi/src/plugin/transformProps.ts @@ -16,15 +16,26 @@ * specific language governing permissions and limitations * under the License. */ +import moment from 'moment'; import { ChartProps, - TimeseriesDataRecord, getMetricLabel, getValueFormatter, NumberFormats, getNumberFormatter, } from '@superset-ui/core'; +export const parseMetricValue = (metricValue: number | string | null) => { + if (typeof metricValue === 'string') { + const dateObject = moment.utc(metricValue, moment.ISO_8601, true); + if (dateObject.isValid()) { + return dateObject.valueOf(); + } + return 0; + } + return metricValue ?? 0; +}; + export default function transformProps(chartProps: ChartProps) { /** * This function is called after a successful response has been @@ -71,12 +82,14 @@ export default function transformProps(chartProps: ChartProps) { currencyFormat, subheaderFontSize, } = formData; - const dataA = queriesData[0].data as TimeseriesDataRecord[]; - const dataB = queriesData[1].data as TimeseriesDataRecord[]; + const { data: dataA = [] } = queriesData[0]; + const { data: dataB = [] } = queriesData[1]; const data = dataA; const metricName = getMetricLabel(metrics[0]); - let bigNumber = data.length === 0 ? null : data[0][metricName]; - let prevNumber = dataB.length === 0 ? null : dataB[0][metricName]; + let bigNumber: number | string = + data.length === 0 ? 0 : parseMetricValue(data[0][metricName]); + let prevNumber: number | string = + data.length === 0 ? 0 : parseMetricValue(dataB[0][metricName]); const numberFormatter = getValueFormatter( metrics[0], @@ -97,13 +110,13 @@ export default function transformProps(chartProps: ChartProps) { NumberFormats.PERCENT_SIGNED_1_POINT, ); - let valueDifference = bigNumber - prevNumber; + let valueDifference: number | string = bigNumber - prevNumber; const percentDifferenceNum = prevNumber ? (bigNumber - prevNumber) / Math.abs(prevNumber) : 0; - const compType= compTitles[formData.timeComparison] + const compType = compTitles[formData.timeComparison]; bigNumber = numberFormatter(bigNumber); prevNumber = numberFormatter(prevNumber); valueDifference = numberFormatter(valueDifference); diff --git a/superset-frontend/plugins/plugin-chart-period-over-period-kpi/src/types.ts b/superset-frontend/plugins/plugin-chart-period-over-period-kpi/src/types.ts index 31bd8f2a5c953..b13f2115ef819 100644 --- a/superset-frontend/plugins/plugin-chart-period-over-period-kpi/src/types.ts +++ b/superset-frontend/plugins/plugin-chart-period-over-period-kpi/src/types.ts @@ -35,6 +35,10 @@ interface PopKPICustomizeProps { headerText: string; } +export interface PopKPIComparisonValueStyleProps { + subheaderFontSize?: keyof typeof supersetTheme.typography.sizes; +} + export type PopKPIQueryFormData = QueryFormData & PopKPIStylesProps & PopKPICustomizeProps; diff --git a/superset-frontend/plugins/plugin-chart-period-over-period-kpi/tsconfig.json b/superset-frontend/plugins/plugin-chart-period-over-period-kpi/tsconfig.json index 29d6c9cd4d10f..b6bfaa2d98446 100644 --- a/superset-frontend/plugins/plugin-chart-period-over-period-kpi/tsconfig.json +++ b/superset-frontend/plugins/plugin-chart-period-over-period-kpi/tsconfig.json @@ -22,4 +22,4 @@ "path": "../../packages/superset-ui-core" } ] -} \ No newline at end of file +} diff --git a/superset-frontend/src/visualizations/presets/MainPreset.js b/superset-frontend/src/visualizations/presets/MainPreset.js index 57b6626a35f9f..040df25b43f94 100644 --- a/superset-frontend/src/visualizations/presets/MainPreset.js +++ b/superset-frontend/src/visualizations/presets/MainPreset.js @@ -76,8 +76,8 @@ import { } from 'src/filters/components'; import { PivotTableChartPlugin as PivotTableChartPluginV2 } from '@superset-ui/plugin-chart-pivot-table'; import { HandlebarsChartPlugin } from '@superset-ui/plugin-chart-handlebars'; +import { PopKPIPlugin } from '@superset-ui/plugin-chart-period-over-period-kpi'; import TimeTableChartPlugin from '../TimeTable'; -import { PopKPIPlugin } from '@superset-ui/plugin-chart-period-over-period-kpi' export default class MainPreset extends Preset { constructor() { @@ -156,7 +156,7 @@ export default class MainPreset extends Preset { new EchartsSunburstChartPlugin().configure({ key: 'sunburst_v2' }), new HandlebarsChartPlugin().configure({ key: 'handlebars' }), new EchartsBubbleChartPlugin().configure({ key: 'bubble_v2' }), - new PopKPIPlugin().configure({ key: 'pop_kpi'}), + new PopKPIPlugin().configure({ key: 'pop_kpi' }), ], }); }