Skip to content
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

[18.0][MIG] stock_picking_product_assortment #2262

Open
wants to merge 15 commits into
base: 18.0
Choose a base branch
from

Conversation

bosd
Copy link
Contributor

@bosd bosd commented Feb 12, 2025

@bosd bosd force-pushed the 18.0-mig-stock_picking_product_assortment branch from 31a4173 to 7f3a12e Compare March 6, 2025 15:52
@bosd bosd force-pushed the 18.0-mig-stock_picking_product_assortment branch from 7f3a12e to a043b71 Compare March 6, 2025 15:56
@bosd
Copy link
Contributor Author

bosd commented Mar 6, 2025

Let's go now OCA/product-attribute#1904 is merged
Oopsie.. it still needs some fixing..

@bosd
Copy link
Contributor Author

bosd commented Mar 6, 2025

@PieterPaulussen
Ok, pushed the hot potato forward to fix the tests. Until we have clean & simple solution in base module.
But let's keep the migration flow going.

Copy link

@Dranyel-Bosd Dranyel-Bosd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 💯

Copy link

@PieterPaulussen PieterPaulussen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The performance of this module should be re-evaluated because of the inefficient domain parameter in the views.

operation="domain_add"
condition="parent.has_assortment"
>
[('id', 'in', parent.assortment_product_ids)]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bosd This should not be solved here, but have you tested this with resulting large domains? This will work fine and well for a small set, but for large results... an ('id', 'in', parent.assortment_product_ids) is really inefficient.
Is there no way to invert the domain like ('assortment_id', 'in', parent.assortment_ids)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PieterPaulussen I did not test this module with large domains or data sets.
I've tried the code suggestion, but it did'nt work yet.

This module allows to use the product assortments related to a partner
on stock pickings. Whith this implementation, on stock pickings, we only allow to
select the products defined on the assortment and we don't allow to select
products which are not defined in the whitelist on the assortment.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about the blacklisted products?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. I've changed it to make it more clear and general.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jbaudoux Is this PR ok for you now?

@bosd bosd force-pushed the 18.0-mig-stock_picking_product_assortment branch from c248653 to 8b32452 Compare March 8, 2025 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants