From 1fdd254a8a866958cb6c14893b04cf68eeb57816 Mon Sep 17 00:00:00 2001 From: "bert.hausmans" Date: Sun, 26 Apr 2026 17:02:48 +0200 Subject: [PATCH] docs: complete ARCH-BINDINGS.md sections 6-9 from session 2 work (WS-6) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sections 6 (apply pipeline), 7 (failures and retry), 8 (multi-tenancy and security tenant resolution), 9 (listener chain) populated from session 2 implementation. Each subsection 200-400 words referencing RFC-WS-6.md sections by number. §8.2 (IDOR class tests) and frontend-specific sections in §3 admin UI remain pending session 3. Refs: RFC-WS-6.md Co-Authored-By: Claude Opus 4.7 (1M context) --- dev-docs/ARCH-BINDINGS.md | 250 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 239 insertions(+), 11 deletions(-) diff --git a/dev-docs/ARCH-BINDINGS.md b/dev-docs/ARCH-BINDINGS.md index f5c86ce7..dc5ba450 100644 --- a/dev-docs/ARCH-BINDINGS.md +++ b/dev-docs/ARCH-BINDINGS.md @@ -168,47 +168,246 @@ i18n message keys live in ### 6.1 Snapshot vs. live (RFC Q6) -> Populated in session 2 — see RFC-WS-6.md §3 Q6. +The `FormBindingApplicator` reads bindings from +`form_submissions.schema_snapshot.fields[*].bindings` (plural), not from +the live `form_field_bindings` table. WS-6 expanded the snapshot's +binding shape to carry every applicator-relevant field — +`{id, mode, entity, column, merge_strategy, trust_level, is_identity_key, sync_direction?}` +— via `FormFieldBindingService::toApplicatorShape()`. The legacy +singular `binding` key is preserved for webhook / GDPR readers; the new +plural `bindings` key is what the pipeline consumes. + +This guarantees that a retry executed days after the original +submission applies bindings as they were configured at submit time, not +as they may have been edited since. Reproducibility for audit. The +`PersonProvisioner` and `BindingConflictResolver` both read the +snapshot exclusively; the live table is only consulted by the publish +guards (config time) and BindingTypeRegistry (target shape). Tests +include a `snapshot_is_truth_ignores_post_submit_binding_edits` +assertion that mutates the live table after submission and verifies the +provisioner ignores the change. ### 6.2 Conflict resolution (RFC Q7) -> Populated in session 2 — see RFC-WS-6.md §3 Q7. +`BindingConflictResolver::resolve(submission, sectionId?)` is pure +logic, no DB writes. It walks `schema_snapshot.fields[*].bindings`, +filters by section when `sectionId` is non-null (Q10 future), and +groups candidates by `(target_entity, target_attribute)`. Within each +group it sorts `trust_level DESC` then `form_field.sort_order ASC`, +picks the first as winner, and returns a `list`. + +Candidate-set rule: a binding is a candidate **iff** the source +form_field has a row in `form_values` for this submission. Absence +excludes; null value is included with `valueIsExplicit = true`. The +write-path invariant test (Task 10) asserts the necessary +precondition: every visible field has a `form_values` row after +submit, every absent field has none. Without this invariant Q7 +collapses — "explicit clear" becomes indistinguishable from +"skipped by conditional logic". ### 6.3 Apply algorithm and merge-strategy null matrix (RFC V1, Q7) -> Populated in session 2 — see RFC-WS-6.md §4 V1 + §3 Q7. +`FormBindingApplicator::apply($submission, ?$sectionId)` is the +orchestrator: + +1. Assert `DB::transactionLevel() > 0` — caller MUST own the + transaction (RFC Q4). Catastrophic violations throw + `FormBindingApplicatorException`. +2. Resolve subject via `PurposeSubjectResolver` (Q9). +3. If subject is null (incident_report anonymous path), return a + COMPLETED `BindingPassResult` with no applications. +4. Resolve bindings via `BindingConflictResolver` filtered by + `sectionId` (Q10) and the candidate-set rule (Q7). +5. Skip identity-key bindings during apply — the subject resolver + already used them for lookup; re-writing is at-best a no-op, + at-worst a clobber. +6. For each non-identity binding compute the new value via the merge + matrix (below), call `$subject->setAttribute()` + `save()`. + Per-binding failures are captured in `BindingApplicationResult::failed()` + inside the result and do NOT throw — partial passes are expected. +7. Return `BindingPassResult` with `applications`, `successCount`, + `failureCount`, derived `applyStatus()`. +8. Pass result to `BindingActivityLogger::logPass()` for the + hierarchical activity-log (§6.7). + +Merge strategy × null winner matrix (Q7 + V1): + +| Strategy | Winner non-null | Winner null | +|---|---|---| +| `overwrite` | write | write null | +| `append` (collection only — V1) | merge with set semantics | no-op | +| `replace` | write only when target null | no-op | +| `first_write_wins` | write only when target null | write null when target null | + +`Append` on a scalar target is a defensive runtime check via +`BindingTypeRegistry::validateAppendStrategy()` — publish guards +catch this at config time, but the runtime check protects against +live-table edits between publish and apply. ### 6.4 Person provisioning (RFC Q8) -> Populated in session 2 — see RFC-WS-6.md §3 Q8. +`PersonProvisioner::provisionFromSubmission()` (called from +`EventRegistrationSubjectResolver`): + +1. Reads bindings from `schema_snapshot.fields[*].bindings`. +2. Finds the unique `is_identity_key=true` binding for + `target_entity='person'` (single-key invariant — composite identity + tracked in BACKLOG `FORM-BINDING-COMPOSITE-IDENTITY`). +3. Reads the form_value for that field (raises if missing/null — + publish guards prevent this at config time). +4. `Person::query()->where('email', $value)->where('event_id', $eventId) + ->lockForUpdate()->first()` → returns existing if found. +5. Otherwise builds attributes from the OTHER (non-identity-key) + bindings filtered to `Person::$fillable`, sets a default + `crowd_type_id` (first active CrowdType in the org), and calls + `Person::firstOrCreate(['email' => ..., 'event_id' => ...], $attrs)`. + +`firstOrCreate` semantics resolve the +"transaction A's lockForUpdate window vs. transaction B's insert" +race — the unique-constraint surfaces and re-reads the existing row. +Tested by `PersonProvisionerConcurrencyTest` with state-injection +under a real DB transaction (RFC V4 — wall-clock load testing is +deferred to BACKLOG `LOAD-TEST-FOUNDATION`). + +Multi-tenancy: Person's `organisationScopeColumn` is `event_id` +(not `organisation_id` directly). The provisioner scopes by +`event_id` only — cross-event submissions never collide. ### 6.5 Per-purpose subject resolution (RFC Q9) -> Populated in session 2 — see RFC-WS-6.md §3 Q9. +Each purpose declares a `subject_resolver_class` in +`config/form_builder/purposes.php` implementing +`App\FormBuilder\Purposes\PurposeSubjectResolver`. The interface is +parallel to `PurposeGuardProvider` from session 1 — `PurposeDefinition` +remains a frozen value object; both interfaces hang off the registry. + +| Purpose | Resolver | Mechanism | +|---|---|---| +| `event_registration` | `EventRegistrationSubjectResolver` | `PersonProvisioner` (may create) | +| `artist_advance` | `ArtistAdvanceSubjectResolver` | Portal token; subject preset on submission | +| `supplier_intake` | `SupplierIntakeSubjectResolver` | Production-request → Company subject | +| `post_event_evaluation` | `PostEventEvaluationSubjectResolver` | Auth user → linked Person | +| `incident_report` | `IncidentReportSubjectResolver` | Anonymous-allowed; may return null | +| `signature_contract` | `SignatureContractSubjectResolver` | Auth user | +| `user_profile` | `UserProfileSubjectResolver` | Auth user | + +Concrete resolvers narrow the return type via PHP covariance +(`Person`, `Company`, `User`) so callers don't need to assert. Only +`IncidentReportSubjectResolver` may return null; the others throw +`PurposeSubjectResolutionException` with a typed `reasonCode`. ### 6.6 Section-level apply stub (RFC Q10) -> Populated in session 2 — see RFC-WS-6.md §3 Q10. +`ApplyBindingsOnFormSectionSubmitted` is a queued listener registered +on `FormSubmissionSectionSubmitted`. Its `handle()` early-returns when +`config('form_builder.section_apply_enabled')` is false (default). +When enabled it forwards to `FormBindingApplicator::apply($submission, +sectionId:
)` inside a `DB::transaction`. + +The publish-time guard `IdentityKeyBindingsOnlyInFirstSection` is +active **regardless** of the runtime flag — schema structure is gated +at publish, runtime behaviour is gated by the flag. This means +section-aware schemas can't ship structurally-unsafe configurations +even before the runtime path activates. + +Removal trigger documented in `config/form_builder.php`: when +ARTIST_ADVANCE feature work begins, set +`FORM_BUILDER_SECTION_APPLY=true`, write section-scoped tests, remove +the early-return guard. Tracking: `ARTIST-ADV-SECTION-APPLY` in +BACKLOG.md. ### 6.7 Activity log granularity (RFC Q12) -> Populated in session 2 — see RFC-WS-6.md §3 Q12. +`BindingActivityLogger::logPass()` writes one parent activity +(`form_submission.bindings_pass_completed` with +`{binding_count, succeeded, failed, apply_status, person_provisioned, subject_type, subject_id}`) +plus one child activity per binding +(`form_submission.binding_applied` with +`{parent_activity_id, binding_id, target_entity, target_attribute, success, old_value, new_value, source_submission_id}`). +Failed bindings get `error_class` / `error_message` in their child +activity in addition to a `FormSubmissionActionFailure` row. + +Two sources of truth for failures (activity_log + action_failures) is +intentional: activity_log is the human-readable timeline, +action_failures is the machine-replayable workflow. Sessions 3's UI +renders pass-level visible with per-binding expand-on-demand. ## 7. Failures and retry ### 7.1 Two-transaction pattern (RFC Q4) -> Populated in session 2 — see RFC-WS-6.md §3 Q4. +`ApplyBindingsOnFormSubmit::handle()` uses two distinct DB +transactions: + +``` +try { + DB::transaction(function () { + // Inner: applicator + apply_status update + (when ApplyBindings + // provisioned a Person) subject_type/subject_id sync to submission. + }); +} catch (Throwable $e) { + // OUTSIDE the failed transaction — survives inner rollback. + DB::transaction(function () { + FormSubmissionActionFailure::create([...]); + FormSubmission::query()->whereKey(...) + ->update(['apply_status' => FAILED, 'apply_completed_at' => now()]); + }); + Log::error('form-builder.apply.transaction_rolled_back', [...]); +} +``` + +The inner transaction owns the apply pass. On exception it rolls +back atomically (any provisioned Person, any partial writes) — but +the outer catch then opens a SECOND transaction and writes the +failure record, which survives because it's not part of the inner +rollback. The second transaction's failure path is Sentry-only with +an explicit error log line for filterability. + +The listener does not rethrow (RFC Q3) so sibling listeners +(TriggerPersonIdentityMatch, queued tag-sync, queued webhooks, +queued mailables) keep running. ### 7.2 Retry, resolve, dismiss flows (RFC V2) -> Populated in session 2 — see RFC-WS-6.md §4 V2. +Three admin actions on a `FormSubmissionActionFailure` row: + +- **Retry** — replay the applicator. Idempotent. Increments + `retry_count`. On success: sets `resolved_at = now()`. On repeat + failure: appends a NEW row preserving the audit trail (with + `context.retry_of` pointing to the original). +- **Mark as resolved** — manual close, optional `resolved_note`. + Used when an admin fixed the data via a different path. +- **Dismiss** — final close, requires `dismissed_reason_type` + (DismissalReasonType enum), `dismissed_reason_note` mandatory only + when reason is `OTHER`. `manually_resolved` is intentionally absent + from the enum — Resolve and Dismiss are different workflows. + +Three artisan commands mirror the API endpoints with the same FK-chain +isolation (`form-failures:retry|resolve|dismiss`). Bulk-retry by +organisation is supported only via the API endpoint and the +`--org=` artisan flag; no in-place mutation, history is preserved. ## 8. Multi-tenancy and security ### 8.1 FK-chain tenant resolution (RFC V3) -> Populated in session 2 — see RFC-WS-6.md §4 V3. +`form_submission_action_failures` has no `organisation_id` column by +design. Tenant scope flows via +`failure.submission.organisation_id`. The +`FormSubmissionActionFailurePolicy` resolves the chain at access time +with `withoutGlobalScopes()` so cross-tenant access reaches the +policy (which then translates denied → 404, never 403, to prevent +resource-existence enumeration). + +The controller's `resolveFailure()` helper performs the same +`withoutGlobalScopes` lookup. Soft-delete on the parent submission is +checked explicitly (`$submission->deleted_at !== null`) since +`withoutGlobalScopes` bypasses the SoftDeletes scope too. + +The policy is registered explicitly in `AppServiceProvider::boot()` +because Laravel's auto-discovery doesn't reliably resolve +`App\Models\FormBuilder\FormSubmissionActionFailure` to +`App\Policies\FormBuilder\FormSubmissionActionFailurePolicy`. ### 8.2 IDOR class tests @@ -216,7 +415,36 @@ i18n message keys live in ## 9. Listener chain -> Populated in session 2 — see RFC-WS-6.md §3 Q1, Q2, Q3. +`FormSubmissionSubmitted` listeners are registered explicitly in +`AppServiceProvider::boot()` (RFC Q1) — Laravel auto-discovery's +filesystem-traversal order is fragile cross-platform. + +Sync chain (registration order is execution order): + +1. `ApplyBindingsOnFormSubmit` — provisions subject + applies bindings + (Q4 two-transaction, swallows exceptions per Q3). +2. `TriggerPersonIdentityMatchOnFormSubmit` — runs identity-match + detection against the freshly-provisioned Person. Per RFC Q2 the + "no person subject → pending" path is now a logged-warning failsafe; + it should never fire for event_registration submissions post-WS-6 + because ApplyBindings runs first. + +Queued (parallel, post-sync): + +- `SyncTagPickerSelectionsOnSubmit` — TAG_PICKER → user_organisation_tags + rebuild. Implements `ShouldQueue`. Deliberately NOT folded into + ApplyBindings: TAG_PICKER → pivot-table-with-source-discrimination + is semantically distinct from a binding-target-attribute write + (RFC Q3). +- (future) FormWebhookDispatcher, RegistrationConfirmation mailable. + +`FormSubmissionSectionSubmitted` listener: `ApplyBindingsOnFormSectionSubmitted` +(queued, feature-flagged, currently a no-op). + +`FormSubmissionSubmitted` itself is dispatched **after** the +`FormSubmissionService::submit()` transaction commits (RFC O2). +Pre-commit dispatch let queued listeners enqueue with state that +might never persist on rollback — fixed in WS-6. ## 10. Out of scope (v1)