-
Notifications
You must be signed in to change notification settings - Fork 44
feat(logging): integrate centralized logger across auth, payout, prof… #128
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
feat(logging): integrate centralized logger across auth, payout, prof… #128
Conversation
…ile & files APIs Add async, non-blocking logger service with Prisma-safe JSON metadata Replace server-side console.error across API routes Preserve API response formats and status codes Integrate logger in auth, payout, profile, and file routes Include user context and request metadata in logs
WalkthroughAdds a new structured logging service and instruments many API routes (auth, payout, helper files, profile) to use it instead of console logging. Also wraps register-role updates in a transaction and updates TypeScript compiler types. Control flow and API responses remain largely unchanged. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant R as Route Handler
participant DB as Database / Services
participant L as logger (info/error)
participant S as logSystemEvent (async)
participant P as Prisma Log Table
C->>R: HTTP request
R->>DB: perform query or mutation
alt Success
R-->>C: 2xx response
R->>L: info(...) %% structured info log enqueued
else Validation / NotFound
R-->>C: 4xx response
R->>L: error(...) %% structured error log enqueued
else Exception
R-->>C: 5xx response
R->>L: error(..., err) %% structured error log enqueued with normalized error
end
note over L,S: Non-blocking / fire-and-forget
L->>S: enqueue writeSystemLog(params)
par Persist (async)
S->>P: write log record
P-->>S: { success | failure }
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/app/api/(helper)/files/manage-files/route.ts (1)
16-18: Critical: Restrict Supabase service role key to server-only
- src/app/api/(helper)/files/manage-files/route.ts (line 17): replace
process.env.NEXT_PUBLIC_SUPABASE_SERVICE_ROLE_KEY→process.env.SUPABASE_SERVICE_ROLE_KEY- src/app/api/(helper)/files/upload-avatar/route.ts (line 21): same replacement
- README.md (line 112): update env var
NEXT_PUBLIC_SUPABASE_SERVICE_ROLE_KEY→SUPABASE_SERVICE_ROLE_KEYsrc/app/api/(helper)/files/upload-avatar/route.ts (1)
20-22: Use server-only env var for Supabase service role key
- src/app/api/(helper)/files/upload-avatar/route.ts: replace
process.env.NEXT_PUBLIC_SUPABASE_SERVICE_ROLE_KEY→process.env.SUPABASE_SERVICE_ROLE_KEY- src/app/api/(helper)/files/manage-files/route.ts: replace
process.env.NEXT_PUBLIC_SUPABASE_SERVICE_ROLE_KEY→process.env.SUPABASE_SERVICE_ROLE_KEY
🧹 Nitpick comments (25)
src/lib/services/logger.ts (4)
20-28: Make metadata JSON-safe to avoid log write failures.If metadata includes non-serializable values (e.g., BigInt, Symbols, circular refs), Prisma JSON serialization can throw and drop the log. Normalize metadata before persisting.
Apply within this block:
- const metadataValue = - params.metadata === null - ? Prisma.JsonNull - : (params.metadata as unknown as Prisma.InputJsonValue | undefined); + const safeMeta = + params.metadata === null + ? Prisma.JsonNull + : (jsonSafe(params.metadata) as unknown as Prisma.InputJsonValue | undefined);Add helper:
function jsonSafe(input: unknown): unknown { try { return JSON.parse(JSON.stringify(input, (_k, v) => (typeof v === "bigint" ? String(v) : v))); } catch { return undefined; } }Also applies to: 34-47
56-63: Gate console output to development.PR notes mention console INFO/ERROR logs in development. Current code logs in all envs. Gate to reduce noise/cost in prod.
- // eslint-disable-next-line no-console - console.info(`[INFO] ${action}: ${description}`, ctx?.metadata ?? ""); + if (process.env.NODE_ENV === "development") { + // eslint-disable-next-line no-console + console.info(`[INFO] ${action}: ${description}`, ctx?.metadata ?? ""); + }- // eslint-disable-next-line no-console - console.error(`[ERROR] ${action}: ${description}`, metadata); + if (process.env.NODE_ENV === "development") { + // eslint-disable-next-line no-console + console.error(`[ERROR] ${action}: ${description}`, metadata); + }Also applies to: 77-79
71-86: Avoid overloading entityType with severity.Defaulting entityType to "ERROR" mixes domain entity with severity. Let entityType reflect the domain (USER, PAYOUT, FILE), and infer severity from action or add a severity field.
- entityType: ctx?.entityType ?? "ERROR", + entityType: ctx?.entityType ?? null,Optional: introduce severity: "INFO" | "ERROR" instead of encoding it in action.
4-10: Centralize action names using an enum/const object.Action strings are repeated across files. Define LogAction to prevent typos and enable discoverability, per guidelines.
Add:
export enum LogAction { INFO = "INFO", ERROR = "ERROR", AUTH_CHECK_ROLE = "AUTH_CHECK_ROLE", AUTH_GET_USER_BY_ID = "AUTH_GET_USER_BY_ID", AUTH_GET_USERS_BY_ROLE = "AUTH_GET_USERS_BY_ROLE", AUTH_REGISTER_ROLE = "AUTH_REGISTER_ROLE", PROFILE_PATCH = "PROFILE_PATCH", PAYOUT_UPDATE_STATUS = "PAYOUT_UPDATE_STATUS", // ...extend as needed }Then prefer action: LogAction.PROFILE_PATCH, etc. Based on learnings
Also applies to: 56-87
src/app/api/(auth)/profile/route.ts (2)
84-89: Add metadata for observability.Include minimal context (what changed) to aid debugging while avoiding PII.
- logger.info("Profile updated successfully", { + logger.info("Profile updated successfully", { action: "PROFILE_PATCH", userId: userId, entityType: "USER", + metadata: { + updated: { + user: Boolean(user && Object.keys(user).length), + grantee: Boolean(grantee), + grantProvider: Boolean(grantProvider), + }, + }, });
92-95: Include userId in error context.Helps correlate failures to a subject without storing PII.
- logger.error("/api/profile PATCH error", error, { + logger.error("/api/profile PATCH error", error, { action: "PROFILE_PATCH", + userId: userId, entityType: "USER", });src/app/api/(auth)/get-users-by-role/route.ts (1)
53-57: Validate role against enum before querying.Casting to UserRole without validation risks runtime errors if the column is an enum.
Add a guard:
import type { Prisma } from "@/generated/prisma"; const allowedRoles = Object.values((Prisma as any).UserRole ?? {}) as string[]; if (!allowedRoles.includes(decodedRole)) { return NextResponse.json({ error: "Invalid role" }, { status: 400 }); }Or use a zod enum synced to Prisma.
src/app/api/(payout)/payout/update-status/[id]/route.ts (2)
25-33: Avoid leaking internal error messages to clients.Returning Supabase error.message can expose internals. Keep logs detailed, client message generic.
- return NextResponse.json( - { error: `Error updating status: ${error.message}` }, - { status: 500 }, - ); + return NextResponse.json( + { error: "Error updating payout status" }, + { status: 500 }, + );
5-8: Fix params typing for Next.js route handlers.params isn’t a Promise in Next.js route handlers; awaiting works but is misleading.
-export async function PATCH( - request: Request, - { params }: { params: Promise<{ id: string }> }, -) { +export async function PATCH( + request: Request, + { params }: { params: { id: string } }, +) {src/app/api/(auth)/check-role/[user_id]/route.ts (2)
13-16: Include raw param in validation log metadata.Aids debugging bad client requests.
- logger.error("Invalid user_id parameter", null, { + logger.error("Invalid user_id parameter", null, { action: "AUTH_CHECK_ROLE", entityType: "USER", + metadata: { user_id_raw: user_id }, });
62-65: Add userId to error metadata when available.Improves traceability.
- logger.error("Error checking user role", error, { + logger.error("Error checking user role", error, { action: "AUTH_CHECK_ROLE", entityType: "USER", + metadata: { user_id }, });src/app/api/(auth)/get-user-by-id/route.ts (2)
12-15: Use info (or a distinct action) for validation errors.This isn’t a server error; consider a specific action like AUTH_GET_USER_BY_ID_INVALID or using info-level semantics.
- logger.error("Missing user_id parameter", null, { + logger.info("Missing user_id parameter", { action: "AUTH_GET_USER_BY_ID", entityType: "USER", + metadata: { has_user_id: false }, });
131-134: Include userId in catch-block error metadata.Improves correlation in logs.
- logger.error("Error getting user by id", error, { + logger.error("Error getting user by id", error, { action: "AUTH_GET_USER_BY_ID", entityType: "USER", + metadata: { user_id }, });src/app/api/(payout)/payout/delete/[id]/route.ts (2)
7-11: Fix Next.js route params typing (not a Promise).
paramsisn’t a Promise in Next route handlers; removeawaitand fix the type.Apply:
-export async function DELETE( - request: Request, - { params }: { params: Promise<{ id: string }> }, -) { +export async function DELETE( + request: Request, + { params }: { params: { id: string } }, +) { try { - const { id } = await params; + const { id } = params;As per coding guidelines
12-16: Optionally verify deletion affected a row.Supabase delete doesn’t return row count by default; consider returning 404 if nothing deleted.
Use returning select:
-const { error } = await supabase +const { data, error } = await supabase .from("payout") .delete() - .eq("payout_id", id); + .eq("payout_id", id) + .select("payout_id"); // requires PostgREST "return=representation" + +if (!error && (!data || data.length === 0)) { + return NextResponse.json({ error: `Payout with ID ${id} not found` }, { status: 404 }); +}Also applies to: 35-36
src/app/api/(auth)/register-user/route.ts (1)
8-8: Style: prefer const function for handlers.Adopt
export const POST = asyncfor consistency.-export async function POST(req: Request) { +export const POST = async (req: Request) => { ... -} +}As per coding guidelines
src/app/api/(helper)/files/upload-avatar/route.ts (2)
41-44: Set content type and derive public URL via SDK.Improve correctness and avoid manual URL concatenation.
-const { error } = await supabase.storage +const { data: uploadData, error } = await supabase.storage .from(bucket) - .upload(path, file, { cacheControl: "3600", upsert: true }); + .upload(path, file, { cacheControl: "3600", upsert: true, contentType: file.type }); -const publicUrl = `${supabaseUrl}/storage/v1/object/public/${bucket}/${path}`; +const { data: publicData } = supabase.storage.from(bucket).getPublicUrl(path); +const publicUrl = publicData.publicUrl;Also applies to: 55-56
7-7: Style: prefer const handler.Use
export const POST = asyncfor consistency.As per coding guidelines
src/app/api/(auth)/verify-oauth-user/route.ts (1)
6-6: Style: prefer const handler.Switch to
export const POST = async.As per coding guidelines
src/app/api/(helper)/files/manage-files/route.ts (3)
32-37: Redundant bucket check.
bucketis a constant string; this branch is dead.Either remove the check or source bucket name from env (then keep the check).
12-13: Validate ‘folder’ against allowed values to prevent path abuse.Restrict to "evidence" | "feedback" and sanitize
payoutId.-const folder = String(formData.get("folder") || "evidence"); +const rawFolder = String(formData.get("folder") || "evidence"); +const folder = rawFolder === "feedback" ? "feedback" : "evidence"; + +const payoutId = String(formData.get("payoutId") || "").replace(/[^a-zA-Z0-9_-]/g, "");
65-67: Set content type for uploaded files.Preserve correct MIME type in storage.
-upload(path, file, { upsert: false, cacheControl: "3600" }); +upload(path, file, { upsert: false, cacheControl: "3600", contentType: file.type });src/app/api/(payout)/payout/update/[id]/route.ts (1)
5-12: Fix Next.js route params typing.
paramsis not a Promise; removeawaitand correct type.-export async function PUT( - request: Request, - { params }: { params: Promise<{ id: string }> }, -) { +export async function PUT( + request: Request, + { params }: { params: { id: string } }, +) { try { const payout = await request.json(); - const { id } = await params; + const { id } = params;src/app/api/(auth)/get-user-by-email/route.ts (2)
37-41: Use info/warn instead of error when user not found.404 is expected; avoid polluting error logs.
-logger.error("User not found by email", null, { +logger.info("User not found by email", { action: "AUTH_GET_USER_BY_EMAIL", entityType: "USER", metadata: { email }, });
5-5: Style: prefer const handler.Use
export const GET = asyncfor consistency.As per coding guidelines
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (17)
src/app/api/(auth)/check-role/[user_id]/route.ts(3 hunks)src/app/api/(auth)/get-user-by-email/route.ts(3 hunks)src/app/api/(auth)/get-user-by-id/route.ts(3 hunks)src/app/api/(auth)/get-user-role-by-id/route.ts(4 hunks)src/app/api/(auth)/get-users-by-role/route.ts(2 hunks)src/app/api/(auth)/profile/route.ts(2 hunks)src/app/api/(auth)/register-role/route.ts(2 hunks)src/app/api/(auth)/register-user/route.ts(3 hunks)src/app/api/(auth)/verify-oauth-user/route.ts(5 hunks)src/app/api/(helper)/files/manage-files/route.ts(4 hunks)src/app/api/(helper)/files/upload-avatar/route.ts(3 hunks)src/app/api/(payout)/payout/create/route.ts(2 hunks)src/app/api/(payout)/payout/delete/[id]/route.ts(2 hunks)src/app/api/(payout)/payout/update-status/[id]/route.ts(2 hunks)src/app/api/(payout)/payout/update/[id]/route.ts(3 hunks)src/lib/services/logger.ts(1 hunks)tsconfig.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/frontend-rule.mdc)
**/*.{js,jsx,ts,tsx}: Use early returns whenever possible to make the code more readable.
Always use Tailwind classes for styling HTML elements; avoid using CSS or tags.
Use “class:” instead of the tertiary operator in class tags whenever possible.
Use descriptive variable and function/const names. Also, event functions should be named with a “handle” prefix, like “handleClick” for onClick and “handleKeyDown” for onKeyDown.
Implement accessibility features on elements. For example, a tag should have a tabindex=“0”, aria-label, on:click, and on:keydown, and similar attributes.
Everything must be 100% responsive (using all the sizes of tailwind) and compatible with dark/light shadcn's mode.
Use consts instead of functions, for example, “const toggle = () =>”. Also, define a type if possible.
Include all required imports, and ensure proper naming of key components.
**/*.{js,jsx,ts,tsx}: enum/const object members should be written UPPERCASE_WITH_UNDERSCORE.
Gate flag-dependent code on a check that verifies the flag's values are valid and expected.
If a custom property for a person or event is at any point referenced in two or more files or two or more callsites in the same file, use an enum or const object, as above in feature flags.
Files:
src/app/api/(auth)/check-role/[user_id]/route.tssrc/app/api/(auth)/register-user/route.tssrc/app/api/(payout)/payout/update/[id]/route.tssrc/app/api/(helper)/files/manage-files/route.tssrc/app/api/(auth)/profile/route.tssrc/app/api/(auth)/register-role/route.tssrc/app/api/(payout)/payout/delete/[id]/route.tssrc/lib/services/logger.tssrc/app/api/(helper)/files/upload-avatar/route.tssrc/app/api/(payout)/payout/update-status/[id]/route.tssrc/app/api/(auth)/get-user-by-email/route.tssrc/app/api/(auth)/get-user-by-id/route.tssrc/app/api/(auth)/get-user-role-by-id/route.tssrc/app/api/(auth)/verify-oauth-user/route.tssrc/app/api/(auth)/get-users-by-role/route.tssrc/app/api/(payout)/payout/create/route.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/frontend-rule.mdc)
You can never use anys.
If using TypeScript, use an enum to store flag names.
Files:
src/app/api/(auth)/check-role/[user_id]/route.tssrc/app/api/(auth)/register-user/route.tssrc/app/api/(payout)/payout/update/[id]/route.tssrc/app/api/(helper)/files/manage-files/route.tssrc/app/api/(auth)/profile/route.tssrc/app/api/(auth)/register-role/route.tssrc/app/api/(payout)/payout/delete/[id]/route.tssrc/lib/services/logger.tssrc/app/api/(helper)/files/upload-avatar/route.tssrc/app/api/(payout)/payout/update-status/[id]/route.tssrc/app/api/(auth)/get-user-by-email/route.tssrc/app/api/(auth)/get-user-by-id/route.tssrc/app/api/(auth)/get-user-role-by-id/route.tssrc/app/api/(auth)/verify-oauth-user/route.tssrc/app/api/(auth)/get-users-by-role/route.tssrc/app/api/(payout)/payout/create/route.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/posthog-integration.mdc)
Never hallucinate an API key. Instead, always use the API key populated in the .env file.
Files:
src/app/api/(auth)/check-role/[user_id]/route.tssrc/app/api/(auth)/register-user/route.tssrc/app/api/(payout)/payout/update/[id]/route.tssrc/app/api/(helper)/files/manage-files/route.tssrc/app/api/(auth)/profile/route.tssrc/app/api/(auth)/register-role/route.tssrc/app/api/(payout)/payout/delete/[id]/route.tssrc/lib/services/logger.tssrc/app/api/(helper)/files/upload-avatar/route.tssrc/app/api/(payout)/payout/update-status/[id]/route.tssrc/app/api/(auth)/get-user-by-email/route.tssrc/app/api/(auth)/get-user-by-id/route.tssrc/app/api/(auth)/get-user-role-by-id/route.tssrc/app/api/(auth)/verify-oauth-user/route.tssrc/app/api/(auth)/get-users-by-role/route.tssrc/app/api/(payout)/payout/create/route.ts
🧠 Learnings (1)
📚 Learning: 2025-07-22T01:46:06.934Z
Learnt from: CR
PR: GrantChain/GrantFox#0
File: .cursor/rules/frontend-rule.mdc:0-0
Timestamp: 2025-07-22T01:46:06.934Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Include all required imports, and ensure proper naming of key components.
Applied to files:
tsconfig.json
🧬 Code graph analysis (16)
src/app/api/(auth)/check-role/[user_id]/route.ts (1)
src/lib/services/logger.ts (1)
logger(57-87)
src/app/api/(auth)/register-user/route.ts (2)
src/lib/services/logger.ts (1)
logger(57-87)src/lib/prisma.ts (1)
handleDatabaseError(24-60)
src/app/api/(payout)/payout/update/[id]/route.ts (1)
src/lib/services/logger.ts (1)
logger(57-87)
src/app/api/(helper)/files/manage-files/route.ts (2)
src/lib/services/logger.ts (1)
logger(57-87)src/components/modules/payouts/services/files.service.ts (1)
FilesService(3-27)
src/app/api/(auth)/profile/route.ts (1)
src/lib/services/logger.ts (1)
logger(57-87)
src/app/api/(auth)/register-role/route.ts (2)
src/lib/services/logger.ts (1)
logger(57-87)src/components/modules/auth/services/auth.service.ts (1)
registerRole(38-57)
src/app/api/(payout)/payout/delete/[id]/route.ts (1)
src/lib/services/logger.ts (1)
logger(57-87)
src/lib/services/logger.ts (1)
src/lib/prisma.ts (1)
prisma(7-19)
src/app/api/(helper)/files/upload-avatar/route.ts (1)
src/lib/services/logger.ts (1)
logger(57-87)
src/app/api/(payout)/payout/update-status/[id]/route.ts (1)
src/lib/services/logger.ts (1)
logger(57-87)
src/app/api/(auth)/get-user-by-email/route.ts (1)
src/lib/services/logger.ts (1)
logger(57-87)
src/app/api/(auth)/get-user-by-id/route.ts (1)
src/lib/services/logger.ts (1)
logger(57-87)
src/app/api/(auth)/get-user-role-by-id/route.ts (1)
src/lib/services/logger.ts (1)
logger(57-87)
src/app/api/(auth)/verify-oauth-user/route.ts (2)
src/lib/services/logger.ts (1)
logger(57-87)src/lib/prisma.ts (1)
handleDatabaseError(24-60)
src/app/api/(auth)/get-users-by-role/route.ts (1)
src/lib/services/logger.ts (1)
logger(57-87)
src/app/api/(payout)/payout/create/route.ts (1)
src/lib/services/logger.ts (1)
logger(57-87)
🔇 Additional comments (15)
src/app/api/(auth)/get-users-by-role/route.ts (1)
53-57: LGTM on logger integration.Structured info log with role and count is appropriate.
src/app/api/(payout)/payout/update-status/[id]/route.ts (1)
25-29: LGTM on success/error logging.Non-blocking structured logs with action and metadata are consistent with the new strategy.
Also applies to: 36-41, 44-47
src/app/api/(auth)/register-role/route.ts (1)
38-44: LGTM on success log fields.Action, userId, entityType, and role metadata are well-structured.
src/app/api/(auth)/check-role/[user_id]/route.ts (2)
45-49: LGTM on not-found logging.Context includes action, userId, and entity type.
53-59: LGTM on success logging.Includes action, userId, and role metadata.
src/app/api/(auth)/get-user-by-id/route.ts (2)
112-116: LGTM on not-found logging.Includes action and userId context.
123-128: LGTM on success logging.Action and userId captured correctly.
src/lib/services/logger.ts (1)
50-54: Verify no client-side imports of server logger
Automated search found no occurrences of@/lib/services/loggeroutsidesrc/lib/services/logger.ts; manually confirm bundle aliases and ensure no browser code imports the server logger.src/app/api/(payout)/payout/delete/[id]/route.ts (1)
18-22: Good structured logging adoption (error/success/catch).Action/entity/metadata are consistent and useful. No behavior change introduced.
Also applies to: 29-33, 37-40
src/app/api/(helper)/files/upload-avatar/route.ts (1)
25-29: LGTM on structured logging coverage.Clear actions and helpful metadata; consistent with the new logger contract.
Also applies to: 46-51, 57-62, 66-69
src/app/api/(payout)/payout/create/route.ts (1)
25-31: Good use of error/info logs with actionable metadata.Consistent actions, entity types, and user context. No control-flow changes.
Also applies to: 38-43, 49-54, 58-61
src/app/api/(helper)/files/manage-files/route.ts (1)
71-75: LGTM on detailed per-file and summary logs.Helpful for debugging and audits.
Also applies to: 81-85, 92-95
src/app/api/(payout)/payout/update/[id]/route.ts (1)
21-25: Structured logging looks good.Action/entity/metadata are consistent; catch path covered.
Also applies to: 39-43, 47-50
src/app/api/(auth)/get-user-by-email/route.ts (1)
12-15: Good logging on invalid input and unexpected errors.These will aid support without changing responses.
Also applies to: 56-59
src/app/api/(auth)/get-user-role-by-id/route.ts (1)
11-135: Structured logging context looks solid.Thanks for weaving the centralized logger through every branch—each path now emits consistent
AUTH_GET_USER_ROLE_BY_IDevents with the right identifiers and metadata, and the catch block safely captures unexpected failures without disturbing the response flow. Looks good to me.
…ng; update tsconfig types
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/app/api/(helper)/files/manage-files/route.ts (3)
61-85: Harden uploads: validate type/size, ensure unique keys, and sanitize folder in-path.
- Validate file size and MIME to reduce abuse.
- Use UUID to avoid collisions vs Date.now().
- Sanitize or whitelist folder to avoid path surprises.
- for (const file of files) { - const safeName = file.name.replace(/[^a-zA-Z0-9._-]/g, "_"); - const path = `${payoutId}/${milestoneIdx}/${folder}/${Date.now()}-${safeName}`; + // Basic controls; consider centralizing these. + const MAX_BYTES = 10 * 1024 * 1024; // 10MB + const ALLOWED_MIME = new Set([ + "image/jpeg", + "image/png", + "application/pdf", + ]); + for (const file of files) { + const safeName = file.name.replace(/[^a-zA-Z0-9._-]/g, "_"); + if (file.size > MAX_BYTES || !ALLOWED_MIME.has(file.type)) { + const reason = + file.size > MAX_BYTES ? "file too large" : `mime not allowed (${file.type})`; + errors.push(`${file.name}: ${reason}`); + logger.error("Rejected file due to policy", null, { + action: "FILE_UPLOAD", + entityType: "FILE", + metadata: { payoutId, milestoneIdx, folder, fileName: file.name, size: file.size, type: file.type }, + }); + continue; + } + const uniqueKey = crypto.randomUUID(); + const folderSafe = /^[a-zA-Z0-9_-]+$/.test(folder) ? folder : "evidence"; + const path = `${payoutId}/${milestoneIdx}/${folderSafe}/${uniqueKey}-${safeName}`;
7-30: Enforce auth in service-role upload endpoint
The POST handler insrc/app/api/(helper)/files/manage-files/route.tsuses a Supabase service role with no session check. Add a guard (e.g. callsupabase.auth.getSession()or apply your existing auth middleware) to verify the requester’s identity and ensure they’re authorized for the specified payout/folder, returning 401/403 if unauthorized.
16-23: Restrict Supabase service role key to server environment
- Replace process.env.NEXT_PUBLIC_SUPABASE_SERVICE_ROLE_KEY with process.env.SUPABASE_SERVICE_ROLE_KEY in:
• src/app/api/(helper)/files/upload-avatar/route.ts (line 21)
• src/app/api/(helper)/files/manage-files/route.ts (line 17)- Rotate the exposed key and update your server-only .env; confirm no remaining NEXT_PUBLIC_SUPABASE_SERVICE_ROLE_KEY references.
🧹 Nitpick comments (7)
src/lib/services/logger.ts (1)
65-95: Centralize log action strings in an enum.
We now repeat literal action strings (AUTH_REGISTER, AVATAR_UPLOAD, PROFILE_PATCH, etc.) across multiple modules, which is easy to mistype and diverge. Please lift them into a shared enum (for example in this module) and reference the constants from callers so future additions stay consistent.+export enum LogAction { + AUTH_REGISTER = "AUTH_REGISTER", + AUTH_REGISTER_ROLE = "AUTH_REGISTER_ROLE", + AVATAR_UPLOAD = "AVATAR_UPLOAD", + FILE_UPLOAD = "FILE_UPLOAD", + PROFILE_PATCH = "PROFILE_PATCH", + // extend as additional actions are introduced +} + export const logger = { info: ( description: string, - ctx?: Omit<LogParams, "description" | "action"> & { action?: string }, + ctx?: Omit<LogParams, "description" | "action"> & { + action?: LogAction | string; + }, ) => { - const action = ctx?.action ?? "INFO"; + const action = ctx?.action ?? LogAction.AUTH_REGISTER; @@ error: ( description: string, error?: unknown, - ctx?: Omit<LogParams, "description" | "action"> & { action?: string }, + ctx?: Omit<LogParams, "description" | "action"> & { + action?: LogAction | string; + }, ) => { - const action = ctx?.action ?? "ERROR"; + const action = ctx?.action ?? LogAction.AUTH_REGISTER;As per coding guidelines.
src/app/api/(helper)/files/manage-files/route.ts (2)
33-36: Remove unreachable bucket check.
bucketis a hard-coded, truthy constant; this branch never runs.- if (!bucket) { - logger.error("Missing Supabase storage bucket", null, { - action: "FILE_UPLOAD", - entityType: "FILE", - }); - return NextResponse.json( - { success: false, message: "Missing Supabase storage bucket" }, - { status: 500 }, - ); - }
87-95: Centralize action/entity strings in an enum/const.Avoid magic strings like "FILE_UPLOAD" and "FILE" across files; define and import a shared enum/const for actions/entities to prevent typos and ease maintenance.
As per coding guidelines
src/app/api/(auth)/profile/route.ts (1)
84-89: Add minimal metadata for observability.Consider including which sections were updated (user, grantee, provider) to aid debugging, plus a request id if available.
- logger.info("Profile updated successfully", { + logger.info("Profile updated successfully", { action: "PROFILE_PATCH", userId: userId, entityType: "USER", + metadata: { + updated: { + user: !!user && Object.keys(user).length > 0, + grantee: !!grantee, + payoutProvider: !!grantProvider, + }, + }, });Also consider centralizing "PROFILE_PATCH" in an enum/const. As per coding guidelines
src/app/api/(auth)/check-role/[user_id]/route.ts (3)
53-59: Centralize action/entity strings in an enum/const.Prevent drift across routes: e.g., AUTH_CHECK_ROLE, USER.
As per coding guidelines
62-65: Include userId in error log for correlation.Keeps context consistent with other paths.
- logger.error("Error checking user role", error, { + logger.error("Error checking user role", error, { action: "AUTH_CHECK_ROLE", - entityType: "USER", + entityType: "USER", + userId: user_id, });
5-11: Fix handler params type; params is not a Promise.In Next.js route handlers,
paramsis a plain object; awaiting a non-Promise is harmless at runtime but the type is wrong.-export async function GET( - _request: Request, - { params }: { params: Promise<{ user_id: string }> }, -) { +export async function GET( + _request: Request, + { params }: { params: { user_id: string } }, +) { try { - const { user_id } = await params; + const { user_id } = params;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
src/app/api/(auth)/check-role/[user_id]/route.ts(3 hunks)src/app/api/(auth)/get-user-by-email/route.ts(3 hunks)src/app/api/(auth)/get-user-by-id/route.ts(3 hunks)src/app/api/(auth)/get-user-role-by-id/route.ts(4 hunks)src/app/api/(auth)/get-users-by-role/route.ts(3 hunks)src/app/api/(auth)/profile/route.ts(2 hunks)src/app/api/(auth)/register-role/route.ts(2 hunks)src/app/api/(auth)/register-user/route.ts(3 hunks)src/app/api/(auth)/verify-oauth-user/route.ts(3 hunks)src/app/api/(helper)/files/manage-files/route.ts(4 hunks)src/app/api/(helper)/files/upload-avatar/route.ts(3 hunks)src/app/api/(payout)/payout/create/route.ts(2 hunks)src/app/api/(payout)/payout/delete/[id]/route.ts(2 hunks)src/app/api/(payout)/payout/update-status/[id]/route.ts(2 hunks)src/app/api/(payout)/payout/update/[id]/route.ts(3 hunks)src/lib/services/logger.ts(1 hunks)tsconfig.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- src/app/api/(auth)/get-users-by-role/route.ts
- src/app/api/(auth)/get-user-by-email/route.ts
- src/app/api/(auth)/verify-oauth-user/route.ts
- src/app/api/(payout)/payout/delete/[id]/route.ts
- src/app/api/(payout)/payout/update-status/[id]/route.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/frontend-rule.mdc)
**/*.{js,jsx,ts,tsx}: Use early returns whenever possible to make the code more readable.
Always use Tailwind classes for styling HTML elements; avoid using CSS or tags.
Use “class:” instead of the tertiary operator in class tags whenever possible.
Use descriptive variable and function/const names. Also, event functions should be named with a “handle” prefix, like “handleClick” for onClick and “handleKeyDown” for onKeyDown.
Implement accessibility features on elements. For example, a tag should have a tabindex=“0”, aria-label, on:click, and on:keydown, and similar attributes.
Everything must be 100% responsive (using all the sizes of tailwind) and compatible with dark/light shadcn's mode.
Use consts instead of functions, for example, “const toggle = () =>”. Also, define a type if possible.
Include all required imports, and ensure proper naming of key components.
**/*.{js,jsx,ts,tsx}: enum/const object members should be written UPPERCASE_WITH_UNDERSCORE.
Gate flag-dependent code on a check that verifies the flag's values are valid and expected.
If a custom property for a person or event is at any point referenced in two or more files or two or more callsites in the same file, use an enum or const object, as above in feature flags.
Files:
src/app/api/(auth)/get-user-role-by-id/route.tssrc/app/api/(payout)/payout/create/route.tssrc/app/api/(helper)/files/manage-files/route.tssrc/app/api/(auth)/get-user-by-id/route.tssrc/app/api/(auth)/register-user/route.tssrc/app/api/(auth)/register-role/route.tssrc/app/api/(auth)/check-role/[user_id]/route.tssrc/app/api/(payout)/payout/update/[id]/route.tssrc/app/api/(helper)/files/upload-avatar/route.tssrc/app/api/(auth)/profile/route.tssrc/lib/services/logger.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/frontend-rule.mdc)
You can never use anys.
If using TypeScript, use an enum to store flag names.
Files:
src/app/api/(auth)/get-user-role-by-id/route.tssrc/app/api/(payout)/payout/create/route.tssrc/app/api/(helper)/files/manage-files/route.tssrc/app/api/(auth)/get-user-by-id/route.tssrc/app/api/(auth)/register-user/route.tssrc/app/api/(auth)/register-role/route.tssrc/app/api/(auth)/check-role/[user_id]/route.tssrc/app/api/(payout)/payout/update/[id]/route.tssrc/app/api/(helper)/files/upload-avatar/route.tssrc/app/api/(auth)/profile/route.tssrc/lib/services/logger.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/posthog-integration.mdc)
Never hallucinate an API key. Instead, always use the API key populated in the .env file.
Files:
src/app/api/(auth)/get-user-role-by-id/route.tssrc/app/api/(payout)/payout/create/route.tssrc/app/api/(helper)/files/manage-files/route.tssrc/app/api/(auth)/get-user-by-id/route.tssrc/app/api/(auth)/register-user/route.tssrc/app/api/(auth)/register-role/route.tssrc/app/api/(auth)/check-role/[user_id]/route.tssrc/app/api/(payout)/payout/update/[id]/route.tssrc/app/api/(helper)/files/upload-avatar/route.tssrc/app/api/(auth)/profile/route.tssrc/lib/services/logger.ts
🧠 Learnings (1)
📚 Learning: 2025-07-22T01:46:06.934Z
Learnt from: CR
PR: GrantChain/GrantFox#0
File: .cursor/rules/frontend-rule.mdc:0-0
Timestamp: 2025-07-22T01:46:06.934Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Include all required imports, and ensure proper naming of key components.
Applied to files:
tsconfig.json
🧬 Code graph analysis (11)
src/app/api/(auth)/get-user-role-by-id/route.ts (2)
src/lib/services/logger.ts (1)
logger(60-97)src/components/modules/auth/services/auth.service.ts (1)
getUserRoleById(120-137)
src/app/api/(payout)/payout/create/route.ts (1)
src/lib/services/logger.ts (1)
logger(60-97)
src/app/api/(helper)/files/manage-files/route.ts (1)
src/lib/services/logger.ts (1)
logger(60-97)
src/app/api/(auth)/get-user-by-id/route.ts (1)
src/lib/services/logger.ts (1)
logger(60-97)
src/app/api/(auth)/register-user/route.ts (2)
src/lib/services/logger.ts (1)
logger(60-97)src/lib/prisma.ts (1)
handleDatabaseError(24-60)
src/app/api/(auth)/register-role/route.ts (2)
src/lib/prisma.ts (1)
prisma(7-19)src/lib/services/logger.ts (1)
logger(60-97)
src/app/api/(auth)/check-role/[user_id]/route.ts (1)
src/lib/services/logger.ts (1)
logger(60-97)
src/app/api/(payout)/payout/update/[id]/route.ts (1)
src/lib/services/logger.ts (1)
logger(60-97)
src/app/api/(helper)/files/upload-avatar/route.ts (1)
src/lib/services/logger.ts (1)
logger(60-97)
src/app/api/(auth)/profile/route.ts (1)
src/lib/services/logger.ts (1)
logger(60-97)
src/lib/services/logger.ts (1)
src/lib/prisma.ts (1)
prisma(7-19)
🔇 Additional comments (31)
tsconfig.json (1)
24-25: Restoring Next ambient types looks good.
Thank you for making sure both Node and Next definitions stay available to the compiler.src/app/api/(helper)/files/upload-avatar/route.ts (1)
57-62: Good call on structured success logging.
Capturing the path and public URL in metadata will make root-cause analysis far easier without leaking sensitive values.src/app/api/(auth)/register-role/route.ts (1)
18-38: Nice transaction hardening.
Wrapping the role update together with the role-specific upserts inside a transaction prevents partial state and closes the data-integrity hole flagged earlier.src/app/api/(auth)/register-user/route.ts (1)
54-59: Appreciate the PII masking.
Masking the email before persisting the log keeps the audit trail useful without leaking user addresses.src/app/api/(payout)/payout/update/[id]/route.ts (4)
1-1: LGTM! Centralized logging import follows established pattern.The import follows the centralized logging pattern being implemented across all API routes in this PR.
21-25: LGTM! Structured error logging with appropriate context.The error logging includes appropriate metadata (payout_id) and follows the consistent action naming convention used across payout operations.
39-43: LGTM! Success logging provides audit trail.The info logging captures successful payout updates with appropriate metadata for audit purposes.
47-50: LGTM! Generic error handling with structured logging.The catch block properly logs errors with consistent action naming while preserving the original error response behavior.
src/app/api/(auth)/get-user-by-id/route.ts (5)
2-2: LGTM! Consistent logger import across auth routes.The logger import follows the established pattern being implemented across all auth-related API routes in this PR.
12-15: LGTM! Parameter validation with structured logging.The missing parameter validation includes appropriate error logging with consistent action naming.
112-116: LGTM! User not found logging with userId context.The error logging appropriately includes the user_id that was searched for, providing valuable debugging context.
123-127: LGTM! Success logging for audit trail.The success logging provides a clear audit trail for user data access with appropriate metadata.
131-134: LGTM! Generic error logging in catch block.The catch block properly logs errors while preserving the existing error handling behavior from
handleDatabaseError.src/app/api/(payout)/payout/create/route.ts (5)
1-1: LGTM! Consistent logger import pattern.The logger import follows the established centralized logging pattern being implemented across all payout routes.
25-30: LGTM! Comprehensive error logging with payload context.The error logging captures useful metadata including payload keys and the user who initiated the creation, which aids in debugging and auditing.
38-46: LGTM! Edge case logging for unexpected scenarios.Logging when Supabase returns no error but also no data helps identify potential integration issues that might otherwise go unnoticed.
53-58: LGTM! Success logging with payout ID for audit trail.The success logging captures the created payout_id, providing a valuable audit trail for tracking payout creation operations.
62-65: LGTM! Generic error handling maintains behavior.The catch block error logging maintains the original response behavior while adding structured logging for better observability.
src/app/api/(auth)/get-user-role-by-id/route.ts (7)
2-2: LGTM! Consistent logger integration.The logger import follows the established pattern across all auth routes in this centralized logging implementation.
12-15: LGTM! Parameter validation with structured logging.The missing user_id parameter validation includes appropriate error logging with consistent action naming pattern.
46-50: LGTM! User not found logging for EMPTY role path.The error logging appropriately captures when a user is not found during the EMPTY role processing path.
57-61: LGTM! Success logging for EMPTY role processing.The success logging provides clear audit trail for user details fetched when role is EMPTY.
110-115: LGTM! Role-specific data not found logging.The error logging includes the specific role in metadata, which helps distinguish between different types of user data lookup failures.
122-127: LGTM! Success logging with role context.The success logging captures both the user_id and role, providing comprehensive audit information for role-based data access.
131-134: LGTM! Comprehensive error logging in catch block.The catch block properly logs errors while maintaining the existing error handling behavior and response structure.
src/app/api/(helper)/files/manage-files/route.ts (1)
102-105: LGTM: catch logging is correct and non-blocking.Structured error with context, without affecting the response path.
src/app/api/(auth)/profile/route.ts (2)
4-4: LGTM: using centralized logger.Import looks correct and server-only.
92-95: LGTM: error logging with context.Preserves existing error handling and adds structured logs.
src/app/api/(auth)/check-role/[user_id]/route.ts (3)
2-2: LGTM: centralized logger import.
13-16: LGTM: validates and logs invalid param.Consistent action/entity usage.
45-49: LGTM: not-found path logs with user context.
Summary
logger.tsleveraging Prisma for persistent system logs.console.errorcalls in server-side handlers with a standardized logger, while preserving response formats and app flow.Implementation
Centralized Logger Service
InputJsonValue/JsonNull)action,userId,entityType,metadataAPI Route Coverage
registerverify OAuthregister roleget users by roleget user by emailget user by idget user role by idcheck rolePATCH /api/(auth)/profilecreate,update,update-status,deletemanage-files,upload-avatarAll existing response shapes and status codes are preserved.
Validations
prisma.tshelpers.Testing
INFOandERRORlogs during development.system_logtable) entries are correctly created with:actionuserIdentityTypemetadataNotes / Future Work
Summary by CodeRabbit
Chores
Refactor