From 8ba65fa2bd25d8d324f2d3796ca4e0c3eae210ed Mon Sep 17 00:00:00 2001 From: Carlos Valente Date: Wed, 22 Jan 2025 19:55:51 +0100 Subject: [PATCH] refactor: handle non numeric comparisons --- .../__tests__/automation.service.test.ts | 120 +++++++++++------- .../api-data/automation/automation.service.ts | 11 +- .../api-data/automation/automation.utils.ts | 38 ++++++ packages/utils/src/common/objectUtils.ts | 2 +- 4 files changed, 119 insertions(+), 52 deletions(-) diff --git a/apps/server/src/api-data/automation/__tests__/automation.service.test.ts b/apps/server/src/api-data/automation/__tests__/automation.service.test.ts index 418fcf5ce..817f3d7df 100644 --- a/apps/server/src/api-data/automation/__tests__/automation.service.test.ts +++ b/apps/server/src/api-data/automation/__tests__/automation.service.test.ts @@ -100,66 +100,88 @@ describe('testConditions()', () => { expect(result).toBe(true); }); - it('should compare two equal values', () => { - const mockStore = makeRuntimeStateData({ clock: 10 }); - const result = testConditions([{ field: 'clock', operator: 'equals', value: '10' }], 'all', mockStore); - expect(result).toBe(true); + describe('equals operator', () => { + it('should compare two equal values', () => { + const mockStore = makeRuntimeStateData({ clock: 10 }); + const result = testConditions([{ field: 'clock', operator: 'equals', value: '10' }], 'all', mockStore); + expect(result).toBe(true); + }); + + it('should check if a value does not exist', () => { + const mockStore = makeRuntimeStateData({ eventNow: null }); + const result = testConditions([{ field: 'eventNow.title', operator: 'equals', value: '' }], 'all', mockStore); + expect(result).toBe(true); + }); }); - it('should check if a value does not exist', () => { - const mockStore = makeRuntimeStateData({ eventNow: null }); - const result = testConditions([{ field: 'eventNow.title', operator: 'equals', value: '' }], 'all', mockStore); - expect(result).toBe(true); + describe('not_equals operator', () => { + it('should check if two values are different', () => { + const mockStore = makeRuntimeStateData({ clock: 10 }); + const result = testConditions([{ field: 'clock', operator: 'not_equals', value: '11' }], 'all', mockStore); + expect(result).toBe(true); + }); }); - it('should check if two values are different', () => { - const mockStore = makeRuntimeStateData({ clock: 10 }); - const result = testConditions([{ field: 'clock', operator: 'not_equals', value: '11' }], 'all', mockStore); - expect(result).toBe(true); + describe('greater_than operator', () => { + it('should check if the given value is smaller', () => { + const mockStore = makeRuntimeStateData({ clock: 10 }); + const result = testConditions([{ field: 'clock', operator: 'greater_than', value: '9' }], 'all', mockStore); + expect(result).toBe(true); + }); + it('should handle values which are not numbers', () => { + const mockStore = makeRuntimeStateData({ clock: 10 }); + const result = testConditions([{ field: 'clock', operator: 'greater_than', value: 'testing' }], 'all', mockStore); + expect(result).toBe(false); + }); }); - it('should check if the given value is smaller', () => { - const mockStore = makeRuntimeStateData({ clock: 10 }); - const result = testConditions([{ field: 'clock', operator: 'greater_than', value: '9' }], 'all', mockStore); - expect(result).toBe(true); + describe('less_than operator', () => { + it('should check if the given value is larger', () => { + const mockStore = makeRuntimeStateData({ clock: 10 }); + const result = testConditions([{ field: 'clock', operator: 'less_than', value: '11' }], 'all', mockStore); + expect(result).toBe(true); + }); + it('should handle values which are not numbers', () => { + const mockStore = makeRuntimeStateData({ clock: 10 }); + const result = testConditions([{ field: 'clock', operator: 'less_than', value: 'testing' }], 'all', mockStore); + expect(result).toBe(false); + }); }); - it('should check if the given value is larger', () => { - const mockStore = makeRuntimeStateData({ clock: 10 }); - const result = testConditions([{ field: 'clock', operator: 'less_than', value: '11' }], 'all', mockStore); - expect(result).toBe(true); + describe('contains operator', () => { + it('should check if value contains given string', () => { + const result = testConditions( + [{ field: 'eventNow.title', operator: 'contains', value: 'lighting' }], + 'all', + makeRuntimeStateData({ eventNow: makeOntimeEvent({ title: 'test-lighting' }) as PlayableEvent }), + ); + expect(result).toBe(true); + + const result2 = testConditions( + [{ field: 'eventNow.title', operator: 'contains', value: 'sound' }], + 'all', + makeRuntimeStateData({ eventNow: makeOntimeEvent({ title: 'test-lighting' }) as PlayableEvent }), + ); + expect(result2).toBe(false); + }); }); - it('should check if value contains given string', () => { - const result = testConditions( - [{ field: 'eventNow.title', operator: 'contains', value: 'lighting' }], - 'all', - makeRuntimeStateData({ eventNow: makeOntimeEvent({ title: 'test-lighting' }) as PlayableEvent }), - ); - expect(result).toBe(true); + describe('not_contains operator', () => { + it('should check if value does not contain given string', () => { + const result = testConditions( + [{ field: 'eventNow.title', operator: 'not_contains', value: 'lighting' }], + 'all', + makeRuntimeStateData({ eventNow: makeOntimeEvent({ title: 'test-lighting' }) as PlayableEvent }), + ); + expect(result).toBe(false); - const result2 = testConditions( - [{ field: 'eventNow.title', operator: 'contains', value: 'sound' }], - 'all', - makeRuntimeStateData({ eventNow: makeOntimeEvent({ title: 'test-lighting' }) as PlayableEvent }), - ); - expect(result2).toBe(false); - }); - - it('should check if value does not contain given string', () => { - const result = testConditions( - [{ field: 'eventNow.title', operator: 'not_contains', value: 'lighting' }], - 'all', - makeRuntimeStateData({ eventNow: makeOntimeEvent({ title: 'test-lighting' }) as PlayableEvent }), - ); - expect(result).toBe(false); - - const result2 = testConditions( - [{ field: 'eventNow.title', operator: 'not_contains', value: 'sound' }], - 'all', - makeRuntimeStateData({ eventNow: makeOntimeEvent({ title: 'test-lighting' }) as PlayableEvent }), - ); - expect(result2).toBe(true); + const result2 = testConditions( + [{ field: 'eventNow.title', operator: 'not_contains', value: 'sound' }], + 'all', + makeRuntimeStateData({ eventNow: makeOntimeEvent({ title: 'test-lighting' }) as PlayableEvent }), + ); + expect(result2).toBe(true); + }); }); describe('for all filter rule', () => { diff --git a/apps/server/src/api-data/automation/automation.service.ts b/apps/server/src/api-data/automation/automation.service.ts index 371f72e5c..1aa6ab349 100644 --- a/apps/server/src/api-data/automation/automation.service.ts +++ b/apps/server/src/api-data/automation/automation.service.ts @@ -14,6 +14,7 @@ import { isOntimeCloud } from '../../externals.js'; import { emitOSC } from './clients/osc.client.js'; import { emitHTTP } from './clients/http.client.js'; import { getAutomationsEnabled, getAutomations, getAutomationTriggers } from './automation.dao.js'; +import { isGreaterThan, isLessThan } from './automation.utils.js'; /** * Exposes a method for triggering actions based on a TimerLifeCycle event @@ -87,9 +88,15 @@ export function testConditions( case 'not_equals': return fieldValue != value; case 'greater_than': - return fieldValue > value; + if (typeof fieldValue !== 'number') { + return false; + } + return isGreaterThan(fieldValue, value); case 'less_than': - return fieldValue < value; + if (typeof fieldValue !== 'number') { + return false; + } + return isLessThan(fieldValue, value); case 'contains': return typeof fieldValue === 'string' && fieldValue.includes(value); case 'not_contains': diff --git a/apps/server/src/api-data/automation/automation.utils.ts b/apps/server/src/api-data/automation/automation.utils.ts index 06ce47aaf..72eb0539d 100644 --- a/apps/server/src/api-data/automation/automation.utils.ts +++ b/apps/server/src/api-data/automation/automation.utils.ts @@ -133,3 +133,41 @@ const quickAliases: AliasesDefinition = { }, startedAt: { key: 'timer.startedAt', cb: (value: string) => formatDisplayFromString(value) }, }; + +/** + * Utility encapsulates logic for comparing two strings which may encode numbers + * @example isGreaterThan('10', '5') // true + * @example isGreaterThan('5', '10') // false + * @example isGreaterThan('Ontime', 'Cool') // false + */ +export function isGreaterThan(a: string, b: string): boolean { + const aValue = Number(a); + const bValue = Number(b); + + // we check if the values encore numbers and compare them + if (!isNaN(aValue) && !isNaN(bValue)) { + return aValue > bValue; + } + + // If either value is not a number, there is no logical comparison to be made + return false; +} + +/** + * Utility encapsulates logic for comparing two strings which may encode numbers + * @example isLessThan('10', '5') // false + * @example isLessThan('5', '10') // true + * @example isLessThan('Ontime', 'Cool') // false + */ +export function isLessThan(a: string, b: string): boolean { + const aValue = Number(a); + const bValue = Number(b); + + // we check if the values encore numbers and compare them + if (!isNaN(aValue) && !isNaN(bValue)) { + return aValue < bValue; + } + + // If either value is not a number, there is no logical comparison to be made + return false; +} diff --git a/packages/utils/src/common/objectUtils.ts b/packages/utils/src/common/objectUtils.ts index b481c9015..d9fe04e40 100644 --- a/packages/utils/src/common/objectUtils.ts +++ b/packages/utils/src/common/objectUtils.ts @@ -1,7 +1,7 @@ /** * Extracts a value from a nested object using a dot-separated path */ -export function getPropertyFromPath(path: string, obj: T): any | undefined { +export function getPropertyFromPath(path: string, obj: T): unknown | undefined { const keys = path.split('.'); let result: any = obj;