-
Notifications
You must be signed in to change notification settings - Fork 3
feat: client payment fields #1100
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: master
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughAdds support for custom client payment fields: validators to parse/validate fields and amount, response messages for new validation errors, DB schema migration to store JSON fields, service API changes to persist/retrieve/update fields, and API handler updates to accept and forward fields. Changes
Sequence DiagramsequenceDiagram
participant Client
participant APIHandler as API Handler
participant Validators
participant Service as Payment Service
participant DB as Database
Client->>APIHandler: POST /api/payments/paymentId {address, amount, fields}
APIHandler->>Validators: parseCreatePaymentIdPOSTRequest(body)
Validators->>Validators: parseAmount(amountInput)
alt Invalid Amount
Validators-->>APIHandler: INVALID_AMOUNT_400
APIHandler-->>Client: 400 Response
end
Validators->>Validators: parseClientPaymentFields(fieldsInput)
alt Invalid Fields Format
Validators-->>APIHandler: INVALID_FIELDS_FORMAT_400
APIHandler-->>Client: 400 Response
end
alt Invalid Field Structure
Validators-->>APIHandler: INVALID_FIELD_STRUCTURE_400
APIHandler-->>Client: 400 Response
end
Validators-->>APIHandler: {amount, fields, address}
APIHandler->>Service: generatePaymentId(address, amount, fields)
Service->>DB: Create ClientPayment (address, amount, fields JSON)
DB-->>Service: Created record
Service-->>APIHandler: payment object
APIHandler-->>Client: 200 OK {paymentId, ...}
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
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 (3)
pages/api/payments/paymentId/index.ts (1)
16-19: RedundantparseAddresscall.
parseAddress(address)is called here on line 17, butgeneratePaymentIdinservices/clientPaymentService.ts(line 21) also callsparseAddress(address)internally. This results in double validation/parsing.Consider either:
- Removing the call here since the service handles it, or
- Removing it from the service and relying on the caller to provide a parsed address
Option 1: Remove redundant call here
- const { amount, fields, address } = parseCreatePaymentIdPOSTRequest(req.body) - const parsedAddress = parseAddress(address) - - const paymentId = await generatePaymentId(parsedAddress, amount, fields) + const { amount, fields, address } = parseCreatePaymentIdPOSTRequest(req.body) + const paymentId = await generatePaymentId(address, amount, fields)services/clientPaymentService.ts (1)
68-81: Silent failure for non-existent payment.
getClientPaymentFieldsreturns an empty array both when the payment doesn't exist and when fields are genuinely empty. If distinguishing these cases matters for callers, consider returningnullfor non-existent payments or throwing an error.Alternative: Return null for non-existent payment
-export const getClientPaymentFields = async (paymentId: string): Promise<ClientPaymentField[]> => { +export const getClientPaymentFields = async (paymentId: string): Promise<ClientPaymentField[] | null> => { const clientPayment = await prisma.clientPayment.findUnique({ where: { paymentId }, select: { fields: true } }) if (clientPayment === null) { - return [] + return null } try { return JSON.parse(clientPayment.fields) as ClientPaymentField[] } catch { return [] } }utils/validators.ts (1)
580-585: Remove unusedClientPaymentFieldInputinterface.This interface is defined at lines 580-585 but has no usages anywhere in the codebase.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
constants/index.tspages/api/payments/paymentId/index.tsprisma-local/migrations/20251228021700_client_payment_fields/migration.sqlprisma-local/schema.prismaservices/clientPaymentService.tsutils/validators.ts
🧰 Additional context used
🧬 Code graph analysis (2)
utils/validators.ts (2)
services/clientPaymentService.ts (1)
ClientPaymentField(10-15)constants/index.ts (1)
RESPONSE_MESSAGES(8-111)
pages/api/payments/paymentId/index.ts (3)
utils/validators.ts (2)
parseCreatePaymentIdPOSTRequest(644-660)parseAddress(32-49)services/clientPaymentService.ts (1)
generatePaymentId(17-52)constants/index.ts (1)
RESPONSE_MESSAGES(8-111)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run Tests
🔇 Additional comments (9)
prisma-local/schema.prisma (1)
289-289: LGTM!The new
fieldscolumn is well-designed:LongTextprovides sufficient capacity for storing JSON arrays, and the default"[]"ensures backward compatibility with existing records.prisma-local/migrations/20251228021700_client_payment_fields/migration.sql (1)
1-2: LGTM!The migration correctly adds the
fieldscolumn withNOT NULLand a default value of'[]', ensuring existing rows are properly handled without requiring data backfill.constants/index.ts (1)
107-110: LGTM!The new error constants follow the existing patterns in the codebase with appropriate status codes and clear, descriptive messages.
utils/validators.ts (2)
629-642: LGTM!The amount parsing correctly handles edge cases: undefined/empty returns undefined, and the validation ensures positive numbers only. Using
Prisma.Decimalfrom the trimmed string preserves precision for monetary values.
644-659: LGTM!The updated parser correctly integrates the new
amountandfieldsparsing while maintaining backward compatibility (both are optional).pages/api/payments/paymentId/index.ts (1)
30-38: LGTM!The new error cases properly handle the validation errors introduced for fields and amount, following the established pattern in this handler.
services/clientPaymentService.ts (3)
10-15: LGTM!The
ClientPaymentFieldinterface is well-structured with a requirednameproperty and sensible optional fields. Thevaluetype ofstring | booleanprovides appropriate flexibility for different field types.
83-88: LGTM!The update function is straightforward. If the
paymentIddoesn't exist, Prisma will throw an appropriate error which is consistent with other update patterns in this service.
39-40: LGTM!The fields are correctly serialized to JSON with a sensible default of
'[]'when undefined.
Related to #1021
Description
Added fields param to client payment table
Changed generate payment id endpoint to add the new fields param
Added functions to update and get fields
Test plan
Test
api/payments/paymentIdendpoint sending an array of objects in string format calledfieldsSummary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.