From 6763e3c54383b0c669c0d22cdfeb3bd5c300d57e Mon Sep 17 00:00:00 2001 From: syntaxbullet Date: Thu, 8 Jan 2026 21:39:01 +0100 Subject: [PATCH] fix: address code review findings for analytics and security --- shared/db/schema.ts | 8 ++- .../dashboard/dashboard.service.test.ts | 61 +++++++++++++++++++ shared/modules/dashboard/dashboard.service.ts | 16 ++--- web/src/components/ActivityChart.tsx | 17 +++--- web/src/hooks/use-activity-stats.ts | 7 ++- web/src/server.ts | 31 +++++++--- 6 files changed, 114 insertions(+), 26 deletions(-) diff --git a/shared/db/schema.ts b/shared/db/schema.ts index 78330de..d96b875 100644 --- a/shared/db/schema.ts +++ b/shared/db/schema.ts @@ -13,7 +13,13 @@ import { bigserial, check } from 'drizzle-orm/pg-core'; -import { relations, sql } from 'drizzle-orm'; +import { relations, sql, type InferSelectModel } from 'drizzle-orm'; + +export type User = InferSelectModel; +export type Transaction = InferSelectModel; +export type ModerationCase = InferSelectModel; +export type Item = InferSelectModel; +export type Inventory = InferSelectModel; // --- TABLES --- diff --git a/shared/modules/dashboard/dashboard.service.test.ts b/shared/modules/dashboard/dashboard.service.test.ts index 9c8638b..644383d 100644 --- a/shared/modules/dashboard/dashboard.service.test.ts +++ b/shared/modules/dashboard/dashboard.service.test.ts @@ -227,5 +227,66 @@ describe("dashboardService", () => { expect(otherHour?.transactions).toBe(0); expect(otherHour?.commands).toBe(0); }); + + test("should return 24 hours of zeros if database is empty", async () => { + mockSelect.mockImplementationOnce(() => ({ + // @ts-ignore + from: mock(() => ({ + where: mock(() => ({ + groupBy: mock(() => ({ + orderBy: mock(() => Promise.resolve([])) + })) + })) + })) + })); + + const activity = await dashboardService.getActivityAggregation(); + expect(activity).toHaveLength(24); + expect(activity.every(a => a.transactions === 0 && a.commands === 0)).toBe(true); + }); + + test("should return 24 hours of zeros if database returns rows with null hours", async () => { + mockSelect.mockImplementationOnce(() => ({ + // @ts-ignore + from: mock(() => ({ + where: mock(() => ({ + groupBy: mock(() => ({ + orderBy: mock(() => Promise.resolve([{ hour: null, transactions: "10", commands: "5" }])) + })) + })) + })) + })); + + const activity = await dashboardService.getActivityAggregation(); + expect(activity).toHaveLength(24); + expect(activity.every(a => a.transactions === 0 && a.commands === 0)).toBe(true); + }); + + test("should correctly map hours regardless of input sort order", async () => { + const now = new Date(); + now.setHours(now.getHours(), 0, 0, 0); + const hourAgo = new Date(now.getTime() - 60 * 60 * 1000); + + mockSelect.mockImplementationOnce(() => ({ + // @ts-ignore + from: mock(() => ({ + where: mock(() => ({ + groupBy: mock(() => ({ + orderBy: mock(() => Promise.resolve([ + { hour: now.toISOString(), transactions: "10", commands: "5" }, + { hour: hourAgo.toISOString(), transactions: "20", commands: "10" } + ])) + })) + })) + })) + })); + + const activity = await dashboardService.getActivityAggregation(); + const current = activity.find(a => a.hour === now.toISOString()); + const past = activity.find(a => a.hour === hourAgo.toISOString()); + + expect(current?.transactions).toBe(10); + expect(past?.transactions).toBe(20); + }); }); }); diff --git a/shared/modules/dashboard/dashboard.service.ts b/shared/modules/dashboard/dashboard.service.ts index 3dd5714..8a45335 100644 --- a/shared/modules/dashboard/dashboard.service.ts +++ b/shared/modules/dashboard/dashboard.service.ts @@ -1,6 +1,6 @@ import { DrizzleClient } from "@shared/db/DrizzleClient"; -import { users, transactions, moderationCases, inventory } from "@db/schema"; -import { desc, sql, and, gte } from "drizzle-orm"; +import { users, transactions, moderationCases, inventory, type User } from "@db/schema"; +import { desc, sql, gte } from "drizzle-orm"; import type { RecentEvent, ActivityData } from "./dashboard.types"; import { TransactionType } from "@shared/lib/constants"; @@ -39,18 +39,18 @@ export const dashboardService = { const allUsers = await DrizzleClient.select().from(users); const totalWealth = allUsers.reduce( - (acc: bigint, u: any) => acc + (u.balance || 0n), + (acc: bigint, u: User) => acc + (u.balance || 0n), 0n ); const avgLevel = allUsers.length > 0 ? Math.round( - allUsers.reduce((acc: number, u: any) => acc + (u.level || 1), 0) / allUsers.length + allUsers.reduce((acc: number, u: User) => acc + (u.level || 1), 0) / allUsers.length ) : 1; const topStreak = allUsers.reduce( - (max: number, u: any) => Math.max(max, u.dailyStreak || 0), + (max: number, u: User) => Math.max(max, u.dailyStreak || 0), 0 ); @@ -83,7 +83,7 @@ export const dashboardService = { }, }); - return recentTx.map((tx: any) => ({ + return recentTx.map((tx) => ({ type: 'info' as const, message: `${tx.user?.username || 'Unknown'}: ${tx.description || 'Transaction'}`, timestamp: tx.createdAt || new Date(), @@ -103,11 +103,11 @@ export const dashboardService = { where: gte(moderationCases.createdAt, oneDayAgo), }); - return recentCases.map((modCase: any) => ({ + return recentCases.map((modCase) => ({ type: modCase.type === 'warn' || modCase.type === 'ban' ? 'error' : 'info', message: `${modCase.type.toUpperCase()}: ${modCase.username} - ${modCase.reason}`, timestamp: modCase.createdAt || new Date(), - icon: getModerationIcon(modCase.type), + icon: getModerationIcon(modCase.type as string), })); }, diff --git a/web/src/components/ActivityChart.tsx b/web/src/components/ActivityChart.tsx index 655cce8..71c37d9 100644 --- a/web/src/components/ActivityChart.tsx +++ b/web/src/components/ActivityChart.tsx @@ -15,23 +15,22 @@ interface ActivityChartProps { loading?: boolean; } -const CustomTooltip = ({ active, payload, label }: any) => { +const CustomTooltip = ({ active, payload }: any) => { if (active && payload && payload.length) { - const date = new Date(label); - const timeStr = date.toLocaleTimeString([], { hour: '2-digit', minute: '2-digit' }); + const data = payload[0].payload; return (

- {timeStr} + {data.displayTime}

- Commands + Commands {payload[0].value}

- Transactions + Transactions {payload[1].value}

@@ -72,8 +71,8 @@ export const ActivityChart: React.FC = ({ data, loading }) = - - + + = ({ data, loading }) = { setLoading(true); try { - const response = await fetch("/api/stats/activity"); + const token = (window as any).AURORA_ENV?.ADMIN_TOKEN; + const response = await fetch("/api/stats/activity", { + headers: { + "Authorization": `Bearer ${token}` + } + }); if (!response.ok) throw new Error(`HTTP error! status: ${response.status}`); const jsonData = await response.json(); setData(jsonData); diff --git a/web/src/server.ts b/web/src/server.ts index 349a2cc..320dabb 100644 --- a/web/src/server.ts +++ b/web/src/server.ts @@ -60,7 +60,8 @@ export async function createWebServer(config: WebServerConfig = {}): Promise | null = null; + let lastActivityFetch: number = 0; const ACTIVITY_CACHE_TTL = 5 * 60 * 1000; // 5 minutes const server = serve({ @@ -103,15 +104,31 @@ export async function createWebServer(config: WebServerConfig = {}): Promise= ACTIVITY_CACHE_TTL)) { + activityPromise = (async () => { + const { dashboardService } = await import("@shared/modules/dashboard/dashboard.service"); + return await dashboardService.getActivityAggregation(); + })(); + lastActivityFetch = now; + } + + const activity = await activityPromise; return Response.json(activity); } catch (error) { console.error("Error fetching activity stats:", error);