Improve xlsx import (#1271)

* use xlsx

* add existing custom fields to import map

* keep colours of existing custom fields

* use unknown

* fix

* revert

* allow space and upper case

* fix test

* define empty sheet

apps/server/src/api-data/excel/excel.service.ts

@@ -8,13 +8,15 @@ import { ImportMap } from 'ontime-utils';
 
 import { extname } from 'path';
 import { existsSync } from 'fs';
-import xlsx from 'node-xlsx';
+import xlsx from 'xlsx';
+import type { WorkBook } from 'xlsx';
 
 import { parseExcel } from '../../utils/parser.js';
 import { parseRundown } from '../../utils/parserFunctions.js';
 import { deleteFile } from '../../utils/parserUtils.js';
+import { getCustomFields } from '../../services/rundown-service/rundownCache.js';
 
-let excelData: { name: string; data: unknown[][] }[] = [];
+let excelData: WorkBook = xlsx.utils.book_new();
 
 export async function saveExcelFile(filePath: string) {
   if (!existsSync(filePath)) {
@@ -23,24 +25,25 @@ export async function saveExcelFile(filePath: string) {
   if (extname(filePath) != '.xlsx') {
     throw new Error('Wrong file format');
   }
-  excelData = xlsx.parse(filePath, { cellDates: true });
+  excelData = xlsx.readFile(filePath, { cellDates: true, cellFormula: false });
 
   await deleteFile(filePath);
 }
 
-export function listWorksheets() {
-  return excelData.map((value) => value.name);
+export function listWorksheets(): string[] {
+  return excelData.SheetNames;
 }
 
 export function generateRundownPreview(options: ImportMap): { rundown: OntimeRundown; customFields: CustomFields } {
-  const data = excelData.find(({ name }) => name.toLowerCase() === options.worksheet.toLowerCase())?.data;
+  const data = excelData.Sheets[options.worksheet];
 
   if (!data) {
     throw new Error(`Could not find data to import, maybe the worksheet name is incorrect: ${options.worksheet}`);
   }
 
-  const dataFromExcel = parseExcel(data, options);
+  const arrayOfData: unknown[][] = xlsx.utils.sheet_to_json(data, { header: 1, blankrows: false, raw: false });
 
+  const dataFromExcel = parseExcel(arrayOfData, getCustomFields(), options);
   // we run the parsed data through an extra step to ensure the objects shape
   const { rundown, customFields } = parseRundown(dataFromExcel);
   if (rundown.length === 0) {
@@ -48,7 +51,7 @@ export function generateRundownPreview(options: ImportMap): { rundown: OntimeRun
   }
 
   // clear the data
-  excelData = [];
+  excelData = undefined;
 
   return { rundown, customFields };
 }

apps/server/src/services/sheet-service/SheetService.ts

@@ -15,6 +15,7 @@ import { parseExcel } from '../../utils/parser.js';
 import { logger } from '../../classes/Logger.js';
 import { parseRundown } from '../../utils/parserFunctions.js';
 import { getRundown } from '../rundown-service/rundownUtils.js';
+import { getCustomFields } from '../rundown-service/rundownCache.js';
 
 import { cellRequestFromEvent, type ClientSecret, getA1Notation, validateClientSecret } from './sheetUtils.js';
 
@@ -286,7 +287,7 @@ export async function upload(sheetId: string, options: ImportMap) {
     throw new Error(`Sheet read failed: ${readResponse.statusText}`);
   }
 
-  const { rundownMetadata } = parseExcel(readResponse.data.values, options);
+  const { rundownMetadata } = parseExcel(readResponse.data.values, getCustomFields(), options);
   const rundown = getRundown();
   const titleRow = Object.values(rundownMetadata)[0]['row'];
   const updateRundown = Array<sheets_v4.Schema$Request>();
@@ -363,7 +364,7 @@ export async function download(
     throw new Error(`Sheet read failed: ${googleResponse.statusText}`);
   }
 
-  const dataFromSheet = parseExcel(googleResponse.data.values, options);
+  const dataFromSheet = parseExcel(googleResponse.data.values, getCustomFields(), options);
   const { customFields, rundown } = parseRundown(dataFromSheet);
   if (rundown.length < 1) {
     throw new Error('Sheet: Could not find data to import in the worksheet');

apps/server/src/utils/__tests__/parser.test.ts

@@ -2,6 +2,7 @@
 import { assertType, vi } from 'vitest';
 
 import {
+  CustomFields,
   DatabaseModel,
   EndAction,
   OntimeEvent,
@@ -788,7 +789,7 @@ describe('getCustomFieldData()', () => {
       },
     } as ImportMap;
 
-    const result = getCustomFieldData(importMap);
+    const result = getCustomFieldData(importMap, {});
     expect(result.customFields).toStrictEqual({
       lighting: {
         type: 'string',
@@ -807,6 +808,61 @@ describe('getCustomFieldData()', () => {
       },
     });
 
+    // it is an inverted record of <importKey, ontimeKey>
+    expect(result.customFieldImportKeys).toStrictEqual({
+      lx: 'lighting',
+      sound: 'sound',
+      av: 'video',
+    });
+  });
+  it('keeps colour information from existing fields', () => {
+    const importMap = {
+      worksheet: 'event schedule',
+      timeStart: 'time start',
+      linkStart: 'link start',
+      timeEnd: 'time end',
+      duration: 'duration',
+      cue: 'cue',
+      title: 'title',
+      isPublic: 'public',
+      skip: 'skip',
+      note: 'notes',
+      colour: 'colour',
+      endAction: 'end action',
+      timerType: 'timer type',
+      timeWarning: 'warning time',
+      timeDanger: 'danger time',
+      custom: {
+        lighting: 'lx',
+        sound: 'sound',
+        video: 'av',
+      },
+    } as ImportMap;
+
+    const customFields: CustomFields = {
+      lighting: { label: 'lx', type: 'string', colour: 'red' },
+      sound: { label: 'sound', type: 'string', colour: 'green' },
+    };
+
+    const result = getCustomFieldData(importMap, customFields);
+    expect(result.customFields).toStrictEqual({
+      lighting: {
+        type: 'string',
+        colour: 'red',
+        label: 'lighting',
+      },
+      sound: {
+        type: 'string',
+        colour: 'green',
+        label: 'sound',
+      },
+      video: {
+        type: 'string',
+        colour: '',
+        label: 'video',
+      },
+    });
+
     // it is an inverted record of <importKey, ontimeKey>
     expect(result.customFieldImportKeys).toStrictEqual({
       lx: 'lighting',
@@ -919,7 +975,7 @@ describe('parseExcel()', () => {
         note: 'Ballyhoo',
         custom: {
           user0: 'a0',
-          user1: 'a1',
+          User1: 'a1',
           user2: 'a2',
           user3: 'a3',
           user4: 'a4',
@@ -952,21 +1008,27 @@ describe('parseExcel()', () => {
       },
     ];
 
-    const parsedData = parseExcel(testdata, importMap);
+    const existingCustomFields: CustomFields = {
+      user0: { type: 'string', colour: 'red', label: 'user0' },
+      User1: { type: 'string', colour: 'green', label: 'user1' },
+      user2: { type: 'string', colour: 'blue', label: 'user2' },
+    };
+
+    const parsedData = parseExcel(testdata, existingCustomFields, importMap);
     expect(parsedData.customFields).toStrictEqual({
       user0: {
         type: 'string',
-        colour: '',
+        colour: 'red',
         label: 'user0',
       },
-      user1: {
+      User1: {
         type: 'string',
-        colour: '',
+        colour: 'green',
         label: 'User1',
       },
       user2: {
         type: 'string',
-        colour: '',
+        colour: 'blue',
         label: 'user2',
       },
       user3: {
@@ -1123,7 +1185,7 @@ describe('parseExcel()', () => {
       },
     ];
 
-    const parsedData = parseExcel(testdata, importMap);
+    const parsedData = parseExcel(testdata, {}, importMap);
     expect(parsedData.customFields).toStrictEqual({
       niu1: {
         type: 'string',
@@ -1229,7 +1291,7 @@ describe('parseExcel()', () => {
       timeDanger: 'danger time',
       custom: {},
     };
-    const result = parseExcel(testdata, importMap);
+    const result = parseExcel(testdata, {}, importMap);
     expect(result.rundown.length).toBe(1);
     expect((result.rundown.at(0) as OntimeEvent).title).toBe('A song from the hearth');
   });
@@ -1322,7 +1384,7 @@ describe('parseExcel()', () => {
       timeDanger: 'danger time',
       custom: {},
     };
-    const result = parseExcel(testdata, importMap);
+    const result = parseExcel(testdata, {}, importMap);
     expect(result.rundown.length).toBe(2);
     expect((result.rundown.at(0) as OntimeEvent).type).toBe(SupportedEvent.Block);
   });
@@ -1392,7 +1454,7 @@ describe('parseExcel()', () => {
       timeDanger: 'danger time',
       custom: {},
     };
-    const result = parseExcel(testdata, importMap);
+    const result = parseExcel(testdata, {}, importMap);
     expect(result.rundown.length).toBe(2);
     expect((result.rundown.at(0) as OntimeEvent).type).toBe(SupportedEvent.Event);
     expect((result.rundown.at(0) as OntimeEvent).timerType).toBe(TimerType.CountDown);
@@ -1510,7 +1572,7 @@ describe('parseExcel()', () => {
       timeDanger: 'danger time',
       custom: {},
     };
-    const result = parseExcel(testdata, importMap);
+    const result = parseExcel(testdata, {}, importMap);
     expect(result.rundown.length).toBe(3);
     expect((result.rundown.at(0) as OntimeEvent).type).toBe(SupportedEvent.Event);
     expect((result.rundown.at(0) as OntimeEvent).timerType).toBe(TimerType.CountDown);
@@ -1551,7 +1613,7 @@ describe('parseExcel()', () => {
       timeDanger: 'danger time',
       custom: {},
     };
-    const result = parseExcel(testData, importMap);
+    const result = parseExcel(testData, {}, importMap);
     const { rundown } = parseRundown(result);
     const events = rundown.filter((e) => e.type === SupportedEvent.Event) as OntimeEvent[];
     expect((events.at(0) as OntimeEvent).timeStart).toEqual(16200000);
@@ -1588,7 +1650,7 @@ describe('parseExcel()', () => {
       custom: {},
     };
 
-    const result = parseExcel(testData, importMap);
+    const result = parseExcel(testData, {}, importMap);
     const { rundown } = parseRundown(result);
     const events = rundown.filter((e) => e.type === SupportedEvent.Event) as OntimeEvent[];
     expect((events.at(0) as OntimeEvent).timeStart).toEqual(16200000); //<--leading white space in MAP
@@ -1640,7 +1702,7 @@ describe('parseExcel()', () => {
       custom: {},
     };
 
-    const result = parseExcel(testData, importMap);
+    const result = parseExcel(testData, {}, importMap);
     const parseResult = parseRundown(result);
 
     cache.init(parseResult.rundown, parseResult.customFields);
@@ -1774,7 +1836,7 @@ describe('parseExcel()', () => {
       ['MEET4', '#779BE7', '', '', 30, true, 'Meeting 4', '', 'count-up', 'none', 11, '00:05:00', 'TRUE', 'FALSE'],
     ];
 
-    const parsedData = parseExcel(testData);
+    const parsedData = parseExcel(testData, {});
     const { rundown } = parsedData;
 
     // elements in bug report

apps/server/src/utils/parser.ts

@@ -1,4 +1,5 @@
 import {
+  customFieldLabelToKey,
   defaultImportMap,
   generateId,
   type ImportMap,
@@ -54,18 +55,22 @@ function parseBooleanString(value: unknown): boolean {
   return value.toLowerCase() !== 'false';
 }
 
-export function getCustomFieldData(importMap: ImportMap): {
+export function getCustomFieldData(
+  importMap: ImportMap,
+  existingCustomFields: CustomFields,
+): {
   customFields: CustomFields;
   customFieldImportKeys: Record<keyof CustomFields, string>;
 } {
   const customFields = {};
   const customFieldImportKeys = {};
   for (const ontimeLabel in importMap.custom) {
-    const ontimeKey = ontimeLabel.toLowerCase();
+    const ontimeKey = customFieldLabelToKey(ontimeLabel);
     const importLabel = importMap.custom[ontimeLabel].toLowerCase();
+    const colour = ontimeKey in existingCustomFields ? existingCustomFields[ontimeKey].colour : '';
     customFields[ontimeKey] = {
       type: 'string',
-      colour: '',
+      colour,
       label: ontimeLabel,
     };
     customFieldImportKeys[importLabel] = ontimeKey;
@@ -79,7 +84,11 @@ export function getCustomFieldData(importMap: ImportMap): {
  * @param {ImportOptions} options - an object that contains the import map
  * @returns {object} - parsed object
  */
-export const parseExcel = (excelData: unknown[][], options?: Partial<ImportMap>): ExcelData => {
+export const parseExcel = (
+  excelData: unknown[][],
+  existingCustomFields: CustomFields,
+  options?: Partial<ImportMap>,
+): ExcelData => {
   const rundownMetadata = {};
   const importMap: ImportMap = { ...defaultImportMap, ...options };
 
@@ -89,7 +98,7 @@ export const parseExcel = (excelData: unknown[][], options?: Partial<ImportMap>)
     }
   }
 
-  const { customFields, customFieldImportKeys } = getCustomFieldData(importMap);
+  const { customFields, customFieldImportKeys } = getCustomFieldData(importMap, existingCustomFields);
   const rundown: OntimeRundown = [];
 
   // title stuff: strings