-
-
Notifications
You must be signed in to change notification settings - Fork 4k
feat(core-flows,order,medusa,types): update item metadata on item_update change action #14570
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: 350a79e 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
|
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.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| expect(updatedItem.metadata).toEqual({ | ||
| updated: "metadata", | ||
| custom_field: "modified", | ||
| }) |
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.
Test expects metadata replacement instead of merge behavior
Medium Severity
The test expects metadata to be { updated: "metadata", custom_field: "modified" } after updating an item that originally had { initial: "value", custom_field: "original" }. However, the implementation uses mergeMetadata (per the PR discussion), which preserves existing keys not present in the update. The expected result should be { initial: "value", custom_field: "modified", updated: "metadata" } to include the preserved initial key.
Additional Locations (1)
| existing.metadata ?? {}, | ||
| action.details.metadata | ||
| ) | ||
| } |
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.
Passing null metadata causes runtime TypeError crash
High Severity
The validator accepts metadata: null via nullish(), but isDefined(null) returns true (it only checks for undefined). When null is passed to mergeMetadata, Object.entries(null) throws a TypeError: Cannot convert undefined or null to object. The internal service uses isPresent which properly handles null, but this code uses isDefined which does not.
Summary
What — What changes are introduced in this PR?
Allow to update item
metadataonITEM_UPDATEorder change action.Why — Why are these changes relevant or necessary?
You are unable to update a draft order item
metadata.How — How have these changes been implemented?
Update item
metadataif specified in the order change action details when theITEM_UPDATEprocessing handler gets executed.Testing — How have these changes been tested, or how can the reviewer test the feature?
Integration 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.
closes #14481
Note
Adds support for updating item
metadataduring draft order item updates.update-draft-order-itemnow includesmetadatain actiondetailsforITEM_UPDATEmetadatainto bothitem.detail.metadataanditem.metadatametadatafor new/existing items and related inputsmetadatain draft order item update payloadsdetail.metadatato exposed itemmetadataWritten by Cursor Bugbot for commit 350a79e. This will update automatically on new commits. Configure here.