fix: braintree refunds#40
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
WalkthroughThis PR centralizes Braintree refund and void failure handling into shared helpers, adds sandbox-only ChangesBraintree refund handling refactor
Estimated code review effort: 4 (Complex) | ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 3
🧹 Nitpick comments (1)
.gitignore (1)
19-19: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate
.gitignoreentry.
*.tgzis already present at line 9; this new entry at line 19 is redundant.🤖 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 @.gitignore at line 19, Remove the redundant `*.tgz` ignore rule from `.gitignore` so the pattern is only listed once; keep the existing entry and delete the duplicate added later.
🤖 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/package.json`:
- Around line 65-73: The Medusa peer dependency range is inconsistent in the
braintree-payment package manifest: `@medusajs/medusa` is pinned while the other
Medusa peers use a caret range. Update the `@medusajs/medusa` entry in
package.json to match the existing ^2.15.2 pattern used by `@medusajs/admin-sdk`,
`@medusajs/framework`, `@medusajs/icons`, `@medusajs/test-utils`, and `@medusajs/ui` so
consumers can install adjacent compatible Medusa versions.
In
`@plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts`:
- Around line 287-297: The refund helpers `logRefundTrace` and `logRefundJson`
currently log at `info` unconditionally, which can expose raw refund payloads
and Braintree response data. Update `braintree-base.ts` so these helpers honor
`this.options_.logging` like the other logging helpers, and add redaction before
`JSON.stringify`/`serializeForLog` to strip sensitive fields from refund
context, `input.data`, transaction objects, and API responses. Keep the changes
localized to `logRefundTrace` and `logRefundJson` so the refund flow continues
to use these symbols consistently.
- Around line 843-851: Guard the TEST_FORCE_SETTLED refund branch in
braintree-base so gateway.testing.settle() only runs in sandbox/test mode, not
just when the env flag is set. Update the conditional around the refund flow in
the code path using this.gateway.testing.settle, retrieveTransaction, and
logRefundTrace/logRefundJson to also verify the gateway environment is sandbox
before settling, so a stray TEST_FORCE_SETTLED value cannot affect live refunds.
---
Nitpick comments:
In @.gitignore:
- Line 19: Remove the redundant `*.tgz` ignore rule from `.gitignore` so the
pattern is only listed once; keep the existing entry and delete the duplicate
added later.
🪄 Autofix (Beta)
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: 5658e2d7-5aa8-408f-8083-767ca122832f
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (6)
.gitignoreplugins/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/braintree-base.tsplugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-import.ts
| "@medusajs/admin-sdk": "^2.15.2", | ||
| "@medusajs/cli": "^2.15.2", | ||
| "@medusajs/framework": "^2.15.2", | ||
| "@medusajs/icons": "^2.15.2", | ||
| "@medusajs/medusa": "2.15.2", | ||
| "@medusajs/test-utils": "^2.15.2", | ||
| "@medusajs/ui": "^4.1.5", | ||
| "jsonwebtoken": "^9.0.2" | ||
| }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the file and inspect the relevant package metadata context.
git ls-files plugins/braintree-payment/package.json
echo "---"
cat -n plugins/braintree-payment/package.json | sed -n '1,140p'
echo "--- peer deps references ---"
rg -n '"`@medusajs/medusa`"|peerDependencies|dependencies|devDependencies' plugins/braintree-payment/package.jsonRepository: lambda-curry/medusa-plugins
Length of output: 3351
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Compare peerDependency ranges across the repository's package manifests.
git ls-files '**/package.json' | while read -r f; do
if rg -n '"peerDependencies"|@"medusajs/medusa"' "$f" >/dev/null 2>&1; then
echo "### $f"
rg -n '"peerDependencies"|@"medusajs/medusa"|@"medusajs/framework"|@"medusajs/admin-sdk"|@"medusajs/cli"|@"medusajs/icons"|@"medusajs/test-utils"|@"medusajs/ui"|jsonwebtoken' "$f"
echo
fi
doneRepository: lambda-curry/medusa-plugins
Length of output: 473
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for f in plugins/product-reviews/package.json plugins/webhooks/package.json; do
echo "### $f"
cat -n "$f" | sed -n '1,140p'
echo
doneRepository: lambda-curry/medusa-plugins
Length of output: 8270
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for exact Medusa peer pins or any repo-wide version-lock pattern.
rg -n '"`@medusajs/medusa`"\s*:\s*"[^"]+"' plugins -g 'package.json'
echo "---"
rg -n '"`@medusajs/medusa`"\s*:\s*"\^' plugins -g 'package.json'Repository: lambda-curry/medusa-plugins
Length of output: 826
Make the @medusajs/medusa peer range match the other Medusa peers. In plugins/braintree-payment/package.json:69, this is pinned to 2.15.2 while the rest of the Medusa peer deps use ^2.15.2, which will block adjacent Medusa versions for consumers.
Suggested fix
- "`@medusajs/medusa`": "2.15.2",
+ "`@medusajs/medusa`": "^2.15.2",📝 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.
| "@medusajs/admin-sdk": "^2.15.2", | |
| "@medusajs/cli": "^2.15.2", | |
| "@medusajs/framework": "^2.15.2", | |
| "@medusajs/icons": "^2.15.2", | |
| "@medusajs/medusa": "2.15.2", | |
| "@medusajs/test-utils": "^2.15.2", | |
| "@medusajs/ui": "^4.1.5", | |
| "jsonwebtoken": "^9.0.2" | |
| }, | |
| "`@medusajs/admin-sdk`": "^2.15.2", | |
| "`@medusajs/cli`": "^2.15.2", | |
| "`@medusajs/framework`": "^2.15.2", | |
| "`@medusajs/icons`": "^2.15.2", | |
| "`@medusajs/medusa`": "^2.15.2", | |
| "`@medusajs/test-utils`": "^2.15.2", | |
| "`@medusajs/ui`": "^4.1.5", | |
| "jsonwebtoken": "^9.0.2" | |
| }, |
🤖 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/package.json` around lines 65 - 73, The Medusa peer
dependency range is inconsistent in the braintree-payment package manifest:
`@medusajs/medusa` is pinned while the other Medusa peers use a caret range.
Update the `@medusajs/medusa` entry in package.json to match the existing ^2.15.2
pattern used by `@medusajs/admin-sdk`, `@medusajs/framework`, `@medusajs/icons`,
`@medusajs/test-utils`, and `@medusajs/ui` so consumers can install adjacent
compatible Medusa versions.
| /** Always-on path tracing for refund debugging. */ | ||
| protected logRefundTrace(step: string, context?: Record<string, unknown>): void { | ||
| const msg = context ? ` ${JSON.stringify(context, null, 2)}` : ''; | ||
| this.logger.info(`[Braintree refund] ${step}${msg}`); | ||
| } | ||
|
|
||
| /** Logs a stringified JSON payload from Braintree (API responses, input data, etc.). */ | ||
| protected logRefundJson(step: string, data: unknown): void { | ||
| this.logger.info(`[Braintree refund] ${step}: ${serializeForLog(data)}`); | ||
| } | ||
|
|
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant file around the reported lines and find all usages
FILE="plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts"
echo "== Line count =="
wc -l "$FILE"
echo
echo "== Relevant excerpt =="
sed -n '240,360p' "$FILE" | cat -n
echo
echo "== Search for logging gate / helpers / refund call sites =="
rg -n "options_\.logging|logRefundTrace\(|logRefundJson\(|serializeForLog\(|payment_method_nonce|client_token|account_holder|refund" "$FILE"Repository: lambda-curry/medusa-plugins
Length of output: 11263
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect nearby code for the refund flow and any redaction helpers
ROOT="plugins/braintree-payment/src/providers/payment-braintree/src/core"
echo "== Files in core =="
find "$ROOT" -maxdepth 1 -type f | sort
echo
echo "== Outline of braintree-base.ts =="
ast-grep outline "$ROOT/braintree-base.ts" --view expanded || trueRepository: lambda-curry/medusa-plugins
Length of output: 5132
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts"
echo "== serializeForLog helper =="
sed -n '110,140p' "$FILE" | cat -n
echo
echo "== refundPayment flow =="
sed -n '822,960p' "$FILE" | cat -n
echo
echo "== Refund input type definitions, if present in this file =="
sed -n '1,90p' "$FILE" | cat -nRepository: lambda-curry/medusa-plugins
Length of output: 10325
Gate refund logging or redact payloads
logRefundTrace and logRefundJson emit at info unconditionally, unlike the other helpers that respect this.options_.logging. They serialize raw input.data, transaction objects, and Braintree responses in the refund flow, which can expose payment details and account-holder data in logs. Gate these helpers behind the logging flag and redact sensitive fields before stringifying.
🤖 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 287 - 297, The refund helpers `logRefundTrace` and `logRefundJson`
currently log at `info` unconditionally, which can expose raw refund payloads
and Braintree response data. Update `braintree-base.ts` so these helpers honor
`this.options_.logging` like the other logging helpers, and add redaction before
`JSON.stringify`/`serializeForLog` to strip sensitive fields from refund
context, `input.data`, transaction objects, and API responses. Keep the changes
localized to `logRefundTrace` and `logRefundJson` so the refund flow continues
to use these symbols consistently.
Restrict verbose refund tracing to logging=true, ignore forced settlement outside sandbox, tighten refund path typing, and document the 0.1.2 changes. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/package.json`:
- Line 3: The version in package.json has been bumped to 0.1.3, but the release
notes in CHANGELOG.md still stop at 0.1.2; update the changelog to add the 0.1.3
entry so it matches the published version. Use the existing release-note
structure in CHANGELOG.md and keep the version references consistent with
package.json.
🪄 Autofix (Beta)
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: 6a07a1f0-d4c8-4bc6-a583-a2ed283aad80
📒 Files selected for processing (2)
plugins/braintree-payment/package.jsonplugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts
| { | ||
| "name": "@lambdacurry/medusa-payment-braintree", | ||
| "version": "0.1.0", | ||
| "version": "0.1.3", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check CHANGELOG.md version header vs package.json version.
fd -a CHANGELOG.md plugins/braintree-payment --exec cat -n {} \; | head -40
rg -n '"version"' plugins/braintree-payment/package.jsonRepository: lambda-curry/medusa-plugins
Length of output: 1177
Sync the release notes with the version bump. package.json is at 0.1.3, but CHANGELOG.md still stops at 0.1.2; update the changelog entry so the release notes match the published version.
🤖 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/package.json` at line 3, The version in
package.json has been bumped to 0.1.3, but the release notes in CHANGELOG.md
still stop at 0.1.2; update the changelog to add the 0.1.3 entry so it matches
the published version. Use the existing release-note structure in CHANGELOG.md
and keep the version references consistent with package.json.
lcmohsen
left a comment
lcmohsen
left a comment
There was a problem hiding this comment.
LGTM. Just 2 minor suggestions.
| const formatBraintreeValidationError = (error: BraintreeValidationErrorLike): string => { | ||
| const prefix = error.attribute ? `${error.attribute}: ` : ''; | ||
| const suffix = error.code ? ` (${error.code})` : ''; | ||
| return `${prefix}${error.message}${suffix}`; | ||
| return `BT: ${error.message}${suffix}`; | ||
| }; |
There was a problem hiding this comment.
Previously messages included attribute (e.g. postalCode: Postal code is invalid. (81813)). Now they are BT: Postal code is invalid. (81813).
This affects authorize/sale as well as refunds. Multi-error responses become harder to map to fields, and anything matching the old format will break. Prefer BT: ${attribute}: ${message} (or keep attribute and add BT: only if needed).
There was a problem hiding this comment.
braintree-import switched from buildBraintreeError (Failed to …, always INVALID_DATA) to throwOnBraintreeFailure (raw Braintree message, PAYMENT_AUTHORIZATION_ERROR vs INVALID_DATA).
braintree-import.spec.ts still only covers happy paths / network rejects / allowRefundOnRefunded with thrown Errors — not Result-object declines (success: false, processor_declined, validation errors). That path is now shared with base but untested for import.
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Documentation