From 28248259beb0694d7066325f9ba564151f0d63e6 Mon Sep 17 00:00:00 2001 From: Julien Nahum Date: Wed, 19 Feb 2025 15:11:27 +0100 Subject: [PATCH] Improve form property logic validation for checkbox conditions - Update FormPropertyLogicRule to handle operators without values - Add support for checkbox conditions like 'is_checked' and 'is_not_checked' - Refactor logic validation in both API and client-side implementations - Remove unnecessary console.log statements - Update error modal text for better user experience --- api/app/Rules/FormPropertyLogicRule.php | 55 +++++++++---------- .../Feature/Forms/FormPropertyLogicTest.php | 54 ++++++++++++++++++ .../open/forms/components/FormEditor.vue | 1 - .../form-components/FormErrorModal.vue | 4 +- .../forms/fields/components/FieldOptions.vue | 6 +- .../forms/validatePropertiesLogic.js | 2 + client/lib/forms/FormPropertyLogicRule.js | 51 ++++++++++------- 7 files changed, 121 insertions(+), 52 deletions(-) diff --git a/api/app/Rules/FormPropertyLogicRule.php b/api/app/Rules/FormPropertyLogicRule.php index eb7c280f..a3879123 100644 --- a/api/app/Rules/FormPropertyLogicRule.php +++ b/api/app/Rules/FormPropertyLogicRule.php @@ -42,73 +42,72 @@ class FormPropertyLogicRule implements DataAwareRule, ValidationRule if (!isset($condition['value'])) { $this->isConditionCorrect = false; $this->conditionErrors[] = 'missing condition body'; - return; } if (!isset($condition['value']['property_meta'])) { $this->isConditionCorrect = false; $this->conditionErrors[] = 'missing condition property'; - return; } if (!isset($condition['value']['property_meta']['type'])) { $this->isConditionCorrect = false; $this->conditionErrors[] = 'missing condition property type'; - return; } if (!isset($condition['value']['operator'])) { $this->isConditionCorrect = false; $this->conditionErrors[] = 'missing condition operator'; - - return; - } - - if (!isset($condition['value']['value'])) { - $this->isConditionCorrect = false; - $this->conditionErrors[] = 'missing condition value'; - return; } $typeField = $condition['value']['property_meta']['type']; $operator = $condition['value']['operator']; $this->operator = $operator; - $value = $condition['value']['value']; if (!isset(self::getConditionMapping()[$typeField])) { $this->isConditionCorrect = false; $this->conditionErrors[] = 'configuration not found for condition type'; - return; } if (!isset(self::getConditionMapping()[$typeField]['comparators'][$operator])) { $this->isConditionCorrect = false; $this->conditionErrors[] = 'configuration not found for condition operator'; - return; } - $type = self::getConditionMapping()[$typeField]['comparators'][$operator]['expected_type'] ?? null; + $comparatorDef = self::getConditionMapping()[$typeField]['comparators'][$operator]; + $needsValue = !empty((array)$comparatorDef); - if (is_array($type)) { - $foundCorrectType = false; - foreach ($type as $subtype) { - if ($this->valueHasCorrectType($subtype, $value)) { - $foundCorrectType = true; + if ($needsValue && !isset($condition['value']['value'])) { + $this->isConditionCorrect = false; + $this->conditionErrors[] = 'missing condition value'; + return; + } + + if ($needsValue) { + $type = $comparatorDef['expected_type'] ?? null; + $value = $condition['value']['value']; + + if (is_array($type)) { + $foundCorrectType = false; + foreach ($type as $subtype) { + if ($this->valueHasCorrectType($subtype, $value)) { + $foundCorrectType = true; + } + } + if (!$foundCorrectType) { + $this->isConditionCorrect = false; + $this->conditionErrors[] = 'wrong type of condition value'; + } + } else { + if (!$this->valueHasCorrectType($type, $value)) { + $this->isConditionCorrect = false; + $this->conditionErrors[] = 'wrong type of condition value'; } - } - if (!$foundCorrectType) { - $this->isConditionCorrect = false; - } - } else { - if (!$this->valueHasCorrectType($type, $value)) { - $this->isConditionCorrect = false; - $this->conditionErrors[] = 'wrong type of condition value'; } } } diff --git a/api/tests/Feature/Forms/FormPropertyLogicTest.php b/api/tests/Feature/Forms/FormPropertyLogicTest.php index 01ed1615..04f7755d 100644 --- a/api/tests/Feature/Forms/FormPropertyLogicTest.php +++ b/api/tests/Feature/Forms/FormPropertyLogicTest.php @@ -199,3 +199,57 @@ it('can validate form logic rules for conditions', function () { $this->assertFalse($validatorObj->passes()); expect($validatorObj->errors()->messages()['properties.0.logic'][0])->toBe('The logic conditions for Name are not complete. Error detail(s): missing operator'); }); + +it('can validate form logic rules for operators without values', function () { + $rules = [ + 'properties.*.logic' => ['array', 'nullable', new FormPropertyLogicRule()], + ]; + + // Test checkbox is_checked without value + $data = [ + 'properties' => [ + [ + 'id' => 'checkbox1', + 'name' => 'Checkbox Field', + 'type' => 'checkbox', + 'logic' => [ + 'conditions' => [ + 'operatorIdentifier' => 'and', + 'children' => [ + [ + 'identifier' => 'test-id', + 'value' => [ + 'operator' => 'is_checked', + 'property_meta' => [ + 'id' => 'test-id', + 'type' => 'checkbox' + ] + ] + ] + ] + ], + 'actions' => ['show-block'] + ] + ] + ] + ]; + $validatorObj = $this->app['validator']->make($data, $rules); + $this->assertTrue($validatorObj->passes()); + + // Test checkbox is_checked with value (should still pass for backward compatibility) + $data['properties'][0]['logic']['conditions']['children'][0]['value']['value'] = true; + $validatorObj = $this->app['validator']->make($data, $rules); + $this->assertTrue($validatorObj->passes()); + + // Test checkbox is_not_checked without value + $data['properties'][0]['logic']['conditions']['children'][0]['value']['operator'] = 'is_not_checked'; + unset($data['properties'][0]['logic']['conditions']['children'][0]['value']['value']); + $validatorObj = $this->app['validator']->make($data, $rules); + $this->assertTrue($validatorObj->passes()); + + // Test checkbox with operator that doesn't exist + $data['properties'][0]['logic']['conditions']['children'][0]['value']['operator'] = 'invalid_operator'; + $validatorObj = $this->app['validator']->make($data, $rules); + $this->assertFalse($validatorObj->passes()); + expect($validatorObj->errors()->messages()['properties.0.logic'][0])->toBe('The logic conditions for Checkbox Field are not complete. Error detail(s): configuration not found for condition operator'); +}); diff --git a/client/components/open/forms/components/FormEditor.vue b/client/components/open/forms/components/FormEditor.vue index 11273a10..7ad0e10b 100644 --- a/client/components/open/forms/components/FormEditor.vue +++ b/client/components/open/forms/components/FormEditor.vue @@ -236,7 +236,6 @@ export default { saveForm() { // Apply defaults to the form const defaultedData = setFormDefaults(this.form.data()) - console.log('defaultedData', defaultedData) this.form.fill(defaultedData) this.form.properties = validatePropertiesLogic(this.form.properties) diff --git a/client/components/open/forms/components/form-components/FormErrorModal.vue b/client/components/open/forms/components/form-components/FormErrorModal.vue index bbcca1c7..bba8700e 100644 --- a/client/components/open/forms/components/form-components/FormErrorModal.vue +++ b/client/components/open/forms/components/form-components/FormErrorModal.vue @@ -4,8 +4,8 @@ @close="$emit('close')" >
-

- Error saving your form +

+ We couldn't save your form

{ + if (!_has(this.field, key)) { + this.field[key] = blocksTypes[this.field.type].default_values[key] + } + }) } // Apply additional defaults from defaultFieldValues if needed diff --git a/client/composables/forms/validatePropertiesLogic.js b/client/composables/forms/validatePropertiesLogic.js index d9b6d625..fa08a39a 100644 --- a/client/composables/forms/validatePropertiesLogic.js +++ b/client/composables/forms/validatePropertiesLogic.js @@ -3,6 +3,8 @@ import FormPropertyLogicRule from "~/lib/forms/FormPropertyLogicRule.js" export const validatePropertiesLogic = (properties) => { properties.forEach((field) => { const isValid = new FormPropertyLogicRule(field).isValid() + console.log('field', field) + console.log('isValid', isValid, field.name) if (!isValid) { field.logic = { conditions: null, diff --git a/client/lib/forms/FormPropertyLogicRule.js b/client/lib/forms/FormPropertyLogicRule.js index 49df65d1..673dade6 100644 --- a/client/lib/forms/FormPropertyLogicRule.js +++ b/client/lib/forms/FormPropertyLogicRule.js @@ -22,6 +22,7 @@ class FormPropertyLogicRule { isValid() { if (this.logic && this.logic["conditions"]) { + console.log('logic', this.logic) this.checkConditions(this.logic["conditions"]) this.checkActions( this.logic && this.logic["actions"] ? this.logic["actions"] : null, @@ -62,8 +63,7 @@ class FormPropertyLogicRule { condition["value"] === undefined || condition["value"]["property_meta"] === undefined || condition["value"]["property_meta"]["type"] === undefined || - condition["value"]["operator"] === undefined || - condition["value"]["value"] === undefined + condition["value"]["operator"] === undefined ) { this.isConditionCorrect = false return @@ -71,8 +71,7 @@ class FormPropertyLogicRule { const typeField = condition["value"]["property_meta"]["type"] const operator = condition["value"]["operator"] - const value = condition["value"]["value"] - + if ( this.CONDITION_MAPPING[typeField] === undefined || this.CONDITION_MAPPING[typeField]["comparators"][operator] === undefined @@ -81,23 +80,34 @@ class FormPropertyLogicRule { return } - const type = - this.CONDITION_MAPPING[typeField]["comparators"][operator][ - "expected_type" - ] - if (Array.isArray(type)) { - let foundCorrectType = false - type.forEach((subtype) => { - if (this.valueHasCorrectType(subtype, value)) { - foundCorrectType = true + // Check if operator needs a value based on comparator definition + const comparatorDef = this.CONDITION_MAPPING[typeField]["comparators"][operator] + const needsValue = Object.keys(comparatorDef).length > 0 + + if (needsValue && condition["value"]["value"] === undefined) { + this.isConditionCorrect = false + return + } + + // Only check value type if comparator expects one + if (needsValue) { + const type = comparatorDef["expected_type"] + const value = condition["value"]["value"] + + if (Array.isArray(type)) { + let foundCorrectType = false + type.forEach((subtype) => { + if (this.valueHasCorrectType(subtype, value)) { + foundCorrectType = true + } + }) + if (!foundCorrectType) { + this.isConditionCorrect = false + } + } else { + if (!this.valueHasCorrectType(type, value)) { + this.isConditionCorrect = false } - }) - if (!foundCorrectType) { - this.isConditionCorrect = false - } - } else { - if (!this.valueHasCorrectType(type, value)) { - this.isConditionCorrect = false } } } @@ -151,3 +161,4 @@ class FormPropertyLogicRule { } export default FormPropertyLogicRule +