From 83fdb9ffa3abf1c3c3651ed24eef7d7330b04ffb Mon Sep 17 00:00:00 2001 From: Jake Bromberg Date: Sat, 22 Aug 2026 21:35:06 -0700 Subject: [PATCH] test(schema): re-home the five FKs #2239 de-declared under the NO ACTION block MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit main is red. #2246 removed five false `onDelete` declarations from schema.ts but left `schema.fk-cascades.test.ts` asserting they were still there, so the suite fails on main at ddfa9445 with 5 failures. CI did not catch it, and the reason generalizes. The unit-tests job runs Jest in affected-tests mode, and this spec reads schema.ts with fs.readFileSync rather than importing it — no edge in Jest's dependency graph, so changing schema.ts never marks the spec as related and it was never selected. #2246's run executed 244 suites / 4917 tests against a full suite of 462 / 8050. Filed as #2249. The five assertions are re-homed rather than deleted. The spec already had a "should NOT have onDelete (intentional NO ACTION)" block, which is now exactly where they belong, so the test keeps its protective value pointing the other way: it pins #2239's decision and fails if someone re-adds a cascade to schema.ts without also moving the deployed constraint. Deleting them would have left the decision unguarded at the unit level. `artist_library_crossreference.library_id` deliberately stays under "cascade" — #2239 did not touch it, and migration 0147 (BS#2112) repaired that constraint to CASCADE in the database, so the declaration is accurate. Adds an `expectNoOnDelete` helper mirroring the existing `expectOnDelete`, since the NO ACTION block was repeating the same four lines per case. Verified: 19/19 in this spec (unchanged count), and the full unit suite at 462 suites / 8050 tests passes, confirming this was the only breakage affected-mode was hiding. Refs #2239, #2246, #2249. --- .../unit/database/schema.fk-cascades.test.ts | 58 ++++++++++++++----- 1 file changed, 42 insertions(+), 16 deletions(-) diff --git a/tests/unit/database/schema.fk-cascades.test.ts b/tests/unit/database/schema.fk-cascades.test.ts index 5d6e7e8e5..775a380cc 100644 --- a/tests/unit/database/schema.fk-cascades.test.ts +++ b/tests/unit/database/schema.fk-cascades.test.ts @@ -34,6 +34,13 @@ function expectOnDelete(tableVar: string, columnDbName: string, expectedAction: expect(block).toContain(`'${expectedAction}'`); } +function expectNoOnDelete(tableVar: string, columnDbName: string) { + const block = getColumnBlock(tableVar, columnDbName); + expect(block).not.toBeNull(); + expect(block).toContain('.references('); + expect(block).not.toContain('onDelete'); +} + describe('FK cascade/set-null rules in schema.ts', () => { describe('should use onDelete: "set null"', () => { it('schedule.assigned_dj_id → user.id', () => { @@ -44,10 +51,6 @@ describe('FK cascade/set-null rules in schema.ts', () => { expectOnDelete('schedule', 'assigned_dj_id2', 'set null'); }); - it('schedule.specialty_id → specialty_shows.id', () => { - expectOnDelete('schedule', 'specialty_id', 'set null'); - }); - it('shows.primary_dj_id → user.id', () => { expectOnDelete('shows', 'primary_dj_id', 'set null'); }); @@ -78,28 +81,51 @@ describe('FK cascade/set-null rules in schema.ts', () => { expectOnDelete('reviews', 'album_id', 'cascade'); }); - it('genre_artist_crossreference.artist_id → artists.id', () => { - expectOnDelete('genre_artist_crossreference', 'artist_id', 'cascade'); + it('artist_library_crossreference.library_id → library.id', () => { + expectOnDelete('artist_library_crossreference', 'library_id', 'cascade'); + }); + }); + + describe('should NOT have onDelete (intentional NO ACTION)', () => { + // The five below were DE-DECLARED by WXYC/Backend-Service#2239. schema.ts + // used to claim CASCADE (or SET NULL, for schedule.specialty_id) on each, + // but every one of them is plain NO ACTION in production and in a CI + // database built from the migration chain from empty -- no migration ever + // created them any other way. #2239 corrected schema.ts to describe what + // the database actually enforces rather than patching the database to + // match a declaration nothing relied on: no code path in apps/ or shared/ + // deletes a shows, artists, genres, or specialty_shows row, so adding the + // cascades would have armed five destructive deletes across decades of + // flowsheet and library history for zero callers. + // + // These assertions are the reason the decision cannot silently rot back. + // If a future change genuinely needs one of these to cascade, add the + // cascade in the PR that introduces the delete path, update the matching + // case here, and pair it with a migration -- the deployed constraint has + // to move too. Do not simply re-add `onDelete` to schema.ts; that is the + // exact drift #2239 existed to remove, and the integration guard at + // tests/integration/fk-on-delete-general-guard.spec.js will fail on it. + + it('schedule.specialty_id → specialty_shows.id (#2239)', () => { + expectNoOnDelete('schedule', 'specialty_id'); }); - it('genre_artist_crossreference.genre_id → genres.id', () => { - expectOnDelete('genre_artist_crossreference', 'genre_id', 'cascade'); + it('genre_artist_crossreference.artist_id → artists.id (#2239)', () => { + expectNoOnDelete('genre_artist_crossreference', 'artist_id'); }); - it('artist_library_crossreference.artist_id → artists.id', () => { - expectOnDelete('artist_library_crossreference', 'artist_id', 'cascade'); + it('genre_artist_crossreference.genre_id → genres.id (#2239)', () => { + expectNoOnDelete('genre_artist_crossreference', 'genre_id'); }); - it('artist_library_crossreference.library_id → library.id', () => { - expectOnDelete('artist_library_crossreference', 'library_id', 'cascade'); + it('artist_library_crossreference.artist_id → artists.id (#2239)', () => { + expectNoOnDelete('artist_library_crossreference', 'artist_id'); }); - it('show_djs.show_id → shows.id', () => { - expectOnDelete('show_djs', 'show_id', 'cascade'); + it('show_djs.show_id → shows.id (#2239)', () => { + expectNoOnDelete('show_djs', 'show_id'); }); - }); - describe('should NOT have onDelete (intentional NO ACTION)', () => { it('library.artist_id → artists.id', () => { const block = getColumnBlock('library', 'artist_id'); expect(block).not.toBeNull();