-
-
Notifications
You must be signed in to change notification settings - Fork 4k
feat(core-flows,order,medusa,types): Version shipping method adjustments & implement missing creation flow for versioned adjustments #14482
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 47cde33 The changes in this PR will be included in the next version bump. This PR includes changesets to release 76 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
|
packages/medusa/src/migration-scripts/backfill-shipping-adjustment-versions.ts
Outdated
Show resolved
Hide resolved
| !!order.promotions.length && !!input.orderChange.carry_over_promotions | ||
| ).then(() => { | ||
| const actionsToComputeItemsInput = transform( | ||
| const actionsToComputeContext = transform( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
todo: belive the part that is missing is that in this file, prepareAdjustmentsFromPromotionActionsStep returns shipping adjustments which we need to store on the change action (or create a new action dedicated to shipping adjustments).
Then this adjustments need to be applied when processing a change, see: packages/modules/order/src/utils/actions/item-adjustments-replace.ts
edit: from the plan, I see that some of this was explicitley skipped but without the full flow, new adjustments wont't be created and won't be filtered for that order version when fetching order
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is a good point, will look into it :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pushed lmkwyt @fPolic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good, think we are almost there...next thing to consider is when we fetch an order at a specific version that only shipping adjustments for that version are fetched: see packages/modules/order/src/utils/base-repository-find.ts and loadItemAdjustments method for referrence
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pushed
|
@adrien2p the migration i included to add unique index on |
Humm I see, I guess we cant just add it like that, we need to either provide a way for people to fix it automatically or guide them about the issue and fix it on their side. Then we need to wait another version to add the unique if we want |
|
You have run out of free Bugbot PR reviews for this billing cycle. This will reset on February 17. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
Summary
What — What changes are introduced in this PR?
Add
versionto shipping method adjustments and implement the missing flow for versioned creation of both line and shipping method adjustments.Why — Why are these changes relevant or necessary?
Adjustments don't align with the versioned nature of order related entities.
How — How have these changes been implemented?
Added the
versionfield to shipping method adjustments and handle their versions when applying order changes. Implemented missing logic to actually create/delete the versioned line/shipping methods adjustments.Testing — How have these changes been tested, or how can the reviewer test the feature?
Tests.
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 usageChecklist
Please ensure the following before requesting a review:
yarn changesetand follow the promptsAdditional Context
Add any additional context, related issues, or references that might help the reviewer understand this PR.
fixes #14384, CORE-1359