[18][stock_reserve_rule][FIX] Make sure quant are really excluded from reservation if a quant_domain is set on the removal rule

[florian-dacosta]

I was testing the module to solve the following use case :
On a location, I have some products inside some packages... and some other not belonging to any package.
My goal is to reserve only product not belonging to a package for a dedicated picking type.

Let's say I have, in stock/shelf1, 1 product A in a package PACK0001 and 1 product A (without package). ProductA/PACK0001 did enters the stock first so it would be taken first with default FIFO removal strategy.

I create a reserve rule for stock location with a removal rule on stock/shelf1 with a quant_domain [('package_id', '=', False)] (and default advanced removal strategy)
If I create a picking with 1 stock move with product A / product_uom_qty = 1, then check the availability. The system does reserve 1 product A with the PACK0001 !
It actually depends on the original default strategy, but it is kind of random.

What happens is :
The system find only the quant product A/no pack (it filters out the quant product A / PACK0001) https://github.com/OCA/stock-logistics-reservation/blob/18.0/stock_reserve_rule/models/stock_move.py#L70
Then it will find it can reserve 1 qty in self1 because of our free eligible quant Product A/no pack : https://github.com/OCA/stock-logistics-reservation/blob/18.0/stock_reserve_rule/models/stock_move.py#L86
And then he will try to make the reservation on the location, but as it is not "strict", Odoo will search a quant, that may or not have a package, because at this time, he is now aware of the quant that are eligible https://github.com/OCA/stock-logistics-reservation/blob/18.0/stock_reserve_rule/models/stock_move.py#L95
So it takes the package quant (because it is the first one with default FIFO strategy)/

Unless I am missing something about the module/quant_domain utility, I guess this is a bug.

My fix consist in ignoring the quants that have been filtered out by our removal rule during the reservation process.

A review/opinion would be gratly appreciated @rousseldenis @jbaudoux @bizzappdev

[jbaudoux]

I'm not surprised by the result. The initial purpose of the module was to choose from which area to reserve a product. If you have an area where products are on packages like stock/packs and another where they are not like stock/shelf, then it will choose the right area. On top of that we use the dynamic routing to split the pick into a split pack or pick shelf. As the source location changes, it will reserve properly.
Ensuring the final reservation respects the rule would be a good improvement.

[bizzappdev]

LGTM code and Function test
The fix looks appropriate.
We tested it with your described case and other scenarios (almost all), and it worked as expected. We also tried various reservation rules and domains, and the correct quants were consistently reserved from the expected locations.
The issue with reserving packaged quants no longer occurs; filtered-out quants are now ignored adequately during the reservation process, and more importantly, it is not impacting any other use case :)

[florian-dacosta]

Hello @jbaudoux
You seem to agree with the PR, if this is the case, could you approve it formally so we can get this merged ?
I just solve some conflicts in test file due to the merge of #26

[florian-dacosta]

@bguillot Could you review please ?