From 85f4777e0cce31120b12b6a4114b859d5b624531 Mon Sep 17 00:00:00 2001 From: "bert.hausmans" Date: Tue, 28 Apr 2026 09:09:41 +0200 Subject: [PATCH] =?UTF-8?q?docs(ws-6):=20RFC-WS-6=20v1.1=20addenda=20+=20A?= =?UTF-8?q?RCH-BINDINGS=20=C2=A76.4=20alignment?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Promote RFC-WS-6 to v1.1 with two §3 addenda capturing the post-session-2 cleanup decisions; align ARCH-BINDINGS.md §6.4 (Person provisioning) with the v1.1 text. No architectural reversals — corrections + one schema addition. §3 Q8 v1.1 addendum — Person provisioning is scoped by `event_id`: - Q8 v1.0 said `Person::firstOrCreate(['email', 'organisation_id'], ...)`. That is incorrect against the actual model: `Person::$organisationScopeColumn` is `event_id`. The provisioner looks up and creates by `(email, event_id)`. - Same email registering across two events in the same org → two distinct Person rows. Cross-event identity reconciliation remains the job of `PersonIdentityService` (out of scope WS-6). - Failsafe: `PersonProvisioningException('no_event', ...)` when `submission.event_id` is null on event_registration; publish guard `SchemaHasLinkedEvent` blocks at config time. §3 Q9 v1.1 addendum — `form_schemas.default_crowd_type_id` replaces `CrowdType::oldest()`: - Session 2's PersonProvisioner used a silent oldest()-in-org heuristic for the new Person's `crowd_type_id` (NOT NULL). Fragile, undocumented, cross-org broken. - v1.1 adds `form_schemas.default_crowd_type_id` (nullable ULID) as the explicit, versioned schema attribute. `RequiresDefaultCrowdType` publish guard wires into `EventRegistrationGuards`. Runtime failsafe in `PersonProvisioner::resolveCrowdTypeId()` throws `PersonProvisioningException('no_default_crowd_type', ...)` when null. - Schema-level FK omitted intentionally (SQLite cascade-delete on ALTER TABLE ADD FOREIGN KEY observed in WS-5b/c backfill tests). Application-level integrity (publish guard + runtime failsafe + Eloquent `belongsTo`) is sufficient because writes always go through `FormSchemaService::publish()`. - Snapshot impact: none. Provisioning reads from live FormSchema by FK; audit replay uses whatever the schema's current `default_crowd_type_id` is at retry time. ARCH-BINDINGS.md §6.4: - Now references "RFC Q8 + Q9, v1.1" in the heading. - Default-crowd-type bullet replaces "first active CrowdType in the org" (the session-2 oldest() heuristic) with the schema attribute lookup. - Multi-tenancy paragraph clarified for cross-event scoping. Cross-references touched up: - `PersonProvisioner::resolveCrowdTypeId()` docblock: §3 Q8 → §3 Q9. - `RequiresDefaultCrowdType` class docblock: §3 Q8 → §3 Q9. - `SCHEMA.md` v2.7 changelog and `default_crowd_type_id` column note: §3 Q8 → §3 Q9. Document history entry added in §10 documenting v1.1 + the snapshot dual-key cleanup and route-model-binding fix landed in earlier commits on this branch. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Bindings/PersonProvisioner.php | 4 +- .../Publishing/RequiresDefaultCrowdType.php | 2 +- dev-docs/ARCH-BINDINGS.md | 28 +++++- dev-docs/RFC-WS-6.md | 89 ++++++++++++++++++- dev-docs/SCHEMA.md | 4 +- 5 files changed, 117 insertions(+), 10 deletions(-) diff --git a/api/app/FormBuilder/Bindings/PersonProvisioner.php b/api/app/FormBuilder/Bindings/PersonProvisioner.php index 7f561a08..99f98847 100644 --- a/api/app/FormBuilder/Bindings/PersonProvisioner.php +++ b/api/app/FormBuilder/Bindings/PersonProvisioner.php @@ -155,7 +155,7 @@ final readonly class PersonProvisioner * Person.crowd_type_id is NOT NULL on the migration; the schema * declares its target CrowdType explicitly via `default_crowd_type_id`. * - * RFC-WS-6 v1.1 §3 Q8 addendum (was: silent oldest() fallback in + * RFC-WS-6 v1.1 §3 Q9 addendum (was: silent oldest() fallback in * session 2). The RequiresDefaultCrowdType publish guard prevents * misconfigured event_registration schemas from publishing; this * runtime throw is a failsafe for live-table edits between publish @@ -211,7 +211,7 @@ final readonly class PersonProvisioner array $bindings, array $identityBinding, ): array { - $personFillable = (new Person())->getFillable(); + $personFillable = (new Person)->getFillable(); $attributes = []; foreach ($bindings as $entry) { diff --git a/api/app/FormBuilder/Publishing/RequiresDefaultCrowdType.php b/api/app/FormBuilder/Publishing/RequiresDefaultCrowdType.php index e7d50d05..42b5cb9d 100644 --- a/api/app/FormBuilder/Publishing/RequiresDefaultCrowdType.php +++ b/api/app/FormBuilder/Publishing/RequiresDefaultCrowdType.php @@ -7,7 +7,7 @@ namespace App\FormBuilder\Publishing; use App\Models\FormBuilder\FormSchema; /** - * RFC-WS-6 v1.1 §3 Q8 addendum — event_registration schemas must + * RFC-WS-6 v1.1 §3 Q9 addendum — event_registration schemas must * declare a default_crowd_type_id so PersonProvisioner can land * new registrants in the right CrowdType. Replaces the silent * oldest() heuristic from session 2. diff --git a/dev-docs/ARCH-BINDINGS.md b/dev-docs/ARCH-BINDINGS.md index dc5ba450..a573ef32 100644 --- a/dev-docs/ARCH-BINDINGS.md +++ b/dev-docs/ARCH-BINDINGS.md @@ -244,7 +244,7 @@ Merge strategy × null winner matrix (Q7 + V1): catch this at config time, but the runtime check protects against live-table edits between publish and apply. -### 6.4 Person provisioning (RFC Q8) +### 6.4 Person provisioning (RFC Q8 + Q9, v1.1) `PersonProvisioner::provisionFromSubmission()` (called from `EventRegistrationSubjectResolver`): @@ -258,8 +258,10 @@ live-table edits between publish and apply. 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 + bindings filtered to `Person::$fillable`, resolves + `crowd_type_id` from `$submission->schema->default_crowd_type_id` + (RFC Q9 v1.1 addendum — replaces the silent `CrowdType::oldest()` + heuristic), and calls `Person::firstOrCreate(['email' => ..., 'event_id' => ...], $attrs)`. `firstOrCreate` semantics resolve the @@ -271,7 +273,25 @@ 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. +`event_id` only — cross-event submissions never collide. Same email +registering across two events in the same org → two distinct Person +rows; identity reconciliation is `PersonIdentityService`'s job +(out of scope for WS-6, RFC §6 / RFC Q8 v1.1 addendum). + +Default crowd type: + +- `form_schemas.default_crowd_type_id` (nullable ULID) is the single + source of truth for the freshly-provisioned Person's `crowd_type_id`. +- `RequiresDefaultCrowdType` publish guard blocks publish when null on + an `event_registration` schema. +- `PersonProvisioner::resolveCrowdTypeId()` throws + `PersonProvisioningException('no_default_crowd_type', ...)` when + null at apply time (failsafe for live-table edits between publish + and apply). +- No DB-level FK — application-level integrity only (SQLite cascade + problem, see RFC Q9 v1.1 addendum). The Eloquent + `FormSchema::defaultCrowdType()` `belongsTo` relation handles + read-side correctness. ### 6.5 Per-purpose subject resolution (RFC Q9) diff --git a/dev-docs/RFC-WS-6.md b/dev-docs/RFC-WS-6.md index eab7486e..3697027e 100644 --- a/dev-docs/RFC-WS-6.md +++ b/dev-docs/RFC-WS-6.md @@ -3,7 +3,8 @@ ## 1. Status - **State:** Authoritative for sessions 1, 2, 3 of WS-6 -- **Frozen:** 2026-04-25 +- **Frozen:** 2026-04-25 (v1.0); refined post-session-2 cleanup as v1.1 (see §10) +- **Version:** v1.1 - **Owner:** Bert Hausmans - **Origin:** Architectural session 2026-04-25 (Claude Chat) — 13 design decisions, 4 refinements, 3 observations - **Related:** @@ -180,6 +181,45 @@ Per-strategy null-winner matrix: Composite identity (email OR (first_name + last_name + DOB)) is **out of scope for v1.0**. Backlog item `FORM-BINDING-COMPOSITE-IDENTITY` tracks the future work. Identity-matching against existing user accounts (via `person_identity_matches`) is a separate flow, not in scope here — that's `PersonIdentityService` running after provision. +#### Q8 v1.1 addendum — Person provisioning is scoped by `event_id`, not `organisation_id` + +The v1.0 text above describes the firstOrCreate predicate as +`(email, organisation_id)`. That is incorrect for the actual Person +model: `Person::$organisationScopeColumn = 'event_id'` (single source +of truth for tenant scoping on this model — `organisation_id` is the +denormalised parent column, `event_id` is the scope discriminator). + +`PersonProvisioner::provisionFromSubmission()` therefore looks up and +creates by `(email, event_id)`: + +```php +Person::query() + ->withoutGlobalScopes() + ->where('email', $emailValue) + ->where('event_id', $submission->event_id) + ->lockForUpdate() + ->first(); +// ... +Person::query() + ->withoutGlobalScopes() + ->firstOrCreate( + ['email' => $emailValue, 'event_id' => $submission->event_id], + $attributes, + ); +``` + +Practical consequence: the same email registering for two different +events in the same organisation creates two distinct `Person` rows. +Cross-event identity reconciliation is the job of +`PersonIdentityService` / `person_identity_matches` (existing flow, +out of scope for WS-6 — see RFC §6). + +`PersonProvisioner` raises `PersonProvisioningException('no_event', ...)` +when `submission.event_id` is null on an `event_registration` +submission. Schemas reaching apply without `event_id` is a structural +defect; the publish guard `SchemaHasLinkedEvent` prevents it at config +time, the runtime throw is the failsafe. + ### Q9 — Apply uniform across all 7 purposes; provisioning per `PurposeDefinition` **Decision:** `FormBindingApplicator::apply()` is purpose-agnostic. Subject resolution lives on `PurposeDefinition::resolveOrProvisionSubject(FormSubmission, PersonProvisioner): ?Model`. @@ -196,6 +236,48 @@ user_profile → UserResolver::fromAuth Test coverage: 4 cases per purpose minimum (happy-path, missing required binding, conflict-resolution, anonymous-when-applicable) → 28 + ~20 purpose-agnostic = ~48 pipeline tests in session 2. +#### Q9 v1.1 addendum — `form_schemas.default_crowd_type_id` replaces `CrowdType::oldest()` + +`Person.crowd_type_id` is `NOT NULL` on the migration; `PersonProvisioner` +must supply a value at create time. Session 2 used a silent +`CrowdType::oldest()->where('organisation_id', ...)` heuristic — fragile +(depends on insertion order, breaks across organisations, no auditable +intent) and undocumented in the published schema. + +**Decision (v1.1):** add `form_schemas.default_crowd_type_id` (nullable +ULID column) to make the target CrowdType an explicit, versioned schema +attribute. `PersonProvisioner::resolveCrowdTypeId()` reads +`$submission->schema->default_crowd_type_id` and throws +`PersonProvisioningException('no_default_crowd_type', ...)` when null. + +A new universal-on-event_registration publish guard +`RequiresDefaultCrowdType` blocks publish when the column is null on a +schema with `purpose = event_registration`. The runtime throw remains +as a failsafe for live-table edits between publish and apply. + +**Schema-level FK omitted intentionally.** The migration adds the +column without a database-level foreign key. SQLite's table-rebuild on +`ALTER TABLE ADD FOREIGN KEY` cascade-deletes existing form_fields rows +when an unrelated FK on the rebuilt table happens to overlap — observed +in WS-5b/c backfill tests. Application-level integrity (publish guard + +runtime failsafe + Eloquent `belongsTo` for read-side correctness) is +sufficient: writes always go through `FormSchemaService::publish()`, +which runs the guard, and the runtime throw blocks any apply that +slipped past. + +Snapshot impact: none. The published schema_snapshot does not embed +`default_crowd_type_id` directly; provisioning reads from the live +`FormSchema` row by FK from `FormSubmission::form_schema_id`. Audit +replay (RFC Q6) of an old snapshot uses whatever the schema's current +`default_crowd_type_id` is at retry time — admins are expected to +either update the column before retrying or dismiss with reason +`SCHEMA_DELETED` / `OTHER`. + +`FormBuilderDevSeeder` resolves a CrowdType via VOLUNTEER → first-active → +create-as-needed fallback chain when seeding event_registration +schemas, so dev environments don't fail the publish guard out of the +box. + ### Q10 — Section-level submit: stub now, activate later **Decision:** Build the listener-class structure now; gate the runtime via `config('form_builder.section_apply_enabled', false)`; activate when ARTIST_ADVANCE feature work lands. @@ -381,3 +463,8 @@ This document. Sessions 2 and 3 reference RFC sections by number rather than re- ## 10. Document history - 2026-04-25 — v1.0 — Initial RFC, frozen at start of WS-6 session 1. +- 2026-04-28 — v1.1 — Post-session-2 cleanup addenda (no architectural reversals; corrections + one schema addition): + - **§3 Q8 addendum** — Person provisioning scopes by `(email, event_id)`, not `(email, organisation_id)`. Aligns with `Person::$organisationScopeColumn = 'event_id'`. + - **§3 Q9 addendum** — `form_schemas.default_crowd_type_id` (nullable ULID, no DB-level FK) replaces the silent `CrowdType::oldest()` heuristic. New `RequiresDefaultCrowdType` publish guard wired into `EventRegistrationGuards`. Runtime failsafe in `PersonProvisioner::resolveCrowdTypeId()`. + - Snapshot dual-key cleanup (separate from RFC §3): legacy `binding` (singular) snapshot key dropped; `bindings` (plural list) is the single source of truth in `schema_snapshot.fields[*]`. ARCH-BINDINGS.md §6.4 / §6.1 already specified the plural list — code converged. + - Route model binding (separate from RFC §3): controller-level workaround `$request->route('formSubmissionActionFailure')` replaced with explicit `Route::bind()` in `AppServiceProvider::boot()` plus `->withoutScopedBindings()` on org-scoped routes. Type-hinted parameters restored. RFC V3 (FK-chain tenant policy) unchanged. diff --git a/dev-docs/SCHEMA.md b/dev-docs/SCHEMA.md index cbf1d2e6..76958974 100644 --- a/dev-docs/SCHEMA.md +++ b/dev-docs/SCHEMA.md @@ -9,7 +9,7 @@ > added (nullable; required at publish time for event_registration via > RequiresDefaultCrowdType guard). Replaces the silent `oldest()` > CrowdType heuristic from session 2's PersonProvisioner. RFC-WS-6.md -> v1.1 §3 Q8 addendum. +> v1.1 §3 Q9 addendum. > > - v2.6: WS-5d — `form_fields.options` and `form_field_library.options` > JSON columns **dropped**; replaced by a single polymorphic relational @@ -1959,7 +1959,7 @@ that aggregates the user's submitted, non-test `form_submissions`. | `name` | string | | | `slug` | string | canonical within organisation | | `purpose` | string(50) | Slug matching a key in `PurposeRegistry::all()`; `FormPurpose` enum mirrors the seven v1.0 values | -| `default_crowd_type_id` | ULID nullable | Default CrowdType for event_registration Person provisioning. NULL allowed for non-event_registration purposes; required at publish time for event_registration via `RequiresDefaultCrowdType` guard. **v2.7 — RFC-WS-6 v1.1 §3 Q8 addendum** | +| `default_crowd_type_id` | ULID nullable | Default CrowdType for event_registration Person provisioning. NULL allowed for non-event_registration purposes; required at publish time for event_registration via `RequiresDefaultCrowdType` guard. **v2.7 — RFC-WS-6 v1.1 §3 Q9 addendum** | | `description` | text nullable | | | `is_published` | bool | default: false | | `submission_mode` | string(20) | `FormSubmissionMode` enum value |