From 38ea11d110263ce8fb39291772989c81ea91c384 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20H=C3=A4ggmark?= Date: Mon, 21 Jan 2019 15:58:36 +0100 Subject: [PATCH 1/3] Added check for null value in ValueMappings and added tests --- .../src/components/Gauge/Gauge.test.tsx | 39 +++++++++++++++++++ .../grafana-ui/src/components/Gauge/Gauge.tsx | 22 +++++++++-- 2 files changed, 57 insertions(+), 4 deletions(-) diff --git a/packages/grafana-ui/src/components/Gauge/Gauge.test.tsx b/packages/grafana-ui/src/components/Gauge/Gauge.test.tsx index b3396841d4d..1dbfb2bbb42 100644 --- a/packages/grafana-ui/src/components/Gauge/Gauge.test.tsx +++ b/packages/grafana-ui/src/components/Gauge/Gauge.test.tsx @@ -135,6 +135,45 @@ describe('Format value with value mappings', () => { expect(result.text).toEqual('1-20'); }); + it('should return if value is null and value to text mapping value is null', () => { + const valueMappings: ValueMapping[] = [ + { id: 0, operator: '', text: '1-20', type: MappingType.RangeToText, from: '1', to: '20' }, + { id: 1, operator: '', text: '', type: MappingType.ValueToText, value: 'null' }, + ]; + const value = null; + const { instance } = setup({ valueMappings }); + + const result = instance.getFirstFormattedValueMapping(valueMappings, value); + + expect(result.text).toEqual(''); + }); + + it('should return if value is null and range to text mapping from is null', () => { + const valueMappings: ValueMapping[] = [ + { id: 0, operator: '', text: '', type: MappingType.RangeToText, from: 'null', to: '10' }, + { id: 1, operator: '', text: 'elva', type: MappingType.ValueToText, value: '11' }, + ]; + const value = null; + const { instance } = setup({ valueMappings }); + + const result = instance.getFirstFormattedValueMapping(valueMappings, value); + + expect(result.text).toEqual(''); + }); + + it('should return if value is null and range to text mapping to is null', () => { + const valueMappings: ValueMapping[] = [ + { id: 0, operator: '', text: '', type: MappingType.RangeToText, from: '1', to: 'null' }, + { id: 1, operator: '', text: 'elva', type: MappingType.ValueToText, value: '11' }, + ]; + const value = null; + const { instance } = setup({ valueMappings }); + + const result = instance.getFirstFormattedValueMapping(valueMappings, value); + + expect(result.text).toEqual(''); + }); + it('should return rangeToText mapping where value equals to', () => { const valueMappings: ValueMapping[] = [ { id: 0, operator: '', text: '1-10', type: MappingType.RangeToText, from: '1', to: '10' }, diff --git a/packages/grafana-ui/src/components/Gauge/Gauge.tsx b/packages/grafana-ui/src/components/Gauge/Gauge.tsx index 63d875e9cd5..e0c60177a34 100644 --- a/packages/grafana-ui/src/components/Gauge/Gauge.tsx +++ b/packages/grafana-ui/src/components/Gauge/Gauge.tsx @@ -60,10 +60,14 @@ export class Gauge extends PureComponent { } addValueToTextMappingText(allValueMappings: ValueMapping[], valueToTextMapping: ValueMap, value: TimeSeriesValue) { - if (!valueToTextMapping.value) { + if (valueToTextMapping.value === undefined) { return allValueMappings; } + if (value === null && valueToTextMapping.value && valueToTextMapping.value.toLowerCase() === 'null') { + return allValueMappings.concat(valueToTextMapping); + } + const valueAsNumber = parseFloat(value as string); const valueToTextMappingAsNumber = parseFloat(valueToTextMapping.value as string); @@ -79,10 +83,19 @@ export class Gauge extends PureComponent { } addRangeToTextMappingText(allValueMappings: ValueMapping[], rangeToTextMapping: RangeMap, value: TimeSeriesValue) { - if (!rangeToTextMapping.from || !rangeToTextMapping.to || !value) { + if (rangeToTextMapping.from === undefined || rangeToTextMapping.to === undefined || value === undefined) { return allValueMappings; } + if ( + value === null && + rangeToTextMapping.from && + rangeToTextMapping.to && + (rangeToTextMapping.from.toLowerCase() === 'null' || rangeToTextMapping.to.toLowerCase() === 'null') + ) { + return allValueMappings.concat(rangeToTextMapping); + } + const valueAsNumber = parseFloat(value as string); const fromAsNumber = parseFloat(rangeToTextMapping.from as string); const toAsNumber = parseFloat(rangeToTextMapping.to as string); @@ -139,8 +152,9 @@ export class Gauge extends PureComponent { const formatFunc = getValueFormat(unit); const formattedValue = formatFunc(value as number, decimals); + const handleNoValueValue = formattedValue || 'no value'; - return `${prefix} ${formattedValue} ${suffix}`; + return `${prefix} ${handleNoValueValue} ${suffix}`; } getFontColor(value: TimeSeriesValue) { @@ -204,7 +218,7 @@ export class Gauge extends PureComponent { if (timeSeries[0]) { value = timeSeries[0].stats[stat]; } else { - value = 'N/A'; + value = null; } const dimension = Math.min(width, height * 1.3); From bbb7596113b56747c86630915e21955b1dcd8a46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20H=C3=A4ggmark?= Date: Mon, 21 Jan 2019 17:30:47 +0100 Subject: [PATCH 2/3] Changed null logic for range value mappings after PR comments --- .../src/components/Gauge/Gauge.test.tsx | 17 ++--------------- .../grafana-ui/src/components/Gauge/Gauge.tsx | 3 ++- 2 files changed, 4 insertions(+), 16 deletions(-) diff --git a/packages/grafana-ui/src/components/Gauge/Gauge.test.tsx b/packages/grafana-ui/src/components/Gauge/Gauge.test.tsx index 1dbfb2bbb42..254044847bd 100644 --- a/packages/grafana-ui/src/components/Gauge/Gauge.test.tsx +++ b/packages/grafana-ui/src/components/Gauge/Gauge.test.tsx @@ -148,22 +148,9 @@ describe('Format value with value mappings', () => { expect(result.text).toEqual(''); }); - it('should return if value is null and range to text mapping from is null', () => { + it('should return if value is null and range to text mapping from and to is null', () => { const valueMappings: ValueMapping[] = [ - { id: 0, operator: '', text: '', type: MappingType.RangeToText, from: 'null', to: '10' }, - { id: 1, operator: '', text: 'elva', type: MappingType.ValueToText, value: '11' }, - ]; - const value = null; - const { instance } = setup({ valueMappings }); - - const result = instance.getFirstFormattedValueMapping(valueMappings, value); - - expect(result.text).toEqual(''); - }); - - it('should return if value is null and range to text mapping to is null', () => { - const valueMappings: ValueMapping[] = [ - { id: 0, operator: '', text: '', type: MappingType.RangeToText, from: '1', to: 'null' }, + { id: 0, operator: '', text: '', type: MappingType.RangeToText, from: 'null', to: 'null' }, { id: 1, operator: '', text: 'elva', type: MappingType.ValueToText, value: '11' }, ]; const value = null; diff --git a/packages/grafana-ui/src/components/Gauge/Gauge.tsx b/packages/grafana-ui/src/components/Gauge/Gauge.tsx index e0c60177a34..ddc5b91f9b7 100644 --- a/packages/grafana-ui/src/components/Gauge/Gauge.tsx +++ b/packages/grafana-ui/src/components/Gauge/Gauge.tsx @@ -91,7 +91,8 @@ export class Gauge extends PureComponent { value === null && rangeToTextMapping.from && rangeToTextMapping.to && - (rangeToTextMapping.from.toLowerCase() === 'null' || rangeToTextMapping.to.toLowerCase() === 'null') + rangeToTextMapping.from.toLowerCase() === 'null' && + rangeToTextMapping.to.toLowerCase() === 'null' ) { return allValueMappings.concat(rangeToTextMapping); } From a9c33ab6580c0e9aea7d932b54bc95d3eb6dc13d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20H=C3=A4ggmark?= Date: Tue, 22 Jan 2019 06:40:04 +0100 Subject: [PATCH 3/3] Moved ValueMapping logic and tests to separate files --- .../src/components/Gauge/Gauge.test.tsx | 103 ------------------ .../grafana-ui/src/components/Gauge/Gauge.tsx | 94 +--------------- .../src/utils/valueMappings.test.ts | 81 ++++++++++++++ .../grafana-ui/src/utils/valueMappings.ts | 89 +++++++++++++++ 4 files changed, 173 insertions(+), 194 deletions(-) create mode 100644 packages/grafana-ui/src/utils/valueMappings.test.ts create mode 100644 packages/grafana-ui/src/utils/valueMappings.ts diff --git a/packages/grafana-ui/src/components/Gauge/Gauge.test.tsx b/packages/grafana-ui/src/components/Gauge/Gauge.test.tsx index 254044847bd..396b7a03162 100644 --- a/packages/grafana-ui/src/components/Gauge/Gauge.test.tsx +++ b/packages/grafana-ui/src/components/Gauge/Gauge.test.tsx @@ -98,109 +98,6 @@ describe('Get thresholds formatted', () => { }); }); -describe('Format value with value mappings', () => { - it('should return undefined with no valuemappings', () => { - const valueMappings: ValueMapping[] = []; - const value = '10'; - const { instance } = setup({ valueMappings }); - - const result = instance.getFirstFormattedValueMapping(valueMappings, value); - - expect(result).toBeUndefined(); - }); - - it('should return undefined with no matching valuemappings', () => { - const valueMappings: ValueMapping[] = [ - { id: 0, operator: '', text: 'elva', type: MappingType.ValueToText, value: '11' }, - { id: 1, operator: '', text: '1-9', type: MappingType.RangeToText, from: '1', to: '9' }, - ]; - const value = '10'; - const { instance } = setup({ valueMappings }); - - const result = instance.getFirstFormattedValueMapping(valueMappings, value); - - expect(result).toBeUndefined(); - }); - - it('should return first matching mapping with lowest id', () => { - const valueMappings: ValueMapping[] = [ - { id: 0, operator: '', text: '1-20', type: MappingType.RangeToText, from: '1', to: '20' }, - { id: 1, operator: '', text: 'tio', type: MappingType.ValueToText, value: '10' }, - ]; - const value = '10'; - const { instance } = setup({ valueMappings }); - - const result = instance.getFirstFormattedValueMapping(valueMappings, value); - - expect(result.text).toEqual('1-20'); - }); - - it('should return if value is null and value to text mapping value is null', () => { - const valueMappings: ValueMapping[] = [ - { id: 0, operator: '', text: '1-20', type: MappingType.RangeToText, from: '1', to: '20' }, - { id: 1, operator: '', text: '', type: MappingType.ValueToText, value: 'null' }, - ]; - const value = null; - const { instance } = setup({ valueMappings }); - - const result = instance.getFirstFormattedValueMapping(valueMappings, value); - - expect(result.text).toEqual(''); - }); - - it('should return if value is null and range to text mapping from and to is null', () => { - const valueMappings: ValueMapping[] = [ - { id: 0, operator: '', text: '', type: MappingType.RangeToText, from: 'null', to: 'null' }, - { id: 1, operator: '', text: 'elva', type: MappingType.ValueToText, value: '11' }, - ]; - const value = null; - const { instance } = setup({ valueMappings }); - - const result = instance.getFirstFormattedValueMapping(valueMappings, value); - - expect(result.text).toEqual(''); - }); - - it('should return rangeToText mapping where value equals to', () => { - const valueMappings: ValueMapping[] = [ - { id: 0, operator: '', text: '1-10', type: MappingType.RangeToText, from: '1', to: '10' }, - { id: 1, operator: '', text: 'elva', type: MappingType.ValueToText, value: '11' }, - ]; - const value = '10'; - const { instance } = setup({ valueMappings }); - - const result = instance.getFirstFormattedValueMapping(valueMappings, value); - - expect(result.text).toEqual('1-10'); - }); - - it('should return rangeToText mapping where value equals from', () => { - const valueMappings: ValueMapping[] = [ - { id: 0, operator: '', text: '10-20', type: MappingType.RangeToText, from: '10', to: '20' }, - { id: 1, operator: '', text: 'elva', type: MappingType.ValueToText, value: '11' }, - ]; - const value = '10'; - const { instance } = setup({ valueMappings }); - - const result = instance.getFirstFormattedValueMapping(valueMappings, value); - - expect(result.text).toEqual('10-20'); - }); - - it('should return rangeToText mapping where value is between from and to', () => { - const valueMappings: ValueMapping[] = [ - { id: 0, operator: '', text: '1-20', type: MappingType.RangeToText, from: '1', to: '20' }, - { id: 1, operator: '', text: 'elva', type: MappingType.ValueToText, value: '11' }, - ]; - const value = '10'; - const { instance } = setup({ valueMappings }); - - const result = instance.getFirstFormattedValueMapping(valueMappings, value); - - expect(result.text).toEqual('1-20'); - }); -}); - describe('Format value', () => { it('should return if value isNaN', () => { const valueMappings: ValueMapping[] = []; diff --git a/packages/grafana-ui/src/components/Gauge/Gauge.tsx b/packages/grafana-ui/src/components/Gauge/Gauge.tsx index ddc5b91f9b7..2dce20543fd 100644 --- a/packages/grafana-ui/src/components/Gauge/Gauge.tsx +++ b/packages/grafana-ui/src/components/Gauge/Gauge.tsx @@ -1,20 +1,10 @@ import React, { PureComponent } from 'react'; import $ from 'jquery'; -import { - ValueMapping, - Threshold, - ThemeName, - MappingType, - BasicGaugeColor, - ThemeNames, - ValueMap, - RangeMap, -} from '../../types/panel'; +import { ValueMapping, Threshold, ThemeName, BasicGaugeColor, ThemeNames } from '../../types/panel'; import { TimeSeriesVMs } from '../../types/series'; import { getValueFormat } from '../../utils/valueFormats/valueFormats'; - -type TimeSeriesValue = string | number | null; +import { TimeSeriesValue, getMappedValue } from '../../utils/valueMappings'; export interface Props { decimals: number; @@ -59,84 +49,6 @@ export class Gauge extends PureComponent { this.draw(); } - addValueToTextMappingText(allValueMappings: ValueMapping[], valueToTextMapping: ValueMap, value: TimeSeriesValue) { - if (valueToTextMapping.value === undefined) { - return allValueMappings; - } - - if (value === null && valueToTextMapping.value && valueToTextMapping.value.toLowerCase() === 'null') { - return allValueMappings.concat(valueToTextMapping); - } - - const valueAsNumber = parseFloat(value as string); - const valueToTextMappingAsNumber = parseFloat(valueToTextMapping.value as string); - - if (isNaN(valueAsNumber) || isNaN(valueToTextMappingAsNumber)) { - return allValueMappings; - } - - if (valueAsNumber !== valueToTextMappingAsNumber) { - return allValueMappings; - } - - return allValueMappings.concat(valueToTextMapping); - } - - addRangeToTextMappingText(allValueMappings: ValueMapping[], rangeToTextMapping: RangeMap, value: TimeSeriesValue) { - if (rangeToTextMapping.from === undefined || rangeToTextMapping.to === undefined || value === undefined) { - return allValueMappings; - } - - if ( - value === null && - rangeToTextMapping.from && - rangeToTextMapping.to && - rangeToTextMapping.from.toLowerCase() === 'null' && - rangeToTextMapping.to.toLowerCase() === 'null' - ) { - return allValueMappings.concat(rangeToTextMapping); - } - - const valueAsNumber = parseFloat(value as string); - const fromAsNumber = parseFloat(rangeToTextMapping.from as string); - const toAsNumber = parseFloat(rangeToTextMapping.to as string); - - if (isNaN(valueAsNumber) || isNaN(fromAsNumber) || isNaN(toAsNumber)) { - return allValueMappings; - } - - if (valueAsNumber >= fromAsNumber && valueAsNumber <= toAsNumber) { - return allValueMappings.concat(rangeToTextMapping); - } - - return allValueMappings; - } - - getAllFormattedValueMappings(valueMappings: ValueMapping[], value: TimeSeriesValue) { - const allFormattedValueMappings = valueMappings.reduce( - (allValueMappings, valueMapping) => { - if (valueMapping.type === MappingType.ValueToText) { - allValueMappings = this.addValueToTextMappingText(allValueMappings, valueMapping as ValueMap, value); - } else if (valueMapping.type === MappingType.RangeToText) { - allValueMappings = this.addRangeToTextMappingText(allValueMappings, valueMapping as RangeMap, value); - } - - return allValueMappings; - }, - [] as ValueMapping[] - ); - - allFormattedValueMappings.sort((t1, t2) => { - return t1.id - t2.id; - }); - - return allFormattedValueMappings; - } - - getFirstFormattedValueMapping(valueMappings: ValueMapping[], value: TimeSeriesValue) { - return this.getAllFormattedValueMappings(valueMappings, value)[0]; - } - formatValue(value: TimeSeriesValue) { const { decimals, valueMappings, prefix, suffix, unit } = this.props; @@ -145,7 +57,7 @@ export class Gauge extends PureComponent { } if (valueMappings.length > 0) { - const valueMappedValue = this.getFirstFormattedValueMapping(valueMappings, value); + const valueMappedValue = getMappedValue(valueMappings, value); if (valueMappedValue) { return `${prefix} ${valueMappedValue.text} ${suffix}`; } diff --git a/packages/grafana-ui/src/utils/valueMappings.test.ts b/packages/grafana-ui/src/utils/valueMappings.test.ts new file mode 100644 index 00000000000..d37e0beedab --- /dev/null +++ b/packages/grafana-ui/src/utils/valueMappings.test.ts @@ -0,0 +1,81 @@ +import { getMappedValue } from './valueMappings'; +import { ValueMapping, MappingType } from '../types/panel'; + +describe('Format value with value mappings', () => { + it('should return undefined with no valuemappings', () => { + const valueMappings: ValueMapping[] = []; + const value = '10'; + + expect(getMappedValue(valueMappings, value)).toBeUndefined(); + }); + + it('should return undefined with no matching valuemappings', () => { + const valueMappings: ValueMapping[] = [ + { id: 0, operator: '', text: 'elva', type: MappingType.ValueToText, value: '11' }, + { id: 1, operator: '', text: '1-9', type: MappingType.RangeToText, from: '1', to: '9' }, + ]; + const value = '10'; + + expect(getMappedValue(valueMappings, value)).toBeUndefined(); + }); + + it('should return first matching mapping with lowest id', () => { + const valueMappings: ValueMapping[] = [ + { id: 0, operator: '', text: '1-20', type: MappingType.RangeToText, from: '1', to: '20' }, + { id: 1, operator: '', text: 'tio', type: MappingType.ValueToText, value: '10' }, + ]; + const value = '10'; + + expect(getMappedValue(valueMappings, value).text).toEqual('1-20'); + }); + + it('should return if value is null and value to text mapping value is null', () => { + const valueMappings: ValueMapping[] = [ + { id: 0, operator: '', text: '1-20', type: MappingType.RangeToText, from: '1', to: '20' }, + { id: 1, operator: '', text: '', type: MappingType.ValueToText, value: 'null' }, + ]; + const value = null; + + expect(getMappedValue(valueMappings, value).text).toEqual(''); + }); + + it('should return if value is null and range to text mapping from and to is null', () => { + const valueMappings: ValueMapping[] = [ + { id: 0, operator: '', text: '', type: MappingType.RangeToText, from: 'null', to: 'null' }, + { id: 1, operator: '', text: 'elva', type: MappingType.ValueToText, value: '11' }, + ]; + const value = null; + + expect(getMappedValue(valueMappings, value).text).toEqual(''); + }); + + it('should return rangeToText mapping where value equals to', () => { + const valueMappings: ValueMapping[] = [ + { id: 0, operator: '', text: '1-10', type: MappingType.RangeToText, from: '1', to: '10' }, + { id: 1, operator: '', text: 'elva', type: MappingType.ValueToText, value: '11' }, + ]; + const value = '10'; + + expect(getMappedValue(valueMappings, value).text).toEqual('1-10'); + }); + + it('should return rangeToText mapping where value equals from', () => { + const valueMappings: ValueMapping[] = [ + { id: 0, operator: '', text: '10-20', type: MappingType.RangeToText, from: '10', to: '20' }, + { id: 1, operator: '', text: 'elva', type: MappingType.ValueToText, value: '11' }, + ]; + const value = '10'; + + expect(getMappedValue(valueMappings, value).text).toEqual('10-20'); + }); + + it('should return rangeToText mapping where value is between from and to', () => { + const valueMappings: ValueMapping[] = [ + { id: 0, operator: '', text: '1-20', type: MappingType.RangeToText, from: '1', to: '20' }, + { id: 1, operator: '', text: 'elva', type: MappingType.ValueToText, value: '11' }, + ]; + const value = '10'; + + expect(getMappedValue(valueMappings, value).text).toEqual('1-20'); + }); +}); diff --git a/packages/grafana-ui/src/utils/valueMappings.ts b/packages/grafana-ui/src/utils/valueMappings.ts new file mode 100644 index 00000000000..c9b926ea0a4 --- /dev/null +++ b/packages/grafana-ui/src/utils/valueMappings.ts @@ -0,0 +1,89 @@ +import { ValueMapping, MappingType, ValueMap, RangeMap } from '../types'; + +export type TimeSeriesValue = string | number | null; + +const addValueToTextMappingText = ( + allValueMappings: ValueMapping[], + valueToTextMapping: ValueMap, + value: TimeSeriesValue +) => { + if (valueToTextMapping.value === undefined) { + return allValueMappings; + } + + if (value === null && valueToTextMapping.value && valueToTextMapping.value.toLowerCase() === 'null') { + return allValueMappings.concat(valueToTextMapping); + } + + const valueAsNumber = parseFloat(value as string); + const valueToTextMappingAsNumber = parseFloat(valueToTextMapping.value as string); + + if (isNaN(valueAsNumber) || isNaN(valueToTextMappingAsNumber)) { + return allValueMappings; + } + + if (valueAsNumber !== valueToTextMappingAsNumber) { + return allValueMappings; + } + + return allValueMappings.concat(valueToTextMapping); +}; + +const addRangeToTextMappingText = ( + allValueMappings: ValueMapping[], + rangeToTextMapping: RangeMap, + value: TimeSeriesValue +) => { + if (rangeToTextMapping.from === undefined || rangeToTextMapping.to === undefined || value === undefined) { + return allValueMappings; + } + + if ( + value === null && + rangeToTextMapping.from && + rangeToTextMapping.to && + rangeToTextMapping.from.toLowerCase() === 'null' && + rangeToTextMapping.to.toLowerCase() === 'null' + ) { + return allValueMappings.concat(rangeToTextMapping); + } + + const valueAsNumber = parseFloat(value as string); + const fromAsNumber = parseFloat(rangeToTextMapping.from as string); + const toAsNumber = parseFloat(rangeToTextMapping.to as string); + + if (isNaN(valueAsNumber) || isNaN(fromAsNumber) || isNaN(toAsNumber)) { + return allValueMappings; + } + + if (valueAsNumber >= fromAsNumber && valueAsNumber <= toAsNumber) { + return allValueMappings.concat(rangeToTextMapping); + } + + return allValueMappings; +}; + +const getAllFormattedValueMappings = (valueMappings: ValueMapping[], value: TimeSeriesValue) => { + const allFormattedValueMappings = valueMappings.reduce( + (allValueMappings, valueMapping) => { + if (valueMapping.type === MappingType.ValueToText) { + allValueMappings = addValueToTextMappingText(allValueMappings, valueMapping as ValueMap, value); + } else if (valueMapping.type === MappingType.RangeToText) { + allValueMappings = addRangeToTextMappingText(allValueMappings, valueMapping as RangeMap, value); + } + + return allValueMappings; + }, + [] as ValueMapping[] + ); + + allFormattedValueMappings.sort((t1, t2) => { + return t1.id - t2.id; + }); + + return allFormattedValueMappings; +}; + +export const getMappedValue = (valueMappings: ValueMapping[], value: TimeSeriesValue): ValueMapping => { + return getAllFormattedValueMappings(valueMappings, value)[0]; +};