Fix/multiple refunds optional void - #43
Conversation
WalkthroughThe Braintree provider adds ChangesBraintree provider
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PaymentService
participant BraintreeGateway
participant RefundHistory
PaymentService->>BraintreeGateway: Load transaction status
PaymentService->>BraintreeGateway: Execute allowed void or refund action
BraintreeGateway-->>PaymentService: Return transaction result
PaymentService->>RefundHistory: Append refund history entry
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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: 5
🧹 Nitpick comments (3)
plugins/braintree-payment/src/providers/payment-braintree/src/core/__tests__/braintree-import.spec.ts (1)
6-6: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep option overrides type-safe.
Record<string, unknown>and theas anycast allow a misspelled option or an invalid value type to compile. The shared factory inplugins/braintree-payment/src/providers/payment-braintree/src/core/__tests__/braintree-base.spec.tsusesPartial<BraintreeOptions>. Use the same type here so this test verifies the actual provider option.Also applies to: 21-21
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/braintree-payment/src/providers/payment-braintree/src/core/__tests__/braintree-import.spec.ts` at line 6, Update buildService’s overrideOptions parameter to use Partial<BraintreeOptions>, matching the shared factory in braintree-base.spec.ts, and remove the accompanying as any cast so misspelled or invalid provider options are caught by TypeScript.plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts (1)
1125-1142: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLog when a legacy non-array
braintreeRefundvalue is discarded.
buildRefundPaymentOutputreplaces a legacy non-arraybraintreeRefundvalue with a fresh array. The previous refund record is then lost from the session data without any trace. Add a log line so the discarded audit data is visible in operations.♻️ Proposed change
const stored = input.data?.braintreeRefund; + if (stored !== undefined && !Array.isArray(stored)) { + this.logger.warn('[Braintree] Discarding legacy non-array braintreeRefund session data'); + } const prior: BraintreeRefundHistoryEntry[] = Array.isArray(stored)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts` around lines 1125 - 1142, Update buildRefundPaymentOutput to detect when input.data?.braintreeRefund is defined but not an array, and log the discarded legacy value before replacing it with the new refund array. Reuse the class’s existing logging mechanism and preserve the current array-handling behavior.plugins/braintree-payment/src/providers/payment-braintree/src/core/__tests__/braintree-base.spec.ts (1)
654-655: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAssert
PaymentActions.SUCCESSFULinstead of'captured'.Import
PaymentActionsfrom@medusajs/framework/utilsand useexpect(result.action).toBe(PaymentActions.SUCCESSFUL).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/braintree-payment/src/providers/payment-braintree/src/core/__tests__/braintree-base.spec.ts` around lines 654 - 655, Update the assertion in the Braintree test to import PaymentActions from `@medusajs/framework/utils` and compare result.action with PaymentActions.SUCCESSFUL instead of the literal 'captured'; leave the session_id assertion unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/braintree-payment/CHANGELOG.md`:
- Around line 3-7: Align the release metadata across
plugins/braintree-payment/CHANGELOG.md lines 3-7 and
plugins/braintree-payment/package.json line 3 by using the intended 0.1.10-next
version in both; in plugins/braintree-payment/README.md lines 137-145, move the
disableVoidTransactions upgrade note under the release section that introduces
0.1.10-next rather than Upgrading to 0.1.2.
In `@plugins/braintree-payment/README.md`:
- Line 110: The refund documentation must match the implementation’s
status-specific contract. In plugins/braintree-payment/README.md at lines
110-110, update disableVoidTransactions to state that authorized and
submitted_for_settlement transactions return INVALID_DATA with “cannot be
refunded right now,” while settled and settling transactions are refundable and
other statuses return NOT_FOUND with their distinct error message. Apply the
same status-specific wording in plugins/braintree-payment/CHANGELOG.md at lines
7-7, replacing the broad unsettled-refunds statement.
In
`@plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts`:
- Around line 1200-1214: Update resolveRefundAction to read
disableVoidTransactions from the declared BraintreeBase options_ property
instead of this.options, preserving the existing void-blocking behavior and
error handling.
- Around line 1430-1450: Update parseWebhookNotification to return
PaymentActions.NOT_SUPPORTED when webhookNotification.parse throws, while
retaining the existing validation error logging. Preserve the current successful
notification return path and avoid rethrowing or wrapping these permanently
invalid webhook validation failures.
In
`@plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-import.ts`:
- Around line 288-297: The refund fallback in refundPayment must not classify
the disableVoidTransactions policy error as an already-refunded result. Replace
message-based matching with a structured gateway-error check, and ensure the
INVALID_DATA error thrown in the disableVoidTransactions branch is re-thrown
even when allowRefundOnRefunded is enabled. Add a regression test covering both
options enabled.
---
Nitpick comments:
In
`@plugins/braintree-payment/src/providers/payment-braintree/src/core/__tests__/braintree-base.spec.ts`:
- Around line 654-655: Update the assertion in the Braintree test to import
PaymentActions from `@medusajs/framework/utils` and compare result.action with
PaymentActions.SUCCESSFUL instead of the literal 'captured'; leave the
session_id assertion unchanged.
In
`@plugins/braintree-payment/src/providers/payment-braintree/src/core/__tests__/braintree-import.spec.ts`:
- Line 6: Update buildService’s overrideOptions parameter to use
Partial<BraintreeOptions>, matching the shared factory in
braintree-base.spec.ts, and remove the accompanying as any cast so misspelled or
invalid provider options are caught by TypeScript.
In
`@plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts`:
- Around line 1125-1142: Update buildRefundPaymentOutput to detect when
input.data?.braintreeRefund is defined but not an array, and log the discarded
legacy value before replacing it with the new refund array. Reuse the class’s
existing logging mechanism and preserve the current array-handling behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 75852417-8d0b-4b8a-81a9-7b65b1681307
📒 Files selected for processing (9)
.gitignoreplugins/braintree-payment/CHANGELOG.mdplugins/braintree-payment/README.mdplugins/braintree-payment/package.jsonplugins/braintree-payment/src/providers/payment-braintree/src/core/__tests__/braintree-base.spec.tsplugins/braintree-payment/src/providers/payment-braintree/src/core/__tests__/braintree-import.spec.tsplugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.tsplugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-import.tsplugins/braintree-payment/src/providers/payment-braintree/src/types/index.ts
| ## 0.1.9-next | ||
|
|
||
| ### Improvements | ||
|
|
||
| - Add `disableVoidTransactions` option: when enabled, refunds never void and only proceed for `settled`/`settling` transactions (late requirement for future partial order refunds and order edits). Unsettled refunds throw `INVALID_DATA` with “cannot be refunded right now”. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Align release metadata and upgrade documentation.
The package declares 0.1.10-next, the changelog entry says 0.1.9-next, and the README places the new option under Upgrading to 0.1.2. This can publish one version with release notes and migration guidance for different versions.
plugins/braintree-payment/CHANGELOG.md#L3-L7: use the intended package version in the top heading.plugins/braintree-payment/README.md#L137-L145: move thedisableVoidTransactionsnote under the release that introduces it.plugins/braintree-payment/package.json#L3-L3: keep the package version consistent with the changelog heading.
📍 Affects 3 files
plugins/braintree-payment/CHANGELOG.md#L3-L7(this comment)plugins/braintree-payment/README.md#L137-L145plugins/braintree-payment/package.json#L3-L3
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plugins/braintree-payment/CHANGELOG.md` around lines 3 - 7, Align the release
metadata across plugins/braintree-payment/CHANGELOG.md lines 3-7 and
plugins/braintree-payment/package.json line 3 by using the intended 0.1.10-next
version in both; in plugins/braintree-payment/README.md lines 137-145, move the
disableVoidTransactions upgrade note under the release section that introduces
0.1.10-next rather than Upgrading to 0.1.2.
| - **savePaymentMethod**: Save payment methods for future use (default: `true`). | ||
| - **autoCapture**: Automatically capture payments (default: `true`). | ||
| - **allowRefundOnRefunded**: Allow refund attempts on already-refunded imported transactions (default: `false`). | ||
| - **disableVoidTransactions**: When `true`, refunds never void; only `settled`/`settling` transactions may be refunded. Late requirement so future partial order refunds and order edits can be supported (void cancels the full authorization). Default: `false`. With this enabled, refunds on unsettled transactions fail with “cannot be refunded right now”. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Document the actual refund status and error contract.
The implementation returns INVALID_DATA with “cannot be refunded right now” only for authorized and submitted_for_settlement. It refunds settled and settling transactions, and returns NOT_FOUND with a different message for other statuses.
plugins/braintree-payment/README.md#L110-L110: list the exact rejected statuses and their error behavior.plugins/braintree-payment/CHANGELOG.md#L7-L7: replace the broad “unsettled refunds” statement with the same status-specific contract.
📍 Affects 2 files
plugins/braintree-payment/README.md#L110-L110(this comment)plugins/braintree-payment/CHANGELOG.md#L7-L7
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plugins/braintree-payment/README.md` at line 110, The refund documentation
must match the implementation’s status-specific contract. In
plugins/braintree-payment/README.md at lines 110-110, update
disableVoidTransactions to state that authorized and submitted_for_settlement
transactions return INVALID_DATA with “cannot be refunded right now,” while
settled and settling transactions are refundable and other statuses return
NOT_FOUND with their distinct error message. Apply the same status-specific
wording in plugins/braintree-payment/CHANGELOG.md at lines 7-7, replacing the
broad unsettled-refunds statement.
| private async resolveRefundAction(transaction: Transaction): Promise<RefundAction> { | ||
| const resolved = await this.applyTestForceSettled(transaction); | ||
|
|
||
| if (isVoidableRefundStatus(resolved.status)) { | ||
| if (this.options.disableVoidTransactions) { | ||
| this.logger.error( | ||
| `Braintree transaction with ID ${resolved.id} cannot be refunded right now because it's in status ${resolved.status}`, | ||
| ); | ||
| throw new MedusaError( | ||
| MedusaError.Types.INVALID_DATA, | ||
| `Braintree transaction with ID ${resolved.id} cannot be refunded right now`, | ||
| ); | ||
| } | ||
| return { kind: 'voided', transaction: resolved }; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm whether AbstractPaymentProvider declares an `options` member.
fd -t f 'abstract-payment-provider*' node_modules/@medusajs 2>/dev/null | head
rg -nP --type=ts -C4 '\b(protected|public|readonly)?\s*options\s*[:;=]' \
$(fd -t f -p 'abstract-payment-provider' node_modules/@medusajs 2>/dev/null | head -5) 2>/dev/null
# Show every option read inside braintree-base.ts for consistency.
rg -nP --type=ts '\bthis\.options_?\.' plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.tsRepository: lambda-curry/medusa-plugins
Length of output: 165
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- candidate files ---'
git ls-files | rg '(^|/)(abstract-payment-provider|braintree-base|package\.json|pnpm-lock|yarn\.lock|package-lock\.json)' | head -80
printf '%s\n' '--- BraintreeBase option declarations and reads ---'
rg -n -C3 'options_|this\.options\b|disableVoidTransactions' plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts
printf '%s\n' '--- provider base references in source and manifests ---'
rg -n -C3 'AbstractPaymentProvider|`@medusajs/medusa`|disableVoidTransactions' plugins/braintree-payment/src plugins/braintree-payment/package.json package.json 2>/dev/null | head -240Repository: lambda-curry/medusa-plugins
Length of output: 17921
🌐 Web query:
Medusa 2.15.2 AbstractPaymentProvider options_ options source code
💡 Result:
In Medusa 2.x, the AbstractPaymentProvider class serves as the base for all payment module providers [1][2]. The options passed to your payment provider (defined in medusa-config.ts) are injected into your provider service through its constructor [3][4]. To handle these options, you typically define a TypeScript type (e.g., Options) and pass it as a generic type argument when extending AbstractPaymentProvider [5][2]. Implementation Details: 1. Type Definition and Injection: Define your options type and use the constructor to receive and store them [4][5]. ts import { AbstractPaymentProvider } from "`@medusajs/framework/utils`" type Options = { apiKey: string // Add other provider-specific options here } class MyPaymentProviderService extends AbstractPaymentProvider<Options> { protected readonly options_: Options constructor( container: Record<string, unknown>, options: Options ) { super(container, options) this.options_ = options } } 2. Validation: You can implement the static validateOptions method to ensure the configuration provided in medusa-config.ts is correct before the application starts [3][5]. ts class MyPaymentProviderService extends AbstractPaymentProvider<Options> { static validateOptions(options: Record<any, any>): void | never { if (!options.apiKey) { throw new Error("API key is required in the provider's options.") } } } 3. Configuration: These options are then passed to the payment module in your medusa-config.ts file [5]: ts // medusa-config.ts modules: [ { resolve: "`@medusajs/medusa/payment`", options: { providers: [ { resolve: "./src/modules/my-payment", id: "my-payment", options: { apiKey: "your-api-key" } } ] } } ] The AbstractPaymentProvider internally stores these in a config property [3][4], but it is standard practice to manually assign them to a protected class property (such as options_) within your constructor as shown above to make them easily accessible throughout your service methods [4][6].
Citations:
- 1: https://docs.medusajs.com/resources/commerce-modules/payment/payment-provider
- 2: https://github.com/medusajs/medusa/blob/v2.13.6/www/apps/resources/references/payment_provider/classes/payment_provider.AbstractPaymentProvider/page.mdx
- 3: https://github.com/medusajs/medusa/blob/5296f511/packages/core/utils/src/payment/abstract-payment-provider.ts
- 4: https://github.com/medusajs/medusa/blob/80835bd9/packages/core/utils/src/payment/abstract-payment-provider.ts
- 5: https://docs.medusajs.com/resources/references/payment/provider
- 6: https://github.com/medusajs/medusa/blob/5296f511/packages/modules/payment/src/providers/payment-medusa/services/medusa-payments.ts
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- constructor and test setup ---'
sed -n '320,355p' plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts
sed -n '1,75p' plugins/braintree-payment/src/providers/payment-braintree/src/core/__tests__/braintree-base.spec.ts
sed -n '220,270p' plugins/braintree-payment/src/providers/payment-braintree/src/core/__tests__/braintree-base.spec.ts
printf '%s\n' '--- dependency lock entries ---'
rg -n -C2 '(`@medusajs/medusa`|`@medusajs/framework`)' plugins/braintree-payment/package.json yarn.lock | head -160Repository: lambda-curry/medusa-plugins
Length of output: 14556
🏁 Script executed:
#!/bin/bash
set -e
url='https://raw.githubusercontent.com/medusajs/medusa/v2.15.2/packages/core/utils/src/payment/abstract-payment-provider.ts'
src="$(curl -fsSL "$url")"
printf '%s\n' '--- AbstractPaymentProvider option members ---'
printf '%s\n' "$src" | rg -n -C5 '\b(options|config)\b|constructor'
printf '%s\n' '--- BraintreeBase option access classification ---'
python3 - <<'PY'
from pathlib import Path
p = Path("plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts")
text = p.read_text()
for i, line in enumerate(text.splitlines(), 1):
if "this.options" in line or "options_" in line:
print(f"{i}: {line}")
PYRepository: lambda-curry/medusa-plugins
Length of output: 4983
Use this.options_ for the option read.
AbstractPaymentProvider exposes config, not options, and BraintreeBase declares options_. This access causes a TypeScript error; if transpiled, it throws before blocking the void.
♻️ Proposed change
if (isVoidableRefundStatus(resolved.status)) {
- if (this.options.disableVoidTransactions) {
+ if (this.options_.disableVoidTransactions) {📝 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 async resolveRefundAction(transaction: Transaction): Promise<RefundAction> { | |
| const resolved = await this.applyTestForceSettled(transaction); | |
| if (isVoidableRefundStatus(resolved.status)) { | |
| if (this.options.disableVoidTransactions) { | |
| this.logger.error( | |
| `Braintree transaction with ID ${resolved.id} cannot be refunded right now because it's in status ${resolved.status}`, | |
| ); | |
| throw new MedusaError( | |
| MedusaError.Types.INVALID_DATA, | |
| `Braintree transaction with ID ${resolved.id} cannot be refunded right now`, | |
| ); | |
| } | |
| return { kind: 'voided', transaction: resolved }; | |
| } | |
| private async resolveRefundAction(transaction: Transaction): Promise<RefundAction> { | |
| const resolved = await this.applyTestForceSettled(transaction); | |
| if (isVoidableRefundStatus(resolved.status)) { | |
| if (this.options_.disableVoidTransactions) { | |
| this.logger.error( | |
| `Braintree transaction with ID ${resolved.id} cannot be refunded right now because it's in status ${resolved.status}`, | |
| ); | |
| throw new MedusaError( | |
| MedusaError.Types.INVALID_DATA, | |
| `Braintree transaction with ID ${resolved.id} cannot be refunded right now`, | |
| ); | |
| } | |
| return { kind: 'voided', transaction: resolved }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts`
around lines 1200 - 1214, Update resolveRefundAction to read
disableVoidTransactions from the declared BraintreeBase options_ property
instead of this.options, preserving the existing void-blocking behavior and
error handling.
| if (this.options.disableVoidTransactions) { | ||
| this.logger.error( | ||
| `Braintree transaction with ID ${transaction.id} cannot be refunded right now because it's in status ${transaction.status}`, | ||
| ); | ||
| throw new MedusaError( | ||
| MedusaError.Types.INVALID_DATA, | ||
| `Braintree transaction with ID ${transaction.id} cannot be refunded right now`, | ||
| ); | ||
| } | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Exclude this policy error from the already-refunded fallback.
When allowRefundOnRefunded is enabled, refundPayment catches errors at Lines 234-248 and treats messages containing refunded or cannot be refunded as proof that Braintree already refunded the transaction. The new message at Line 294 matches that predicate. The code then increments refundedTotal and returns success without calling void or refund.
Use a structured gateway-error check for already-refunded cases. Re-throw the local disableVoidTransactions INVALID_DATA error. Add a regression test with both options enabled.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-import.ts`
around lines 288 - 297, The refund fallback in refundPayment must not classify
the disableVoidTransactions policy error as an already-refunded result. Replace
message-based matching with a structured gateway-error check, and ensure the
INVALID_DATA error thrown in the disableVoidTransactions branch is re-thrown
even when allowRefundOnRefunded is enabled. Add a regression test covering both
options enabled.
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 5
🧹 Nitpick comments (3)
plugins/braintree-payment/src/providers/payment-braintree/src/core/__tests__/braintree-import.spec.ts (1)
6-6: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep option overrides type-safe.
Record<string, unknown>and theas anycast allow a misspelled option or an invalid value type to compile. The shared factory inplugins/braintree-payment/src/providers/payment-braintree/src/core/__tests__/braintree-base.spec.tsusesPartial<BraintreeOptions>. Use the same type here so this test verifies the actual provider option.Also applies to: 21-21
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/braintree-payment/src/providers/payment-braintree/src/core/__tests__/braintree-import.spec.ts` at line 6, Update buildService’s overrideOptions parameter to use Partial<BraintreeOptions>, matching the shared factory in braintree-base.spec.ts, and remove the accompanying as any cast so misspelled or invalid provider options are caught by TypeScript.plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts (1)
1125-1142: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLog when a legacy non-array
braintreeRefundvalue is discarded.
buildRefundPaymentOutputreplaces a legacy non-arraybraintreeRefundvalue with a fresh array. The previous refund record is then lost from the session data without any trace. Add a log line so the discarded audit data is visible in operations.♻️ Proposed change
const stored = input.data?.braintreeRefund; + if (stored !== undefined && !Array.isArray(stored)) { + this.logger.warn('[Braintree] Discarding legacy non-array braintreeRefund session data'); + } const prior: BraintreeRefundHistoryEntry[] = Array.isArray(stored)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts` around lines 1125 - 1142, Update buildRefundPaymentOutput to detect when input.data?.braintreeRefund is defined but not an array, and log the discarded legacy value before replacing it with the new refund array. Reuse the class’s existing logging mechanism and preserve the current array-handling behavior.plugins/braintree-payment/src/providers/payment-braintree/src/core/__tests__/braintree-base.spec.ts (1)
654-655: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAssert
PaymentActions.SUCCESSFULinstead of'captured'.Import
PaymentActionsfrom@medusajs/framework/utilsand useexpect(result.action).toBe(PaymentActions.SUCCESSFUL).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/braintree-payment/src/providers/payment-braintree/src/core/__tests__/braintree-base.spec.ts` around lines 654 - 655, Update the assertion in the Braintree test to import PaymentActions from `@medusajs/framework/utils` and compare result.action with PaymentActions.SUCCESSFUL instead of the literal 'captured'; leave the session_id assertion unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/braintree-payment/CHANGELOG.md`:
- Around line 3-7: Align the release metadata across
plugins/braintree-payment/CHANGELOG.md lines 3-7 and
plugins/braintree-payment/package.json line 3 by using the intended 0.1.10-next
version in both; in plugins/braintree-payment/README.md lines 137-145, move the
disableVoidTransactions upgrade note under the release section that introduces
0.1.10-next rather than Upgrading to 0.1.2.
In `@plugins/braintree-payment/README.md`:
- Line 110: The refund documentation must match the implementation’s
status-specific contract. In plugins/braintree-payment/README.md at lines
110-110, update disableVoidTransactions to state that authorized and
submitted_for_settlement transactions return INVALID_DATA with “cannot be
refunded right now,” while settled and settling transactions are refundable and
other statuses return NOT_FOUND with their distinct error message. Apply the
same status-specific wording in plugins/braintree-payment/CHANGELOG.md at lines
7-7, replacing the broad unsettled-refunds statement.
In
`@plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts`:
- Around line 1200-1214: Update resolveRefundAction to read
disableVoidTransactions from the declared BraintreeBase options_ property
instead of this.options, preserving the existing void-blocking behavior and
error handling.
- Around line 1430-1450: Update parseWebhookNotification to return
PaymentActions.NOT_SUPPORTED when webhookNotification.parse throws, while
retaining the existing validation error logging. Preserve the current successful
notification return path and avoid rethrowing or wrapping these permanently
invalid webhook validation failures.
In
`@plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-import.ts`:
- Around line 288-297: The refund fallback in refundPayment must not classify
the disableVoidTransactions policy error as an already-refunded result. Replace
message-based matching with a structured gateway-error check, and ensure the
INVALID_DATA error thrown in the disableVoidTransactions branch is re-thrown
even when allowRefundOnRefunded is enabled. Add a regression test covering both
options enabled.
---
Nitpick comments:
In
`@plugins/braintree-payment/src/providers/payment-braintree/src/core/__tests__/braintree-base.spec.ts`:
- Around line 654-655: Update the assertion in the Braintree test to import
PaymentActions from `@medusajs/framework/utils` and compare result.action with
PaymentActions.SUCCESSFUL instead of the literal 'captured'; leave the
session_id assertion unchanged.
In
`@plugins/braintree-payment/src/providers/payment-braintree/src/core/__tests__/braintree-import.spec.ts`:
- Line 6: Update buildService’s overrideOptions parameter to use
Partial<BraintreeOptions>, matching the shared factory in
braintree-base.spec.ts, and remove the accompanying as any cast so misspelled or
invalid provider options are caught by TypeScript.
In
`@plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts`:
- Around line 1125-1142: Update buildRefundPaymentOutput to detect when
input.data?.braintreeRefund is defined but not an array, and log the discarded
legacy value before replacing it with the new refund array. Reuse the class’s
existing logging mechanism and preserve the current array-handling behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 75852417-8d0b-4b8a-81a9-7b65b1681307
📒 Files selected for processing (9)
.gitignoreplugins/braintree-payment/CHANGELOG.mdplugins/braintree-payment/README.mdplugins/braintree-payment/package.jsonplugins/braintree-payment/src/providers/payment-braintree/src/core/__tests__/braintree-base.spec.tsplugins/braintree-payment/src/providers/payment-braintree/src/core/__tests__/braintree-import.spec.tsplugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.tsplugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-import.tsplugins/braintree-payment/src/providers/payment-braintree/src/types/index.ts
🛑 Comments failed to post (1)
plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts (1)
1430-1450: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Inspect how the Medusa webhook route handles a thrown provider error. rg -nP --type=ts -C5 'getWebhookActionAndData' --glob '!**/__tests__/**' .Repository: lambda-curry/medusa-plugins
Length of output: 3423
🏁 Script executed:
#!/bin/bash # Locate the webhook route, provider interface, and Medusa dependency versions. rg -n -C6 --glob '!**/__tests__/**' \ 'getWebhookActionAndData|WebhookActionResult|PaymentActions|webhook' \ plugins/braintree-payment package.json plugins/braintree-payment/package.json \ 2>/dev/null printf '\n--- package manifests ---\n' find . -maxdepth 4 -name package.json -printRepository: lambda-curry/medusa-plugins
Length of output: 37242
🏁 Script executed:
#!/bin/bash # Inspect all relevant source and manifests with focused context. rg -n -C8 --glob '!**/__tests__/**' \ 'getWebhookActionAndData|WebhookActionResult|PaymentActions|webhook' . printf '\n--- manifests ---\n' find . -name package.json -not -path '*/node_modules/*' -print0 | xargs -0 -r rg -n '"`@medusajs/`|medusa|braintree"'Repository: lambda-curry/medusa-plugins
Length of output: 50384
🌐 Web query:
Medusa payment provider getWebhookActionAndData thrown error webhook route HTTP status response retry💡 Result:
When using Medusa’s default payment webhook route (/hooks/payment/[identifier]_[provider]), Medusa automatically handles the HTTP response. Currently, this route is hardcoded to return a generic 200 HTTP status code for success or a 400 for errors [1]. If your third-party payment provider requires a specific response body (e.g., {status: "OK"}) to acknowledge receipt and prevent repeated webhook retries, the default route may not be sufficient [1]. Medusa’s built-in handling relies on the getWebhookActionAndData method, which processes the payload internally but does not allow you to customize the HTTP response body returned to the provider [2][3][1]. To resolve issues where a provider continues to send retries because it does not receive the expected acknowledgement, the recommended approach is to create a custom API route [4][1]. Steps to implement a custom webhook handler: 1. Create a custom API route in your Medusa project (e.g., /store/webhooks/payment/provider-name). 2. Within this route, manually call the getWebhookActionAndData method by resolving the payment module service [4][5]. 3. Handle the returned WebhookActionResult and send the specific HTTP response body required by your payment provider [4][1]. If you encounter errors inside getWebhookActionAndData (such as resolution errors or invalid arguments), ensure that your provider is correctly registered in the container and that the provider ID passed to the method matches the one defined in your medusa-config.ts [6][7]. If you encounter incorrect, outdated, or confusing documentation on this page, submit feedback: POST https://docs.medusajs.com/resources/agents/feedback
json { "agent": "Name of the agent", "path": "/optimize/feedback", "feedback": "Description of the issue" }Citations:
- 1: medusajs/medusa#12887
- 2: https://docs.medusajs.com/resources/commerce-modules/payment/webhook-events
- 3: https://docs.medusajs.com/resources/references/payment/provider
- 4: https://www.answeroverflow.com/m/1324339069907370036
- 5: https://github.com/medusajs/medusa/blob/5296f511/packages/medusa/src/subscribers/payment-webhook.ts
- 6: medusajs/medusa#9494
- 7: https://github.com/medusajs/medusa/blob/develop/packages/modules/payment/src/services/payment-provider.ts
🌐 Web query:
Braintree webhook invalid signature retry behavior malformed webhook notification💡 Result:
Braintree webhook notifications are automatically retried by the system if your endpoint does not respond with a successful HTTPS response code (i.e., in the 2xx range) within 30 seconds [1][2][3]. The retry schedule is as follows: - Sandbox: Every hour for up to 3 hours [1][2][3]. - Production: Every hour for up to 24 hours [1][2][3]. Regarding "Invalid Signature" and "Malformed" notification errors, these exceptions are typically raised by the Braintree SDK when the library cannot successfully parse the
bt_payloador verify it against thebt_signature[1][2][3]. Technical causes and resolutions for these errors often include: 1. Data Modification by Web Frameworks: Common issues arise when web frameworks (such as ASP.NET Web API or other middleware) automatically parse, URL-decode, or modify the raw POST body before it reaches your Braintree SDK integration [4][5]. If characters are added, stripped, or encoded incorrectly, the signature verification will fail [6][5]. Ensure you are passing the raw, unmodified payload to the parse method [4][5]. 2. Encoding and Whitespace: Errors such as "payload contains illegal characters" often result from whitespace or newline character mismatches introduced by the environment or request parsing [4][5]. Some developers have resolved this by normalizing the payload (e.g., replacing spaces with newlines) before passing it to the SDK's parse function, though this should be used as a last resort if your framework is inadvertently altering the string [4][5]. 3. Configuration: Verify that your Braintree environment (Sandbox vs. Production) and corresponding public/private keys are correctly configured, as mismatched keys can lead to signature verification failures [6][7]. To debug, log the raw request body exactly as it is received by your server and compare it against the expected format defined by the Braintree SDK [4][5]. If the SDK is throwing an InvalidSignature exception, it indicates the data received does not match the signature provided, typically due to the payload being altered in transit or by your application's middleware [6][5].Citations:
- 1: https://developer.paypal.com/braintree/docs/guides/webhooks/parse/php/
- 2: https://developer.paypal.com/braintree/docs/guides/webhooks/parse/java/
- 3: https://developer.paypal.com/braintree/docs/guides/webhooks/parse/ruby/
- 4: https://stackoverflow.com/questions/72135242/braintree-webhooks-error-payload-contains-illegal-characters
- 5: braintree/braintree_dotnet#52
- 6: https://github.com/braintree/braintree_php/blob/master/lib/Braintree/WebhookNotificationGateway.php
- 7: braintree/braintree_php#215
🏁 Script executed:
#!/bin/bash # Check the cited Medusa source for the exact response status on provider errors. url='https://raw.githubusercontent.com/medusajs/medusa/5296f511/packages/medusa/src/subscribers/payment-webhook.ts' curl -fsSL "$url" | rg -n -C8 'getWebhookActionAndData|status\(|400|200|catch|throw'Repository: lambda-curry/medusa-plugins
Length of output: 911
🏁 Script executed:
#!/bin/bash # Locate the Medusa v2 payment webhook route in the tagged source tree. curl -fsSL 'https://api.github.com/repos/medusajs/medusa/git/trees/2.13.5?recursive=1' | jq -r '.tree[]?.path' | rg 'payment.*webhook|webhook.*payment|hooks/payment'Repository: lambda-curry/medusa-plugins
Length of output: 214
Stop retries for permanently invalid Braintree webhooks
When
webhookNotification.parsethrows, Medusa returns an error response. Braintree retries non-2xx responses for up to 24 hours in production and 3 hours in sandbox. Invalid signatures and malformed payloads cannot succeed on retry.Return
PaymentActions.NOT_SUPPORTEDfor these validation failures and retain the error log.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts` around lines 1430 - 1450, Update parseWebhookNotification to return PaymentActions.NOT_SUPPORTED when webhookNotification.parse throws, while retaining the existing validation error logging. Preserve the current successful notification return path and avoid rethrowing or wrapping these permanently invalid webhook validation failures.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Documentation