From 99de7fc389c00e4303dbd97e8885684d0b40c806 Mon Sep 17 00:00:00 2001 From: Krist Wongsuphasawat Date: Wed, 4 Mar 2020 14:08:21 -0800 Subject: [PATCH] feat: add more support for undefined format to number and time formatters (#308) * feat: add more support for undefined format * test: add unit tests * fix: handle empty string --- .../src/NumberFormatterRegistry.ts | 6 +++++- .../src/NumberFormatterRegistrySingleton.ts | 4 ++-- .../test/NumberFormatterRegistry.test.ts | 16 ++++++++++++++++ .../NumberFormatterRegistrySingleton.test.ts | 7 +++++++ .../src/TimeFormatterRegistry.ts | 4 +++- .../src/TimeFormatterRegistrySingleton.ts | 4 ++-- .../test/TimeFormatterRegistry.test.ts | 16 ++++++++++++++++ .../test/TimeFormatterRegistrySingleton.test.ts | 7 +++++++ 8 files changed, 58 insertions(+), 6 deletions(-) diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-number-format/src/NumberFormatterRegistry.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-number-format/src/NumberFormatterRegistry.ts index 324b067824cb..6cac4463837b 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-number-format/src/NumberFormatterRegistry.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-number-format/src/NumberFormatterRegistry.ts @@ -23,7 +23,11 @@ export default class NumberFormatterRegistry extends RegistryWithDefaultKey< } get(formatterId?: string) { - const targetFormat = `${formatterId ?? this.defaultKey}`.trim(); + const targetFormat = `${ + formatterId === null || typeof formatterId === 'undefined' || formatterId === '' + ? this.defaultKey + : formatterId + }`.trim(); if (this.has(targetFormat)) { return super.get(targetFormat) as NumberFormatter; diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-number-format/src/NumberFormatterRegistrySingleton.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-number-format/src/NumberFormatterRegistrySingleton.ts index 8decd5de335b..098a35bfe3e4 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-number-format/src/NumberFormatterRegistrySingleton.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-number-format/src/NumberFormatterRegistrySingleton.ts @@ -5,10 +5,10 @@ const getInstance = makeSingleton(NumberFormatterRegistry); export default getInstance; -export function getNumberFormatter(format: string) { +export function getNumberFormatter(format?: string) { return getInstance().get(format); } -export function formatNumber(format: string, value: number | null | undefined) { +export function formatNumber(format: string | undefined, value: number | null | undefined) { return getInstance().format(format, value); } diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-number-format/test/NumberFormatterRegistry.test.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-number-format/test/NumberFormatterRegistry.test.ts index 023f06c77305..4787d05c4d98 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-number-format/test/NumberFormatterRegistry.test.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-number-format/test/NumberFormatterRegistry.test.ts @@ -26,6 +26,22 @@ describe('NumberFormatterRegistry', () => { const formatter = registry.get(); expect(formatter.format(100)).toEqual('100.0'); }); + it('falls back to default format if format is null', () => { + registry.setDefaultKey('.1f'); + // @ts-ignore + const formatter = registry.get(null); + expect(formatter.format(100)).toEqual('100.0'); + }); + it('falls back to default format if format is undefined', () => { + registry.setDefaultKey('.1f'); + const formatter = registry.get(undefined); + expect(formatter.format(100)).toEqual('100.0'); + }); + it('falls back to default format if format is empty string', () => { + registry.setDefaultKey('.1f'); + const formatter = registry.get(''); + expect(formatter.format(100)).toEqual('100.0'); + }); it('removes leading and trailing spaces from format', () => { const formatter = registry.get(' .2f'); expect(formatter).toBeInstanceOf(NumberFormatter); diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-number-format/test/NumberFormatterRegistrySingleton.test.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-number-format/test/NumberFormatterRegistrySingleton.test.ts index e7f5bf27e23a..0f8a4828480d 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-number-format/test/NumberFormatterRegistrySingleton.test.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-number-format/test/NumberFormatterRegistrySingleton.test.ts @@ -19,11 +19,18 @@ describe('NumberFormatterRegistrySingleton', () => { const format = getNumberFormatter('xkcd'); expect(format(12345)).toEqual('12345 (Invalid format: xkcd)'); }); + it('falls back to default format if format is not specified', () => { + const formatter = getNumberFormatter(); + expect(formatter.format(100)).toEqual('100'); + }); }); describe('formatNumber(format, value)', () => { it('format the given number using the specified format', () => { const output = formatNumber('.3s', 12345); expect(output).toEqual('12.3k'); }); + it('falls back to the default formatter if the format is undefined', () => { + expect(formatNumber(undefined, 1000)).toEqual('1k'); + }); }); }); diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-time-format/src/TimeFormatterRegistry.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-time-format/src/TimeFormatterRegistry.ts index 12a89330dff5..8e1f1c2db569 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-time-format/src/TimeFormatterRegistry.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-time-format/src/TimeFormatterRegistry.ts @@ -16,7 +16,9 @@ export default class TimeFormatterRegistry extends RegistryWithDefaultKey< } get(format?: string) { - const targetFormat = `${format ?? this.defaultKey}`.trim(); + const targetFormat = `${ + format === null || typeof format === 'undefined' || format === '' ? this.defaultKey : format + }`.trim(); if (this.has(targetFormat)) { return super.get(targetFormat) as TimeFormatter; diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-time-format/src/TimeFormatterRegistrySingleton.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-time-format/src/TimeFormatterRegistrySingleton.ts index 99f2872146c3..bd7f9c4f56fe 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-time-format/src/TimeFormatterRegistrySingleton.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-time-format/src/TimeFormatterRegistrySingleton.ts @@ -5,10 +5,10 @@ const getInstance = makeSingleton(TimeFormatterRegistry); export default getInstance; -export function getTimeFormatter(formatId: string) { +export function getTimeFormatter(formatId?: string) { return getInstance().get(formatId); } -export function formatTime(formatId: string, value: Date | null | undefined) { +export function formatTime(formatId: string | undefined, value: Date | null | undefined) { return getInstance().format(formatId, value); } diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-time-format/test/TimeFormatterRegistry.test.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-time-format/test/TimeFormatterRegistry.test.ts index 84448bfeccb5..de3593a317c4 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-time-format/test/TimeFormatterRegistry.test.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-time-format/test/TimeFormatterRegistry.test.ts @@ -22,6 +22,22 @@ describe('TimeFormatterRegistry', () => { const formatter = registry.get(); expect(formatter.format(PREVIEW_TIME)).toEqual('14/02/2017'); }); + it('falls back to default format if format is null', () => { + registry.setDefaultKey(TimeFormats.INTERNATIONAL_DATE); + // @ts-ignore + const formatter = registry.get(null); + expect(formatter.format(PREVIEW_TIME)).toEqual('14/02/2017'); + }); + it('falls back to default format if format is undefined', () => { + registry.setDefaultKey(TimeFormats.INTERNATIONAL_DATE); + const formatter = registry.get(undefined); + expect(formatter.format(PREVIEW_TIME)).toEqual('14/02/2017'); + }); + it('falls back to default format if format is empty string', () => { + registry.setDefaultKey(TimeFormats.INTERNATIONAL_DATE); + const formatter = registry.get(''); + expect(formatter.format(PREVIEW_TIME)).toEqual('14/02/2017'); + }); it('removes leading and trailing spaces from format', () => { const formatter = registry.get(' %Y '); expect(formatter).toBeInstanceOf(TimeFormatter); diff --git a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-time-format/test/TimeFormatterRegistrySingleton.test.ts b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-time-format/test/TimeFormatterRegistrySingleton.test.ts index 72d471b80149..466bf58a079f 100644 --- a/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-time-format/test/TimeFormatterRegistrySingleton.test.ts +++ b/superset-frontend/temporary_superset_ui/superset-ui/packages/superset-ui-time-format/test/TimeFormatterRegistrySingleton.test.ts @@ -16,11 +16,18 @@ describe('TimeFormatterRegistrySingleton', () => { const format = getTimeFormatter('%d/%m/%Y'); expect(format(PREVIEW_TIME)).toEqual('14/02/2017'); }); + it('falls back to default format if format is not specified', () => { + const formatter = getTimeFormatter(); + expect(formatter.format(PREVIEW_TIME)).toEqual('2017-02-14 11:22:33'); + }); }); describe('formatTime(format, value)', () => { it('format the given time using the specified format', () => { const output = formatTime('%Y-%m-%d', PREVIEW_TIME); expect(output).toEqual('2017-02-14'); }); + it('falls back to the default formatter if the format is undefined', () => { + expect(formatTime(undefined, PREVIEW_TIME)).toEqual('2017-02-14 11:22:33'); + }); }); });