From 803741c5e6e1b1eb8232b1818eac1539ff1e6318 Mon Sep 17 00:00:00 2001 From: Jake Bromberg Date: Mon, 18 May 2026 16:27:01 -0700 Subject: [PATCH 1/2] Populate flowsheet.dj_name on marker rows and surface it in v2 (BS#952) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit show_start/show_end/dj_join/dj_leave rows were inserted without a dj_name column value, and the v2 serializer was regex-parsing the human-readable message field to recover it. The regex silently failed for upstream inserts, so /flowsheet returned dj_name: "" on every marker entry — visible in the iOS app and dj-site as bare "Signed on/off" rows with no DJ attribution. - startShow / endShow / createJoinNotification / createLeaveNotification now persist dj_name on insert, matching the precedence used by resolveDjNameForShow and the search service's DJ_NAME_EXPR. - transformToV2 reads entry.dj_name directly for the four marker types and derives timestamp from add_time instead of parsing the message string. The message format is unchanged for v1 consumers. - flowsheet-dj-name-backfill widens its filter to cover the four marker entry types alongside track. Guest dj_join/dj_leave rows backfill against shows.primary_dj_id (best-effort historically; new rows are exact). --- CLAUDE.md | 2 +- apps/backend/services/flowsheet.service.ts | 80 +++------- jobs/flowsheet-dj-name-backfill/job.ts | 26 ++-- .../flowsheet-dj-name-backfill/job.test.ts | 31 +++- tests/unit/services/flowsheet.endShow.test.ts | 77 +++++++++ .../flowsheet.joinLeaveNotifications.test.ts | 146 ++++++++++++++++++ tests/unit/services/flowsheet.service.test.ts | 51 +++--- .../unit/services/flowsheet.startShow.test.ts | 80 ++++++++++ 8 files changed, 402 insertions(+), 91 deletions(-) create mode 100644 tests/unit/services/flowsheet.endShow.test.ts create mode 100644 tests/unit/services/flowsheet.joinLeaveNotifications.test.ts diff --git a/CLAUDE.md b/CLAUDE.md index 7959636c..9c2f1d91 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -32,7 +32,7 @@ npm workspaces: | `@wxyc/flowsheet-etl` | `jobs/flowsheet-etl/` | Flowsheet ETL: sync from tubafrenzy | | `@wxyc/rotation-etl` | `jobs/rotation-etl/` | Rotation ETL: sync from tubafrenzy | | `@wxyc/artist-identity-etl` | `jobs/artist-identity-etl/` | Artist identity ETL: sync from LML's `entity.identity` | -| `@wxyc/flowsheet-dj-name-backfill` | `jobs/flowsheet-dj-name-backfill/` | One-shot backfill: populate `flowsheet.dj_name` on legacy track rows after migration 0053 | +| `@wxyc/flowsheet-dj-name-backfill` | `jobs/flowsheet-dj-name-backfill/` | One-shot backfill: populate `flowsheet.dj_name` on legacy track + marker rows (show_start, show_end, dj_join, dj_leave) after migration 0053 / #952 | | `@wxyc/library-artist-name-backfill` | `jobs/library-artist-name-backfill/` | One-shot backfill: populate `library.artist_name` from the `artists` join after migration 0058 (Epic A.2) | | `@wxyc/flowsheet-metadata-backfill` | `jobs/flowsheet-metadata-backfill/` | Recurring metadata drift-repair: enrich `flowsheet` track rows where LML metadata enrichment never ran (#631 / #638 / #641). Cron-registered via deploy-base; default schedule `0 6 * * *` UTC (02:00 ET) from `package.json` `cron-schedule`, overridable per-deploy via the `BACKFILL_CRON_SCHEDULE` GHA repository variable (BS#914). Orchestrator's cooperative pause (#735) defers when DJs are active. | | `@wxyc/library-artwork-url-backfill` | `jobs/library-artwork-url-backfill/` | One-shot warm: populate `library.artwork_url` for Discogs-resolvable rows (joined to `artists.discogs_artist_id`) so search-time `enrichWithArtwork` short-circuits (#637). | diff --git a/apps/backend/services/flowsheet.service.ts b/apps/backend/services/flowsheet.service.ts index 82f45223..b8ade271 100644 --- a/apps/backend/services/flowsheet.service.ts +++ b/apps/backend/services/flowsheet.service.ts @@ -417,6 +417,7 @@ export const startShow = async (dj_id: string, show_name?: string, specialty_id? await db.insert(flowsheet).values({ show_id: new_show[0].id, entry_type: 'show_start', + dj_name: dj_info.djName || dj_info.name || null, play_order: await nextPlayOrder(new_show[0].id), message: `Start of Show: DJ ${dj_info.djName || dj_info.name} joined the set at ${new Date().toLocaleString( 'en-US', @@ -469,18 +470,19 @@ export const addDJToShow = async (dj_id: string, current_show: Show): Promise => { - let dj_name = 'A DJ'; const dj = (await db.select().from(user).where(eq(user.id, id)).limit(1))[0]; - dj_name = dj?.djName || dj?.name || dj_name; + const persisted_dj_name = dj?.djName || dj?.name || null; + const display_dj_name = persisted_dj_name ?? 'A DJ'; - const message = `${dj_name} joined the set!`; + const message = `${display_dj_name} joined the set!`; const notification = await db .insert(flowsheet) .values({ show_id: show_id, entry_type: 'dj_join', + dj_name: persisted_dj_name, play_order: await nextPlayOrder(show_id), message: message, }) @@ -511,13 +513,15 @@ export const endShow = async (currentShow: Show): Promise => { ); const dj_information = (await db.select().from(user).where(eq(user.id, primary_dj_id)).limit(1))[0]; - const dj_name = dj_information?.djName || dj_information?.name || 'A DJ'; + const persisted_dj_name = dj_information?.djName || dj_information?.name || null; + const display_dj_name = persisted_dj_name ?? 'A DJ'; await db.insert(flowsheet).values({ show_id: currentShow.id, entry_type: 'show_end', + dj_name: persisted_dj_name, play_order: await nextPlayOrder(currentShow.id), - message: `End of Show: ${dj_name} left the set at ${new Date().toLocaleString('en-US', { + message: `End of Show: ${display_dj_name} left the set at ${new Date().toLocaleString('en-US', { timeZone: 'America/New_York', })}`, }); @@ -551,18 +555,19 @@ export const leaveShow = async (dj_id: string, currentShow: Show): Promise => { - let dj_name = 'A DJ'; const dj = (await db.select().from(user).where(eq(user.id, dj_id)).limit(1))[0]; - dj_name = dj?.djName || dj?.name || dj_name; + const persisted_dj_name = dj?.djName || dj?.name || null; + const display_dj_name = persisted_dj_name ?? 'A DJ'; - const message = `${dj_name} left the set!`; + const message = `${display_dj_name} left the set!`; const notification = await db .insert(flowsheet) .values({ show_id: show_id, entry_type: 'dj_leave', + dj_name: persisted_dj_name, play_order: await nextPlayOrder(show_id), message: message, }) @@ -754,11 +759,12 @@ export const transformToV2 = (entry: IFSEntry): Record => { entry_type: entry.entry_type, }; - // dj_name is intentionally not propagated here. It is denormalized onto the - // flowsheet row purely to let the search service skip the shows -> auth_user - // join (steps 5b.1-5b.3); V2 API consumers should keep deriving the display - // name from the show metadata so this denormalization stays an internal - // implementation detail of the search path. + // For marker entry types (show_start, show_end, dj_join, dj_leave), dj_name is + // surfaced directly from the flowsheet.dj_name column — see the v2 contract in + // apps/backend/app.yaml. Track entries do not include dj_name in the v2 payload + // (the artist_name / album_title / track_title fields carry the relevant + // attribution); flowsheet.dj_name on track rows exists solely for the search + // service's hot path (search.service.ts, originally steps 5b.1-5b.3). switch (entry.entry_type) { case 'track': return { @@ -789,58 +795,22 @@ export const transformToV2 = (entry: IFSEntry): Record => { case 'show_start': case 'show_end': { - // Parse DJ name and timestamp from message - // Format: "Start of Show: DJ {name} joined the set at {timestamp}" - // Format: "End of Show: {name} left the set at {timestamp}" - const message = entry.message || ''; - let dj_name = ''; - let timestamp = ''; - - if (entry.entry_type === 'show_start') { - const match = message.match(/^Start of Show: DJ (.+) joined the set at (.+)$/); - if (match) { - dj_name = match[1]; - timestamp = match[2]; - } - } else { - const match = message.match(/^End of Show: (.+) left the set at (.+)$/); - if (match) { - dj_name = match[1]; - timestamp = match[2]; - } - } - + const timestamp = entry.add_time + ? entry.add_time.toLocaleString('en-US', { timeZone: 'America/New_York' }) + : ''; return { ...baseFields, - dj_name, + dj_name: entry.dj_name ?? '', timestamp, }; } case 'dj_join': - case 'dj_leave': { - // Parse DJ name from message - // Format: "{name} joined the set!" or "{name} left the set!" - const message = entry.message || ''; - let dj_name = ''; - - if (entry.entry_type === 'dj_join') { - const match = message.match(/^(.+) joined the set!$/); - if (match) { - dj_name = match[1]; - } - } else { - const match = message.match(/^(.+) left the set!$/); - if (match) { - dj_name = match[1]; - } - } - + case 'dj_leave': return { ...baseFields, - dj_name, + dj_name: entry.dj_name ?? '', }; - } case 'talkset': case 'message': diff --git a/jobs/flowsheet-dj-name-backfill/job.ts b/jobs/flowsheet-dj-name-backfill/job.ts index ba228810..8a6ba7b3 100644 --- a/jobs/flowsheet-dj-name-backfill/job.ts +++ b/jobs/flowsheet-dj-name-backfill/job.ts @@ -1,5 +1,5 @@ /** - * One-shot backfill: populate flowsheet.dj_name on legacy track rows. + * One-shot backfill: populate flowsheet.dj_name on legacy track + marker rows. * * Splits the large UPDATE that was originally embedded in migration 0053 into * batched, individually-committed UPDATEs so a long run cannot hold an @@ -12,9 +12,15 @@ * insert path, so every row carries the same resolved name regardless of * which subsystem populated it. * - * Track-only filter: matches the precondition guard in migration 0054, which - * only requires dj_name on track rows. Non-track entries (talkset, breakpoint, - * show_start, etc.) keep dj_name NULL — search never reads them. + * Covered entry types (#952): track (search hot path) plus the four marker + * types — show_start, show_end, dj_join, dj_leave — which surface dj_name in + * the v2 /flowsheet API contract. + * + * Known limitation: dj_join / dj_leave rows for guest DJs join through + * shows.primary_dj_id, so the backfill writes the primary DJ's name, not the + * joining guest's. New rows from the live insert path are correct (they look + * up the joining DJ directly). Talkset / breakpoint / message rows still keep + * dj_name NULL — those aren't attributed to a specific DJ. * * Run procedure: see Backend-Service/CLAUDE.md and issue #511. Build via * `Manual Build & Deploy` with `target=flowsheet-dj-name-backfill`, then SSH @@ -61,11 +67,11 @@ const applyBatch = async (batchSize: number): Promise => { FROM "wxyc_schema"."shows" AS s LEFT JOIN "auth_user" AS u ON u."id" = s."primary_dj_id" WHERE f."show_id" = s."id" - AND f."entry_type" = 'track' + AND f."entry_type" IN ('track', 'show_start', 'show_end', 'dj_join', 'dj_leave') AND f."dj_name" IS NULL AND f."id" IN ( SELECT "id" FROM "wxyc_schema"."flowsheet" - WHERE "entry_type" = 'track' + WHERE "entry_type" IN ('track', 'show_start', 'show_end', 'dj_join', 'dj_leave') AND "dj_name" IS NULL ORDER BY "id" LIMIT ${batchSize} @@ -85,7 +91,7 @@ const formatDuration = (ms: number): string => { }; const runBackfill = async () => { - console.log(`[${JOB_NAME}] Starting backfill of flowsheet.dj_name (track rows).`); + console.log(`[${JOB_NAME}] Starting backfill of flowsheet.dj_name (track + marker rows).`); console.log(`[${JOB_NAME}] Batch size: ${BATCH_SIZE}.`); const startedAt = Date.now(); @@ -134,16 +140,16 @@ const runBackfill = async () => { */ const verifyComplete = async () => { const result = await db.execute( - sql`SELECT count(*)::int AS missing FROM "wxyc_schema"."flowsheet" WHERE entry_type = 'track' AND dj_name IS NULL` + sql`SELECT count(*)::int AS missing FROM "wxyc_schema"."flowsheet" WHERE entry_type IN ('track', 'show_start', 'show_end', 'dj_join', 'dj_leave') AND dj_name IS NULL` ); const missing = Number((result as unknown as Array<{ missing: number }>)[0]?.missing ?? 0); if (missing > 0) { throw new Error( - `[${JOB_NAME}] Verification failed: ${missing} track row(s) still have dj_name IS NULL. ` + + `[${JOB_NAME}] Verification failed: ${missing} row(s) still have dj_name IS NULL. ` + `Re-run the backfill — it is idempotent and will pick up the remaining rows.` ); } - console.log(`[${JOB_NAME}] Verification passed: 0 track rows with dj_name IS NULL.`); + console.log(`[${JOB_NAME}] Verification passed: 0 track + marker rows with dj_name IS NULL.`); }; const main = async () => { diff --git a/tests/unit/jobs/flowsheet-dj-name-backfill/job.test.ts b/tests/unit/jobs/flowsheet-dj-name-backfill/job.test.ts index ce7a90b7..1b0092c3 100644 --- a/tests/unit/jobs/flowsheet-dj-name-backfill/job.test.ts +++ b/tests/unit/jobs/flowsheet-dj-name-backfill/job.test.ts @@ -59,9 +59,9 @@ describe('flowsheet-dj-name-backfill: applyBatch', () => { expect(sqlText).toMatch(/COALESCE\(\s*u\."dj_name",\s*s\."legacy_dj_name",\s*u\."name"\s*\)/); }); - it('restricts the UPDATE to track rows with NULL dj_name within a bounded batch', async () => { + it('restricts the UPDATE to track + marker rows with NULL dj_name within a bounded batch', async () => { // Three regression guards in one test: - // - entry_type = 'track' filter (matches 0054's precondition guard) + // - entry_type IN (...) covers track + four marker types (#952) // - dj_name IS NULL (idempotency: re-run skips already-backfilled rows) // - LIMIT in an inner SELECT (bounded batch — never an unbounded UPDATE) (db.execute as jest.Mock).mockResolvedValueOnce({ count: 5000 }); @@ -70,7 +70,12 @@ describe('flowsheet-dj-name-backfill: applyBatch', () => { const call = findExecuteCallMatching(/UPDATE[\s\S]*flowsheet[\s\S]*dj_name/i); const sqlText = renderSql(call?.[0]); - expect(sqlText).toMatch(/entry_type"?\s*=\s*'track'/i); + expect(sqlText).toMatch(/entry_type"?\s+IN\s*\(/i); + expect(sqlText).toMatch(/'track'/); + expect(sqlText).toMatch(/'show_start'/); + expect(sqlText).toMatch(/'show_end'/); + expect(sqlText).toMatch(/'dj_join'/); + expect(sqlText).toMatch(/'dj_leave'/); expect(sqlText).toMatch(/dj_name"?\s+IS\s+NULL/i); expect(sqlText).toMatch(/LIMIT/i); }); @@ -189,18 +194,32 @@ describe('flowsheet-dj-name-backfill: verifyComplete', () => { jest.restoreAllMocks(); }); - it('passes when zero track rows have NULL dj_name', async () => { + it('passes when zero track + marker rows have NULL dj_name', async () => { (db.execute as jest.Mock).mockResolvedValueOnce([{ missing: 0 }]); await expect(verifyComplete()).resolves.toBeUndefined(); }); - it('throws with a remediation hint when track rows still have NULL dj_name', async () => { + it('checks all four marker types plus track in the verification query', async () => { + (db.execute as jest.Mock).mockResolvedValueOnce([{ missing: 0 }]); + + await verifyComplete(); + + const sqlText = renderSql((db.execute as jest.Mock).mock.calls[0]?.[0]); + expect(sqlText).toMatch(/entry_type"?\s+IN\s*\(/i); + expect(sqlText).toMatch(/'track'/); + expect(sqlText).toMatch(/'show_start'/); + expect(sqlText).toMatch(/'show_end'/); + expect(sqlText).toMatch(/'dj_join'/); + expect(sqlText).toMatch(/'dj_leave'/); + }); + + it('throws with a remediation hint when rows still have NULL dj_name', async () => { // mockResolvedValue (not Once) so both rejection assertions get the // same response — the second await re-invokes verifyComplete. (db.execute as jest.Mock).mockResolvedValue([{ missing: 137 }]); - await expect(verifyComplete()).rejects.toThrow(/137 track row\(s\)/); + await expect(verifyComplete()).rejects.toThrow(/137 row\(s\)/); await expect(verifyComplete()).rejects.toThrow(/idempotent/i); }); }); diff --git a/tests/unit/services/flowsheet.endShow.test.ts b/tests/unit/services/flowsheet.endShow.test.ts new file mode 100644 index 00000000..d60f3d1f --- /dev/null +++ b/tests/unit/services/flowsheet.endShow.test.ts @@ -0,0 +1,77 @@ +import { db, createMockQueryChain } from '../../mocks/database.mock'; +import { endShow } from '../../../apps/backend/services/flowsheet.service'; + +const makeAwaitablePlayOrderChain = (max: number) => { + const chain = createMockQueryChain(); + (chain as unknown as { then: (resolve: (v: unknown) => void) => void }).then = (resolve) => + resolve([{ max }]); + return chain; +}; + +describe('endShow', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('persists dj_name (from user.djName) on the show_end flowsheet row', async () => { + // remaining_djs select — no guests so the loop is a no-op + const remainingDjsSelect = createMockQueryChain(); + remainingDjsSelect.where.mockResolvedValue([]); + db.select.mockReturnValueOnce(remainingDjsSelect); + + // primary DJ user lookup + const userSelect = createMockQueryChain(); + userSelect.limit.mockResolvedValue([{ djName: 'DJ Night Owl', name: 'Riley Owens' }]); + db.select.mockReturnValueOnce(userSelect); + + // nextPlayOrder select for the flowsheet insert + db.select.mockReturnValueOnce(makeAwaitablePlayOrderChain(7)); + + // flowsheet insert — inspection target + const flowsheetInsert = createMockQueryChain([{ id: 999 }]); + db.insert.mockReturnValueOnce(flowsheetInsert); + + // shows update (end_time) + db.update.mockReturnValueOnce(createMockQueryChain([{}])); + + // getLatestShow() select chain — returns the show we just ended + const latestShowSelect = createMockQueryChain(); + latestShowSelect.limit.mockResolvedValue([{ id: 42, end_time: new Date() }]); + db.select.mockReturnValueOnce(latestShowSelect); + + await endShow({ id: 42, primary_dj_id: 'user-1' } as unknown as Parameters[0]); + + const flowsheetValues = flowsheetInsert.values.mock.calls[0]?.[0] as Record; + expect(flowsheetValues).toEqual( + expect.objectContaining({ + entry_type: 'show_end', + dj_name: 'DJ Night Owl', + }) + ); + }); + + it('falls back to user.name when djName is null', async () => { + const remainingDjsSelect = createMockQueryChain(); + remainingDjsSelect.where.mockResolvedValue([]); + db.select.mockReturnValueOnce(remainingDjsSelect); + + const userSelect = createMockQueryChain(); + userSelect.limit.mockResolvedValue([{ djName: null, name: 'Riley Owens' }]); + db.select.mockReturnValueOnce(userSelect); + + db.select.mockReturnValueOnce(makeAwaitablePlayOrderChain(0)); + + const flowsheetInsert = createMockQueryChain([{ id: 999 }]); + db.insert.mockReturnValueOnce(flowsheetInsert); + db.update.mockReturnValueOnce(createMockQueryChain([{}])); + + const latestShowSelect = createMockQueryChain(); + latestShowSelect.limit.mockResolvedValue([{ id: 42, end_time: new Date() }]); + db.select.mockReturnValueOnce(latestShowSelect); + + await endShow({ id: 42, primary_dj_id: 'user-1' } as unknown as Parameters[0]); + + const flowsheetValues = flowsheetInsert.values.mock.calls[0]?.[0] as Record; + expect(flowsheetValues.dj_name).toBe('Riley Owens'); + }); +}); diff --git a/tests/unit/services/flowsheet.joinLeaveNotifications.test.ts b/tests/unit/services/flowsheet.joinLeaveNotifications.test.ts new file mode 100644 index 00000000..3ced92d8 --- /dev/null +++ b/tests/unit/services/flowsheet.joinLeaveNotifications.test.ts @@ -0,0 +1,146 @@ +import { db, createMockQueryChain } from '../../mocks/database.mock'; +import { addDJToShow, leaveShow } from '../../../apps/backend/services/flowsheet.service'; + +const makeAwaitablePlayOrderChain = (max: number) => { + const chain = createMockQueryChain(); + (chain as unknown as { then: (resolve: (v: unknown) => void) => void }).then = (resolve) => + resolve([{ max }]); + return chain; +}; + +describe('createJoinNotification (via addDJToShow)', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('persists dj_name (from user.djName) on the dj_join flowsheet row', async () => { + // initial show_djs select — no existing membership + const showDjsSelect = createMockQueryChain(); + showDjsSelect.limit.mockResolvedValue([]); + db.select.mockReturnValueOnce(showDjsSelect); + + // show_djs insert (returning the new membership) + db.insert.mockReturnValueOnce(createMockQueryChain([{ show_id: 42, dj_id: 'user-2', active: true }])); + + // user lookup inside createJoinNotification + const userSelect = createMockQueryChain(); + userSelect.limit.mockResolvedValue([{ djName: 'DJ Bluejay', name: 'Sam Blue' }]); + db.select.mockReturnValueOnce(userSelect); + + // nextPlayOrder + db.select.mockReturnValueOnce(makeAwaitablePlayOrderChain(3)); + + // flowsheet insert — inspection target + const flowsheetInsert = createMockQueryChain([{ id: 999 }]); + db.insert.mockReturnValueOnce(flowsheetInsert); + + await addDJToShow('user-2', { id: 42 } as unknown as Parameters[1]); + + const flowsheetValues = flowsheetInsert.values.mock.calls[0]?.[0] as Record; + expect(flowsheetValues).toEqual( + expect.objectContaining({ + entry_type: 'dj_join', + dj_name: 'DJ Bluejay', + }) + ); + }); + + it('falls back to user.name when djName is null', async () => { + const showDjsSelect = createMockQueryChain(); + showDjsSelect.limit.mockResolvedValue([]); + db.select.mockReturnValueOnce(showDjsSelect); + + db.insert.mockReturnValueOnce(createMockQueryChain([{ show_id: 42, dj_id: 'user-2', active: true }])); + + const userSelect = createMockQueryChain(); + userSelect.limit.mockResolvedValue([{ djName: null, name: 'Sam Blue' }]); + db.select.mockReturnValueOnce(userSelect); + + db.select.mockReturnValueOnce(makeAwaitablePlayOrderChain(0)); + + const flowsheetInsert = createMockQueryChain([{ id: 999 }]); + db.insert.mockReturnValueOnce(flowsheetInsert); + + await addDJToShow('user-2', { id: 42 } as unknown as Parameters[1]); + + const flowsheetValues = flowsheetInsert.values.mock.calls[0]?.[0] as Record; + expect(flowsheetValues.dj_name).toBe('Sam Blue'); + }); + + it('persists null dj_name when the user has neither djName nor name', async () => { + const showDjsSelect = createMockQueryChain(); + showDjsSelect.limit.mockResolvedValue([]); + db.select.mockReturnValueOnce(showDjsSelect); + + db.insert.mockReturnValueOnce(createMockQueryChain([{ show_id: 42, dj_id: 'user-2', active: true }])); + + const userSelect = createMockQueryChain(); + userSelect.limit.mockResolvedValue([{ djName: null, name: null }]); + db.select.mockReturnValueOnce(userSelect); + + db.select.mockReturnValueOnce(makeAwaitablePlayOrderChain(0)); + + const flowsheetInsert = createMockQueryChain([{ id: 999 }]); + db.insert.mockReturnValueOnce(flowsheetInsert); + + await addDJToShow('user-2', { id: 42 } as unknown as Parameters[1]); + + const flowsheetValues = flowsheetInsert.values.mock.calls[0]?.[0] as Record; + expect(flowsheetValues.dj_name).toBeNull(); + }); +}); + +describe('createLeaveNotification (via leaveShow service)', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('persists dj_name (from user.djName) on the dj_leave flowsheet row', async () => { + // show_djs update returning the updated row + db.update.mockReturnValueOnce( + createMockQueryChain([{ show_id: 42, dj_id: 'user-2', active: false }]) + ); + + // user lookup + const userSelect = createMockQueryChain(); + userSelect.limit.mockResolvedValue([{ djName: 'DJ Shadow', name: 'Josh Davis' }]); + db.select.mockReturnValueOnce(userSelect); + + // nextPlayOrder + db.select.mockReturnValueOnce(makeAwaitablePlayOrderChain(5)); + + // flowsheet insert + const flowsheetInsert = createMockQueryChain([{ id: 999 }]); + db.insert.mockReturnValueOnce(flowsheetInsert); + + await leaveShow('user-2', { id: 42 } as unknown as Parameters[1]); + + const flowsheetValues = flowsheetInsert.values.mock.calls[0]?.[0] as Record; + expect(flowsheetValues).toEqual( + expect.objectContaining({ + entry_type: 'dj_leave', + dj_name: 'DJ Shadow', + }) + ); + }); + + it('falls back to user.name when djName is null', async () => { + db.update.mockReturnValueOnce( + createMockQueryChain([{ show_id: 42, dj_id: 'user-2', active: false }]) + ); + + const userSelect = createMockQueryChain(); + userSelect.limit.mockResolvedValue([{ djName: null, name: 'Josh Davis' }]); + db.select.mockReturnValueOnce(userSelect); + + db.select.mockReturnValueOnce(makeAwaitablePlayOrderChain(0)); + + const flowsheetInsert = createMockQueryChain([{ id: 999 }]); + db.insert.mockReturnValueOnce(flowsheetInsert); + + await leaveShow('user-2', { id: 42 } as unknown as Parameters[1]); + + const flowsheetValues = flowsheetInsert.values.mock.calls[0]?.[0] as Record; + expect(flowsheetValues.dj_name).toBe('Josh Davis'); + }); +}); diff --git a/tests/unit/services/flowsheet.service.test.ts b/tests/unit/services/flowsheet.service.test.ts index f758628c..f3b53166 100644 --- a/tests/unit/services/flowsheet.service.test.ts +++ b/tests/unit/services/flowsheet.service.test.ts @@ -39,6 +39,7 @@ const createBaseEntry = (overrides: Partial { }); describe('show_start entries', () => { - it('parses dj_name and timestamp from message', () => { + it('returns dj_name from the column (not regex-parsed from message)', () => { const entry = createBaseEntry({ entry_type: 'show_start', - message: 'Start of Show: DJ Cool Cat joined the set at 1/15/2024, 7:00:00 PM', + dj_name: 'Cool Cat', + // Deliberately malformed message to prove the serializer no longer reads it + message: 'this is not the canonical format', + add_time: new Date('2024-01-16T00:00:00Z'), }); const result = transformToV2(entry); expect(result.entry_type).toBe('show_start'); expect(result.dj_name).toBe('Cool Cat'); + // timestamp derived from add_time in en-US America/New_York locale expect(result.timestamp).toBe('1/15/2024, 7:00:00 PM'); }); it('excludes track-specific fields from show_start entries', () => { const entry = createBaseEntry({ entry_type: 'show_start', - message: 'Start of Show: DJ Test joined the set at 1/15/2024, 7:00:00 PM', + dj_name: 'Test', artist_name: 'should not appear', album_title: 'should not appear', }); @@ -270,11 +275,15 @@ describe('flowsheet.service', () => { expect(result.rotation_bin).toBeUndefined(); }); - it('handles malformed show_start message gracefully', () => { + it('returns empty strings when dj_name is null and add_time is missing', () => { const entry = createBaseEntry({ entry_type: 'show_start', - message: 'Some other message format', + dj_name: null, }); + // The schema-inferred type marks add_time as non-null, but legacy rows + // can carry a null value through the read path. Force it via override + // to exercise the empty-string fallback. + (entry as { add_time: Date | null }).add_time = null; const result = transformToV2(entry); @@ -285,10 +294,12 @@ describe('flowsheet.service', () => { }); describe('show_end entries', () => { - it('parses dj_name and timestamp from message', () => { + it('returns dj_name from the column (not regex-parsed from message)', () => { const entry = createBaseEntry({ entry_type: 'show_end', - message: 'End of Show: DJ Night Owl left the set at 1/15/2024, 10:00:00 PM', + dj_name: 'DJ Night Owl', + message: 'not the canonical format', + add_time: new Date('2024-01-16T03:00:00Z'), }); const result = transformToV2(entry); @@ -301,7 +312,7 @@ describe('flowsheet.service', () => { it('excludes track-specific fields from show_end entries', () => { const entry = createBaseEntry({ entry_type: 'show_end', - message: 'End of Show: Test left the set at 1/15/2024, 10:00:00 PM', + dj_name: 'Test', }); const result = transformToV2(entry); @@ -313,10 +324,11 @@ describe('flowsheet.service', () => { }); describe('dj_join entries', () => { - it('parses dj_name from message', () => { + it('returns dj_name from the column (not regex-parsed from message)', () => { const entry = createBaseEntry({ entry_type: 'dj_join', - message: 'MC Hammer joined the set!', + dj_name: 'MC Hammer', + message: 'not the canonical format', }); const result = transformToV2(entry); @@ -328,7 +340,7 @@ describe('flowsheet.service', () => { it('excludes track and message fields', () => { const entry = createBaseEntry({ entry_type: 'dj_join', - message: 'Test DJ joined the set!', + dj_name: 'Test DJ', }); const result = transformToV2(entry); @@ -337,10 +349,10 @@ describe('flowsheet.service', () => { expect(result.artist_name).toBeUndefined(); }); - it('handles malformed dj_join message gracefully', () => { + it('returns empty string when dj_name is null', () => { const entry = createBaseEntry({ entry_type: 'dj_join', - message: 'Invalid format', + dj_name: null, }); const result = transformToV2(entry); @@ -350,10 +362,11 @@ describe('flowsheet.service', () => { }); describe('dj_leave entries', () => { - it('parses dj_name from message', () => { + it('returns dj_name from the column (not regex-parsed from message)', () => { const entry = createBaseEntry({ entry_type: 'dj_leave', - message: 'DJ Shadow left the set!', + dj_name: 'DJ Shadow', + message: 'not the canonical format', }); const result = transformToV2(entry); @@ -362,10 +375,10 @@ describe('flowsheet.service', () => { expect(result.dj_name).toBe('DJ Shadow'); }); - it('handles malformed dj_leave message gracefully', () => { + it('returns empty string when dj_name is null', () => { const entry = createBaseEntry({ entry_type: 'dj_leave', - message: null, + dj_name: null, }); const result = transformToV2(entry); @@ -521,10 +534,10 @@ describe('flowsheet.service', () => { expect(result.show_id).toBeNull(); }); - it('handles special characters in DJ names', () => { + it('preserves special characters in DJ names', () => { const entry = createBaseEntry({ entry_type: 'dj_join', - message: "DJ O'Brien & The Crew joined the set!", + dj_name: "DJ O'Brien & The Crew", }); const result = transformToV2(entry); diff --git a/tests/unit/services/flowsheet.startShow.test.ts b/tests/unit/services/flowsheet.startShow.test.ts index 9221b6ff..4848780f 100644 --- a/tests/unit/services/flowsheet.startShow.test.ts +++ b/tests/unit/services/flowsheet.startShow.test.ts @@ -1,7 +1,20 @@ import { db, createMockQueryChain } from '../../mocks/database.mock'; import { startShow } from '../../../apps/backend/services/flowsheet.service'; +// nextPlayOrder() does `await db.select(...).from(...).where(...)` — no .returning() — +// so the chain itself must resolve to the max-row result. +const makeAwaitablePlayOrderChain = (max: number) => { + const chain = createMockQueryChain(); + (chain as unknown as { then: (resolve: (v: unknown) => void) => void }).then = (resolve) => + resolve([{ max }]); + return chain; +}; + describe('startShow', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + it('throws a 404 error before inserting a show when the DJ does not exist', async () => { const selectChain = createMockQueryChain(); selectChain.limit.mockResolvedValue([]); @@ -10,4 +23,71 @@ describe('startShow', () => { await expect(startShow('nonexistent-dj-id')).rejects.toThrow('not found'); expect(db.insert).not.toHaveBeenCalled(); }); + + it('persists dj_name (from user.djName) on the show_start flowsheet row', async () => { + // user lookup + const userSelect = createMockQueryChain(); + userSelect.limit.mockResolvedValue([{ djName: 'DJ Stardust', name: 'Alex Stardust' }]); + db.select.mockReturnValueOnce(userSelect); + + // shows insert returns the new show id + const showsInsert = createMockQueryChain([{ id: 42 }]); + db.insert.mockReturnValueOnce(showsInsert); + + // show_djs insert + db.insert.mockReturnValueOnce(createMockQueryChain([{ show_id: 42, dj_id: 'user-1' }])); + + // nextPlayOrder select + db.select.mockReturnValueOnce(makeAwaitablePlayOrderChain(0)); + + // flowsheet insert — this is the call we want to inspect + const flowsheetInsert = createMockQueryChain([{ id: 999 }]); + db.insert.mockReturnValueOnce(flowsheetInsert); + + await startShow('user-1', 'Some Show'); + + const flowsheetValues = flowsheetInsert.values.mock.calls[0]?.[0] as Record; + expect(flowsheetValues).toEqual( + expect.objectContaining({ + entry_type: 'show_start', + dj_name: 'DJ Stardust', + }) + ); + }); + + it('falls back to user.name when djName is null', async () => { + const userSelect = createMockQueryChain(); + userSelect.limit.mockResolvedValue([{ djName: null, name: 'Alex Stardust' }]); + db.select.mockReturnValueOnce(userSelect); + + db.insert.mockReturnValueOnce(createMockQueryChain([{ id: 42 }])); + db.insert.mockReturnValueOnce(createMockQueryChain([{ show_id: 42, dj_id: 'user-1' }])); + db.select.mockReturnValueOnce(makeAwaitablePlayOrderChain(0)); + + const flowsheetInsert = createMockQueryChain([{ id: 999 }]); + db.insert.mockReturnValueOnce(flowsheetInsert); + + await startShow('user-1'); + + const flowsheetValues = flowsheetInsert.values.mock.calls[0]?.[0] as Record; + expect(flowsheetValues.dj_name).toBe('Alex Stardust'); + }); + + it('persists null dj_name when the user has neither djName nor name', async () => { + const userSelect = createMockQueryChain(); + userSelect.limit.mockResolvedValue([{ djName: null, name: null }]); + db.select.mockReturnValueOnce(userSelect); + + db.insert.mockReturnValueOnce(createMockQueryChain([{ id: 42 }])); + db.insert.mockReturnValueOnce(createMockQueryChain([{ show_id: 42, dj_id: 'user-1' }])); + db.select.mockReturnValueOnce(makeAwaitablePlayOrderChain(0)); + + const flowsheetInsert = createMockQueryChain([{ id: 999 }]); + db.insert.mockReturnValueOnce(flowsheetInsert); + + await startShow('user-1'); + + const flowsheetValues = flowsheetInsert.values.mock.calls[0]?.[0] as Record; + expect(flowsheetValues.dj_name).toBeNull(); + }); }); From da45fbd7b0a7aa552b63cd0ffba0065e594a3c75 Mon Sep 17 00:00:00 2001 From: Jake Bromberg Date: Mon, 18 May 2026 16:33:28 -0700 Subject: [PATCH 2/2] Apply Prettier formatting --- apps/backend/services/flowsheet.service.ts | 4 +--- tests/unit/services/flowsheet.endShow.test.ts | 3 +-- .../services/flowsheet.joinLeaveNotifications.test.ts | 11 +++-------- tests/unit/services/flowsheet.startShow.test.ts | 3 +-- 4 files changed, 6 insertions(+), 15 deletions(-) diff --git a/apps/backend/services/flowsheet.service.ts b/apps/backend/services/flowsheet.service.ts index b8ade271..47fe98af 100644 --- a/apps/backend/services/flowsheet.service.ts +++ b/apps/backend/services/flowsheet.service.ts @@ -795,9 +795,7 @@ export const transformToV2 = (entry: IFSEntry): Record => { case 'show_start': case 'show_end': { - const timestamp = entry.add_time - ? entry.add_time.toLocaleString('en-US', { timeZone: 'America/New_York' }) - : ''; + const timestamp = entry.add_time ? entry.add_time.toLocaleString('en-US', { timeZone: 'America/New_York' }) : ''; return { ...baseFields, dj_name: entry.dj_name ?? '', diff --git a/tests/unit/services/flowsheet.endShow.test.ts b/tests/unit/services/flowsheet.endShow.test.ts index d60f3d1f..a660f70c 100644 --- a/tests/unit/services/flowsheet.endShow.test.ts +++ b/tests/unit/services/flowsheet.endShow.test.ts @@ -3,8 +3,7 @@ import { endShow } from '../../../apps/backend/services/flowsheet.service'; const makeAwaitablePlayOrderChain = (max: number) => { const chain = createMockQueryChain(); - (chain as unknown as { then: (resolve: (v: unknown) => void) => void }).then = (resolve) => - resolve([{ max }]); + (chain as unknown as { then: (resolve: (v: unknown) => void) => void }).then = (resolve) => resolve([{ max }]); return chain; }; diff --git a/tests/unit/services/flowsheet.joinLeaveNotifications.test.ts b/tests/unit/services/flowsheet.joinLeaveNotifications.test.ts index 3ced92d8..44275ab2 100644 --- a/tests/unit/services/flowsheet.joinLeaveNotifications.test.ts +++ b/tests/unit/services/flowsheet.joinLeaveNotifications.test.ts @@ -3,8 +3,7 @@ import { addDJToShow, leaveShow } from '../../../apps/backend/services/flowsheet const makeAwaitablePlayOrderChain = (max: number) => { const chain = createMockQueryChain(); - (chain as unknown as { then: (resolve: (v: unknown) => void) => void }).then = (resolve) => - resolve([{ max }]); + (chain as unknown as { then: (resolve: (v: unknown) => void) => void }).then = (resolve) => resolve([{ max }]); return chain; }; @@ -97,9 +96,7 @@ describe('createLeaveNotification (via leaveShow service)', () => { it('persists dj_name (from user.djName) on the dj_leave flowsheet row', async () => { // show_djs update returning the updated row - db.update.mockReturnValueOnce( - createMockQueryChain([{ show_id: 42, dj_id: 'user-2', active: false }]) - ); + db.update.mockReturnValueOnce(createMockQueryChain([{ show_id: 42, dj_id: 'user-2', active: false }])); // user lookup const userSelect = createMockQueryChain(); @@ -125,9 +122,7 @@ describe('createLeaveNotification (via leaveShow service)', () => { }); it('falls back to user.name when djName is null', async () => { - db.update.mockReturnValueOnce( - createMockQueryChain([{ show_id: 42, dj_id: 'user-2', active: false }]) - ); + db.update.mockReturnValueOnce(createMockQueryChain([{ show_id: 42, dj_id: 'user-2', active: false }])); const userSelect = createMockQueryChain(); userSelect.limit.mockResolvedValue([{ djName: null, name: 'Josh Davis' }]); diff --git a/tests/unit/services/flowsheet.startShow.test.ts b/tests/unit/services/flowsheet.startShow.test.ts index 4848780f..c8496633 100644 --- a/tests/unit/services/flowsheet.startShow.test.ts +++ b/tests/unit/services/flowsheet.startShow.test.ts @@ -5,8 +5,7 @@ import { startShow } from '../../../apps/backend/services/flowsheet.service'; // so the chain itself must resolve to the max-row result. const makeAwaitablePlayOrderChain = (max: number) => { const chain = createMockQueryChain(); - (chain as unknown as { then: (resolve: (v: unknown) => void) => void }).then = (resolve) => - resolve([{ max }]); + (chain as unknown as { then: (resolve: (v: unknown) => void) => void }).then = (resolve) => resolve([{ max }]); return chain; };