From 5fb44b3ba4157084282f77458341bc1f41a81d35 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Mar 2026 22:20:00 +0000 Subject: [PATCH 1/2] fix: ensure tags and monitor_tags tables exist at startup The tags feature (PR #86) added new tables but missed the startup ensureTable migration, causing getMonitorsWithTags() to fail with a SQL error on databases that haven't run drizzle-kit push. https://claude.ai/code/session_012qN2riMykNDMYS26j3TJkY --- server/routes.migration.test.ts | 7 +++++-- server/routes.ts | 3 ++- server/services/ensureTables.ts | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 40 insertions(+), 3 deletions(-) diff --git a/server/routes.migration.test.ts b/server/routes.migration.test.ts index 72e8913..21407fd 100644 --- a/server/routes.migration.test.ts +++ b/server/routes.migration.test.ts @@ -167,11 +167,12 @@ describe("error_logs dedup column migration at startup", () => { const app = makeMockApp(); await registerRoutes(app as any, app as any); - // db.execute should have been called 11 times: + // db.execute should have been called 16 times: // 3 for the ALTER TABLE error_logs statements (first_occurrence, occurrence_count, deleted_at) // 2 for the api_keys table creation (CREATE TABLE + CREATE INDEX) // 6 for notification channel tables (3 CREATE TABLE + 2 indexes + 1 unique index) - expect(mockDbExecute).toHaveBeenCalledTimes(11); + // 5 for tag tables (2 CREATE TABLE + 2 indexes + 1 unique index) + expect(mockDbExecute).toHaveBeenCalledTimes(16); // Verify specific DDL statements were issued (drizzle sql`` produces SQL objects) const callStrings = mockDbExecute.mock.calls.map((c: any[]) => { @@ -184,6 +185,8 @@ describe("error_logs dedup column migration at startup", () => { expect(callStrings.some((s: string) => s.includes("notification_channels"))).toBe(true); expect(callStrings.some((s: string) => s.includes("delivery_log"))).toBe(true); expect(callStrings.some((s: string) => s.includes("slack_connections"))).toBe(true); + expect(callStrings.some((s: string) => s.includes("tags"))).toBe(true); + expect(callStrings.some((s: string) => s.includes("monitor_tags"))).toBe(true); }); it("still registers all route groups when migration succeeds", async () => { diff --git a/server/routes.ts b/server/routes.ts index be5e5a7..7afd254 100644 --- a/server/routes.ts +++ b/server/routes.ts @@ -33,7 +33,7 @@ import { encryptToken, decryptToken, isValidEncryptedToken } from "./utils/encry import { validateHost } from "./utils/hostValidation"; import { createHmac } from "node:crypto"; import rateLimit from "express-rate-limit"; -import { ensureErrorLogColumns, ensureApiKeysTable, ensureChannelTables } from "./services/ensureTables"; +import { ensureErrorLogColumns, ensureApiKeysTable, ensureChannelTables, ensureTagTables } from "./services/ensureTables"; // ------------------------------------------------------------------ @@ -62,6 +62,7 @@ export async function registerRoutes( await ensureErrorLogColumns(); const apiKeysReady = await ensureApiKeysTable(); await ensureChannelTables(); + await ensureTagTables(); // Setup Auth (must be before rate limiter so req.user is populated) await setupAuth(app); diff --git a/server/services/ensureTables.ts b/server/services/ensureTables.ts index 3c3b602..e1ebe72 100644 --- a/server/services/ensureTables.ts +++ b/server/services/ensureTables.ts @@ -96,3 +96,36 @@ export async function ensureChannelTables(): Promise { console.error("Could not ensure notification channel tables:", e); } } + +/** + * Ensures tags and monitor_tags tables exist (added in PR #86). + * Without this, getMonitorsWithTags() fails when the tables have not been created yet. + */ +export async function ensureTagTables(): Promise { + try { + await db.execute(sql` + CREATE TABLE IF NOT EXISTS tags ( + id SERIAL PRIMARY KEY, + user_id TEXT NOT NULL REFERENCES users(id), + name TEXT NOT NULL, + name_lower TEXT NOT NULL, + colour TEXT NOT NULL, + created_at TIMESTAMP NOT NULL DEFAULT NOW() + ) + `); + await db.execute(sql`CREATE INDEX IF NOT EXISTS tags_user_idx ON tags(user_id)`); + await db.execute(sql`CREATE UNIQUE INDEX IF NOT EXISTS tags_user_name_lower_uniq ON tags(user_id, name_lower)`); + + await db.execute(sql` + CREATE TABLE IF NOT EXISTS monitor_tags ( + id SERIAL PRIMARY KEY, + monitor_id INTEGER NOT NULL REFERENCES monitors(id) ON DELETE CASCADE, + tag_id INTEGER NOT NULL REFERENCES tags(id) ON DELETE CASCADE, + created_at TIMESTAMP NOT NULL DEFAULT NOW() + ) + `); + await db.execute(sql`CREATE UNIQUE INDEX IF NOT EXISTS monitor_tags_monitor_tag_uniq ON monitor_tags(monitor_id, tag_id)`); + } catch (e) { + console.error("Could not ensure tag tables:", e); + } +} From b4be4028268de1680385d4ee93ae67ac4cebe464 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Mar 2026 22:30:54 +0000 Subject: [PATCH 2/2] test: make tag DDL assertions more specific Address CodeRabbit feedback: replace weak `includes("tags")` assertion (which also matches "monitor_tags") with `includes("CREATE TABLE IF NOT EXISTS tags")`. Add dedicated test for tag table DDL columns, indexes, and constraints, following the pattern of existing table tests. https://claude.ai/code/session_012qN2riMykNDMYS26j3TJkY --- server/routes.migration.test.ts | 41 ++++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/server/routes.migration.test.ts b/server/routes.migration.test.ts index 21407fd..9f8dde8 100644 --- a/server/routes.migration.test.ts +++ b/server/routes.migration.test.ts @@ -185,7 +185,7 @@ describe("error_logs dedup column migration at startup", () => { expect(callStrings.some((s: string) => s.includes("notification_channels"))).toBe(true); expect(callStrings.some((s: string) => s.includes("delivery_log"))).toBe(true); expect(callStrings.some((s: string) => s.includes("slack_connections"))).toBe(true); - expect(callStrings.some((s: string) => s.includes("tags"))).toBe(true); + expect(callStrings.some((s: string) => s.includes("CREATE TABLE IF NOT EXISTS tags"))).toBe(true); expect(callStrings.some((s: string) => s.includes("monitor_tags"))).toBe(true); }); @@ -349,6 +349,45 @@ describe("error_logs dedup column migration at startup", () => { expect(scCreate).not.toContain("CHECK"); }); + it("issues DDL for tags and monitor_tags with correct columns and indexes", async () => { + vi.clearAllMocks(); + mockDbExecute.mockResolvedValue({ rows: [] }); + process.env.APP_OWNER_ID = "owner-123"; + + const { registerRoutes } = await import("./routes"); + const app = makeMockApp(); + await registerRoutes(app as any, app as any); + + const callStrings = mockDbExecute.mock.calls.map((c: any[]) => { + try { return JSON.stringify(c[0]); } catch { return String(c[0]); } + }); + + // tags CREATE TABLE includes required columns + const tagsCreate = callStrings.find((s: string) => + s.includes("CREATE TABLE IF NOT EXISTS tags") + ); + expect(tagsCreate).toBeDefined(); + expect(tagsCreate).toContain("user_id"); + expect(tagsCreate).toContain("name_lower"); + expect(tagsCreate).toContain("colour"); + + // tags indexes + expect(callStrings.some((s: string) => s.includes("tags_user_idx"))).toBe(true); + expect(callStrings.some((s: string) => s.includes("tags_user_name_lower_uniq"))).toBe(true); + + // monitor_tags CREATE TABLE includes required columns and constraints + const mtCreate = callStrings.find((s: string) => + s.includes("CREATE TABLE IF NOT EXISTS monitor_tags") + ); + expect(mtCreate).toBeDefined(); + expect(mtCreate).toContain("monitor_id"); + expect(mtCreate).toContain("tag_id"); + expect(mtCreate).toContain("ON DELETE CASCADE"); + + // monitor_tags unique index + expect(callStrings.some((s: string) => s.includes("monitor_tags_monitor_tag_uniq"))).toBe(true); + }); + it("logs error and continues when notification channel table creation fails", async () => { vi.clearAllMocks(); const channelError = new Error("permission denied for schema public");