Skip to content

Conversation

@NicolasGorga
Copy link
Contributor

Summary

What — What changes are introduced in this PR?

Refresh cart payment collection inside of updateCartPromotionsWorkflow

Why — Why are these changes relevant or necessary?

Without updating the cart payment collection after the workflow runs, you would end up with stale payment_collection amounts, that don't account for the removal/addition of the promotions

How — How have these changes been implemented?

Added a call to refreshPaymentCollectionForCartWorkflow right after the promotions are updated

Testing — How have these changes been tested, or how can the reviewer test the feature?

I've included missing integration test for the delete cart promotions endpoint


Examples

Provide examples or code snippets that demonstrate how this feature works, or how it can be used in practice.
This helps with documentation and ensures maintainers can quickly understand and verify the change.

// Example usage

Checklist

Please ensure the following before requesting a review:

  • I have added a changeset for this PR
    • Every non-breaking change should be marked as a patch
    • To add a changeset, run yarn changeset and follow the prompts
  • The changes are covered by relevant tests
  • I have verified the code works as intended locally
  • I have linked the related issue(s) if applicable

Additional Context

Add any additional context, related issues, or references that might help the reviewer understand this PR.

fixes #13936

@NicolasGorga NicolasGorga requested a review from a team as a code owner November 5, 2025 00:16
@changeset-bot
Copy link

changeset-bot bot commented Nov 5, 2025

🦋 Changeset detected

Latest commit: ee3f16f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 74 packages
Name Type
@medusajs/core-flows Patch
@medusajs/medusa Patch
integration-tests-http Patch
@medusajs/test-utils Patch
@medusajs/medusa-oas-cli Patch
@medusajs/analytics Patch
@medusajs/api-key Patch
@medusajs/auth Patch
@medusajs/caching Patch
@medusajs/cart Patch
@medusajs/currency Patch
@medusajs/customer Patch
@medusajs/file Patch
@medusajs/fulfillment Patch
@medusajs/index Patch
@medusajs/inventory Patch
@medusajs/link-modules Patch
@medusajs/locking Patch
@medusajs/notification Patch
@medusajs/order Patch
@medusajs/payment Patch
@medusajs/pricing Patch
@medusajs/product Patch
@medusajs/promotion Patch
@medusajs/region Patch
@medusajs/sales-channel Patch
@medusajs/settings Patch
@medusajs/stock-location Patch
@medusajs/store Patch
@medusajs/tax Patch
@medusajs/user Patch
@medusajs/workflow-engine-inmemory Patch
@medusajs/workflow-engine-redis Patch
@medusajs/draft-order Patch
@medusajs/oas-github-ci Patch
@medusajs/cache-inmemory Patch
@medusajs/cache-redis Patch
@medusajs/event-bus-local Patch
@medusajs/event-bus-redis Patch
@medusajs/analytics-local Patch
@medusajs/analytics-posthog Patch
@medusajs/auth-emailpass Patch
@medusajs/auth-github Patch
@medusajs/auth-google Patch
@medusajs/caching-redis Patch
@medusajs/file-local Patch
@medusajs/file-s3 Patch
@medusajs/fulfillment-manual Patch
@medusajs/locking-postgres Patch
@medusajs/locking-redis Patch
@medusajs/notification-local Patch
@medusajs/notification-sendgrid Patch
@medusajs/payment-stripe Patch
@medusajs/framework Patch
@medusajs/js-sdk Patch
@medusajs/modules-sdk Patch
@medusajs/orchestration Patch
@medusajs/types Patch
@medusajs/utils Patch
@medusajs/workflows-sdk Patch
@medusajs/cli Patch
@medusajs/deps Patch
@medusajs/telemetry Patch
@medusajs/admin-bundler Patch
@medusajs/admin-sdk Patch
@medusajs/admin-shared Patch
@medusajs/admin-vite-plugin Patch
@medusajs/dashboard Patch
@medusajs/icons Patch
@medusajs/toolbox Patch
@medusajs/ui-preset Patch
create-medusa-app Patch
medusa-dev-cli Patch
@medusajs/ui Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link

vercel bot commented Nov 5, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

8 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
api-reference Ignored Ignored Nov 6, 2025 11:31am
api-reference-v2 Ignored Ignored Preview Nov 6, 2025 11:31am
cloud-docs Ignored Ignored Preview Nov 6, 2025 11:31am
docs-ui Ignored Ignored Preview Nov 6, 2025 11:31am
docs-v2 Ignored Ignored Preview Nov 6, 2025 11:31am
medusa-docs Ignored Ignored Preview Nov 6, 2025 11:31am
resources-docs Ignored Ignored Preview Nov 6, 2025 11:31am
user-guide Ignored Ignored Preview Nov 6, 2025 11:31am

@@ -0,0 +1,6 @@
---
"@medusajs/core-flows": patch
"integration-tests-http": patch
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

todo: integration tests does not have to be in the changeset

Copy link
Contributor

@fPolic fPolic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall. Just wondering if we should refresh payment collections as part of updateCartPromotionsWorkflow workflow, or if we should separate those and call refreshPaymentCollectionForCartWorkflow outside. Some flows like refreshCartItemsWorkflow call the update workflow, then the refresh workflow explicitly and in those cases, payment refresh will be called twice. Alternatively, in that case we can pass a flag to updateCartPromotionsWorkflow to skip the refresh workflow?

@NicolasGorga
Copy link
Contributor Author

Looks good overall. Just wondering if we should refresh payment collections as part of updateCartPromotionsWorkflow workflow, or if we should separate those and call refreshPaymentCollectionForCartWorkflow outside. Some flows like refreshCartItemsWorkflow call the update workflow, then the refresh workflow explicitly and in those cases, payment refresh will be called twice. Alternatively, in that case we can pass a flag to updateCartPromotionsWorkflow to skip the refresh workflow?

Hey Frane so far from what I saw from current usage of refreshPaymentCollectionForCartWorkflow and refreshCartItemsWorkflow with this PR, this would be the only place we would be causing the double call, but we have the potential to cause it elsewhere if we forget.

I think I am more inclined to your second suggestion, since with the first one one would need to remember/know of the refreshPaymentCollectionForCartWorkflow (which is a good assumption to make) but I think it is more evident to have a force_refresh_payment_collection option in the input of workflows that would call it. See #13929 not exactly the same but I would use a similar approach for tax calculation refresh inside refreshCartItemsWorkflow wyt?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: DELETE promotion from cart returns STALE payment collection/session

4 participants