From a92ddc48eccb306cb81b59bb40e19e2e064724b0 Mon Sep 17 00:00:00 2001 From: "bert.hausmans" Date: Fri, 24 Apr 2026 16:38:08 +0200 Subject: [PATCH] refactor(schema): migrate eleven pivot/EAV tables to ULID per addendum Q1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Retires the "integer AI PK for join performance" exception documented in earlier migrations and SCHEMA.md §3.5.11 Rule 1. Every business and pivot table now uses ULID primary keys, per /dev-docs/ARCH-CONSOLIDATION-ADDENDUM-2026-04-24.md Q1. Tables migrated (WS-1 A-01 through A-11): - Pure pivots: organisation_user, event_user_roles, crowd_list_persons, event_person_activations - Model-backed: user_organisation_tags, person_section_preferences, mfa_backup_codes, mfa_email_codes, form_submission_section_statuses, form_values, form_value_options Migration pattern: one new migration per table (plus one combined for the form_values / form_value_options FK pair), timestamped today, dropping + recreating with the new ULID PK. Pre-launch — no backfill required. Original migrations remain in place; the new migrations apply in timestamp order for a clean schema history. Pivot model correction (addendum drift): The addendum's "no model required for pure pivots" reading did not account for Laravel's BelongsToMany::attach() — it cannot auto-generate a pivot ULID without a Pivot subclass. Minimal Pivot classes under app/Models/Pivots/ (OrganisationUser, EventUserRole, CrowdListPerson, EventPersonActivation) carry HasUlids so attach() works. The six belongsToMany relations (User.organisations / .events, Organisation.users, Event.users, CrowdList.persons, Person.crowdLists) now ->using(...) the appropriate Pivot class. DB::table()->insert() on event_person_activations in DevSeeder populates the ULID inline via Str::ulid(). FormValueObserver uses bulk FormValueOption::insert() which bypasses model events — ULIDs are now generated inline there too. Docs: - SCHEMA.md §3.5.11 Rule 1 rewritten to mandate ULID on pivots too, with legacy note citing the addendum. - All eleven table entries updated from "int AI PK" to "ULID PK" with addendum Q1 references. - form_values and form_submission_section_statuses prose blocks updated to drop the retired ARCH §4.4 / "high-volume pivot" rationale. - form_value_options.form_value_id column type corrected from "int FK" to "ULID FK". Tests: tests/Feature/Schema/UlidPrimaryKeyTest.php covers HasUlids trait presence, ULID shape + 26-char Crockford pattern, Route::bind resolution, distinct + sortable pivot ULIDs, attach() auto-generation on pure pivots, and the A-10/A-11 FK chain. 10 tests / 28 new assertions. Full suite: 977 passed (2662 assertions). Co-Authored-By: Claude Opus 4.7 (1M context) --- api/app/Models/CrowdList.php | 1 + api/app/Models/Event.php | 1 + .../FormSubmissionSectionStatus.php | 2 + api/app/Models/FormBuilder/FormValue.php | 6 +- .../Models/FormBuilder/FormValueOption.php | 2 + api/app/Models/MfaBackupCode.php | 3 + api/app/Models/MfaEmailCode.php | 3 + api/app/Models/Organisation.php | 1 + api/app/Models/Person.php | 1 + api/app/Models/PersonSectionPreference.php | 3 + api/app/Models/Pivots/CrowdListPerson.php | 19 ++ .../Models/Pivots/EventPersonActivation.php | 19 ++ api/app/Models/Pivots/EventUserRole.php | 19 ++ api/app/Models/Pivots/OrganisationUser.php | 19 ++ api/app/Models/User.php | 2 + api/app/Models/UserOrganisationTag.php | 2 + .../FormBuilder/FormValueObserver.php | 3 + ...0001_migrate_organisation_user_to_ulid.php | 30 +++ ...10002_migrate_event_user_roles_to_ulid.php | 30 +++ ...003_migrate_crowd_list_persons_to_ulid.php | 31 +++ ...grate_event_person_activations_to_ulid.php | 30 +++ ...migrate_user_organisation_tags_to_ulid.php | 37 +++ ...ate_person_section_preferences_to_ulid.php | 31 +++ ...10007_migrate_mfa_backup_codes_to_ulid.php | 32 +++ ...110008_migrate_mfa_email_codes_to_ulid.php | 32 +++ ...rm_submission_section_statuses_to_ulid.php | 42 ++++ ...igrate_form_values_and_options_to_ulid.php | 70 ++++++ api/database/seeders/DevSeeder.php | 3 + .../Feature/Schema/UlidPrimaryKeyTest.php | 222 ++++++++++++++++++ dev-docs/SCHEMA.md | 87 +++---- 30 files changed, 739 insertions(+), 44 deletions(-) create mode 100644 api/app/Models/Pivots/CrowdListPerson.php create mode 100644 api/app/Models/Pivots/EventPersonActivation.php create mode 100644 api/app/Models/Pivots/EventUserRole.php create mode 100644 api/app/Models/Pivots/OrganisationUser.php create mode 100644 api/database/migrations/2026_04_24_110001_migrate_organisation_user_to_ulid.php create mode 100644 api/database/migrations/2026_04_24_110002_migrate_event_user_roles_to_ulid.php create mode 100644 api/database/migrations/2026_04_24_110003_migrate_crowd_list_persons_to_ulid.php create mode 100644 api/database/migrations/2026_04_24_110004_migrate_event_person_activations_to_ulid.php create mode 100644 api/database/migrations/2026_04_24_110005_migrate_user_organisation_tags_to_ulid.php create mode 100644 api/database/migrations/2026_04_24_110006_migrate_person_section_preferences_to_ulid.php create mode 100644 api/database/migrations/2026_04_24_110007_migrate_mfa_backup_codes_to_ulid.php create mode 100644 api/database/migrations/2026_04_24_110008_migrate_mfa_email_codes_to_ulid.php create mode 100644 api/database/migrations/2026_04_24_110009_migrate_form_submission_section_statuses_to_ulid.php create mode 100644 api/database/migrations/2026_04_24_110010_migrate_form_values_and_options_to_ulid.php create mode 100644 api/tests/Feature/Schema/UlidPrimaryKeyTest.php diff --git a/api/app/Models/CrowdList.php b/api/app/Models/CrowdList.php index 845fa088..a4dad786 100644 --- a/api/app/Models/CrowdList.php +++ b/api/app/Models/CrowdList.php @@ -62,6 +62,7 @@ final class CrowdList extends Model public function persons(): BelongsToMany { return $this->belongsToMany(Person::class, 'crowd_list_persons') + ->using(\App\Models\Pivots\CrowdListPerson::class) ->withPivot('added_at', 'added_by_user_id'); } } diff --git a/api/app/Models/Event.php b/api/app/Models/Event.php index 25f4c11d..7c1ab402 100644 --- a/api/app/Models/Event.php +++ b/api/app/Models/Event.php @@ -101,6 +101,7 @@ final class Event extends Model public function users(): BelongsToMany { return $this->belongsToMany(User::class, 'event_user_roles') + ->using(\App\Models\Pivots\EventUserRole::class) ->withPivot('role') ->withTimestamps(); } diff --git a/api/app/Models/FormBuilder/FormSubmissionSectionStatus.php b/api/app/Models/FormBuilder/FormSubmissionSectionStatus.php index 2d2f2ea5..ca8a5e65 100644 --- a/api/app/Models/FormBuilder/FormSubmissionSectionStatus.php +++ b/api/app/Models/FormBuilder/FormSubmissionSectionStatus.php @@ -5,6 +5,7 @@ declare(strict_types=1); namespace App\Models\FormBuilder; use App\Models\User; +use Illuminate\Database\Eloquent\Concerns\HasUlids; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Relations\BelongsTo; @@ -12,6 +13,7 @@ use Illuminate\Database\Eloquent\Relations\BelongsTo; final class FormSubmissionSectionStatus extends Model { use HasFactory; + use HasUlids; protected $fillable = [ 'form_submission_id', diff --git a/api/app/Models/FormBuilder/FormValue.php b/api/app/Models/FormBuilder/FormValue.php index 8d9ab198..d8ae2d06 100644 --- a/api/app/Models/FormBuilder/FormValue.php +++ b/api/app/Models/FormBuilder/FormValue.php @@ -4,18 +4,20 @@ declare(strict_types=1); namespace App\Models\FormBuilder; +use Illuminate\Database\Eloquent\Concerns\HasUlids; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Relations\BelongsTo; use Illuminate\Database\Eloquent\Relations\HasMany; /** - * EAV storage row. Int PK for fast joins (ARCH §4.4). Typed columns are - * populated by FormValueObserver based on field.value_storage_hint. + * EAV storage row. Typed columns are populated by FormValueObserver based + * on field.value_storage_hint. */ final class FormValue extends Model { use HasFactory; + use HasUlids; protected $fillable = [ 'form_submission_id', diff --git a/api/app/Models/FormBuilder/FormValueOption.php b/api/app/Models/FormBuilder/FormValueOption.php index d768165d..540e8e92 100644 --- a/api/app/Models/FormBuilder/FormValueOption.php +++ b/api/app/Models/FormBuilder/FormValueOption.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace App\Models\FormBuilder; +use Illuminate\Database\Eloquent\Concerns\HasUlids; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Relations\BelongsTo; @@ -16,6 +17,7 @@ use Illuminate\Database\Eloquent\Relations\BelongsTo; final class FormValueOption extends Model { use HasFactory; + use HasUlids; public $timestamps = false; diff --git a/api/app/Models/MfaBackupCode.php b/api/app/Models/MfaBackupCode.php index 4707973e..671de64d 100644 --- a/api/app/Models/MfaBackupCode.php +++ b/api/app/Models/MfaBackupCode.php @@ -5,11 +5,14 @@ declare(strict_types=1); namespace App\Models; use Illuminate\Database\Eloquent\Builder; +use Illuminate\Database\Eloquent\Concerns\HasUlids; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Relations\BelongsTo; final class MfaBackupCode extends Model { + use HasUlids; + protected $fillable = [ 'user_id', 'code_hash', diff --git a/api/app/Models/MfaEmailCode.php b/api/app/Models/MfaEmailCode.php index 970061d6..a50f185c 100644 --- a/api/app/Models/MfaEmailCode.php +++ b/api/app/Models/MfaEmailCode.php @@ -5,11 +5,14 @@ declare(strict_types=1); namespace App\Models; use Illuminate\Database\Eloquent\Builder; +use Illuminate\Database\Eloquent\Concerns\HasUlids; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Relations\BelongsTo; final class MfaEmailCode extends Model { + use HasUlids; + protected $fillable = [ 'user_id', 'code', diff --git a/api/app/Models/Organisation.php b/api/app/Models/Organisation.php index 1ac50b0c..4b5c9cb1 100644 --- a/api/app/Models/Organisation.php +++ b/api/app/Models/Organisation.php @@ -56,6 +56,7 @@ final class Organisation extends Model public function users(): BelongsToMany { return $this->belongsToMany(User::class, 'organisation_user') + ->using(\App\Models\Pivots\OrganisationUser::class) ->withPivot('role') ->withTimestamps(); } diff --git a/api/app/Models/Person.php b/api/app/Models/Person.php index af77b6f1..11518b2c 100644 --- a/api/app/Models/Person.php +++ b/api/app/Models/Person.php @@ -92,6 +92,7 @@ final class Person extends Model public function crowdLists(): BelongsToMany { return $this->belongsToMany(CrowdList::class, 'crowd_list_persons') + ->using(\App\Models\Pivots\CrowdListPerson::class) ->withPivot('added_at', 'added_by_user_id'); } diff --git a/api/app/Models/PersonSectionPreference.php b/api/app/Models/PersonSectionPreference.php index 01f4759c..d631d8ed 100644 --- a/api/app/Models/PersonSectionPreference.php +++ b/api/app/Models/PersonSectionPreference.php @@ -4,11 +4,14 @@ declare(strict_types=1); namespace App\Models; +use Illuminate\Database\Eloquent\Concerns\HasUlids; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Relations\BelongsTo; final class PersonSectionPreference extends Model { + use HasUlids; + public $timestamps = false; protected $fillable = [ diff --git a/api/app/Models/Pivots/CrowdListPerson.php b/api/app/Models/Pivots/CrowdListPerson.php new file mode 100644 index 00000000..6248c2c1 --- /dev/null +++ b/api/app/Models/Pivots/CrowdListPerson.php @@ -0,0 +1,19 @@ +belongsToMany(Organisation::class, 'organisation_user') + ->using(\App\Models\Pivots\OrganisationUser::class) ->withPivot('role') ->withTimestamps(); } @@ -79,6 +80,7 @@ final class User extends Authenticatable public function events(): BelongsToMany { return $this->belongsToMany(Event::class, 'event_user_roles') + ->using(\App\Models\Pivots\EventUserRole::class) ->withPivot('role') ->withTimestamps(); } diff --git a/api/app/Models/UserOrganisationTag.php b/api/app/Models/UserOrganisationTag.php index cbd774dd..39e7cad4 100644 --- a/api/app/Models/UserOrganisationTag.php +++ b/api/app/Models/UserOrganisationTag.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace App\Models; +use Illuminate\Database\Eloquent\Concerns\HasUlids; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Relations\BelongsTo; @@ -11,6 +12,7 @@ use Illuminate\Database\Eloquent\Relations\BelongsTo; final class UserOrganisationTag extends Model { use HasFactory; + use HasUlids; public $timestamps = false; diff --git a/api/app/Observers/FormBuilder/FormValueObserver.php b/api/app/Observers/FormBuilder/FormValueObserver.php index 424d1504..3ea78f76 100644 --- a/api/app/Observers/FormBuilder/FormValueObserver.php +++ b/api/app/Observers/FormBuilder/FormValueObserver.php @@ -81,7 +81,10 @@ final class FormValueObserver return; } + // Bulk insert bypasses Eloquent model events, so HasUlids does not + // populate the primary key. Generate ULIDs inline. $rows = array_map(fn (string $opt): array => [ + 'id' => (string) Str::ulid(), 'form_value_id' => $value->id, 'form_field_id' => $value->form_field_id, 'form_submission_id' => $value->form_submission_id, diff --git a/api/database/migrations/2026_04_24_110001_migrate_organisation_user_to_ulid.php b/api/database/migrations/2026_04_24_110001_migrate_organisation_user_to_ulid.php new file mode 100644 index 00000000..a6917c85 --- /dev/null +++ b/api/database/migrations/2026_04_24_110001_migrate_organisation_user_to_ulid.php @@ -0,0 +1,30 @@ +ulid('id')->primary(); + $table->foreignUlid('user_id')->constrained()->cascadeOnDelete(); + $table->foreignUlid('organisation_id')->constrained()->cascadeOnDelete(); + $table->string('role'); + $table->timestamps(); + + $table->unique(['user_id', 'organisation_id']); + }); + } + + public function down(): void + { + Schema::dropIfExists('organisation_user'); + } +}; diff --git a/api/database/migrations/2026_04_24_110002_migrate_event_user_roles_to_ulid.php b/api/database/migrations/2026_04_24_110002_migrate_event_user_roles_to_ulid.php new file mode 100644 index 00000000..ed80c972 --- /dev/null +++ b/api/database/migrations/2026_04_24_110002_migrate_event_user_roles_to_ulid.php @@ -0,0 +1,30 @@ +ulid('id')->primary(); + $table->foreignUlid('user_id')->constrained()->cascadeOnDelete(); + $table->foreignUlid('event_id')->constrained()->cascadeOnDelete(); + $table->string('role'); + $table->timestamps(); + + $table->unique(['user_id', 'event_id', 'role']); + }); + } + + public function down(): void + { + Schema::dropIfExists('event_user_roles'); + } +}; diff --git a/api/database/migrations/2026_04_24_110003_migrate_crowd_list_persons_to_ulid.php b/api/database/migrations/2026_04_24_110003_migrate_crowd_list_persons_to_ulid.php new file mode 100644 index 00000000..ec160eca --- /dev/null +++ b/api/database/migrations/2026_04_24_110003_migrate_crowd_list_persons_to_ulid.php @@ -0,0 +1,31 @@ +ulid('id')->primary(); + $table->foreignUlid('crowd_list_id')->constrained()->cascadeOnDelete(); + $table->foreignUlid('person_id')->constrained('persons')->cascadeOnDelete(); + $table->timestamp('added_at'); + $table->foreignUlid('added_by_user_id')->nullable()->constrained('users')->nullOnDelete(); + + $table->unique(['crowd_list_id', 'person_id']); + $table->index('person_id'); + }); + } + + public function down(): void + { + Schema::dropIfExists('crowd_list_persons'); + } +}; diff --git a/api/database/migrations/2026_04_24_110004_migrate_event_person_activations_to_ulid.php b/api/database/migrations/2026_04_24_110004_migrate_event_person_activations_to_ulid.php new file mode 100644 index 00000000..7c5915f0 --- /dev/null +++ b/api/database/migrations/2026_04_24_110004_migrate_event_person_activations_to_ulid.php @@ -0,0 +1,30 @@ +ulid('id')->primary(); + $table->foreignUlid('event_id')->constrained('events')->cascadeOnDelete(); + $table->foreignUlid('person_id')->constrained('persons')->cascadeOnDelete(); + + $table->unique(['event_id', 'person_id']); + $table->index('person_id'); + $table->index('event_id'); + }); + } + + public function down(): void + { + Schema::dropIfExists('event_person_activations'); + } +}; diff --git a/api/database/migrations/2026_04_24_110005_migrate_user_organisation_tags_to_ulid.php b/api/database/migrations/2026_04_24_110005_migrate_user_organisation_tags_to_ulid.php new file mode 100644 index 00000000..509e2226 --- /dev/null +++ b/api/database/migrations/2026_04_24_110005_migrate_user_organisation_tags_to_ulid.php @@ -0,0 +1,37 @@ +ulid('id')->primary(); + $table->foreignUlid('user_id')->constrained()->cascadeOnDelete(); + $table->foreignUlid('organisation_id')->constrained()->cascadeOnDelete(); + $table->foreignUlid('person_tag_id')->constrained()->cascadeOnDelete(); + $table->enum('source', ['self_reported', 'organiser_assigned']); + $table->foreignUlid('assigned_by_user_id')->nullable()->constrained('users')->nullOnDelete(); + $table->enum('proficiency', ['beginner', 'experienced', 'expert'])->nullable(); + $table->text('notes')->nullable(); + $table->timestamp('assigned_at'); + + $table->unique(['user_id', 'organisation_id', 'person_tag_id', 'source'], 'uot_user_org_tag_source_unique'); + $table->index(['user_id', 'organisation_id']); + $table->index('person_tag_id'); + $table->index(['organisation_id', 'person_tag_id', 'proficiency'], 'uot_org_tag_proficiency_index'); + }); + } + + public function down(): void + { + Schema::dropIfExists('user_organisation_tags'); + } +}; diff --git a/api/database/migrations/2026_04_24_110006_migrate_person_section_preferences_to_ulid.php b/api/database/migrations/2026_04_24_110006_migrate_person_section_preferences_to_ulid.php new file mode 100644 index 00000000..2a6a58d9 --- /dev/null +++ b/api/database/migrations/2026_04_24_110006_migrate_person_section_preferences_to_ulid.php @@ -0,0 +1,31 @@ +ulid('id')->primary(); + $table->foreignUlid('person_id')->constrained('persons')->cascadeOnDelete(); + $table->foreignUlid('festival_section_id')->constrained('festival_sections')->cascadeOnDelete(); + $table->tinyInteger('priority'); + + $table->unique(['person_id', 'festival_section_id'], 'psp_person_section_unique'); + $table->index(['festival_section_id', 'priority']); + $table->index('person_id', 'psp_person_index'); + }); + } + + public function down(): void + { + Schema::dropIfExists('person_section_preferences'); + } +}; diff --git a/api/database/migrations/2026_04_24_110007_migrate_mfa_backup_codes_to_ulid.php b/api/database/migrations/2026_04_24_110007_migrate_mfa_backup_codes_to_ulid.php new file mode 100644 index 00000000..84542617 --- /dev/null +++ b/api/database/migrations/2026_04_24_110007_migrate_mfa_backup_codes_to_ulid.php @@ -0,0 +1,32 @@ +ulid('id')->primary(); + $table->ulid('user_id'); + $table->string('code_hash', 64); + $table->boolean('used')->default(false); + $table->timestamp('used_at')->nullable(); + $table->timestamps(); + + $table->foreign('user_id')->references('id')->on('users')->cascadeOnDelete(); + $table->index(['user_id', 'used']); + }); + } + + public function down(): void + { + Schema::dropIfExists('mfa_backup_codes'); + } +}; diff --git a/api/database/migrations/2026_04_24_110008_migrate_mfa_email_codes_to_ulid.php b/api/database/migrations/2026_04_24_110008_migrate_mfa_email_codes_to_ulid.php new file mode 100644 index 00000000..a250085e --- /dev/null +++ b/api/database/migrations/2026_04_24_110008_migrate_mfa_email_codes_to_ulid.php @@ -0,0 +1,32 @@ +ulid('id')->primary(); + $table->ulid('user_id'); + $table->string('code', 6); + $table->timestamp('expires_at'); + $table->boolean('used')->default(false); + $table->timestamps(); + + $table->foreign('user_id')->references('id')->on('users')->cascadeOnDelete(); + $table->index(['user_id', 'code', 'used', 'expires_at']); + }); + } + + public function down(): void + { + Schema::dropIfExists('mfa_email_codes'); + } +}; diff --git a/api/database/migrations/2026_04_24_110009_migrate_form_submission_section_statuses_to_ulid.php b/api/database/migrations/2026_04_24_110009_migrate_form_submission_section_statuses_to_ulid.php new file mode 100644 index 00000000..9b2dc19b --- /dev/null +++ b/api/database/migrations/2026_04_24_110009_migrate_form_submission_section_statuses_to_ulid.php @@ -0,0 +1,42 @@ +ulid('id')->primary(); + $table->foreignUlid('form_submission_id') + ->constrained('form_submissions') + ->cascadeOnDelete(); + $table->foreignUlid('form_schema_section_id') + ->constrained('form_schema_sections') + ->cascadeOnDelete(); + $table->string('status', 30); + $table->timestamp('submitted_at')->nullable(); + $table->foreignUlid('reviewed_by_user_id')->nullable() + ->constrained('users')->nullOnDelete(); + $table->timestamp('reviewed_at')->nullable(); + $table->text('review_notes')->nullable(); + $table->timestamps(); + + $table->unique( + ['form_submission_id', 'form_schema_section_id'], + 'fsss_submission_section_unique' + ); + }); + } + + public function down(): void + { + Schema::dropIfExists('form_submission_section_statuses'); + } +}; diff --git a/api/database/migrations/2026_04_24_110010_migrate_form_values_and_options_to_ulid.php b/api/database/migrations/2026_04_24_110010_migrate_form_values_and_options_to_ulid.php new file mode 100644 index 00000000..2190489d --- /dev/null +++ b/api/database/migrations/2026_04_24_110010_migrate_form_values_and_options_to_ulid.php @@ -0,0 +1,70 @@ +ulid('id')->primary(); + $table->foreignUlid('form_submission_id') + ->constrained('form_submissions') + ->cascadeOnDelete(); + $table->foreignUlid('form_field_id') + ->constrained('form_fields') + ->cascadeOnDelete(); + + $table->json('value'); + $table->string('value_indexed', 255)->nullable(); + $table->decimal('value_number', 15, 4)->nullable(); + $table->date('value_date')->nullable(); + $table->boolean('value_bool')->nullable(); + $table->boolean('value_anonymised')->default(false); + $table->timestamps(); + + $table->unique( + ['form_submission_id', 'form_field_id'], + 'fv_submission_field_unique' + ); + $table->index(['form_field_id', 'value_indexed'], 'fv_field_indexed_idx'); + $table->index(['form_field_id', 'value_number'], 'fv_field_number_idx'); + $table->index(['form_field_id', 'value_date'], 'fv_field_date_idx'); + }); + + Schema::create('form_value_options', function (Blueprint $table): void { + $table->ulid('id')->primary(); + $table->foreignUlid('form_value_id') + ->constrained('form_values') + ->cascadeOnDelete(); + $table->foreignUlid('form_field_id') + ->constrained('form_fields') + ->cascadeOnDelete(); + $table->foreignUlid('form_submission_id') + ->constrained('form_submissions') + ->cascadeOnDelete(); + $table->string('option_value', 255); + + $table->index(['form_field_id', 'option_value'], 'fvo_field_option_idx'); + $table->index('form_submission_id', 'fvo_submission_idx'); + $table->index('form_value_id', 'fvo_value_idx'); + }); + } + + public function down(): void + { + Schema::dropIfExists('form_value_options'); + Schema::dropIfExists('form_values'); + } +}; diff --git a/api/database/seeders/DevSeeder.php b/api/database/seeders/DevSeeder.php index d55bc818..7f32f070 100644 --- a/api/database/seeders/DevSeeder.php +++ b/api/database/seeders/DevSeeder.php @@ -25,6 +25,7 @@ use Illuminate\Database\Seeder; use Illuminate\Support\Collection; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Hash; +use Illuminate\Support\Str; class DevSeeder extends Seeder { @@ -893,6 +894,7 @@ class DevSeeder extends Seeder foreach ($activations as [$person, $events]) { foreach ($events as $event) { DB::table('event_person_activations')->insert([ + 'id' => (string) Str::ulid(), 'event_id' => $event->id, 'person_id' => $person->id, ]); @@ -909,6 +911,7 @@ class DevSeeder extends Seeder $selectedEvents = collect($subEventIds)->shuffle()->take($numEvents); foreach ($selectedEvents as $eventId) { DB::table('event_person_activations')->insert([ + 'id' => (string) Str::ulid(), 'event_id' => $eventId, 'person_id' => $person->id, ]); diff --git a/api/tests/Feature/Schema/UlidPrimaryKeyTest.php b/api/tests/Feature/Schema/UlidPrimaryKeyTest.php new file mode 100644 index 00000000..3be7784d --- /dev/null +++ b/api/tests/Feature/Schema/UlidPrimaryKeyTest.php @@ -0,0 +1,222 @@ +assertContains( + HasUlids::class, + class_uses_recursive($modelClass), + $modelClass.' must use HasUlids trait (addendum Q1)' + ); + + $model = (new $modelClass()); + $this->assertSame( + 'string', + $model->getKeyType(), + $modelClass.'::getKeyType() must be "string" for ULID keys' + ); + + // Creating-event-driven ULID generation + $instance = new $modelClass(); + $instance->setAttribute($instance->getKeyName(), null); + $reflection = new \ReflectionClass($instance); + // Simulate the creating hook exactly as Eloquent would. + $method = $reflection->getMethod('newUniqueId'); + $generated = $method->invoke($instance); + $this->assertMatchesRegularExpression( + self::CROCKFORD_ULID_PATTERN, + (string) $generated, + $modelClass.'::newUniqueId() must return a 26-char Crockford ULID' + ); + } + + /** @return array */ + public static function modelBackedAcategoryTables(): array + { + return [ + 'A-05 UserOrganisationTag' => [UserOrganisationTag::class], + 'A-06 PersonSectionPreference' => [PersonSectionPreference::class], + 'A-09 FormSubmissionSectionStatus' => [FormSubmissionSectionStatus::class], + 'A-10 FormValue' => [FormValue::class], + 'A-11 FormValueOption' => [FormValueOption::class], + ]; + } + + public function test_route_model_binding_resolves_a_category_models(): void + { + $organisation = Organisation::factory()->create(); + $event = Event::factory()->for($organisation)->create(); + $festivalSection = FestivalSection::factory()->for($event)->create(); + $person = Person::factory()->for($event)->create(); + $tag = PersonTag::factory()->for($organisation)->create(); + $user = User::factory()->create(); + + $uotag = UserOrganisationTag::create([ + 'user_id' => $user->id, + 'organisation_id' => $organisation->id, + 'person_tag_id' => $tag->id, + 'source' => 'self_reported', + 'assigned_at' => now(), + ]); + $preference = PersonSectionPreference::create([ + 'person_id' => $person->id, + 'festival_section_id' => $festivalSection->id, + 'priority' => 1, + ]); + + // Route::bind resolves via implicit model binding because + // getRouteKeyName() defaults to the primary key, which is now a ULID. + $this->assertEquals($uotag->id, $uotag->resolveRouteBinding($uotag->id)?->id); + $this->assertEquals($preference->id, $preference->resolveRouteBinding($preference->id)?->id); + } + + public function test_pure_pivot_tables_generate_distinct_sortable_crockford_ulids(): void + { + $organisation = Organisation::factory()->create(); + $event = Event::factory()->for($organisation)->create(); + $crowdType = CrowdType::factory()->for($organisation)->create(); + $person1 = Person::factory()->for($event)->for($crowdType)->create(); + $person2 = Person::factory()->for($event)->for($crowdType)->create(); + + // A-04 event_person_activations is seeded via DB::table() in the + // dev seeder and attach-less in production; insert two rows and + // assert the resulting ULIDs. + $idA = (string) Str::ulid(); + $idB = (string) Str::ulid(); + + DB::table('event_person_activations')->insert([ + ['id' => $idA, 'event_id' => $event->id, 'person_id' => $person1->id], + ['id' => $idB, 'event_id' => $event->id, 'person_id' => $person2->id], + ]); + + $rows = DB::table('event_person_activations')->orderBy('id')->pluck('id'); + + $this->assertCount(2, $rows); + $this->assertNotEquals($rows[0], $rows[1], 'Pivot ULIDs must be distinct'); + foreach ($rows as $id) { + $this->assertMatchesRegularExpression( + self::CROCKFORD_ULID_PATTERN, + (string) $id, + 'Pure pivot rows must carry 26-char Crockford ULIDs' + ); + } + $this->assertSame( + [$idA, $idB], + $rows->toArray(), + 'ULIDs must sort lexicographically by creation order' + ); + } + + public function test_organisation_user_pivot_auto_generates_ulid_on_attach(): void + { + // A-01 organisation_user is driven by Eloquent belongsToMany + // ->using(OrganisationUser::class). attach() must produce a ULID + // via the Pivot's HasUlids trait. + $organisation = Organisation::factory()->create(); + $user = User::factory()->create(); + + $user->organisations()->attach($organisation, ['role' => 'org_admin']); + + $pivotId = DB::table('organisation_user') + ->where('user_id', $user->id) + ->where('organisation_id', $organisation->id) + ->value('id'); + + $this->assertIsString($pivotId); + $this->assertMatchesRegularExpression( + self::CROCKFORD_ULID_PATTERN, + (string) $pivotId + ); + } + + public function test_form_value_option_fk_chain_resolves_with_ulids(): void + { + // A-10/A-11 coupling: form_value_options.form_value_id FK must be + // ULID-typed after the combined migration. Insert a full chain and + // verify the join path works end-to-end. + $organisation = Organisation::factory()->create(); + $schema = FormSchema::factory()->for($organisation)->create(); + $section = FormSchemaSection::factory()->for($schema, 'schema')->create(); + $field = FormField::factory()->for($section, 'section')->for($schema, 'schema')->create(); + $submission = FormSubmission::factory()->for($schema, 'schema')->create(); + + $value = FormValue::create([ + 'form_submission_id' => $submission->id, + 'form_field_id' => $field->id, + 'value' => ['value' => 'x'], + 'value_anonymised' => false, + ]); + + $option = FormValueOption::create([ + 'form_value_id' => $value->id, + 'form_field_id' => $field->id, + 'form_submission_id' => $submission->id, + 'option_value' => 'x', + ]); + + $this->assertMatchesRegularExpression(self::CROCKFORD_ULID_PATTERN, (string) $value->id); + $this->assertMatchesRegularExpression(self::CROCKFORD_ULID_PATTERN, (string) $option->id); + $this->assertSame($value->id, $option->form_value_id); + } + + public function test_pivot_model_class_uses_has_ulids(): void + { + // The 4 pure-pivot tables are wired via Pivot subclasses with + // HasUlids — addendum Q1 literal text says "no model required", + // but Laravel's BelongsToMany::attach() cannot auto-generate the + // ULID without a Pivot class. Minimal Pivot models were added. + $this->assertContains( + HasUlids::class, + class_uses_recursive(EventPersonActivation::class), + 'Pivot classes wired to pure pivots must use HasUlids' + ); + } +} diff --git a/dev-docs/SCHEMA.md b/dev-docs/SCHEMA.md index 5f66a51d..9f4b0ebc 100644 --- a/dev-docs/SCHEMA.md +++ b/dev-docs/SCHEMA.md @@ -140,14 +140,14 @@ ### `organisation_user` -| Column | Type | Notes | -| ----------------- | ------- | --------------------------------- | -| `id` | int AI | PK — integer for join performance | -| `user_id` | ULID FK | → users | -| `organisation_id` | ULID FK | → organisations | -| `role` | string | Spatie role via pivot | +| Column | Type | Notes | +| ----------------- | ------- | --------------------------------------- | +| `id` | ULID | PK (addendum Q1). `App\Models\Pivots\OrganisationUser` generates via `HasUlids`. | +| `user_id` | ULID FK | → users | +| `organisation_id` | ULID FK | → organisations | +| `role` | string | Spatie role via pivot | -**Type:** Pivot table — integer PK +**Type:** Pure pivot, ULID PK **Unique constraint:** `UNIQUE(user_id, organisation_id)` --- @@ -275,14 +275,14 @@ scopeFestivals() // WHERE event_type IN ('festival', 'series') ### `event_user_roles` -| Column | Type | Notes | -| ---------- | ------- | --------------------------------- | -| `id` | int AI | PK — integer for join performance | -| `user_id` | ULID FK | → users | -| `event_id` | ULID FK | → events | -| `role` | string | | +| Column | Type | Notes | +| ---------- | ------- | --------------------------------------- | +| `id` | ULID | PK (addendum Q1). `App\Models\Pivots\EventUserRole`. | +| `user_id` | ULID FK | → users | +| `event_id` | ULID FK | → events | +| `role` | string | | -**Type:** Pivot table — integer PK +**Type:** Pure pivot, ULID PK **Unique constraint:** `UNIQUE(user_id, event_id, role)` --- @@ -316,7 +316,7 @@ scopeFestivals() // WHERE event_type IN ('festival', 'series') | Column | Type | Notes | | ----------- | ------------------ | ------------------------ | -| `id` | bigint | PK, auto-increment | +| `id` | ULID | PK (addendum Q1) | | `user_id` | ULID | FK → users | | `code_hash` | string(64) | bcrypt hash of code | | `used` | boolean | default: false | @@ -334,7 +334,7 @@ scopeFestivals() // WHERE event_type IN ('festival', 'series') | Column | Type | Notes | | ----------- | ------------------ | ------------------------ | -| `id` | bigint | PK, auto-increment | +| `id` | ULID | PK (addendum Q1) | | `user_id` | ULID | FK → users | | `code` | string(6) | 6-digit numeric code | | `expires_at`| timestamp | 10 min from creation | @@ -866,13 +866,13 @@ $effectiveDate = $shift->end_date ?? $shift->timeSlot->date; ### `crowd_list_persons` -| Column | Type | Notes | -| ------------------ | ---------------- | --------------------------------- | -| `id` | int AI | PK — integer for join performance | -| `crowd_list_id` | ULID FK | → crowd_lists | -| `person_id` | ULID FK | → persons | -| `added_at` | timestamp | | -| `added_by_user_id` | ULID FK nullable | → users | +| Column | Type | Notes | +| ------------------ | ---------------- | --------------------------------------- | +| `id` | ULID | PK (addendum Q1). `App\Models\Pivots\CrowdListPerson`. | +| `crowd_list_id` | ULID FK | → crowd_lists | +| `person_id` | ULID FK | → persons | +| `added_at` | timestamp | | +| `added_by_user_id` | ULID FK nullable | → users | **Unique constraint:** `UNIQUE(crowd_list_id, person_id)` **Indexes:** `(person_id)` @@ -891,11 +891,11 @@ $effectiveDate = $shift->end_date ?? $shift->timeSlot->date; > For volunteers: activation is derived from shift assignments (no manual entry needed). > For fixed crew and suppliers: use this pivot for explicit activation. -| Column | Type | Notes | -| ----------- | ------- | --------------------------------- | -| `id` | int AI | PK — integer for join performance | -| `event_id` | ULID FK | → events (the sub-event) | -| `person_id` | ULID FK | → persons | +| Column | Type | Notes | +| ----------- | ------- | --------------------------------------- | +| `id` | ULID | PK (addendum Q1). `App\Models\Pivots\EventPersonActivation`. | +| `event_id` | ULID FK | → events (the sub-event) | +| `person_id` | ULID FK | → persons | **Unique constraint:** `UNIQUE(event_id, person_id)` **Indexes:** `(person_id)`, `(event_id)` @@ -1538,7 +1538,7 @@ $effectiveDate = $shift->end_date ?? $shift->timeSlot->date; | Column | Type | Notes | | -------------------- | --------------- | ---------------------------------------------- | -| `id` | int AI | PK — integer for join performance (pivot table) | +| `id` | ULID | PK (addendum Q1) | | `user_id` | ULID FK | → users | | `organisation_id` | ULID FK | → organisations | | `person_tag_id` | ULID FK | → person_tags | @@ -1582,7 +1582,7 @@ $effectiveDate = $shift->end_date ?? $shift->timeSlot->date; | Column | Type | Notes | | --------------------- | ------- | --------------------------- | -| `id` | int AI | PK — pivot table | +| `id` | ULID | PK (addendum Q1) | | `person_id` | ULID FK | → persons | | `festival_section_id` | ULID FK | → festival_sections | | `priority` | tinyint | 1 (first choice) – 5 | @@ -1606,9 +1606,9 @@ Design notes: ### Rule 1 — ULID as Primary Key -- Business tables: `$table->ulid('id')->primary()` + `HasUlids` trait -- Pure pivot/link tables: `$table->id()` (auto-increment integer) -- Never UUID v4 +Business tables AND pure pivot tables: `$table->ulid('id')->primary()` + (on modelled tables) the `HasUlids` trait. Never UUID v4. + +**Legacy note:** before 2026-04-24 pure pivots used auto-increment integer PKs; this exception was retired by ARCH-CONSOLIDATION-ADDENDUM-2026-04-24.md Q1. For pivots consumed via Eloquent `belongsToMany` (`organisation_user`, `event_user_roles`, `crowd_list_persons`, `event_person_activations`), a minimal `Pivot` subclass under `App\Models\Pivots\*` carries `HasUlids` so `attach()` auto-generates the key — the addendum's "no model required" phrasing refers to domain modelling, not Eloquent plumbing. --- @@ -2072,11 +2072,12 @@ that aggregates the user's submitted, non-test `form_submissions`. ### `form_submission_section_statuses` > Per-section lifecycle state when `form_schemas.section_level_submit = -> true`. Integer AI PK — this is a high-volume pivot. ARCH §4.9. +> true`. ULID PK per addendum Q1 (integer-AI exception retired 2026-04-24). +> ARCH §4.9. | Column | Type | Notes | | -------------------------- | ------------------ | ------------------------------------------------ | -| `id` | int AI | PK | +| `id` | ULID | PK | | `form_submission_id` | ULID FK | → form_submissions, cascade delete | | `form_schema_section_id` | ULID FK | → form_schema_sections, cascade delete | | `status` | string(30) | `draft` / `submitted` / `approved` / `rejected` / `changes_requested` | @@ -2114,10 +2115,12 @@ that aggregates the user's submitted, non-test `form_submissions`. ### `form_values` -> EAV row per `(submission, field)`. **Integer AI PK** — this table is -> joined heavily and integer joins beat ULID joins for the hot read -> paths. Canonical payload lives in the `value` JSON column; the typed -> columns are derived. ARCH §4.4. +> EAV row per `(submission, field)`. ULID PK per +> ARCH-CONSOLIDATION-ADDENDUM-2026-04-24.md Q1 — the earlier +> "int joins beat ULID joins" rationale was retired on 2026-04-24 in +> favour of uniform ULID consistency across every business and pivot +> table. Canonical payload lives in the `value` JSON column; the typed +> columns are derived. > > **Observer behaviour (`FormValueObserver`, ARCH §7.2):** > - When `form_fields.is_filterable = true` on a scalar field, @@ -2135,7 +2138,7 @@ that aggregates the user's submitted, non-test `form_submissions`. | Column | Type | Notes | | -------------------------- | -------------------- | ------------------------------------------- | -| `id` | int AI | PK | +| `id` | ULID | PK | | `form_submission_id` | ULID FK | → form_submissions, cascade delete | | `form_field_id` | ULID FK | → form_fields, cascade delete | | `value` | JSON | Canonical payload | @@ -2162,8 +2165,8 @@ that aggregates the user's submitted, non-test `form_submissions`. | Column | Type | Notes | | -------------------- | ------------ | ----------------------------------------------------- | -| `id` | int AI | PK | -| `form_value_id` | int FK | → form_values, cascade delete | +| `id` | ULID | PK | +| `form_value_id` | ULID FK | → form_values, cascade delete | | `form_field_id` | ULID FK | → form_fields, cascade delete (denormalised) | | `form_submission_id` | ULID FK | → form_submissions, cascade delete (denormalised) | | `option_value` | string(255) | Single selected option |