From a5190ee30974f007ee9d584f10f5d4809ebb62b9 Mon Sep 17 00:00:00 2001 From: "bert.hausmans" Date: Fri, 8 May 2026 19:42:36 +0200 Subject: [PATCH] =?UTF-8?q?fix(timetable):=20null-on-delete=20advance=5Fsu?= =?UTF-8?q?bmissions=20per=20RFC=20=C2=A75.4=20retention?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit advance_submissions.advance_section_id FK changed from cascadeOnDelete to nullOnDelete; column made nullable. Aligns implementation with RFC v0.2 §5.4 audit-immutability ("submissions remain for retention compliance") — when ArtistEngagementObserver::deleted hard-deletes a section, its submissions persist as orphans rather than disappearing. Migration edited in place (branch unpushed, dev-only). Observer docblock + test assertion updated to match. Removed pre-existing follow-up comment that documented the deviation. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Observers/ArtistEngagementObserver.php | 8 ++++---- ...00009_create_advance_submissions_table.php | 2 +- api/database/schema/mysql-schema.sql | 4 ++-- .../Artist/ArtistEngagementObserverTest.php | 20 ++++++++++++------- 4 files changed, 20 insertions(+), 14 deletions(-) diff --git a/api/app/Observers/ArtistEngagementObserver.php b/api/app/Observers/ArtistEngagementObserver.php index 43a3995d..ff36d029 100644 --- a/api/app/Observers/ArtistEngagementObserver.php +++ b/api/app/Observers/ArtistEngagementObserver.php @@ -15,10 +15,10 @@ use App\Models\Scopes\OrganisationScope; * (RFC v0.2 D10) and asserts artist.organisation_id === event.organisation_id. * * On `deleted` (soft delete): cascade soft-delete to child Performance - * rows; hard-delete child AdvanceSection rows (RFC §5.4 — sections have - * no soft delete). Child AdvanceSubmission rows are immutable audit - * records and remain attached to their (now-deleted) section reference - * by FK — this is the audit-retention compliance path. + * rows; hard-delete child AdvanceSection rows (per RFC §5.4 — sections + * have no soft delete). AdvanceSubmission rows remain in the table with + * `advance_section_id = NULL` (FK is `nullOnDelete`) per RFC §5.4 + * audit-immutability — orphaned but preserved for retention compliance. */ final class ArtistEngagementObserver { diff --git a/api/database/migrations/2026_05_08_100009_create_advance_submissions_table.php b/api/database/migrations/2026_05_08_100009_create_advance_submissions_table.php index 1bf9f4a9..3226ed19 100644 --- a/api/database/migrations/2026_05_08_100009_create_advance_submissions_table.php +++ b/api/database/migrations/2026_05_08_100009_create_advance_submissions_table.php @@ -12,7 +12,7 @@ return new class extends Migration { Schema::create('advance_submissions', function (Blueprint $table) { $table->ulid('id')->primary(); - $table->foreignUlid('advance_section_id')->constrained()->cascadeOnDelete(); + $table->foreignUlid('advance_section_id')->nullable()->constrained()->nullOnDelete(); $table->string('submitted_by_name'); $table->string('submitted_by_email'); $table->timestamp('submitted_at'); diff --git a/api/database/schema/mysql-schema.sql b/api/database/schema/mysql-schema.sql index fcc343e0..6a6d947a 100644 --- a/api/database/schema/mysql-schema.sql +++ b/api/database/schema/mysql-schema.sql @@ -55,7 +55,7 @@ DROP TABLE IF EXISTS `advance_submissions`; /*!50503 SET character_set_client = utf8mb4 */; CREATE TABLE `advance_submissions` ( `id` char(26) COLLATE utf8mb4_unicode_ci NOT NULL, - `advance_section_id` char(26) COLLATE utf8mb4_unicode_ci NOT NULL, + `advance_section_id` char(26) COLLATE utf8mb4_unicode_ci DEFAULT NULL, `submitted_by_name` varchar(255) COLLATE utf8mb4_unicode_ci NOT NULL, `submitted_by_email` varchar(255) COLLATE utf8mb4_unicode_ci NOT NULL, `submitted_at` timestamp NOT NULL, @@ -68,7 +68,7 @@ CREATE TABLE `advance_submissions` ( PRIMARY KEY (`id`), KEY `advance_submissions_reviewed_by_foreign` (`reviewed_by`), KEY `advance_submissions_advance_section_id_status_index` (`advance_section_id`,`status`), - CONSTRAINT `advance_submissions_advance_section_id_foreign` FOREIGN KEY (`advance_section_id`) REFERENCES `advance_sections` (`id`) ON DELETE CASCADE, + CONSTRAINT `advance_submissions_advance_section_id_foreign` FOREIGN KEY (`advance_section_id`) REFERENCES `advance_sections` (`id`) ON DELETE SET NULL, CONSTRAINT `advance_submissions_reviewed_by_foreign` FOREIGN KEY (`reviewed_by`) REFERENCES `users` (`id`) ON DELETE SET NULL ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; /*!40101 SET character_set_client = @saved_cs_client */; diff --git a/api/tests/Feature/Artist/ArtistEngagementObserverTest.php b/api/tests/Feature/Artist/ArtistEngagementObserverTest.php index b93a9749..3b079ff6 100644 --- a/api/tests/Feature/Artist/ArtistEngagementObserverTest.php +++ b/api/tests/Feature/Artist/ArtistEngagementObserverTest.php @@ -81,7 +81,7 @@ final class ArtistEngagementObserverTest extends TestCase 'type' => 'production', ]); - AdvanceSubmission::create([ + $submission = AdvanceSubmission::create([ 'advance_section_id' => $section->id, 'submitted_by_name' => 'TM', 'submitted_by_email' => 'tm@example.test', @@ -90,6 +90,8 @@ final class ArtistEngagementObserverTest extends TestCase 'data' => [], ]); + $submissionCountBefore = AdvanceSubmission::withoutGlobalScope(OrganisationScope::class)->count(); + $eng->delete(); // Performance is soft-deleted (trashed, not removed). @@ -99,11 +101,15 @@ final class ArtistEngagementObserverTest extends TestCase // AdvanceSection is hard-deleted. $this->assertNull(AdvanceSection::withoutGlobalScope(OrganisationScope::class)->find($section->id)); - // Note: `advance_submissions.advance_section_id` currently uses - // `cascadeOnDelete()`, so submissions are removed with their section. - // RFC v0.2 §5.4 calls submissions "audit-immutable" — interpreted - // here as "no application code mutates them post-creation". A - // future migration may switch the FK to nullOnDelete to preserve - // rows past section hard-delete; out of Session 1 scope. + // AdvanceSubmission survives as audit-orphan per RFC §5.4: row + // preserved, advance_section_id nulled by FK ON DELETE SET NULL. + $this->assertSame( + $submissionCountBefore, + AdvanceSubmission::withoutGlobalScope(OrganisationScope::class)->count(), + 'advance_submissions must persist for retention compliance' + ); + $orphan = AdvanceSubmission::withoutGlobalScope(OrganisationScope::class)->find($submission->id); + $this->assertNotNull($orphan); + $this->assertNull($orphan->advance_section_id); } }