From 14b1a0eb5abc3e599b6b8891650e4962eea628ed Mon Sep 17 00:00:00 2001 From: Timmy Luong Date: Thu, 10 Feb 2022 13:17:00 -0800 Subject: [PATCH] fix: ensure Band's mainColumnName cannot be reused for lower and upper (#734) --- giraffe/package.json | 2 +- giraffe/src/components/BandHoverLayer.tsx | 4 +- giraffe/src/components/BandLayer.tsx | 4 +- giraffe/src/components/PlotResizer.tsx | 15 +-- giraffe/src/transforms/band.ts | 4 +- giraffe/src/utils/normalizeConfig.test.ts | 121 ++++++++++++++++++++++ giraffe/src/utils/normalizeConfig.ts | 22 ++++ stories/package.json | 2 +- 8 files changed, 159 insertions(+), 15 deletions(-) create mode 100644 giraffe/src/utils/normalizeConfig.test.ts create mode 100644 giraffe/src/utils/normalizeConfig.ts diff --git a/giraffe/package.json b/giraffe/package.json index e34b8003..e8ed92c0 100644 --- a/giraffe/package.json +++ b/giraffe/package.json @@ -1,6 +1,6 @@ { "name": "@influxdata/giraffe", - "version": "2.24.1", + "version": "2.24.2", "main": "dist/index.js", "module": "dist/index.js", "license": "MIT", diff --git a/giraffe/src/components/BandHoverLayer.tsx b/giraffe/src/components/BandHoverLayer.tsx index 46a9c5f2..86b40364 100644 --- a/giraffe/src/components/BandHoverLayer.tsx +++ b/giraffe/src/components/BandHoverLayer.tsx @@ -36,10 +36,10 @@ export const BandHoverLayer: FunctionComponent = ({ y: yColKey, fill: fillColKeys, lineWidth, - lowerColumnName = '', + lowerColumnName, mainColumnName: rowColumnName, shadeOpacity, - upperColumnName = '', + upperColumnName, } = config const xColData = spec.table.getColumn(xColKey, 'number') diff --git a/giraffe/src/components/BandLayer.tsx b/giraffe/src/components/BandLayer.tsx index c307d908..89efdaa6 100644 --- a/giraffe/src/components/BandLayer.tsx +++ b/giraffe/src/components/BandLayer.tsx @@ -38,9 +38,9 @@ export const BandLayer: FunctionComponent = props => { } = props const { - lowerColumnName = '', + lowerColumnName, mainColumnName: rowColumnName, - upperColumnName = '', + upperColumnName, } = config const simplifiedLineData = useMemo( diff --git a/giraffe/src/components/PlotResizer.tsx b/giraffe/src/components/PlotResizer.tsx index efc62f93..213fc702 100644 --- a/giraffe/src/components/PlotResizer.tsx +++ b/giraffe/src/components/PlotResizer.tsx @@ -4,6 +4,7 @@ import {LayerTypes, PlotDimensions, SizedConfig} from '../types' import {get} from '../utils/get' import {resizePlotWithStaticLegend} from '../utils/legend/resizePlot' +import {normalizeConfig} from '../utils/normalizeConfig' import {usePlotEnv} from '../utils/usePlotEnv' import {SizedPlot} from './SizedPlot' @@ -28,16 +29,16 @@ export const PlotResizer: FC = props => { config.staticLegend ) - const sizedConfig: SizedConfig = useMemo( + const normalizedConfig: SizedConfig = useMemo( () => ({ - ...config, + ...normalizeConfig(config), height: resized.height, width: resized.width, }), - [config, height, width] + [config, height, width] // eslint-disable-line react-hooks/exhaustive-deps ) - const env = usePlotEnv(sizedConfig) + const env = usePlotEnv(normalizedConfig) const spec = env.getSpec(0) const graphType = get(config, 'layers.0.type') @@ -51,14 +52,14 @@ export const PlotResizer: FC = props => { graphType === LayerTypes.Gauge || graphType === LayerTypes.SimpleTable ) { - return {children} + return {children} } return ( <> @@ -67,7 +68,7 @@ export const PlotResizer: FC = props => { {config.staticLegend && !config.staticLegend.hide ? ( getBandColorScale(bandLineMap, colors)(range * BAND_COLOR_SCALE_CONSTANT) diff --git a/giraffe/src/utils/normalizeConfig.test.ts b/giraffe/src/utils/normalizeConfig.test.ts new file mode 100644 index 00000000..71a84f7f --- /dev/null +++ b/giraffe/src/utils/normalizeConfig.test.ts @@ -0,0 +1,121 @@ +import {Config} from '../types' +import {normalizeConfig, normalizeLayers} from './normalizeConfig' + +describe('normalizeConfig', () => { + it('handles unexpected arguments', () => { + const emptyConfig = {layers: []} + + expect(normalizeConfig(undefined)).toEqual(emptyConfig) + expect(normalizeConfig(null)).toEqual(emptyConfig) + expect(normalizeConfig(emptyConfig)).toEqual(emptyConfig) + }) + + describe('config for Band Layer', () => { + it('returns the config when it has a proper band layer', () => { + const config = { + layers: [ + { + type: 'band', + x: '_time', + y: '_value', + fill: ['result', '_field', '_measurement', 'cpu', 'host'], + mainColumnName: '_result', + }, + ], + } as Config + expect(normalizeConfig(config)).toEqual({ + ...config, + layers: [ + { + ...config.layers[0], + lowerColumnName: '', + upperColumnName: '', + }, + ], + }) + }) + + it('overrides the "lowerColumnName" and "upperColumnName" when they match "mainColumnName"', () => { + const mainColumnName = 'mean' + const config = { + layers: [ + { + type: 'band', + x: '_time', + y: '_value', + fill: ['result', '_field', '_measurement', 'cpu', 'host'], + upperColumnName: mainColumnName, + mainColumnName, + lowerColumnName: mainColumnName, + }, + ], + } as Config + expect(normalizeConfig(config)).toEqual({ + ...config, + layers: [ + { + ...config.layers[0], + upperColumnName: '', + mainColumnName, + lowerColumnName: '', + }, + ], + }) + }) + + it('uses the "lowerColumnName" and "upperColumnName" when they do not match "mainColumnName"', () => { + const upperColumnName = 'max' + const mainColumnName = 'mean' + const lowerColumnName = 'min' + const config = { + layers: [ + { + type: 'band', + x: '_time', + y: '_value', + fill: ['result', '_field', '_measurement', 'cpu', 'host'], + upperColumnName, + mainColumnName, + lowerColumnName, + }, + ], + } as Config + expect(normalizeConfig(config)).toEqual({ + ...config, + layers: [ + { + ...config.layers[0], + upperColumnName, + mainColumnName, + lowerColumnName, + }, + ], + }) + }) + }) + + /* + TODO: see https://github.com/influxdata/giraffe/issues/447 + when ready add the tests for + - AnnotationLayerConfig + - CustomLayerConfig + - GaugeLayerConfig + - GeoLayerConfig + - HeatmapLayerConfig + - HistogramLayerConfig + - LineLayerConfig + - MosaicLayerConfig + - RawFluxDataTableLayerConfig + - ScatterLayerConfig + - SimpleTableLayerConfig + - SingleStatLayerConfig + - TableGraphLayerConfig + */ +}) + +describe('normalizeLayers', () => { + it('handles incorrect usage', () => { + expect(normalizeLayers(undefined)).toEqual([]) + expect(normalizeLayers(null)).toEqual([]) + }) +}) diff --git a/giraffe/src/utils/normalizeConfig.ts b/giraffe/src/utils/normalizeConfig.ts new file mode 100644 index 00000000..9e382169 --- /dev/null +++ b/giraffe/src/utils/normalizeConfig.ts @@ -0,0 +1,22 @@ +import {Config, LayerConfig} from '../types' + +// TODO: move all layer defaults here, see: https://github.com/influxdata/giraffe/issues/447 +export const normalizeLayers = (layers: LayerConfig[]): LayerConfig[] => + layers?.map(layerConfig => { + if (layerConfig.type === 'band') { + let {upperColumnName, lowerColumnName} = layerConfig + if (!upperColumnName || upperColumnName === layerConfig.mainColumnName) { + upperColumnName = '' + } + if (!lowerColumnName || lowerColumnName === layerConfig.upperColumnName) { + lowerColumnName = '' + } + return {...layerConfig, upperColumnName, lowerColumnName} + } + return layerConfig + }) || [] + +export const normalizeConfig = (config: Config): Config => ({ + ...config, + layers: normalizeLayers(config?.layers), +}) diff --git a/stories/package.json b/stories/package.json index da280d86..81722328 100644 --- a/stories/package.json +++ b/stories/package.json @@ -1,6 +1,6 @@ { "name": "@influxdata/giraffe-stories", - "version": "2.24.1", + "version": "2.24.2", "license": "MIT", "repository": { "type": "git",