-
Notifications
You must be signed in to change notification settings - Fork 1
Feat: Braintree import provider #22
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
Conversation
WalkthroughIntroduces a new “import” Braintree payment provider, adds tests and Jest tooling, refactors the core base provider (constructor args, status mapping, amount formatting, token caching), adds account-holder methods, updates exports to register the new service, and removes the currency smallest-unit utility and its tests. Also updates editor settings. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ImportProvider as BraintreeImport
participant Gateway as Braintree Gateway
Client->>ImportProvider: refundPayment(session, amount)
alt importedAsRefunded = true
ImportProvider-->>Client: Update session.refundedTotal (no gateway call)
else status = authorized or submitted_for_settlement
ImportProvider->>Gateway: transaction.void(transactionId)
Gateway-->>ImportProvider: void result
ImportProvider-->>Client: refundedTotal updated
else status = settling/settled
ImportProvider->>Gateway: transaction.refund(transactionId, amount)
Gateway-->>ImportProvider: refund result
ImportProvider-->>Client: refundedTotal updated
else other statuses
ImportProvider-->>Client: error (non-refundable)
end
sequenceDiagram
participant Client
participant Base as BraintreeBase
participant Gateway as Braintree Gateway
Client->>Base: authorizePayment(session, amount, autoCapture?)
Base->>Base: formatToTwoDecimalString(amount)
Base->>Gateway: transaction.sale({...})
Gateway-->>Base: result (authorized or settled)
alt auto-capture enabled and authorized
Base->>Gateway: transaction.submitForSettlement(txId, amount)
Gateway-->>Base: settlement submission result
end
Base-->>Client: status mapped via mapTransactionStatusToPaymentStatus
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewersTip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 4
🧹 Nitpick comments (11)
plugins/braintree-payment/package.json (1)
58-58
: Optional: expand test scripts for DX.
Consider adding watch and coverage variants to speed up local workflows.You can extend the scripts like:
{ "scripts": { "test": "jest --config jest.config.cjs --runInBand", "test:watch": "jest --config jest.config.cjs --watch", "test:cov": "jest --config jest.config.cjs --coverage" } }plugins/braintree-payment/src/providers/payment-braintree/src/types/index.ts (1)
14-17
: Make PaymentProviderKeys literal and expose a type alias for safer usage.
Const-assert the map so consumers get a literal union of values; optionally export a type alias to centralize constraints.Apply:
-export const PaymentProviderKeys = { - BRAINTREE: 'braintree', - IMPORTED: 'imported', -}; +export const PaymentProviderKeys = { + BRAINTREE: 'braintree', + IMPORTED: 'imported', +} as const;Then add this type alias just below:
export type PaymentProviderKey = typeof PaymentProviderKeys[keyof typeof PaymentProviderKeys];plugins/braintree-payment/jest.config.cjs (1)
6-7
: Simplify testMatch to avoid redundant patterns.Both patterns currently match the same files; Jest will dedupe, but one pattern is sufficient.
Apply:
- testMatch: ['**/__tests__/**/*.spec.ts', '**/*.spec.ts'], + testMatch: ['**/*.spec.ts'],plugins/braintree-payment/src/providers/payment-braintree/src/core/__tests__/braintree-base.spec.ts (3)
85-111
: Strengthen auto-capture assertions in authorizePayment test.Given autoCapture: true, assert that submitForSettlement was invoked to make the test intention explicit.
Apply:
const result = await service.authorizePayment(input); expect(gateway.transaction.sale).toHaveBeenCalled(); const saleArgs = gateway.transaction.sale.mock.calls[0][0]; expect(saleArgs.amount).toBe('10.00'); expect(result.status).toBe('captured'); + expect(gateway.transaction.submitForSettlement).toHaveBeenCalled();
115-135
: Align test input units in capturePayment for clarity.input.data.amount is 1000 while expectations use '10.00'. To avoid confusion about units, make the input amount 10 to mirror the expected formatted value.
Apply:
- amount: 1000, + amount: 10,
262-279
: Verify webhook parsing invocation and input shape.The test passes a combined query string. If getWebhookActionAndData expects signature and payload separately (common in Braintree), add an assertion to ensure parse is called as intended, or adjust the input shape accordingly.
For example, you could add:
const result = await service.getWebhookActionAndData({ data: payloadStr } as any); expect(result.action).toBe('captured'); expect((result as any).data.session_id).toBe('sess_123'); +expect(gateway.webhookNotification.parse).toHaveBeenCalled();
If the method actually expects separate args, consider parsing payloadStr before calling and updating the service accordingly.
plugins/braintree-payment/src/providers/payment-braintree/src/services/braintree-provider.ts (1)
2-4
: Minor import cleanup (combine types and values).Not required, but this reduces duplicate import lines from the same module.
Apply:
-import type { BraintreeOptions } from '../types'; -import { PaymentProviderKeys } from '../types'; +import { PaymentProviderKeys, type BraintreeOptions } from '../types';plugins/braintree-payment/src/providers/payment-braintree/src/services/braintree-import.ts (1)
1-4
: Minor import consolidation.Optional style tweak to merge imports from ../types.
Apply:
-import type { BraintreeOptions } from '../types'; -import { PaymentProviderKeys } from '../types'; +import { PaymentProviderKeys, type BraintreeOptions } from '../types';plugins/braintree-payment/src/providers/payment-braintree/src/core/__tests__/braintree-import.spec.ts (1)
5-36
: Consider extracting test setup to a shared test utility.The test helper function
buildService
follows a good pattern, but similar setup code might be needed across multiple test files. Consider extracting this to a shared test utility file for better reusability.plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts (1)
88-94
: Potential issue with TTL calculation edge case.The TTL calculation could result in a negative value if
expiresOnEpochSeconds
is in the past, but the conditionttlSeconds <= 0
on line 92 prevents caching. However, line 92 also checks!customerId || !clientToken
which are already validated. Consider simplifying the logic.async saveClientTokenToCache(clientToken: string, customerId: string, expiresOnEpochSeconds: number): Promise<void> { if (!customerId) throw new MedusaError(MedusaError.Types.INVALID_ARGUMENT, 'Customer ID is required'); const nowSeconds = Math.floor(Date.now() / 1000); const ttlSeconds = expiresOnEpochSeconds - nowSeconds - 1; - if (!customerId || !clientToken || ttlSeconds <= 0) return; + if (ttlSeconds <= 0) return; await this.cache.set(buildTokenCacheKey(customerId), clientToken, ttlSeconds); }plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-import.ts (1)
221-223
: Consider implementing webhook support for imported payments.The method returns
NOT_SUPPORTED
for all webhook actions. Consider whether webhook handling might be needed for tracking refund status of imported payments that undergo real refunds.Would you like me to help implement webhook handling for real refund operations on imported payments?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (15)
.vscode/settings.json
(1 hunks)plugins/braintree-payment/jest.config.cjs
(1 hunks)plugins/braintree-payment/package.json
(2 hunks)plugins/braintree-payment/src/providers/payment-braintree/src/core/__tests__/braintree-base.spec.ts
(1 hunks)plugins/braintree-payment/src/providers/payment-braintree/src/core/__tests__/braintree-import.spec.ts
(1 hunks)plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts
(15 hunks)plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-import.ts
(1 hunks)plugins/braintree-payment/src/providers/payment-braintree/src/index.ts
(1 hunks)plugins/braintree-payment/src/providers/payment-braintree/src/services/braintree-import.ts
(1 hunks)plugins/braintree-payment/src/providers/payment-braintree/src/services/braintree-provider.ts
(1 hunks)plugins/braintree-payment/src/providers/payment-braintree/src/services/index.ts
(1 hunks)plugins/braintree-payment/src/providers/payment-braintree/src/types/index.ts
(1 hunks)plugins/braintree-payment/src/providers/payment-braintree/src/utils/__tests__/get-smallest-unit.spec.ts
(0 hunks)plugins/braintree-payment/src/providers/payment-braintree/src/utils/get-smallest-unit.ts
(0 hunks)plugins/braintree-payment/tsconfig.spec.json
(1 hunks)
💤 Files with no reviewable changes (2)
- plugins/braintree-payment/src/providers/payment-braintree/src/utils/tests/get-smallest-unit.spec.ts
- plugins/braintree-payment/src/providers/payment-braintree/src/utils/get-smallest-unit.ts
🧰 Additional context used
🧬 Code Graph Analysis (6)
plugins/braintree-payment/src/providers/payment-braintree/src/services/braintree-import.ts (2)
plugins/braintree-payment/src/providers/payment-braintree/src/types/index.ts (2)
PaymentProviderKeys
(14-17)BraintreeOptions
(3-12)plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts (1)
BraintreeConstructorArgs
(50-53)
plugins/braintree-payment/src/providers/payment-braintree/src/core/__tests__/braintree-import.spec.ts (1)
plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts (2)
BraintreeConstructorArgs
(50-53)init
(136-154)
plugins/braintree-payment/src/providers/payment-braintree/src/core/__tests__/braintree-base.spec.ts (1)
plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts (2)
BraintreeConstructorArgs
(50-53)BraintreePaymentSessionData
(55-63)
plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-import.ts (2)
plugins/braintree-payment/src/providers/payment-braintree/src/types/index.ts (2)
BraintreeOptions
(3-12)PaymentProviderKeys
(14-17)plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts (1)
BraintreeConstructorArgs
(50-53)
plugins/braintree-payment/src/providers/payment-braintree/src/services/braintree-provider.ts (2)
plugins/braintree-payment/src/providers/payment-braintree/src/types/index.ts (2)
PaymentProviderKeys
(14-17)BraintreeOptions
(3-12)plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts (1)
BraintreeConstructorArgs
(50-53)
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 (24)
plugins/braintree-payment/package.json (1)
3-3
: Version bump to 0.0.9 looks good.
No concerns from me on the semver bump itself..vscode/settings.json (1)
12-15
: Editor spell-check entry LGTM.
Adding “Braintree” to cSpell reduces noise in reviews; no runtime impact.plugins/braintree-payment/tsconfig.spec.json (1)
1-25
: Test tsconfig looks solid.
Settings are consistent with ts-jest + Node16 modules; includes and excludes make sense for this plugin.plugins/braintree-payment/src/providers/payment-braintree/src/services/index.ts (2)
2-2
: Exporting BraintreeImportService: LGTM.
Publicly exposing the import service aligns with the new provider flow.
1-2
: BraintreeImportService registration verified
BraintreeImportService is correctly imported and included in theservices
array withinplugins/braintree-payment/src/providers/payment-braintree/src/index.ts
, ensuring it’s discoverable at runtime.plugins/braintree-payment/src/providers/payment-braintree/src/index.ts (2)
2-5
: Wiring looks correct: new Import service is registered.Importing BraintreeImportService and adding it to services alongside BraintreeProviderService is the right move for exposing the new provider.
2-2
: Re-exports and static identifiers verified
- Both services are exported in
services/index.ts
:
export { default as BraintreeProviderService } from './braintree-provider';
export { default as BraintreeImportService } from './braintree-import';
- Each class defines a distinct
static identifier
:
BraintreeProviderService
:PaymentProviderKeys.BRAINTREE
BraintreeImportService
:PaymentProviderKeys.IMPORTED
No changes required.
plugins/braintree-payment/src/providers/payment-braintree/src/core/__tests__/braintree-base.spec.ts (1)
47-279
: Nice coverage of core flows and edge cases.Good breadth across token caching TTL, amount formatting, authorize/capture, refund/void paths, status mapping, and webhook handling.
plugins/braintree-payment/src/providers/payment-braintree/src/services/braintree-provider.ts (1)
1-12
: Constructor type update matches the new base; service identifier remains stable.Switching to BraintreeConstructorArgs keeps the contract aligned with the refactored base while preserving the provider key.
plugins/braintree-payment/src/providers/payment-braintree/src/services/braintree-import.ts (1)
6-14
: Import provider service class looks correct and mirrors the base provider.Static identifier, constructor forwarding, and options storage are consistent with the provider pattern.
plugins/braintree-payment/src/providers/payment-braintree/src/core/__tests__/braintree-import.spec.ts (4)
43-59
: Test coverage looks good for the simulated flow.The test properly validates the import provider's ability to simulate payment flows without API calls, which aligns with the PR's import functionality objectives.
61-69
: Well-structured test for the imported-as-refunded scenario.The test properly validates that refunds are simulated (no gateway calls) when
importedAsRefunded=true
, with correct arithmetic for the refunded total.
71-80
: Comprehensive void flow test with proper mocking.The test correctly validates the void operation for authorized transactions, including proper gateway mock setup and assertions.
82-91
: Good coverage of the refund flow with decimal handling.The test validates the refund operation for settled transactions, including proper decimal formatting and refunded total calculations.
plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts (6)
156-160
: Good addition of decimal formatting helper.The
formatToTwoDecimalString
method properly handles currency formatting for Braintree's requirements.
372-394
: Excellent centralization of status mapping logic.The
mapTransactionStatusToPaymentStatus
method consolidates status mapping in one place, improving maintainability and consistency across the codebase.
653-661
: Good conversion to synchronous updatePayment.The change from async to sync with immediate Promise resolution is appropriate for this operation that only updates local data.
663-672
: Well-structured account holder creation.The new
createAccountHolder
method properly delegates to the private helper and returns the appropriate structure.
674-708
: Comprehensive account holder update logic with proper error handling.The method includes validation, existence checks, field merging, and appropriate error handling.
782-798
: Clean extraction of customer creation logic.The private
createBraintreeCustomer
helper properly encapsulates the customer creation logic with error handling.plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-import.ts (4)
102-114
: Well-implemented payment initiation for imports.The method properly handles the import-specific data structure with appropriate defaults for the refunded state.
125-129
: Clean authorization implementation.The authorization properly updates the session status to AUTHORIZED.
153-214
: Excellent refund logic with proper branching.The refund implementation correctly handles:
- Simulated refunds for imported-as-refunded transactions
- Void operations for authorized/submitted_for_settlement status
- Actual refunds for settled/settling status
The decimal rounding and error handling are also properly implemented.
145-147
: Confirm imported-customer handling in createAccountHolderAn initial search found no references to this method in import flows. The current implementation simply returns the existing Braintree customer ID (
input.context.customer.id
) without calling any Braintree API:
- File: plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-import.ts (Lines 145–147)
Please verify that for imported payments we indeed only need to echo back the existing customer ID. If a new Braintree customer must be created here, update this method to invoke the appropriate Braintree API.
"@types/jest": "^29.5.12", | ||
"@types/jsonwebtoken": "^9.0.10", | ||
"jsonwebtoken": "^9.0.2" | ||
"jest": "^29.7.0", | ||
"jsonwebtoken": "^9.0.2", | ||
"ts-jest": "^29.2.5" | ||
}, |
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.
💡 Verification agent
🧩 Analysis chain
Confirm TypeScript is available for ts-jest to work.
ts-jest requires TypeScript in the dependency graph. I don’t see "typescript" declared in this package; if it’s only at the repo root, that’s fine for a monorepo, but worth verifying to avoid runtime transform errors.
Run this script to confirm TypeScript is present somewhere in the workspace and check jest/ts-jest versions:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for TypeScript in any package.json..."
rg -nP -C2 --no-ignore-vcs --glob 'package.json' '\b"typescript"\s*:\s*"' || true
echo
echo "Checking plugin test tooling versions from plugins/braintree-payment/package.json..."
if command -v jq >/dev/null 2>&1; then
jq '{ jest: .devDependencies.jest, tsjest: .devDependencies["ts-jest"], typescript: .devDependencies.typescript }' plugins/braintree-payment/package.json
else
sed -n '1,120p' plugins/braintree-payment/package.json | rg -n '("jest"|"ts-jest"|"typescript")\s*:\s*"[^"]+"'
fi
echo
echo "Verifying jest config presence..."
rg -n -C2 --type-add 'cjs:*.cjs' --type cjs 'module\.exports|export' plugins/braintree-payment/jest.config.cjs || fd -a jest.config.cjs plugins/braintree-payment || true
Length of output: 943
Ensure TypeScript is declared for ts-jest
We’re using the ts-jest
preset in plugins/braintree-payment/jest.config.cjs
, but I don’t see typescript
in this package’s dependencies. ts-jest requires TypeScript in the dependency graph to compile tests.
Please update:
- File: plugins/braintree-payment/package.json
• UnderdevDependencies
, add
"typescript": "<your repo’s TS version>"
or confirm that the root workspace’stypescript
is installed and will be hoisted into this package to avoid runtime transform failures.
🤖 Prompt for AI Agents
In plugins/braintree-payment/package.json around lines 50-55, ts-jest is used
but TypeScript is not declared; add "typescript" to this package's
devDependencies using the repository's TypeScript version (or confirm the root
workspace's typescript will be hoisted into this package), then run install to
ensure ts-jest can resolve the compiler; update package.json devDependencies
accordingly and commit the change.
private validateInitiatePaymentData(data: Record<string, unknown>): BraintreeInitiatePaymentData { | ||
const schema = z.object({ | ||
transactionId: z.string().optional(), | ||
previouslyRefundedAmount: z.number().optional(), | ||
paymentMethodNonce: z.string().optional(), | ||
}); | ||
|
||
const result = schema.safeParse(data); | ||
|
||
if (!result.success) { | ||
throw new MedusaError(MedusaError.Types.INVALID_ARGUMENT, result.error.message); | ||
} | ||
|
||
return result.data; | ||
} |
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.
Schema validation is thorough but has an inconsistency.
The schema includes previouslyRefundedAmount
but it's not used in the BraintreeInitiatePaymentData
interface.
private validateInitiatePaymentData(data: Record<string, unknown>): BraintreeInitiatePaymentData {
const schema = z.object({
transactionId: z.string().optional(),
- previouslyRefundedAmount: z.number().optional(),
paymentMethodNonce: z.string().optional(),
});
📝 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.
private validateInitiatePaymentData(data: Record<string, unknown>): BraintreeInitiatePaymentData { | |
const schema = z.object({ | |
transactionId: z.string().optional(), | |
previouslyRefundedAmount: z.number().optional(), | |
paymentMethodNonce: z.string().optional(), | |
}); | |
const result = schema.safeParse(data); | |
if (!result.success) { | |
throw new MedusaError(MedusaError.Types.INVALID_ARGUMENT, result.error.message); | |
} | |
return result.data; | |
} | |
private validateInitiatePaymentData(data: Record<string, unknown>): BraintreeInitiatePaymentData { | |
const schema = z.object({ | |
transactionId: z.string().optional(), | |
paymentMethodNonce: z.string().optional(), | |
}); | |
const result = schema.safeParse(data); | |
if (!result.success) { | |
throw new MedusaError(MedusaError.Types.INVALID_ARGUMENT, result.error.message); | |
} | |
return result.data; | |
} |
🤖 Prompt for AI Agents
In
plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts
around lines 396 to 410, the Zod schema validates a previouslyRefundedAmount
field that is not present on the BraintreeInitiatePaymentData interface; update
the types to match the runtime schema by adding previouslyRefundedAmount?:
number to the BraintreeInitiatePaymentData interface (or, if that field is not
needed, remove it from the schema and adjust any call sites), and ensure any
code that consumes BraintreeInitiatePaymentData handles the optional number
accordingly.
constructor(container: BraintreeConstructorArgs, options: BraintreeOptions) { | ||
super(container, options); | ||
this.options = options; | ||
// Create a lightweight Braintree gateway for refund operations only | ||
const envKey = (this.options.environment || 'sandbox').toLowerCase(); | ||
const envMap: Record<string, Braintree.Environment> = { | ||
qa: Braintree.Environment.Qa, | ||
sandbox: Braintree.Environment.Sandbox, | ||
production: Braintree.Environment.Production, | ||
development: Braintree.Environment.Development, | ||
}; | ||
const environment = envMap[envKey] ?? Braintree.Environment.Sandbox; | ||
|
||
this.gateway = new Braintree.BraintreeGateway({ | ||
environment, | ||
merchantId: this.options.merchantId!, | ||
publicKey: this.options.publicKey!, | ||
privateKey: this.options.privateKey!, | ||
}); | ||
} |
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.
🛠️ Refactor suggestion
Duplicate gateway initialization code.
The gateway initialization logic is duplicated from braintree-base.ts
. Consider extracting this into a shared utility function to maintain DRY principles.
Create a shared utility function:
// In a shared utilities file
export function createBraintreeGateway(options: BraintreeOptions): Braintree.BraintreeGateway {
const envKey = (options.environment || 'sandbox').toLowerCase();
const envMap: Record<string, Braintree.Environment> = {
qa: Braintree.Environment.Qa,
sandbox: Braintree.Environment.Sandbox,
production: Braintree.Environment.Production,
development: Braintree.Environment.Development,
};
const environment = envMap[envKey] ?? Braintree.Environment.Sandbox;
return new Braintree.BraintreeGateway({
environment,
merchantId: options.merchantId!,
publicKey: options.publicKey!,
privateKey: options.privateKey!,
});
}
Then use it in both classes:
- // Create a lightweight Braintree gateway for refund operations only
- const envKey = (this.options.environment || 'sandbox').toLowerCase();
- const envMap: Record<string, Braintree.Environment> = {
- qa: Braintree.Environment.Qa,
- sandbox: Braintree.Environment.Sandbox,
- production: Braintree.Environment.Production,
- development: Braintree.Environment.Development,
- };
- const environment = envMap[envKey] ?? Braintree.Environment.Sandbox;
-
- this.gateway = new Braintree.BraintreeGateway({
- environment,
- merchantId: this.options.merchantId!,
- publicKey: this.options.publicKey!,
- privateKey: this.options.privateKey!,
- });
+ this.gateway = createBraintreeGateway(this.options);
private formatToTwoDecimalString(amount: number): string { | ||
const rounded = Math.round(Number(amount) * 100) / 100; | ||
return rounded.toFixed(2); | ||
} |
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.
🛠️ Refactor suggestion
Duplicate decimal formatting helper.
The formatToTwoDecimalString
method is duplicated from braintree-base.ts
. Consider making this a shared utility or protected method in a base class.
🤖 Prompt for AI Agents
In
plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-import.ts
around lines 97 to 100, the formatToTwoDecimalString method is a duplicate of
the helper in braintree-base.ts; remove this duplicate and instead reference a
single shared implementation by either (A) moving the helper into a shared util
module (e.g., plugins/braintree-payment/src/utils/number.ts) and importing it
here and in braintree-base.ts, or (B) making it a protected method on the common
base class and calling super.formatToTwoDecimalString(...) from subclasses;
update imports/usages and run tests to ensure no references remain to the
removed method.
); | ||
} | ||
|
||
const braintreeTransaction = await this.gateway.transaction.find(transactionId); |
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.
Suggestion: put in a try/catch
const { transaction: refundTransaction } = await this.gateway.transaction.refund( | ||
braintreeTransaction.id, | ||
refundAmountDecimal, | ||
); |
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.
Suggestion: put in try/catch
|
||
const shouldVoid = ['submitted_for_settlement', 'authorized'].includes(braintreeTransaction.status); | ||
if (shouldVoid) { | ||
const cancelledTransaction = await this.gateway.transaction.void(braintreeTransaction.id); |
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.
Suggestion: put in try/catch
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.
@dwene looks like code rabbit is suggesting a few ways to DRY it up. I'm glad you caught this and added tests.
Summary by CodeRabbit