-
-
Notifications
You must be signed in to change notification settings - Fork 4k
fix(core): draft order variants requires_shipping field #14515
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
Open
digistaal
wants to merge
4
commits into
medusajs:develop
Choose a base branch
from
digistaal:fix/order-requires-shipping-field
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
fix(core): draft order variants requires_shipping field #14515
digistaal
wants to merge
4
commits into
medusajs:develop
from
digistaal:fix/order-requires-shipping-field
+181
−0
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
Add missing fields to productVariantsFields in order/utils/fields.ts to match the cart version. This fixes medusajs#14304 where orders created via admin panel have incorrect requires_shipping=false on fulfillments. Added fields: - requires_shipping - variant_option_values - thumbnail - product.is_giftcard - product.shipping_profile.id (critical fix) The missing product.shipping_profile.id field caused prepareLineItemData() to always evaluate hasShippingProfile as false, resulting in requires_shipping=false on line items and fulfillments for draft orders.
Add integration test that validates requires_shipping is correctly set to true when a product has a shipping profile linked, even when the inventory item has requires_shipping=false. This test ensures the fix for medusajs#14304 is properly covered and prevents regression. The test specifically validates that the product.shipping_profile.id field is being fetched and used in the requires_shipping calculation.
🦋 Changeset detectedLatest commit: e841b3d 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 |
|
@digistaal is attempting to deploy a commit to the medusajs Team on Vercel. A member of the Team first needs to authorize it. |
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
Add missing fields to
productVariantsFieldsinpackages/core/core-flows/src/order/utils/fields.tsto match the cart version.Why
Fixes #14304
The order version of
productVariantsFieldswas missing several fields compared to the cart version atpackages/core/core-flows/src/cart/utils/fields.ts. Most critically,product.shipping_profile.idwas missing.This causes orders created via the admin panel (draft orders) to have
requires_shipping=falseon their line items and fulfillments, which prevents the "Mark as shipped" button from appearing in the admin dashboard.Root Cause
When creating draft orders in the admin:
createOrdersStepcallsvalidateVariantPrices()which fetches variants usingproductVariantsFieldsproduct.shipping_profile.idprepareLineItemData()evaluateshasShippingProfileasfalsebecause the field is never fetchedrequires_shippingis set tofalseon line itemsfulfillment.requires_shipping === true)Changes
Added fields to
order/utils/fields.tsrequires_shippingvariant_option_valuesthumbnailproduct.is_giftcardproduct.shipping_profile.id(critical fix)Added integration test
Added a test to
integration-tests/modules/__tests__/order/draft-order.spec.tsthat:requires_shipping=falseto isolate the shipping profile logicrequires_shipping=trueon the order item due to the shipping profileThis ensures the fix is properly covered and prevents regression.
Testing
After applying this fix, draft orders created in the admin panel will correctly have
requires_shipping=trueon their fulfillments, and the "Mark as shipped" button will appear as expected.Checklist
Please ensure the following before requesting a review:
yarn changesetand follow the promptsAdditional Context
This issue only occurs for variants that do not have inventory management enabled, which is why it wasn't caught by other unit/integration tests. Normally it would fall back to one of the inventory to require shipping (which is true by default), but if inventory management is disabled, this is not applicable.
Note
Ensures
requires_shippingis derived correctly for draft orders by aligning order variant fields with the cart.order/utils/fields.tsproductVariantsFieldswithrequires_shipping,variant_option_values,thumbnail,product.is_giftcard, andproduct.shipping_profile.id(critical for shipping determination)integration-tests/.../draft-order.spec.tsthat links a product to a shipping profile and validatesrequires_shipping=trueeven when the inventory item hasrequires_shipping=false; resolvesFULFILLMENTmodule in tests@medusajs/core-flowsWritten by Cursor Bugbot for commit e841b3d. This will update automatically on new commits. Configure here.