From 53c1c9e98564df073a04ac201e30841eb3a5b3af Mon Sep 17 00:00:00 2001 From: Konstantin Wohlwend Date: Wed, 25 Feb 2026 11:15:00 -0800 Subject: [PATCH] Fix sign-up rules glitch --- .cursor/commands/repro-fix.md | 1 + .../internal/sign-up-rules-stats/route.tsx | 11 ++-- apps/backend/src/lib/events.tsx | 30 ++++++++++- .../endpoints/api/v1/internal-metrics.test.ts | 2 +- .../v1/internal/sign-up-rules-stats.test.ts | 54 +++++++++++++++++++ 5 files changed, 92 insertions(+), 6 deletions(-) create mode 100644 .cursor/commands/repro-fix.md diff --git a/.cursor/commands/repro-fix.md b/.cursor/commands/repro-fix.md new file mode 100644 index 000000000..7c2d21d80 --- /dev/null +++ b/.cursor/commands/repro-fix.md @@ -0,0 +1 @@ +Reproduce the bug and write a test for it, then fix it. diff --git a/apps/backend/src/app/api/latest/internal/sign-up-rules-stats/route.tsx b/apps/backend/src/app/api/latest/internal/sign-up-rules-stats/route.tsx index ad541f3d8..cd9f77b75 100644 --- a/apps/backend/src/app/api/latest/internal/sign-up-rules-stats/route.tsx +++ b/apps/backend/src/app/api/latest/internal/sign-up-rules-stats/route.tsx @@ -55,7 +55,10 @@ export const GET = createSmartRouteHandler({ const result = await client.query({ query: ` SELECT - data.ruleId as rule_id, + COALESCE( + NULLIF(CAST(data.rule_id, 'Nullable(String)'), ''), + NULLIF(CAST(data.ruleId, 'Nullable(String)'), '') + ) as rule_id, data.action as action, toStartOfHour(event_at) as hour FROM analytics_internal.events @@ -72,12 +75,14 @@ export const GET = createSmartRouteHandler({ }, format: "JSONEachRow", }); - const rows: { - rule_id: string, + const rawRows: { + rule_id: string | null, action: "allow" | "reject" | "restrict" | "log", hour: string, }[] = await result.json(); + const rows = rawRows.filter((row): row is typeof row & { rule_id: string } => row.rule_id != null && row.rule_id !== ''); + // Group by rule and hour for sparkline data const ruleTriggersMap = new Map( is_anonymous: isAnonymous, ip_info: toClickhouseEndUserIpInfo(ipInfo ?? null), }; - } else { + } else if (matchingEventType.id === "$sign-up-rule-trigger") { + const ruleId = + typeof dataRecord === "object" && dataRecord && typeof dataRecord.ruleId === "string" + ? dataRecord.ruleId + : throwErr(new StackAssertionError("ruleId is required for $sign-up-rule-trigger ClickHouse event", { dataRecord })); + const action = + typeof dataRecord === "object" && dataRecord && typeof dataRecord.action === "string" + ? dataRecord.action + : throwErr(new StackAssertionError("action is required for $sign-up-rule-trigger ClickHouse event", { dataRecord })); + const email = + typeof dataRecord === "object" && dataRecord + ? (dataRecord.email as string | null | undefined) ?? null + : null; + const authMethod = + typeof dataRecord === "object" && dataRecord + ? (dataRecord.authMethod as string | null | undefined) ?? null + : null; + const oauthProvider = + typeof dataRecord === "object" && dataRecord + ? (dataRecord.oauthProvider as string | null | undefined) ?? null + : null; clickhouseEventData = { - ...(data as Record), + rule_id: ruleId, + action, + email, + auth_method: authMethod, + oauth_provider: oauthProvider, }; + } else { + throw new StackAssertionError(`Unhandled ClickHouse event type: ${matchingEventType.id}`, { matchingEventType }); } if (!projectId) { diff --git a/apps/e2e/tests/backend/endpoints/api/v1/internal-metrics.test.ts b/apps/e2e/tests/backend/endpoints/api/v1/internal-metrics.test.ts index 7ea0f6866..33255ece0 100644 --- a/apps/e2e/tests/backend/endpoints/api/v1/internal-metrics.test.ts +++ b/apps/e2e/tests/backend/endpoints/api/v1/internal-metrics.test.ts @@ -81,7 +81,7 @@ it("should return metrics data with users", async ({ expect }) => { await ensureAnonymousUsersAreStillExcluded(response); }, { - timeout: 120_000, + timeout: 240_000, }); it("should not work for non-admins", async ({ expect }) => { diff --git a/apps/e2e/tests/backend/endpoints/api/v1/internal/sign-up-rules-stats.test.ts b/apps/e2e/tests/backend/endpoints/api/v1/internal/sign-up-rules-stats.test.ts index 00d225b48..bcc422936 100644 --- a/apps/e2e/tests/backend/endpoints/api/v1/internal/sign-up-rules-stats.test.ts +++ b/apps/e2e/tests/backend/endpoints/api/v1/internal/sign-up-rules-stats.test.ts @@ -169,4 +169,58 @@ describe("with admin access", () => { expect(lastHourlyCount.hour).toEqual(new Date().toISOString().slice(0, 13) + ':00:00.000Z'); expect(lastHourlyCount.count).toBe(1); }); + + it("should read rule_id from ClickHouse events with COALESCE for both camelCase and snake_case data", async ({ expect }) => { + // Regression test: a ClickHouse migration converts ruleId -> rule_id (snake_case). + // The stats query must handle both field name formats via COALESCE. + // This test verifies the COALESCE query reads the correct rule_id from the event. + await Project.createAndSwitch(); + await Project.updateConfig({ + 'auth.signUpRules.coalesce-rule': { + enabled: true, + displayName: 'COALESCE Test Rule', + priority: 1, + condition: 'true', + action: { type: 'log' }, + }, + }); + + await Auth.Password.signUpWithEmail(); + + // Wait for the ClickHouse event to appear and verify via a raw COALESCE query + let chResult: any; + for (let attempt = 0; attempt < 15; attempt++) { + await wait(500); + chResult = await niceBackendFetch("/api/v1/internal/analytics/query", { + method: "POST", + accessType: "admin", + body: { + query: ` + SELECT + COALESCE( + NULLIF(CAST(data.rule_id, 'Nullable(String)'), ''), + NULLIF(CAST(data.ruleId, 'Nullable(String)'), '') + ) as rule_id + FROM events + WHERE event_type = '$sign-up-rule-trigger' + LIMIT 1 + `, + params: {}, + }, + }); + if (chResult.status === 200 && chResult.body?.result?.length > 0) break; + } + + expect(chResult.status).toBe(200); + expect(chResult.body.result.length).toBeGreaterThan(0); + expect(chResult.body.result[0].rule_id).toBe('coalesce-rule'); + + // Verify the stats endpoint returns correct data with the COALESCE-based query + const response = await niceBackendFetch("/api/v1/internal/sign-up-rules-stats", { accessType: "admin" }); + expect(response.status).toBe(200); + expect(response.body.rule_triggers.length).toBeGreaterThan(0); + const trigger = response.body.rule_triggers.find((t: any) => t.rule_id === 'coalesce-rule'); + expect(trigger).toBeTruthy(); + expect(trigger.total_count).toBeGreaterThanOrEqual(1); + }); });