OCA
[14.0] [IMP] sale_order_product_assortment: Use original product_id field keeping domain. Use cache to performance
Back port of https://github.com/OCA/sale-workflow/pull/2538
Depends on:
- [x] https://github.com/OCA/server-tools/pull/2941
- [x] https://github.com/OCA/server-tools/pull/2942
Hi @CarlosRoca13, some modules you are maintaining are being modified, check this out!
@CarlosRoca13 what do you think?
We had to apply the latest commit as the behavior was inconsistent, eg:
Create assortment "Chair", product allowed: "Office Chair", partner: Azure Interior Create assortment "Screen", product allowed: "Acoustic bloc screens", partner: Azure Interior SO for Azure Interior: 2 products can be selected
Create assortment "Chair", allowed product domain: "Name contains Office Chair", partner: Azure Interior Create assortment "Screen", allowed product domain: "Name contains Acoustic bloc screens", partner: Azure Interior SO for Azure Interior: no products can be selected
We'll fw-port this commit
This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖
Makes sense. Can you please add in the commit a brief explanation and the steps to reproduce the problem?
@CarlosRoca13 yours is a valid point, but it seems like this is a short blanket situation, as some assortments will be used to exclude products and others to include them and some to do both - there is no right or wrong.
We will look into a solution for the matter, do you have any idea how this could be solved?
Right now, I don't know how this could be faced... But I know that we have customers who are using the assortments excluding products and we cannot assume this change...
@pedrobaeza @carlosdauden @sergio-teruel can you give your point of view about this?
There shouldn't be any feature regression. It's OK to optimize, but not losing features.
There shouldn't be any feature regression. It's OK to optimize, but not losing features.
We're on the same page on that (we've always been), but we need to address the inconsistency in the example I posted above:
Create assortment "Chair", product allowed: "Office Chair", partner: Azure Interior Create assortment "Screen", product allowed: "Acoustic bloc screens", partner: Azure Interior SO for Azure Interior: 2 products can be selected
Create assortment "Chair", allowed product domain: "Name contains Office Chair", partner: Azure Interior Create assortment "Screen", allowed product domain: "Name contains Acoustic bloc screens", partner: Azure Interior SO for Azure Interior: no products can be selected
Also note that website assortment module works with OR as per https://github.com/OCA/e-commerce/pull/838
Are you telling that previously it was not correct? I agree then to cover the case correctly with the optimization.
The real problem of this is that we are taking all assortment domain at this point: https://github.com/OCA/sale-workflow/pull/3135/commits/7e8b7a484f85ed4fb4be881b95ac8ea9dc4f921c#diff-d857107550de88517b0f4cb32ae7770a25a630dc254b97191e7f71f262183089R36 Since we don't know what type of assortment is involved
I think the right solution would be to take 2 product domains out of the assortment. One for excluded products and one for included products. In this way we could combine them in the way that suits us.
Note: The excluded products can't fit with the included with OR in the domain
Sounds like something we could think about!
Note: The excluded products can't fit with the included with OR in the domain
@CarlosRoca13 Can you expand on this?
4 sure, what I mean is that the excluded domain needs to be set with AND because it is restrictive, an excluded product discards any posible included product
@CarlosRoca13 @pedrobaeza
Ok so the idea is to have in each assortment one "inclusive" domain working with OR and an "exclusive" domain working with AND, is that correct?
Maybe for user clarity we can specify below the domains the logic that will be applied, would that work?
At the end, both should combine to get a unique domain for the product selection, but include part should be treated on one part, joining all the inclusions together, and exclusions should be also joined, and excluded from the remaining result.
This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖
On my way to merge this fine PR! Prepared branch 14.0-ocabot-merge-pr-3135-by-pedrobaeza-bump-major, awaiting test results.
Congratulations, your PR was merged at e40ff5e270c99945ca1638fee8df7e282d7e5d16. Thanks a lot for contributing to OCA. ❤️