feat: carry over promotions toggle on exchanges #14128
Open
+1,367
−214
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: 44398cd The changes in this PR will be included in the next version bump. This PR includes changesets to release 74 packages
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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 8 Skipped Deployments
|
fPolic
commented
Nov 26, 2025
packages/core/core-flows/src/order/workflows/set-carry-over-promotion-flag.ts
Show resolved
Hide resolved
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on December 17
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
packages/core/core-flows/src/order/workflows/exchange/exchange-add-new-item.ts
Outdated
Show resolved
Hide resolved
olivermrbl
reviewed
Nov 27, 2025
Contributor
olivermrbl
left a comment
olivermrbl
left a comment
There was a problem hiding this comment.
Overall LGTM, few nits
...routes/orders/order-create-exchange/components/exchange-create-form/exchange-create-form.tsx
Show resolved
Hide resolved
.../dashboard/src/routes/orders/order-create-exchange/components/exchange-create-form/schema.ts
Outdated
Show resolved
Hide resolved
packages/core/core-flows/src/order/workflows/set-carry-over-promotion-flag.ts
Show resolved
Hide resolved
packages/core/core-flows/src/order/workflows/set-carry-over-promotion-flag.ts
Show resolved
Hide resolved
packages/core/core-flows/src/order/workflows/set-carry-over-promotion-flag.ts
Outdated
Show resolved
Hide resolved
| acc = | ||
| acc + | ||
| ((action?.details.quantity || 0) / item.quantity) * | ||
| item.total |
There was a problem hiding this comment.
Bug: Missing optional chaining on details causes potential TypeError
The calculation for inbound total uses action?.details.quantity which only applies optional chaining to action, not to details. According to the type definition, details can be null (details: Record<string, unknown> | null). If action exists but action.details is null, accessing .quantity will throw a TypeError. The codebase elsewhere uses the safer pattern action?.details?.quantity with optional chaining on both properties.
| carry_over_promotions: checked, | ||
| }) | ||
| } | ||
| }} |
There was a problem hiding this comment.
Bug: Toggle state not reverted on API failure
The onChange(checked) call updates the form state immediately before the updateOrderChange API call completes. If the API call fails, the form's carry_over_promotions value remains toggled while the backend retains the previous value. The onError handler only displays a toast without reverting the form state. Since the exchange confirmation uses the backend value (not the form value), users will see a toggle indicating promotions will carry over, but the actual exchange behavior won't apply promotions. The onChange call needs to be moved after the successful API call, or the state needs to be reverted in the error handling.
| acc = | ||
| acc + | ||
| ((action?.details.quantity || 0) / item.quantity) * | ||
| item.total |
There was a problem hiding this comment.
Bug: Potential division by zero in inbound total calculation
The inbound total calculation divides by item.quantity without checking if it's zero. The expression ((action?.details.quantity || 0) / item.quantity) * item.total would produce NaN or Infinity if item.quantity is 0. While order items should typically always have a positive quantity, if there's any data integrity issue, this would cause incorrect monetary values to be displayed in the UI. Adding a guard like item.quantity || 1 or checking before division would prevent this edge case.
Contributor
|
/snapshot-this |
Contributor
🚀 A snapshot release has been made for this PRTest the snapshots by updating your yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add [email protected]yarn add @medusajs/[email protected]yarn add [email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]
|
Contributor
Author
|
tought: now when exchange is confirmed, and promotions are not carried over, it's not clear from the summary on the order details how the promotion has been applied. We might consider adding an indicator with promotions per line item as we did for edit items recently, wdyt - something to tackle in a follow up? |
Contributor
|
/snapshot-this |
Contributor
🚀 A snapshot release has been made for this PRTest the snapshots by updating your yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add [email protected]yarn add @medusajs/[email protected]yarn add [email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]
|
packages/core/core-flows/src/order/workflows/set-carry-over-promotion-flag.ts
Outdated
Show resolved
Hide resolved
packages/core/core-flows/src/order/workflows/exchange/update-exchange-add-item.ts
Outdated
Show resolved
Hide resolved
packages/core/core-flows/src/order/workflows/compute-adjustments-for-preview.ts
Show resolved
Hide resolved
Contributor
Author
|
/snapshot-this |
Contributor
🚀 A snapshot release has been made for this PRTest the snapshots by updating your yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add [email protected]yarn add @medusajs/[email protected]yarn add [email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]yarn add @medusajs/[email protected]
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
What — What changes are introduced in this PR?
Add ability to apply/remove promotions from outbound exchange item.
Checklist
Please ensure the following before requesting a review:
yarn changesetand follow the promptsScreen.Recording.2025-11-26.at.18.49.37.mov
TODO
CLOSES CORE-1291
Note
Adds a toggle to carry over promotions on exchanges/order changes, wiring DB, workflows, API/SDK, admin UI, and tests to apply/remove adjustments accordingly.
compute-adjustments-for-preview(generalized),on-carry-promotions-flag-set,update-order-changeworkflow, andlist-order-change-actions-by-typestep; compute or deleteITEM_ADJUSTMENTS_REPLACEbased onorder_change.carry_over_promotions.POST /admin/order-changes/:idto updatecarry_over_promotionsand returnorder_change.AdminOrderChangeand mutations/responses withcarry_over_promotions; addAdminOrderChangeResponse.carry_over_promotionstoorder_change(migration + model + query config).sdk.admin.order.updateOrderChange(id, { carry_over_promotions }).preview.order_changeand updates via new mutation; adjust totals calculation; show applied promo codes icon in order summary.useUpdateOrderChangehook; i18n strings/schema updates.Written by Cursor Bugbot for commit 44398cd. This will update automatically on new commits. Configure here.