fix(payments-plugin): improve order state transition handling for channel-specific contexts and default channel as a fallback #3420
Conversation
…nnel-specific contexts and default channel as a fallback
|
@LeftoversTodayAppAdmin is attempting to deploy a commit to the Vendure Team on Vercel. A member of the Team first needs to authorize it. |
|
|
Hey @dlhck and @michaelbromley Any updated on this PR? I see its been moved to a future release and is planned, please let me know what that means in terms of timeline. Best regards |
|
Hi @LeftoversTodayAppAdmin, unfortunately we didn't have time to get it into the feature release v3.3, so I have assigned it to the next feature release. |
|
Thank you @michaelbromley ! Looking forward to getting a my pending PRs in so I can finally upgrade from v3.0.3! |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant Stripe as Stripe Webhook
participant StripeController as StripeController
participant OrderService as OrderService
Stripe->>StripeController: webhook(event)
StripeController->>OrderService: transitionOrderToState(ctx, 'ArrangingPayment')
alt Success
OrderService-->>StripeController: Success
else OrderStateTransitionError
StripeController->>OrderService: transitionOrderToState(ctxWithDefaultChannel, 'ArrangingPayment')
alt Success
OrderService-->>StripeController: Success
else OrderStateTransitionError
StripeController->>StripeController: Log error
end
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/payments-plugin/src/stripe/stripe.controller.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: e2e tests (22.x, mysql)
- GitHub Check: e2e tests (24.x, postgres)
- GitHub Check: e2e tests (24.x, mysql)
- GitHub Check: e2e tests (24.x, sqljs)
- GitHub Check: e2e tests (20.x, sqljs)
- GitHub Check: codegen / codegen
- GitHub Check: unit tests (22.x)
- GitHub Check: unit tests (20.x)
- GitHub Check: publish_install (macos-latest, 24.x)
- GitHub Check: build (24.x)
- GitHub Check: build (20.x)
- GitHub Check: build (22.x)
- GitHub Check: publish_install (ubuntu-latest, 24.x)
- GitHub Check: publish_install (macos-latest, 20.x)
- GitHub Check: publish_install (windows-latest, 22.x)
- GitHub Check: publish_install (windows-latest, 20.x)
- GitHub Check: publish_install (windows-latest, 24.x)
- GitHub Check: publish_install (macos-latest, 22.x)
- GitHub Check: publish_install (ubuntu-latest, 22.x)
- GitHub Check: publish_install (ubuntu-latest, 20.x)
🔇 Additional comments (2)
packages/payments-plugin/src/stripe/stripe.controller.ts (2)
39-39: Minor formatting change approved.The spacing adjustment in the constructor is a formatting improvement with no functional impact.
117-124: Fallback mechanism implementation approved.The logic correctly implements the intended fallback behavior: first attempting with the channel-specific context (
ctx), then falling back to the default channel context if needed. This addresses the issue described in #3072 while maintaining backward compatibility.
…y if required. Updating to resolve the comment: vendure-ecommerce#3420 (comment) Moving default channel context into the if statement where it is required.
|
Description
Please include a summary of the changes and the related issue.
A change was introduced to use the default context instead of the channel specific context to change state to arranging payment in the stripe plugin:
#3072
This however causes issues when requiring channel specific context as noted here:
#3139
Instead of choosing just one or the other, a compromise will be to first try to use the channel specific context, if it fails then try to use the default context and if it still fails then throw an error.
This
Breaking changes
Does this PR include any breaking changes we should be aware of?
No breaking changes.
Screenshots
You can add screenshots here if applicable.
Checklist
📌 Always:
👍 Most of the time:
Summary by CodeRabbit
Bug Fixes
Style