Fix Braintree Currency Line Item #29
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughBumps plugin version to 0.0.16; refactors the Braintree provider to remove cartService, use an optional context Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Caller
participant Provider as BraintreeBase
participant Context as ProviderContext
participant Logger as Logger
rect rgb(245,250,240)
Note over Client,Provider: Build transaction request using resolved currency
end
Client->>Provider: getTransactionCreateRequestBody(context, items)
Provider->>Context: read order_currency_code
alt order_currency_code present
Context-->>Provider: return order_currency_code
else
Provider-->>Provider: use options.defaultCurrencyCode or "USD"
end
Provider->>Provider: construct line items and unitOfMeasure with resolved currencyCode
Provider-->>Client: return transaction request body
alt error
Provider->>Logger: log error with context
Provider-->>Client: throw buildBraintreeError(error, operation, Logger, context)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (7)
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts (1)
445-446: Remove unusedcartvariable.This variable is declared but never assigned or used. It appears to be leftover from the cart service removal refactor.
Apply this diff:
- let cart: CartDTO | undefined; - const lineItems = context.items
🧹 Nitpick comments (2)
plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts (2)
113-120: Remove or utilize the unusedcontextparameter.The
contextparameter is declared but never used within the function body. If it's intended for future logging enhancements, consider adding a comment explaining this; otherwise, remove it to reduce confusion.Apply this diff if the parameter should be removed:
-export const buildBraintreeError = (error: unknown, operation: string, logger: Logger, context?: Record<string, unknown>): MedusaError => { +export const buildBraintreeError = (error: unknown, operation: string, logger: Logger): MedusaError => {Then update call sites at lines 633, 636, 655, 792, 868, and 891 to remove the context argument.
464-464: Remove unnecessary type cast and optional chaining.The
as stringtype cast and optional chaining operator?.are unnecessary sincecurrencyCodeis already guaranteed to be a string (with the 'USD' fallback on line 456).Apply this diff:
- unitOfMeasure: (currencyCode as string)?.toUpperCase().substring(0, 12), // Max 12 characters + unitOfMeasure: currencyCode.toUpperCase().substring(0, 12), // Max 12 characters
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
plugins/braintree-payment/package.json(1 hunks)plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts(3 hunks)
🔇 Additional comments (2)
plugins/braintree-payment/package.json (1)
3-3: LGTM! Version bump aligns with the refactoring scope.The version increment from 0.0.15 to 0.0.16 appropriately reflects the changes in this PR, which include removing the cart service dependency and adding new context fields.
plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts (1)
65-65: LGTM! Appropriate addition to support cart-independent currency handling.The new optional
order_currency_codefield enables currency derivation without depending on the cart service, which aligns with the PR objectives.
plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts
Outdated
Show resolved
Hide resolved
plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts
Outdated
Show resolved
Hide resolved
plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts
Show resolved
Hide resolved
plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts
Outdated
Show resolved
Hide resolved
plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/braintree-payment/src/providers/payment-braintree/README.md (1)
21-21: Fix typo in documentation."merchnat" should be "merchant".
Apply this diff:
-BRAINTREE_MERCHANT_ID=<Your merchnat Id> +BRAINTREE_MERCHANT_ID=<Your merchant Id>
🧹 Nitpick comments (1)
plugins/braintree-payment/src/providers/payment-braintree/README.md (1)
60-60: Consider expanding the documentation.The documentation could be more detailed to help users understand when and how
defaultCurrencyCodeis used, such as explaining it's used as a fallback when currency information is not provided in the payment context.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
plugins/braintree-payment/src/providers/payment-braintree/README.md(2 hunks)plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts(4 hunks)plugins/braintree-payment/src/providers/payment-braintree/src/types/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- plugins/braintree-payment/src/providers/payment-braintree/src/core/braintree-base.ts
🔇 Additional comments (1)
plugins/braintree-payment/src/providers/payment-braintree/src/types/index.ts (1)
4-4: LGTM!The addition of the optional
defaultCurrencyCodeproperty is correctly typed and aligns with the documentation changes in the README.
Summary
This PR fixes the Braintree currency line item handling by removing the cart service dependency and adding support to include line extended data.
Changes
Related Commits
Created by @govind Diwakar via Codegen
💻 View my work • About Codegen
⛔ Remove Codegen from PR • 🚫 Ban action checks
Summary by CodeRabbit
Chores
Improvements
Documentation