From 853760484c790fa2e05b0b9b7da14109207d5695 Mon Sep 17 00:00:00 2001
From: Julien Nahum <julien@nahum.net>
Date: Tue, 11 Feb 2025 15:05:27 +0100
Subject: [PATCH] Enhance form logic property resolver and validation UI

- Refactor `shouldBeRequired()` method in FormLogicPropertyResolver for clearer logic and better handling of conditional requirements
- Add comprehensive test cases for dynamic field requirement logic
- Update CustomFieldValidation component with improved UI and guidance for validation setup
- Improve error message and validation rule description in form validation interface
---
 .../Forms/FormLogicPropertyResolver.php       | 26 +++---
 .../Forms/FormLogicPropertyResolverTest.php   | 87 +++++++++++++++++++
 .../components/CustomFieldValidation.vue      | 60 ++++++++-----
 3 files changed, 142 insertions(+), 31 deletions(-)

diff --git a/api/app/Service/Forms/FormLogicPropertyResolver.php b/api/app/Service/Forms/FormLogicPropertyResolver.php
index 9319072c..88404ec7 100644
--- a/api/app/Service/Forms/FormLogicPropertyResolver.php
+++ b/api/app/Service/Forms/FormLogicPropertyResolver.php
@@ -29,22 +29,28 @@ class FormLogicPropertyResolver
 
     public function shouldBeRequired(): bool
     {
-        if (! isset($this->property['required'])) {
-            return false;
-        }
+        // Default required to false if not set
+        $isRequired = $this->property['required'] ?? false;
 
         if (! $this->logic) {
-            return $this->property['required'];
+            return $isRequired;
         }
 
         $conditionsMet = FormLogicConditionChecker::conditionsMet($this->logic['conditions'], $this->formData);
-        if ($conditionsMet && $this->property['required'] && count($this->logic['actions']) > 0 && (in_array('make-it-optional', $this->logic['actions']) || in_array('hide-block', $this->logic['actions']))) {
-            return false;
-        } elseif ($conditionsMet && ! $this->property['required'] && count($this->logic['actions']) > 0 && in_array('require-answer', $this->logic['actions'])) {
-            return true;
-        } else {
-            return $this->property['required'];
+
+        // If conditions are met and we have actions
+        if ($conditionsMet && !empty($this->logic['actions'])) {
+            // If field is required but should be made optional
+            if ($isRequired && (in_array('make-it-optional', $this->logic['actions']) || in_array('hide-block', $this->logic['actions']))) {
+                return false;
+            }
+            // If field is not required but should be required
+            if (!$isRequired && in_array('require-answer', $this->logic['actions'])) {
+                return true;
+            }
         }
+
+        return $isRequired;
     }
 
     public function shouldBeHidden(): bool
diff --git a/api/tests/Unit/Service/Forms/FormLogicPropertyResolverTest.php b/api/tests/Unit/Service/Forms/FormLogicPropertyResolverTest.php
index 5d7cdd7d..985a6408 100644
--- a/api/tests/Unit/Service/Forms/FormLogicPropertyResolverTest.php
+++ b/api/tests/Unit/Service/Forms/FormLogicPropertyResolverTest.php
@@ -107,4 +107,91 @@ it('can validate form logic property resolver', function ($property, $formData,
         ['93ea3198-353f-440b-8dc9-2ac9a7bee124' => [], '93ea3198-353f-440b-8dc9-2ac9a7bee222' => ['abc']],
         false,
     ],
+    [
+        [
+            'id' => 'text_field',
+            'name' => 'Required if checked',
+            'type' => 'text',
+            'hidden' => false,
+            'logic' => [
+                'conditions' => [
+                    'operatorIdentifier' => 'and',
+                    'children' => [
+                        [
+                            'identifier' => 'checkbox',
+                            'value' => [
+                                'operator' => 'is_checked',
+                                'property_meta' => [
+                                    'id' => 'checkbox_field',
+                                    'type' => 'checkbox'
+                                ],
+                                'value' => true
+                            ]
+                        ]
+                    ]
+                ],
+                'actions' => ['require-answer']
+            ]
+        ],
+        ['checkbox_field' => true],
+        true
+    ],
+    [
+        [
+            'id' => 'text_field',
+            'name' => 'Required if checked',
+            'type' => 'text',
+            'hidden' => false,
+            'logic' => [
+                'conditions' => [
+                    'operatorIdentifier' => 'and',
+                    'children' => [
+                        [
+                            'identifier' => 'checkbox',
+                            'value' => [
+                                'operator' => 'is_checked',
+                                'property_meta' => [
+                                    'id' => 'checkbox_field',
+                                    'type' => 'checkbox'
+                                ],
+                                'value' => true
+                            ]
+                        ]
+                    ]
+                ],
+                'actions' => ['require-answer']
+            ]
+        ],
+        ['checkbox_field' => false],
+        false
+    ],
+    [
+        [
+            'id' => 'text_field',
+            'name' => 'Required if checked',
+            'type' => 'text',
+            'hidden' => false,
+            'logic' => [
+                'conditions' => [
+                    'operatorIdentifier' => 'and',
+                    'children' => [
+                        [
+                            'identifier' => 'checkbox',
+                            'value' => [
+                                'operator' => 'is_checked',
+                                'property_meta' => [
+                                    'id' => 'checkbox_field',
+                                    'type' => 'checkbox'
+                                ],
+                                'value' => true
+                            ]
+                        ]
+                    ]
+                ],
+                'actions' => ['require-answer']
+            ]
+        ],
+        ['checkbox_field' => null],
+        false
+    ]
 ]);
diff --git a/client/components/open/forms/components/CustomFieldValidation.vue b/client/components/open/forms/components/CustomFieldValidation.vue
index 4d70d5c7..82c01789 100644
--- a/client/components/open/forms/components/CustomFieldValidation.vue
+++ b/client/components/open/forms/components/CustomFieldValidation.vue
@@ -1,33 +1,51 @@
 <template>
   <div class="py-2 px-4">
-    <p class="text-gray-500 text-xs mb-3">
-      Add some custom validation. Save your form before testing.
-    </p>
-    
-    <div class="py-2">
-      <p class="font-semibold text-sm text-gray-700">
-        Validation criteria for field acceptance
-      </p>
-      <condition-editor
-        ref="filter-editor"
-        v-model="validation.conditions"
-        class="mt-1 border-t border rounded-md mb-3"
-        :form="form"
-      />
-      <text-input
-        name="error_message"
-        class=""
-        :form="field.validation"
-        label="Error message"
-        help="Displayed when the validation fails"
+    <div class="space-y-4">
+      <!-- Step 1: Validation Rules -->
+      <div>
+        <h3 class="font-medium text-gray-900 text-sm mb-1">
+          Step 1: Set Validation Rules
+        </h3>
+        <p class="text-gray-500 text-xs mb-3">
+          Define <span class="font-semibold">conditions that must be met</span> for this field to be valid. If these conditions are not met, the field will be marked as invalid.
+        </p>
+        <condition-editor
+          ref="filter-editor"
+          v-model="validation.conditions"
+          class="mt-1 border-t border rounded-md"
+          :form="form"
+        />
+      </div>
+
+      <!-- Step 2: Error Message -->
+      <div>
+        <h3 class="font-medium text-gray-900 text-sm mb-1">
+          Step 2: Set Error Message
+        </h3>
+        <p class="text-gray-500 text-xs mb-2">
+          Enter the message that will be <span class="font-semibold">shown to users when the validation rules above are not met</span>.
+        </p>
+        <text-input
+          name="error_message"
+          :form="field.validation"
+          label="Error message"
+        />
+      </div>
+
+      <UAlert
+        icon="i-heroicons-information-circle"
+        color="yellow"
+        variant="subtle"
+        size="sm"
+        description="Remember to save your form to apply these validation rules."
       />
     </div>
   </div>
 </template>
 
 <script>
-import ConditionEditor from "./form-logic-components/ConditionEditor.client.vue"
 import { default as _has } from "lodash/has"
+import ConditionEditor from "./form-logic-components/ConditionEditor.client.vue"
 export default {
     name: 'FormValidation',
     components: {ConditionEditor},