-
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
📝 WalkthroughWalkthroughThis PR adds support for custom fields in client payments by introducing field validation and persistence. Changes span constants (error messages), database schema (new 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<br/>{amount, fields, address}
APIHandler->>Validators: parseCreatePaymentIdPOSTRequest(body)
Validators->>Validators: parseAmount(amountInput)
alt Invalid Amount
Validators-->>APIHandler: INVALID_AMOUNT_400 error
APIHandler-->>Client: 400 Response
end
Validators->>Validators: parseClientPaymentFields(fieldsInput)
alt Invalid Fields Format
Validators-->>APIHandler: INVALID_FIELDS_FORMAT_400 error
APIHandler-->>Client: 400 Response
end
alt Invalid Field Structure
Validators-->>APIHandler: INVALID_FIELD_STRUCTURE_400 error
APIHandler-->>Client: 400 Response
end
Validators-->>APIHandler: {amount, fields, address}
APIHandler->>Service: generatePaymentId(address, amount, fields)
Service->>DB: Create ClientPayment<br/>(address, amount, fields as JSON)
DB-->>Service: payment record created
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 and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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.
| export const parseClientPaymentFields = function (fieldsInput: string | undefined): ClientPaymentField[] | undefined { | ||
| if (fieldsInput === undefined || fieldsInput === '') { | ||
| return undefined | ||
| } | ||
|
|
||
| let parsedFields: unknown | ||
| try { | ||
| parsedFields = JSON.parse(fieldsInput) | ||
| } catch { | ||
| throw new Error(RESPONSE_MESSAGES.INVALID_FIELDS_FORMAT_400.message) | ||
| } | ||
|
|
||
| if (!Array.isArray(parsedFields)) { | ||
| throw new Error(RESPONSE_MESSAGES.INVALID_FIELDS_FORMAT_400.message) | ||
| } | ||
|
|
||
| for (const field of parsedFields) { | ||
| if ( | ||
| typeof field !== 'object' || | ||
| field === null || | ||
| typeof field.name !== 'string' || | ||
| field.name?.trim() === '' | ||
| ) { | ||
| throw new Error(RESPONSE_MESSAGES.INVALID_FIELD_STRUCTURE_400.message) | ||
| } | ||
| if (field.type !== undefined && typeof field.type !== 'string') { | ||
| throw new Error(RESPONSE_MESSAGES.INVALID_FIELD_STRUCTURE_400.message) | ||
| } | ||
| if (field.value !== undefined && typeof field.value !== 'string' && typeof field.value !== 'boolean') { | ||
| throw new Error(RESPONSE_MESSAGES.INVALID_FIELD_STRUCTURE_400.message) | ||
| } | ||
| } | ||
|
|
||
| return parsedFields as ClientPaymentField[] | ||
| } |
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.
🧩 Analysis chain
🏁 Script executed:
ast-grep --pattern 'interface ClientPaymentField {
$$$
}'Repository: PayButton/paybutton-server
Length of output: 399
🏁 Script executed:
# Also search with rg in case the interface is defined differently
rg 'interface ClientPaymentField' -A 10Repository: PayButton/paybutton-server
Length of output: 1265
🏁 Script executed:
# Check for type definitions as well
rg 'type ClientPaymentField' -A 10Repository: PayButton/paybutton-server
Length of output: 52
🏁 Script executed:
rg 'field\.text' -B 2 -A 2Repository: PayButton/paybutton-server
Length of output: 52
🏁 Script executed:
# Check if text is accessed without type checking anywhere
rg '\.text' services/clientPaymentService.ts -B 2 -A 2Repository: PayButton/paybutton-server
Length of output: 52
🏁 Script executed:
# Find where parseClientPaymentFields is used
rg 'parseClientPaymentFields' -B 2 -A 2Repository: PayButton/paybutton-server
Length of output: 586
🏁 Script executed:
# Search for "text" in the context of fields to understand its role
rg 'text.*field|field.*text' -iRepository: PayButton/paybutton-server
Length of output: 699
🏁 Script executed:
# Check the broader context of field properties usage
rg 'field\.(name|type|value|text)' -B 1 -A 1Repository: PayButton/paybutton-server
Length of output: 1475
Add validation for the optional text property to be consistent with type and value validation.
The ClientPaymentField interface includes an optional text: string property, but the validation function checks the types of type and value when present while skipping validation for text. Add a type check for text similar to the existing conditional checks:
if (field.text !== undefined && typeof field.text !== 'string') {
throw new Error(RESPONSE_MESSAGES.INVALID_FIELD_STRUCTURE_400.message)
}🤖 Prompt for AI Agents
In utils/validators.ts around lines 593 to 627, the parser validates optional
fields `type` and `value` but omits validation for the optional `text` property;
add a check that if field.text is defined it must be a string and throw
RESPONSE_MESSAGES.INVALID_FIELD_STRUCTURE_400.message on violation, mirroring
the existing conditional style used for `type` and `value`.
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.