feat: add more support for undefined format to number and time format…

…ters (#308) * feat: add more support for undefined format * test: add unit tests * fix: handle empty string
apache · Nov 26, 2021 · 5966ff1 · 5966ff1
1 parent 64dca92
commit 5966ff1
Show file tree

Hide file tree

Showing 8 changed files with 58 additions and 6 deletions.
diff --git a/...superset_ui/superset-ui/packages/superset-ui-number-format/src/NumberFormatterRegistry.ts b/...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/...ui/superset-ui/packages/superset-ui-number-format/src/NumberFormatterRegistrySingleton.ts b/...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/...et_ui/superset-ui/packages/superset-ui-number-format/test/NumberFormatterRegistry.test.ts b/...et_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/...erset-ui/packages/superset-ui-number-format/test/NumberFormatterRegistrySingleton.test.ts b/...erset-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/...ary_superset_ui/superset-ui/packages/superset-ui-time-format/src/TimeFormatterRegistry.ts b/...ary_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/...set_ui/superset-ui/packages/superset-ui-time-format/src/TimeFormatterRegistrySingleton.ts b/...set_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/...perset_ui/superset-ui/packages/superset-ui-time-format/test/TimeFormatterRegistry.test.ts b/...perset_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-ui/packages/superset-ui-time-format/test/TimeFormatterRegistrySingleton.test.ts b/.../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');
+    });
   });
 });