Fix sign-up rules glitch
Some checks failed
all-good: Did all the other checks pass? / all-good (push) Has been cancelled
Ensure Prisma migrations are in sync with the schema / check_prisma_migrations (22.x) (push) Has been cancelled
Docker Server Build and Push / Docker Build and Push Server (push) Has been cancelled
Docker Server Build and Run / docker (push) Has been cancelled
Runs E2E API Tests / E2E Tests (Node ${{ matrix.node-version }}, Freestyle ${{ matrix.freestyle-mode }}) (mock, 22.x) (push) Has been cancelled
Runs E2E API Tests / E2E Tests (Node ${{ matrix.node-version }}, Freestyle ${{ matrix.freestyle-mode }}) (prod, 22.x) (push) Has been cancelled
Runs E2E API Tests with custom port prefix / build (22.x) (push) Has been cancelled
Lint & build / lint_and_build (latest) (push) Has been cancelled
Mirror main branch to main-mirror-for-wdb / lint_and_build (push) Has been cancelled
Publish npm packages / publish (push) Has been cancelled
Dev Environment Test With Custom Base Port / restart-dev-and-test-with-custom-base-port (push) Has been cancelled
Dev Environment Test / restart-dev-and-test (push) Has been cancelled
Run setup tests with custom base port / setup-tests-with-custom-base-port (push) Has been cancelled
Run setup tests / setup-tests (push) Has been cancelled
Sync Main to Dev / sync-commits (push) Has been cancelled
TOC Generator / TOC Generator (push) Has been cancelled

This commit is contained in:
Konstantin Wohlwend 2026-02-25 11:15:00 -08:00
parent 09aa7576cb
commit 53c1c9e985
5 changed files with 92 additions and 6 deletions

View File

@ -0,0 +1 @@
Reproduce the bug and write a test for it, then fix it.

View File

@ -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<string, {
totalCount: number,

View File

@ -310,10 +310,36 @@ export async function logEvent<T extends EventType[]>(
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<string, unknown>),
rule_id: ruleId,
action,
email,
auth_method: authMethod,
oauth_provider: oauthProvider,
};
} else {
throw new StackAssertionError(`Unhandled ClickHouse event type: ${matchingEventType.id}`, { matchingEventType });
}
if (!projectId) {

View File

@ -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 }) => {

View File

@ -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);
});
});