diff --git a/api/app/Http/Requests/Api/V1/FormBuilder/StoreFormFieldRequest.php b/api/app/Http/Requests/Api/V1/FormBuilder/StoreFormFieldRequest.php index 48286a84..30801bc0 100644 --- a/api/app/Http/Requests/Api/V1/FormBuilder/StoreFormFieldRequest.php +++ b/api/app/Http/Requests/Api/V1/FormBuilder/StoreFormFieldRequest.php @@ -7,8 +7,10 @@ namespace App\Http\Requests\Api\V1\FormBuilder; use App\Enums\FormBuilder\FormFieldDisplayWidth; use App\Enums\FormBuilder\FormFieldType; use App\Enums\FormBuilder\FormValueStorageHint; +use App\Exceptions\FormBuilder\InvalidConditionalLogicSpecException; use App\Exceptions\FormBuilder\UnknownValidationRuleTypeException; use App\Models\FormBuilder\FormSchema; +use App\Services\FormBuilder\FormFieldConditionalLogicService; use App\Services\FormBuilder\FormFieldValidationRuleService; use Illuminate\Contracts\Validation\Validator; use Illuminate\Foundation\Http\FormRequest; @@ -89,6 +91,61 @@ final class StoreFormFieldRequest extends FormRequest $validator->errors()->add('validation_rules', $e->getMessage()); } }, + function (Validator $validator): void { + $logic = $this->input('conditional_logic'); + if ($logic === null || $logic === [] || ! is_array($logic)) { + return; + } + $root = isset($logic['show_when']) && is_array($logic['show_when']) + ? $logic['show_when'] + : $logic; + $normalised = $this->normaliseLegacyGroupShape($root); + try { + app(FormFieldConditionalLogicService::class)->assertSpecsValid($normalised); + } catch (InvalidConditionalLogicSpecException $e) { + $validator->errors()->add('conditional_logic', $e->getMessage()); + } + }, ]; } + + /** + * 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 dbba642b..16f92dce 100644 --- a/api/app/Http/Requests/Api/V1/FormBuilder/UpdateFormFieldRequest.php +++ b/api/app/Http/Requests/Api/V1/FormBuilder/UpdateFormFieldRequest.php @@ -7,8 +7,10 @@ namespace App\Http\Requests\Api\V1\FormBuilder; use App\Enums\FormBuilder\FormFieldDisplayWidth; use App\Enums\FormBuilder\FormFieldType; use App\Enums\FormBuilder\FormValueStorageHint; +use App\Exceptions\FormBuilder\InvalidConditionalLogicSpecException; use App\Exceptions\FormBuilder\UnknownValidationRuleTypeException; use App\Models\FormBuilder\FormSchema; +use App\Services\FormBuilder\FormFieldConditionalLogicService; use App\Services\FormBuilder\FormFieldValidationRuleService; use Illuminate\Contracts\Validation\Validator; use Illuminate\Foundation\Http\FormRequest; @@ -89,6 +91,59 @@ final class UpdateFormFieldRequest extends FormRequest $validator->errors()->add('validation_rules', $e->getMessage()); } }, + function (Validator $validator): void { + if (! $this->has('conditional_logic')) { + return; + } + $logic = $this->input('conditional_logic'); + if ($logic === null || $logic === [] || ! is_array($logic)) { + return; + } + $root = isset($logic['show_when']) && is_array($logic['show_when']) + ? $logic['show_when'] + : $logic; + $normalised = $this->normaliseLegacyGroupShape($root); + try { + app(FormFieldConditionalLogicService::class)->assertSpecsValid($normalised); + } catch (InvalidConditionalLogicSpecException $e) { + $validator->errors()->add('conditional_logic', $e->getMessage()); + } + }, ]; } + + /** + * @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/Models/FormBuilder/FormField.php b/api/app/Models/FormBuilder/FormField.php index ee8cf880..eb2fd1c6 100644 --- a/api/app/Models/FormBuilder/FormField.php +++ b/api/app/Models/FormBuilder/FormField.php @@ -58,7 +58,6 @@ final class FormField extends Model 'is_unique', 'is_pii', 'display_width', - 'conditional_logic', 'role_restrictions', 'translations', 'value_storage_hint', @@ -69,7 +68,6 @@ final class FormField extends Model /** @var array */ protected $casts = [ 'options' => 'array', - 'conditional_logic' => 'array', 'role_restrictions' => 'array', 'translations' => 'array', 'is_required' => 'bool', @@ -141,8 +139,9 @@ final class FormField extends Model * events are worth logging — e.g. created/deleted/restored, field_type * changed (value storage changes), binding changed, is_pii toggled, * is_filterable toggled (triggers backfill), structural options changes. - * NOT logged (noise): label/help_text/sort_order/conditional_logic/ - * translations. + * Conditional-logic changes emit `field.conditional_logic_replaced` + * via FormFieldConditionalLogicService (ARCH §8; WS-5c commit 2). + * NOT logged (noise): label/help_text/sort_order/translations. * * Bulk-fixture suppression: the activitylog.enabled config key is the * kill-switch. Seeders and one-shot commands wrap themselves in diff --git a/api/database/factories/FormBuilder/FormFieldFactory.php b/api/database/factories/FormBuilder/FormFieldFactory.php index c2895eb4..691d3455 100644 --- a/api/database/factories/FormBuilder/FormFieldFactory.php +++ b/api/database/factories/FormBuilder/FormFieldFactory.php @@ -58,7 +58,6 @@ final class FormFieldFactory extends Factory 'is_unique' => false, 'is_pii' => false, 'display_width' => FormFieldDisplayWidth::FULL, - 'conditional_logic' => null, 'role_restrictions' => null, 'translations' => null, 'value_storage_hint' => $fieldType->recommendedValueStorageHint(), diff --git a/api/database/migrations/2026_04_26_100003_drop_conditional_logic_json_column.php b/api/database/migrations/2026_04_26_100003_drop_conditional_logic_json_column.php new file mode 100644 index 00000000..42d4a9a5 --- /dev/null +++ b/api/database/migrations/2026_04_26_100003_drop_conditional_logic_json_column.php @@ -0,0 +1,44 @@ +dropColumn('conditional_logic'); + }); + } + } + + public function down(): void + { + if (! Schema::hasColumn('form_fields', 'conditional_logic')) { + Schema::table('form_fields', function (Blueprint $table): void { + $table->json('conditional_logic')->nullable()->after('display_width'); + }); + } + } +}; diff --git a/api/database/seeders/FormBuilderDevSeeder.php b/api/database/seeders/FormBuilderDevSeeder.php index 479ee565..03669cb9 100644 --- a/api/database/seeders/FormBuilderDevSeeder.php +++ b/api/database/seeders/FormBuilderDevSeeder.php @@ -100,7 +100,7 @@ final class FormBuilderDevSeeder 'is_filterable' => $field['is_filterable'] ?? false, 'is_pii' => $field['is_pii'] ?? false, 'binding' => null, // Pattern B — snapshot embeds null for form-owned fields. - 'conditional_logic' => null, + 'conditional_logic' => null, // snapshot shape: null for fields without conditional logic 'translations' => null, 'value_storage_hint' => $field['type']->recommendedValueStorageHint()->value, 'sort_order' => $sortOrder + 1, diff --git a/api/tests/Feature/FormBuilder/Bindings/FormFieldBindingMigrationTest.php b/api/tests/Feature/FormBuilder/Bindings/FormFieldBindingMigrationTest.php index 6cb40747..9dfc41a5 100644 --- a/api/tests/Feature/FormBuilder/Bindings/FormFieldBindingMigrationTest.php +++ b/api/tests/Feature/FormBuilder/Bindings/FormFieldBindingMigrationTest.php @@ -39,7 +39,7 @@ final class FormFieldBindingMigrationTest extends TestCase // (drop-validation-cols, configs-backfill, create-configs, // validation-rules-backfill, create-validation-rules) + // 2 WS-5a migrations (drop-binding-cols, create-bindings) = 10. - $this->artisan('migrate:rollback', ['--step' => 10])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 11])->assertSuccessful(); $this->assertFalse(Schema::hasTable('form_field_bindings')); $this->assertTrue(Schema::hasColumn('form_fields', 'binding')); $this->assertTrue(Schema::hasColumn('form_field_library', 'default_binding')); @@ -101,7 +101,7 @@ final class FormFieldBindingMigrationTest extends TestCase public function test_rollback_reconstructs_json_and_drops_table(): void { // Walk back the full WS-5c + WS-5b + WS-5a stack (10 migrations). - $this->artisan('migrate:rollback', ['--step' => 10])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 11])->assertSuccessful(); [$fieldAId, , ] = $this->seedFieldsWithBindingJson(); [$libAId, ] = $this->seedLibraryWithBindingJson(); @@ -115,7 +115,7 @@ final class FormFieldBindingMigrationTest extends TestCase // go → restores the pre-WS-5b state (conditional-logic, // validation-rules and configs tables gone, validation_rules JSON // columns reappear on source tables; binding contract intact). - $this->artisan('migrate:rollback', ['--step' => 8])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 9])->assertSuccessful(); $this->assertFalse(Schema::hasTable('form_field_conditional_logic_groups')); $this->assertFalse(Schema::hasTable('form_field_conditional_logic_conditions')); $this->assertFalse(Schema::hasTable('form_field_validation_rules')); diff --git a/api/tests/Feature/FormBuilder/ConditionalLogic/ConditionalLogicBackfillTest.php b/api/tests/Feature/FormBuilder/ConditionalLogic/ConditionalLogicBackfillTest.php index 5325ea46..5483fd6a 100644 --- a/api/tests/Feature/FormBuilder/ConditionalLogic/ConditionalLogicBackfillTest.php +++ b/api/tests/Feature/FormBuilder/ConditionalLogic/ConditionalLogicBackfillTest.php @@ -32,7 +32,7 @@ final class ConditionalLogicBackfillTest extends TestCase public function test_forward_backfill_builds_nested_tree_from_legacy_json(): void { // Roll back only the backfill migration (latest WS-5c step). - $this->artisan('migrate:rollback', ['--step' => 1])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 2])->assertSuccessful(); $this->assertTrue(Schema::hasColumn('form_fields', 'conditional_logic')); $fieldId = $this->seedFieldWithJson([ @@ -90,9 +90,11 @@ final class ConditionalLogicBackfillTest extends TestCase public function test_rollback_reconstructs_canonical_json(): void { - // Starting state: fully migrated. Seed relational rows bypassing the - // service, then roll back one step (backfill) — it should repopulate - // the JSON column. + // Starting state: fully migrated. The conditional_logic column is + // already gone (WS-5c commit 3 drop). Seed relational rows bypassing + // the service, then roll back two steps — drop-column reverses first + // (column re-appears), then the backfill's `down()` reads relational + // rows and writes the JSON back. $org = Organisation::factory()->create(); $schema = FormSchema::factory()->create(['organisation_id' => $org->id]); @@ -105,7 +107,6 @@ final class ConditionalLogicBackfillTest extends TestCase 'label' => 'Subject', 'value_storage_hint' => 'json', 'sort_order' => 0, - 'conditional_logic' => null, 'created_at' => now(), 'updated_at' => now(), ]); @@ -152,7 +153,7 @@ final class ConditionalLogicBackfillTest extends TestCase ]); // Roll back only the backfill migration — writes the JSON back. - $this->artisan('migrate:rollback', ['--step' => 1])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 2])->assertSuccessful(); $reconstructed = DB::table('form_fields') ->where('id', $fieldId) @@ -179,7 +180,7 @@ final class ConditionalLogicBackfillTest extends TestCase public function test_unknown_top_level_key_fails_migration(): void { - $this->artisan('migrate:rollback', ['--step' => 1])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 2])->assertSuccessful(); $this->seedFieldWithJson([ 'hide_when' => ['all' => [['field_slug' => 'x', 'operator' => 'equals', 'value' => 1]]], @@ -192,7 +193,7 @@ final class ConditionalLogicBackfillTest extends TestCase public function test_unknown_comparison_operator_fails_migration(): void { - $this->artisan('migrate:rollback', ['--step' => 1])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 2])->assertSuccessful(); $this->seedFieldWithJson([ 'show_when' => ['all' => [['field_slug' => 'x', 'operator' => 'matches_regex', 'value' => 'y']]], diff --git a/api/tests/Feature/FormBuilder/ConditionalLogic/ConditionalLogicJsonColumnDroppedTest.php b/api/tests/Feature/FormBuilder/ConditionalLogic/ConditionalLogicJsonColumnDroppedTest.php new file mode 100644 index 00000000..4be33bc1 --- /dev/null +++ b/api/tests/Feature/FormBuilder/ConditionalLogic/ConditionalLogicJsonColumnDroppedTest.php @@ -0,0 +1,26 @@ +assertFalse( + Schema::hasColumn('form_fields', 'conditional_logic'), + 'WS-5c commit 3 must drop form_fields.conditional_logic — it now lives in the relational tree.', + ); + } +} diff --git a/api/tests/Feature/FormBuilder/ConditionalLogic/FormFieldStrictConditionalLogicRequestTest.php b/api/tests/Feature/FormBuilder/ConditionalLogic/FormFieldStrictConditionalLogicRequestTest.php new file mode 100644 index 00000000..c094d240 --- /dev/null +++ b/api/tests/Feature/FormBuilder/ConditionalLogic/FormFieldStrictConditionalLogicRequestTest.php @@ -0,0 +1,166 @@ +seed(RoleSeeder::class); + $this->org = Organisation::factory()->create(); + $this->admin = User::factory()->create(); + $this->org->users()->attach($this->admin, ['role' => 'org_admin']); + $this->schema = FormSchema::factory()->create(['organisation_id' => $this->org->id]); + } + + public function test_store_rejects_unknown_comparison_operator(): void + { + Sanctum::actingAs($this->admin); + + FormField::factory()->create(['form_schema_id' => $this->schema->id, 'slug' => 'gate']); + + $response = $this->postJson( + "/api/v1/organisations/{$this->org->id}/forms/schemas/{$this->schema->id}/fields", + [ + 'field_type' => FormFieldType::TEXT->value, + 'slug' => 'target', + 'label' => 'Target', + 'conditional_logic' => [ + 'show_when' => [ + 'all' => [ + ['field_slug' => 'gate', 'operator' => 'matches_regex', 'value' => 'y'], + ], + ], + ], + ], + ); + + $response->assertStatus(422); + $response->assertJsonValidationErrors(['conditional_logic']); + } + + public function test_store_rejects_root_condition_without_group(): void + { + Sanctum::actingAs($this->admin); + + $response = $this->postJson( + "/api/v1/organisations/{$this->org->id}/forms/schemas/{$this->schema->id}/fields", + [ + 'field_type' => FormFieldType::TEXT->value, + 'slug' => 'target', + 'label' => 'Target', + // Condition at root — invalid; ARCH §8 requires a group wrapper. + 'conditional_logic' => [ + 'show_when' => [ + 'field_slug' => 'gate', + 'operator' => 'equals', + 'value' => 'y', + ], + ], + ], + ); + + $response->assertStatus(422); + $response->assertJsonValidationErrors(['conditional_logic']); + } + + public function test_store_rejects_empty_group(): void + { + Sanctum::actingAs($this->admin); + + $response = $this->postJson( + "/api/v1/organisations/{$this->org->id}/forms/schemas/{$this->schema->id}/fields", + [ + 'field_type' => FormFieldType::TEXT->value, + 'slug' => 'target', + 'label' => 'Target', + 'conditional_logic' => ['show_when' => ['all' => []]], + ], + ); + + $response->assertStatus(422); + $response->assertJsonValidationErrors(['conditional_logic']); + } + + public function test_store_rejects_unknown_field_slug(): void + { + Sanctum::actingAs($this->admin); + + $response = $this->postJson( + "/api/v1/organisations/{$this->org->id}/forms/schemas/{$this->schema->id}/fields", + [ + 'field_type' => FormFieldType::TEXT->value, + 'slug' => 'target', + 'label' => 'Target', + 'conditional_logic' => [ + 'show_when' => [ + 'all' => [ + ['field_slug' => 'nonexistent', 'operator' => 'equals', 'value' => 'y'], + ], + ], + ], + ], + ); + + $response->assertStatus(422); + $this->assertStringContainsString('nonexistent', (string) $response->json('message')); + } + + public function test_store_accepts_valid_tree_and_persists_relational_rows(): void + { + Sanctum::actingAs($this->admin); + FormField::factory()->create(['form_schema_id' => $this->schema->id, 'slug' => 'gate']); + + $response = $this->postJson( + "/api/v1/organisations/{$this->org->id}/forms/schemas/{$this->schema->id}/fields", + [ + 'field_type' => FormFieldType::TEXT->value, + 'slug' => 'target', + 'label' => 'Target', + 'conditional_logic' => [ + 'show_when' => [ + 'all' => [ + ['field_slug' => 'gate', 'operator' => 'equals', 'value' => 'yes'], + ], + ], + ], + ], + ); + + $response->assertStatus(201); + $this->assertSame([ + 'show_when' => [ + 'all' => [ + ['field_slug' => 'gate', 'operator' => 'equals', 'value' => 'yes'], + ], + ], + ], $response->json('data.conditional_logic')); + } +} diff --git a/api/tests/Feature/FormBuilder/Configs/FormFieldConfigBackfillAndDropTest.php b/api/tests/Feature/FormBuilder/Configs/FormFieldConfigBackfillAndDropTest.php index cd210440..0543e1ee 100644 --- a/api/tests/Feature/FormBuilder/Configs/FormFieldConfigBackfillAndDropTest.php +++ b/api/tests/Feature/FormBuilder/Configs/FormFieldConfigBackfillAndDropTest.php @@ -30,7 +30,7 @@ final class FormFieldConfigBackfillAndDropTest extends TestCase // Roll back 2 WS-5c migrations + 5 WS-5b migrations = 7, to get the // pre-WS-5b state where the JSON column still exists on form_fields // / form_field_library. - $this->artisan('migrate:rollback', ['--step' => 8])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 9])->assertSuccessful(); $this->assertTrue(Schema::hasColumn('form_fields', 'validation_rules')); $fieldId = $this->seedField([ diff --git a/api/tests/Feature/FormBuilder/ValidationRules/FormFieldValidationRuleBackfillTest.php b/api/tests/Feature/FormBuilder/ValidationRules/FormFieldValidationRuleBackfillTest.php index 6274ac7c..750a5c4b 100644 --- a/api/tests/Feature/FormBuilder/ValidationRules/FormFieldValidationRuleBackfillTest.php +++ b/api/tests/Feature/FormBuilder/ValidationRules/FormFieldValidationRuleBackfillTest.php @@ -37,7 +37,7 @@ final class FormFieldValidationRuleBackfillTest extends TestCase // validation-rules-backfill + create-validation-rules) = 7. // Brings us to the pre-WS-5b state: validation_rules JSON column // present, no relational tables for WS-5b. - $this->artisan('migrate:rollback', ['--step' => 8])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 9])->assertSuccessful(); $this->assertFalse(Schema::hasTable('form_field_validation_rules')); $this->assertTrue(Schema::hasColumn('form_fields', 'validation_rules')); @@ -98,7 +98,7 @@ final class FormFieldValidationRuleBackfillTest extends TestCase // (validation_rules JSON column present; no relational tables for // WS-5b). Step count: drop-cols + configs-backfill + create-configs // + validation-rules-backfill + create-validation-rules = 5. - $this->artisan('migrate:rollback', ['--step' => 8])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 9])->assertSuccessful(); $fieldId = $this->seedFieldWithJson([ 'field_type' => 'TAG_PICKER', @@ -122,7 +122,7 @@ final class FormFieldValidationRuleBackfillTest extends TestCase // (validation_rules JSON column present; no relational tables for // WS-5b). Step count: drop-cols + configs-backfill + create-configs // + validation-rules-backfill + create-validation-rules = 5. - $this->artisan('migrate:rollback', ['--step' => 8])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 9])->assertSuccessful(); $fieldId = $this->seedFieldWithJson([ 'field_type' => 'TEXT', @@ -149,7 +149,7 @@ final class FormFieldValidationRuleBackfillTest extends TestCase // (validation_rules JSON column present; no relational tables for // WS-5b). Step count: drop-cols + configs-backfill + create-configs // + validation-rules-backfill + create-validation-rules = 5. - $this->artisan('migrate:rollback', ['--step' => 8])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 9])->assertSuccessful(); $this->seedFieldWithJson([ 'field_type' => 'TEXT', @@ -166,7 +166,7 @@ final class FormFieldValidationRuleBackfillTest extends TestCase // (validation_rules JSON column present; no relational tables for // WS-5b). Step count: drop-cols + configs-backfill + create-configs // + validation-rules-backfill + create-validation-rules = 5. - $this->artisan('migrate:rollback', ['--step' => 8])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 9])->assertSuccessful(); $this->seedFieldWithJson([ 'field_type' => 'BOOLEAN', @@ -185,7 +185,7 @@ final class FormFieldValidationRuleBackfillTest extends TestCase // full-back-then-full-forward cycle — rolling back all WS-5b // migrations restores the pre-WS-5b state (columns present on // source tables; validation rules relational table gone). - $this->artisan('migrate:rollback', ['--step' => 8])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 9])->assertSuccessful(); [$numberId] = $this->seedFields(); $this->artisan('migrate')->assertSuccessful(); @@ -200,7 +200,7 @@ final class FormFieldValidationRuleBackfillTest extends TestCase // Roll back WS-5b fully → column reappears and carries canonical JSON // reconstructed from the relational rows. - $this->artisan('migrate:rollback', ['--step' => 8])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 9])->assertSuccessful(); $this->assertTrue(Schema::hasColumn('form_fields', 'validation_rules')); $field = DB::table('form_fields')->where('id', $numberId)->first();