-
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 structured client payment fields: request parsing/validation for amount and fields, three new validation error messages, DB schema + migration to store JSON fields, service methods to persist/retrieve/update fields, and API handler updates to accept and forward fields. Changes
Sequence Diagram(s)sequenceDiagram
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 { INVALID_AMOUNT_400 }
end
Validators->>Validators: parseClientPaymentFields(fieldsInput)
alt invalid format
Validators-->>APIHandler: INVALID_FIELDS_FORMAT_400
APIHandler-->>Client: 400 { INVALID_FIELDS_FORMAT_400 }
end
alt invalid structure
Validators-->>APIHandler: INVALID_FIELD_STRUCTURE_400
APIHandler-->>Client: 400 { INVALID_FIELD_STRUCTURE_400 }
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
APIHandler-->>Client: 200 { 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)
⏰ 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)
🔇 Additional comments (4)
✏️ Tip: You can disable this entire section by setting 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.
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: 3
🤖 Fix all issues with AI agents
In `@utils/validators.ts`:
- Line 620: Remove the leftover debug console.log('field:', field) statement
from utils/validators.ts; locate the occurrence where the variable field is
logged and delete that line (or replace it with an appropriate logging call like
processLogger.debug if persistent debug output is required), ensuring no other
stray console.log calls remain in the validator functions.
- Around line 634-647: The parseAmount function currently accepts values like
"Infinity" because Number(trimmedAmount) yields Infinity which is not NaN and >
0; update parseAmount to explicitly reject non-finite numbers by checking
Number.isFinite(numericAmount) (or !isFinite(numericAmount)) in addition to the
existing isNaN/numericAmount <= 0 checks, and throw
RESPONSE_MESSAGES.INVALID_AMOUNT_400 when the value is not finite so that new
Prisma.Decimal(...) never receives "Infinity" or other special numeric strings.
- Around line 623-628: Validation is missing for the optional
ClientPaymentField.type property; update the same validator that checks
field.value and field.text to also check that if field.type !== undefined then
typeof field.type === 'string', and throw
RESPONSE_MESSAGES.INVALID_FIELD_STRUCTURE_400.message on failure. Locate the
validator where field.value and field.text are validated (the block referencing
field.value, field.text and
RESPONSE_MESSAGES.INVALID_FIELD_STRUCTURE_400.message) and add the analogous
type check for field.type to maintain consistent structure validation.
🧹 Nitpick comments (2)
utils/validators.ts (2)
580-585: Unused interfaceClientPaymentFieldInput.This interface is defined but never referenced. The
parseClientPaymentFieldsfunction works withunknownand casts directly toClientPaymentField[]. Consider removing this to avoid dead code, or use it as a type guard in the parsing function.
600-604: Type inconsistency: signature saysstring | undefinedbut code checks forobject.The function signature declares
fieldsInput: string | undefined, yet line 600 checkstypeof fieldsInput === 'object'. This suggests either defensive coding for untyped JavaScript callers, or a mismatch between the declared type and actual runtime usage. Consider aligning the signature with actual usage or documenting why the object check is necessary.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
utils/validators.ts
⏰ 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 (2)
utils/validators.ts (2)
13-13: LGTM!Import is correctly added and aligns with the type usage in the new parsing functions.
649-664: LGTM!The function correctly integrates the new
parseAmountandparseClientPaymentFieldsutilities, maintaining clean separation of concerns. The return structure aligns with the updatedCreatePaymentIdInputinterface.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
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
🤖 Fix all issues with AI agents
In `@utils/validators.ts`:
- Around line 598-607: The runtime checks treat fieldsInput as possibly an
object but the signature is string | undefined; fix by either (A) updating the
function parameter/type annotation for fieldsInput to string | object |
undefined (and keep the existing branch that assigns parsedFields = fieldsInput
when typeof fieldsInput === 'object') or (B) removing the unreachable object
branch and always JSON.parse(fieldsInput) when fieldsInput is a string; ensure
parsedFields remains typed as unknown and keep throwing
RESPONSE_MESSAGES.INVALID_FIELDS_FORMAT_400.message on parse errors.
🧹 Nitpick comments (1)
utils/validators.ts (1)
580-585: Remove unused interface definition.
ClientPaymentFieldInputis defined at lines 580-585 but is never used anywhere in the codebase. Remove this interface or clarify its intended purpose if it's planned for future use.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
utils/validators.ts
🧰 Additional context used
🧬 Code graph analysis (1)
utils/validators.ts (2)
services/clientPaymentService.ts (1)
ClientPaymentField(10-15)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 (4)
utils/validators.ts (4)
13-13: LGTM!Import is correctly added and used by the new parsing functions.
613-631: LGTM!Field validation is thorough and properly enforces the
ClientPaymentFieldinterface contract. All optional properties (value,type,text) are validated when present.
636-649: LGTM!The
parseAmountfunction correctly validates the input and handles edge cases includingInfinityandNaN. Using the trimmed string forPrisma.Decimalconstruction preserves precision.
659-666: LGTM!The integration of
parseAmountandparseClientPaymentFieldsis clean and follows the existing pattern in this file.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
74ed7f5 to
c978174
Compare
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
🤖 Fix all issues with AI agents
In `@utils/validators.ts`:
- Around line 580-585: Remove the dead ClientPaymentFieldInput interface: delete
the interface declaration for ClientPaymentFieldInput and any export that
references it, since parseClientPaymentFields validates unknown directly and
code uses ClientPaymentField[]; also search for and remove any re-exports or
type references to ClientPaymentFieldInput (or replace with ClientPaymentField
where appropriate). If the type was intentionally exposed for external
consumers, instead update parseClientPaymentFields signature or module exports
to expose ClientPaymentField and add a brief comment in place of the removed
interface documenting expected shape.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
utils/validators.ts
🧰 Additional context used
🧬 Code graph analysis (1)
utils/validators.ts (1)
services/clientPaymentService.ts (1)
ClientPaymentField(10-15)
⏰ 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 (4)
utils/validators.ts (4)
13-13: LGTM!Import correctly added for the
ClientPaymentFieldtype used in the parsing function.
593-634: LGTM!The validation logic is comprehensive and all previously identified issues have been addressed:
- Function signature correctly accepts
string | object | undefined- Validation covers all optional properties (
type,value,text)- Debug statement removed
636-649: LGTM!The
parseAmountfunction correctly validates numeric input, including theisFinitecheck that preventsInfinityfrom passing validation.
651-667: LGTM!The function cleanly delegates to the new helper functions and returns a properly typed
CreatePaymentIdInputobject.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| export interface ClientPaymentFieldInput { | ||
| name?: string | ||
| text?: string | ||
| type?: string | ||
| value?: string | boolean | ||
| } |
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:
rg -n 'ClientPaymentFieldInput' --type=ts -g '!utils/validators.ts'Repository: PayButton/paybutton-server
Length of output: 52
Remove unused ClientPaymentFieldInput interface.
This interface is not referenced anywhere in the codebase. The parseClientPaymentFields function validates unknown input directly and casts to ClientPaymentField[], making ClientPaymentFieldInput dead code. Remove it unless it's intentionally exposed for external API documentation or client-side typing.
🤖 Prompt for AI Agents
In `@utils/validators.ts` around lines 580 - 585, Remove the dead
ClientPaymentFieldInput interface: delete the interface declaration for
ClientPaymentFieldInput and any export that references it, since
parseClientPaymentFields validates unknown directly and code uses
ClientPaymentField[]; also search for and remove any re-exports or type
references to ClientPaymentFieldInput (or replace with ClientPaymentField where
appropriate). If the type was intentionally exposed for external consumers,
instead update parseClientPaymentFields signature or module exports to expose
ClientPaymentField and add a brief comment in place of the removed interface
documenting expected shape.
c978174 to
c552672
Compare
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
Bug Fixes / Validation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.