From aa01577ad7f38dac0c719ef9b79c897cb082506f Mon Sep 17 00:00:00 2001 From: rajashish147 Date: Sun, 29 Mar 2026 14:25:48 +0530 Subject: [PATCH 1/2] feat: optimize attendance and expenses controllers with logging and pagination improvements --- .../attendance/attendance.controller.ts | 8 +- .../attendance/attendance.repository.ts | 39 ++++-- .../modules/expenses/expenses.controller.ts | 18 ++- .../modules/expenses/expenses.repository.ts | 118 +++++++----------- .../20260329000100_phase30_perf_indexes.sql | 78 ++++++++++++ 5 files changed, 170 insertions(+), 91 deletions(-) create mode 100644 supabase/migrations/20260329000100_phase30_perf_indexes.sql diff --git a/apps/api/src/modules/attendance/attendance.controller.ts b/apps/api/src/modules/attendance/attendance.controller.ts index 74b6075..d848679 100644 --- a/apps/api/src/modules/attendance/attendance.controller.ts +++ b/apps/api/src/modules/attendance/attendance.controller.ts @@ -29,7 +29,13 @@ export const attendanceController = { try { const parsed = paginationSchema.parse(request.query); const result = await attendanceService.getMySessions(request, parsed.page, parsed.limit); - reply.status(200).send(paginated(result.data, parsed.page, parsed.limit, result.total)); + const response = paginated(result.data, parsed.page, parsed.limit, result.total); + const payloadBytes = Buffer.byteLength(JSON.stringify(response)); + request.log.info( + { route: "/attendance/my-sessions", payloadBytes, sessionCount: result.data.length }, + "phase30:my-sessions", + ); + reply.status(200).send(response); } catch (error) { handleError(error, request, reply, "Unexpected error fetching user sessions"); } diff --git a/apps/api/src/modules/attendance/attendance.repository.ts b/apps/api/src/modules/attendance/attendance.repository.ts index c058e54..8c35545 100644 --- a/apps/api/src/modules/attendance/attendance.repository.ts +++ b/apps/api/src/modules/attendance/attendance.repository.ts @@ -214,7 +214,8 @@ export const attendanceRepository = { /** * Get all sessions for a specific employee (employee's own sessions). - * Joins employees to include employee_code, employee_name, and activityStatus. + * Paginated, ordered by checkin_at DESC. employee_name/employee_code are null + * (caller already knows their own identity — no join needed). */ async findSessionsByUser( request: FastifyRequest, @@ -222,9 +223,15 @@ export const attendanceRepository = { page: number, limit: number, ): Promise<{ data: EnrichedAttendanceSession[]; total: number }> { + // Phase 30: removed employees join (employee knows their own identity) and + // distance_recalculation_status (always null, not used by frontend). + // count:"estimated" eliminates the shadow SELECT COUNT(*) on every list call. const { data, error, count } = await applyPagination( orgTable(request, "attendance_sessions") - .select("id, employee_id, organization_id, checkin_at, checkout_at, distance_recalculation_status, total_distance_km, total_duration_seconds, created_at, updated_at, employees!attendance_sessions_employee_id_fkey(name, employee_code)", { count: "exact" }) + .select( + "id, employee_id, organization_id, checkin_at, checkout_at, total_distance_km, total_duration_seconds, created_at, updated_at", + { count: "estimated" }, + ) .eq("employee_id", employeeId) .order("checkin_at", { ascending: false }), page, @@ -235,16 +242,21 @@ export const attendanceRepository = { throw new Error(`Failed to fetch user sessions: ${error.message}`); } - const mapped = ((data ?? []) as Array>).map((row) => { - const emp = row.employees as { name?: string; employee_code?: string } | null; - const { employees: _, ...rest } = row; - return { - ...rest, - employee_name: emp?.name ?? null, - employee_code: emp?.employee_code ?? null, - activityStatus: computeActivityStatus(rest.checkout_at as string | null), - } as EnrichedAttendanceSession; - }); + const mapped = ((data ?? []) as Array>).map((row) => ({ + id: row.id as string, + employee_id: row.employee_id as string, + organization_id: row.organization_id as string, + checkin_at: row.checkin_at as string, + checkout_at: row.checkout_at as string | null, + total_distance_km: row.total_distance_km as number | null, + total_duration_seconds: row.total_duration_seconds as number | null, + distance_recalculation_status: null, + created_at: row.created_at as string, + updated_at: row.updated_at as string, + employee_name: null, + employee_code: null, + activityStatus: computeActivityStatus(row.checkout_at as string | null), + } as EnrichedAttendanceSession)); return { data: mapped, total: count ?? 0 }; }, @@ -264,11 +276,12 @@ export const attendanceRepository = { const safeOffset = (Math.max(1, page) - 1) * safeLimit; // Join employees via FK so employee_name and employee_code are always present. + // Phase 30: count:"estimated" eliminates the shadow SELECT COUNT(*) query. let query = supabase .from("employee_latest_sessions") .select( "employee_id, organization_id, session_id, latest_checkin, latest_checkout, total_distance_km, total_duration_seconds, status, updated_at, employees!employee_latest_sessions_employee_id_fkey(name, employee_code)", - { count: "exact" }, + { count: "estimated" }, ) .eq("organization_id", request.organizationId); diff --git a/apps/api/src/modules/expenses/expenses.controller.ts b/apps/api/src/modules/expenses/expenses.controller.ts index f1511ed..f05c509 100644 --- a/apps/api/src/modules/expenses/expenses.controller.ts +++ b/apps/api/src/modules/expenses/expenses.controller.ts @@ -44,7 +44,13 @@ export const expensesController = { parsed.page, parsed.limit, ); - reply.status(200).send(paginated(result.data, parsed.page, parsed.limit, result.total)); + const response = paginated(result.data, parsed.page, parsed.limit, result.total); + const payloadBytes = Buffer.byteLength(JSON.stringify(response)); + request.log.info( + { route: "/expenses/my", payloadBytes, expenseCount: result.data.length }, + "phase30:expenses-my", + ); + reply.status(200).send(response); } catch (error) { handleError(error, request, reply, "Unexpected error fetching user expenses"); } @@ -69,11 +75,17 @@ export const expensesController = { parsed.limit, employeeId, ); - reply.status(200).send(paginated(result.data, parsed.page, parsed.limit, result.total)); + const response = paginated(result.data, parsed.page, parsed.limit, result.total); + const payloadBytes = Buffer.byteLength(JSON.stringify(response)); + request.log.info( + { route: "/admin/expenses", payloadBytes, expenseCount: result.data.length }, + "phase30:admin-expenses", + ); + reply.status(200).send(response); } catch (error) { handleError(error, request, reply, "Unexpected error fetching org expenses"); } - }, + } /** * PATCH /admin/expenses/:id diff --git a/apps/api/src/modules/expenses/expenses.repository.ts b/apps/api/src/modules/expenses/expenses.repository.ts index ca378a7..b8dfcbc 100644 --- a/apps/api/src/modules/expenses/expenses.repository.ts +++ b/apps/api/src/modules/expenses/expenses.repository.ts @@ -76,9 +76,13 @@ export const expensesRepository = { page: number, limit: number, ): Promise<{ data: EnrichedExpense[]; total: number }> { + // Phase 30: removed employees join (caller already knows their own identity). + // count:"estimated" eliminates the shadow SELECT COUNT(*) on every list call. + // Index idx_expenses_org_emp_submitted (org_id, emp_id, submitted_at DESC) + // covers both the WHERE clause and the ORDER BY in a single index scan. const { data, error, count } = await applyPagination( orgTable(request, "expenses") - .select(EXPENSE_ENRICHED_COLS, { count: "exact" }) + .select(EXPENSE_COLS, { count: "estimated" }) .eq("employee_id", employeeId) .order("submitted_at", { ascending: false }), page, @@ -89,7 +93,11 @@ export const expensesRepository = { throw new Error(`Failed to fetch user expenses: ${error.message}`); } return { - data: ((data ?? []) as Array>).map(flattenEmployee), + data: ((data ?? []) as Array>).map((row) => ({ + ...(row as Expense), + employee_name: null, + employee_code: null, + })), total: count ?? 0, }; }, @@ -100,8 +108,11 @@ export const expensesRepository = { limit: number, employeeId?: string, ): Promise<{ data: EnrichedExpense[]; total: number }> { + // Phase 30: count:"estimated" eliminates the shadow SELECT COUNT(*) query. + // idx_expenses_org_submitted_desc (org_id, submitted_at DESC) covers the + // org-wide list; idx_expenses_org_emp_submitted covers the per-employee filter. let baseQuery = orgTable(request, "expenses") - .select(EXPENSE_ENRICHED_COLS, { count: "exact" }) + .select(EXPENSE_ENRICHED_COLS, { count: "estimated" }) .order("submitted_at", { ascending: false }); if (employeeId) { @@ -147,89 +158,48 @@ export const expensesRepository = { }, /** - * Returns one summary row per employee with pending/total expense aggregates. - * Sorted: employees with pending expenses first, then by latest expense date DESC. - * O(distinct employees) — significantly faster than returning all expense rows. + * Returns one aggregated row per employee with pending/total expense metrics. + * Sorted: employees with ≥1 PENDING expense first, then by latest submitted_at DESC. + * + * Phase 30: replaced the previous 50 000-row in-memory GROUP BY with a DB-side + * SQL aggregation via get_expense_summary_by_employee(). The function runs a + * single indexed GROUP BY on the expenses table and joins employees once — + * O(distinct employees) instead of O(total expenses). */ async findExpenseSummaryByEmployee( request: FastifyRequest, page: number, limit: number, ): Promise<{ data: EmployeeExpenseSummary[]; total: number }> { - // Fetch the full (org-scoped) expense list with employee info. - // We group in application code to avoid a raw SQL RPC; the expense - // table is orders of magnitude smaller than attendance_sessions. - // - // Safety cap: 50 000 rows is sufficient for any realistic org over several - // years of operation (100 employees × 10 expenses/month × 48 months = 48 000). - // Without this limit a pathological dataset could cause an unbounded fetch - // that exhausts server memory. If the cap is hit, the structured warning - // below will be visible in Loki so operators know to migrate to a DB-side - // GROUP BY aggregation. - const EXPENSE_SUMMARY_LIMIT = 50_000; - const { data, error } = await orgTable(request, "expenses") - .select(EXPENSE_ENRICHED_COLS) - .order("submitted_at", { ascending: false }) - .limit(EXPENSE_SUMMARY_LIMIT); + const { data, error } = await supabase.rpc("get_expense_summary_by_employee", { + p_org_id: request.organizationId, + }); if (error) { throw new Error(`Failed to fetch expense summary: ${error.message}`); } - const rows = ((data ?? []) as Array>).map(flattenEmployee); - - // Warn operators when the safety cap fires — this is a signal to migrate - // findExpenseSummaryByEmployee to a DB-side GROUP BY aggregation. - if (rows.length >= EXPENSE_SUMMARY_LIMIT) { - (request as { log?: { warn: (obj: object, msg: string) => void } }).log?.warn( - { - organizationId: request.organizationId, - rowsCapped: EXPENSE_SUMMARY_LIMIT, - }, - "findExpenseSummaryByEmployee hit safety row cap — summary may be incomplete; migrate to DB-side aggregation", - ); - } - - // Aggregate per employee - const map = new Map(); - for (const row of rows) { - const empId = row.employee_id as string; - const existing = map.get(empId); - const amount = row.amount as number; - const isPending = row.status === "PENDING"; - - if (!existing) { - map.set(empId, { - employeeId: empId, - employeeName: row.employee_name ?? `Employee …${empId.slice(-4)}`, - employeeCode: row.employee_code ?? null, - pendingCount: isPending ? 1 : 0, - pendingAmount: isPending ? amount : 0, - totalCount: 1, - totalAmount: amount, - latestExpenseDate: row.submitted_at as string, - }); - } else { - existing.totalCount++; - existing.totalAmount += amount; - if (isPending) { - existing.pendingCount++; - existing.pendingAmount += amount; - } - } - } - - // Sort: pending first, then by latest expense date - const groups = [...map.values()].sort((a, b) => { - if (b.pendingCount !== a.pendingCount) return b.pendingCount - a.pendingCount; - return (b.latestExpenseDate ?? "").localeCompare(a.latestExpenseDate ?? ""); - }); + type SummaryRow = { + employee_id: string; + employee_name: string; + employee_code: string | null; + pending_count: number | string; + pending_amount: number | string; + total_count: number | string; + total_amount: number | string; + latest_expense_date: string | null; + }; - // Round amounts - for (const g of groups) { - g.pendingAmount = Math.round(g.pendingAmount * 100) / 100; - g.totalAmount = Math.round(g.totalAmount * 100) / 100; - } + const groups: EmployeeExpenseSummary[] = ((data ?? []) as SummaryRow[]).map((row) => ({ + employeeId: row.employee_id, + employeeName: row.employee_name, + employeeCode: row.employee_code, + pendingCount: Number(row.pending_count), + pendingAmount: Math.round(Number(row.pending_amount) * 100) / 100, + totalCount: Number(row.total_count), + totalAmount: Math.round(Number(row.total_amount) * 100) / 100, + latestExpenseDate: row.latest_expense_date, + })); const total = groups.length; const safeLimit = Math.min(100, Math.max(1, limit)); diff --git a/supabase/migrations/20260329000100_phase30_perf_indexes.sql b/supabase/migrations/20260329000100_phase30_perf_indexes.sql new file mode 100644 index 0000000..25f4576 --- /dev/null +++ b/supabase/migrations/20260329000100_phase30_perf_indexes.sql @@ -0,0 +1,78 @@ +-- Phase 30: Query performance optimisation +-- +-- Bottlenecks identified: +-- 1. GET /expenses/my — no covering index for (org_id, emp_id, submitted_at DESC) +-- 2. GET /admin/expenses — missing DESC variant of (org_id, submitted_at) +-- 3. GET /admin/expenses/summary — 50 000-row in-memory GROUP BY replaced by SQL aggregation +-- +-- Changes: +-- A. Add two covering composite indexes on expenses. +-- B. Create SQL function get_expense_summary_by_employee() that performs the +-- GROUP BY aggregation on the DB side, eliminating the application-level scan. +-- +-- ────────────────────────────────────────────────────────────────────────────── +-- A. INDEXES +-- ────────────────────────────────────────────────────────────────────────────── + +-- Covers: WHERE organization_id = ? AND employee_id = ? ORDER BY submitted_at DESC +-- Used by: GET /expenses/my (findExpensesByUser) and +-- GET /admin/expenses?employee_id=... (findExpensesByOrg with filter) +-- Existing idx_expenses_org_employee (org_id, emp_id) had no ordering column, +-- forcing a post-filter sort. This eliminates it. +CREATE INDEX IF NOT EXISTS idx_expenses_org_emp_submitted + ON public.expenses (organization_id, employee_id, submitted_at DESC); + +-- Covers: WHERE organization_id = ? ORDER BY submitted_at DESC +-- Used by: GET /admin/expenses (findExpensesByOrg without employee filter) +-- Postgres supports backward B-tree scans so the existing idx_expenses_org_submitted +-- (ASC) could already be used in reverse; the explicit DESC index below ensures the +-- planner always picks an index-only forward scan for the most common admin list. +CREATE INDEX IF NOT EXISTS idx_expenses_org_submitted_desc + ON public.expenses (organization_id, submitted_at DESC); + +-- ────────────────────────────────────────────────────────────────────────────── +-- B. SQL AGGREGATION FUNCTION +-- ────────────────────────────────────────────────────────────────────────────── + +-- Replaces the application-level "fetch up to 50 000 rows then GROUP BY" pattern +-- in expensesRepository.findExpenseSummaryByEmployee(). +-- +-- Returns one row per employee (matching EmployeeExpenseSummary interface). +-- Sorted: employees with ≥1 PENDING expense first, then by latest submitted_at DESC. +-- LEFT JOIN preserves expense rows whose employee_id has been soft-deleted. +-- COALESCE name fallback mirrors the existing JS fallback for missing employee rows. +-- +-- Security: called exclusively by supabaseServiceClient (service role), which +-- bypasses RLS. The p_org_id parameter enforces tenant isolation at the query level. +-- SET search_path = public prevents search-path injection (Phase 29 hardening pattern). + +CREATE OR REPLACE FUNCTION get_expense_summary_by_employee(p_org_id UUID) +RETURNS TABLE ( + employee_id UUID, + employee_name TEXT, + employee_code TEXT, + pending_count BIGINT, + pending_amount NUMERIC, + total_count BIGINT, + total_amount NUMERIC, + latest_expense_date TIMESTAMPTZ +) +LANGUAGE SQL +STABLE +SET search_path = public +AS $$ + SELECT + e.employee_id, + COALESCE(emp.name, 'Employee …' || RIGHT(e.employee_id::TEXT, 4)) AS employee_name, + emp.employee_code, + COUNT(*) FILTER (WHERE e.status = 'PENDING') AS pending_count, + COALESCE(SUM(e.amount) FILTER (WHERE e.status = 'PENDING'), 0::NUMERIC) AS pending_amount, + COUNT(*) AS total_count, + COALESCE(SUM(e.amount), 0::NUMERIC) AS total_amount, + MAX(e.submitted_at) AS latest_expense_date + FROM public.expenses e + LEFT JOIN public.employees emp ON emp.id = e.employee_id + WHERE e.organization_id = p_org_id + GROUP BY e.employee_id, emp.name, emp.employee_code + ORDER BY pending_count DESC, latest_expense_date DESC; +$$; From 529b93ed94e51925b3c5b2db93639acb40d45e23 Mon Sep 17 00:00:00 2001 From: rajashish147 Date: Sun, 29 Mar 2026 14:33:47 +0530 Subject: [PATCH 2/2] fix: update fastify version in package-lock and correct devOptional flags --- apps/api/src/modules/expenses/expenses.controller.ts | 2 +- package-lock.json | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/api/src/modules/expenses/expenses.controller.ts b/apps/api/src/modules/expenses/expenses.controller.ts index f05c509..58e1f01 100644 --- a/apps/api/src/modules/expenses/expenses.controller.ts +++ b/apps/api/src/modules/expenses/expenses.controller.ts @@ -85,7 +85,7 @@ export const expensesController = { } catch (error) { handleError(error, request, reply, "Unexpected error fetching org expenses"); } - } + }, /** * PATCH /admin/expenses/:id diff --git a/package-lock.json b/package-lock.json index dbca4bf..0bb5557 100644 --- a/package-lock.json +++ b/package-lock.json @@ -37,7 +37,7 @@ "@types/jsonwebtoken": "^9.0.10", "bullmq": "^5.70.4", "dotenv": "^17.3.1", - "fastify": "^5.8.2", + "fastify": "^5.8.3", "fastify-plugin": "^5.1.0", "fastify-type-provider-zod": "^6.1.0", "ioredis": "^5.10.0", @@ -8910,7 +8910,7 @@ "version": "19.2.14", "resolved": "https://registry.npmjs.org/@types/react/-/react-19.2.14.tgz", "integrity": "sha512-ilcTH/UniCkMdtexkoCN0bI7pMcJDvmQFPvuPvmEaYA/NSfFTAgdUSLAoVjaRJm7+6PvcM+q1zYOwS4wTYMF9w==", - "devOptional": true, + "dev": true, "license": "MIT", "dependencies": { "csstype": "^3.2.2" @@ -8920,7 +8920,7 @@ "version": "19.2.3", "resolved": "https://registry.npmjs.org/@types/react-dom/-/react-dom-19.2.3.tgz", "integrity": "sha512-jp2L/eY6fn+KgVVQAOqYItbF0VY/YApe5Mz2F0aykSO8gx31bYCZyvSeYxCHKvzHG5eZjc+zyaS5BrBWya2+kQ==", - "devOptional": true, + "dev": true, "license": "MIT", "peerDependencies": { "@types/react": "^19.2.0"