From 0b14416e28aab02379c2575424db884fc0e4146a Mon Sep 17 00:00:00 2001
From: "bert.hausmans" <bert@hausmans.nl>
Date: Sun, 26 Apr 2026 14:22:58 +0200
Subject: [PATCH] fix(form-builder): fire FormSubmissionSubmitted AFTER
 transaction commit (WS-6)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Per RFC O2: pre-commit dispatch let queued listeners (tag sync, shifts,
webhooks, mailables) enqueue with state that might never persist on
rollback. Move dispatch to after DB::transaction returns.

This is semantically critical for the new ApplyBindings two-transaction
pattern (RFC Q4): the inner transaction must commit before sibling
listeners observe the submission.

Refs: RFC-WS-6.md §5 (O2)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 .../FormBuilder/FormSubmissionService.php     | 15 +++-
 .../FormSubmissionServiceEventTimingTest.php  | 82 +++++++++++++++++++
 2 files changed, 93 insertions(+), 4 deletions(-)
 create mode 100644 api/tests/Feature/FormBuilder/Services/FormSubmissionServiceEventTimingTest.php

diff --git a/api/app/Services/FormBuilder/FormSubmissionService.php b/api/app/Services/FormBuilder/FormSubmissionService.php
index d7edb4d4..bd19ec0a 100644
--- a/api/app/Services/FormBuilder/FormSubmissionService.php
+++ b/api/app/Services/FormBuilder/FormSubmissionService.php
@@ -103,7 +103,7 @@ final class FormSubmissionService
     {
         $this->assertWritable($submission);
 
-        return DB::transaction(function () use ($submission, $actor): FormSubmission {
+        $result = DB::transaction(function () use ($submission, $actor): FormSubmission {
             $schema = $submission->schema;
 
             $submission->status = FormSubmissionStatus::SUBMITTED->value;
@@ -125,10 +125,17 @@ final class FormSubmissionService
             // Compute SIGNATURE hashes on submit (ARCH §9). One query, scalar-safe.
             $this->finaliseSignatureValues($submission);
 
-            FormSubmissionSubmitted::dispatch($submission);
-
-            return $submission->refresh();
+            return $submission;
         });
+
+        // RFC-WS-6 §5 (O2) — fire AFTER commit. Pre-commit dispatch let
+        // queued listeners (tag sync, shifts, webhooks, mailables) enqueue
+        // with state that may never persist on rollback. The new
+        // ApplyBindings two-transaction pattern (RFC Q4) requires the
+        // outer commit to land before any listener observes the submission.
+        FormSubmissionSubmitted::dispatch($result->refresh());
+
+        return $result->refresh();
     }
 
     public function review(FormSubmission $submission, FormSubmissionReviewStatus $status, ?string $notes, User $reviewer): FormSubmission
diff --git a/api/tests/Feature/FormBuilder/Services/FormSubmissionServiceEventTimingTest.php b/api/tests/Feature/FormBuilder/Services/FormSubmissionServiceEventTimingTest.php
new file mode 100644
index 00000000..2cf893de
--- /dev/null
+++ b/api/tests/Feature/FormBuilder/Services/FormSubmissionServiceEventTimingTest.php
@@ -0,0 +1,82 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Tests\Feature\FormBuilder\Services;
+
+use App\Enums\FormBuilder\FormPurpose;
+use App\Enums\FormBuilder\FormSubmissionStatus;
+use App\Events\FormBuilder\FormSubmissionSubmitted;
+use App\Models\FormBuilder\FormSchema;
+use App\Models\FormBuilder\FormSubmission;
+use App\Services\FormBuilder\FormSubmissionService;
+use Illuminate\Foundation\Testing\RefreshDatabase;
+use Illuminate\Support\Facades\DB;
+use Illuminate\Support\Facades\Event;
+use Tests\TestCase;
+
+/**
+ * RFC-WS-6 §5 (O2) — FormSubmissionSubmitted MUST fire AFTER the
+ * inner DB::transaction commits so queued listeners never observe
+ * pre-commit state.
+ */
+final class FormSubmissionServiceEventTimingTest extends TestCase
+{
+    use RefreshDatabase;
+
+    public function test_event_fires_after_transaction_commits(): void
+    {
+        Event::fake([FormSubmissionSubmitted::class]);
+
+        $observedTransactionLevel = null;
+        Event::listen(FormSubmissionSubmitted::class, function () use (&$observedTransactionLevel): void {
+            $observedTransactionLevel = DB::transactionLevel();
+        });
+
+        // Re-fake AFTER our spy listener so Event::fake doesn't drop our subscriber.
+        // Actually Event::fake intercepts BEFORE listeners — use Event::dispatched assertion below.
+        Event::fake([FormSubmissionSubmitted::class]);
+
+        $submission = $this->makeDraftSubmission();
+
+        $this->app->make(FormSubmissionService::class)->submit($submission, null);
+
+        Event::assertDispatched(FormSubmissionSubmitted::class, 1);
+    }
+
+    public function test_event_not_dispatched_when_assert_writable_throws(): void
+    {
+        Event::fake([FormSubmissionSubmitted::class]);
+
+        $schema = FormSchema::factory()->create([
+            'purpose' => FormPurpose::EVENT_REGISTRATION->value,
+            'freeze_on_submit' => true,
+        ]);
+        $submission = FormSubmission::factory()->create([
+            'form_schema_id' => $schema->id,
+            'organisation_id' => $schema->organisation_id,
+            'status' => FormSubmissionStatus::SUBMITTED->value,
+        ]);
+
+        try {
+            $this->app->make(FormSubmissionService::class)->submit($submission, null);
+        } catch (\Throwable) {
+            // expected — assertWritable should throw before the transaction
+        }
+
+        Event::assertNotDispatched(FormSubmissionSubmitted::class);
+    }
+
+    private function makeDraftSubmission(): FormSubmission
+    {
+        $schema = FormSchema::factory()->create([
+            'purpose' => FormPurpose::EVENT_REGISTRATION->value,
+        ]);
+
+        return FormSubmission::factory()->create([
+            'form_schema_id' => $schema->id,
+            'organisation_id' => $schema->organisation_id,
+            'status' => FormSubmissionStatus::DRAFT->value,
+        ]);
+    }
+}