Skip to content
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

feat(payments-plugin): Add multi-currency support for Braintree #3223

Closed

Conversation

kkerti
Copy link
Contributor

@kkerti kkerti commented Nov 21, 2024

Description

As briefly described in #3218, this PR adds new feature to the core BraintreePlugin, extending the plugin options with an optional object named merchantAccountIds.

  1. Usage of Braintree Merchant Account Ids are optional, as without them, the Braintree account's default currency (and Merchant Id) is used for authorising transactions.
// src/braintree/types.ts
export interface BraintreePluginOptions {
  //...other defs
  merchantAccountIds?: BraintreeMerchantAccountIds;
}

// e2e/braintree-dev-server.ts
// Plugin initialisation with currency-merchantAccountId key and value pairs.
BraintreePlugin.init({
    //...other config
    merchantAccountIds: {
        USD: process.env.BRAINTREE_MERCHANT_ACCOUNT_ID_USD,
        EUR: process.env.BRAINTREE_MERCHANT_ACCOUNT_ID_EUR,
    },
}),
  1. Two of braintree's nodejs API use merchantAccountId, the gateway.clientToken.generate() and gateway.transaction.sale(). Both of these methods accept the merchantAccountId with type string | undefined. To look up the correct merchantAccountId to use, we are using the active order.currencyCode and the plugin options. I did not implement heavy input error checks as the plugin option validity is evaluated runtime thanks to typescript.
// src/braintree/braintree.handler.ts
// used in clientToken generation and processPayment
const merchantAccountId = lookupMerchantAccountIdByCurrency(
    options.merchantAccountIds,
    order.currencyCode,
);

// src/braintree/braintree-common.ts
export function lookupMerchantAccountIdByCurrency(
    merchantAccountIds: BraintreeMerchantAccountIds | undefined,
    currencyCode: CurrencyCode,
): string | undefined {
    if (!merchantAccountIds || !currencyCode) {
        return undefined;
    }
    const merchantAccountIdForCurrency = merchantAccountIds[currencyCode];
    return merchantAccountIdForCurrency;
}
  1. There was no development server setup in the Braintree plugin, so I took inspiration from the Mollie and Stripe examples. The heavy lifting is done here by using simply a custom payment handler, clientToken generation and relying on the braintree-drop-in UI client side. The braintree-dev-server sets up an Order, logs info for the developer and exposes the shopClient and generated clientToken for a test plugin found under /fixtures/braintree-checkout-test.plugin.ts.
  2. The additional test plugin exposes a controller with the checkout UI, similar to the Stripe fixture plugin. Results of the attempted payments are dumped in <div id="result"/>.
image
  1. I added a BraintreeTestPlugin, but I did not add any automated tests, as the nonce generated by the client drop-in UI doesn't have fake-currency-mismatch-nonce test value. The drop-in UI in the browser has to be manually completed and the value sent to the vendure server. I think the development server introduced here helps a lot in this, but I'm open for suggestions to make complete e2e here.

Possible improvement

I think it's not optimal to add the merchantAccountId values during plugin initialisation, this would be better off as args for the payment handler itself. But I would rather wait for the new struct type to create the currency-merchantAccountId pairs as handler arguments to make it bullet-proof.

Breaking changes

As far as I'm concerned, there are none.

Checklist

📌 Always:

  • I have set a clear title
  • My PR is small and contains a single feature
  • I have checked my own PR

👍 Most of the time:

  • I have added or updated test cases
  • I have updated the README if needed

Copy link

vercel bot commented Nov 21, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview Nov 26, 2024 10:07pm

@kkerti
Copy link
Contributor Author

kkerti commented Nov 26, 2024

It seems to me that the package-lock.json merge conflict is present because I forked the vendure master repo and developed against it, instead of the minor repo which I made the PR against, based on the contribution guidelines.

Anyways let me know if I should open a new PR in a different format to avoid the merge conflict, or I should merge the minor to my fork first.

Copy link

sonarcloud bot commented Nov 26, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
1 Security Hotspot

See analysis details on SonarQube Cloud

@kkerti kkerti closed this Nov 26, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Nov 26, 2024
@kkerti kkerti deleted the feat/braintree-multi-currency branch November 26, 2024 22:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant