From cf4688d75ef5da2986676c10aab1ce1a3222900d Mon Sep 17 00:00:00 2001 From: Julien Nahum Date: Mon, 3 Mar 2025 19:03:09 +0800 Subject: [PATCH] Improve numeric condition handling in form logic checker - Add robust numeric comparison for zero and negative numbers - Implement safe numeric parsing and validation - Enhance condition checking for numeric fields across PHP and JavaScript - Add comprehensive test cases for edge case numeric comparisons - Fix 'is_not_empty' operator logic in JavaScript condition checker --- .../Forms/FormLogicConditionChecker.php | 37 ++++++++- .../Forms/FormLogicConditionCheckerTest.php | 76 +++++++++++++++++++ client/lib/forms/FormLogicConditionChecker.js | 48 +++++++----- 3 files changed, 136 insertions(+), 25 deletions(-) diff --git a/api/app/Service/Forms/FormLogicConditionChecker.php b/api/app/Service/Forms/FormLogicConditionChecker.php index b5a50a06..6dc26147 100644 --- a/api/app/Service/Forms/FormLogicConditionChecker.php +++ b/api/app/Service/Forms/FormLogicConditionChecker.php @@ -81,6 +81,15 @@ class FormLogicConditionChecker private function checkEquals($condition, $fieldValue): bool { + // For numeric values, convert to numbers before comparison + if ( + $this->areValidNumbers($condition, $fieldValue) && + is_numeric($condition['value']) && + is_numeric($fieldValue) + ) { + return (float) $condition['value'] === (float) $fieldValue; + } + return $condition['value'] === $fieldValue; } @@ -152,24 +161,44 @@ class FormLogicConditionChecker return $fieldValue == '' || $fieldValue == null || !$fieldValue; } + /** + * Helper function to check if values are valid for numeric comparison + */ + private function areValidNumbers($condition, $fieldValue): bool + { + return isset($condition['value']) && $fieldValue !== null && $fieldValue !== ''; + } + private function checkGreaterThan($condition, $fieldValue): bool { - return $condition['value'] && $fieldValue && (float) $fieldValue > (float) $condition['value']; + if (!$this->areValidNumbers($condition, $fieldValue)) { + return false; + } + return (float) $fieldValue > (float) $condition['value']; } private function checkGreaterThanEqual($condition, $fieldValue): bool { - return $condition['value'] && $fieldValue && (float) $fieldValue >= (float) $condition['value']; + if (!$this->areValidNumbers($condition, $fieldValue)) { + return false; + } + return (float) $fieldValue >= (float) $condition['value']; } private function checkLessThan($condition, $fieldValue): bool { - return $condition['value'] && $fieldValue && (float) $fieldValue < (float) $condition['value']; + if (!$this->areValidNumbers($condition, $fieldValue)) { + return false; + } + return (float) $fieldValue < (float) $condition['value']; } private function checkLessThanEqual($condition, $fieldValue): bool { - return $condition['value'] && $fieldValue && (float) $fieldValue <= (float) $condition['value']; + if (!$this->areValidNumbers($condition, $fieldValue)) { + return false; + } + return (float) $fieldValue <= (float) $condition['value']; } private function checkBefore($condition, $fieldValue): bool diff --git a/api/tests/Unit/Service/Forms/FormLogicConditionCheckerTest.php b/api/tests/Unit/Service/Forms/FormLogicConditionCheckerTest.php index e47ad5f5..cd6fef3c 100644 --- a/api/tests/Unit/Service/Forms/FormLogicConditionCheckerTest.php +++ b/api/tests/Unit/Service/Forms/FormLogicConditionCheckerTest.php @@ -150,6 +150,82 @@ describe('FormLogicConditionChecker', function () { expect(FormLogicConditionChecker::conditionsMet($condition, $formData))->toBeTrue(); }); + it('handles zero values correctly', function () { + $condition = [ + 'value' => [ + 'property_meta' => [ + 'id' => 'number_field', + 'type' => 'number' + ], + 'operator' => 'equals', + 'value' => 0 + ] + ]; + + // Test zero equality + $formData = ['number_field' => 0]; + expect(FormLogicConditionChecker::conditionsMet($condition, $formData))->toBeTrue(); + + $formData = ['number_field' => 1]; + expect(FormLogicConditionChecker::conditionsMet($condition, $formData))->toBeFalse(); + + // Test less than with zero + $condition['value']['operator'] = 'less_than'; + $condition['value']['value'] = 0; + $formData = ['number_field' => -1]; + expect(FormLogicConditionChecker::conditionsMet($condition, $formData))->toBeTrue(); + + $formData = ['number_field' => 0]; + expect(FormLogicConditionChecker::conditionsMet($condition, $formData))->toBeFalse(); + + // Test greater than with zero + $condition['value']['operator'] = 'greater_than'; + $condition['value']['value'] = 0; + $formData = ['number_field' => 1]; + expect(FormLogicConditionChecker::conditionsMet($condition, $formData))->toBeTrue(); + + $formData = ['number_field' => 0]; + expect(FormLogicConditionChecker::conditionsMet($condition, $formData))->toBeFalse(); + }); + + it('handles negative numbers correctly', function () { + $condition = [ + 'value' => [ + 'property_meta' => [ + 'id' => 'number_field', + 'type' => 'number' + ], + 'operator' => 'equals', + 'value' => -5 + ] + ]; + + // Test negative number equality + $formData = ['number_field' => -5]; + expect(FormLogicConditionChecker::conditionsMet($condition, $formData))->toBeTrue(); + + // Test less than with negative numbers + $condition['value']['operator'] = 'less_than'; + $condition['value']['value'] = -5; + $formData = ['number_field' => -10]; + expect(FormLogicConditionChecker::conditionsMet($condition, $formData))->toBeTrue(); + + $formData = ['number_field' => -5]; + expect(FormLogicConditionChecker::conditionsMet($condition, $formData))->toBeFalse(); + + $formData = ['number_field' => 0]; + expect(FormLogicConditionChecker::conditionsMet($condition, $formData))->toBeFalse(); + + // Test greater than with negative numbers + $condition['value']['operator'] = 'greater_than'; + $condition['value']['value'] = -10; + $formData = ['number_field' => -5]; + expect(FormLogicConditionChecker::conditionsMet($condition, $formData))->toBeTrue(); + + $formData = ['number_field' => -10]; + expect(FormLogicConditionChecker::conditionsMet($condition, $formData))->toBeFalse(); + }); + it('handles empty checks correctly', function () { $condition = [ 'value' => [ diff --git a/client/lib/forms/FormLogicConditionChecker.js b/client/lib/forms/FormLogicConditionChecker.js index cd683945..fb0374a0 100644 --- a/client/lib/forms/FormLogicConditionChecker.js +++ b/client/lib/forms/FormLogicConditionChecker.js @@ -67,7 +67,25 @@ function propertyConditionMet(propertyCondition, value) { return false } +// Helper function to safely parse numeric values +function safeParseFloat(value) { + if (value === undefined || value === null) return null + const parsed = parseFloat(value) + return isNaN(parsed) ? null : parsed +} + +// Helper function to check if values are valid for numeric comparison +function areValidNumbers(condition, fieldValue) { + const conditionValue = safeParseFloat(condition.value) + const parsedFieldValue = safeParseFloat(fieldValue) + return conditionValue !== null && parsedFieldValue !== null +} + function checkEquals(condition, fieldValue) { + // For numeric values, convert to numbers before comparison + if (areValidNumbers(condition, fieldValue)) { + return parseFloat(condition.value) === parseFloat(fieldValue) + } return condition.value === fieldValue } @@ -116,35 +134,23 @@ function checkIsEmpty(condition, fieldValue) { } function checkGreaterThan(condition, fieldValue) { - return ( - condition.value && - fieldValue && - parseFloat(fieldValue) > parseFloat(condition.value) - ) + if (!areValidNumbers(condition, fieldValue)) return false + return parseFloat(fieldValue) > parseFloat(condition.value) } function checkGreaterThanEqual(condition, fieldValue) { - return ( - condition.value && - fieldValue && - parseFloat(fieldValue) >= parseFloat(condition.value) - ) + if (!areValidNumbers(condition, fieldValue)) return false + return parseFloat(fieldValue) >= parseFloat(condition.value) } function checkLessThan(condition, fieldValue) { - return ( - condition.value && - fieldValue && - parseFloat(fieldValue) < parseFloat(condition.value) - ) + if (!areValidNumbers(condition, fieldValue)) return false + return parseFloat(fieldValue) < parseFloat(condition.value) } function checkLessThanEqual(condition, fieldValue) { - return ( - condition.value && - fieldValue && - parseFloat(fieldValue) <= parseFloat(condition.value) - ) + if (!areValidNumbers(condition, fieldValue)) return false + return parseFloat(fieldValue) <= parseFloat(condition.value) } function checkBefore(condition, fieldValue) { @@ -313,7 +319,7 @@ function numberConditionMet(propertyCondition, value) { case "is_empty": return checkIsEmpty(propertyCondition, value) case "is_not_empty": - return checkIsEmpty(propertyCondition, value) + return !checkIsEmpty(propertyCondition, value) case "content_length_equals": return checkLength(propertyCondition, value, "===") case "content_length_does_not_equal":