From ae06b16d9be95c889d8eddbe748ffb347d5a4b8a Mon Sep 17 00:00:00 2001 From: "bert.hausmans" Date: Tue, 28 Apr 2026 12:56:52 +0200 Subject: [PATCH] fix(form-builder): restore FK on form_schemas.default_crowd_type_id (WS-6) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The original session 2.5 migration had to omit this FK due to an SQLite-only "rebuild on FK add" cascade-delete quirk. Now that the test infrastructure has moved to MySQL (Task 1 of this session), the quirk does not apply and the FK is restored to match every other FK in this table. Changes: - New migration `2026_04_28_100000_restore_default_crowd_type_id_foreign_key` adds a FOREIGN KEY (default_crowd_type_id) REFERENCES crowd_types(id) ON DELETE SET NULL. Deleting a CrowdType nulls the column on dependent schemas instead of cascading the schema delete. - Original migration's comment block rewritten — the SQLite-quirk rationale was demonstrably misleading; replaced with a forward-looking pointer to the FK-restore migration. - PersonProvisioner::resolveCrowdTypeId() docblock updated: the runtime failsafe is now defense in depth alongside the DB-level FK + publish guard, not the sole load-bearing check. New test (`DefaultCrowdTypeForeignKeyTest`) exercises both the ON-DELETE-SET-NULL cascade and the existence of the FK in information_schema.REFERENTIAL_CONSTRAINTS — the second assertion would have been impossible on SQLite, which is exactly the point. Migration step counts in 5 backfill tests bumped +1 because the FK- restore migration sits at the top of the migration stack: - FormFieldBindingMigrationTest: 17→18, 15→16 - ConditionalLogicBackfillTest: 6→7 - FormFieldConfigBackfillAndDropTest: 12→13 - FormFieldOptionsBackfillTest: 2→3 - FormFieldValidationRuleBackfillTest: 15→16 All 1388 tests pass on MySQL (1386 prior + 2 new FK tests). Larastan baseline unchanged. Refs: RFC-WS-6.md v1.1 §3 Q9 addendum, WS-6 session 2.5 deviation #1 Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Bindings/PersonProvisioner.php | 9 +-- ..._default_crowd_type_id_to_form_schemas.php | 17 +++--- ...tore_default_crowd_type_id_foreign_key.php | 36 +++++++++++ .../FormFieldBindingMigrationTest.php | 6 +- .../ConditionalLogicBackfillTest.php | 8 +-- .../FormFieldConfigBackfillAndDropTest.php | 2 +- .../Options/FormFieldOptionsBackfillTest.php | 20 +++--- .../Schema/DefaultCrowdTypeForeignKeyTest.php | 61 +++++++++++++++++++ .../FormFieldValidationRuleBackfillTest.php | 14 ++--- 9 files changed, 134 insertions(+), 39 deletions(-) create mode 100644 api/database/migrations/2026_04_28_100000_restore_default_crowd_type_id_foreign_key.php create mode 100644 api/tests/Feature/FormBuilder/Schema/DefaultCrowdTypeForeignKeyTest.php diff --git a/api/app/FormBuilder/Bindings/PersonProvisioner.php b/api/app/FormBuilder/Bindings/PersonProvisioner.php index 99f98847..98a8f9e5 100644 --- a/api/app/FormBuilder/Bindings/PersonProvisioner.php +++ b/api/app/FormBuilder/Bindings/PersonProvisioner.php @@ -156,10 +156,11 @@ final readonly class PersonProvisioner * declares its target CrowdType explicitly via `default_crowd_type_id`. * * RFC-WS-6 v1.1 §3 Q9 addendum (was: silent oldest() fallback in - * session 2). The RequiresDefaultCrowdType publish guard prevents - * misconfigured event_registration schemas from publishing; this - * runtime throw is a failsafe for live-table edits between publish - * and apply. + * session 2). Defense in depth: the FK on form_schemas (added in + * session 2.6 after the SQLite → MySQL test switch) gives DB-level + * referential integrity; the RequiresDefaultCrowdType publish guard + * blocks publish when null; this runtime throw is the failsafe for + * live-table edits between publish and apply. * * @throws PersonProvisioningException */ diff --git a/api/database/migrations/2026_04_26_120000_add_default_crowd_type_id_to_form_schemas.php b/api/database/migrations/2026_04_26_120000_add_default_crowd_type_id_to_form_schemas.php index ca33d51f..b1aaee62 100644 --- a/api/database/migrations/2026_04_26_120000_add_default_crowd_type_id_to_form_schemas.php +++ b/api/database/migrations/2026_04_26_120000_add_default_crowd_type_id_to_form_schemas.php @@ -7,27 +7,24 @@ use Illuminate\Database\Schema\Blueprint; use Illuminate\Support\Facades\Schema; /** - * RFC-WS-6 v1.1 §3 Q8 addendum — per-schema default CrowdType for + * RFC-WS-6.md v1.1 §3 Q9 addendum — per-schema default CrowdType for * Person provisioning. Replaces the silent oldest() heuristic from * WS-6 session 2 with explicit configuration; RequiresDefaultCrowdType * publish guard ensures event_registration schemas declare it. * * No backfill: pre-launch the table is empty. Dev seeders populate * the column when reseeding (FormBuilderDevSeeder). + * + * The FK constraint is added in a follow-up migration + * (2026_04_28_100000_restore_default_crowd_type_id_foreign_key) once + * the test database moved off SQLite to MySQL — the original session + * 2.5 skipped it due to an SQLite-only "rebuild on FK add" cascade + * quirk that does not apply on MySQL. */ return new class extends Migration { public function up(): void { - // Plain nullable ULID column. NO database-level foreign key — - // SQLite's rebuild-on-FK-add cascade-deletes form_fields rows - // (form_fields.form_schema_id has cascadeOnDelete on - // form_schemas), which corrupts running migration tests. - // Application-level integrity: FormSchema::defaultCrowdType() - // belongsTo loads from CrowdType; CrowdTypeObserver could add - // a soft-handle on delete if needed in production. The new - // RequiresDefaultCrowdType publish guard plus the runtime - // failsafe in PersonProvisioner are the load-bearing checks. Schema::table('form_schemas', function (Blueprint $table): void { $table->ulid('default_crowd_type_id') ->nullable() diff --git a/api/database/migrations/2026_04_28_100000_restore_default_crowd_type_id_foreign_key.php b/api/database/migrations/2026_04_28_100000_restore_default_crowd_type_id_foreign_key.php new file mode 100644 index 00000000..3058a716 --- /dev/null +++ b/api/database/migrations/2026_04_28_100000_restore_default_crowd_type_id_foreign_key.php @@ -0,0 +1,36 @@ +foreign('default_crowd_type_id') + ->references('id') + ->on('crowd_types') + ->nullOnDelete(); + }); + } + + public function down(): void + { + Schema::table('form_schemas', function (Blueprint $table): void { + $table->dropForeign(['default_crowd_type_id']); + }); + } +}; diff --git a/api/tests/Feature/FormBuilder/Bindings/FormFieldBindingMigrationTest.php b/api/tests/Feature/FormBuilder/Bindings/FormFieldBindingMigrationTest.php index de61cabf..9b9f1890 100644 --- a/api/tests/Feature/FormBuilder/Bindings/FormFieldBindingMigrationTest.php +++ b/api/tests/Feature/FormBuilder/Bindings/FormFieldBindingMigrationTest.php @@ -56,7 +56,7 @@ final class FormFieldBindingMigrationTest extends TestCase // validation-rules-backfill, create-validation-rules) + // 2 WS-6 migrations (action-failures, apply-status) + // 2 WS-5a migrations (drop-binding-cols, create-bindings) = 16. - $this->artisan('migrate:rollback', ['--step' => 17])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 18])->assertSuccessful(); $this->assertFalse(Schema::hasTable('form_field_bindings')); $this->assertTrue(Schema::hasColumn('form_fields', 'binding')); $this->assertTrue(Schema::hasColumn('form_field_library', 'default_binding')); @@ -118,7 +118,7 @@ final class FormFieldBindingMigrationTest extends TestCase public function test_rollback_reconstructs_json_and_drops_table(): void { // Walk back the full WS-5d + WS-5c + WS-6 + WS-5b + WS-5a stack (16 migrations). - $this->artisan('migrate:rollback', ['--step' => 17])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 18])->assertSuccessful(); [$fieldAId] = $this->seedFieldsWithBindingJson(); [$libAId] = $this->seedLibraryWithBindingJson(); @@ -133,7 +133,7 @@ final class FormFieldBindingMigrationTest extends TestCase // the pre-WS-5b state (conditional-logic, validation-rules, configs // and options tables gone, validation_rules + options JSON columns // reappear on source tables; binding contract intact). - $this->artisan('migrate:rollback', ['--step' => 15])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 16])->assertSuccessful(); $this->assertFalse(Schema::hasTable('form_field_options')); $this->assertFalse(Schema::hasTable('form_field_conditional_logic_groups')); $this->assertFalse(Schema::hasTable('form_field_conditional_logic_conditions')); diff --git a/api/tests/Feature/FormBuilder/ConditionalLogic/ConditionalLogicBackfillTest.php b/api/tests/Feature/FormBuilder/ConditionalLogic/ConditionalLogicBackfillTest.php index 893ecd96..887f65b4 100644 --- a/api/tests/Feature/FormBuilder/ConditionalLogic/ConditionalLogicBackfillTest.php +++ b/api/tests/Feature/FormBuilder/ConditionalLogic/ConditionalLogicBackfillTest.php @@ -48,7 +48,7 @@ final class ConditionalLogicBackfillTest extends TestCase // create-options + WS-5c drop-cl-col + WS-5c backfill-cl // migrations to land in the conditional-logic JSON-era state with // no relational form_field_options table yet. - $this->artisan('migrate:rollback', ['--step' => 6])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 7])->assertSuccessful(); $this->assertTrue(Schema::hasColumn('form_fields', 'conditional_logic')); $fieldId = $this->seedFieldWithJson([ @@ -169,7 +169,7 @@ final class ConditionalLogicBackfillTest extends TestCase ]); // Roll back only the backfill migration — writes the JSON back. - $this->artisan('migrate:rollback', ['--step' => 6])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 7])->assertSuccessful(); $reconstructed = DB::table('form_fields') ->where('id', $fieldId) @@ -198,7 +198,7 @@ final class ConditionalLogicBackfillTest extends TestCase public function test_unknown_top_level_key_fails_migration(): void { - $this->artisan('migrate:rollback', ['--step' => 6])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 7])->assertSuccessful(); $this->seedFieldWithJson([ 'hide_when' => ['all' => [['field_slug' => 'x', 'operator' => 'equals', 'value' => 1]]], @@ -211,7 +211,7 @@ final class ConditionalLogicBackfillTest extends TestCase public function test_unknown_comparison_operator_fails_migration(): void { - $this->artisan('migrate:rollback', ['--step' => 6])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 7])->assertSuccessful(); $this->seedFieldWithJson([ 'show_when' => ['all' => [['field_slug' => 'x', 'operator' => 'matches_regex', 'value' => 'y']]], diff --git a/api/tests/Feature/FormBuilder/Configs/FormFieldConfigBackfillAndDropTest.php b/api/tests/Feature/FormBuilder/Configs/FormFieldConfigBackfillAndDropTest.php index 31731a5d..05cdc917 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 4 WS-5c migrations + 2 WS-6 migrations + 5 WS-5b // migrations = 11, to get the pre-WS-5b state where the JSON column // still exists on form_fields / form_field_library. - $this->artisan('migrate:rollback', ['--step' => 12])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 13])->assertSuccessful(); $this->assertTrue(Schema::hasColumn('form_fields', 'validation_rules')); $fieldId = $this->seedField([ diff --git a/api/tests/Feature/FormBuilder/Options/FormFieldOptionsBackfillTest.php b/api/tests/Feature/FormBuilder/Options/FormFieldOptionsBackfillTest.php index 90bc2a6a..f2e9af3f 100644 --- a/api/tests/Feature/FormBuilder/Options/FormFieldOptionsBackfillTest.php +++ b/api/tests/Feature/FormBuilder/Options/FormFieldOptionsBackfillTest.php @@ -46,7 +46,7 @@ final class FormFieldOptionsBackfillTest extends TestCase // Roll back only the backfill migration (latest WS-5d step). // Leaves the form_field_options table in place, JSON columns // present on the source tables, and snapshots in pre-WS-5d shape. - $this->artisan('migrate:rollback', ['--step' => 2])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 3])->assertSuccessful(); $this->assertTrue(Schema::hasTable('form_field_options')); $this->assertTrue(Schema::hasColumn('form_fields', 'options')); @@ -130,7 +130,7 @@ final class FormFieldOptionsBackfillTest extends TestCase public function test_rollback_reconstructs_json_columns_and_snapshots(): void { - $this->artisan('migrate:rollback', ['--step' => 2])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 3])->assertSuccessful(); [$selectId, $multiId, $libraryId] = $this->seedFieldsAndLibraryWithJson(); $submissionId = $this->seedSubmissionWithSnapshot($selectId); @@ -143,7 +143,7 @@ final class FormFieldOptionsBackfillTest extends TestCase // Step back over only the backfill migration → JSON columns repopulate // and snapshots revert to flat-string-array shape. - $this->artisan('migrate:rollback', ['--step' => 2])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 3])->assertSuccessful(); $this->assertSame(0, DB::table('form_field_options')->count()); $select = DB::table('form_fields')->where('id', $selectId)->first(); @@ -162,7 +162,7 @@ final class FormFieldOptionsBackfillTest extends TestCase public function test_fails_when_options_present_on_non_option_field_type(): void { - $this->artisan('migrate:rollback', ['--step' => 2])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 3])->assertSuccessful(); $this->seedFieldWithOptions('TAG_PICKER', ['Veiligheid', 'Horeca']); $this->expectException(\RuntimeException::class); @@ -172,7 +172,7 @@ final class FormFieldOptionsBackfillTest extends TestCase public function test_fails_when_options_contains_non_string_entry(): void { - $this->artisan('migrate:rollback', ['--step' => 2])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 3])->assertSuccessful(); $this->seedFieldWithOptionsRaw('SELECT', json_encode([ ['label' => 'A'], @@ -186,7 +186,7 @@ final class FormFieldOptionsBackfillTest extends TestCase public function test_fails_when_options_is_object_shape(): void { - $this->artisan('migrate:rollback', ['--step' => 2])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 3])->assertSuccessful(); $this->seedFieldWithOptionsRaw('SELECT', json_encode([ 'XS' => 'Extra small', @@ -200,7 +200,7 @@ final class FormFieldOptionsBackfillTest extends TestCase public function test_fails_on_translations_length_mismatch(): void { - $this->artisan('migrate:rollback', ['--step' => 2])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 3])->assertSuccessful(); $this->seedFieldWithOptionsRaw('SELECT', json_encode(['XS', 'S', 'M']), json_encode([ 'de' => ['options' => ['Klein', 'Mittel']], // 2 vs 3 ])); @@ -212,7 +212,7 @@ final class FormFieldOptionsBackfillTest extends TestCase public function test_fails_on_non_string_translation(): void { - $this->artisan('migrate:rollback', ['--step' => 2])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 3])->assertSuccessful(); $this->seedFieldWithOptionsRaw('SELECT', json_encode(['XS', 'S']), json_encode([ 'de' => ['options' => ['Klein', 42]], ])); @@ -224,7 +224,7 @@ final class FormFieldOptionsBackfillTest extends TestCase public function test_fails_on_oversized_translation(): void { - $this->artisan('migrate:rollback', ['--step' => 2])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 3])->assertSuccessful(); $this->seedFieldWithOptionsRaw('SELECT', json_encode(['XS']), json_encode([ 'de' => ['options' => [str_repeat('x', 256)]], ])); @@ -236,7 +236,7 @@ final class FormFieldOptionsBackfillTest extends TestCase public function test_fails_when_snapshot_options_present_on_non_option_field_type(): void { - $this->artisan('migrate:rollback', ['--step' => 2])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 3])->assertSuccessful(); $this->seedTemplateWithSnapshotRaw([ 'fields' => [[ 'id' => (string) Str::ulid(), diff --git a/api/tests/Feature/FormBuilder/Schema/DefaultCrowdTypeForeignKeyTest.php b/api/tests/Feature/FormBuilder/Schema/DefaultCrowdTypeForeignKeyTest.php new file mode 100644 index 00000000..a9380c1f --- /dev/null +++ b/api/tests/Feature/FormBuilder/Schema/DefaultCrowdTypeForeignKeyTest.php @@ -0,0 +1,61 @@ +create(); + $schema = FormSchema::factory()->create([ + 'organisation_id' => $crowdType->organisation_id, + 'default_crowd_type_id' => $crowdType->id, + ]); + + $this->assertSame($crowdType->id, $schema->fresh()->default_crowd_type_id); + + // Force-delete bypasses the SoftDeletes trait — only the + // DB-level FK can null the column on form_schemas. + $crowdType->forceDelete(); + + $this->assertNull($schema->fresh()->default_crowd_type_id); + } + + public function test_database_level_foreign_key_constraint_exists(): void + { + $row = DB::selectOne( + 'SELECT rc.CONSTRAINT_NAME, kcu.REFERENCED_TABLE_NAME, ' + .'kcu.REFERENCED_COLUMN_NAME, rc.DELETE_RULE ' + .'FROM information_schema.REFERENTIAL_CONSTRAINTS rc ' + .'JOIN information_schema.KEY_COLUMN_USAGE kcu ' + .' ON kcu.CONSTRAINT_NAME = rc.CONSTRAINT_NAME ' + .' AND kcu.CONSTRAINT_SCHEMA = rc.CONSTRAINT_SCHEMA ' + .'WHERE rc.CONSTRAINT_SCHEMA = DATABASE() ' + ." AND kcu.TABLE_NAME = 'form_schemas' " + ." AND kcu.COLUMN_NAME = 'default_crowd_type_id'" + ); + + $this->assertNotNull($row, 'Expected FK on form_schemas.default_crowd_type_id'); + $this->assertSame('crowd_types', $row->REFERENCED_TABLE_NAME); + $this->assertSame('id', $row->REFERENCED_COLUMN_NAME); + $this->assertSame('SET NULL', $row->DELETE_RULE); + } +} diff --git a/api/tests/Feature/FormBuilder/ValidationRules/FormFieldValidationRuleBackfillTest.php b/api/tests/Feature/FormBuilder/ValidationRules/FormFieldValidationRuleBackfillTest.php index 1bd9d7a5..f493d9ab 100644 --- a/api/tests/Feature/FormBuilder/ValidationRules/FormFieldValidationRuleBackfillTest.php +++ b/api/tests/Feature/FormBuilder/ValidationRules/FormFieldValidationRuleBackfillTest.php @@ -53,7 +53,7 @@ final class FormFieldValidationRuleBackfillTest extends TestCase // validation-rules-backfill + create-validation-rules) = 14. // Brings us to the pre-WS-5b state: validation_rules JSON column // present, no relational tables for WS-5b/c/d. - $this->artisan('migrate:rollback', ['--step' => 15])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 16])->assertSuccessful(); $this->assertFalse(Schema::hasTable('form_field_validation_rules')); $this->assertTrue(Schema::hasColumn('form_fields', 'validation_rules')); @@ -114,7 +114,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' => 15])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 16])->assertSuccessful(); $fieldId = $this->seedFieldWithJson([ 'field_type' => 'TAG_PICKER', @@ -138,7 +138,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' => 15])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 16])->assertSuccessful(); $fieldId = $this->seedFieldWithJson([ 'field_type' => 'TEXT', @@ -165,7 +165,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' => 15])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 16])->assertSuccessful(); $this->seedFieldWithJson([ 'field_type' => 'TEXT', @@ -182,7 +182,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' => 15])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 16])->assertSuccessful(); $this->seedFieldWithJson([ 'field_type' => 'BOOLEAN', @@ -201,7 +201,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' => 15])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 16])->assertSuccessful(); [$numberId] = $this->seedFields(); $this->artisan('migrate')->assertSuccessful(); @@ -216,7 +216,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' => 15])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 16])->assertSuccessful(); $this->assertTrue(Schema::hasColumn('form_fields', 'validation_rules')); $field = DB::table('form_fields')->where('id', $numberId)->first();