-
Notifications
You must be signed in to change notification settings - Fork 10.7k
fix: zod errors on /payments and /payments/[uid] api #24316
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: main
Are you sure you want to change the base?
Conversation
@shaun-ak is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis PR exposes the Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (3)**/*.ts📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Files:
**/*.{ts,tsx,js,jsx}⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (1)📚 Learning: 2025-07-28T11:50:23.946Z
Applied to files:
🧬 Code graph analysis (1)apps/api/v1/pages/api/payments/index.ts (1)
🔇 Additional comments (3)
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: 1
🧹 Nitpick comments (1)
apps/api/v1/pages/api/payments/[id].ts (1)
51-51
: Prefer explicitselect
to fetch only required fields.The payment query should explicitly use
select
to retrieve only the fields defined inschemaPaymentPublic
, 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.
📒 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 useinclude
, always useselect
Ensure thecredential.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 includepaymentUid
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.
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.
please address code rabbit suggestions.
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: 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
toselect
, butselect: { bookings: true }
still fetches all booking columns. Line 61 only needs booking IDs for authorization. As noted in the previous review, only request theid
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.
📒 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 useinclude
, always useselect
Ensure thecredential.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 thePaymentResponse
type annotation on line 43. While this is common in API routes, verify that error response typing aligns with your API contracts.
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: 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
: Useselect
instead ofinclude
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
toselect
(addressing a previous review comment), Line 56 still selects all booking fields withselect: { 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 chaininguserWithBookings?.
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.
📒 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 useinclude
, always useselect
Ensure thecredential.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 inapps/api/v1/pages/api/payments/index.ts
(see related review comment on that file about the unreachable 404 path).
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 (2)
apps/api/v1/pages/api/payments/index.ts (2)
38-41
: Replaceinclude
withselect
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 useinclude
, always useselect
." 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.
📒 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 useinclude
, always useselect
Ensure thecredential.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.
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: 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 includinguid
. Unless there's a specific regression risk or edge case foruid
exposure, this test is redundant.
40-185
: Refactor to reduce code duplication.There's significant duplication in mock setup across test cases. The
mockUser
andmockPayments
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.
📒 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 useinclude
, always useselect
Ensure thecredential.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
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: 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.
📒 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 useinclude
, always useselect
Ensure thecredential.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.
What does this PR do?
Visual Demo (For contributors especially)
Before-
Before.mp4
After-
After.mp4
Mandatory Tasks (DO NOT REMOVE)