fix(form-builder): fire FormSubmissionSubmitted AFTER transaction commit (WS-6)
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>
This commit is contained in:
@@ -103,7 +103,7 @@ final class FormSubmissionService
|
|||||||
{
|
{
|
||||||
$this->assertWritable($submission);
|
$this->assertWritable($submission);
|
||||||
|
|
||||||
return DB::transaction(function () use ($submission, $actor): FormSubmission {
|
$result = DB::transaction(function () use ($submission, $actor): FormSubmission {
|
||||||
$schema = $submission->schema;
|
$schema = $submission->schema;
|
||||||
|
|
||||||
$submission->status = FormSubmissionStatus::SUBMITTED->value;
|
$submission->status = FormSubmissionStatus::SUBMITTED->value;
|
||||||
@@ -125,10 +125,17 @@ final class FormSubmissionService
|
|||||||
// Compute SIGNATURE hashes on submit (ARCH §9). One query, scalar-safe.
|
// Compute SIGNATURE hashes on submit (ARCH §9). One query, scalar-safe.
|
||||||
$this->finaliseSignatureValues($submission);
|
$this->finaliseSignatureValues($submission);
|
||||||
|
|
||||||
FormSubmissionSubmitted::dispatch($submission);
|
return $submission;
|
||||||
|
|
||||||
return $submission->refresh();
|
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// 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
|
public function review(FormSubmission $submission, FormSubmissionReviewStatus $status, ?string $notes, User $reviewer): FormSubmission
|
||||||
|
|||||||
@@ -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,
|
||||||
|
]);
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user