feat(payments-plugin): Add multi-currency support for Braintree merchant accounts #4067
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
All contributors have signed the CLA ✍️ ✅ |
WalkthroughThe changes add dynamic merchant account ID resolution to the Braintree payments plugin. A new helper function resolves merchant account IDs based on currency codes, with support for both global defaults and currency-specific mappings. Configuration options are exposed through an updated plugin interface. Changes
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ 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 |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/payments-plugin/src/braintree/types.ts (1)
1-12: Critical: PR objectives are inconsistent with actual changes.The PR objectives reference Issue #3254 about adding
stateto promotion side-effects (onActivateandonDeactivate), which is completely unrelated to the Braintree multi-currency support implemented in this PR. This appears to be a copy-paste error in the PR metadata.
Consider documenting the new fields in the main interface.
The new
merchantAccountIdandmerchantAccountIdByCurrencyfields are added via declaration merging but lack JSDoc documentation. For consistency with the extensively documentedBraintreePluginOptionsinterface at lines 28-103, consider moving these fields into that block with proper documentation explaining:
- When to use
merchantAccountId(global default)- When to use
merchantAccountIdByCurrency(currency-specific overrides)- The fallback behavior
📝 Suggested documentation structure
export interface BraintreePluginOptions { /** * @description * The Braintree environment being targeted, e.g. sandbox or production. * * @default Environment.Sandbox */ environment?: Environment; + /** + * @description + * The default Braintree merchant account ID to use for transactions. + * Can be overridden on a per-currency basis using `merchantAccountIdByCurrency`. + * + * @since 1.9.0 + */ + merchantAccountId?: string; + /** + * @description + * A mapping of currency codes to Braintree merchant account IDs. + * When processing a payment, the merchant account ID for the order's currency + * will be used if available, otherwise falling back to `merchantAccountId`. + * + * @example + * ```ts + * { + * merchantAccountId: 'default_account', + * merchantAccountIdByCurrency: { + * [CurrencyCode.GBP]: 'uk_account', + * [CurrencyCode.EUR]: 'eu_account', + * } + * } + * ``` + * + * @since 1.9.0 + */ + merchantAccountIdByCurrency?: Partial<Record<CurrencyCode, string>>; /** * @description * If set to `true`, a [Customer](https://developer.paypal.com/braintree/docs/guides/customers) objectpackages/payments-plugin/src/braintree/braintree-common.ts (1)
56-67: Simplify the error throwing pattern.The function logic is correct, but the IIFE pattern
(() => { throw new Error(...) })()for throwing an error is unnecessarily complex. Consider simplifying to a direct throw or conditional check.🔎 Simpler alternatives
Option 1: Extract and validate
export function resolveMerchantAccountId( currencyCode: CurrencyCode, options: BraintreePluginOptions, ): string { - return ( - options.merchantAccountIdByCurrency?.[currencyCode] ?? - options.merchantAccountId ?? - (() => { - throw new Error(`No Braintree merchantAccountId configured for currency ${currencyCode}`); - })() - ); + const merchantAccountId = + options.merchantAccountIdByCurrency?.[currencyCode] ?? + options.merchantAccountId; + + if (!merchantAccountId) { + throw new Error(`No Braintree merchantAccountId configured for currency ${currencyCode}`); + } + + return merchantAccountId; }Option 2: Keep inline but simplify (if you prefer one-liner)
export function resolveMerchantAccountId( currencyCode: CurrencyCode, options: BraintreePluginOptions, ): string { - return ( - options.merchantAccountIdByCurrency?.[currencyCode] ?? - options.merchantAccountId ?? - (() => { - throw new Error(`No Braintree merchantAccountId configured for currency ${currencyCode}`); - })() - ); + const merchantAccountId = options.merchantAccountIdByCurrency?.[currencyCode] ?? options.merchantAccountId; + if (!merchantAccountId) throw new Error(`No Braintree merchantAccountId configured for currency ${currencyCode}`); + return merchantAccountId; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
packages/payments-plugin/src/braintree/braintree-common.ts(2 hunks)packages/payments-plugin/src/braintree/braintree.handler.ts(3 hunks)packages/payments-plugin/src/braintree/types.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/payments-plugin/src/braintree/braintree.handler.ts (1)
packages/payments-plugin/src/braintree/braintree-common.ts (1)
resolveMerchantAccountId(56-67)
packages/payments-plugin/src/braintree/types.ts (1)
packages/payments-plugin/src/braintree/braintree.handler.ts (1)
braintreePaymentMethodHandler(25-87)
🔇 Additional comments (2)
packages/payments-plugin/src/braintree/braintree.handler.ts (1)
14-14: LGTM: Import added for merchant account resolution.The import of
resolveMerchantAccountIdcorrectly brings in the new helper function for dynamic merchant account ID resolution.packages/payments-plugin/src/braintree/braintree-common.ts (1)
1-1: LGTM: Import added for currency code type.The
CurrencyCodeimport is correctly added to support the new multi-currency merchant account resolution.
| customerId = await getBraintreeCustomerId(ctx, gateway, customer); | ||
| } | ||
| return processPayment(ctx, gateway, order, amount, metadata.nonce, customerId, options); | ||
| return processPayment(ctx, gateway, order, amount, metadata.nonce, customerId, options, args); |
There was a problem hiding this comment.
Critical: Wrong parameter passed to resolveMerchantAccountId.
Line 104 calls resolveMerchantAccountId(order.currencyCode, args), but the function signature expects BraintreePluginOptions as the second parameter, not PaymentMethodArgsHash.
argscontainsmerchantId,publicKey, andprivateKey(payment method configuration)resolveMerchantAccountIdexpectsoptionswhich containsmerchantAccountIdandmerchantAccountIdByCurrency(plugin options)
This will cause the function to always throw the error "No Braintree merchantAccountId configured" because args doesn't have the required properties.
🔧 Proposed fix
The pluginOptions variable is already available in the processPayment function scope, so you can either:
Option 1: Pass pluginOptions instead of args (recommended - no signature change needed)
- return processPayment(ctx, gateway, order, amount, metadata.nonce, customerId, options, args);
+ return processPayment(ctx, gateway, order, amount, metadata.nonce, customerId, options); async function processPayment(
ctx: RequestContext,
gateway: BraintreeGateway,
order: Order,
amount: number,
paymentMethodNonce: any,
customerId: string | undefined,
pluginOptions: BraintreePluginOptions,
- args: any,
) {
const response = await gateway.transaction.sale({
customerId,
amount: (amount / 100).toString(10),
orderId: order.code,
paymentMethodNonce,
- merchantAccountId: resolveMerchantAccountId(order.currencyCode, args),
+ merchantAccountId: resolveMerchantAccountId(order.currencyCode, pluginOptions),
options: {
submitForSettlement: true,
storeInVaultOnSuccess: !!customerId,
},
});Option 2: Keep args parameter but pass pluginOptions to the resolver
- merchantAccountId: resolveMerchantAccountId(order.currencyCode, args),
+ merchantAccountId: resolveMerchantAccountId(order.currencyCode, pluginOptions),Also applies to: 97-97, 104-104
🤖 Prompt for AI Agents
In packages/payments-plugin/src/braintree/braintree.handler.ts around lines 52,
97 and 104, the call to resolveMerchantAccountId is passing the payment method
args object instead of the plugin options, causing missing merchantAccountId
errors; update those calls to pass the pluginOptions variable (which contains
merchantAccountId and merchantAccountIdByCurrency) as the second argument so
resolveMerchantAccountId receives the correct BraintreePluginOptions and no
signature changes are required.
|
I have read the CLA Document and I hereby sign the CLA |
1 participant
This PR adds support for multi-currency setups in the Braintree payments plugin.
Vendure already supports multiple currencies per channel, but the Braintree plugin
previously assumed a single merchant account. This change resolves the appropriate
merchantAccountId dynamically based on the order currency, with a configurable
per-currency mapping and a safe fallback to the default merchantAccountId.
The change is backward-compatible and does not affect existing single-currency setups.
Closes #3254