Skip to content

Conversation

@FabianSanchezD
Copy link

@FabianSanchezD FabianSanchezD commented Sep 29, 2025

CLR-S (2)

Pull Request | GrantChain

1. Issue Link


2. Brief Description of the Issue

Implemented the API routes for comments: added create (POST), find by payout (GET), update (PATCH) and delete (DELETE).

These work in a way so that the comments can only be edited or deleted by their creator. Also, comments have a user_id and payout_id.


3. Type of Change

Mark with an x all the checkboxes that apply (like [x]).

  • 📝 Documentation (updates to README, docs, or comments)
  • 🐛 Bug fix (non-breaking change which fixes an issue)
  • 👌 Enhancement (non-breaking change which adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to change)

4. Changes Made

Implemented comment API Routes: CRUD functionality.
Added proper error handling.
When doing find-by-payout, comments are ordered chronologically.


5. Evidence Before Solution

No UI Change.


6. Evidence After Solution

No UI Change.


7. Important Notes

Any feedback/suggestion is greatly appreciated! Thanks for reviewing this PR.

If you don't use this template, you'd be ignored

Summary by CodeRabbit

  • New Features
    • Post comments on payouts with input validation and success/error responses.
    • View comments for a payout, including commenter details and chronological ordering.
    • Edit your own comments (validated input, auth checks, detailed error statuses).
    • Delete your own comments with ownership verification and clear success/error responses.
    • Ownership and access checks ensure only authors can modify or remove their comments, with appropriate HTTP status feedback.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 29, 2025

Walkthrough

Adds four Comment API routes: POST /api/comment/create, GET /api/comment/find-by-payout/[payout_id], PATCH /api/comment/update/[id], and DELETE /api/comment/delete/[id]. Each route validates input, checks existence/ownership (User, Payout), performs DB operations, returns structured JSON, and centralizes database error mapping.

Changes

Cohort / File(s) Summary
Comment CRUD API routes
src/app/api/(comment)/comment/create/route.ts, src/app/api/(comment)/comment/find-by-payout/[payout_id]/route.ts, src/app/api/(comment)/comment/update/[id]/route.ts, src/app/api/(comment)/comment/delete/[id]/route.ts
Added POST create with schema validation, user active check, payout existence check, and 201 response; GET by payout with payout validation, chronological ordering, and user fields included; PATCH update with schema validation, auth/session check, ownership verification, and updated comment response; DELETE with id/user validation, ownership verification, and deletion. All routes use centralized DB error handling and return appropriate HTTP statuses (201/200/400/401/403/404).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Client
  participant API
  participant DB
  rect rgba(200,230,255,0.25)
    note over Client,API: Create Comment (POST /api/comment/create)
    Client->>API: POST { message, user_id, payout_id }
    API->>DB: SELECT user WHERE id=user_id
    DB-->>API: user | 404 if missing
    API->>DB: SELECT payout WHERE id=payout_id
    DB-->>API: payout | 404 if missing
    API->>DB: INSERT comment (link user+payout)
    DB-->>API: comment (with user fields)
    API-->>Client: 201 { success, comment }
  end

  rect rgba(220,255,220,0.18)
    note over Client,API: Get by Payout (GET /comment/find-by-payout/:payout_id)
    Client->>API: GET payout_id
    API->>DB: SELECT payout WHERE id=payout_id
    DB-->>API: payout | 404 if missing
    API->>DB: SELECT comments WHERE payout_id ORDER BY created_at ASC INCLUDE user
    DB-->>API: comments[]
    API-->>Client: 200 { success, comments, count }
  end

  rect rgba(255,245,200,0.18)
    note over Client,API: Update Comment (PATCH /comment/update/:id)
    Client->>API: PATCH { id, user_id, message }
    API->>DB: SELECT comment WHERE id
    DB-->>API: comment | 404 if missing
    API->>DB: Verify owner (comment.user_id == user_id) -> 403 if not
    API->>DB: UPDATE comment.message
    DB-->>API: updated comment (with user fields)
    API-->>Client: 200 { success, comment }
  end

  rect rgba(255,225,225,0.18)
    note over Client,API: Delete Comment (DELETE /comment/delete/:id)
    Client->>API: DELETE { id, user_id }
    API->>DB: SELECT comment WHERE id
    DB-->>API: comment | 404 if missing
    API->>DB: Verify owner -> 403 if not
    API->>DB: DELETE comment
    DB-->>API: OK
    API-->>Client: 200 { success, message }
  end

  alt DB error or invalid input
    API-->>Client: 400/4xx/5xx with mapped error via handleDatabaseError
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • coxmars

Poem

I thump my paws and tap the keys,
Four hopping routes now join the trees.
Create, fetch, update, delete in line,
Ownership checked, responses fine.
A carrot clap — our comments shine! 🥕🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Title Check ✅ Passed The title “feat: implement comment API routes” succinctly captures the primary enhancement introduced by this pull request, namely the addition of full CRUD comment endpoints, and it aligns directly with the changeset without extraneous detail.
Linked Issues Check ✅ Passed This PR fully implements the objectives of issue #122 by adding the four required API route files under src/app/api/(comment)/comment/, utilizing Prisma with consistent error handling, enforcing authorization so only comment authors may edit or delete, validating payout existence before creation, and returning comments in chronological order.
Out of Scope Changes Check ✅ Passed All code modifications are confined to the comment API routes and related logic as specified in the linked issue, with no unrelated files or features altered.
Description Check ✅ Passed The description adheres to the repository’s template by including all required sections—issue link, brief issue context, change type, detailed list of changes, evidence before and after, and important notes—each populated with appropriate content.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Nitpick comments (12)
src/app/api/(comment)/comment/find-by-payout/[payout_id]/route.ts (3)

17-22: Tighten param validation; drop string-literal checks.

"undefined"/"null" string checks aren’t useful for route params. Validate presence and shape (uuid/cuid) and return 400 on failure.

Example:

-    if (!payout_id || payout_id === "undefined" || payout_id === "null") {
+    if (!payout_id?.trim()) {
       return NextResponse.json(
         { error: "Invalid payout_id parameter" },
         { status: 400 }
       );
     }

Optionally add Zod validation (uuid/cuid) for payout_id.


8-10: Make response freshness explicit (avoid unintended caching).

If comments must always be fresh, mark this route dynamic (or set Cache-Control: no-store). Next.js 15 changed GET Route Handler caching defaults; be explicit.

 import { handleDatabaseError, prisma } from "@/lib/prisma";
 import { NextResponse } from "next/server";
 
+export const dynamic = "force-dynamic";

Based on learnings.


37-51: DB indexes for query path (payout_id, created_at).

This query filters by payout_id and orders by created_at. Ensure a composite index (payout_id, created_at) exists to avoid scans on large tables.

src/app/api/(comment)/comment/update/[id]/route.ts (3)

19-21: Return 400 on invalid JSON; don’t map it as a DB error.

If request.json() fails, it hits the catch and becomes a 500 “Database error”. Handle parse errors explicitly.

-    const body = await request.json();
+    let body: unknown;
+    try {
+      body = await request.json();
+    } catch {
+      return NextResponse.json({ error: "Invalid JSON body" }, { status: 400 });
+    }

13-19: Validate route param shape (uuid/cuid) before querying.

Add a quick Zod check for id to fail fast with 400 instead of a DB round trip.


31-39: Minor: prefer const + destructuring.
s/user_id isn’t reassigned; use const { user_id } = body as { user_id?: string };.

src/app/api/(comment)/comment/delete/[id]/route.ts (3)

18-19: Return 400 on invalid JSON; don’t treat as DB error.

Guard request.json() with a try/catch and return 400 on parse failure.

-    const body = await request.json();
+    let body: unknown;
+    try {
+      body = await request.json();
+    } catch {
+      return NextResponse.json({ error: "Invalid JSON body" }, { status: 400 });
+    }

20-25: Simplify/strengthen ID validation.

Drop "undefined"/"null" string checks; validate presence and format (uuid/cuid) and return 400 if invalid.


12-15: Note: DELETE bodies can be brittle across clients.

Some clients/proxies strip DELETE bodies. If feasible, derive user from auth and avoid needing a request body here.

src/app/api/(comment)/comment/create/route.ts (3)

13-21: Return 400 on invalid JSON; don’t let parse errors become 500.

-    const body = await request.json();
-    const parsed = commentCreateSchema.safeParse(body);
+    let body: unknown;
+    try {
+      body = await request.json();
+    } catch {
+      return NextResponse.json({ error: "Invalid JSON body" }, { status: 400 });
+    }
+    const parsed = commentCreateSchema.safeParse(body);

46-53: Minor: avoid selecting unused fields.

status and created_by aren’t used. Drop them to reduce payload/compute.

-      select: { 
-        payout_id: true,
-        status: true,
-        created_by: true
-      }
+      select: { payout_id: true }

63-79: Operational: add/confirm indexes.

Comment creation and listing will benefit from indexes on (payout_id, created_at) and (user_id, created_at). Add a migration if not present.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4c89fa2 and 8086e1f.

📒 Files selected for processing (4)
  • src/app/api/(comment)/comment/create/route.ts (1 hunks)
  • src/app/api/(comment)/comment/delete/[id]/route.ts (1 hunks)
  • src/app/api/(comment)/comment/find-by-payout/[payout_id]/route.ts (1 hunks)
  • src/app/api/(comment)/comment/update/[id]/route.ts (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/(comment)/comment/find-by-payout/[payout_id]/route.ts
  • src/app/api/(comment)/comment/create/route.ts
  • src/app/api/(comment)/comment/delete/[id]/route.ts
  • src/app/api/(comment)/comment/update/[id]/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/(comment)/comment/find-by-payout/[payout_id]/route.ts
  • src/app/api/(comment)/comment/create/route.ts
  • src/app/api/(comment)/comment/delete/[id]/route.ts
  • src/app/api/(comment)/comment/update/[id]/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/(comment)/comment/find-by-payout/[payout_id]/route.ts
  • src/app/api/(comment)/comment/create/route.ts
  • src/app/api/(comment)/comment/delete/[id]/route.ts
  • src/app/api/(comment)/comment/update/[id]/route.ts
🧬 Code graph analysis (4)
src/app/api/(comment)/comment/find-by-payout/[payout_id]/route.ts (2)
src/lib/prisma.ts (2)
  • prisma (7-19)
  • handleDatabaseError (24-60)
src/app/api/(payout)/payout/find-one/[id]/route.ts (1)
  • GET (4-29)
src/app/api/(comment)/comment/create/route.ts (2)
src/components/modules/comment/schema/comment.schema.ts (1)
  • commentCreateSchema (3-14)
src/lib/prisma.ts (2)
  • prisma (7-19)
  • handleDatabaseError (24-60)
src/app/api/(comment)/comment/delete/[id]/route.ts (1)
src/lib/prisma.ts (2)
  • prisma (7-19)
  • handleDatabaseError (24-60)
src/app/api/(comment)/comment/update/[id]/route.ts (2)
src/components/modules/comment/schema/comment.schema.ts (1)
  • commentUpdateSchema (16-21)
src/lib/prisma.ts (2)
  • prisma (7-19)
  • handleDatabaseError (24-60)

@FabianSanchezD
Copy link
Author

Let me know if you consider the changes suggested by coderabbit are needed!

@FabianSanchezD
Copy link
Author

Fixed 👍

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/app/api/(comment)/comment/update/[id]/route.ts (1)

33-41: Past critical issue resolved: now uses server-side session authentication.

This implementation correctly addresses the IDOR vulnerability flagged in the previous review. The code now derives the authenticated user from the server-side Supabase session instead of trusting body.user_id, eliminating the spoofing risk.


Note: Consider migrating from deprecated auth package.

The learnings indicate @supabase/auth-helpers-nextjs is deprecated in favor of @supabase/ssr. While the current implementation follows established project patterns correctly, consider planning a codebase-wide migration to the recommended package.

Based on learnings

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8086e1f and 19f412b.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (4)
  • src/app/api/(comment)/comment/create/route.ts (1 hunks)
  • src/app/api/(comment)/comment/delete/[id]/route.ts (1 hunks)
  • src/app/api/(comment)/comment/find-by-payout/[payout_id]/route.ts (1 hunks)
  • src/app/api/(comment)/comment/update/[id]/route.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/app/api/(comment)/comment/find-by-payout/[payout_id]/route.ts
  • src/app/api/(comment)/comment/delete/[id]/route.ts
  • src/app/api/(comment)/comment/create/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/(comment)/comment/update/[id]/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/(comment)/comment/update/[id]/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/(comment)/comment/update/[id]/route.ts
🧬 Code graph analysis (1)
src/app/api/(comment)/comment/update/[id]/route.ts (2)
src/components/modules/comment/schema/comment.schema.ts (1)
  • commentUpdateSchema (16-21)
src/lib/prisma.ts (2)
  • prisma (7-19)
  • handleDatabaseError (24-60)
🔇 Additional comments (5)
src/app/api/(comment)/comment/update/[id]/route.ts (5)

19-31: LGTM!

Request parsing and validation logic is correct. Using safeParse with detailed error responses provides good developer experience.


43-55: LGTM!

Comment existence check is properly implemented with efficient field selection and appropriate 404 response.


57-60: LGTM!

Authorization check correctly compares the authenticated user's ID from the session against the comment owner's ID, completing the fix for the IDOR vulnerability. The 403 status is appropriate for authorization failures.


62-76: LGTM!

Update operation correctly modifies only the message field and includes the user relation with appropriate field selection for API response consistency.


78-86: LGTM!

Response structure and error handling follow established patterns. Proper delegation to handleDatabaseError ensures consistent error responses across the API.

Comment on lines +1 to +8
/**
* PATCH /api/comment/update/[id]
* Update an existing comment (only author can edit)
*
* Params: { id: string }
* Body: { message: string, user_id: string }
* Returns: { success: boolean, comment: Comment, message: string }
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Update JSDoc to reflect actual request body.

The JSDoc states Body: { message: string, user_id: string } but user_id is no longer accepted in the request body—it's now derived from the authenticated session. This outdated documentation could mislead API consumers.

Apply this diff to correct the documentation:

 /**
  * PATCH /api/comment/update/[id]
  * Update an existing comment (only author can edit)
  *
  * Params: { id: string }
- * Body: { message: string, user_id: string }
+ * Body: { message: string }
  * Returns: { success: boolean, comment: Comment, message: string }
  */
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/**
* PATCH /api/comment/update/[id]
* Update an existing comment (only author can edit)
*
* Params: { id: string }
* Body: { message: string, user_id: string }
* Returns: { success: boolean, comment: Comment, message: string }
*/
/**
* PATCH /api/comment/update/[id]
* Update an existing comment (only author can edit)
*
* Params: { id: string }
* Body: { message: string }
* Returns: { success: boolean, comment: Comment, message: string }
*/
🤖 Prompt for AI Agents
In src/app/api/(comment)/comment/update/[id]/route.ts around lines 1 to 8, the
JSDoc incorrectly lists the request body as { message: string, user_id: string }
even though user_id is now derived from the authenticated session; update the
comment to remove user_id from the Body section (e.g., Body: { message: string
}) and add a short note that the user is taken from the authenticated session so
consumers know not to send user_id.

@FabianSanchezD
Copy link
Author

hey @coxmars, could you review this?

@JoelVR17 JoelVR17 assigned JoelVR17 and unassigned FabianSanchezD Nov 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEAT]: Implement Comment System API Routes

2 participants