Fix additional card validation #27
Conversation
WalkthroughThe PR bumps the Braintree payment plugin to version 0.0.15 and integrates cart service functionality into BraintreeBase. Transaction request creation is now asynchronous and includes cart validation, enhanced line item construction with extended fields, and expanded validation schema for card details. Changes
Sequence DiagramsequenceDiagram
participant Caller as Caller (initiatePayment/<br/>createTransaction)
participant BraintreeBase as BraintreeBase
participant CartService as CartService
participant Braintree as Braintree API
Caller->>BraintreeBase: initiatePayment() / createTransaction()
Note over BraintreeBase: New: Async flow
BraintreeBase->>BraintreeBase: getTransactionCreateRequestBody() [async]
BraintreeBase->>CartService: retrieve cart by cart_id
CartService-->>BraintreeBase: CartDTO
alt Cart found
BraintreeBase->>BraintreeBase: Build enhanced line items<br/>(with cart currency,<br/>extended fields)
BraintreeBase-->>Caller: Promise<TransactionRequest>
Caller->>Braintree: submit transaction
Braintree-->>Caller: response
else Cart not found
BraintreeBase-->>Caller: throw NOT_FOUND error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts (1)
551-563: Remove unused cardDetails schema validation — type contract mismatchThe
BraintreeInitiatePaymentDatainterface (line 92) only declarespayment_method_nonce?: string, but the schema acceptscardDetails(lines 555–561). This creates a type safety violation: the validated data includes fields the return type doesn't declare, and these fields are never used.Since
cardDetailsis not in the type interface and is not extracted or persisted ininitiatePayment(verified at lines 577–596), remove this schema validation entirely:const schema = z.object({ paymentMethodNonce: z.string().optional(), payment_method_nonce: z.string().optional(), - cardDetails:z.object({ - cardType: z.string().optional(), - lastFour: z.string().optional(), - lastTwo: z.string().optional(), - expirationMonth: z.string().optional(), - expirationYear: z.string().optional(), - cardholderName: z.string().optional(), - }).optional(), });
🧹 Nitpick comments (2)
plugins/braintree-payment/package.json (1)
3-3: Version bump to 0.0.15 — proceed, but align release hygiene
- Make sure CHANGELOG and tag/release notes reflect the async API change and new L2/L3 fields.
- If the plugin now requires the CART module at runtime, document this in README.
- Optional: peerDependency "@medusajs/medusa" is pinned to "2.8.2". Consider "^2.8.2" to avoid resolution conflicts when stores upgrade patch/minor.
plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts (1)
445-452: Avoid fetching cart just to derive unitOfMeasure; and don’t assume first item’s cart_id
- Using items[0].cart_id is brittle if items is empty or mixed.
- You only use cart to set unitOfMeasure later, but unitOfMeasure is not currency. This fetch adds latency for little value.
Consider removing the cart fetch and sourcing unitOfMeasure from item metadata (or default 'EA'), not currency. If you keep the fetch, prefer a cart_id from context rather than inferring from the first item.
📜 Review details
Configuration used: CodeRabbit UI
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 ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (2)
plugins/braintree-payment/package.json(1 hunks)plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts(9 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts (1)
plugins/braintree-payment/src/providers/payment-braintree/src/types/index.ts (1)
BraintreeOptions(3-12)
🔇 Additional comments (3)
plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts (3)
21-33: New CartDTO/ICartModuleService imports — OKImports look correct for using Modules.CART.
Confirm runtime availability of Modules.CART in all environments where this plugin runs (local, worker, background processors). If it can be absent, add a guard in constructor.
399-416: Async request body — LGTMChanging getTransactionCreateRequestBody to async is appropriate given the cart fetch. Call sites updated below.
Also applies to: 413-413
615-622: Awaiting async request body — LGTMCall site updated to await the async builder.
Please re-run typecheck and end-to-end sale flow to ensure no other paths depended on the old sync signature.
| cartService: ICartModuleService; | ||
|
|
||
| protected constructor(container: BraintreeConstructorArgs, options: BraintreeOptions) { | ||
| super(container, options); | ||
|
|
||
| this.options_ = options; | ||
| this.logger = container[ContainerRegistrationKeys.LOGGER]; | ||
| this.cache = container[Modules.CACHE]; | ||
| this.cartService = container[Modules.CART] as ICartModuleService; | ||
| this.init(); |
There was a problem hiding this comment.
Guard DI for cartService or declare hard requirement
container[Modules.CART] may be undefined in some deployments, causing a TypeError on use. Either explicitly document CART as a hard requirement or guard the resolution and fallback.
Apply this minimal guard:
- this.cartService = container[Modules.CART] as ICartModuleService;
+ this.cartService = container[Modules.CART] as ICartModuleService;
+ if (!this.cartService) {
+ this.logger.warn('BraintreeBase: CART module not available; skipping cart-derived fields');
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cartService: ICartModuleService; | |
| protected constructor(container: BraintreeConstructorArgs, options: BraintreeOptions) { | |
| super(container, options); | |
| this.options_ = options; | |
| this.logger = container[ContainerRegistrationKeys.LOGGER]; | |
| this.cache = container[Modules.CACHE]; | |
| this.cartService = container[Modules.CART] as ICartModuleService; | |
| this.init(); | |
| cartService: ICartModuleService; | |
| protected constructor(container: BraintreeConstructorArgs, options: BraintreeOptions) { | |
| super(container, options); | |
| this.options_ = options; | |
| this.logger = container[ContainerRegistrationKeys.LOGGER]; | |
| this.cache = container[Modules.CACHE]; | |
| this.cartService = container[Modules.CART] as ICartModuleService; | |
| if (!this.cartService) { | |
| this.logger.warn('BraintreeBase: CART module not available; skipping cart-derived fields'); | |
| } | |
| this.init(); |
🤖 Prompt for AI Agents
In
plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts
around lines 127 to 136, the DI resolution for cartService uses
container[Modules.CART] directly which can be undefined and cause a TypeError;
add a guard when resolving cartService: check if container[Modules.CART] exists
and either assign it to this.cartService or throw a clear error indicating that
the CART module is required (or assign a safe fallback/null and update usages to
handle absence). Ensure the constructor documents the hard requirement if you
choose to throw, or implement the safe-null pattern and defensive checks where
cartService is used.
There was a problem hiding this comment.
Added in Documentation
There was a problem hiding this comment.
@SGFGOV, thank you for addressing the concern! Documenting the CART module as a required dependency is a valid approach. This ensures that users of the plugin are aware of the requirement during setup and configuration.
🐰
plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts
Show resolved
Hide resolved
| if(context.items?.[0]?.cart_id) { | ||
| cart = await this.cartService.retrieveCart(context.items?.[0]?.cart_id as string); | ||
| if (!cart) { | ||
| throw new MedusaError(MedusaError.Types.NOT_FOUND, 'Cart not found'); | ||
| } | ||
| } |
There was a problem hiding this comment.
Hmmm @SGFGOV it might not have been the best choice to pull in the cart module here. The braintree payment plugin can be used for cart purchases, but also order edit, which doesn't have a cart. Is there a reason you needed to pull in the cart here, instead of including more information here: https://github.com/lambda-curry/360training/blob/dev/apps/medusa/src/workflows/complete-360-cart/complete-360-cart.ts#L254C13-L254C25
There was a problem hiding this comment.
@SGFGOV any reason we decided to commit a package-lock when we only use yarn here?
Added a few extra fields to capture some of line item and card related data in the custom fields
Summary by CodeRabbit
New Features
Chores