From 36572c38033022cbf259d4e37fde507f60af4354 Mon Sep 17 00:00:00 2001 From: mohamed yahia Date: Fri, 8 Nov 2024 03:13:46 +0200 Subject: [PATCH] Improve conditional variables (#6301) * Add some operations to strings * Rename some types for conditional variables * Ignore type error ( for now ) * Fix tests * Add tests for the newly added string operations --------- Co-authored-by: Artur Arseniev --- .../model/DataVariableListenerManager.ts | 1 + .../model/conditional_variables/Condition.ts | 15 +++++--- .../ConditionalComponent.ts | 4 +- .../conditional_variables/DataCondition.ts | 36 ++++++++++-------- .../LogicalGroupStatement.ts | 4 +- .../operators/StringOperations.ts | 9 ++++- .../conditional_variables/DataCondition.ts | 38 +++++++++---------- .../operators/StringOperator.ts | 37 ++++++++++++++++++ 8 files changed, 99 insertions(+), 45 deletions(-) diff --git a/packages/core/src/data_sources/model/DataVariableListenerManager.ts b/packages/core/src/data_sources/model/DataVariableListenerManager.ts index 46d1528b91..d10fbd43dc 100644 --- a/packages/core/src/data_sources/model/DataVariableListenerManager.ts +++ b/packages/core/src/data_sources/model/DataVariableListenerManager.ts @@ -40,6 +40,7 @@ export default class DynamicVariableListenerManager { const { em, dynamicVariable, model } = this; this.removeListeners(); + // @ts-ignore const type = dynamicVariable.get('type'); let dataListeners: DataVariableListener[] = []; switch (type) { diff --git a/packages/core/src/data_sources/model/conditional_variables/Condition.ts b/packages/core/src/data_sources/model/conditional_variables/Condition.ts index 6af9152925..a17fd70bf8 100644 --- a/packages/core/src/data_sources/model/conditional_variables/Condition.ts +++ b/packages/core/src/data_sources/model/conditional_variables/Condition.ts @@ -1,7 +1,7 @@ import { DataVariableDefinition, DataVariableType } from './../DataVariable'; import EditorModel from '../../../editor/model/Editor'; import { evaluateVariable, isDataVariable } from '../utils'; -import { Expression, LogicGroup } from './DataCondition'; +import { ExpressionDefinition, LogicGroupDefinition } from './DataCondition'; import { LogicalGroupStatement } from './LogicalGroupStatement'; import { Operator } from './operators'; import { GenericOperation, GenericOperator } from './operators/GenericOperator'; @@ -11,10 +11,10 @@ import { StringOperator, StringOperation } from './operators/StringOperations'; import { Model } from '../../../common'; export class Condition extends Model { - private condition: Expression | LogicGroup | boolean; + private condition: ExpressionDefinition | LogicGroupDefinition | boolean; private em: EditorModel; - constructor(condition: Expression | LogicGroup | boolean, opts: { em: EditorModel }) { + constructor(condition: ExpressionDefinition | LogicGroupDefinition | boolean, opts: { em: EditorModel }) { super(condition); this.condition = condition; this.em = opts.em; @@ -76,7 +76,10 @@ export class Condition extends Model { /** * Recursively extracts variables from expressions or logic groups. */ - private extractVariables(condition: boolean | LogicGroup | Expression, variables: DataVariableDefinition[]): void { + private extractVariables( + condition: boolean | LogicGroupDefinition | ExpressionDefinition, + variables: DataVariableDefinition[], + ): void { if (this.isExpression(condition)) { if (isDataVariable(condition.left)) variables.push(condition.left); if (isDataVariable(condition.right)) variables.push(condition.right); @@ -88,14 +91,14 @@ export class Condition extends Model { /** * Checks if a condition is a LogicGroup. */ - private isLogicGroup(condition: any): condition is LogicGroup { + private isLogicGroup(condition: any): condition is LogicGroupDefinition { return condition && typeof condition.logicalOperator !== 'undefined' && Array.isArray(condition.statements); } /** * Checks if a condition is an Expression. */ - private isExpression(condition: any): condition is Expression { + private isExpression(condition: any): condition is ExpressionDefinition { return condition && typeof condition.left !== 'undefined' && typeof condition.operator === 'string'; } diff --git a/packages/core/src/data_sources/model/conditional_variables/ConditionalComponent.ts b/packages/core/src/data_sources/model/conditional_variables/ConditionalComponent.ts index ccf4142b93..c6bba9aecd 100644 --- a/packages/core/src/data_sources/model/conditional_variables/ConditionalComponent.ts +++ b/packages/core/src/data_sources/model/conditional_variables/ConditionalComponent.ts @@ -2,10 +2,10 @@ import Component from '../../../dom_components/model/Component'; import Components from '../../../dom_components/model/Components'; import { ComponentDefinition, ComponentOptions } from '../../../dom_components/model/types'; import { toLowerCase } from '../../../utils/mixins'; -import { DataCondition, ConditionalVariableType, Expression, LogicGroup } from './DataCondition'; +import { DataCondition, ConditionalVariableType, ExpressionDefinition, LogicGroupDefinition } from './DataCondition'; type ConditionalComponentDefinition = { - condition: Expression | LogicGroup | boolean; + condition: ExpressionDefinition | LogicGroupDefinition | boolean; ifTrue: any; ifFalse: any; }; diff --git a/packages/core/src/data_sources/model/conditional_variables/DataCondition.ts b/packages/core/src/data_sources/model/conditional_variables/DataCondition.ts index 5630198f17..d5a481645d 100644 --- a/packages/core/src/data_sources/model/conditional_variables/DataCondition.ts +++ b/packages/core/src/data_sources/model/conditional_variables/DataCondition.ts @@ -10,40 +10,40 @@ import DataVariable, { DataVariableDefinition } from '../DataVariable'; import { evaluateVariable, isDataVariable } from '../utils'; export const ConditionalVariableType = 'conditional-variable'; -export type Expression = { +export type ExpressionDefinition = { left: any; operator: GenericOperation | StringOperation | NumberOperation; right: any; }; -export type LogicGroup = { +export type LogicGroupDefinition = { logicalOperator: LogicalOperation; - statements: (Expression | LogicGroup | boolean)[]; + statements: (ExpressionDefinition | LogicGroupDefinition | boolean)[]; }; +export type ConditionDefinition = ExpressionDefinition | LogicGroupDefinition | boolean; export type ConditionalVariableDefinition = { type: typeof ConditionalVariableType; - condition: Expression | LogicGroup | boolean; + condition: ConditionDefinition; ifTrue: any; ifFalse: any; }; -export class DataCondition extends Model { +type DataConditionType = { + type: typeof ConditionalVariableType; + condition: Condition; + ifTrue: any; + ifFalse: any; +}; +export class DataCondition extends Model { lastEvaluationResult: boolean; private condition: Condition; private em: EditorModel; private variableListeners: DynamicVariableListenerManager[] = []; private _onValueChange?: () => void; - defaults() { - return { - type: ConditionalVariableType, - condition: false, - }; - } - constructor( - condition: Expression | LogicGroup | boolean, + condition: ExpressionDefinition | LogicGroupDefinition | boolean, public ifTrue: any, public ifFalse: any, opts: { em: EditorModel; onValueChange?: () => void }, @@ -52,8 +52,14 @@ export class DataCondition extends Model { throw new MissingConditionError(); } - super(); - this.condition = new Condition(condition, { em: opts.em }); + const conditionInstance = new Condition(condition, { em: opts.em }); + super({ + type: ConditionalVariableType, + condition: conditionInstance, + ifTrue, + ifFalse, + }); + this.condition = conditionInstance; this.em = opts.em; this.lastEvaluationResult = this.evaluate(); this.listenToDataVariables(); diff --git a/packages/core/src/data_sources/model/conditional_variables/LogicalGroupStatement.ts b/packages/core/src/data_sources/model/conditional_variables/LogicalGroupStatement.ts index cba3764955..284e99e96e 100644 --- a/packages/core/src/data_sources/model/conditional_variables/LogicalGroupStatement.ts +++ b/packages/core/src/data_sources/model/conditional_variables/LogicalGroupStatement.ts @@ -1,5 +1,5 @@ import { LogicalOperator } from './operators/LogicalOperator'; -import { Expression, LogicGroup } from './DataCondition'; +import { ExpressionDefinition, LogicGroupDefinition } from './DataCondition'; import { Condition } from './Condition'; import EditorModel from '../../../editor/model/Editor'; @@ -8,7 +8,7 @@ export class LogicalGroupStatement { constructor( private operator: LogicalOperator, - private statements: (Expression | LogicGroup | boolean)[], + private statements: (ExpressionDefinition | LogicGroupDefinition | boolean)[], opts: { em: EditorModel }, ) { this.em = opts.em; diff --git a/packages/core/src/data_sources/model/conditional_variables/operators/StringOperations.ts b/packages/core/src/data_sources/model/conditional_variables/operators/StringOperations.ts index d7ac89ab63..52c5eb6fb8 100644 --- a/packages/core/src/data_sources/model/conditional_variables/operators/StringOperations.ts +++ b/packages/core/src/data_sources/model/conditional_variables/operators/StringOperations.ts @@ -5,6 +5,8 @@ export enum StringOperation { startsWith = 'startsWith', endsWith = 'endsWith', matchesRegex = 'matchesRegex', + equalsIgnoreCase = 'equalsIgnoreCase', + trimEquals = 'trimEquals', } export class StringOperator extends Operator { @@ -12,7 +14,7 @@ export class StringOperator extends Operator { super(); } - evaluate(left: string, right: string): boolean { + evaluate(left: string, right: string) { switch (this.operator) { case StringOperation.contains: return left.includes(right); @@ -21,7 +23,12 @@ export class StringOperator extends Operator { case StringOperation.endsWith: return left.endsWith(right); case StringOperation.matchesRegex: + if (!right) throw new Error('Regex pattern must be provided.'); return new RegExp(right).test(left); + case StringOperation.equalsIgnoreCase: + return left.toLowerCase() === right.toLowerCase(); + case StringOperation.trimEquals: + return left.trim() === right.trim(); default: throw new Error(`Unsupported string operator: ${this.operator}`); } diff --git a/packages/core/test/specs/data_sources/model/conditional_variables/DataCondition.ts b/packages/core/test/specs/data_sources/model/conditional_variables/DataCondition.ts index f756313089..31c350adeb 100644 --- a/packages/core/test/specs/data_sources/model/conditional_variables/DataCondition.ts +++ b/packages/core/test/specs/data_sources/model/conditional_variables/DataCondition.ts @@ -1,8 +1,8 @@ import { DataSourceManager } from '../../../../../src'; import { DataCondition, - Expression, - LogicGroup, + ExpressionDefinition, + LogicGroupDefinition, } from '../../../../../src/data_sources/model/conditional_variables/DataCondition'; import { GenericOperation } from '../../../../../src/data_sources/model/conditional_variables/operators/GenericOperator'; import { LogicalOperation } from '../../../../../src/data_sources/model/conditional_variables/operators/LogicalOperator'; @@ -52,14 +52,14 @@ describe('DataCondition', () => { describe('Operator Tests', () => { test('should evaluate using GenericOperation operators', () => { - const condition: Expression = { left: 5, operator: GenericOperation.equals, right: 5 }; + const condition: ExpressionDefinition = { left: 5, operator: GenericOperation.equals, right: 5 }; const dataCondition = new DataCondition(condition, 'Equal', 'Not Equal', { em }); expect(dataCondition.getDataValue()).toBe('Equal'); }); test('equals (false)', () => { - const condition: Expression = { + const condition: ExpressionDefinition = { left: 'hello', operator: GenericOperation.equals, right: 'world', @@ -69,21 +69,21 @@ describe('DataCondition', () => { }); test('should evaluate using StringOperation operators', () => { - const condition: Expression = { left: 'apple', operator: StringOperation.contains, right: 'app' }; + const condition: ExpressionDefinition = { left: 'apple', operator: StringOperation.contains, right: 'app' }; const dataCondition = new DataCondition(condition, 'Contains', "Doesn't contain", { em }); expect(dataCondition.getDataValue()).toBe('Contains'); }); test('should evaluate using NumberOperation operators', () => { - const condition: Expression = { left: 10, operator: NumberOperation.lessThan, right: 15 }; + const condition: ExpressionDefinition = { left: 10, operator: NumberOperation.lessThan, right: 15 }; const dataCondition = new DataCondition(condition, 'Valid', 'Invalid', { em }); expect(dataCondition.getDataValue()).toBe('Valid'); }); test('should evaluate using LogicalOperation operators', () => { - const logicGroup: LogicGroup = { + const logicGroup: LogicGroupDefinition = { logicalOperator: LogicalOperation.and, statements: [ { left: true, operator: GenericOperation.equals, right: true }, @@ -103,7 +103,7 @@ describe('DataCondition', () => { }); test('should evaluate complex nested conditions', () => { - const nestedLogicGroup: LogicGroup = { + const nestedLogicGroup: LogicGroupDefinition = { logicalOperator: LogicalOperation.or, statements: [ { @@ -124,7 +124,7 @@ describe('DataCondition', () => { describe('LogicalGroup Tests', () => { test('should correctly handle AND logical operator', () => { - const logicGroup: LogicGroup = { + const logicGroup: LogicGroupDefinition = { logicalOperator: LogicalOperation.and, statements: [ { left: true, operator: GenericOperation.equals, right: true }, @@ -137,7 +137,7 @@ describe('DataCondition', () => { }); test('should correctly handle OR logical operator', () => { - const logicGroup: LogicGroup = { + const logicGroup: LogicGroupDefinition = { logicalOperator: LogicalOperation.or, statements: [ { left: true, operator: GenericOperation.equals, right: false }, @@ -150,7 +150,7 @@ describe('DataCondition', () => { }); test('should correctly handle XOR logical operator', () => { - const logicGroup: LogicGroup = { + const logicGroup: LogicGroupDefinition = { logicalOperator: LogicalOperation.xor, statements: [ { left: true, operator: GenericOperation.equals, right: true }, @@ -164,7 +164,7 @@ describe('DataCondition', () => { }); test('should handle nested logical groups', () => { - const logicGroup: LogicGroup = { + const logicGroup: LogicGroupDefinition = { logicalOperator: LogicalOperation.and, statements: [ { left: true, operator: GenericOperation.equals, right: true }, @@ -183,7 +183,7 @@ describe('DataCondition', () => { }); test('should handle groups with false conditions', () => { - const logicGroup: LogicGroup = { + const logicGroup: LogicGroupDefinition = { logicalOperator: LogicalOperation.and, statements: [ { left: true, operator: GenericOperation.equals, right: true }, @@ -199,7 +199,7 @@ describe('DataCondition', () => { describe('Conditions with dataVariables', () => { test('should return "Yes" when dataVariable matches expected value', () => { - const condition: Expression = { + const condition: ExpressionDefinition = { left: { type: DataVariableType, path: 'USER_STATUS_SOURCE.USER_1.status' }, operator: GenericOperation.equals, right: 'active', @@ -210,7 +210,7 @@ describe('DataCondition', () => { }); test('should return "No" when dataVariable does not match expected value', () => { - const condition: Expression = { + const condition: ExpressionDefinition = { left: { type: DataVariableType, path: 'USER_STATUS_SOURCE.USER_1.status' }, operator: GenericOperation.equals, right: 'inactive', @@ -222,7 +222,7 @@ describe('DataCondition', () => { // TODO: unskip after adding UndefinedOperator test.skip('should handle missing data variable gracefully', () => { - const condition: Expression = { + const condition: ExpressionDefinition = { left: { type: DataVariableType, path: 'USER_STATUS_SOURCE.not_a_user.status' }, operator: GenericOperation.isDefined, right: undefined, @@ -233,7 +233,7 @@ describe('DataCondition', () => { }); test('should correctly compare numeric values from dataVariables', () => { - const condition: Expression = { + const condition: ExpressionDefinition = { left: { type: DataVariableType, path: 'USER_STATUS_SOURCE.USER_1.age' }, operator: NumberOperation.greaterThan, right: 24, @@ -249,7 +249,7 @@ describe('DataCondition', () => { }; dsm.add(dataSource2); - const logicGroup: LogicGroup = { + const logicGroup: LogicGroupDefinition = { logicalOperator: LogicalOperation.and, statements: [ { @@ -270,7 +270,7 @@ describe('DataCondition', () => { }); test('should handle nested logical conditions with data variables', () => { - const logicGroup: LogicGroup = { + const logicGroup: LogicGroupDefinition = { logicalOperator: LogicalOperation.or, statements: [ { diff --git a/packages/core/test/specs/data_sources/model/conditional_variables/operators/StringOperator.ts b/packages/core/test/specs/data_sources/model/conditional_variables/operators/StringOperator.ts index 6e291cf942..6767aaeeb6 100644 --- a/packages/core/test/specs/data_sources/model/conditional_variables/operators/StringOperator.ts +++ b/packages/core/test/specs/data_sources/model/conditional_variables/operators/StringOperator.ts @@ -52,6 +52,43 @@ describe('StringOperator', () => { }); }); + describe('Operator: equalsIgnoreCase', () => { + test('should return true when left equals right ignoring case', () => { + const operator = new StringOperator(StringOperation.equalsIgnoreCase); + expect(operator.evaluate('Hello World', 'hello world')).toBe(true); + }); + + test('should return false when left does not equal right ignoring case', () => { + const operator = new StringOperator(StringOperation.equalsIgnoreCase); + expect(operator.evaluate('Hello World', 'hello there')).toBe(false); + }); + + test('should handle empty strings correctly', () => { + const operator = new StringOperator(StringOperation.equalsIgnoreCase); + expect(operator.evaluate('', '')).toBe(true); + expect(operator.evaluate('Hello', '')).toBe(false); + expect(operator.evaluate('', 'Hello')).toBe(false); + }); + }); + + describe('Operator: trimEquals', () => { + test('should return true when left equals right after trimming', () => { + const operator = new StringOperator(StringOperation.trimEquals); + expect(operator.evaluate(' Hello World ', 'Hello World')).toBe(true); + }); + + test('should return false when left does not equal right after trimming', () => { + const operator = new StringOperator(StringOperation.trimEquals); + expect(operator.evaluate(' Hello World ', 'Hello there')).toBe(false); + }); + + test('should handle cases with only whitespace', () => { + const operator = new StringOperator(StringOperation.trimEquals); + expect(operator.evaluate(' ', '')).toBe(true); // Both should trim to empty + expect(operator.evaluate(' ', 'non-empty')).toBe(false); + }); + }); + describe('Edge Case Tests', () => { test('should return false for contains with empty right string', () => { const operator = new StringOperator(StringOperation.contains);