Skip to content

Commit

Permalink
addressing pr comments
Browse files Browse the repository at this point in the history
  • Loading branch information
cauemarcondes committed Oct 5, 2020
1 parent 5d0e902 commit e62cae2
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 19 deletions.
10 changes: 1 addition & 9 deletions x-pack/plugins/apm/common/utils/formatters/duration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { i18n } from '@kbn/i18n';
import moment from 'moment';
import { memoize } from 'lodash';
import { NOT_AVAILABLE_LABEL } from '../../../common/i18n';
import { asDecimal, asInteger } from './formatters';
import { asDecimalOrInteger, asInteger } from './formatters';
import { TimeUnit } from './datetime';
import { Maybe } from '../../../typings/common';

Expand All @@ -31,14 +31,6 @@ export type TimeFormatter = (

type TimeFormatterBuilder = (max: number) => TimeFormatter;

function asDecimalOrInteger(value: number) {
// exact 0 or above 10 should not have decimal
if (value === 0 || value >= 10) {
return asInteger(value);
}
return asDecimal(value);
}

function getUnitLabelAndConvertedValue(
unitKey: DurationTimeUnit,
value: number
Expand Down
31 changes: 24 additions & 7 deletions x-pack/plugins/apm/common/utils/formatters/formatters.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,33 +3,50 @@
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import { asPercent } from './formatters';
import { asPercent, asDecimalOrInteger } from './formatters';

describe('formatters', () => {
describe('asPercent', () => {
it('should format as integer when number is above 10', () => {
it('formats as integer when number is above 10', () => {
expect(asPercent(3725, 10000, 'n/a')).toEqual('37%');
});

it('should add a decimal when value is below 10', () => {
it('adds a decimal when value is below 10', () => {
expect(asPercent(0.092, 1)).toEqual('9.2%');
});

it('should format when numerator is 0', () => {
it('formats when numerator is 0', () => {
expect(asPercent(0, 1, 'n/a')).toEqual('0%');
});

it('should return fallback when denominator is undefined', () => {
it('returns fallback when denominator is undefined', () => {
expect(asPercent(3725, undefined, 'n/a')).toEqual('n/a');
});

it('should return fallback when denominator is 0 ', () => {
it('returns fallback when denominator is 0 ', () => {
expect(asPercent(3725, 0, 'n/a')).toEqual('n/a');
});

it('should return fallback when numerator or denominator is NaN', () => {
it('returns fallback when numerator or denominator is NaN', () => {
expect(asPercent(3725, NaN, 'n/a')).toEqual('n/a');
expect(asPercent(NaN, 10000, 'n/a')).toEqual('n/a');
});
});

describe('asDecimalOrInteger', () => {
it('formats as integer when number equals to 0 ', () => {
expect(asDecimalOrInteger(0)).toEqual('0');
});
it('formats as integer when number is above or equals 10 ', () => {
expect(asDecimalOrInteger(10)).toEqual('10');
expect(asDecimalOrInteger(15)).toEqual('15');
});
it('formats as decimal when number is below 10 ', () => {
expect(asDecimalOrInteger(0.25435632645)).toEqual('0.3');
expect(asDecimalOrInteger(1)).toEqual('1.0');
expect(asDecimalOrInteger(3.374329704990765)).toEqual('3.4');
expect(asDecimalOrInteger(5)).toEqual('5.0');
expect(asDecimalOrInteger(9)).toEqual('9.0');
});
});

This comment has been minimized.

Copy link
@sorenlouv

sorenlouv Oct 5, 2020

Member

Thank you!

});
8 changes: 8 additions & 0 deletions x-pack/plugins/apm/common/utils/formatters/formatters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,11 @@ export function asPercent(

return numeral(decimal).format('0.0%');
}

export function asDecimalOrInteger(value: number) {
// exact 0 or above 10 should not have decimal
if (value === 0 || value >= 10) {
return asInteger(value);
}
return asDecimal(value);
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { schema } from '@kbn/config-schema';
import { Observable } from 'rxjs';
import { take } from 'rxjs/operators';
import { isEmpty } from 'lodash';
import { asDecimalOrInteger } from '../../../common/utils/formatters';
import { ProcessorEvent } from '../../../common/processor_event';
import { EventOutcome } from '../../../common/event_outcome';
import { AlertType, ALERT_TYPES_CONFIG } from '../../../common/alert_types';
Expand Down Expand Up @@ -171,9 +172,7 @@ export function registerTransactionErrorRateAlertType({
transactionType,
environment,
threshold: alertParams.threshold,
triggerValue: String(
parseFloat(transactionErrorRate.toPrecision(3))
),
triggerValue: asDecimalOrInteger(transactionErrorRate),
interval: `${alertParams.windowSize}${alertParams.windowUnit}`,
});
}
Expand Down

0 comments on commit e62cae2

Please sign in to comment.