-
-
Notifications
You must be signed in to change notification settings - Fork 3.8k
feat(medusa,product,core-flows,types): product options redesign (server-side) #13817
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?
feat(medusa,product,core-flows,types): product options redesign (server-side) #13817
Conversation
🦋 Changeset detectedLatest commit: 8041f0e 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
|
|
Hey Will, one thing i noticed is if the pivot entity is not defined between product and product option, additional columns can't be added to the pivot table, like for example the rank field, to be able to specify for a given product, the order you want its options to show. |
The rank field will be on |
packages/modules/product/integration-tests/__tests__/product-module-service/products.spec.ts
Show resolved
Hide resolved
|
/snapshot-this |
🚀 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]
|
fPolic
left a comment
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.
Great work @willbouch! Left a few questions, but looks good overall!
| { | ||
| title: "option2", | ||
| values: ["D", "E"], | ||
| is_exclusive: true, |
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.
q: do we validate is_exclusive when assigning products
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.
Not at the moment, you are right, I had planned to do most the error validation scenarios like that in the next PRs but forgot to add it in the list. Added it to the list
packages/core/core-flows/src/product/steps/process-product-options-for-import.ts
Show resolved
Hide resolved
packages/core/core-flows/src/product/workflows/link-product-options-to-product.ts
Outdated
Show resolved
Hide resolved
| ) => { | ||
| const id = req.params.id | ||
|
|
||
| await deleteProductOptionsWorkflow(req.scope).run({ |
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.
q: do we validate if a global option can be deleted if there are linked products?
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.
Not at the moment, good point. I will do that with the other error scenarios. Added it to the list
| $or: pairs, | ||
| } | ||
| ) | ||
| await this.productProductOptionService_.delete( |
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.
q: what happens with variants that have values of the removed option?
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.
At the moment nothing. I think this wil also be something that I need to do in upcoming PRs. The notion of cleaning up when deleting an option. I will have a discussion with you guys before that to make sure we align on the behaviour
olivermrbl
left a comment
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.
this is epic work @willbouch!
packages/core/core-flows/src/product/workflows/link-product-options-to-product.ts
Outdated
Show resolved
Hide resolved
packages/modules/product/src/migrations/Migration20251022153442.ts
Outdated
Show resolved
Hide resolved
| // get product_id from the first product in the products array of the first option | ||
| ...(options.length && options[0].products?.length | ||
| ? { product_id: options[0].products[0].id } | ||
| : {}), |
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.
Bug: Variants Lack Product Identity
The code attempts to extract product_id from options[0].products[0].id, but the products relation isn't loaded when productRepository_.deepUpdate calls validateVariantOptions. The repository only populates options and options.values relations, leaving products as an uninitialized Collection with length 0. This causes the condition to evaluate false, preventing variants from receiving a product_id, which breaks the filtering logic in assignOptionsToVariants that relies on variant.product_id to match options.
| }) | ||
| } | ||
| }), | ||
| option_ids: z.array(IdAssociation).optional(), |
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.
Bug: API option_ids Schema Conflict
The option_ids validator expects z.array(IdAssociation) requiring objects like [{ id: "opt_1" }], but the HTTP type definition at packages/core/types/src/http/product/admin/payloads.ts line 462 and service implementation expect string[] like ["opt_1", "opt_2"]. This mismatch causes the validator to reject valid API requests following the type definition, and if bypassed, would cause the service logic to fail when iterating over objects instead of strings.
| }) | ||
| } | ||
| }), | ||
| option_ids: z.array(IdAssociation).optional(), |
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.
Bug: Type Mismatch Blocks Product Update Flow
The option_ids validator expects z.array(IdAssociation) which validates { id: string }[], but the service implementation at product-module-service.ts lines 2027-2028 and 2074 expects a flat array of strings string[]. This mismatch causes the service to attempt operations like flatMap and new Set(product.option_ids) on an array of objects instead of strings, breaking the product update logic when option_ids are provided.
| "updateRule": "cascade" | ||
| } | ||
| }, | ||
| "foreignKeys": {}, |
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.
Bug: Migration Snapshot Foreign Key Mismatch
The migration snapshot for product_product_option table has empty foreignKeys object, but the corresponding migration Migration20251022153442.ts creates foreign key constraints product_product_option_product_id_foreign and product_product_option_product_option_id_foreign. The snapshot should include these foreign keys to match the actual database schema created by the migration, otherwise there will be a mismatch between the snapshot and the actual database state.
This is the SERVER-SIDE part of the PR. The client-side PR can be found here. To test via snapshot, it will be easier to fo it from the client-side PR, since it is based on this one.
WHAT
This PR handles an important and heavily requested redesign in the Core. For context, the relationship between
ProductandProductOptionscurrently is 1-many. We want to change that to a many-many relationship, which will allow linking multipleProductsto the sameProductOption. This will be useful to implement filters on the storefront for example.HOW

Here is an image of the redesign:
To clarify, the pivot table between
product_option_valueand the other pivot table will be used to enable linking only a subset of theProdutOptionValuesto aProduct. For example, aProductOptionnamedColorcould have 10 colors defined, and I might have aProductthat only exists in 5 colors. In that case, to avoid making manyColoroptions, the user will have the ability to only choose the colors they want for that productIn this PR, you will find:
I will leave comments in the PR to make the review a bit easier
WHAT IS LEFT
I decided to put the stuff I needed to have a "functional" frontend, but there are still things that are missing that I will address in another PR
Note
Redesigns product options to a many‑to‑many model with CRUD/admin APIs, linking workflows, value ranking, and updated types, events, and migrations.
Product↔ProductOptionto many‑to‑many viaproduct_product_optionpivot; addis_exclusiveonproduct_optionandrankonproduct_option_value.addProductOptionToProduct/removeProductOptionFromProduct.ranksandis_exclusive; preserve options on product soft delete; validate variant option combinations against linked options.link-product-options-to-product,create-and-link-product-options-to-product,process-product-options-for-import(transformsoptions→option_ids).create/update-product-optionsto handleranks; batch products workflow pipes option processing on update.GET/POST/DELETE /admin/product-optionsandGET/POST/DELETE /admin/product-options/:id(CRUD withis_exclusive,ranks).POST /admin/products/:id/optionsto add/remove existing or create‑and‑link options;GET /admin/products/:id/optionslists by product.optionspayload; useoption_ids.is_exclusive,ranks,option_ids, and product‑option link types.productProductOptionevent namespace; adjust event emissions for link/unlink.Written by Cursor Bugbot for commit 8041f0e. This will update automatically on new commits. Configure here.