E54B fix: zod errors on /payments and /payments/[uid] api by shaun-ak · Pull Request #24316 · calcom/cal.com · GitHub
[go: up one dir, main page]

Skip to content

Conversation

shaun-ak
Copy link
Contributor
@shaun-ak shaun-ak commented Oct 7, 2025

What does this PR do?

Visual Demo (For contributors especially)

Before-

Before.mp4

After-

After.mp4

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

@shaun-ak shaun-ak requested a review from a team as a code owner October 7, 2025 07:39
Copy link
vercel bot commented Oct 7, 2025

@shaun-ak is attempting to deploy a commit to the cal Team on Vercel.

A member of the Team first needs to authorize it.

@graphite-app graphite-app bot added the community Created by Linear-GitHub Sync label Oct 7, 2025
@graphite-app graphite-app bot requested a review from a team October 7, 2025 07:39
@github-actions github-actions bot added api area: API, enterprise API, access token, OAuth Medium priority Created by Linear-GitHub Sync 🐛 bug Something isn't working labels Oct 7, 2025
Copy link
Contributor
coderabbitai bot commented Oct 7, 2025

Walkthrough

This PR exposes the uid field in the public payment validation schema, refactors payments API handlers to use linear async/await flows with explicit 404/401 responses, adds OpenAPI schemas Payment and ArrayOfPayments and updates Booking.payment to reference ArrayOfPayments, and introduces unit tests for GET /api/payments and GET /api/payments/[id] covering success, not-found, unauthorized, and validation scenarios.

Possibly related PRs

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Title Check ✅ Passed The title clearly indicates that this PR fixes Zod validation errors on the /payments and /payments/[uid] API endpoints by adjusting schemas to include the uid field and resolving parse issues. It concisely summarizes the main purpose of the changeset without unnecessary detail. Reviewers can quickly understand the primary fix from the title alone.
Linked Issues Check ✅ Passed The changes fulfill the acceptance criteria from issues #24287 and CAL-6525 by updating the public payment schema to expose uid, aligning both list and by-id endpoints to return validated payloads without Zod errors, implementing 404 and 401 error handling, adding comprehensive tests for success and failure scenarios, and updating the API documentation with new Payment and ArrayOfPayments schemas. Both endpoints now meet the expected types and error contracts, and contract tests cover list, by-id, invalid-id, and schema shape requirements. Documentation updates reflect the final response schema as specified.
Out of Scope Changes Check ✅ Passed All modifications—including schema adjustments, handler refactoring, error handling enhancements, new tests, and documentation revisions—directly address the linked issues’ objectives of fixing Zod validation errors on the /payments endpoints. There are no unrelated or extraneous changes outside the defined scope of resolving schema mismatches, improving validation, and ensuring robust error handling.
Description Check ✅ Passed The description references the specific GitHub and Linear issue numbers for the Zod errors, includes before/after visual demos, and lists verification tasks, all of which relate directly to the schema and API changes in this PR. It stays on topic by explaining why the changes were made and what issues are addressed without diverging into unrelated details.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 62b4924 and 99e1cfa.

📒 Files selected for processing (1)
  • apps/api/v1/pages/api/payments/index.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

**/*.ts: For Prisma queries, only select data you need; never use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • apps/api/v1/pages/api/payments/index.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js .utc() in hot paths like loops

Files:

  • apps/api/v1/pages/api/payments/index.ts
**/*.{ts,tsx,js,jsx}

⚙️ CodeRabbit configuration file

Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.

Files:

  • apps/api/v1/pages/api/payments/index.ts
🧠 Learnings (1)
📚 Learning: 2025-07-28T11:50:23.946Z
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: Applies to **/*.ts : For Prisma queries, only select data you need; never use `include`, always use `select`

Applied to files:

  • apps/api/v1/pages/api/payments/index.ts
🧬 Code graph analysis (1)
apps/api/v1/pages/api/payments/index.ts (1)
apps/api/v1/lib/validations/payment.ts (1)
  • schemaPaymentPublic (3-13)
🔇 Additional comments (3)
apps/api/v1/pages/api/payments/index.ts (3)

16-37: LGTM! OpenAPI documentation properly updated.

The documentation improvements include a properly documented apiKey parameter, a response schema referencing ArrayOfPayments, and clearer error descriptions. These changes align with the PR objectives to provide robust API documentation.


44-46: Excellent addition of explicit user validation.

The null check for userWithBookings provides clear error handling with an appropriate 404 response. This aligns with the PR objectives to deliver explicit error messages for non-200 responses.


52-52: Good fix: Correctly returns 200 for all valid cases.

The change to always return status 200 with the payments array (even when empty) resolves the unreachable 404 branch issue flagged in previous reviews. This approach aligns with REST API best practices for collection endpoints and matches the test expectations.


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.

@dosubot dosubot bot added the payments Created by Linear-GitHub Sync label Oct 7, 2025
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)
apps/api/v1/pages/api/payments/[id].ts (1)

51-51: Prefer explicit select to fetch only required fields.

The payment query should explicitly use select to retrieve only the fields defined in schemaPaymentPublic, avoiding over-fetching.

As per coding guidelines

Apply this diff:

-    const data = await prisma.payment.findUnique({ where: { id: safeQuery.data.id } });
+    const data = await prisma.payment.findUnique({ 
+      where: { id: safeQuery.data.id },
+      select: {
+        id: true,
+        uid: true,
+        amount: true,
+        success: true,
+        refunded: true,
+        fee: true,
+        paymentOption: true,
+        currency: true,
+        bookingId: true,
+      }
+    });
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 3fc8c6c and c5cb2df.

📒 Files selected for processing (2)
  • apps/api/v1/lib/validations/payment.ts (1 hunks)
  • apps/api/v1/pages/api/payments/[id].ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

**/*.ts: For Prisma queries, only select data you need; never use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • apps/api/v1/lib/validations/payment.ts
  • apps/api/v1/pages/api/payments/[id].ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js .utc() in hot paths like loops

Files:

  • apps/api/v1/lib/validations/payment.ts
  • apps/api/v1/pages/api/payments/[id].ts
**/*.{ts,tsx,js,jsx}

⚙️ CodeRabbit configuration file

Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.

Files:

  • apps/api/v1/lib/validations/payment.ts
  • apps/api/v1/pages/api/payments/[id].ts
🧬 Code graph analysis (1)
apps/api/v1/pages/api/payments/[id].ts (1)
apps/api/v1/lib/validations/payment.ts (1)
  • schemaPaymentPublic (3-13)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Install dependencies / Yarn install & cache
  • GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (5)
apps/api/v1/lib/validations/payment.ts (1)

5-5: LGTM! uid field now exposed in public schema.

This addition ensures uid is included in the public payment data, directly addressing the PR objective to include paymentUid when available.

apps/api/v1/pages/api/payments/[id].ts (4)

53-57: LGTM! Proper 404 handling before parsing.

This defensive check ensures a 404 is returned before attempting to parse null/undefined data, directly addressing the PR objective to provide clear error messaging for non-200 paths.


59-59: LGTM! Zod validation applied correctly.

The payment data is now validated with schemaPaymentPublic.parse(), which will catch schema mismatches and throw clear Zod errors if the data doesn't conform to the expected shape. This directly addresses the PR objective to fix Zod validation errors.


61-63: LGTM! Authorization check is correct.

The authorization logic correctly verifies that the payment's booking belongs to the requesting user before returning the data.


65-65: LGTM! Standardized 200 response.

The response shape { payment } is clean and consistent, returning the validated payment object on success.

Copy link
Contributor
@Pallava-Joshi Pallava-Joshi left a comment

Choose a reason for hiding this comment

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

please address code rabbit suggestions.

@Pallava-Joshi Pallava-Joshi marked this pull request as draft October 7, 2025 07:59
@shaun-ak shaun-ak marked this pull request as ready for review October 7, 2025 10:18
@shaun-ak shaun-ak requested a review from Pallava-Joshi October 7, 2025 10:19
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

♻️ Duplicate comments (1)
apps/api/v1/pages/api/payments/[id].ts (1)

47-50: Still fetching unnecessary booking fields.

The code was updated from include to select, but select: { bookings: true } still fetches all booking columns. Line 61 only needs booking IDs for authorization. As noted in the previous review, only request the id field.

Apply this diff to fetch only booking IDs:

     const userWithBookings = await prisma.user.findUnique({
       where: { id: userId },
-      select: { bookings: true },
+      select: { 
+        bookings: { 
+          select: { id: true } 
+        } 
+      },
     });
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between c5cb2df and e747a1e.

📒 Files selected for processing (1)
  • apps/api/v1/pages/api/payments/[id].ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

**/*.ts: For Prisma queries, only select data you need; never use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • apps/api/v1/pages/api/payments/[id].ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js .utc() in hot paths like loops

Files:

  • apps/api/v1/pages/api/payments/[id].ts
**/*.{ts,tsx,js,jsx}

⚙️ CodeRabbit configuration file

Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.

Files:

  • apps/api/v1/pages/api/payments/[id].ts
🧬 Code graph analysis (1)
apps/api/v1/pages/api/payments/[id].ts (1)
apps/api/v1/lib/validations/payment.ts (1)
  • schemaPaymentPublic (3-13)
🔇 Additional comments (1)
apps/api/v1/pages/api/payments/[id].ts (1)

51-65: Good refactoring that addresses the PR objectives.

The changes successfully:

  • Add explicit 404 handling when payment data is not found.
  • Parse payment data with Zod to prevent schema mismatches (fixing the Zod errors mentioned in issue #24287).
  • Introduce authorization to verify the payment belongs to one of the user's bookings.
  • Return the properly typed payment including the uid field per the updated schema.

The async/await flow is clearer than the previous promise chain.

Minor note: Error responses return { message: string }, which may not strictly conform to the PaymentResponse type annotation on line 43. While this is common in API routes, verify that error response typing aligns with your API contracts.

@pull-request-size pull-request-size bot added size/XL and removed size/M labels Oct 7, 2025
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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/api/v1/pages/api/payments/index.ts (1)

39-43: Use select instead of include for Prisma queries.

Line 41 uses include: { bookings: true }, which fetches all booking fields. According to the coding guidelines, only the booking IDs are needed for filtering payments.

As per coding guidelines

Apply this diff to fetch only the necessary fields:

   const userWithBookings = await prisma.user.findUnique({
     where: { id: userId },
-    include: { bookings: true },
+    select: { 
+      bookings: { 
+        select: { id: true } 
+      } 
+    },
   });
♻️ Duplicate comments (1)
apps/api/v1/pages/api/payments/[id].ts (1)

54-57: Incomplete fix: still selecting all booking fields.

While the code was updated from include to select (addressing a previous review comment), Line 56 still selects all booking fields with select: { bookings: true }. The authorization check on Line 68 only needs the booking IDs, not the full booking objects.

As per coding guidelines

Apply this diff to select only the necessary fields:

   const userWithBookings = await prisma.user.findUnique({
     where: { id: userId },
-    select: { bookings: true },
+    select: { 
+      bookings: { 
+        select: { id: true } 
+      } 
+    },
   });
🧹 Nitpick comments (1)
apps/api/v1/pages/api/payments/[id].ts (1)

68-70: Optional: simplify redundant optional chaining.

Line 68 checks !userWithBookings || !userWithBookings?.bookings.map(...). The optional chaining userWithBookings?. is redundant since the first part of the condition already checks for !userWithBookings. If that's true, the second part won't execute due to short-circuit evaluation.

Consider this cleaner version:

-  if (!userWithBookings || !userWithBookings?.bookings.map((b) => b.id).includes(payment.bookingId)) {
+  if (!userWithBookings || !userWithBookings.bookings.map((b) => b.id).includes(payment.bookingId)) {
     return res.status(401).json({ message: "Unauthorized" });
   }
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between e747a1e and 3a03311.

📒 Files selected for processing (5)
  • apps/api/v1/pages/api/docs.ts (2 hunks)
  • apps/api/v1/pages/api/payments/[id].ts (2 hunks)
  • apps/api/v1/pages/api/payments/index.ts (2 hunks)
  • apps/api/v1/test/lib/payments/[id]/_get.test.ts (1 hunks)
  • apps/api/v1/test/lib/payments/_get.test.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

**/*.ts: For Prisma queries, only select data you need; never use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • apps/api/v1/pages/api/payments/index.ts
  • apps/api/v1/pages/api/payments/[id].ts
  • apps/api/v1/test/lib/payments/[id]/_get.test.ts
  • apps/api/v1/test/lib/payments/_get.test.ts
  • apps/api/v1/pages/api/docs.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js .utc() in hot paths like loops

Files:

  • apps/api/v1/pages/api/payments/index.ts
  • apps/api/v1/pages/api/payments/[id].ts
  • apps/api/v1/test/lib/payments/[id]/_get.test.ts
  • apps/api/v1/test/lib/payments/_get.test.ts
  • apps/api/v1/pages/api/docs.ts
**/*.{ts,tsx,js,jsx}

⚙️ CodeRabbit configuration file

Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.

Files:

  • apps/api/v1/pages/api/payments/index.ts
  • apps/api/v1/pages/api/payments/[id].ts
  • apps/api/v1/test/lib/payments/[id]/_get.test.ts
  • apps/api/v1/test/lib/payments/_get.test.ts
  • apps/api/v1/pages/api/docs.ts
🧬 Code graph analysis (2)
apps/api/v1/pages/api/payments/[id].ts (1)
apps/api/v1/lib/validations/payment.ts (1)
  • schemaPaymentPublic (3-13)
apps/api/v1/test/lib/payments/[id]/_get.test.ts (1)
apps/api/v1/pages/api/payments/[id].ts (1)
  • paymentById (48-74)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Install dependencies / Yarn install & cache
  • GitHub Check: Detect changes
🔇 Additional comments (8)
apps/api/v1/pages/api/docs.ts (2)

36-102: Well-structured OpenAPI schemas for Payment and ArrayOfPayments.

The Payment schema comprehensively defines all payment fields with proper types, examples, and descriptions. The inclusion of uid in the required fields aligns with the PR objective to expose paymentUid for abandoned-cart recovery. The ArrayOfPayments schema correctly references the Payment schema for consistency.


204-206: LGTM on Booking.payment schema update.

Updating the Booking.payment field to reference the new ArrayOfPayments schema ensures consistency across the API documentation and aligns with the refactored payment response structure.

apps/api/v1/pages/api/payments/index.ts (1)

16-37: LGTM on Swagger documentation updates.

The updated parameters and response schemas correctly document the API endpoint with the new ArrayOfPayments schema reference.

apps/api/v1/pages/api/payments/[id].ts (2)

36-46: LGTM on Swagger documentation updates.

The updated response schema correctly references the Payment schema and provides clear descriptions for different response codes.


58-72: LGTM on linear async/await flow with explicit error handling.

The refactored logic properly handles the 404 case before parsing, performs authorization checks, and returns structured responses. This addresses the PR objective to provide clear error messaging for non-200 responses.

apps/api/v1/test/lib/payments/[id]/_get.test.ts (2)

1-247: Comprehensive test coverage for GET /api/payments/[id].

The test suite thoroughly covers all critical paths:

  • Success path with payment object including uid field
  • 404 when payment not found with specific error message
  • 401 when user lacks authorization
  • Invalid query parameters handling
  • UID field presence validation
  • Non-GET method handling

This aligns well with the PR objectives to add contract tests for happy and failure paths, including uid exposure and typed error responses.


173-188: Verify actual middleware status codes
Add tests to assert that invalid queries produce a 400 (via withValidation) and unsupported methods produce a 405 (via withMiddleware("HTTP_GET")) rather than defaulting to 200.

apps/api/v1/test/lib/payments/_get.test.ts (1)

1-183: Comprehensive test coverage for GET /api/payments.

The test suite covers key scenarios:

  • Success path with payments array including all fields and uid
  • Empty array when no payments exist (Lines 111-133)
  • Error handling when user not found
  • UID field presence validation

The test on Lines 111-133 expects a 200 status with an empty payments array, which aligns with standard REST API practices. This behavior should be consistent with the implementation in apps/api/v1/pages/api/payments/index.ts (see related review comment on that file about the unreachable 404 path).

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: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
apps/api/v1/pages/api/payments/index.ts (2)

38-41: Replace include with select to align with coding guidelines.

Line 40 uses include: { bookings: true }, which violates the project's coding guideline: "For Prisma queries, only select data you need; never use include, always use select." Since only booking IDs are needed (line 44), this unnecessarily loads all booking fields, impacting performance and potentially exposing sensitive data.

As per coding guidelines.

Apply this diff to use select instead:

-  const userWithBookings = await prisma.user.findUnique({
-    where: { id: userId },
-    include: { bookings: true },
-  });
+  const userWithBookings = await prisma.user.findUnique({
+    where: { id: userId },
+    select: { 
+      bookings: { 
+        select: { id: true } 
+      } 
+    },
+  });

42-42: Return explicit 404 response instead of throwing generic Error.

Line 42 throws a generic Error when the user is not found. While middleware might catch this, it's clearer and more consistent with REST API patterns to return an explicit 404 response with a proper error object.

Apply this diff to return a proper 404 response:

-  if (!userWithBookings) throw new Error("No user found");
+  if (!userWithBookings) {
+    return res.status(404).json({ message: "User not found" });
+  }
🧹 Nitpick comments (1)
apps/api/v1/pages/api/payments/index.ts (1)

45-46: Consider selecting only public payment fields for better performance.

Line 45 fetches all payment fields before filtering through schemaPaymentPublic.parse() on line 46. While the parse ensures only public fields are returned in the response, selecting only the needed fields at the database level would improve query performance.

Consider applying this diff to explicitly select only public fields:

-  const data = await prisma.payment.findMany({ where: { bookingId: { in: bookingIds } } });
+  const data = await prisma.payment.findMany({ 
+    where: { bookingId: { in: bookingIds } },
+    select: {
+      id: true,
+      uid: true,
+      bookingId: true,
+      amount: true,
+      fee: true,
+      currency: true,
+      success: true,
+      refunded: true,
+      data: true,
+      externalId: true,
+      // Add other fields exposed by schemaPaymentPublic
+    }
+  });

Note: Verify the exact fields exposed by schemaPaymentPublic to ensure alignment.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 3a03311 and 786d236.

📒 Files selected for processing (1)
  • apps/api/v1/pages/api/payments/index.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

**/*.ts: For Prisma queries, only select data you need; never use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • apps/api/v1/pages/api/payments/index.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js .utc() in hot paths like loops

Files:

  • apps/api/v1/pages/api/payments/index.ts
**/*.{ts,tsx,js,jsx}

⚙️ CodeRabbit configuration file

Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.

Files:

  • apps/api/v1/pages/api/payments/index.ts
🔇 Additional comments (1)
apps/api/v1/pages/api/payments/index.ts (1)

48-48: LGTM! Correctly returns 200 with payments array.

Line 48 correctly addresses the previous review feedback about the unreachable 404 response. Returning 200 with an empty array when no payments exist is the standard REST API pattern and aligns with test expectations.

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: 2

🧹 Nitpick comments (3)
apps/api/v1/test/lib/payments/_get.test.ts (3)

7-7: Consider using named exports.

The handler is imported as a default export. According to coding guidelines, named exports are preferred for better tree-shaking, easier refactoring, and clearer imports.

Based on coding guidelines.


150-185: Consider removing redundant test case.

This test specifically checks for the uid field in the response, but the first test case (lines 40-110) already validates the full response structure including uid. Unless there's a specific regression risk or edge case for uid exposure, this test is redundant.


40-185: Refactor to reduce code duplication.

There's significant duplication in mock setup across test cases. The mockUser and mockPayments structures are recreated in each test with minor variations.

Consider extracting common mock data into factory functions or constants:

const createMockUser = (overrides = {}) => ({
  id: 1,
  bookings: [{ id: 1 }],
  ...overrides,
});

const createMockPayment = (overrides = {}) => ({
  id: 1,
  uid: "payment_123",
  amount: 5000,
  success: true,
  refunded: false,
  fee: 150,
  paymentOption: "ON_BOOKING",
  currency: "USD",
  bookingId: 1,
  ...overrides,
});

This improves maintainability and makes it easier to update mock structures consistently across tests.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 786d236 and 737ddb3.

📒 Files selected for processing (2)
  • apps/api/v1/test/lib/payments/[id]/_get.test.ts (1 hunks)
  • apps/api/v1/test/lib/payments/_get.test.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/api/v1/test/lib/payments/[id]/_get.test.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

**/*.ts: For Prisma queries, only select data you need; never use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • apps/api/v1/test/lib/payments/_get.test.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js .utc() in hot paths like loops

Files:

  • apps/api/v1/test/lib/payments/_get.test.ts
**/*.{ts,tsx,js,jsx}

⚙️ CodeRabbit configuration file

Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.

Files:

  • apps/api/v1/test/lib/payments/_get.test.ts
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (1)
apps/api/v1/test/lib/payments/_get.test.ts (1)

76-76: Verify the userId attachment mechanism.

The test sets userId: 1 directly on the mock request object. Typically, userId is attached by authentication middleware. Ensure this mock pattern accurately reflects how the handler receives the user ID in production, or clarify that this is testing post-middleware behavior.

Also applies to: 124-124, 142-142, 176-176

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

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 737ddb3 and 62b4924.

📒 Files selected for processing (2)
  • apps/api/v1/pages/api/payments/index.ts (1 hunks)
  • apps/api/v1/test/lib/payments/_get.test.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/api/v1/test/lib/payments/_get.test.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

**/*.ts: For Prisma queries, only select data you need; never use include, always use select
Ensure the credential.key field is never returned from tRPC endpoints or APIs

Files:

  • apps/api/v1/pages/api/payments/index.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/review.mdc)

Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js .utc() in hot paths like loops

Files:

  • apps/api/v1/pages/api/payments/index.ts
**/*.{ts,tsx,js,jsx}

⚙️ CodeRabbit configuration file

Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.

Files:

  • apps/api/v1/pages/api/payments/index.ts
🧬 Code graph analysis (1)
apps/api/v1/pages/api/payments/index.ts (1)
apps/api/v1/lib/validations/payment.ts (1)
  • schemaPaymentPublic (3-13)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Linters / lint
  • GitHub Check: Type check / check-types
  • GitHub Check: Tests / Unit
🔇 Additional comments (5)
apps/api/v1/pages/api/payments/index.ts (5)

16-22: LGTM! API parameter documentation is clear.

The apiKey parameter documentation is appropriate for API consumers, even though the implementation delegates authentication to the middleware layer.


26-32: LGTM! Response schema properly references typed definitions.

The ArrayOfPayments schema reference improves type safety and API contract clarity, aligning with the PR objectives to fix Zod validation errors.


34-37: LGTM! Documentation descriptions match the implementation.

The updated 401 and 404 descriptions accurately reflect the endpoint's behavior, particularly the new user existence check.


44-46: LGTM! User existence check provides proper error handling.

The 404 response for missing users improves error handling and aligns with the PR objective to provide clear error messaging for non-200 responses.


52-52: LGTM! Correctly resolves past review feedback.

The implementation now always returns 200 with the payments array (which may be empty), resolving the past review comment about the unreachable 404 branch when checking a truthy array. This approach aligns with the test expectations and is a valid API pattern.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api area: API, enterprise API, access token, OAuth 🐛 bug Something isn't working community Created by Linear-GitHub Sync Medium priority Created by Linear-GitHub Sync payments Created by Linear-GitHub Sync size/XL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Zod validation errors on /payments and /payments/[id] API endpoints
2 participants
0