diff --git a/api/app/FormBuilder/Bindings/FormBindingApplicator.php b/api/app/FormBuilder/Bindings/FormBindingApplicator.php index 8501cd10..ddeafd6d 100644 --- a/api/app/FormBuilder/Bindings/FormBindingApplicator.php +++ b/api/app/FormBuilder/Bindings/FormBindingApplicator.php @@ -7,6 +7,7 @@ namespace App\FormBuilder\Bindings; use App\Enums\FormBuilder\BindingTargetType; use App\Enums\FormBuilder\FormFieldBindingMergeStrategy; use App\Exceptions\FormBuilder\FormBindingApplicatorException; +use App\Exceptions\FormBuilder\FormBindingApplicatorTimeoutException; use App\Exceptions\FormBuilder\FormBindingInfraException; use App\Exceptions\FormBuilder\FormBindingSchemaConfigException; use App\FormBuilder\Purposes\PurposeRegistry; @@ -28,6 +29,10 @@ use Throwable; * - Q9: subject resolution via per-purpose PurposeSubjectResolver. * - Q10: optional sectionId for future section-level apply. * - Q12: hierarchical activity log via BindingActivityLogger. + * - v1.3 Q1 add 4: optional deadline (withDeadline()) — soft post-call + * microtime check throwing FormBindingApplicatorTimeoutException. + * Cannot interrupt mid-query; intended to catch the long-tail of + * slow applies before they hang the public flow. */ // Not final + not readonly: listener tests need to override `apply()` for // throw-path coverage (Mockery can't mock final classes; PHP doesn't allow @@ -35,6 +40,15 @@ use Throwable; // individually to preserve immutability. class FormBindingApplicator { + /** + * Per RFC-WS-6 §Q1 v1.3 addition 4 — soft deadline (seconds). NULL + * means "no deadline check" (default). Set via withDeadline() so the + * value travels with a clone and the original instance stays + * deadline-free for other callers (e.g. the retry-service path, + * which currently does not bound apply() — see ARCH-BINDINGS §5.3). + */ + private ?int $deadlineSeconds = null; + public function __construct( private readonly PurposeRegistry $purposeRegistry, private readonly BindingConflictResolver $conflictResolver, @@ -42,11 +56,34 @@ class FormBindingApplicator private readonly BindingActivityLogger $activityLogger, ) {} + /** + * Returns a clone configured to throw FormBindingApplicatorTimeoutException + * if apply() exceeds the given deadline. + * + * Per RFC-WS-6 §Q1 v1.3 addition 4 + ARCH-BINDINGS §5.3. + * + * Implementation note: this is a soft post-call deadline check via + * microtime. It cannot interrupt mid-query — for that, configure MySQL + * connection timeouts at the database driver level. The soft deadline + * is sufficient to prevent runaway apply() calls from hanging the + * public flow indefinitely; a typical apply() takes <100ms, so a 5s + * deadline catches the long tail. + */ + public function withDeadline(int $seconds): self + { + $clone = clone $this; + $clone->deadlineSeconds = $seconds; + + return $clone; + } + /** * @throws FormBindingApplicatorException */ public function apply(FormSubmission $submission, ?string $sectionId = null): BindingPassResult { + $start = microtime(true); + if (DB::transactionLevel() < 1) { throw new FormBindingInfraException( submissionId: (string) $submission->id, @@ -81,38 +118,61 @@ class FormBindingApplicator provisionedSubjectId: null, applications: [], ); - $this->activityLogger->logPass($submission, $result); + } else { + $resolved = $this->conflictResolver->resolve($submission, $sectionId); - return $result; - } - - $resolved = $this->conflictResolver->resolve($submission, $sectionId); - - // Persist subject identity for the result + apply each binding. - $applications = []; - foreach ($resolved as $binding) { - // Skip identity-key bindings — the resolver already used them - // for subject lookup in EventRegistration's PersonProvisioner - // path. Writing them again is a no-op at best, a clobber at - // worst. - if ($binding->isIdentityKey) { - continue; + // Persist subject identity for the result + apply each binding. + $applications = []; + foreach ($resolved as $binding) { + // Skip identity-key bindings — the resolver already used them + // for subject lookup in EventRegistration's PersonProvisioner + // path. Writing them again is a no-op at best, a clobber at + // worst. + if ($binding->isIdentityKey) { + continue; + } + $applications[] = $this->applyOne($subject, $binding); } - $applications[] = $this->applyOne($subject, $binding); - } - $result = new BindingPassResult( - formSubmissionId: (string) $submission->id, - provisionedSubjectType: $this->morphAlias($subject), - provisionedSubjectId: (string) $subject->getKey(), - applications: $applications, - ); + $result = new BindingPassResult( + formSubmissionId: (string) $submission->id, + provisionedSubjectType: $this->morphAlias($subject), + provisionedSubjectId: (string) $subject->getKey(), + applications: $applications, + ); + } $this->activityLogger->logPass($submission, $result); + $this->checkDeadline((string) $submission->id, $start); + return $result; } + /** + * Throws FormBindingApplicatorTimeoutException if a deadline is + * configured and the elapsed wall-clock time exceeds it. + */ + private function checkDeadline(string $submissionId, float $startMicrotime): void + { + if ($this->deadlineSeconds === null) { + return; + } + + $elapsed = microtime(true) - $startMicrotime; + if ($elapsed > $this->deadlineSeconds) { + throw new FormBindingApplicatorTimeoutException( + submissionId: $submissionId, + message: sprintf( + 'FormBindingApplicator exceeded deadline of %ds (elapsed: %.2fs) for submission %s', + $this->deadlineSeconds, + $elapsed, + $submissionId, + ), + ); + } + } + private function applyOne(Model $subject, ResolvedBinding $binding): BindingApplicationResult { try { diff --git a/api/app/Listeners/FormBuilder/ApplyBindingsOnFormSubmit.php b/api/app/Listeners/FormBuilder/ApplyBindingsOnFormSubmit.php index e6a8fd12..42e10ad2 100644 --- a/api/app/Listeners/FormBuilder/ApplyBindingsOnFormSubmit.php +++ b/api/app/Listeners/FormBuilder/ApplyBindingsOnFormSubmit.php @@ -6,7 +6,9 @@ namespace App\Listeners\FormBuilder; use App\Enums\FormBuilder\ApplyStatus; use App\Events\FormBuilder\FormSubmissionSubmitted; +use App\Exceptions\FormBuilder\FormBindingApplicatorException; use App\FormBuilder\Bindings\FormBindingApplicator; +use App\FormBuilder\Bindings\FormBindingExceptionClassifier; use App\Models\FormBuilder\FormSchema; use App\Models\FormBuilder\FormSubmission; use App\Models\FormBuilder\FormSubmissionActionFailure; @@ -21,10 +23,24 @@ use Throwable; * FormSubmissionActionFailure in a separate transaction (survives * inner rollback). * + * v1.3 changes (per RFC v1.3.1 + ARCH-BINDINGS v1.2): + * - Q1 addition 1: writes identity_match_status='pending' inside the + * inner transaction so the HTTP response carries the right state + * for the IdentityMatchBanner first-paint copy. Final state is + * written by the queued TriggerPersonIdentityMatch. + * - Q1 addition 4: wraps applicator with config-driven deadline so + * a runaway apply() throws FormBindingApplicatorTimeoutException + * instead of hanging the public flow. + * - Q3 addition 2: outer-transaction catch uses + * FormBindingExceptionClassifier::classify to write + * failure_response_code on the parent submission. The action-failure + * row is the canonical machine-replayable artefact; the column is + * the response-shape driver. + * * Throws are swallowed (RFC Q3) — sibling listeners must keep running. * - * SYNCHRONOUS by design — does NOT implement ShouldQueue. Identity - * match runs after this in the registered listener order. + * SYNCHRONOUS by design — does NOT implement ShouldQueue. Identity-match + * runs queued (post-v1.3) and is gated on apply_status=COMPLETED. */ final readonly class ApplyBindingsOnFormSubmit { @@ -37,28 +53,40 @@ final readonly class ApplyBindingsOnFormSubmit return; } - try { - DB::transaction(function () use ($submission): void { - $result = $this->applicator->apply($submission); + $deadlineSeconds = (int) config('form_builder.apply_deadline_seconds', 5); - FormSubmission::query() - ->whereKey($submission->id) - ->update([ - 'apply_status' => $result->applyStatus()->value, - 'apply_completed_at' => now(), - ]); + try { + DB::transaction(function () use ($submission, $deadlineSeconds): void { + $result = $this->applicator + ->withDeadline($deadlineSeconds) + ->apply($submission); + + $updates = [ + 'apply_status' => $result->applyStatus()->value, + 'apply_completed_at' => now(), + ]; + + // Initial identity_match_status='pending' write (RFC §Q1 + // v1.3 addition 1). Only meaningful when ApplyBindings + // produced a person subject; non-person purposes leave + // the column NULL per ARCH-BINDINGS §7.3 contract. + if ($result->provisionedSubjectType === 'person' + || $submission->subject_type === 'person') { + $updates['identity_match_status'] = 'pending'; + } if ($result->provisionedSubjectType !== null && $submission->subject_type === null) { // ApplyBindings just provisioned a Person; reflect it // on the submission so TriggerPersonIdentityMatch (next - // sync listener) can find it. - FormSubmission::query() - ->whereKey($submission->id) - ->update([ - 'subject_type' => $result->provisionedSubjectType, - 'subject_id' => $result->provisionedSubjectId, - ]); + // queued listener) can find it and the gating-invariant + // sees a coherent (subject_type, subject_id) pair. + $updates['subject_type'] = $result->provisionedSubjectType; + $updates['subject_id'] = $result->provisionedSubjectId; } + + FormSubmission::query() + ->whereKey($submission->id) + ->update($updates); }); } catch (Throwable $e) { // OUTSIDE the failed transaction — survives rollback. @@ -74,6 +102,9 @@ final readonly class ApplyBindingsOnFormSubmit 'exception_trace' => $e->getTraceAsString(), 'context' => [ 'purpose' => $purposeValue, + 'submission_id' => $e instanceof FormBindingApplicatorException + ? $e->submissionId + : (string) $submission->id, ], ]); FormSubmission::query() @@ -81,6 +112,7 @@ final readonly class ApplyBindingsOnFormSubmit ->update([ 'apply_status' => ApplyStatus::FAILED->value, 'apply_completed_at' => now(), + 'failure_response_code' => FormBindingExceptionClassifier::classify($e), ]); }); Log::error('form-builder.apply.transaction_rolled_back', [ diff --git a/api/app/Listeners/FormBuilder/SyncTagPickerSelectionsOnSubmit.php b/api/app/Listeners/FormBuilder/SyncTagPickerSelectionsOnSubmit.php index d8504bab..62a3dd2e 100644 --- a/api/app/Listeners/FormBuilder/SyncTagPickerSelectionsOnSubmit.php +++ b/api/app/Listeners/FormBuilder/SyncTagPickerSelectionsOnSubmit.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace App\Listeners\FormBuilder; +use App\Enums\FormBuilder\ApplyStatus; use App\Enums\FormBuilder\FormFieldType; use App\Enums\FormBuilder\FormPurpose; use App\Events\FormBuilder\FormSubmissionSubmitted; @@ -24,6 +25,11 @@ use Illuminate\Support\Facades\Log; * person's linked user. No-ops when person.user_id is null (deferred * sync runs on PersonIdentityService::confirmMatch). * + * Gating-invariant first statement per ARCH-BINDINGS §5.6: skip unless + * apply_status is COMPLETED. PARTIAL and FAILED both fall through — + * rebuilding tags against a Person whose tag-binding may have been the + * binding that failed would propagate partial state into derived data. + * * Failure mode: log at error level; never throw. Event propagation must * reach sibling listeners (§31.1 identity, §31.3 shifts, §31.8 crowd lists). */ @@ -43,11 +49,25 @@ final class SyncTagPickerSelectionsOnSubmit implements ShouldQueue public function handle(FormSubmissionSubmitted $event): void { try { + // Gating-invariant first statement per ARCH-BINDINGS §5.6. + // The fresh() reload is required because the inner-txn commit + // happens between dispatch and worker pickup; the in-memory + // event submission may carry pre-commit state. $submission = $event->submission->fresh(['schema']); if ($submission === null) { return; } + if ($submission->apply_status !== ApplyStatus::COMPLETED) { + Log::info('form-builder.queued-listener.skipped_apply_failed', [ + 'listener' => self::class, + 'submission_id' => (string) $submission->id, + 'apply_status' => $submission->apply_status?->value, + ]); + + return; + } + $schema = $submission->schema; if ($schema === null) { return; diff --git a/api/app/Listeners/FormBuilder/TriggerPersonIdentityMatchOnFormSubmit.php b/api/app/Listeners/FormBuilder/TriggerPersonIdentityMatchOnFormSubmit.php index da006082..9065bf43 100644 --- a/api/app/Listeners/FormBuilder/TriggerPersonIdentityMatchOnFormSubmit.php +++ b/api/app/Listeners/FormBuilder/TriggerPersonIdentityMatchOnFormSubmit.php @@ -4,112 +4,139 @@ declare(strict_types=1); namespace App\Listeners\FormBuilder; +use App\Enums\FormBuilder\ApplyStatus; use App\Enums\FormBuilder\FormPurpose; +use App\Events\FormBuilder\FormSubmissionIdentityMatchResolved; use App\Events\FormBuilder\FormSubmissionSubmitted; +use App\Exceptions\FormBuilder\IdentityMatchInvariantViolation; use App\Models\FormBuilder\FormSubmission; use App\Models\Person; use App\Services\PersonIdentityService; +use Illuminate\Contracts\Queue\ShouldQueue; +use Illuminate\Queue\InteractsWithQueue; use Illuminate\Support\Facades\Log; /** - * ARCH §31.1 — trigger PersonIdentityService::detectMatches on - * event_registration submissions and record the outcome on the - * submission so the portal can tell the submitter what's happening. + * RFC-WS-6 §Q1 v1.3 (queued) + §Q2 (invariant) + §Q1 v1.3 addition 2 (broadcast). + * + * Runs asynchronously after ApplyBindingsOnFormSubmit commits. Identity + * matching joins the persons table scoped to organisation; in large orgs + * (10k+ Persons) this can take seconds at peak load. Keeping it sync would + * block PHP-FPM workers during public-form submission spikes — operationally + * unacceptable for enterprise SaaS. The IdentityMatchBanner first-paint + * copy stays correct because ApplyBindingsOnFormSubmit writes + * identity_match_status='pending' inside its inner transaction; this + * listener writes the final state and broadcasts on the + * `submission.{id}` private channel so the frontend can refetch. + * + * Gating-invariant per ARCH-BINDINGS §5.6: skip unless apply_status is + * COMPLETED. PARTIAL and FAILED both fall through to the early-return — + * sibling state is incoherent, identity-match against possibly-half-applied + * data is meaningless. + * + * The post-ApplyBindings invariant per §Q2: subject_type='person' AND + * subject_id IS NOT NULL, OR apply_status=FAILED. No third state exists. + * If we see subject_type='person' AND subject_id IS NULL AND + * apply_status=COMPLETED, that's a structural defect — strict throw via + * IdentityMatchInvariantViolation routes to GlitchTip + (via the queue + * exception handler) form_submission_action_failures. * * States written to form_submissions.identity_match_status: * - 'matched' — the person is already linked to a user account - * - 'pending' — one or more PersonIdentityMatch(pending) rows exist - * OR the submission is public (no subject yet; organiser - * will attach a person and matching runs later) - * - 'none' — the person exists, is unlinked, and nothing matched - * - * Failure mode per §31.1: log at error level, never rethrow so sibling - * listeners on the same event (§31.10 tag sync, §31.3 shift provisioning) - * keep running. - * - * Runs synchronously (no ShouldQueue) so identity_match_status is - * already written by the time the HTTP submit-response serialises the - * submission — the portal's IdentityMatchBanner then renders on first - * confirmation-page load instead of after a queue worker tick. When - * FORM-05 proper adds heavier value-based matching, that work will - * dispatch as a separate queued job from within this listener so the - * eager state transition stays sync and the slow resolution stays - * async. + * - 'pending' — one or more PersonIdentityMatch(pending) candidates exist + * - 'none' — no candidates and not linked */ -final class TriggerPersonIdentityMatchOnFormSubmit +final class TriggerPersonIdentityMatchOnFormSubmit implements ShouldQueue { + use InteractsWithQueue; + + // Default queue connection — redis in production, sync in tests. + public string $queue = 'default'; + public function __construct( private readonly PersonIdentityService $identityService, ) {} public function handle(FormSubmissionSubmitted $event): void { - try { - $submission = $event->submission->fresh(['schema']); - if ($submission === null) { - return; - } - - $schema = $submission->schema; - if ($schema === null) { - return; - } - - $purpose = $schema->purpose instanceof \BackedEnum - ? $schema->purpose->value - : (string) $schema->purpose; - - if ($purpose !== FormPurpose::EVENT_REGISTRATION->value) { - return; - } - - $status = $this->resolveStatus($submission); - - // Use a raw UPDATE so we don't re-fire Eloquent events / an - // observer cascade on the submission itself. - FormSubmission::query() - ->whereKey($submission->id) - ->update(['identity_match_status' => $status]); - } catch (\Throwable $e) { - Log::error('form-builder.identity-match.listener_failed', [ - 'submission_id' => $event->submission->id, - 'message' => $e->getMessage(), - ]); + // Gating-invariant first statement per ARCH-BINDINGS §5.6. + // The fresh() reload is required because the inner-txn commit + + // outer-txn failure-record write happens between dispatch and + // worker pickup; the in-memory event submission may be stale. + $submission = $event->submission->fresh(['schema']); + if (! $submission instanceof FormSubmission) { + return; } - } - private function resolveStatus(FormSubmission $submission): string - { - if ($submission->subject_type !== 'person' || $submission->subject_id === null) { - // Post-WS-6 (RFC Q2): this path should be unreachable for - // event_registration submissions because ApplyBindingsOnFormSubmit - // provisions the Person synchronously before this listener fires. - // If we reach here, either: - // - ApplyBindings failed silently (check form_submission_action_failures) - // - Schema is misconfigured (no email binding, no identity-key) - // - Different purpose where subject genuinely is null - // Failsafe: preserve the existing 'pending' state so portal banner - // still renders sensibly, and surface the misconfiguration in logs. - Log::warning('form-builder.identity-match.no_person_subject_post_apply', [ + if ($submission->apply_status !== ApplyStatus::COMPLETED) { + Log::info('form-builder.queued-listener.skipped_apply_failed', [ + 'listener' => self::class, 'submission_id' => (string) $submission->id, - 'schema_id' => (string) $submission->form_schema_id, - 'subject_type' => $submission->subject_type, + 'apply_status' => $submission->apply_status?->value, ]); - return 'pending'; + return; + } + + // Non-event_registration purposes: no-op by design. Identity-match + // is a person-resolution flow specific to public registration. + $schema = $submission->schema; + if ($schema === null) { + return; + } + $purpose = $schema->purpose instanceof \BackedEnum + ? $schema->purpose->value + : (string) $schema->purpose; + if ($purpose !== FormPurpose::EVENT_REGISTRATION->value) { + return; + } + + // Non-person subjects on event_registration: also a no-op (defensive). + if ($submission->subject_type !== 'person') { + return; + } + + // Invariant per §Q2: subject_id IS NOT NULL when apply_status=COMPLETED + // and subject_type='person'. Violation = structural defect. + if ($submission->subject_id === null) { + throw new IdentityMatchInvariantViolation(sprintf( + "subject_type='person' but subject_id=null after ApplyBindings COMPLETED. submission_id=%s", + $submission->id, + )); } $person = Person::withoutGlobalScopes()->find($submission->subject_id); if ($person === null) { - return 'none'; - } + // The person was deleted between ApplyBindings COMPLETED and this + // worker pickup. Rare but possible; treat as a no-match terminal + // state so the banner shows a sensible final copy. + FormSubmission::query() + ->whereKey($submission->id) + ->update(['identity_match_status' => 'none']); - if ($person->user_id !== null) { - return 'matched'; + return; } $matches = $this->identityService->detectMatches($person); + $status = match (true) { + $person->user_id !== null => 'matched', + $matches->isNotEmpty() => 'pending', + default => 'none', + }; - return $matches->isNotEmpty() ? 'pending' : 'none'; + FormSubmission::query() + ->whereKey($submission->id) + ->update(['identity_match_status' => $status]); + + // Per RFC §Q1 v1.3 addition 2 — broadcast on the submission's + // private channel so the frontend portal IdentityMatchBanner can + // refetch the submission resource and transition copy from + // "we're checking matches…" to the final state without a manual + // reload. matchCount is an ephemeral DTO field; not persisted. + broadcast(new FormSubmissionIdentityMatchResolved( + submissionId: (string) $submission->id, + status: $status, + matchCount: $matches->count(), + ))->toOthers(); } } diff --git a/api/app/Providers/AppServiceProvider.php b/api/app/Providers/AppServiceProvider.php index f16031bd..a0718ada 100644 --- a/api/app/Providers/AppServiceProvider.php +++ b/api/app/Providers/AppServiceProvider.php @@ -163,16 +163,24 @@ class AppServiceProvider extends ServiceProvider FormField::observe(FormFieldChildTablesCascadeObserver::class); FormFieldLibrary::observe(FormFieldChildTablesCascadeObserver::class); - // RFC-WS-6 §3 (Q1) — sync chain on FormSubmissionSubmitted, in - // this exact order: - // 1. ApplyBindingsOnFormSubmit (sync) - // 2. TriggerPersonIdentityMatchOnFormSubmit (sync) - // Queued listeners on the same event (SyncTagPickerSelectionsOnSubmit, - // future webhook dispatcher, mailables) run in parallel after the - // sync chain via the queue. Their relative registration position - // is irrelevant. + // RFC-WS-6 v1.3 §Q1 — FormSubmissionSubmitted listener layout. + // + // SYNC chain (single listener): + // 1. ApplyBindingsOnFormSubmit + // + // QUEUED listeners (parallel, all gated on apply_status=COMPLETED + // per ARCH-BINDINGS §5.6 — PARTIAL and FAILED both fall through to + // the early-return): + // - TriggerPersonIdentityMatchOnFormSubmit (gates + invariant + broadcast) + // - SyncTagPickerSelectionsOnSubmit + // + // The listener-ordering structural test + // (FormSubmissionSubmittedListenerOrderTest) asserts ApplyBindings + // is the only sync listener and that every queued listener has + // the apply_status=COMPLETED gate as its first executable + // statement. - // RFC Q1 — applies bindings sync before identity match runs. + // RFC Q1 — applies bindings sync before queued siblings fire. \Illuminate\Support\Facades\Event::listen( FormSubmissionSubmitted::class, ApplyBindingsOnFormSubmit::class, @@ -184,7 +192,8 @@ class AppServiceProvider extends ServiceProvider SyncTagPickerSelectionsOnSubmit::class, ); - // ARCH §31.1 — identity-match trigger on event_registration (sync). + // ARCH §31.1 — identity-match trigger on event_registration (queued + // post-v1.3; was sync in v1.0/v1.2 layout). \Illuminate\Support\Facades\Event::listen( FormSubmissionSubmitted::class, TriggerPersonIdentityMatchOnFormSubmit::class, diff --git a/api/app/Services/FormBuilder/FormFailureRetryService.php b/api/app/Services/FormBuilder/FormFailureRetryService.php index 7361bf83..55a4c91a 100644 --- a/api/app/Services/FormBuilder/FormFailureRetryService.php +++ b/api/app/Services/FormBuilder/FormFailureRetryService.php @@ -8,6 +8,7 @@ use App\Enums\FormBuilder\ApplyStatus; use App\Exceptions\FormBuilder\FailureNotRetriableException; use App\Exceptions\FormBuilder\ParentSubmissionGoneException; use App\FormBuilder\Bindings\FormBindingApplicator; +use App\FormBuilder\Bindings\FormBindingExceptionClassifier; use App\Models\FormBuilder\FormSubmission; use App\Models\FormBuilder\FormSubmissionActionFailure; use App\Models\FormBuilder\FormSubmissionActionFailureRetryAttempt; @@ -30,6 +31,13 @@ use Throwable; * exception details, increment retry_count. Parent's own * exception_class / exception_message stay audit-immutable — * they represent the FIRST failure. + * + * v1.3-delta D2 (per ARCH-BINDINGS §7.1 v1.2 + RFC-WS-6 §Q3 v1.3 addition 2): + * - recordFailure now mirrors ApplyBindingsOnFormSubmit's outer-txn path: + * writes failure_response_code via FormBindingExceptionClassifier and + * apply_completed_at = now() (closes the asymmetry where the listener + * wrote this column on both happy and failure paths but the retry + * service only wrote it on the success path). */ final readonly class FormFailureRetryService { @@ -125,9 +133,18 @@ final readonly class FormFailureRetryService ->whereKey($failure->id) ->update(['retry_count' => DB::raw('retry_count + 1')]); + // Per ARCH-BINDINGS §7.1 v1.2 retry-service asymmetry note + + // RFC-WS-6 §Q3 v1.3 addition 2 — mirror ApplyBindingsOnFormSubmit's + // outer-transaction failure path: same failure_response_code + // classifier + apply_completed_at write. Single behaviour-change + // point per the v1.3-delta D1 design. FormSubmission::query() ->whereKey($submission->id) - ->update(['apply_status' => ApplyStatus::FAILED->value]); + ->update([ + 'apply_status' => ApplyStatus::FAILED->value, + 'apply_completed_at' => now(), + 'failure_response_code' => FormBindingExceptionClassifier::classify($e), + ]); return $attempt; }); diff --git a/api/bootstrap/app.php b/api/bootstrap/app.php index 5d702be5..d8978393 100644 --- a/api/bootstrap/app.php +++ b/api/bootstrap/app.php @@ -19,6 +19,11 @@ return Application::configure(basePath: dirname(__DIR__)) web: __DIR__.'/../routes/web.php', api: __DIR__.'/../routes/api.php', commands: __DIR__.'/../routes/console.php', + // Per RFC-WS-6 §Q1 v1.3 addition 2 — broadcast channel auth callbacks + // live in routes/channels.php. Registers Laravel's broadcasting auth + // middleware so private/presence channel subscriptions reach the + // closures defined there. + channels: __DIR__.'/../routes/channels.php', health: '/up', apiPrefix: 'api/v1', ) diff --git a/api/config/form_builder.php b/api/config/form_builder.php index a1b99e0f..ae31ebb6 100644 --- a/api/config/form_builder.php +++ b/api/config/form_builder.php @@ -84,4 +84,28 @@ return [ */ 'section_apply_enabled' => env('FORM_BUILDER_SECTION_APPLY', false), + /** + * FormBindingApplicator deadline in seconds. + * + * The wrapper around FormBindingApplicator::apply() throws + * FormBindingApplicatorTimeoutException if the call takes longer + * than this value. The exception is caught by + * ApplyBindingsOnFormSubmit's outer transaction handler and recorded + * as a form_submission_action_failures row with apply_status=FAILED, + * failure_response_code='temporary_error'. + * + * Default 5 seconds — typical apply() takes <100ms; the deadline + * catches the long tail (slow query against a massive person pool, + * lock-for-update wait that should have been resolved at the + * database level, etc.) before it hangs the public flow. + * + * Tune upward if a legitimate use-case surfaces (very large schemas + * with many bindings against multi-million-row entity tables). Tune + * downward if SLO requirements demand stricter response-time + * guarantees. + * + * Per RFC-WS-6 §Q1 v1.3 addition 4. + */ + 'apply_deadline_seconds' => env('FORM_BUILDER_APPLY_DEADLINE_SECONDS', 5), + ]; diff --git a/api/routes/channels.php b/api/routes/channels.php new file mode 100644 index 00000000..b6ec7bc2 --- /dev/null +++ b/api/routes/channels.php @@ -0,0 +1,57 @@ +withoutGlobalScopes() + ->find($submissionId); + + if ($submission === null) { + return false; + } + + // TODO TECH-CHANNEL-AUTH-ORG-ADMIN — extend to organisation admins + // once we audit the Spatie Permission helper for an + // organisation-scoped role check (hasRoleInOrganisation or + // similar). Until that audit lands, only the submitter has + // channel access. + return $submission->submitted_by_user_id === $user->id; + }, +); diff --git a/api/tests/Feature/Api/V1/Public/FormBuilder/IdentityMatchOnSubmitTest.php b/api/tests/Feature/Api/V1/Public/FormBuilder/IdentityMatchOnSubmitTest.php index 33de6b51..5f633b2a 100644 --- a/api/tests/Feature/Api/V1/Public/FormBuilder/IdentityMatchOnSubmitTest.php +++ b/api/tests/Feature/Api/V1/Public/FormBuilder/IdentityMatchOnSubmitTest.php @@ -4,9 +4,11 @@ declare(strict_types=1); namespace Tests\Feature\Api\V1\Public\FormBuilder; +use App\Enums\FormBuilder\ApplyStatus; use App\Enums\FormBuilder\FormFieldType; use App\Enums\FormBuilder\FormPurpose; use App\Events\FormBuilder\FormSubmissionSubmitted; +use App\Listeners\FormBuilder\TriggerPersonIdentityMatchOnFormSubmit; use App\Models\CrowdType; use App\Models\Event; use App\Models\FormBuilder\FormField; @@ -23,6 +25,12 @@ use Tests\TestCase; * ARCH §31.1 — TriggerPersonIdentityMatchOnFormSubmit contract. * Verifies: (a) only fires for event_registration, (b) matched/pending/ * none states written correctly, (c) listener failures never rethrow. + * + * D2 update: post-RFC-WS-6 v1.3, the listener is queued + gated on + * apply_status=COMPLETED. These tests bypass the full submit pipeline + * (no real bindings configured) and directly invoke the listener with + * apply_status=COMPLETED pre-set on the submission, mirroring the + * gate-passing happy-path output of ApplyBindingsOnFormSubmit. */ final class IdentityMatchOnSubmitTest extends TestCase { @@ -59,6 +67,31 @@ final class IdentityMatchOnSubmitTest extends TestCase ]); } + /** + * Build a submission with apply_status=COMPLETED pre-set so the v1.3 + * gate passes. Then invoke the listener directly — bypasses + * ApplyBindings (which would fail without a properly-configured + * schema with identity-key binding + default_crowd_type_id). + * + * @param array $overrides + */ + private function submitAndDispatch(array $overrides): FormSubmission + { + $submission = FormSubmission::factory()->create(array_merge([ + 'form_schema_id' => $this->schema->id, + 'status' => 'submitted', + 'submitted_at' => now(), + ], $overrides)); + + $submission->apply_status = ApplyStatus::COMPLETED; + $submission->save(); + + $listener = $this->app->make(TriggerPersonIdentityMatchOnFormSubmit::class); + $listener->handle(new FormSubmissionSubmitted($submission->fresh())); + + return $submission->fresh(); + } + public function test_event_registration_triggers_matched_when_person_has_user(): void { $user = User::factory()->create(); @@ -68,17 +101,12 @@ final class IdentityMatchOnSubmitTest extends TestCase 'user_id' => $user->id, ]); - $submission = FormSubmission::factory()->create([ - 'form_schema_id' => $this->schema->id, + $submission = $this->submitAndDispatch([ 'subject_type' => 'person', 'subject_id' => $person->id, - 'status' => 'submitted', - 'submitted_at' => now(), ]); - FormSubmissionSubmitted::dispatch($submission->fresh()); - - $this->assertSame('matched', $submission->fresh()->identity_match_status); + $this->assertSame('matched', $submission->identity_match_status); } public function test_event_registration_triggers_none_when_person_unlinked_no_match(): void @@ -92,17 +120,12 @@ final class IdentityMatchOnSubmitTest extends TestCase 'last_name' => 'NoMatch', ]); - $submission = FormSubmission::factory()->create([ - 'form_schema_id' => $this->schema->id, + $submission = $this->submitAndDispatch([ 'subject_type' => 'person', 'subject_id' => $person->id, - 'status' => 'submitted', - 'submitted_at' => now(), ]); - FormSubmissionSubmitted::dispatch($submission->fresh()); - - $this->assertSame('none', $submission->fresh()->identity_match_status); + $this->assertSame('none', $submission->identity_match_status); } public function test_event_registration_triggers_pending_when_matcher_finds_candidate(): void @@ -122,17 +145,12 @@ final class IdentityMatchOnSubmitTest extends TestCase 'last_name' => 'Match', ]); - $submission = FormSubmission::factory()->create([ - 'form_schema_id' => $this->schema->id, + $submission = $this->submitAndDispatch([ 'subject_type' => 'person', 'subject_id' => $person->id, - 'status' => 'submitted', - 'submitted_at' => now(), ]); - FormSubmissionSubmitted::dispatch($submission->fresh()); - - $this->assertSame('pending', $submission->fresh()->identity_match_status); + $this->assertSame('pending', $submission->identity_match_status); } public function test_non_event_registration_purpose_does_not_trigger(): void @@ -141,6 +159,7 @@ final class IdentityMatchOnSubmitTest extends TestCase 'organisation_id' => $this->org->id, 'purpose' => FormPurpose::INCIDENT_REPORT, ]); + $submission = FormSubmission::factory()->create([ 'form_schema_id' => $otherSchema->id, 'subject_type' => null, @@ -148,13 +167,24 @@ final class IdentityMatchOnSubmitTest extends TestCase 'status' => 'submitted', 'submitted_at' => now(), ]); + $submission->apply_status = ApplyStatus::COMPLETED; + $submission->save(); - FormSubmissionSubmitted::dispatch($submission->fresh()); + $listener = $this->app->make(TriggerPersonIdentityMatchOnFormSubmit::class); + $listener->handle(new FormSubmissionSubmitted($submission->fresh())); $this->assertNull($submission->fresh()->identity_match_status); } - public function test_public_submission_marked_pending(): void + /** + * v1.3 contract change: the failsafe-pad ("subject_id null → write + * 'pending' + log warning") is gone. With apply_status=COMPLETED but + * subject_type='person' AND subject_id=null, the listener throws + * IdentityMatchInvariantViolation per RFC §Q2. With subject_type=null + * (this test's scenario), the listener simply returns at the + * subject-type check without writing anything. + */ + public function test_public_submission_with_null_subject_does_not_write_status(): void { $submission = FormSubmission::factory()->create([ 'form_schema_id' => $this->schema->id, @@ -163,9 +193,12 @@ final class IdentityMatchOnSubmitTest extends TestCase 'status' => 'submitted', 'submitted_at' => now(), ]); + $submission->apply_status = ApplyStatus::COMPLETED; + $submission->save(); - FormSubmissionSubmitted::dispatch($submission->fresh()); + $listener = $this->app->make(TriggerPersonIdentityMatchOnFormSubmit::class); + $listener->handle(new FormSubmissionSubmitted($submission->fresh())); - $this->assertSame('pending', $submission->fresh()->identity_match_status); + $this->assertNull($submission->fresh()->identity_match_status); } } diff --git a/api/tests/Feature/FormBuilder/Bindings/FormBindingApplicatorDeadlineTest.php b/api/tests/Feature/FormBuilder/Bindings/FormBindingApplicatorDeadlineTest.php new file mode 100644 index 00000000..c8b0700a --- /dev/null +++ b/api/tests/Feature/FormBuilder/Bindings/FormBindingApplicatorDeadlineTest.php @@ -0,0 +1,149 @@ +app->make(FormBindingApplicator::class); + $clone = $applicator->withDeadline(5); + + $this->assertInstanceOf(FormBindingApplicator::class, $clone); + $this->assertNotSame($applicator, $clone, 'withDeadline must return a clone, not mutate the receiver'); + } + + public function test_apply_without_deadline_does_not_throw_for_fast_runs(): void + { + $submission = $this->makeAnonymousSubmission(); + $applicator = $this->app->make(FormBindingApplicator::class); + + DB::transaction(function () use ($applicator, $submission): void { + $result = $applicator->apply($submission); + $this->assertInstanceOf(BindingPassResult::class, $result); + }); + } + + public function test_apply_with_generous_deadline_does_not_throw(): void + { + $submission = $this->makeAnonymousSubmission(); + $applicator = $this->app->make(FormBindingApplicator::class); + + DB::transaction(function () use ($applicator, $submission): void { + $result = $applicator->withDeadline(60)->apply($submission); + $this->assertInstanceOf(BindingPassResult::class, $result); + }); + } + + public function test_apply_throws_timeout_when_elapsed_exceeds_deadline(): void + { + $submission = $this->makeAnonymousSubmission(); + $applicator = new SlowApplicator( + $this->app->make(\App\FormBuilder\Purposes\PurposeRegistry::class), + $this->app->make(\App\FormBuilder\Bindings\BindingConflictResolver::class), + $this->app->make(\App\FormBuilder\Bindings\BindingTypeRegistry::class), + $this->app->make(\App\FormBuilder\Bindings\BindingActivityLogger::class), + ); + $applicator->sleepMicroseconds = 50_000; // 50ms + + $this->expectException(FormBindingApplicatorTimeoutException::class); + $this->expectExceptionMessageMatches('/exceeded deadline of 0s/'); + + DB::transaction(function () use ($applicator, $submission): void { + // 0 seconds — any wall-clock elapsed should exceed it. + $applicator->withDeadline(0)->apply($submission); + }); + } + + public function test_timeout_exception_carries_submission_id_and_reason_code(): void + { + $submission = $this->makeAnonymousSubmission(); + $applicator = new SlowApplicator( + $this->app->make(\App\FormBuilder\Purposes\PurposeRegistry::class), + $this->app->make(\App\FormBuilder\Bindings\BindingConflictResolver::class), + $this->app->make(\App\FormBuilder\Bindings\BindingTypeRegistry::class), + $this->app->make(\App\FormBuilder\Bindings\BindingActivityLogger::class), + ); + $applicator->sleepMicroseconds = 50_000; + + $caught = null; + try { + DB::transaction(function () use ($applicator, $submission): void { + $applicator->withDeadline(0)->apply($submission); + }); + } catch (FormBindingApplicatorTimeoutException $e) { + $caught = $e; + } + + $this->assertNotNull($caught); + $this->assertSame((string) $submission->id, $caught->submissionId); + // Inherited via FormBindingInfraException — Timeout extends Infra. + $this->assertSame('temporary_error', $caught->reasonCode()); + } + + /** + * Anonymous incident-report submission: the applicator's + * IncidentReportSubjectResolver returns null when submitted_by_user_id + * is null, and the applicator's anonymous-allowed branch returns an + * empty BindingPassResult. The deadline check still fires at the end. + */ + private function makeAnonymousSubmission(): FormSubmission + { + $schema = FormSchema::factory()->create([ + 'purpose' => FormPurpose::INCIDENT_REPORT->value, + ]); + + $submission = FormSubmission::factory()->create([ + 'form_schema_id' => $schema->id, + 'subject_type' => null, + 'subject_id' => null, + 'submitted_by_user_id' => null, + ]); + + return $submission->fresh(); + } +} + +/** + * Test double — sleeps inside apply() to force the deadline check at + * the end of apply() to fail for any reasonable deadline value (incl. 0). + */ +final class SlowApplicator extends FormBindingApplicator +{ + public int $sleepMicroseconds = 0; + + public function apply(FormSubmission $submission, ?string $sectionId = null): BindingPassResult + { + usleep($this->sleepMicroseconds); + + return parent::apply($submission, $sectionId); + } +} diff --git a/api/tests/Feature/FormBuilder/Bindings/RetryServiceFailureClassifierTest.php b/api/tests/Feature/FormBuilder/Bindings/RetryServiceFailureClassifierTest.php new file mode 100644 index 00000000..cbe0ff5c --- /dev/null +++ b/api/tests/Feature/FormBuilder/Bindings/RetryServiceFailureClassifierTest.php @@ -0,0 +1,160 @@ +create(); + $schema = FormSchema::factory()->create([ + 'organisation_id' => $org->id, + 'purpose' => FormPurpose::EVENT_REGISTRATION->value, + ]); + $submission = FormSubmission::factory()->create([ + 'form_schema_id' => $schema->id, + 'organisation_id' => $org->id, + ]); + + return FormSubmissionActionFailure::factory() + ->for($submission, 'submission') + ->create([ + 'exception_class' => 'OriginalException', + 'exception_message' => 'first failure message', + ]); + } + + /** + * @param callable(FormSubmission, ?string): BindingPassResult $apply + */ + private function bindApplicator(callable $apply): void + { + $stub = new class($apply) extends FormBindingApplicator + { + /** @var callable(FormSubmission, ?string): BindingPassResult */ + private $apply; + + /** @param callable(FormSubmission, ?string): BindingPassResult $apply */ + public function __construct(callable $apply) + { + $this->apply = $apply; + } + + public function apply(FormSubmission $submission, ?string $sectionId = null): BindingPassResult + { + return ($this->apply)($submission, $sectionId); + } + }; + $this->app->instance(FormBindingApplicator::class, $stub); + } + + private function retryWithThrow(callable $throwFn): FormSubmissionActionFailure + { + $failure = $this->makeFailure(); + $this->bindApplicator(fn () => $throwFn()); + + $service = $this->app->make(FormFailureRetryService::class); + $result = $service->retry($failure); + + $this->assertSame('failed', $result['outcome']); + + return $failure->fresh(); + } + + public function test_record_failure_writes_failure_response_code_for_schema_config(): void + { + $failure = $this->retryWithThrow( + fn () => throw new FormBindingSchemaConfigException(submissionId: 'x', message: 'cfg'), + ); + + $submission = FormSubmission::query()->withoutGlobalScopes()->find($failure->form_submission_id); + $this->assertSame('schema_config_error', $submission->failure_response_code); + $this->assertSame(ApplyStatus::FAILED, $submission->apply_status); + } + + public function test_record_failure_writes_failure_response_code_for_infra(): void + { + $failure = $this->retryWithThrow( + fn () => throw new FormBindingInfraException(submissionId: 'x', message: 'infra'), + ); + + $submission = FormSubmission::query()->withoutGlobalScopes()->find($failure->form_submission_id); + $this->assertSame('temporary_error', $submission->failure_response_code); + } + + public function test_record_failure_writes_failure_response_code_for_timeout(): void + { + // Timeout extends Infra → temporary_error inherited. + $failure = $this->retryWithThrow( + fn () => throw new FormBindingApplicatorTimeoutException(submissionId: 'x', message: 'deadline'), + ); + + $submission = FormSubmission::query()->withoutGlobalScopes()->find($failure->form_submission_id); + $this->assertSame('temporary_error', $submission->failure_response_code); + } + + public function test_record_failure_writes_failure_response_code_for_data_integrity(): void + { + $failure = $this->retryWithThrow( + fn () => throw new FormBindingDataIntegrityException(submissionId: 'x', message: 'fk'), + ); + + $submission = FormSubmission::query()->withoutGlobalScopes()->find($failure->form_submission_id); + $this->assertSame('data_integrity_error', $submission->failure_response_code); + } + + public function test_record_failure_writes_unknown_error_for_arbitrary_throwable(): void + { + $failure = $this->retryWithThrow(fn () => throw new RuntimeException('arbitrary')); + + $submission = FormSubmission::query()->withoutGlobalScopes()->find($failure->form_submission_id); + $this->assertSame('unknown_error', $submission->failure_response_code); + } + + public function test_record_failure_writes_apply_completed_at_symmetry_fix(): void + { + // Per ARCH-BINDINGS §7.1 v1.2 retry-service asymmetry note — + // pre-D2, recordFailure did NOT write apply_completed_at; only + // recordSuccess did. D2 closes the asymmetry. + $failure = $this->retryWithThrow( + fn () => throw new FormBindingInfraException(submissionId: 'x', message: 'transient'), + ); + + $submission = FormSubmission::query()->withoutGlobalScopes()->find($failure->form_submission_id); + $this->assertNotNull( + $submission->apply_completed_at, + 'apply_completed_at must be set on the failure path; pre-D2 only the success path wrote it.', + ); + } +} diff --git a/api/tests/Feature/FormBuilder/Channels/SubmissionChannelAuthTest.php b/api/tests/Feature/FormBuilder/Channels/SubmissionChannelAuthTest.php new file mode 100644 index 00000000..b55616db --- /dev/null +++ b/api/tests/Feature/FormBuilder/Channels/SubmissionChannelAuthTest.php @@ -0,0 +1,119 @@ +id}"; + + foreach ($channels as $pattern => $callback) { + // Pattern is 'submission.{submissionId}'; convert to a regex + // so we can match the concrete channel name. + $regex = '/^'.str_replace(['{submissionId}', '.'], ['([^.]+)', '\\.'], $pattern).'$/'; + if (! preg_match($regex, $name, $matches)) { + continue; + } + $result = $callback($user, $matches[1]); + + return (bool) $result; + } + + $this->fail("No channel callback registered for {$name}"); + } + + private function makeSubmission(?User $submitter): FormSubmission + { + $org = Organisation::factory()->create(); + $schema = FormSchema::factory()->create(['organisation_id' => $org->id]); + + return FormSubmission::factory()->create([ + 'form_schema_id' => $schema->id, + 'organisation_id' => $org->id, + 'submitted_by_user_id' => $submitter?->id, + ]); + } + + public function test_submitter_is_authorised(): void + { + $submitter = User::factory()->create(); + $submission = $this->makeSubmission($submitter); + + $this->assertTrue($this->authoriseSubscription($submitter, $submission)); + } + + public function test_other_authenticated_user_is_denied(): void + { + $submitter = User::factory()->create(); + $other = User::factory()->create(); + $submission = $this->makeSubmission($submitter); + + $this->assertFalse($this->authoriseSubscription($other, $submission)); + } + + public function test_subscription_is_denied_when_submission_does_not_exist(): void + { + $user = User::factory()->create(); + + // Build a submission, then delete it so the FK lookup returns null. + $submission = $this->makeSubmission($user); + $submissionId = (string) $submission->id; + $submission->forceDelete(); + + // Reuse the submitter's User and a fake submission shell. + $shell = new FormSubmission; + $shell->id = $submissionId; + + $this->assertFalse($this->authoriseSubscription($user, $shell)); + } + + public function test_org_admin_is_currently_denied_per_backlog_entry(): void + { + // Locks the v1 contract: even an org admin of the submission's + // organisation is denied because the canonical Spatie helper + // hasn't been audited yet. See BACKLOG entry + // TECH-CHANNEL-AUTH-ORG-ADMIN. When that lands, this test should + // flip to expectTrue() in the same PR as the auth-callback + // extension. + $orgAdmin = User::factory()->create(); + $submitter = User::factory()->create(); + $submission = $this->makeSubmission($submitter); + + $this->assertFalse( + $this->authoriseSubscription($orgAdmin, $submission), + 'V1 contract: org admins are NOT authorised. Flip with TECH-CHANNEL-AUTH-ORG-ADMIN.', + ); + } +} diff --git a/api/tests/Feature/FormBuilder/FormSubmissionResourceIdentityMatchTest.php b/api/tests/Feature/FormBuilder/FormSubmissionResourceIdentityMatchTest.php index 15cb9b97..5bc8cdf7 100644 --- a/api/tests/Feature/FormBuilder/FormSubmissionResourceIdentityMatchTest.php +++ b/api/tests/Feature/FormBuilder/FormSubmissionResourceIdentityMatchTest.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Tests\Feature\FormBuilder; +use App\Enums\FormBuilder\FormPurpose; use App\Enums\FormBuilder\FormSubmissionStatus; use App\Http\Resources\FormBuilder\FormSubmissionResource; use App\Models\FormBuilder\FormSchema; @@ -13,6 +14,7 @@ use App\Models\User; use Database\Seeders\RoleSeeder; use Illuminate\Foundation\Testing\RefreshDatabase; use Laravel\Sanctum\Sanctum; +use PHPUnit\Framework\Attributes\DataProvider; use Tests\TestCase; /** @@ -97,4 +99,53 @@ final class FormSubmissionResourceIdentityMatchTest extends TestCase $response->assertOk() ->assertJsonPath('data.identity_match.status', 'matched'); } + + /** + * Per RFC-WS-6 §Q2 v1.3 — for non-person purposes the identity_match + * block is null. ApplyBindings only writes identity_match_status='pending' + * when the subject is a person; non-person purposes leave the column + * NULL and the resource emits null. This test pins the contract per + * non-person purpose so a future refactor can't silently drift. + * + * `event_registration` is intentionally excluded — that purpose IS + * person-typed and identity_match is non-null after ApplyBindings runs. + * + * @return array + */ + public static function nonPersonPurposes(): array + { + return [ + 'signature_contract' => [FormPurpose::SIGNATURE_CONTRACT], + 'user_profile' => [FormPurpose::USER_PROFILE], + 'incident_report' => [FormPurpose::INCIDENT_REPORT], + 'post_event_evaluation' => [FormPurpose::POST_EVENT_EVALUATION], + 'supplier_intake' => [FormPurpose::SUPPLIER_INTAKE], + 'artist_advance' => [FormPurpose::ARTIST_ADVANCE], + ]; + } + + #[DataProvider('nonPersonPurposes')] + public function test_identity_match_is_null_for_non_person_purpose(FormPurpose $purpose): void + { + $schema = FormSchema::factory()->create([ + 'organisation_id' => $this->org->id, + 'purpose' => $purpose->value, + ]); + + $submission = FormSubmission::create([ + 'form_schema_id' => $schema->id, + 'status' => FormSubmissionStatus::SUBMITTED->value, + 'submitted_at' => now(), + 'is_test' => false, + // identity_match_status intentionally not set; for non-person + // purposes the column stays NULL. + ]); + + $array = $this->toArray($submission); + + $this->assertNull( + $array['identity_match'], + "FormSubmissionResource.identity_match must be null for non-person purpose '{$purpose->value}' per RFC-WS-6 §Q2 v1.3.", + ); + } } diff --git a/api/tests/Feature/FormBuilder/Integration/TagPickerSyncListenerTest.php b/api/tests/Feature/FormBuilder/Integration/TagPickerSyncListenerTest.php index 35df4f12..3ba84a02 100644 --- a/api/tests/Feature/FormBuilder/Integration/TagPickerSyncListenerTest.php +++ b/api/tests/Feature/FormBuilder/Integration/TagPickerSyncListenerTest.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Tests\Feature\FormBuilder\Integration; +use App\Enums\FormBuilder\ApplyStatus; use App\Enums\FormBuilder\FormFieldType; use App\Enums\FormBuilder\FormPurpose; use App\Enums\FormBuilder\FormSubmissionStatus; @@ -11,6 +12,7 @@ use App\Enums\IdentityMatchConfidence; use App\Enums\IdentityMatchMethod; use App\Enums\IdentityMatchStatus; use App\Events\FormBuilder\FormSubmissionSubmitted; +use App\Listeners\FormBuilder\SyncTagPickerSelectionsOnSubmit; use App\Models\CrowdType; use App\Models\Event; use App\Models\FormBuilder\FormField; @@ -237,9 +239,17 @@ final class TagPickerSyncListenerTest extends TestCase $value->value = $tagIds; $value->save(); - // Fire the event manually (we bypass the service during this test - // to isolate the listener contract). - FormSubmissionSubmitted::dispatch($submission->fresh()); + // Per ARCH-BINDINGS §5.6 — SyncTagPickerSelectionsOnSubmit is gated + // on apply_status === COMPLETED. These integration tests bypass the + // applicator (the schema has no bindings configured), so we set the + // gate-passing state explicitly and invoke the listener under test + // directly. Mirrors the original "bypass the service to isolate the + // listener contract" intent. + $submission->apply_status = ApplyStatus::COMPLETED; + $submission->save(); + + $listener = $this->app->make(SyncTagPickerSelectionsOnSubmit::class); + $listener->handle(new FormSubmissionSubmitted($submission->fresh())); return $submission; } diff --git a/api/tests/Feature/FormBuilder/Listeners/ApplyBindingsOnFormSubmitTest.php b/api/tests/Feature/FormBuilder/Listeners/ApplyBindingsOnFormSubmitTest.php index 081c5f28..451918d3 100644 --- a/api/tests/Feature/FormBuilder/Listeners/ApplyBindingsOnFormSubmitTest.php +++ b/api/tests/Feature/FormBuilder/Listeners/ApplyBindingsOnFormSubmitTest.php @@ -9,6 +9,11 @@ use App\Enums\FormBuilder\FormFieldBindingMergeStrategy; use App\Enums\FormBuilder\FormFieldType; use App\Enums\FormBuilder\FormPurpose; use App\Events\FormBuilder\FormSubmissionSubmitted; +use App\Exceptions\FormBuilder\FormBindingApplicatorTimeoutException; +use App\Exceptions\FormBuilder\FormBindingDataIntegrityException; +use App\Exceptions\FormBuilder\FormBindingInfraException; +use App\Exceptions\FormBuilder\FormBindingSchemaConfigException; +use App\FormBuilder\Bindings\BindingPassResult; use App\FormBuilder\Bindings\FormBindingApplicator; use App\Listeners\FormBuilder\ApplyBindingsOnFormSubmit; use App\Models\CrowdType; @@ -19,8 +24,8 @@ use App\Models\FormBuilder\FormSchema; use App\Models\FormBuilder\FormSubmission; use App\Models\FormBuilder\FormSubmissionActionFailure; use App\Models\FormBuilder\FormValue; -use App\FormBuilder\Bindings\BindingPassResult; use Illuminate\Foundation\Testing\RefreshDatabase; +use Illuminate\Support\Facades\Config; use Illuminate\Support\Facades\Log; use Tests\TestCase; @@ -40,11 +45,129 @@ final class ApplyBindingsOnFormSubmitTest extends TestCase $this->assertNotNull($reloaded->apply_completed_at); } + public function test_writes_initial_pending_to_identity_match_status_for_person_subject(): void + { + // Per RFC-WS-6 §Q1 v1.3 addition 1 — ApplyBindings writes the + // initial 'pending' identity_match_status inside its inner + // transaction so the HTTP response carries the right state for + // the IdentityMatchBanner first-paint copy. + $submission = $this->makeSubmission(); + + $listener = $this->app->make(ApplyBindingsOnFormSubmit::class); + $listener->handle(new FormSubmissionSubmitted($submission)); + + $reloaded = FormSubmission::query()->withoutGlobalScopes()->find($submission->id); + $this->assertSame('pending', $reloaded->identity_match_status); + } + + public function test_writes_apply_completed_at_on_failure(): void + { + $submission = $this->makeSubmission(); + + $applicator = $this->throwingApplicator(\RuntimeException::class, 'boom'); + + Log::shouldReceive('error')->once(); + + $listener = new ApplyBindingsOnFormSubmit($applicator); + $listener->handle(new FormSubmissionSubmitted($submission)); + + $reloaded = FormSubmission::query()->withoutGlobalScopes()->find($submission->id); + $this->assertSame(ApplyStatus::FAILED, $reloaded->apply_status); + $this->assertNotNull($reloaded->apply_completed_at); + } + + public function test_writes_failure_response_code_for_schema_config_exception(): void + { + $this->assertFailureResponseCode( + FormBindingSchemaConfigException::class, + 'schema_config_error', + ); + } + + public function test_writes_failure_response_code_for_infra_exception(): void + { + $this->assertFailureResponseCode( + FormBindingInfraException::class, + 'temporary_error', + ); + } + + public function test_writes_failure_response_code_for_data_integrity_exception(): void + { + $this->assertFailureResponseCode( + FormBindingDataIntegrityException::class, + 'data_integrity_error', + ); + } + + public function test_writes_unknown_error_for_arbitrary_throwable(): void + { + $submission = $this->makeSubmission(); + $applicator = $this->throwingApplicator(\RuntimeException::class, 'boom'); + + Log::shouldReceive('error')->once(); + + $listener = new ApplyBindingsOnFormSubmit($applicator); + $listener->handle(new FormSubmissionSubmitted($submission)); + + $reloaded = FormSubmission::query()->withoutGlobalScopes()->find($submission->id); + $this->assertSame('unknown_error', $reloaded->failure_response_code); + } + + public function test_writes_temporary_error_when_deadline_exceeds(): void + { + // Per RFC-WS-6 §Q1 v1.3 addition 4 — wrapper throws Timeout, which + // extends FormBindingInfraException, so failure_response_code + // resolves to 'temporary_error' via the classifier. + $this->assertFailureResponseCode( + FormBindingApplicatorTimeoutException::class, + 'temporary_error', + ); + } + + public function test_deadline_wrapper_is_invoked_with_config_value(): void + { + // Drive the applicator clone via a test double that asserts the + // deadline value reached withDeadline(). + Config::set('form_builder.apply_deadline_seconds', 3); + + $submission = $this->makeSubmission(); + $applicator = new DeadlineRecordingApplicator( + $this->app->make(\App\FormBuilder\Purposes\PurposeRegistry::class), + $this->app->make(\App\FormBuilder\Bindings\BindingConflictResolver::class), + $this->app->make(\App\FormBuilder\Bindings\BindingTypeRegistry::class), + $this->app->make(\App\FormBuilder\Bindings\BindingActivityLogger::class), + ); + + $listener = new ApplyBindingsOnFormSubmit($applicator); + $listener->handle(new FormSubmissionSubmitted($submission)); + + $this->assertSame(3, DeadlineRecordingApplicator::$lastDeadline); + } + + public function test_outer_transaction_writes_failure_record_on_inner_rollback(): void + { + $submission = $this->makeSubmission(); + $applicator = $this->throwingApplicator(FormBindingSchemaConfigException::class, 'no schema'); + + Log::shouldReceive('error')->once(); + + $listener = new ApplyBindingsOnFormSubmit($applicator); + $listener->handle(new FormSubmissionSubmitted($submission)); + + $failure = FormSubmissionActionFailure::query() + ->where('form_submission_id', $submission->id) + ->first(); + + $this->assertNotNull($failure, 'Outer transaction must write the failure record even after the inner rollback'); + $this->assertSame(FormBindingSchemaConfigException::class, $failure->exception_class); + } + public function test_exception_path_records_failure_and_marks_failed(): void { $submission = $this->makeSubmission(); - $applicator = $this->throwingApplicator(); + $applicator = $this->throwingApplicator(\RuntimeException::class, 'boom'); Log::shouldReceive('error')->once(); @@ -66,7 +189,7 @@ final class ApplyBindingsOnFormSubmitTest extends TestCase { $submission = $this->makeSubmission(); - $applicator = $this->throwingApplicator(); + $applicator = $this->throwingApplicator(\RuntimeException::class, 'boom'); Log::shouldReceive('error')->once(); @@ -81,14 +204,33 @@ final class ApplyBindingsOnFormSubmitTest extends TestCase $this->assertFalse($threw, 'Listener must swallow throws so siblings keep running'); } - private function throwingApplicator(): ThrowingApplicator + private function assertFailureResponseCode(string $exceptionClass, string $expectedCode): void { - return new ThrowingApplicator( + $submission = $this->makeSubmission(); + $applicator = $this->throwingApplicator($exceptionClass, 'test'); + + Log::shouldReceive('error')->once(); + + $listener = new ApplyBindingsOnFormSubmit($applicator); + $listener->handle(new FormSubmissionSubmitted($submission)); + + $reloaded = FormSubmission::query()->withoutGlobalScopes()->find($submission->id); + $this->assertSame(ApplyStatus::FAILED, $reloaded->apply_status); + $this->assertSame($expectedCode, $reloaded->failure_response_code); + } + + private function throwingApplicator(string $exceptionClass, string $message): ThrowingApplicator + { + $applicator = new ThrowingApplicator( $this->app->make(\App\FormBuilder\Purposes\PurposeRegistry::class), $this->app->make(\App\FormBuilder\Bindings\BindingConflictResolver::class), $this->app->make(\App\FormBuilder\Bindings\BindingTypeRegistry::class), $this->app->make(\App\FormBuilder\Bindings\BindingActivityLogger::class), ); + $applicator->exceptionClass = $exceptionClass; + $applicator->message = $message; + + return $applicator; } private function makeSubmission(): FormSubmission @@ -156,7 +298,7 @@ final class ApplyBindingsOnFormSubmitTest extends TestCase private function writeValue(string $submissionId, string $fieldId, mixed $value): void { - $row = new FormValue(); + $row = new FormValue; $row->form_submission_id = $submissionId; $row->form_field_id = $fieldId; $row->setAttribute('value', $value); @@ -192,10 +334,53 @@ final class ApplyBindingsOnFormSubmitTest extends TestCase } } +/** + * Test double — apply() throws an instance of the configured exception + * class. Subclass of FormBindingApplicator so withDeadline() (which + * returns clone $this) preserves the override. + */ final class ThrowingApplicator extends FormBindingApplicator { + public string $exceptionClass = \RuntimeException::class; + + public string $message = 'boom'; + public function apply(FormSubmission $submission, ?string $sectionId = null): BindingPassResult { - throw new \RuntimeException('boom'); + $class = $this->exceptionClass; + + // FormBindingApplicatorException subclasses use named-arg constructor; + // generic Throwables use the standard message ctor. + if (is_subclass_of($class, \App\Exceptions\FormBuilder\FormBindingApplicatorException::class)) { + throw new $class(submissionId: (string) $submission->id, message: $this->message); + } + + throw new $class($this->message); + } +} + +/** + * Test double — captures the deadline value passed via withDeadline(). + * Used by test_deadline_wrapper_is_invoked_with_config_value. + */ +final class DeadlineRecordingApplicator extends FormBindingApplicator +{ + public static ?int $lastDeadline = null; + + public function withDeadline(int $seconds): self + { + self::$lastDeadline = $seconds; + + return $this; + } + + public function apply(FormSubmission $submission, ?string $sectionId = null): BindingPassResult + { + return new BindingPassResult( + formSubmissionId: (string) $submission->id, + provisionedSubjectType: null, + provisionedSubjectId: null, + applications: [], + ); } } diff --git a/api/tests/Feature/FormBuilder/Listeners/FormSubmissionSubmittedListenerOrderTest.php b/api/tests/Feature/FormBuilder/Listeners/FormSubmissionSubmittedListenerOrderTest.php index 68799002..8d52855f 100644 --- a/api/tests/Feature/FormBuilder/Listeners/FormSubmissionSubmittedListenerOrderTest.php +++ b/api/tests/Feature/FormBuilder/Listeners/FormSubmissionSubmittedListenerOrderTest.php @@ -10,12 +10,23 @@ use App\Listeners\FormBuilder\SyncTagPickerSelectionsOnSubmit; use App\Listeners\FormBuilder\TriggerPersonIdentityMatchOnFormSubmit; use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Support\Facades\Event; +use ReflectionClass; use Tests\TestCase; /** - * RFC-WS-6 §3 (Q1) — listener order on FormSubmissionSubmitted is - * load-bearing: ApplyBindings sync (1st) → IdentityMatch sync (2nd) → - * queued siblings parallel. + * RFC-WS-6 v1.3 §Q1 — listener layout on FormSubmissionSubmitted is + * load-bearing. + * + * v1.3 layout: ApplyBindingsOnFormSubmit is the only sync listener; + * every other listener implements ShouldQueue and is gated on + * apply_status=COMPLETED per ARCH-BINDINGS §5.6. + * + * v1.0/v1.2 layout had two sync listeners (ApplyBindings then + * TriggerPersonIdentityMatch); the v1.3 review (2026-05-07) reduced the + * sync chain to one. The assertions below lock the v1.3 layout so + * a future contributor cannot silently revert TriggerPersonIdentityMatch + * to sync, or queue ApplyBindings, or omit the gating-invariant on a + * new queued listener. */ final class FormSubmissionSubmittedListenerOrderTest extends TestCase { @@ -41,28 +52,70 @@ final class FormSubmissionSubmittedListenerOrderTest extends TestCase public function test_apply_bindings_listener_is_synchronous(): void { - $reflection = new \ReflectionClass(ApplyBindingsOnFormSubmit::class); + $reflection = new ReflectionClass(ApplyBindingsOnFormSubmit::class); $this->assertFalse( $reflection->implementsInterface(ShouldQueue::class), - 'ApplyBindingsOnFormSubmit must be sync (no ShouldQueue)', + 'ApplyBindingsOnFormSubmit must be sync (no ShouldQueue) — subject_id must land in the HTTP response.', ); } - public function test_identity_match_listener_is_synchronous(): void + public function test_identity_match_listener_is_queued(): void { - $reflection = new \ReflectionClass(TriggerPersonIdentityMatchOnFormSubmit::class); - $this->assertFalse( + // v1.3 flip — was sync in v1.0/v1.2. RFC-WS-6 §Q1 v1.3. + $reflection = new ReflectionClass(TriggerPersonIdentityMatchOnFormSubmit::class); + $this->assertTrue( $reflection->implementsInterface(ShouldQueue::class), - 'TriggerPersonIdentityMatchOnFormSubmit must be sync (no ShouldQueue)', + 'TriggerPersonIdentityMatchOnFormSubmit must be queued (ShouldQueue) per RFC-WS-6 v1.3 §Q1.', ); } public function test_tag_picker_sync_listener_is_queued(): void { - $reflection = new \ReflectionClass(SyncTagPickerSelectionsOnSubmit::class); + $reflection = new ReflectionClass(SyncTagPickerSelectionsOnSubmit::class); $this->assertTrue( $reflection->implementsInterface(ShouldQueue::class), 'SyncTagPickerSelectionsOnSubmit must be queued (ShouldQueue)', ); } + + /** + * Structural guard: every queued listener attached to + * FormSubmissionSubmitted must include the apply_status=COMPLETED + * gate as an early statement of handle(). + * + * Implementation: file-content scan rather than full AST parsing — + * good enough as a regression guard, deliberately fragile if someone + * tries to be clever (e.g. comparing via a helper method). Keep the + * guard simple and let it fail loudly when the canonical pattern + * drifts; rewrite both the listener and this test together if you + * deliberately change the pattern. + */ + public function test_every_queued_listener_has_apply_status_completed_gate(): void + { + $listeners = Event::getRawListeners()[FormSubmissionSubmitted::class] ?? []; + $listenerClasses = array_values(array_filter(array_map( + static fn ($listener): ?string => is_string($listener) ? $listener : null, + $listeners, + ))); + + foreach ($listenerClasses as $class) { + $reflection = new ReflectionClass($class); + if (! $reflection->implementsInterface(ShouldQueue::class)) { + continue; + } + + $source = file_get_contents($reflection->getFileName()); + + $this->assertStringContainsString( + 'apply_status !== ApplyStatus::COMPLETED', + (string) $source, + "Queued listener {$class} must include the apply_status=COMPLETED gating-invariant per ARCH-BINDINGS §5.6.", + ); + $this->assertStringContainsString( + 'form-builder.queued-listener.skipped_apply_failed', + (string) $source, + "Queued listener {$class} must log the canonical skip event for triage visibility.", + ); + } + } } diff --git a/api/tests/Feature/FormBuilder/Listeners/SyncTagPickerSelectionsOnSubmitGateTest.php b/api/tests/Feature/FormBuilder/Listeners/SyncTagPickerSelectionsOnSubmitGateTest.php new file mode 100644 index 00000000..263b461c --- /dev/null +++ b/api/tests/Feature/FormBuilder/Listeners/SyncTagPickerSelectionsOnSubmitGateTest.php @@ -0,0 +1,147 @@ +org = Organisation::factory()->create(); + $this->event = Event::factory()->create(['organisation_id' => $this->org->id]); + $this->crowdType = CrowdType::factory()->systemType('CREW')->create([ + 'organisation_id' => $this->org->id, + ]); + $this->schema = FormSchema::factory()->create([ + 'organisation_id' => $this->org->id, + 'purpose' => FormPurpose::EVENT_REGISTRATION, + 'owner_type' => 'event', + 'owner_id' => $this->event->id, + ]); + } + + private function makeSubmission(?ApplyStatus $applyStatus): FormSubmission + { + $person = Person::factory()->create([ + 'event_id' => $this->event->id, + 'crowd_type_id' => $this->crowdType->id, + ]); + + $submission = FormSubmission::create([ + 'form_schema_id' => $this->schema->id, + 'subject_type' => 'person', + 'subject_id' => $person->id, + 'status' => FormSubmissionStatus::SUBMITTED->value, + 'submitted_at' => now(), + 'is_test' => false, + ]); + + if ($applyStatus !== null) { + $submission->apply_status = $applyStatus; + $submission->save(); + } + + return $submission->fresh(); + } + + private function dispatchListener(FormSubmission $submission): void + { + $listener = $this->app->make(SyncTagPickerSelectionsOnSubmit::class); + $listener->handle(new FormSubmissionSubmitted($submission)); + } + + private function assertSkipLogged(?ApplyStatus $applyStatus): void + { + // Spy on Log so we can pick out the canonical skip-log line + // without strict-mocking other Log calls (siblings may log too). + $logSpy = Log::spy(); + + $submission = $this->makeSubmission($applyStatus); + $this->dispatchListener($submission); + + $logSpy->shouldHaveReceived( + 'info', + [ + 'form-builder.queued-listener.skipped_apply_failed', + Mockery::on(static fn (array $context): bool => ($context['listener'] ?? null) === SyncTagPickerSelectionsOnSubmit::class), + ], + ); + } + + public function test_skips_when_apply_status_is_null(): void + { + $this->assertSkipLogged(null); + } + + public function test_skips_when_apply_status_is_pending(): void + { + $this->assertSkipLogged(ApplyStatus::PENDING); + } + + public function test_skips_when_apply_status_is_partial(): void + { + // PARTIAL is treated identically to FAILED per ARCH-BINDINGS §5.6. + $this->assertSkipLogged(ApplyStatus::PARTIAL); + } + + public function test_skips_when_apply_status_is_failed(): void + { + $this->assertSkipLogged(ApplyStatus::FAILED); + } + + public function test_does_not_log_skip_when_apply_status_is_completed(): void + { + // When the gate passes, the canonical skip-log line must NOT + // fire. Other Log calls (e.g. tag-sync.deferred when person.user_id + // is null) may still fire and are out of scope here. + $logSpy = Log::spy(); + + $submission = $this->makeSubmission(ApplyStatus::COMPLETED); + $this->dispatchListener($submission); + + $logSpy->shouldNotHaveReceived('info', [ + 'form-builder.queued-listener.skipped_apply_failed', + Mockery::any(), + ]); + } +} diff --git a/api/tests/Feature/FormBuilder/Listeners/TriggerPersonIdentityMatchOnFormSubmitTest.php b/api/tests/Feature/FormBuilder/Listeners/TriggerPersonIdentityMatchOnFormSubmitTest.php index 5123834a..f995f988 100644 --- a/api/tests/Feature/FormBuilder/Listeners/TriggerPersonIdentityMatchOnFormSubmitTest.php +++ b/api/tests/Feature/FormBuilder/Listeners/TriggerPersonIdentityMatchOnFormSubmitTest.php @@ -4,13 +4,15 @@ declare(strict_types=1); namespace Tests\Feature\FormBuilder\Listeners; -use App\Enums\FormBuilder\FormFieldType; +use App\Enums\FormBuilder\ApplyStatus; use App\Enums\FormBuilder\FormPurpose; use App\Enums\FormBuilder\FormSubmissionStatus; +use App\Events\FormBuilder\FormSubmissionIdentityMatchResolved; use App\Events\FormBuilder\FormSubmissionSubmitted; +use App\Exceptions\FormBuilder\IdentityMatchInvariantViolation; +use App\Listeners\FormBuilder\TriggerPersonIdentityMatchOnFormSubmit; use App\Models\CrowdType; use App\Models\Event; -use App\Models\FormBuilder\FormField; use App\Models\FormBuilder\FormSchema; use App\Models\FormBuilder\FormSubmission; use App\Models\Organisation; @@ -18,20 +20,20 @@ use App\Models\Person; use App\Models\User; use Database\Seeders\RoleSeeder; use Illuminate\Foundation\Testing\RefreshDatabase; -use Illuminate\Support\Facades\Config; -use Illuminate\Support\Str; +use Illuminate\Support\Facades\Broadcast; use Tests\TestCase; /** - * Contract tests for TriggerPersonIdentityMatchOnFormSubmit. The listener - * acts as the FORM-05 stub + first-cut match detector per ARCH §31.1 and - * drives the form_submissions.identity_match_status column for the portal - * banner. These tests lock in the transition matrix so follow-up work - * extending PersonIdentityService can't silently break the contract. + * Contract tests for TriggerPersonIdentityMatchOnFormSubmit (v1.3 layout). * - * NOTE: PersonIdentityService is final, so we drive state via real - * Person/User fixtures (deterministic outcomes of detectMatches) rather - * than mocking. + * v1.3 changes: + * - Listener is now queued (was sync in v1.0/v1.2). + * - Failsafe-pad ("subject_id null → write 'pending' + log warning") + * replaced with strict invariant: throws IdentityMatchInvariantViolation + * when subject_type='person' AND subject_id IS NULL AND apply_status=COMPLETED. + * - Gate as first statement: skip when apply_status !== COMPLETED. + * - Dispatches FormSubmissionIdentityMatchResolved on the + * submission.{id} private channel after writing the final status. */ final class TriggerPersonIdentityMatchOnFormSubmitTest extends TestCase { @@ -64,10 +66,24 @@ final class TriggerPersonIdentityMatchOnFormSubmitTest extends TestCase } /** + * Build a submission then directly invoke the listener so the test + * fixture controls apply_status without going through ApplyBindings. + * + * `apply_status` is intentionally not in $fillable on the model + * (production writes go through ApplyBindings, never mass-assign); + * tests pass it via $overrides and we route it through direct + * attribute assignment. + * * @param array $overrides */ - private function submit(array $overrides = []): FormSubmission + private function makeSubmission(array $overrides = []): FormSubmission { + $applyStatus = null; + if (array_key_exists('apply_status', $overrides)) { + $applyStatus = $overrides['apply_status']; + unset($overrides['apply_status']); + } + $submission = FormSubmission::create(array_merge([ 'form_schema_id' => $this->schema->id, 'subject_type' => null, @@ -77,30 +93,76 @@ final class TriggerPersonIdentityMatchOnFormSubmitTest extends TestCase 'is_test' => false, ], $overrides)); - FormSubmissionSubmitted::dispatch($submission->fresh()); + if ($applyStatus !== null) { + $submission->apply_status = $applyStatus; + $submission->save(); + } return $submission->fresh(); } - public function test_public_event_registration_submission_is_marked_pending(): void + private function dispatchListener(FormSubmission $submission): void { - // RFC Q2: this path is now a logged-warning failsafe. The pending - // state is preserved for portal banner rendering, but a misconfigured - // schema or silently-failed ApplyBindings should surface in logs. - // Spy mode (not strict mock) so other listeners' Log calls don't - // interfere — ApplyBindingsOnFormSubmit may also log error here. - $logSpy = \Illuminate\Support\Facades\Log::spy(); + $listener = $this->app->make(TriggerPersonIdentityMatchOnFormSubmit::class); + $listener->handle(new FormSubmissionSubmitted($submission)); + } - $submission = $this->submit(); + public function test_handle_skips_when_apply_status_is_not_completed(): void + { + // v1.3 §Q2 + ARCH-BINDINGS §5.6 — gate skips on null/PENDING/PARTIAL/FAILED. + $person = Person::factory()->create([ + 'event_id' => $this->event->id, + 'crowd_type_id' => $this->crowdType->id, + 'user_id' => null, + 'email' => 'gate-skip@example.test', + ]); - $this->assertSame('pending', $submission->identity_match_status); + $submission = $this->makeSubmission([ + 'subject_type' => 'person', + 'subject_id' => $person->id, + // apply_status is NULL — gate must skip. + ]); - /** @var \Mockery\Expectation $expectation */ - $expectation = $logSpy->shouldHaveReceived('warning'); - $expectation->with( - 'form-builder.identity-match.no_person_subject_post_apply', - \Mockery::type('array'), - ); + $this->dispatchListener($submission); + + // identity_match_status stays NULL because the listener returned at the gate. + $this->assertNull($submission->fresh()->identity_match_status); + } + + public function test_handle_skips_when_apply_status_is_partial(): void + { + // PARTIAL is treated identically to FAILED by the gate per ARCH-BINDINGS §5.6. + $person = Person::factory()->create([ + 'event_id' => $this->event->id, + 'crowd_type_id' => $this->crowdType->id, + 'user_id' => null, + ]); + + $submission = $this->makeSubmission([ + 'subject_type' => 'person', + 'subject_id' => $person->id, + 'apply_status' => ApplyStatus::PARTIAL->value, + ]); + + $this->dispatchListener($submission); + + // PARTIAL → gate skips → identity_match_status untouched. + $this->assertNull($submission->fresh()->identity_match_status); + } + + public function test_handle_throws_invariant_violation_when_subject_id_null_after_completed_with_person_subject_type(): void + { + // v1.3 §Q2 — strict invariant replaces the v1.2 failsafe-pad. + $submission = $this->makeSubmission([ + 'subject_type' => 'person', + 'subject_id' => null, // invariant violation + 'apply_status' => ApplyStatus::COMPLETED->value, + ]); + + $this->expectException(IdentityMatchInvariantViolation::class); + $this->expectExceptionMessageMatches('/subject_id=null after ApplyBindings COMPLETED/'); + + $this->dispatchListener($submission); } public function test_linked_person_is_marked_matched(): void @@ -112,12 +174,15 @@ final class TriggerPersonIdentityMatchOnFormSubmitTest extends TestCase 'user_id' => $user->id, ]); - $submission = $this->submit([ + $submission = $this->makeSubmission([ 'subject_type' => 'person', 'subject_id' => $person->id, + 'apply_status' => ApplyStatus::COMPLETED->value, ]); - $this->assertSame('matched', $submission->identity_match_status); + $this->dispatchListener($submission); + + $this->assertSame('matched', $submission->fresh()->identity_match_status); } public function test_unlinked_person_with_no_matches_is_marked_none(): void @@ -134,12 +199,15 @@ final class TriggerPersonIdentityMatchOnFormSubmitTest extends TestCase 'registration_source' => 'self', ]); - $submission = $this->submit([ + $submission = $this->makeSubmission([ 'subject_type' => 'person', 'subject_id' => $person->id, + 'apply_status' => ApplyStatus::COMPLETED->value, ]); - $this->assertSame('none', $submission->identity_match_status); + $this->dispatchListener($submission); + + $this->assertSame('none', $submission->fresh()->identity_match_status); } public function test_unlinked_person_with_pending_match_is_marked_pending(): void @@ -156,12 +224,15 @@ final class TriggerPersonIdentityMatchOnFormSubmitTest extends TestCase 'email' => 'match@example.test', ]); - $submission = $this->submit([ + $submission = $this->makeSubmission([ 'subject_type' => 'person', 'subject_id' => $person->id, + 'apply_status' => ApplyStatus::COMPLETED->value, ]); - $this->assertSame('pending', $submission->identity_match_status); + $this->dispatchListener($submission); + + $this->assertSame('pending', $submission->fresh()->identity_match_status); } public function test_non_event_registration_submission_is_left_untouched(): void @@ -180,75 +251,63 @@ final class TriggerPersonIdentityMatchOnFormSubmitTest extends TestCase 'status' => FormSubmissionStatus::SUBMITTED->value, 'submitted_at' => now(), 'is_test' => false, + 'apply_status' => ApplyStatus::COMPLETED->value, ]); - FormSubmissionSubmitted::dispatch($submission->fresh()); + $this->dispatchListener($submission->fresh()); $this->assertNull($submission->fresh()->identity_match_status); } - public function test_it_writes_identity_match_status_before_http_response_returns(): void - { - // Regression guard: the listener must run synchronously so the - // public submit response already carries identity_match.status. - // If someone reinstates ShouldQueue, the column stays null in - // the response body (only written later by a queue worker) and - // this assertion fails. - Config::set('form_builder.captcha.required_for_purposes', []); - - $this->schema->update([ - 'is_published' => true, - 'public_token' => (string) Str::ulid(), - ]); - FormField::factory()->create([ - 'form_schema_id' => $this->schema->id, - 'field_type' => FormFieldType::TEXT->value, - 'slug' => 'motivatie', - 'label' => 'Motivatie', - 'is_portal_visible' => true, - 'is_admin_only' => false, - ]); - $token = $this->schema->fresh()->public_token; - - $create = $this->postJson( - "/api/v1/public/forms/{$token}/submissions", - [ - 'idempotency_key' => 'sync-regression-001', - 'public_submitter_name' => 'Sync Tester', - 'public_submitter_email' => 'sync-tester@example.test', - ], - ); - $create->assertCreated(); - $submissionId = $create->json('data.id'); - - $this->postJson( - "/api/v1/public/forms/{$token}/submissions/{$submissionId}/submit", - ['values' => ['motivatie' => 'test']], - ) - ->assertCreated() - ->assertJsonPath('data.status', 'submitted') - ->assertJsonPath('data.identity_match.status', 'pending'); - } - public function test_submission_with_no_schema_is_left_untouched(): void { // Guard branch: if the schema relation can't resolve, the listener // must early-return without touching the status and without throwing // (so sibling listeners keep executing). - $submission = FormSubmission::create([ - 'form_schema_id' => $this->schema->id, + $submission = $this->makeSubmission([ 'subject_type' => null, 'subject_id' => null, - 'status' => FormSubmissionStatus::SUBMITTED->value, - 'submitted_at' => now(), - 'is_test' => false, + 'apply_status' => ApplyStatus::COMPLETED->value, ]); // Soft-delete the schema so fresh(['schema']) returns null for the relation. FormSchema::query()->whereKey($this->schema->id)->delete(); - FormSubmissionSubmitted::dispatch($submission->fresh()); + $this->dispatchListener($submission); $this->assertNull($submission->fresh()->identity_match_status); } + + public function test_dispatches_broadcast_after_successful_match(): void + { + // broadcast(...) helper returns a PendingBroadcast which on + // __destruct calls Event::dispatch($event). Faking the Event + // facade for our specific event class captures the dispatch + // without it leaving the test boundary. + \Illuminate\Support\Facades\Event::fake([FormSubmissionIdentityMatchResolved::class]); + + $user = User::factory()->create(); + $person = Person::factory()->create([ + 'event_id' => $this->event->id, + 'crowd_type_id' => $this->crowdType->id, + 'user_id' => $user->id, + ]); + + $submission = $this->makeSubmission([ + 'subject_type' => 'person', + 'subject_id' => $person->id, + 'apply_status' => ApplyStatus::COMPLETED->value, + ]); + + $this->dispatchListener($submission); + + \Illuminate\Support\Facades\Event::assertDispatched( + FormSubmissionIdentityMatchResolved::class, + function (FormSubmissionIdentityMatchResolved $event) use ($submission): bool { + return $event->submissionId === (string) $submission->id + && $event->status === 'matched' + && $event->matchCount >= 0; + }, + ); + } } diff --git a/dev-docs/BACKLOG.md b/dev-docs/BACKLOG.md index 00506071..526fa9df 100644 --- a/dev-docs/BACKLOG.md +++ b/dev-docs/BACKLOG.md @@ -1003,6 +1003,22 @@ ARCH-discussie en RFC. --- +### TECH-CHANNEL-AUTH-ORG-ADMIN — Extend `submission.{id}` private channel auth to organisation admins + +**Aanleiding:** WS-6 v1.3-delta D2 wires the broadcast event `FormSubmissionIdentityMatchResolved` (RFC-WS-6 §Q1 v1.3 addition 2) on the `submission.{id}` private channel. The auth callback in `routes/channels.php` currently authorises only the submitter (`submitted_by_user_id === user.id`). Org-admin access was deferred because the codebase does not yet have a vetted Spatie Permission helper for organisation-scoped role checks (e.g. `hasRoleInOrganisation('organizer_admin', $orgId)`); guessing the API would risk authorising too broadly or too narrowly without test coverage. Phase A audit confirmed no precedent for this check pattern in `app/`. + +**Wat:** +- Audit Spatie Permission usage across the codebase to identify (or design) the canonical "is X a role-holder in organisation Y" helper. Likely candidates: extension method on `User`, query scope on `Organisation::users()` pivot, or a dedicated Policy method. +- Extend `routes/channels.php`'s `submission.{submissionId}` callback to additionally authorise organisation admins of the submission's organisation. Replace the inline TODO with the resolved helper. +- Add channel authorization tests covering the four cases: submitter (allow), org admin (allow), other org user (deny), anonymous (deny). +- Update RFC-WS-6 §Q1 v1.3 addition 2 reference if the auth contract changes shape; otherwise leave RFC alone. + +**Prioriteit:** Medium — frontend Echo subscription is also out of WS-6 scope, so the channel has no live subscribers yet. When the frontend follow-up ships, org admins are expected to see live updates on submissions in their orgs — that is when this work blocks the user-facing feature. + +**Refs:** `api/routes/channels.php` (TODO marker in submission channel callback), RFC-WS-6 §Q1 v1.3 addition 2, ARCH-BINDINGS §11 (admin failures UI — analogous tenant scope via FK chain). + +--- + ### ~~TECH-02 — scopeForFestival helper op Event model~~ ✅ OPGELOST ---