From 931188a8ab41bef9a676ae411bbd9f8effff7245 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carl-Gerhard=20Lindesva=CC=88rd?= Date: Thu, 30 Oct 2025 11:04:41 +0100 Subject: [PATCH] fix: timezone issue + improvements for funnel and conversion charts --- apps/start/package.json | 1 + apps/start/src/components/color-square.tsx | 3 +- .../components/profiles/profile-metrics.tsx | 7 ++-- .../report-chart/conversion/chart.tsx | 18 ++++---- .../components/report-chart/funnel/chart.tsx | 37 +++++++++------- .../components/report-chart/funnel/index.tsx | 2 +- apps/start/src/components/widget-table.tsx | 7 +++- ...d.$projectId_.dashboards_.$dashboardId.tsx | 6 +-- packages/db/src/clickhouse/query-builder.ts | 5 ++- .../db/src/services/conversion.service.ts | 19 +++++---- packages/db/src/services/funnel.service.ts | 42 ++++++++++++++----- packages/db/src/services/profile.service.ts | 24 ++++++++--- packages/db/test.ts | 38 ----------------- packages/trpc/src/routers/chart.ts | 12 ++++-- pnpm-lock.yaml | 10 +++++ 15 files changed, 128 insertions(+), 103 deletions(-) delete mode 100644 packages/db/test.ts diff --git a/apps/start/package.json b/apps/start/package.json index 555de5c9..b9de5ab5 100644 --- a/apps/start/package.json +++ b/apps/start/package.json @@ -163,6 +163,7 @@ "@types/ramda": "^0.31.0", "@types/react": "catalog:", "@types/react-dom": "catalog:", + "@types/react-grid-layout": "^1.3.5", "@types/react-simple-maps": "^3.0.4", "@types/react-syntax-highlighter": "^15.5.11", "@vitejs/plugin-react": "^4.3.4", diff --git a/apps/start/src/components/color-square.tsx b/apps/start/src/components/color-square.tsx index f707dc45..928dd468 100644 --- a/apps/start/src/components/color-square.tsx +++ b/apps/start/src/components/color-square.tsx @@ -3,13 +3,14 @@ import { cn } from '@/utils/cn'; type ColorSquareProps = HtmlProps; -export function ColorSquare({ children, className }: ColorSquareProps) { +export function ColorSquare({ children, className, color }: ColorSquareProps) { return (
{children}
diff --git a/apps/start/src/components/profiles/profile-metrics.tsx b/apps/start/src/components/profiles/profile-metrics.tsx index 03e8c145..abfe7731 100644 --- a/apps/start/src/components/profiles/profile-metrics.tsx +++ b/apps/start/src/components/profiles/profile-metrics.tsx @@ -92,11 +92,10 @@ export const ProfileMetrics = ({ data }: Props) => { label={metric.title} metric={{ current: - metric.unit === 'timeAgo' && - typeof data[metric.key] === 'string' - ? new Date(data[metric.key] as string).getTime() + metric.unit === 'timeAgo' + ? new Date(data[metric.key]).getTime() : (data[metric.key] as number) || 0, - previous: null, // Profile metrics don't have previous period comparison + previous: null, }} unit={metric.unit} data={[]} diff --git a/apps/start/src/components/report-chart/conversion/chart.tsx b/apps/start/src/components/report-chart/conversion/chart.tsx index 66c6885e..38c0992e 100644 --- a/apps/start/src/components/report-chart/conversion/chart.tsx +++ b/apps/start/src/components/report-chart/conversion/chart.tsx @@ -191,14 +191,16 @@ const { Tooltip, TooltipProvider } = createChartTooltip< {item.total} -
- - - ({prevItem?.total}) - -
+ {!!prevItem && ( +
+ + + ({prevItem?.total}) + +
+ )} diff --git a/apps/start/src/components/report-chart/funnel/chart.tsx b/apps/start/src/components/report-chart/funnel/chart.tsx index 57a24885..54f8bddf 100644 --- a/apps/start/src/components/report-chart/funnel/chart.tsx +++ b/apps/start/src/components/report-chart/funnel/chart.tsx @@ -6,15 +6,17 @@ import { ChevronRightIcon, InfoIcon } from 'lucide-react'; import { alphabetIds } from '@openpanel/constants'; import { createChartTooltip } from '@/components/charts/chart-tooltip'; +import { BarShapeBlue } from '@/components/charts/common-bar'; import { Tooltiper } from '@/components/ui/tooltip'; import { WidgetTable } from '@/components/widget-table'; import { useNumber } from '@/hooks/use-numer-formatter'; import { getChartColor } from '@/utils/theme'; import { getPreviousMetric } from '@openpanel/common'; import { + Bar, + BarChart, CartesianGrid, - Line, - LineChart, + Cell, ResponsiveContainer, XAxis, YAxis, @@ -205,8 +207,10 @@ export function Tables({ { name: 'Event', render: (item, index) => ( -
- {alphabetIds[index]} +
+ + {alphabetIds[index]} + {item.event.displayName}
), @@ -265,6 +269,7 @@ const useRechartData = ({ return ( firstFunnel?.steps.map((step, stepIndex) => { return { + id: step?.event.id ?? '', name: step?.event.displayName ?? '', ...current.reduce((acc, item, index) => { const diff = previous?.[index]; @@ -299,7 +304,7 @@ export function Chart({ data }: { data: RouterOutputs['chart']['funnel'] }) {
- + + data.current[0].steps.find((step) => step.event.id === id) + ?.event.displayName ?? '' + } /> - {data.current.map((item, index) => ( - - ))} + + {rechartData.map((item, index) => ( + + ))} + - +
diff --git a/apps/start/src/components/report-chart/funnel/index.tsx b/apps/start/src/components/report-chart/funnel/index.tsx index c9483f0f..e80063cc 100644 --- a/apps/start/src/components/report-chart/funnel/index.tsx +++ b/apps/start/src/components/report-chart/funnel/index.tsx @@ -44,7 +44,7 @@ export function ReportFunnelChart() { const trpc = useTRPC(); const res = useQuery( trpc.chart.funnel.queryOptions(input, { - enabled: !isLazyLoading, + enabled: !isLazyLoading && input.events.length > 0, }), ); diff --git a/apps/start/src/components/widget-table.tsx b/apps/start/src/components/widget-table.tsx index 212eed51..c3376914 100644 --- a/apps/start/src/components/widget-table.tsx +++ b/apps/start/src/components/widget-table.tsx @@ -85,12 +85,15 @@ export function WidgetTable({ )} > {eachRow?.(item, index)} -
+
{columns.map((column) => (
1 && column !== columns[0] ? 'text-right' : 'text-left', diff --git a/apps/start/src/routes/_app.$organizationId.$projectId_.dashboards_.$dashboardId.tsx b/apps/start/src/routes/_app.$organizationId.$projectId_.dashboards_.$dashboardId.tsx index 7c748d53..e8dff57b 100644 --- a/apps/start/src/routes/_app.$organizationId.$projectId_.dashboards_.$dashboardId.tsx +++ b/apps/start/src/routes/_app.$organizationId.$projectId_.dashboards_.$dashboardId.tsx @@ -12,7 +12,6 @@ import { import { cn } from '@/utils/cn'; import { createProjectTitle } from '@/utils/title'; import { - ChevronRight, LayoutPanelTopIcon, MoreHorizontal, PlusIcon, @@ -30,16 +29,13 @@ import { OverviewRange } from '@/components/overview/overview-range'; import { PageContainer } from '@/components/page-container'; import { PageHeader } from '@/components/page-header'; import { handleErrorToastOptions, useTRPC } from '@/integrations/trpc/react'; +import { showConfirm } from '@/modals'; import { useMutation, useQuery } from '@tanstack/react-query'; import { createFileRoute, useRouter } from '@tanstack/react-router'; import { useCallback, useEffect, useMemo, useState } from 'react'; -// @ts-ignore - types will be installed separately import { Responsive, WidthProvider } from 'react-grid-layout'; import 'react-grid-layout/css/styles.css'; import 'react-resizable/css/styles.css'; -import { ReportChartLoading } from '@/components/report-chart/common/loading'; -import { ReportChartProvider } from '@/components/report-chart/context'; -import { showConfirm } from '@/modals'; const ResponsiveGridLayout = WidthProvider(Responsive); diff --git a/packages/db/src/clickhouse/query-builder.ts b/packages/db/src/clickhouse/query-builder.ts index 946cff0b..fd2afa2d 100644 --- a/packages/db/src/clickhouse/query-builder.ts +++ b/packages/db/src/clickhouse/query-builder.ts @@ -508,7 +508,10 @@ export class Query { // Execution methods async execute(): Promise { const query = this.buildQuery(); - console.log('query', query); + console.log( + 'query', + `${query} SETTINGS session_timezone = '${this.timezone}'`, + ); const result = await this.client.query({ query, diff --git a/packages/db/src/services/conversion.service.ts b/packages/db/src/services/conversion.service.ts index cc036fa9..4046ced5 100644 --- a/packages/db/src/services/conversion.service.ts +++ b/packages/db/src/services/conversion.service.ts @@ -20,7 +20,10 @@ export class ConversionService { events, breakdowns = [], interval, - }: Omit) { + timezone, + }: Omit & { + timezone: string; + }) { const group = funnelGroup === 'profile_id' ? 'profile_id' : 'session_id'; const breakdownColumns = breakdowns.map( (b, index) => `${getSelectPropertyKey(b.name)} as b_${index}`, @@ -44,7 +47,7 @@ export class ConversionService { getEventFiltersWhereClause(eventB.filters), ).join(' AND '); - const eventACte = clix(this.client) + const eventACte = clix(this.client, timezone) .select([ `DISTINCT ${group}`, 'created_at AS a_time', @@ -56,22 +59,22 @@ export class ConversionService { .where('name', '=', eventA.name) .rawWhere(whereA) .where('created_at', 'BETWEEN', [ - clix.datetime(startDate), - clix.datetime(endDate), + clix.datetime(startDate, 'toDateTime'), + clix.datetime(endDate, 'toDateTime'), ]); - const eventBCte = clix(this.client) + const eventBCte = clix(this.client, timezone) .select([group, 'created_at AS b_time']) .from(TABLE_NAMES.events) .where('project_id', '=', projectId) .where('name', '=', eventB.name) .rawWhere(whereB) .where('created_at', 'BETWEEN', [ - clix.datetime(startDate), - clix.datetime(endDate), + clix.datetime(startDate, 'toDateTime'), + clix.datetime(endDate, 'toDateTime'), ]); - const query = clix(this.client) + const query = clix(this.client, timezone) .with('event_a', eventACte) .with('event_b', eventBCte) .select<{ diff --git a/packages/db/src/services/funnel.service.ts b/packages/db/src/services/funnel.service.ts index e60b08e6..83a3251c 100644 --- a/packages/db/src/services/funnel.service.ts +++ b/packages/db/src/services/funnel.service.ts @@ -1,6 +1,6 @@ import { ifNaN } from '@openpanel/common'; import type { IChartEvent, IChartInput } from '@openpanel/validation'; -import { last, reverse } from 'ramda'; +import { last, reverse, uniq } from 'ramda'; import sqlstring from 'sqlstring'; import { ch } from '../clickhouse/client'; import { TABLE_NAMES } from '../clickhouse/client'; @@ -98,6 +98,14 @@ export class FunnelService { return Object.values(series); } + getProfileFilters(events: IChartEvent[]) { + return events.flatMap((e) => + e.filters + ?.filter((f) => f.name.startsWith('profile.')) + .map((f) => f.name.replace('profile.', '')), + ); + } + async getFunnel({ projectId, startDate, @@ -106,7 +114,8 @@ export class FunnelService { funnelWindow = 24, funnelGroup, breakdowns = [], - }: IChartInput) { + timezone = 'UTC', + }: IChartInput & { timezone: string }) { if (!startDate || !endDate) { throw new Error('startDate and endDate are required'); } @@ -118,9 +127,14 @@ export class FunnelService { const funnelWindowSeconds = funnelWindow * 3600; const group = this.getFunnelGroup(funnelGroup); const funnels = this.getFunnelConditions(events); + const profileFilters = this.getProfileFilters(events); + const anyFilterOnProfile = profileFilters.length > 0; + const anyBreakdownOnProfile = breakdowns.some((b) => + b.name.startsWith('profile.'), + ); // Create the funnel CTE - const funnelCte = clix(this.client) + const funnelCte = clix(this.client, timezone) .select([ `${group[0]} AS ${group[1]}`, ...breakdowns.map( @@ -131,8 +145,8 @@ export class FunnelService { .from(TABLE_NAMES.events, false) .where('project_id', '=', projectId) .where('created_at', 'BETWEEN', [ - clix.datetime(startDate), - clix.datetime(endDate), + clix.datetime(startDate, 'toDateTime'), + clix.datetime(endDate, 'toDateTime'), ]) .where( 'name', @@ -141,21 +155,29 @@ export class FunnelService { ) .groupBy([group[1], ...breakdowns.map((b, index) => `b_${index}`)]); + if (anyFilterOnProfile || anyBreakdownOnProfile) { + funnelCte.leftJoin( + `(SELECT id, ${uniq(profileFilters.map((f) => f.split('.')[0]))} FROM ${TABLE_NAMES.profiles} FINAL + WHERE project_id = ${sqlstring.escape(projectId)}) as profile`, + 'profile.id = profile_id', + ); + } + // Create the sessions CTE if needed const sessionsCte = group[0] !== 'session_id' - ? clix(this.client) + ? clix(this.client, timezone) .select(['profile_id', 'id']) .from(TABLE_NAMES.sessions) .where('project_id', '=', projectId) .where('created_at', 'BETWEEN', [ - clix.datetime(startDate), - clix.datetime(endDate), + clix.datetime(startDate, 'toDateTime'), + clix.datetime(endDate, 'toDateTime'), ]) : null; // Base funnel query with CTEs - const funnelQuery = clix(this.client); + const funnelQuery = clix(this.client, timezone); if (sessionsCte) { funnelCte.leftJoin('sessions s', 's.id = session_id'); @@ -202,7 +224,7 @@ export class FunnelService { { event: { ...event, - displayName: event.displayName ?? event.name, + displayName: event.displayName || event.name, }, count: item.count, percent: (item.count / totalSessions) * 100, diff --git a/packages/db/src/services/profile.service.ts b/packages/db/src/services/profile.service.ts index c9a49bf2..73d2eb89 100644 --- a/packages/db/src/services/profile.service.ts +++ b/packages/db/src/services/profile.service.ts @@ -10,13 +10,14 @@ import { TABLE_NAMES, ch, chQuery, + convertClickhouseDateToJs, formatClickhouseDate, } from '../clickhouse/client'; import { createSqlBuilder } from '../sql-builder'; export type IProfileMetrics = { - lastSeen: string; - firstSeen: string; + lastSeen: Date; + firstSeen: Date; screenViews: number; sessions: number; durationAvg: number; @@ -29,7 +30,12 @@ export type IProfileMetrics = { avgTimeBetweenSessions: number; }; export function getProfileMetrics(profileId: string, projectId: string) { - return chQuery(` + return chQuery< + Omit & { + lastSeen: string; + firstSeen: string; + } + >(` WITH lastSeen AS ( SELECT max(created_at) as lastSeen FROM ${TABLE_NAMES.events} WHERE profile_id = ${sqlstring.escape(profileId)} AND project_id = ${sqlstring.escape(projectId)} ), @@ -84,7 +90,15 @@ export function getProfileMetrics(profileId: string, projectId: string) { (SELECT avgEventsPerSession FROM avgEventsPerSession) as avgEventsPerSession, (SELECT conversionEvents FROM conversionEvents) as conversionEvents, (SELECT avgTimeBetweenSessions FROM avgTimeBetweenSessions) as avgTimeBetweenSessions - `).then((data) => data[0]!); + `) + .then((data) => data[0]!) + .then((data) => { + return { + ...data, + lastSeen: convertClickhouseDateToJs(data.lastSeen), + firstSeen: convertClickhouseDateToJs(data.firstSeen), + }; + }); } export async function getProfileById(id: string, projectId: string) { @@ -259,7 +273,7 @@ export function transformProfile({ lastName: last_name, isExternal: profile.is_external, properties: toObject(profile.properties), - createdAt: new Date(created_at), + createdAt: convertClickhouseDateToJs(created_at), projectId: profile.project_id, id: profile.id, email: profile.email, diff --git a/packages/db/test.ts b/packages/db/test.ts deleted file mode 100644 index 06ad41d5..00000000 --- a/packages/db/test.ts +++ /dev/null @@ -1,38 +0,0 @@ -import { conversionService } from './src/services/conversion.service'; -// 68/37 -async function main() { - const conversion = await conversionService.getConversion({ - projectId: 'kiddokitchen-app', - startDate: '2025-02-01', - endDate: '2025-03-01', - funnelGroup: 'session_id', - breakdowns: [ - { - name: 'os', - }, - ], - interval: 'day', - events: [ - { - segment: 'event', - name: 'screen_view', - filters: [ - { - name: 'path', - operator: 'is', - value: ['Start'], - }, - ], - }, - { - segment: 'event', - name: 'sign_up', - filters: [], - }, - ], - }); - - console.dir(conversion, { depth: null }); -} - -main(); diff --git a/packages/trpc/src/routers/chart.ts b/packages/trpc/src/routers/chart.ts index 6fb30572..c4438ac3 100644 --- a/packages/trpc/src/routers/chart.ts +++ b/packages/trpc/src/routers/chart.ts @@ -322,9 +322,9 @@ export const chartRouter = createTRPCRouter({ const previousPeriod = getChartPrevStartEndDate(currentPeriod); const [current, previous] = await Promise.all([ - funnelService.getFunnel({ ...input, ...currentPeriod }), + funnelService.getFunnel({ ...input, ...currentPeriod, timezone }), input.previous - ? funnelService.getFunnel({ ...input, ...previousPeriod }) + ? funnelService.getFunnel({ ...input, ...previousPeriod, timezone }) : Promise.resolve(null), ]); @@ -340,9 +340,13 @@ export const chartRouter = createTRPCRouter({ const previousPeriod = getChartPrevStartEndDate(currentPeriod); const [current, previous] = await Promise.all([ - conversionService.getConversion({ ...input, ...currentPeriod }), + conversionService.getConversion({ ...input, ...currentPeriod, timezone }), input.previous - ? conversionService.getConversion({ ...input, ...previousPeriod }) + ? conversionService.getConversion({ + ...input, + ...previousPeriod, + timezone, + }) : Promise.resolve(null), ]); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 6771587f..f15819bd 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -767,6 +767,9 @@ importers: '@types/react-dom': specifier: 'catalog:' version: 19.1.8(@types/react@19.1.11) + '@types/react-grid-layout': + specifier: ^1.3.5 + version: 1.3.5 '@types/react-simple-maps': specifier: ^3.0.4 version: 3.0.4 @@ -8617,6 +8620,9 @@ packages: peerDependencies: '@types/react': ^19.0.0 + '@types/react-grid-layout@1.3.5': + resolution: {integrity: sha512-WH/po1gcEcoR6y857yAnPGug+ZhkF4PaTUxgAbwfeSH/QOgVSakKHBXoPGad/sEznmkiaK3pqHk+etdWisoeBQ==} + '@types/react-simple-maps@3.0.4': resolution: {integrity: sha512-U9qnX0wVhxldrTpsase44fIoLpyO1OT/hgNMRoJTixj1qjpMRdSRIfih93mR3D/Tss/8CmM7dPwKMjtaGkDpmw==} @@ -24161,6 +24167,10 @@ snapshots: dependencies: '@types/react': 19.1.11 + '@types/react-grid-layout@1.3.5': + dependencies: + '@types/react': 19.1.11 + '@types/react-simple-maps@3.0.4': dependencies: '@types/d3-geo': 2.0.7