From 2656818c35efaf36bffb332b6429aaa0261d368d Mon Sep 17 00:00:00 2001 From: "bert.hausmans" Date: Sat, 25 Apr 2026 00:57:06 +0200 Subject: [PATCH] refactor(form-field): extract legacy conditional_logic shape normaliser MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three byte-identical copies of `normaliseLegacyGroupShape` lived in FormFieldService, StoreFormFieldRequest, and UpdateFormFieldRequest. WS-5d (form_fields.options) would have been the fourth copy. Hoist the helper to a single public static on FormFieldConditionalLogicService and have all three call sites delegate. Implementation: - `FormFieldConditionalLogicService::normaliseLegacyShape(array)` — pure recursive passthrough. Translates the ARCH §8 JSON group shape (`{"all": [...]}` / `{"any": [...]}`) into the service's internal `{"operator", "children"}` form. Does NOT validate; malformed shapes return as-is and surface downstream as `InvalidConditionalLogicSpecException` from `assertSpecsValid`. - Group operator catalogue sourced from `FormFieldConditionalLogicGroupOperator::values()` instead of an `['all', 'any']` literal — single source of truth for future operator additions. - All three call sites switched to the static method. The two FormRequests reach it via the existing `use` import; FormFieldService sits in the same namespace. Behaviour preserved exactly: - Existing FormFieldApiTest (cyclic logic rejection), FormFieldStrictConditionalLogicRequestTest (strict-validator rejection paths), and FormFieldConditionalLogicServiceTest (service-level paths) all green without modification. New unit tests pin the passthrough contract (8 tests): - Valid ALL / ANY translations - Recursive nested-group translation (depth 2) - Internal shape unchanged - Condition leaf passthrough - Unknown group key (`xor`) returned unchanged for downstream `assertSpecsValid` to reject - Empty array unchanged - Non-array children stripped silently Tests: 1150 → 1158 green (3110 → 3124 assertions). Co-Authored-By: Claude Opus 4.7 (1M context) --- ...FormFieldConditionalLogicGroupOperator.php | 12 ++ .../V1/FormBuilder/StoreFormFieldRequest.php | 42 +----- .../V1/FormBuilder/UpdateFormFieldRequest.php | 37 +---- .../FormFieldConditionalLogicService.php | 51 +++++++ .../Services/FormBuilder/FormFieldService.php | 39 +---- ...ormFieldConditionalLogicNormaliserTest.php | 139 ++++++++++++++++++ 6 files changed, 205 insertions(+), 115 deletions(-) create mode 100644 api/tests/Unit/Services/FormBuilder/FormFieldConditionalLogicNormaliserTest.php diff --git a/api/app/Enums/FormBuilder/FormFieldConditionalLogicGroupOperator.php b/api/app/Enums/FormBuilder/FormFieldConditionalLogicGroupOperator.php index 2f2a3c19..ad82f9d7 100644 --- a/api/app/Enums/FormBuilder/FormFieldConditionalLogicGroupOperator.php +++ b/api/app/Enums/FormBuilder/FormFieldConditionalLogicGroupOperator.php @@ -14,4 +14,16 @@ enum FormFieldConditionalLogicGroupOperator: string { case All = 'all'; case Any = 'any'; + + /** + * String values of every case — used by the legacy-shape normaliser + * to walk `{"all": [...]}` / `{"any": [...]}` JSON nodes without + * duplicating the operator catalogue at the call site. + * + * @return list + */ + public static function values(): array + { + return array_map(static fn (self $case): string => $case->value, self::cases()); + } } diff --git a/api/app/Http/Requests/Api/V1/FormBuilder/StoreFormFieldRequest.php b/api/app/Http/Requests/Api/V1/FormBuilder/StoreFormFieldRequest.php index 30801bc0..8069912e 100644 --- a/api/app/Http/Requests/Api/V1/FormBuilder/StoreFormFieldRequest.php +++ b/api/app/Http/Requests/Api/V1/FormBuilder/StoreFormFieldRequest.php @@ -99,7 +99,7 @@ final class StoreFormFieldRequest extends FormRequest $root = isset($logic['show_when']) && is_array($logic['show_when']) ? $logic['show_when'] : $logic; - $normalised = $this->normaliseLegacyGroupShape($root); + $normalised = FormFieldConditionalLogicService::normaliseLegacyShape($root); try { app(FormFieldConditionalLogicService::class)->assertSpecsValid($normalised); } catch (InvalidConditionalLogicSpecException $e) { @@ -108,44 +108,4 @@ final class StoreFormFieldRequest extends FormRequest }, ]; } - - /** - * Mirror of FormFieldService::normaliseLegacyGroupShape — translates - * the ARCH §8 JSON group shape (`{"all": [...]}` / `{"any": [...]}`) - * to the service's internal `{"operator", "children"}` form so the - * boundary validator can reuse the service's canonical assertion. - * - * @param array $node - * @return array - */ - private function normaliseLegacyGroupShape(array $node): array - { - if (isset($node['field_slug'])) { - return $node; - } - if (isset($node['operator'], $node['children']) && is_array($node['children'])) { - $children = []; - foreach ($node['children'] as $child) { - if (is_array($child)) { - $children[] = $this->normaliseLegacyGroupShape($child); - } - } - - return ['operator' => $node['operator'], 'children' => $children]; - } - foreach (['all', 'any'] as $candidate) { - if (isset($node[$candidate]) && is_array($node[$candidate])) { - $children = []; - foreach ($node[$candidate] as $child) { - if (is_array($child)) { - $children[] = $this->normaliseLegacyGroupShape($child); - } - } - - return ['operator' => $candidate, 'children' => $children]; - } - } - - return $node; - } } diff --git a/api/app/Http/Requests/Api/V1/FormBuilder/UpdateFormFieldRequest.php b/api/app/Http/Requests/Api/V1/FormBuilder/UpdateFormFieldRequest.php index 16f92dce..8eb2c565 100644 --- a/api/app/Http/Requests/Api/V1/FormBuilder/UpdateFormFieldRequest.php +++ b/api/app/Http/Requests/Api/V1/FormBuilder/UpdateFormFieldRequest.php @@ -102,7 +102,7 @@ final class UpdateFormFieldRequest extends FormRequest $root = isset($logic['show_when']) && is_array($logic['show_when']) ? $logic['show_when'] : $logic; - $normalised = $this->normaliseLegacyGroupShape($root); + $normalised = FormFieldConditionalLogicService::normaliseLegacyShape($root); try { app(FormFieldConditionalLogicService::class)->assertSpecsValid($normalised); } catch (InvalidConditionalLogicSpecException $e) { @@ -111,39 +111,4 @@ final class UpdateFormFieldRequest extends FormRequest }, ]; } - - /** - * @param array $node - * @return array - */ - private function normaliseLegacyGroupShape(array $node): array - { - if (isset($node['field_slug'])) { - return $node; - } - if (isset($node['operator'], $node['children']) && is_array($node['children'])) { - $children = []; - foreach ($node['children'] as $child) { - if (is_array($child)) { - $children[] = $this->normaliseLegacyGroupShape($child); - } - } - - return ['operator' => $node['operator'], 'children' => $children]; - } - foreach (['all', 'any'] as $candidate) { - if (isset($node[$candidate]) && is_array($node[$candidate])) { - $children = []; - foreach ($node[$candidate] as $child) { - if (is_array($child)) { - $children[] = $this->normaliseLegacyGroupShape($child); - } - } - - return ['operator' => $candidate, 'children' => $children]; - } - } - - return $node; - } } diff --git a/api/app/Services/FormBuilder/FormFieldConditionalLogicService.php b/api/app/Services/FormBuilder/FormFieldConditionalLogicService.php index 7dded8fd..ae2b1fc2 100644 --- a/api/app/Services/FormBuilder/FormFieldConditionalLogicService.php +++ b/api/app/Services/FormBuilder/FormFieldConditionalLogicService.php @@ -115,6 +115,57 @@ final class FormFieldConditionalLogicService $this->assertNodeValid($tree, isRoot: true); } + /** + * Translate the legacy ARCH §8 JSON group shape (`{"all": [...]}` / + * `{"any": [...]}`) into the service's internal tree form + * (`{"operator": "all"|"any", "children": [...]}`). Pure recursive + * passthrough — does NOT validate; malformed shapes are returned + * as-is and surface as `InvalidConditionalLogicSpecException` from + * the downstream `assertSpecsValid`. Static so both the service path + * (`FormFieldService::extractConditionalLogicTree`) and the boundary + * validators (Store/Update FormRequests' `after()` hooks) can call + * it without container resolution. + * + * Group operator catalogue is sourced from + * `FormFieldConditionalLogicGroupOperator::values()` so a future + * operator addition lands in one place. + * + * @param array $node + * @return array + */ + public static function normaliseLegacyShape(array $node): array + { + if (isset($node['field_slug'])) { + return $node; + } + + if (isset($node['operator'], $node['children']) && is_array($node['children'])) { + $children = []; + foreach ($node['children'] as $child) { + if (is_array($child)) { + $children[] = self::normaliseLegacyShape($child); + } + } + + return ['operator' => $node['operator'], 'children' => $children]; + } + + foreach (FormFieldConditionalLogicGroupOperator::values() as $candidate) { + if (isset($node[$candidate]) && is_array($node[$candidate])) { + $children = []; + foreach ($node[$candidate] as $child) { + if (is_array($child)) { + $children[] = self::normaliseLegacyShape($child); + } + } + + return ['operator' => $candidate, 'children' => $children]; + } + } + + return $node; + } + /** * Cross-field cycle detection (ARCH §8; contract preserved from the * pre-WS-5c `FormFieldService::assertNoConditionalCycle`). Builds a diff --git a/api/app/Services/FormBuilder/FormFieldService.php b/api/app/Services/FormBuilder/FormFieldService.php index 1bb13344..62b56c70 100644 --- a/api/app/Services/FormBuilder/FormFieldService.php +++ b/api/app/Services/FormBuilder/FormFieldService.php @@ -367,44 +367,7 @@ final class FormFieldService $rootGroup = $raw; } - return [$this->normaliseLegacyGroupShape($rootGroup), true]; - } - - /** - * @param array $node - * @return array - */ - private function normaliseLegacyGroupShape(array $node): array - { - if (isset($node['field_slug'])) { - return $node; - } - - if (isset($node['operator'], $node['children']) && is_array($node['children'])) { - $children = []; - foreach ($node['children'] as $child) { - if (is_array($child)) { - $children[] = $this->normaliseLegacyGroupShape($child); - } - } - - return ['operator' => $node['operator'], 'children' => $children]; - } - - foreach (['all', 'any'] as $candidate) { - if (isset($node[$candidate]) && is_array($node[$candidate])) { - $children = []; - foreach ($node[$candidate] as $child) { - if (is_array($child)) { - $children[] = $this->normaliseLegacyGroupShape($child); - } - } - - return ['operator' => $candidate, 'children' => $children]; - } - } - - return $node; + return [FormFieldConditionalLogicService::normaliseLegacyShape($rootGroup), true]; } /** diff --git a/api/tests/Unit/Services/FormBuilder/FormFieldConditionalLogicNormaliserTest.php b/api/tests/Unit/Services/FormBuilder/FormFieldConditionalLogicNormaliserTest.php new file mode 100644 index 00000000..6291d869 --- /dev/null +++ b/api/tests/Unit/Services/FormBuilder/FormFieldConditionalLogicNormaliserTest.php @@ -0,0 +1,139 @@ + [ + ['field_slug' => 'gate', 'operator' => 'equals', 'value' => 'yes'], + ], + ]); + + $this->assertSame([ + 'operator' => 'all', + 'children' => [ + ['field_slug' => 'gate', 'operator' => 'equals', 'value' => 'yes'], + ], + ], $result); + } + + public function test_translates_any_group_to_internal_form(): void + { + $result = FormFieldConditionalLogicService::normaliseLegacyShape([ + 'any' => [ + ['field_slug' => 'region', 'operator' => 'equals', 'value' => 'NL'], + ['field_slug' => 'region', 'operator' => 'equals', 'value' => 'BE'], + ], + ]); + + $this->assertSame('any', $result['operator']); + $this->assertCount(2, $result['children']); + } + + public function test_recursively_normalises_nested_groups(): void + { + $result = FormFieldConditionalLogicService::normaliseLegacyShape([ + 'all' => [ + ['field_slug' => 'gate', 'operator' => 'equals', 'value' => 'yes'], + [ + 'any' => [ + ['field_slug' => 'region', 'operator' => 'equals', 'value' => 'NL'], + ['field_slug' => 'region', 'operator' => 'equals', 'value' => 'BE'], + ], + ], + ], + ]); + + $this->assertSame('all', $result['operator']); + $this->assertCount(2, $result['children']); + + // First child: condition leaf — passthrough. + $this->assertSame( + ['field_slug' => 'gate', 'operator' => 'equals', 'value' => 'yes'], + $result['children'][0], + ); + + // Second child: nested ANY group — translated. + $this->assertSame('any', $result['children'][1]['operator']); + $this->assertCount(2, $result['children'][1]['children']); + } + + public function test_passes_through_internal_shape_unchanged(): void + { + $internal = [ + 'operator' => 'all', + 'children' => [ + ['field_slug' => 'gate', 'operator' => 'equals', 'value' => 'yes'], + ], + ]; + + $this->assertSame($internal, FormFieldConditionalLogicService::normaliseLegacyShape($internal)); + } + + public function test_passes_through_condition_leaf_unchanged(): void + { + // Conditions at root are technically invalid (a tree must start + // with a group); the normaliser returns the leaf as-is and the + // downstream `assertSpecsValid` produces the 422 error. + $leaf = ['field_slug' => 'gate', 'operator' => 'equals', 'value' => 'yes']; + + $this->assertSame($leaf, FormFieldConditionalLogicService::normaliseLegacyShape($leaf)); + } + + public function test_returns_unknown_group_key_as_passthrough_for_downstream_assert(): void + { + // `xor` is not in the FormFieldConditionalLogicGroupOperator enum. + // Normaliser returns the node unchanged; assertSpecsValid will + // raise `InvalidConditionalLogicSpecException` with "Unknown + // group operator ''" because the passthrough lacks `operator`. + $node = [ + 'xor' => [ + ['field_slug' => 'a', 'operator' => 'equals', 'value' => 1], + ], + ]; + + $this->assertSame($node, FormFieldConditionalLogicService::normaliseLegacyShape($node)); + } + + public function test_returns_empty_array_unchanged(): void + { + $this->assertSame([], FormFieldConditionalLogicService::normaliseLegacyShape([])); + } + + public function test_skips_non_array_children_silently(): void + { + // Defensive: malformed children that are scalars get stripped + // (existing behaviour — `if (is_array($child))` guard). Surfaces + // downstream as a "group requires at least one child" error if + // every child was scalar. + $result = FormFieldConditionalLogicService::normaliseLegacyShape([ + 'all' => [ + 'string-not-an-array', + ['field_slug' => 'gate', 'operator' => 'equals', 'value' => 'yes'], + 42, + ], + ]); + + $this->assertCount(1, $result['children']); + $this->assertSame( + ['field_slug' => 'gate', 'operator' => 'equals', 'value' => 'yes'], + $result['children'][0], + ); + } +}