From f576fbde3ecf8fc42f6b9601a6338e2ef9b41eed Mon Sep 17 00:00:00 2001 From: iris Date: Thu, 30 Jul 2026 21:49:22 +0200 Subject: [PATCH] stats: zero-fill by_hour gaps; fix chart blank labels + unnecessary rebuilds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #52, closes #55. Follow-ups from the #47 review round. #52: by_hour_of_day was already zero-filled across 0-23 (deliberately, so a quiet hour doesn't misread as missing data) but by_hour — the continuous timeline — wasn't. The client renders it on a categorical axis, so closed hours between two festival nights collapsed to nothing and the last hour of one night sat directly next to the first hour of the next. Now zero-filled between the first and last real bucket, same reasoning as by_hour_of_day. #55, two fixes: - DayChart's x scale is now explicitly ordinal (distr: 2). The prior default (linear) let uPlot's tick generator pick fractional increments on a short series, and the index-based label lookup misses on a non-integer tick, rendering a blank label. - The four per-day/per-hour chart data preps in Stats.tsx now produce a single memoized {labels, series} object each, instead of building fresh labels/series array literals inline in JSX on every render. DayChart's effect is keyed on those props by reference, so the old code destroyed and recreated every uPlot instance on any unrelated Dashboard re-render (e.g. the isAdmin check resolving after data already loaded). Both sides build/typecheck clean. Manually verified the zero-fill against a seeded DB with a 3-hour gap between two transactions — by_hour correctly returned 4 buckets (2 real, 2 zero-filled) in order. --- client/src/stats/DayChart.tsx | 7 ++++- client/src/stats/Stats.tsx | 54 ++++++++++++++++++++++------------- server/src/routes/admin.ts | 35 +++++++++++++++++++++-- 3 files changed, 73 insertions(+), 23 deletions(-) diff --git a/client/src/stats/DayChart.tsx b/client/src/stats/DayChart.tsx index 780369b..c71608d 100644 --- a/client/src/stats/DayChart.tsx +++ b/client/src/stats/DayChart.tsx @@ -32,7 +32,12 @@ export function DayChart({ labels, series }: { labels: string[]; series: ChartSe height: 220, legend: { show: series.length > 1 }, cursor: { drag: { x: false, y: false } }, - scales: { x: { time: false } }, + // distr: 2 (ordinal) — the default (1, linear) lets uPlot's tick + // generator pick fractional increments on a short/sparse series, + // and the label lookup below (`labels[v]`) misses on a non-integer + // v, rendering a blank tick. Ordinal forces whole-number splits, + // which is what an index-based x-axis always wants anyway (#55). + scales: { x: { time: false, distr: 2 } }, axes: [ { stroke: '#888', grid: { stroke: '#333' }, values: (_u, vals) => vals.map(v => labels[v] ?? '') }, { stroke: '#888', grid: { stroke: '#333' } }, diff --git a/client/src/stats/Stats.tsx b/client/src/stats/Stats.tsx index 6596c3b..4f16384 100644 --- a/client/src/stats/Stats.tsx +++ b/client/src/stats/Stats.tsx @@ -135,26 +135,40 @@ function Dashboard({ data, isAdmin, onReset }: { data: StatsData; isAdmin: boole // by_day/per_drink_by_day come pre-sorted from the server (by_day newest // first for the table below, per_drink_by_day oldest first for the // chart's left-to-right reading order) — no re-sort needed here. - const revenueSeries = useMemo( - () => [...data.by_day].reverse().map(d => ({ x: fmtDayShort(d.day), y: d.paid_cents / 100 })), - [data.by_day] - ); - const txSeries = useMemo( - () => [...data.by_day].reverse().map(d => ({ x: fmtDayShort(d.day), y: d.tx_count })), - [data.by_day] - ); + // + // Each of these produces the full {labels, series} shape DayChart wants, + // memoized as one object (#55) — building `series`/`labels` as fresh + // array/object literals inline in JSX on every render defeats DayChart's + // effect (keyed on those props by reference, not deep-equality), so + // Dashboard re-rendering for any unrelated reason (e.g. the isAdmin + // check resolving after data already loaded) would destroy/recreate + // every uPlot instance for no data-related reason. + const revenueChart = useMemo(() => { + const days = [...data.by_day].reverse(); + return { labels: days.map(d => fmtDayShort(d.day)), series: [{ name: 'Umsatz (€)', values: days.map(d => d.paid_cents / 100) }] }; + }, [data.by_day]); + const txChart = useMemo(() => { + const days = [...data.by_day].reverse(); + return { labels: days.map(d => fmtDayShort(d.day)), series: [{ name: 'Transaktionen', values: days.map(d => d.tx_count) }] }; + }, [data.by_day]); // #45's two hourly readings — see the by_hour/by_hour_of_day comment on // computeStats() server-side for why they're separate. Revenue only // here (not also a tx-count variant like the per-day charts above) to // keep the page from growing a chart per metric per granularity; ask if // you want tx-count broken out hourly too. - const hourlyRevenue = useMemo( - () => data.by_hour.map(h => ({ x: fmtHour(h.hour), y: h.paid_cents / 100 })), + const hourlyRevenueChart = useMemo( + () => ({ + labels: data.by_hour.map(h => fmtHour(h.hour)), + series: [{ name: 'Umsatz (€)', values: data.by_hour.map(h => h.paid_cents / 100) }], + }), [data.by_hour] ); - const hourOfDayRevenue = useMemo( - () => data.by_hour_of_day.map(h => ({ x: `${String(h.hour_of_day).padStart(2, '0')}h`, y: h.paid_cents / 100 })), + const hourOfDayRevenueChart = useMemo( + () => ({ + labels: data.by_hour_of_day.map(h => `${String(h.hour_of_day).padStart(2, '0')}h`), + series: [{ name: 'Umsatz (€)', values: data.by_hour_of_day.map(h => h.paid_cents / 100) }], + }), [data.by_hour_of_day] ); @@ -187,24 +201,24 @@ function Dashboard({ data, isAdmin, onReset }: { data: StatsData; isAdmin: boole

Umsatz pro Tag

- {revenueSeries.length === 0 + {revenueChart.labels.length === 0 ?

Noch keine Verkäufe

- : p.x)} series={[{ name: 'Umsatz (€)', values: revenueSeries.map(p => p.y) }]} />} + : }

Transaktionen pro Tag

- {txSeries.length === 0 + {txChart.labels.length === 0 ?

Noch keine Verkäufe

- : p.x)} series={[{ name: 'Transaktionen', values: txSeries.map(p => p.y) }]} />} + : }

Umsatz nach Stunde

- {hourlyRevenue.length === 0 + {hourlyRevenueChart.labels.length === 0 ?

Noch keine Verkäufe

- : p.x)} series={[{ name: 'Umsatz (€)', values: hourlyRevenue.map(p => p.y) }]} />} + : }

Umsatz nach Tageszeit

- {hourOfDayRevenue.every(p => p.y === 0) + {hourOfDayRevenueChart.series[0]!.values.every(v => v === 0) ?

Noch keine Verkäufe

- : p.x)} series={[{ name: 'Umsatz (€)', values: hourOfDayRevenue.map(p => p.y) }]} />} + : }

Top-Getränke im Verlauf

{drinkTrend.series.length === 0 diff --git a/server/src/routes/admin.ts b/server/src/routes/admin.ts index 363e808..62c7b6c 100644 --- a/server/src/routes/admin.ts +++ b/server/src/routes/admin.ts @@ -166,8 +166,39 @@ function computeStats(db: DB) { bumpHourAgg(hodAgg, r); } // Oldest first, same reasoning as perDrinkByDay below — this feeds a - // left-to-right timeline chart. - const byHour = [...hourMap.values()].sort((a, b) => a.hour.localeCompare(b.hour)); + // left-to-right timeline chart. Zero-filled between the first and last + // bucket (issue #52) for the same reason byHourOfDay is zero-filled + // below: the client renders this on a categorical axis, so a hidden gap + // (closed hours between two festival nights) would sit flush against + // real data and read as "quiet", not "closed". The fill walks the + // bucket *strings* as if they were UTC wall-clock (Date.UTC, +1h steps) + // rather than real local time — deliberately: this only needs a + // monotonic hour sequence with correct calendar rollover, not real DST + // handling, and reusing localHourBucket()'s actual local-time path here + // would double-apply the DST question this file already takes a + // documented, deliberate simplification on (see localHourBucket's own + // comment in time.ts). + const hourKeyToDate = (key: string): Date => { + const [datePart, timePart] = key.split(' '); + const [y, m, d] = datePart!.split('-').map(Number); + const h = Number(timePart!.split(':')[0]); + return new Date(Date.UTC(y!, m! - 1, d!, h)); + }; + const dateToHourKey = (d: Date): string => { + const pad = (n: number) => String(n).padStart(2, '0'); + return `${d.getUTCFullYear()}-${pad(d.getUTCMonth() + 1)}-${pad(d.getUTCDate())} ${pad(d.getUTCHours())}:00`; + }; + const sortedHourKeys = [...hourMap.keys()].sort(); + const byHour: Array<{ hour: string } & HourAgg> = []; + if (sortedHourKeys.length > 0) { + let cursor = hourKeyToDate(sortedHourKeys[0]!); + const end = hourKeyToDate(sortedHourKeys[sortedHourKeys.length - 1]!); + while (cursor <= end) { + const key = dateToHourKey(cursor); + byHour.push(hourMap.get(key) ?? { hour: key, tx_count: 0, paid_cents: 0, crew_count: 0, pfand_returns: 0 }); + cursor = new Date(cursor.getTime() + 60 * 60 * 1000); + } + } // Full 0-23 axis, zero-filled — a bar chart with silently missing hours // (e.g. no sales at 6am) reads as a data gap, not "zero", if the bucket // is just absent.