fix(export): Excel export auto-detect number with Formatters.multiple (…

…#902) - this PR will now auto-detect numbers even when using Formatters.multiple, it wasn't at all prior to this PR - also improve parsing of numbers with formatter options, user might provide formatter options like `decimalSeparator: ','` and/or `thousandSeparator: ' '` and the Excel export should be able to parse this number even with these formatter options (it wasn't able to parse at all prior to this PR and was previously returning the string as is)
ghiscoding · Feb 14, 2023 · be33a68 · be33a68
1 parent eaf4155
commit be33a68
Show file tree

Hide file tree

Showing 13 changed files with 227 additions and 70 deletions.
diff --git a/examples/webpack-demo-vanilla-bundle/src/examples/example12.ts b/examples/webpack-demo-vanilla-bundle/src/examples/example12.ts
@@ -220,8 +220,8 @@ export class Example12 {
         id: 'complexity', name: 'Complexity', field: 'complexity', minWidth: 100,
         type: FieldType.number,
         sortable: true, filterable: true, columnGroup: 'Analysis',
-        formatter: (_row, _cell, value) => this.complexityLevelList[value].label,
-        exportCustomFormatter: (_row, _cell, value) => this.complexityLevelList[value].label,
+        formatter: (_row, _cell, value) => this.complexityLevelList[value]?.label,
+        exportCustomFormatter: (_row, _cell, value) => this.complexityLevelList[value]?.label,
         filter: {
           model: Filters.multipleSelect,
           collection: this.complexityLevelList

diff --git a/examples/webpack-demo-vanilla-bundle/src/examples/example16.ts b/examples/webpack-demo-vanilla-bundle/src/examples/example16.ts
@@ -128,16 +128,22 @@ export class Example16 {
         },
       },
       {
-        id: 'cost', name: '<span title="custom cost title tooltip text">Cost</span>', field: 'cost',
+        id: 'cost', name: '<span title="custom cost title tooltip text">Cost (in €)</span>', field: 'cost',
         width: 90,
         sortable: true,
         filterable: true,
-        exportWithFormatter: false,
+        exportWithFormatter: true,
         // filter: { model: Filters.compoundInput },
-        // formatter: Formatters.dollar,
+        // formatter: Formatters.currency,
         formatter: Formatters.multiple,
-        // params: { formatters: [Formatters.dollar, (row, cell, value) => `<span title="regular tooltip, cost: ${value}">${value || ''}</span>`] },
-        params: { formatters: [Formatters.dollar, (row, cell, value) => `<span title="regular tooltip (from title attribute) -\rcell value:\n\n${value || ''}">${value || ''}</span>`] },
+        // params: { formatters: [Formatters.currency, (row, cell, value) => `<span title="regular tooltip, cost: ${value}">${value || ''}</span>`] },
+        params: {
+          formatters: [
+            Formatters.currency,
+            (row, cell, value) => `<span title="regular tooltip (from title attribute) -\rcell value:\n\n${value || ''}">${value || ''}</span>`
+          ],
+          currencySuffix: ' €'
+        },
         customTooltip: {
           useRegularTooltip: true,
           useRegularTooltipFromFormatterOnly: true,
@@ -328,6 +334,10 @@ export class Example16 {
       textExportOptions: {
         exportWithFormatter: true
       },
+      formatterOptions: {
+        // decimalSeparator: ',',
+        thousandSeparator: ' '
+      },
       // Custom Tooltip options can be defined in a Column or Grid Options or a mixed of both (first options found wins)
       registerExternalResources: [new SlickCustomTooltip(), new ExcelExportService(), new TextExportService()],
       customTooltip: {
@@ -393,7 +403,7 @@ export class Example16 {
         percentComplete: Math.floor(Math.random() * (100 - 5 + 1) + 5),
         start: new Date(randomYear, randomMonth, randomDay),
         finish: randomFinish < new Date() ? '' : randomFinish, // make sure the random date is earlier than today
-        cost: (i % 33 === 0) ? null : Math.round(Math.random() * 10000) / 100,
+        cost: (i % 33 === 0) ? null : Math.round(Math.random() * 1000000) / 100,
         effortDriven: (i % 5 === 0),
         prerequisites: (i % 2 === 0) && i !== 0 && i < 50 ? [i, i - 1] : [],
       };

diff --git a/package.json b/package.json
@@ -26,7 +26,7 @@
     "cypress:ci": "cypress run --config-file test/cypress.config.ts",
     "predev": "pnpm run -r build:incremental && pnpm run -r sass:copy",
     "dev": "run-p dev:watch webpack:watch",
-    "dev:watch": "lerna watch --file-delimiter=\",\" --glob=\"src/**/*.{ts,scss}\" --ignored=\"src/**/*.spec.ts\" -- cross-env-shell pnpm run -r --filter $LERNA_PACKAGE_NAME dev",
+    "dev:watch": "lerna watch --no-bail --file-delimiter=\",\" --glob=\"src/**/*.{ts,scss}\" --ignored=\"src/**/*.spec.ts\" -- cross-env-shell pnpm run -r --filter $LERNA_PACKAGE_NAME dev",
     "webpack:watch": "pnpm -r --parallel run webpack:dev",
     "preview:publish": "lerna publish from-package --dry-run",
     "preview:version": "lerna version --dry-run",

diff --git a/packages/common/src/formatters/__tests__/formatterUtilities.spec.ts b/packages/common/src/formatters/__tests__/formatterUtilities.spec.ts
@@ -59,27 +59,23 @@ describe('formatterUtilities', () => {
   describe('getValueFromParamsOrGridOptions method', () => {
     it('should return options found in the Grid Option when not found in Column Definition "params" property', () => {
       const gridOptions = { formatterOptions: { minDecimal: 2 } } as GridOption;
-      const gridSpy = (gridStub.getOptions as jest.Mock).mockReturnValue(gridOptions);
 
-      const output = getValueFromParamsOrFormatterOptions('minDecimal', {} as Column, gridStub, -1);
+      const output = getValueFromParamsOrFormatterOptions('minDecimal', {} as Column, gridOptions, -1);
 
-      expect(gridSpy).toHaveBeenCalled();
       expect(output).toBe(2);
     });
 
     it('should return options found in the Column Definition "params" even if exist in the Grid Option as well', () => {
       const gridOptions = { formatterOptions: { minDecimal: 2 } } as GridOption;
-      const gridSpy = (gridStub.getOptions as jest.Mock).mockReturnValue(gridOptions);
 
-      const output = getValueFromParamsOrFormatterOptions('minDecimal', { params: { minDecimal: 3 } } as Column, gridStub, -1);
+      const output = getValueFromParamsOrFormatterOptions('minDecimal', { params: { minDecimal: 3 } } as Column, gridOptions, -1);
 
-      expect(gridSpy).toHaveBeenCalled();
       expect(output).toBe(3);
     });
 
     it('should return default value when not found in "params" (columnDef) neither the "formatterOptions" (gridOption)', () => {
       const defaultValue = 5;
-      const output = getValueFromParamsOrFormatterOptions('minDecimal', { field: 'column1' } as Column, {} as unknown as SlickGrid, defaultValue);
+      const output = getValueFromParamsOrFormatterOptions('minDecimal', { field: 'column1' } as Column, {} as unknown as GridOption, defaultValue);
       expect(output).toBe(defaultValue);
     });
   });

diff --git a/packages/common/src/formatters/formatterUtilities.ts b/packages/common/src/formatters/formatterUtilities.ts
@@ -59,17 +59,18 @@ export function retrieveFormatterOptions(columnDef: Column, grid: SlickGrid, num
     default:
       break;
   }
-  const minDecimal = getValueFromParamsOrFormatterOptions('minDecimal', columnDef, grid, defaultMinDecimal);
-  const maxDecimal = getValueFromParamsOrFormatterOptions('maxDecimal', columnDef, grid, defaultMaxDecimal);
-  const decimalSeparator = getValueFromParamsOrFormatterOptions('decimalSeparator', columnDef, grid, Constants.DEFAULT_NUMBER_DECIMAL_SEPARATOR);
-  const thousandSeparator = getValueFromParamsOrFormatterOptions('thousandSeparator', columnDef, grid, Constants.DEFAULT_NUMBER_THOUSAND_SEPARATOR);
-  const wrapNegativeNumber = getValueFromParamsOrFormatterOptions('displayNegativeNumberWithParentheses', columnDef, grid, Constants.DEFAULT_NEGATIVE_NUMBER_WRAPPED_IN_BRAQUET);
-  const currencyPrefix = getValueFromParamsOrFormatterOptions('currencyPrefix', columnDef, grid, '');
-  const currencySuffix = getValueFromParamsOrFormatterOptions('currencySuffix', columnDef, grid, '');
+  const gridOptions = ((grid && typeof grid.getOptions === 'function') ? grid.getOptions() : {}) as GridOption;
+  const minDecimal = getValueFromParamsOrFormatterOptions('minDecimal', columnDef, gridOptions, defaultMinDecimal);
+  const maxDecimal = getValueFromParamsOrFormatterOptions('maxDecimal', columnDef, gridOptions, defaultMaxDecimal);
+  const decimalSeparator = getValueFromParamsOrFormatterOptions('decimalSeparator', columnDef, gridOptions, Constants.DEFAULT_NUMBER_DECIMAL_SEPARATOR);
+  const thousandSeparator = getValueFromParamsOrFormatterOptions('thousandSeparator', columnDef, gridOptions, Constants.DEFAULT_NUMBER_THOUSAND_SEPARATOR);
+  const wrapNegativeNumber = getValueFromParamsOrFormatterOptions('displayNegativeNumberWithParentheses', columnDef, gridOptions, Constants.DEFAULT_NEGATIVE_NUMBER_WRAPPED_IN_BRAQUET);
+  const currencyPrefix = getValueFromParamsOrFormatterOptions('currencyPrefix', columnDef, gridOptions, '');
+  const currencySuffix = getValueFromParamsOrFormatterOptions('currencySuffix', columnDef, gridOptions, '');
 
   if (formatterType === 'cell') {
-    numberPrefix = getValueFromParamsOrFormatterOptions('numberPrefix', columnDef, grid, '');
-    numberSuffix = getValueFromParamsOrFormatterOptions('numberSuffix', columnDef, grid, '');
+    numberPrefix = getValueFromParamsOrFormatterOptions('numberPrefix', columnDef, gridOptions, '');
+    numberSuffix = getValueFromParamsOrFormatterOptions('numberSuffix', columnDef, gridOptions, '');
   }
 
   return { minDecimal, maxDecimal, decimalSeparator, thousandSeparator, wrapNegativeNumber, currencyPrefix, currencySuffix, numberPrefix, numberSuffix };
@@ -81,8 +82,7 @@ export function retrieveFormatterOptions(columnDef: Column, grid: SlickGrid, num
  * 2- Grid Options "formatterOptions"
  * 3- nothing found, return default value provided
  */
-export function getValueFromParamsOrFormatterOptions(optionName: string, columnDef: Column, grid: SlickGrid, defaultValue?: any) {
-  const gridOptions = ((grid && typeof grid.getOptions === 'function') ? grid.getOptions() : {}) as GridOption;
+export function getValueFromParamsOrFormatterOptions(optionName: string, columnDef: Column, gridOptions: GridOption, defaultValue?: any) {
   const params = columnDef && columnDef.params;
 
   if (params && params.hasOwnProperty(optionName)) {

diff --git a/packages/common/src/global-grid-options.ts b/packages/common/src/global-grid-options.ts
@@ -157,7 +157,7 @@ export const GlobalGridOptions: GridOption = {
     groupCollapsedSymbol: '⮞',
     groupExpandedSymbol: '⮟',
     groupingAggregatorRowText: '',
-    sanitizeDataExport: false,
+    sanitizeDataExport: true,
   },
   textExportOptions: {
     delimiter: DelimiterType.comma,
@@ -166,7 +166,7 @@ export const GlobalGridOptions: GridOption = {
     format: FileType.csv,
     groupingColumnHeaderTitle: 'Group By',
     groupingAggregatorRowText: '',
-    sanitizeDataExport: false,
+    sanitizeDataExport: true,
     useUtf8WithBom: true
   },
   gridAutosizeColsMode: GridAutosizeColsMode.none,

diff --git a/packages/common/src/interfaces/columnExcelExportOption.interface.ts b/packages/common/src/interfaces/columnExcelExportOption.interface.ts
@@ -1,5 +1,6 @@
 import { Column } from './column.interface';
 import { ExcelCellFormat } from './excelCellFormat.interface';
+import { GridOption } from './gridOption.interface';
 
 /** Excel custom export options (formatting & width) that can be applied to a column */
 export interface ColumnExcelExportOption {
@@ -27,7 +28,7 @@ export interface GroupTotalExportOption {
   valueParserCallback?: GetGroupTotalValueCallback;
 }
 
-export type GetDataValueCallback = (data: Date | string | number, columnDef: Column, excelFormatterId: number | undefined, excelStylesheet: unknown) => Date | string | number | ExcelCellFormat;
+export type GetDataValueCallback = (data: Date | string | number, columnDef: Column, excelFormatterId: number | undefined, excelStylesheet: unknown, gridOptions: GridOption) => Date | string | number | ExcelCellFormat;
 export type GetGroupTotalValueCallback = (totals: any, columnDef: Column, groupType: string, excelStylesheet: unknown) => Date | string | number;
 
 /**

diff --git a/packages/excel-export/src/excelExport.service.spec.ts b/packages/excel-export/src/excelExport.service.spec.ts
@@ -819,13 +819,14 @@ describe('ExcelExportService', () => {
       let mockItem1;
       let mockItem2;
       let mockGroup1;
-      let parserCallbackSpy = jest.fn();
-      let groupTotalParserCallbackSpy = jest.fn();
+      const parserCallbackSpy = jest.fn();
+      const groupTotalParserCallbackSpy = jest.fn();
 
       beforeEach(() => {
         mockGridOptions.enableGrouping = true;
         mockGridOptions.enableTranslate = false;
         mockGridOptions.excelExportOptions = { sanitizeDataExport: true, addGroupIndentation: true };
+        mockGridOptions.formatterOptions = { decimalSeparator: ',' };
 
         mockColumns = [
           { id: 'id', field: 'id', excludeFromExport: true },
@@ -863,7 +864,7 @@ describe('ExcelExportService', () => {
         };
 
         mockItem1 = { id: 0, userId: '1E06', firstName: 'John', lastName: 'X', position: 'SALES_REP', order: 10, cost: 22 };
-        mockItem2 = { id: 1, userId: '2B02', firstName: 'Jane', lastName: 'Doe', position: 'FINANCE_MANAGER', order: 10, cost: 33 };
+        mockItem2 = { id: 1, userId: '2B02', firstName: 'Jane', lastName: 'Doe', position: 'FINANCE_MANAGER', order: 10, cost: '$33,01' };
         mockGroup1 = {
           collapsed: 0, count: 2, groupingKey: '10', groups: null, level: 0, selectChecked: false,
           rows: [mockItem1, mockItem2],
@@ -908,8 +909,8 @@ describe('ExcelExportService', () => {
               { metadata: { style: 1, }, value: 'Cost', },
             ],
             ['⮟ Order: 20 (2 items)'],
-            ['', '1E06', 'John', 'X', 'SALES_REP', { metadata: { style: 3, type: "number", }, value: 10, }, 8888],
-            ['', '2B02', 'Jane', 'DOE', 'FINANCE_MANAGER', { metadata: { style: 3, type: "number", }, value: 10, }, 8888],
+            ['', '1E06', 'John', 'X', 'SALES_REP', { metadata: { style: 3, type: 'number', }, value: 10, }, 8888],
+            ['', '2B02', 'Jane', 'DOE', 'FINANCE_MANAGER', { metadata: { style: 3, type: 'number', }, value: 10, }, 8888],
             ['', '', '', '', '', { value: 20, metadata: { style: 5, type: 'number' } }, ''],
           ]
         });
@@ -921,7 +922,7 @@ describe('ExcelExportService', () => {
             numFmtId: 103,
           }
         });
-        expect(parserCallbackSpy).toHaveBeenCalledWith(22, mockColumns[6], undefined, expect.anything());
+        expect(parserCallbackSpy).toHaveBeenCalledWith(22, mockColumns[6], undefined, expect.anything(), mockGridOptions);
       });
     });
 
@@ -1028,7 +1029,7 @@ describe('ExcelExportService', () => {
       let mockGroup2;
       let mockGroup3;
       let mockGroup4;
-      let groupTotalParserCallbackSpy = jest.fn();
+      const groupTotalParserCallbackSpy = jest.fn();
 
       beforeEach(() => {
         mockGridOptions.enableGrouping = true;

diff --git a/packages/excel-export/src/excelExport.service.ts b/packages/excel-export/src/excelExport.service.ts
@@ -239,7 +239,7 @@ export class ExcelExportService implements ExternalResource, BaseExcelExportServ
    * All other browsers will use plain javascript on client side to produce a file download.
    * @param options
    */
-  startDownloadFile(options: { filename: string, blob: Blob, data: any[] }) {
+  startDownloadFile(options: { filename: string, blob: Blob, data: any[]; }) {
     // when using IE/Edge, then use different download call
     if (typeof (navigator as any).msSaveOrOpenBlob === 'function') {
       (navigator as any).msSaveOrOpenBlob(options.blob, options.filename);
@@ -583,14 +583,15 @@ export class ExcelExportService implements ExternalResource, BaseExcelExportServ
           }
           this._regularCellExcelFormats[columnDef.id] = cellStyleFormat;
         }
-        const { stylesheetFormatterId, getDataValueParser } = this._regularCellExcelFormats[columnDef.id];
-        itemData = getDataValueParser(itemData, columnDef, stylesheetFormatterId, this._stylesheet);
 
-        // does the user want to sanitize the output data (remove HTML tags)?
+        // sanitize early, when enabled, any HTML tags (remove HTML tags)
         if (typeof itemData === 'string' && (columnDef.sanitizeDataExport || this._excelExportOptions.sanitizeDataExport)) {
           itemData = sanitizeHtmlToText(itemData as string);
         }
 
+        const { stylesheetFormatterId, getDataValueParser } = this._regularCellExcelFormats[columnDef.id];
+        itemData = getDataValueParser(itemData, columnDef, stylesheetFormatterId, this._stylesheet, this._gridOptions);
+
         rowOutputStrings.push(itemData);
         idx++;
       }