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
2025-02-19 15:11:27 +01:00 · 2025-02-19 15:11:27 +01:00 · 28248259be
parent efd31133cc
commit 28248259be
7 changed files with 121 additions and 52 deletions
--- 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';
            }
        }
    }
--- 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');
+});
--- 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)
--- 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')"
  >
    <div class="-mx-5">
-      <h2 class="text-red-600 text-2xl font-bold mb-4 px-4">
-        Error saving your form
+      <h2 class="text-red-600 text-2xl font-medium mb-4 px-4">
+        We couldn't save your form
      </h2>

      <div
--- a/client/components/open/forms/fields/components/FieldOptions.vue
+++ b/client/components/open/forms/fields/components/FieldOptions.vue
@ -833,7 +833,11 @@ export default {

      // Apply type-specific defaults from blocks_types.json if available
      if (this.field.type in blocksTypes && blocksTypes[this.field.type]?.default_values) {
-        Object.assign(this.field, blocksTypes[this.field.type].default_values)
+        Object.keys(blocksTypes[this.field.type].default_values).forEach(key => {
+          if (!_has(this.field, key)) {
+            this.field[key] = blocksTypes[this.field.type].default_values[key]
+          }
+        })
      }

      // Apply additional defaults from defaultFieldValues if needed
--- 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,
--- 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
+