From dae6f84af0dd25c390b9e762ab963f118ca0c0e1 Mon Sep 17 00:00:00 2001 From: Carlos Valente Date: Thu, 16 May 2024 17:49:36 +0200 Subject: [PATCH] fix: excel parsing --- .../server/src/utils/__tests__/parser.test.ts | 129 +++++++++++++++++- apps/server/src/utils/__tests__/time.test.ts | 13 +- apps/server/src/utils/parser.ts | 4 + apps/server/src/utils/time.ts | 39 +++--- .../utils/src/date-utils/isTimeString.test.ts | 15 +- packages/utils/src/date-utils/isTimeString.ts | 14 +- 6 files changed, 188 insertions(+), 26 deletions(-) diff --git a/apps/server/src/utils/__tests__/parser.test.ts b/apps/server/src/utils/__tests__/parser.test.ts index 70e0964da..84d5d57f0 100644 --- a/apps/server/src/utils/__tests__/parser.test.ts +++ b/apps/server/src/utils/__tests__/parser.test.ts @@ -19,7 +19,7 @@ import { dbModel } from '../../models/dataModel.js'; import { createEvent, getCustomFieldData, parseExcel, parseJson } from '../parser.js'; import { makeString } from '../parserUtils.js'; import { parseRundown, parseUrlPresets, parseViewSettings } from '../parserFunctions.js'; -import { ImportMap } from 'ontime-utils'; +import { ImportMap, MILLIS_PER_MINUTE } from 'ontime-utils'; import * as cache from '../../services/rundown-service/rundownCache.js'; describe('test json parser with valid def', () => { @@ -1515,4 +1515,131 @@ describe('parseExcel()', () => { expect((rundown[sixthId] as OntimeEvent).timeStart).toEqual((rundown[fourthId] as OntimeEvent).timeEnd); expect((rundown[sixthId] as OntimeEvent).linkStart).toEqual((rundown[fourthId] as OntimeEvent).id); }); + + it('#971 BUG: parses time fields and booleans', () => { + const testData = [ + [ + 'Cue', + 'Colour', + 'Time Start', + 'Time End', + 'Duration', + 'Link Start', + 'Title', + 'Note', + 'Timer Type', + 'End Action', + 'Warning time', + 'Danger time', + 'Public', + 'Skip', + ], + [ + 'SETUP', + '', + '1899-12-30T07:15:00.000Z', + '1899-12-30T08:30:00.000Z', + '', + 'false', + 'Setup', + '', + 'time-to-end', + 'none', + '15', + '00:05:00', + 'FALSE', + 'TRUE', + ], + [ + 'MEET1', + '#779BE7', + '1899-12-30T08:30:00.000Z', + '1899-12-30T10:00:00.000Z', + '', + 'false', + 'Meeting 1', + '', + 'time-to-end', + 'none', + 15, + '00:05:00', + 'TRUE', + 'FALSE', + ], + [ + 'MEET2', + '#779BE7', + '1899-12-30T10:00:00.000Z', + '', + '60', + 'false', + 'Meeting 2', + '', + 'time-to-end', + 'none', + '13', + '5', + 'TRUE', + 'FALSE', + ], + [ + 'lunch', + '#77C785', + '', + '1899-12-30T11:30:00.000Z', + '', + 'true', + 'Lunch', + '', + 'time-to-end', + 'none', + 13, + 5, + 'FALSE', + 'FALSE', + ], + [ + 'MEET3', + '#779BE7', + '1899-12-30T11:30:00.000Z', + '', + 90, + false, + 'Meeting 3', + '', + 'count-up', + 'none', + '11', + 5, + 'TRUE', + 'FALSE', + ], + ['MEET4', '#779BE7', '', '', 30, true, 'Meeting 4', '', 'count-up', 'none', 11, '00:05:00', 'TRUE', 'FALSE'], + ]; + + const parsedData = parseExcel(testData); + const { rundown } = parsedData; + + // elements in bug report + // 15 is a number, in which case we parse it as a minutes value + expect((rundown.at(1) as OntimeEvent).timeWarning).toBe(15 * MILLIS_PER_MINUTE); + + // in the case where a string is passed, we need to check whether it is an ISO 8601 date + expect((rundown.at(2) as OntimeEvent).duration).toBe(60 * MILLIS_PER_MINUTE); + expect((rundown.at(2) as OntimeEvent).timeDanger).toBe(5 * MILLIS_PER_MINUTE); + + expect((rundown.at(3) as OntimeEvent).timeWarning).toBe(13 * MILLIS_PER_MINUTE); + expect((rundown.at(3) as OntimeEvent).timeDanger).toBe(5 * MILLIS_PER_MINUTE); + + expect((rundown.at(4) as OntimeEvent).duration).toBe(90 * MILLIS_PER_MINUTE); + expect((rundown.at(4) as OntimeEvent).linkStart).toBe(false); + expect((rundown.at(4) as OntimeEvent).timeWarning).toBe(11 * MILLIS_PER_MINUTE); + expect((rundown.at(4) as OntimeEvent).timeDanger).toBe(5 * MILLIS_PER_MINUTE); + + expect((rundown.at(5) as OntimeEvent).duration).toBe(30 * MILLIS_PER_MINUTE); + + // if we get a boolean, we should just use that + expect((rundown.at(5) as OntimeEvent).linkStart).toBe(true); + expect((rundown.at(5) as OntimeEvent).timeWarning).toBe(11 * MILLIS_PER_MINUTE); + }); }); diff --git a/apps/server/src/utils/__tests__/time.test.ts b/apps/server/src/utils/__tests__/time.test.ts index 48521a4d3..31febd9a0 100644 --- a/apps/server/src/utils/__tests__/time.test.ts +++ b/apps/server/src/utils/__tests__/time.test.ts @@ -1,3 +1,4 @@ +import { MILLIS_PER_MINUTE } from 'ontime-utils'; import { parseExcelDate } from '../time.js'; describe('parseExcelDate', () => { @@ -46,8 +47,18 @@ describe('parseExcelDate', () => { }); }); + describe('uses numeric fields as minutes', () => { + const invalidFields = [1, 10, 100]; + invalidFields.forEach((field) => { + it(`handles ${field}`, () => { + const millis = parseExcelDate(field); + expect(millis).toBe(field * MILLIS_PER_MINUTE); + }); + }); + }); + describe('returns 0 on other strings', () => { - const invalidFields = ['10', 'test', '']; + const invalidFields = ['test', '']; invalidFields.forEach((field) => { it(`handles ${field}`, () => { const millis = parseExcelDate(field); diff --git a/apps/server/src/utils/parser.ts b/apps/server/src/utils/parser.ts index 0d402465e..95e186d2d 100644 --- a/apps/server/src/utils/parser.ts +++ b/apps/server/src/utils/parser.ts @@ -43,6 +43,10 @@ type ExcelData = Pick & { }; function parseBooleanString(value: unknown): boolean { + if (typeof value === 'boolean') { + return value; + } + // falsy values would be nullish or empty string if (!value || typeof value !== 'string') { return false; diff --git a/apps/server/src/utils/time.ts b/apps/server/src/utils/time.ts index f09d44126..34feb90f9 100644 --- a/apps/server/src/utils/time.ts +++ b/apps/server/src/utils/time.ts @@ -1,4 +1,5 @@ -import { isTimeString } from 'ontime-utils'; +import { MILLIS_PER_MINUTE } from 'ontime-utils'; +import { isISO8601 } from '../../../../packages/utils/src/date-utils/isTimeString.js'; const mts = 1000; // millis to seconds const mtm = 1000 * 60; // millis to minutes @@ -49,10 +50,9 @@ const stripAMPM = (value: string) => { /** * @description Parses a time string to millis, copied from client code * @param {string} value - time string - * @param {boolean} fillLeft - autofill left = hours / right = seconds * @returns {number} - time string in millis */ -export const forgivingStringToMillis = (value: string, fillLeft: boolean = true): number => { +export const forgivingStringToMillis = (value: string): number => { let millis = 0; // check for AM/PM indicators @@ -87,16 +87,9 @@ export const forgivingStringToMillis = (value: string, fillLeft: boolean = true) } } if (first != null && second != null && third == null) { - // if string has two sections - if (fillLeft) { - // treat as [hours] [minutes] - millis += parse(first) * mth; - millis += parse(second) * mtm; - } else { - // treat as [minutes] [seconds] - millis += parse(first) * mtm; - millis += parse(second) * mts; - } + // if string has two sections treat as [hours] [minutes] + millis += parse(first) * mth; + millis += parse(second) * mtm; } return millis; }; @@ -109,13 +102,21 @@ export const forgivingStringToMillis = (value: string, fillLeft: boolean = true) export const parseExcelDate = (excelDate: unknown): number => { if (excelDate instanceof Date) { return dateToMillis(excelDate); - } else if (typeof excelDate === 'string') { - const date = new Date(excelDate); - if (date instanceof Date && !isNaN(date.getTime())) { - return dateToMillis(date); - } else if (isTimeString(excelDate)) { - return forgivingStringToMillis(excelDate); + } + + if (typeof excelDate === 'string') { + if (isISO8601(excelDate)) { + const date = new Date(excelDate); + if (date instanceof Date && !isNaN(date.getTime())) { + return dateToMillis(date); + } } + return forgivingStringToMillis(excelDate); + } + + // if the user uses a number value eg. 15, excel could format the cell as number + if (typeof excelDate === 'number') { + return excelDate * MILLIS_PER_MINUTE; } return 0; diff --git a/packages/utils/src/date-utils/isTimeString.test.ts b/packages/utils/src/date-utils/isTimeString.test.ts index dfb829570..5c5a545ce 100644 --- a/packages/utils/src/date-utils/isTimeString.test.ts +++ b/packages/utils/src/date-utils/isTimeString.test.ts @@ -1,4 +1,4 @@ -import { isTimeString } from './isTimeString'; +import { isISO8601, isTimeString } from './isTimeString'; describe('test isTimeString() function', () => { it('it validates time strings', () => { @@ -33,3 +33,16 @@ describe('test isTimeString() function handle AM/PM', () => { }); } }); + +describe('isISO8601()', () => { + it('returns true for valid ISO 8601 date-time strings', () => { + expect(isISO8601('1899-12-30T08:30:00.000Z')).toBe(true); + expect(isISO8601('2022-01-01T00:00:00.000Z')).toBe(true); + }); + + it('returns false for invalid ISO 8601 date-time strings', () => { + expect(isISO8601('not a date')).toBe(false); + expect(isISO8601('1899-12-30T08:30:00Z')).toBe(false); // missing milliseconds + expect(isISO8601('1899-12-30 08:30:00.000Z')).toBe(false); // space instead of 'T' + }); +}); diff --git a/packages/utils/src/date-utils/isTimeString.ts b/packages/utils/src/date-utils/isTimeString.ts index 52bce2164..c8db67e5e 100644 --- a/packages/utils/src/date-utils/isTimeString.ts +++ b/packages/utils/src/date-utils/isTimeString.ts @@ -1,9 +1,15 @@ /** * @description Validates a time string - * @param {string} text - time string "23:00:12" - * @returns {boolean} string represents time */ -export const isTimeString = (text: string): boolean => { +export function isTimeString(text: string): boolean { const regex = /^(?:(?:([01]?\d|2[0-3])[:,.])?([0-5]?\d)[:,.])?([0-5]?\d)?(\s)?([APap][Mm])?$/; return regex.test(text); -}; +} + +/** + * @description Validates a ISO8601 date-time string + */ +export function isISO8601(text: string): boolean { + const regex = /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3}Z$/; + return regex.test(text); +}