From 195087fb8f7beb431e5c6ef15f3f43df9ecfc63f Mon Sep 17 00:00:00 2001 From: NJ-2020 Date: Tue, 11 Nov 2025 14:13:06 +0700 Subject: [PATCH 1/3] fix variable names, test name & improvement --- src/libs/Formula.ts | 26 ++++++++++++-------------- tests/unit/FormulaTest.ts | 7 +------ 2 files changed, 13 insertions(+), 20 deletions(-) diff --git a/src/libs/Formula.ts b/src/libs/Formula.ts index b64fd91c7858..e003bfae956e 100644 --- a/src/libs/Formula.ts +++ b/src/libs/Formula.ts @@ -199,44 +199,43 @@ function parsePart(definition: string): FormulaPart { * Check if the report field formula value is containing circular references, e.g example: A -> A, A->B->A, A->B->C->A, etc */ function hasCircularReferences(fieldValue: string, fieldName: string, fieldList?: FieldList): boolean { - const formulaValues = extract(fieldValue); - if (formulaValues.length === 0 || isEmptyObject(fieldList)) { + const formulaParts = extract(fieldValue); + if (formulaParts.length === 0 || isEmptyObject(fieldList)) { return false; } - const visitedLists = new Set(); + const visitedFields = new Set(); const fieldsByName = new Map(Object.values(fieldList).map((field) => [field.name, field])); // Helper function to check if a field has circular references const hasCircularReferencesRecursive = (currentFieldValue: string, currentFieldName: string): boolean => { // If we've already visited this field in the current path, return true - if (visitedLists.has(currentFieldName)) { + if (visitedFields.has(currentFieldName)) { return true; } // Add current field to the visited lists - visitedLists.add(currentFieldName); + visitedFields.add(currentFieldName); // Extract all formula values from the current field - const currentFormulaValues = extract(currentFieldValue); + const currentFormulaParts = extract(currentFieldValue); - for (const formula of currentFormulaValues) { - const part = parsePart(formula); + for (const part of currentFormulaParts) { + const partDefinition = parsePart(part); // Only check field references (skip report, user, or freetext) - if (part.type !== FORMULA_PART_TYPES.FIELD) { + if (partDefinition.type !== FORMULA_PART_TYPES.FIELD) { continue; } // Get the referenced field name (first element in fieldPath) - const referencedFieldName = part.fieldPath.at(0)?.trim(); + const referencedFieldName = partDefinition.fieldPath.at(0)?.trim(); if (!referencedFieldName) { continue; } // Check if this reference creates a cycle - if (referencedFieldName === fieldName || visitedLists.has(referencedFieldName)) { - visitedLists.delete(currentFieldName); + if (referencedFieldName === fieldName || visitedFields.has(referencedFieldName)) { return true; } @@ -245,14 +244,13 @@ function hasCircularReferences(fieldValue: string, fieldName: string, fieldList? if (referencedField?.defaultValue) { // Recursively check the referenced field if (hasCircularReferencesRecursive(referencedField.defaultValue, referencedFieldName)) { - visitedLists.delete(currentFieldName); return true; } } } // Remove current field from visited lists - visitedLists.delete(currentFieldName); + visitedFields.delete(currentFieldName); return false; }; diff --git a/tests/unit/FormulaTest.ts b/tests/unit/FormulaTest.ts index 97930088a033..2efe45f253eb 100644 --- a/tests/unit/FormulaTest.ts +++ b/tests/unit/FormulaTest.ts @@ -966,7 +966,6 @@ describe('CustomFormula', () => { test3: {name: 'test-c', defaultValue: '{field:test-b}'}, test4: {name: 'test-d', defaultValue: ''}, test6: {name: 'test-f', defaultValue: '{field:test-d}'}, - test5: {name: 'test-e', defaultValue: '{field:test-f}'}, }; // Then make sure the circular references work as expected @@ -986,14 +985,10 @@ describe('CustomFormula', () => { expect(hasCircularReferences('{field:test-c}', 'test-example', fieldList)).toBe(true); }); - test('should allow non-circular reference', () => { + test('should allow when there is no circular references', () => { expect(hasCircularReferences('{field:test-o}', 'test-example', fieldList)).toBe(false); }); - test('should return false when no references', () => { - expect(hasCircularReferences('{field:test-e}', 'test-example', fieldList)).toBe(false); - }); - test('should return false when there is no formula field', () => { expect(hasCircularReferences('hi test', 'test-example', fieldList)).toBe(false); }); From c378c754d588c769c503cef26ba0b70e15ff7ab3 Mon Sep 17 00:00:00 2001 From: NJ-2020 Date: Wed, 12 Nov 2025 09:34:12 +0700 Subject: [PATCH 2/3] fix: small improvement --- src/libs/Formula.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libs/Formula.ts b/src/libs/Formula.ts index e003bfae956e..874ae9cbf0d6 100644 --- a/src/libs/Formula.ts +++ b/src/libs/Formula.ts @@ -217,19 +217,19 @@ function hasCircularReferences(fieldValue: string, fieldName: string, fieldList? // Add current field to the visited lists visitedFields.add(currentFieldName); - // Extract all formula values from the current field + // Extract all formula parts from the current field const currentFormulaParts = extract(currentFieldValue); - for (const part of currentFormulaParts) { - const partDefinition = parsePart(part); + for (const currentFormulaPartDefinitions of currentFormulaParts) { + const part = parsePart(currentFormulaPartDefinitions); // Only check field references (skip report, user, or freetext) - if (partDefinition.type !== FORMULA_PART_TYPES.FIELD) { + if (part.type !== FORMULA_PART_TYPES.FIELD) { continue; } // Get the referenced field name (first element in fieldPath) - const referencedFieldName = partDefinition.fieldPath.at(0)?.trim(); + const referencedFieldName = part.fieldPath.at(0)?.trim(); if (!referencedFieldName) { continue; } From f50f132481fa3173a008785b155628c7cc47d389 Mon Sep 17 00:00:00 2001 From: NJ-2020 Date: Thu, 13 Nov 2025 16:50:07 +0700 Subject: [PATCH 3/3] fix spelling name --- src/libs/Formula.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/libs/Formula.ts b/src/libs/Formula.ts index 874ae9cbf0d6..bd399df5f54e 100644 --- a/src/libs/Formula.ts +++ b/src/libs/Formula.ts @@ -199,8 +199,8 @@ function parsePart(definition: string): FormulaPart { * Check if the report field formula value is containing circular references, e.g example: A -> A, A->B->A, A->B->C->A, etc */ function hasCircularReferences(fieldValue: string, fieldName: string, fieldList?: FieldList): boolean { - const formulaParts = extract(fieldValue); - if (formulaParts.length === 0 || isEmptyObject(fieldList)) { + const formulaPartDefinitions = extract(fieldValue); + if (formulaPartDefinitions.length === 0 || isEmptyObject(fieldList)) { return false; } @@ -217,11 +217,11 @@ function hasCircularReferences(fieldValue: string, fieldName: string, fieldList? // Add current field to the visited lists visitedFields.add(currentFieldName); - // Extract all formula parts from the current field - const currentFormulaParts = extract(currentFieldValue); + // Extract all formula part definitions + const currentFormulaPartDefinitions = extract(currentFieldValue); - for (const currentFormulaPartDefinitions of currentFormulaParts) { - const part = parsePart(currentFormulaPartDefinitions); + for (const formulaPartDefinition of currentFormulaPartDefinitions) { + const part = parsePart(formulaPartDefinition); // Only check field references (skip report, user, or freetext) if (part.type !== FORMULA_PART_TYPES.FIELD) {