From c033dc6cd2157435051c25ceb8c962bb6ea5c097 Mon Sep 17 00:00:00 2001 From: "bert.hausmans" Date: Sat, 25 Apr 2026 22:33:39 +0200 Subject: [PATCH] feat(form-builder): add apply_status columns and action-failures table (WS-6) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - form_submissions: apply_status (nullable, NO default for legacy rows per RFC O1), apply_completed_at, indexed on (form_schema_id, apply_status) and (organisation_id, apply_status) - form_submission_action_failures: ULID PK, FK to submission + binding, resolve/dismiss state separated (RFC V2), retention via parent cascade-delete - Migration rehearsal test added (invokes down() directly because the new migrations land between WS-5a and WS-5b chronologically, not at the tail of the migration list) Three pre-existing WS-5 backfill tests also bump their --step rollback counts by +2 (FormFieldBindingMigrationTest, FormFieldConfigBackfillAndDropTest, FormFieldValidationRuleBackfillTest) to account for the two new migrations sitting in the chronological middle of the WS-5 stack — required to keep those tests' pre-WS-5b rollback target reachable. SCHEMA.md updated to v2.3. Refs: RFC-WS-6.md §3 (Q4, Q5), §4 (V2), §5 (O1) Co-Authored-By: Claude Opus 4.7 (1M context) --- ...end_form_submissions_with_apply_status.php | 42 +++++ ...create_form_submission_action_failures.php | 73 +++++++++ .../FormFieldBindingMigrationTest.php | 19 ++- .../FormFieldConfigBackfillAndDropTest.php | 8 +- .../Schema/Ws6FoundationMigrationTest.php | 155 ++++++++++++++++++ .../FormFieldValidationRuleBackfillTest.php | 19 ++- dev-docs/SCHEMA.md | 44 ++++- 7 files changed, 337 insertions(+), 23 deletions(-) create mode 100644 api/database/migrations/2026_04_25_140000_extend_form_submissions_with_apply_status.php create mode 100644 api/database/migrations/2026_04_25_140100_create_form_submission_action_failures.php create mode 100644 api/tests/Feature/FormBuilder/Schema/Ws6FoundationMigrationTest.php diff --git a/api/database/migrations/2026_04_25_140000_extend_form_submissions_with_apply_status.php b/api/database/migrations/2026_04_25_140000_extend_form_submissions_with_apply_status.php new file mode 100644 index 00000000..b79355d9 --- /dev/null +++ b/api/database/migrations/2026_04_25_140000_extend_form_submissions_with_apply_status.php @@ -0,0 +1,42 @@ +string('apply_status', 20) + ->nullable() + ->after('identity_match_status'); + + $table->timestamp('apply_completed_at') + ->nullable() + ->after('apply_status'); + + $table->index(['form_schema_id', 'apply_status'], 'fs_schema_apply_status_idx'); + $table->index(['organisation_id', 'apply_status'], 'fs_org_apply_status_idx'); + }); + } + + public function down(): void + { + Schema::table('form_submissions', function (Blueprint $table): void { + $table->dropIndex('fs_org_apply_status_idx'); + $table->dropIndex('fs_schema_apply_status_idx'); + $table->dropColumn(['apply_completed_at', 'apply_status']); + }); + } +}; diff --git a/api/database/migrations/2026_04_25_140100_create_form_submission_action_failures.php b/api/database/migrations/2026_04_25_140100_create_form_submission_action_failures.php new file mode 100644 index 00000000..0d351fdb --- /dev/null +++ b/api/database/migrations/2026_04_25_140100_create_form_submission_action_failures.php @@ -0,0 +1,73 @@ +ulid('id')->primary(); + + $table->foreignUlid('form_submission_id') + ->constrained('form_submissions') + ->cascadeOnDelete(); + + $table->string('listener_class', 255); + + $table->foreignUlid('binding_id') + ->nullable() + ->constrained('form_field_bindings') + ->nullOnDelete(); + + $table->timestamp('failed_at'); + $table->string('exception_class', 255); + $table->text('exception_message'); + $table->json('context'); + $table->unsignedTinyInteger('retry_count')->default(0); + + $table->timestamp('resolved_at')->nullable(); + $table->foreignUlid('resolved_by_user_id') + ->nullable() + ->constrained('users') + ->nullOnDelete(); + $table->text('resolved_note')->nullable(); + + $table->timestamp('dismissed_at')->nullable(); + $table->foreignUlid('dismissed_by_user_id') + ->nullable() + ->constrained('users') + ->nullOnDelete(); + $table->string('dismissed_reason_type', 40)->nullable(); + $table->string('dismissed_reason_note', 500)->nullable(); + + $table->timestamps(); + + $table->index('form_submission_id', 'fsaf_submission_idx'); + $table->index(['listener_class', 'failed_at'], 'fsaf_listener_failed_idx'); + $table->index('resolved_at', 'fsaf_resolved_idx'); + $table->index('dismissed_at', 'fsaf_dismissed_idx'); + $table->index('binding_id', 'fsaf_binding_idx'); + $table->index('dismissed_reason_type', 'fsaf_reason_type_idx'); + }); + } + + public function down(): void + { + Schema::dropIfExists('form_submission_action_failures'); + } +}; diff --git a/api/tests/Feature/FormBuilder/Bindings/FormFieldBindingMigrationTest.php b/api/tests/Feature/FormBuilder/Bindings/FormFieldBindingMigrationTest.php index 9850bca5..b86424ea 100644 --- a/api/tests/Feature/FormBuilder/Bindings/FormFieldBindingMigrationTest.php +++ b/api/tests/Feature/FormBuilder/Bindings/FormFieldBindingMigrationTest.php @@ -40,8 +40,9 @@ final class FormFieldBindingMigrationTest extends TestCase // create-conditional-logic-groups) + 5 WS-5b migrations // (drop-validation-cols, configs-backfill, create-configs, // validation-rules-backfill, create-validation-rules) + - // 2 WS-5a migrations (drop-binding-cols, create-bindings) = 14. - $this->artisan('migrate:rollback', ['--step' => 14])->assertSuccessful(); + // 2 WS-6 migrations (action-failures, apply-status) + + // 2 WS-5a migrations (drop-binding-cols, create-bindings) = 16. + $this->artisan('migrate:rollback', ['--step' => 16])->assertSuccessful(); $this->assertFalse(Schema::hasTable('form_field_bindings')); $this->assertTrue(Schema::hasColumn('form_fields', 'binding')); $this->assertTrue(Schema::hasColumn('form_field_library', 'default_binding')); @@ -102,8 +103,8 @@ final class FormFieldBindingMigrationTest extends TestCase public function test_rollback_reconstructs_json_and_drops_table(): void { - // Walk back the full WS-5d + WS-5c + WS-5b + WS-5a stack (14 migrations). - $this->artisan('migrate:rollback', ['--step' => 14])->assertSuccessful(); + // Walk back the full WS-5d + WS-5c + WS-6 + WS-5b + WS-5a stack (16 migrations). + $this->artisan('migrate:rollback', ['--step' => 16])->assertSuccessful(); [$fieldAId, , ] = $this->seedFieldsWithBindingJson(); [$libAId, ] = $this->seedLibraryWithBindingJson(); @@ -114,11 +115,11 @@ final class FormFieldBindingMigrationTest extends TestCase $this->assertSame(5, DB::table('form_field_bindings')->count()); // Step back over WS-5d (3 migrations) + WS-5c (4 migrations) + - // WS-5b (5 migrations) in one go → restores 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' => 12])->assertSuccessful(); + // WS-6 (2 migrations) + WS-5b (5 migrations) in one go → restores + // 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' => 14])->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/Configs/FormFieldConfigBackfillAndDropTest.php b/api/tests/Feature/FormBuilder/Configs/FormFieldConfigBackfillAndDropTest.php index 0543e1ee..f6ef5dbd 100644 --- a/api/tests/Feature/FormBuilder/Configs/FormFieldConfigBackfillAndDropTest.php +++ b/api/tests/Feature/FormBuilder/Configs/FormFieldConfigBackfillAndDropTest.php @@ -27,10 +27,10 @@ final class FormFieldConfigBackfillAndDropTest extends TestCase public function test_backfill_translates_tag_categories_and_storage_disk(): void { - // 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' => 9])->assertSuccessful(); + // 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' => 11])->assertSuccessful(); $this->assertTrue(Schema::hasColumn('form_fields', 'validation_rules')); $fieldId = $this->seedField([ diff --git a/api/tests/Feature/FormBuilder/Schema/Ws6FoundationMigrationTest.php b/api/tests/Feature/FormBuilder/Schema/Ws6FoundationMigrationTest.php new file mode 100644 index 00000000..0c7fb5ab --- /dev/null +++ b/api/tests/Feature/FormBuilder/Schema/Ws6FoundationMigrationTest.php @@ -0,0 +1,155 @@ +assertTrue(Schema::hasColumn('form_submissions', 'apply_status')); + $this->assertTrue(Schema::hasColumn('form_submissions', 'apply_completed_at')); + + $indexes = $this->indexNamesFor('form_submissions'); + $this->assertContains('fs_schema_apply_status_idx', $indexes); + $this->assertContains('fs_org_apply_status_idx', $indexes); + } + + public function test_apply_status_has_no_database_default_so_legacy_rows_remain_null(): void + { + $organisation = Organisation::factory()->create(); + $schema = FormSchema::factory()->for($organisation)->create(); + + // Direct DB insert simulating a legacy row written before the + // applicator existed: nothing writes apply_status. + $id = (string) \Illuminate\Support\Str::ulid(); + DB::table('form_submissions')->insert([ + 'id' => $id, + 'form_schema_id' => $schema->id, + 'organisation_id' => $organisation->id, + 'status' => 'submitted', + 'is_test' => false, + 'auto_save_count' => 0, + 'created_at' => now(), + 'updated_at' => now(), + ]); + + $row = DB::table('form_submissions')->where('id', $id)->first(); + $this->assertNotNull($row); + $this->assertNull($row->apply_status); + $this->assertNull($row->apply_completed_at); + } + + public function test_form_submission_action_failures_table_has_expected_columns_and_indexes(): void + { + $this->assertTrue(Schema::hasTable('form_submission_action_failures')); + + $expected = [ + 'id', 'form_submission_id', 'listener_class', 'binding_id', + 'failed_at', 'exception_class', 'exception_message', 'context', + 'retry_count', + 'resolved_at', 'resolved_by_user_id', 'resolved_note', + 'dismissed_at', 'dismissed_by_user_id', + 'dismissed_reason_type', 'dismissed_reason_note', + 'created_at', 'updated_at', + ]; + foreach ($expected as $column) { + $this->assertTrue( + Schema::hasColumn('form_submission_action_failures', $column), + "Missing column: {$column}", + ); + } + + // RFC V3 — table intentionally has NO denormalized organisation_id. + $this->assertFalse( + Schema::hasColumn('form_submission_action_failures', 'organisation_id'), + 'form_submission_action_failures must not carry organisation_id (FK-chain tenancy per RFC V3)', + ); + + $indexes = $this->indexNamesFor('form_submission_action_failures'); + foreach ([ + 'fsaf_submission_idx', + 'fsaf_listener_failed_idx', + 'fsaf_resolved_idx', + 'fsaf_dismissed_idx', + 'fsaf_binding_idx', + 'fsaf_reason_type_idx', + ] as $idx) { + $this->assertContains($idx, $indexes, "Missing index: {$idx}"); + } + } + + public function test_down_methods_clean_up_columns_and_table(): void + { + // The two WS-6 foundation migrations sit chronologically between + // 2026_04_25_100000 (WS-5a) and 2026_04_26_100000 (WS-5c) — they + // are NOT the last batch, so `migrate:rollback --step=N` would + // target unrelated migrations. Invoke the down() methods directly. + $createFailures = require database_path( + 'migrations/2026_04_25_140100_create_form_submission_action_failures.php', + ); + $applyStatus = require database_path( + 'migrations/2026_04_25_140000_extend_form_submissions_with_apply_status.php', + ); + + $createFailures->down(); + $applyStatus->down(); + + try { + $this->assertFalse(Schema::hasColumn('form_submissions', 'apply_status')); + $this->assertFalse(Schema::hasColumn('form_submissions', 'apply_completed_at')); + $this->assertFalse(Schema::hasTable('form_submission_action_failures')); + + $indexes = $this->indexNamesFor('form_submissions'); + $this->assertNotContains('fs_schema_apply_status_idx', $indexes); + $this->assertNotContains('fs_org_apply_status_idx', $indexes); + } finally { + // Restore state for any subsequent tests in this class. + $applyStatus->up(); + $createFailures->up(); + } + } + + /** + * @return list + */ + private function indexNamesFor(string $table): array + { + $driver = DB::connection()->getDriverName(); + if ($driver === 'mysql' || $driver === 'mariadb') { + return collect(DB::select("SHOW INDEX FROM {$table}")) + ->pluck('Key_name') + ->unique() + ->values() + ->all(); + } + if ($driver === 'sqlite') { + return collect(DB::select("SELECT name FROM sqlite_master WHERE type='index' AND tbl_name = ?", [$table])) + ->pluck('name') + ->all(); + } + + return Schema::getIndexes($table) + ? collect(Schema::getIndexes($table))->pluck('name')->all() + : []; + } +} diff --git a/api/tests/Feature/FormBuilder/ValidationRules/FormFieldValidationRuleBackfillTest.php b/api/tests/Feature/FormBuilder/ValidationRules/FormFieldValidationRuleBackfillTest.php index 3c296c26..cd54162a 100644 --- a/api/tests/Feature/FormBuilder/ValidationRules/FormFieldValidationRuleBackfillTest.php +++ b/api/tests/Feature/FormBuilder/ValidationRules/FormFieldValidationRuleBackfillTest.php @@ -34,12 +34,13 @@ final class FormFieldValidationRuleBackfillTest extends TestCase // Roll back: 3 WS-5d migrations (drop-options-cols, backfill-options, // create-options) + 4 WS-5c migrations (drop-conditional-logic-col, // backfill-conditional-logic, create-conditional-logic-conditions, - // create-conditional-logic-groups) + 5 WS-5b migrations + // create-conditional-logic-groups) + 2 WS-6 migrations + // (action-failures, apply-status) + 5 WS-5b migrations // (drop-cols + configs-backfill + create-configs + - // validation-rules-backfill + create-validation-rules) = 12. + // 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' => 12])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 14])->assertSuccessful(); $this->assertFalse(Schema::hasTable('form_field_validation_rules')); $this->assertTrue(Schema::hasColumn('form_fields', 'validation_rules')); @@ -100,7 +101,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' => 12])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 14])->assertSuccessful(); $fieldId = $this->seedFieldWithJson([ 'field_type' => 'TAG_PICKER', @@ -124,7 +125,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' => 12])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 14])->assertSuccessful(); $fieldId = $this->seedFieldWithJson([ 'field_type' => 'TEXT', @@ -151,7 +152,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' => 12])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 14])->assertSuccessful(); $this->seedFieldWithJson([ 'field_type' => 'TEXT', @@ -168,7 +169,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' => 12])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 14])->assertSuccessful(); $this->seedFieldWithJson([ 'field_type' => 'BOOLEAN', @@ -187,7 +188,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' => 12])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 14])->assertSuccessful(); [$numberId] = $this->seedFields(); $this->artisan('migrate')->assertSuccessful(); @@ -202,7 +203,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' => 12])->assertSuccessful(); + $this->artisan('migrate:rollback', ['--step' => 14])->assertSuccessful(); $this->assertTrue(Schema::hasColumn('form_fields', 'validation_rules')); $field = DB::table('form_fields')->where('id', $numberId)->first(); diff --git a/dev-docs/SCHEMA.md b/dev-docs/SCHEMA.md index eb996978..7c374243 100644 --- a/dev-docs/SCHEMA.md +++ b/dev-docs/SCHEMA.md @@ -2336,12 +2336,14 @@ that aggregates the user's submitted, non-test `form_submissions`. | `idempotency_key` | ULID nullable | Duplicate-submit guard — UNIQUE `(form_schema_id, idempotency_key)` since v2.1 | | `anonymised_at` | timestamp nullable | | | `identity_match_status` | string(20) null | **v2.1** null\|pending\|matched\|none — written by `TriggerPersonIdentityMatchOnFormSubmit` (ARCH §31.1) | +| `apply_status` | string(20) null | **v2.3 — RFC-WS-6 §3 (Q4) + §5 (O1)** null\|pending\|completed\|partial\|failed — written by `FormBindingApplicator`. NULL for legacy rows by design (no DB default) | +| `apply_completed_at` | timestamp nullable | **v2.3 — RFC-WS-6 §3 (Q4)** Set when applicator finishes (success or fail) | | `search_index` | mediumText null | Concatenated text of text-type values; FULLTEXT-indexed on MySQL when supported | | `created_at`, `updated_at` | timestamps | | | `deleted_at` | timestamp nullable | Soft delete | **Relations:** `belongsTo` schema, organisation, event, submittedBy / reviewedBy (User); `morphsTo` subject; `hasMany` values, section statuses, delegations -**Indexes:** `(form_schema_id, status)`, `(organisation_id, status)` (v2.2 — addendum Q2), `(event_id, status)` (v2.2 — addendum Q2), `(subject_type, subject_id)`, `(submitted_by_user_id)`, `(form_schema_id, review_status)`, **UNIQUE** `(form_schema_id, idempotency_key)` (v2.1; replaced the non-unique composite index from v2.0), `(form_schema_id, identity_match_status)` (v2.1), `FULLTEXT(search_index)` (MySQL/InnoDB — best-effort, skipped gracefully on SQLite) +**Indexes:** `(form_schema_id, status)`, `(organisation_id, status)` (v2.2 — addendum Q2), `(event_id, status)` (v2.2 — addendum Q2), `(subject_type, subject_id)`, `(submitted_by_user_id)`, `(form_schema_id, review_status)`, **UNIQUE** `(form_schema_id, idempotency_key)` (v2.1; replaced the non-unique composite index from v2.0), `(form_schema_id, identity_match_status)` (v2.1), `(form_schema_id, apply_status)` (v2.3 — RFC-WS-6), `(organisation_id, apply_status)` (v2.3 — RFC-WS-6), `FULLTEXT(search_index)` (MySQL/InnoDB — best-effort, skipped gracefully on SQLite) **Events fired:** `FormSubmissionCreated`, `FormSubmissionDraftUpdated`, `FormSubmissionSubmitted`, `FormSubmissionReviewed`, `FormSubmissionSectionSubmitted`, `FormSubmissionSectionReviewed`, `FormSubmissionAnonymised`, `FormSubmissionArchived`, `FormSubmissionDeleted` **Soft delete:** yes @@ -2544,6 +2546,46 @@ that aggregates the user's submitted, non-test `form_submissions`. --- +### `form_submission_action_failures` + +> **v2.3 — RFC-WS-6 Q5** Audit table for binding-pipeline failures. +> Populated by `ApplyBindingsOnFormSubmit` (and any future listener +> that wraps a `FormBindingApplicator` invocation) when the inner +> apply transaction rolls back. Retry / Resolve / Dismiss workflows +> consume this table from session 2 onward. +> +> No `organisation_id` column — tenant scope flows via +> `form_submission_id → form_submissions.organisation_id`. Enforced +> at access time by `FormSubmissionActionFailurePolicy` (RFC V3, +> IDOR-class FK-chain pattern). Do NOT register `OrganisationScope` +> directly on this table. + +| Column | Type | Notes | +| ------------------------- | ------------------- | ------------------------------------------------------------------ | +| `id` | ULID | PK | +| `form_submission_id` | ULID FK | → form_submissions, cascade delete | +| `listener_class` | string(255) | e.g. `App\Listeners\FormBuilder\ApplyBindingsOnFormSubmit` | +| `binding_id` | ULID FK nullable | → form_field_bindings, null on delete (binding may be edited later) | +| `failed_at` | timestamp | | +| `exception_class` | string(255) | | +| `exception_message` | text | | +| `context` | json | Free-form: `{target_entity, target_attribute, value_excerpt, merge_strategy}` | +| `retry_count` | tinyint unsigned | default: 0 | +| `resolved_at` | timestamp nullable | Set when retry succeeds OR organiser marks resolved | +| `resolved_by_user_id` | ULID FK nullable | → users, null on delete | +| `resolved_note` | text nullable | Optional human note on resolution | +| `dismissed_at` | timestamp nullable | Mutex with `resolved_at` | +| `dismissed_by_user_id` | ULID FK nullable | → users, null on delete | +| `dismissed_reason_type` | string(40) nullable | `DismissalReasonType` enum (RFC V2). Constrained at request layer | +| `dismissed_reason_note` | string(500) nullable | Required at request layer when reason_type = `other` | +| `created_at`, `updated_at` | timestamps | | + +**Relations:** `belongsTo` submission, binding (nullable), resolvedBy / dismissedBy (User) +**Indexes:** `(form_submission_id)`, `(listener_class, failed_at)`, `(resolved_at)`, `(dismissed_at)`, `(binding_id)`, `(dismissed_reason_type)` (analytics) +**Soft delete:** no — audit table; retention via parent submission cascade-delete + +--- + **Activity log strategy:** explicit calls via `FormSchema::logSchemaChange()` and `FormField::logFieldChange()` — no `LogsActivity` trait (would produce noise). Only impactful events