Fix application of removal rules too broad

Example of configuration:

Rule location: Stock
Removal rule 1: Stock/Zone1
Removal rule 2: Stock/Zone2

Reservation of a stock move with Stock/Zone2 as source location.

Previously, it would reserve in Stock/Zone1.
Now, it will never be allowed to reserve in Stock/Zone1.

A warning message was added previously to warn the user about potential
issues, which is now obsolete so I removed it.
This commit is contained in:
Guewen Baconnier
2020-05-14 16:09:35 +02:00
parent c99c97ffa4
commit 89e4ea230f
6 changed files with 62 additions and 45 deletions

View File

@@ -1,3 +1,4 @@
from . import stock_move
from . import stock_location
from . import stock_quant
from . import stock_reserve_rule

View File

@@ -0,0 +1,14 @@
# Copyright 2019 Camptocamp SA
# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
from odoo import models
class StockLocation(models.Model):
_inherit = "stock.location"
def is_sublocation_of(self, others):
"""Return True if self is a sublocation of at least one other"""
self.ensure_one()
# Efficient way to verify that the current location is
# below one of the other location without using SQL.
return any(self.parent_path.startswith(other.parent_path) for other in others)

View File

@@ -41,6 +41,18 @@ class StockMove(models.Model):
continue
for removal_rule in rule.rule_removal_ids:
# Exclude any rule which does not share the same path as the
# move's location. Example:
# Rule location: Stock
# Removal rule 1: Stock/Zone1
# Removal rule 2: Stock/Zone2
# If we have a stock.move with "Stock" as source location,
# it can use both rules.
# If we have a stock.move with "Stock/Zone2" as source location,
# it should never use "Stock/Zone1"
if not removal_rule.location_id.is_sublocation_of(location_id):
continue
quants = self.env["stock.quant"]._gather(
self.product_id,
removal_rule.location_id,

View File

@@ -62,18 +62,6 @@ class StockReserveRule(models.Model):
"rule is applicable or not.",
)
missing_reserve_rule = fields.Boolean(
string="Miss a reserve rule with higher priority?",
compute="_compute_missing_reserve_rule",
)
@api.depends()
def _compute_missing_reserve_rule(self):
for rule in self:
rule.missing_reserve_rule = any(
rule.rule_removal_ids.mapped("missing_reserve_rule")
)
@api.constrains("fallback_location_id")
def _constraint_fallback_location_id(self):
"""The fallback location has to be a child of the main location."""
@@ -178,26 +166,6 @@ class StockReserveRuleRemoval(models.Model):
"When empty, any packaging can be removed.",
)
missing_reserve_rule = fields.Boolean(
string="Miss a reserve rule with higher priority?",
compute="_compute_missing_reserve_rule",
)
@api.depends()
def _compute_missing_reserve_rule(self):
for removal_rule in self:
removal_rule.missing_reserve_rule = False
# The current rule could satisfies the need already
if removal_rule.location_id == removal_rule.rule_id.location_id:
break
rules = self.env["stock.reserve.rule"].search(
[
("location_id", "=", removal_rule.location_id.id),
("sequence", "<", removal_rule.rule_id.sequence),
]
)
removal_rule.missing_reserve_rule = not rules
@api.constrains("location_id")
def _constraint_location_id(self):
"""The location has to be a child of the rule location."""

View File

@@ -224,7 +224,13 @@ class TestReserveRule(common.SavepointCase):
move = picking.move_lines
self.assertRecordValues(
move,
[{"state": "confirmed", "location_id": fallback.id, "product_uom_qty": 150}],
[
{
"state": "confirmed",
"location_id": fallback.id,
"product_uom_qty": 150,
}
],
)
def test_rule_take_all_in_2(self):
@@ -653,3 +659,30 @@ class TestReserveRule(common.SavepointCase):
],
)
self.assertEqual(move.state, "assigned")
def test_rule_excluded_not_child_location(self):
self._update_qty_in_location(self.loc_zone1_bin1, self.product1, 100)
self._update_qty_in_location(self.loc_zone1_bin2, self.product1, 100)
self._update_qty_in_location(self.loc_zone2_bin1, self.product1, 100)
picking = self._create_picking(self.wh, [(self.product1, 80)])
self._create_rule(
{},
[
{"location_id": self.loc_zone1.id, "sequence": 1},
{"location_id": self.loc_zone2.id, "sequence": 2},
],
)
move = picking.move_lines
move.location_id = self.loc_zone2
picking.action_assign()
ml = move.move_line_ids
# As the source location of the stock.move is loc_zone2, we should
# never take any quantity in zone1.
self.assertRecordValues(
ml, [{"location_id": self.loc_zone2_bin1.id, "product_qty": 80.0}]
)
self.assertEqual(move.state, "assigned")

View File

@@ -35,15 +35,11 @@
</group>
<group string="Removal Rules" name="rule" col="1">
<field name="rule_removal_ids" nolabel="1">
<tree
string="Removal Rules"
decoration-warning="missing_reserve_rule"
>
<tree string="Removal Rules">
<field name="sequence" widget="handle" />
<field name="name" />
<field name="location_id" />
<field name="removal_strategy" />
<field name="missing_reserve_rule" invisible="1" />
</tree>
<form string="Removal Rule">
<group>
@@ -66,13 +62,6 @@
</group>
</form>
</field>
<field name="missing_reserve_rule" invisible="1" />
<strong
attrs="{'invisible': [('missing_reserve_rule', '=', False)]}"
style="color:red;"
>
WARNING: some of the removal rules are missing a reservation rule with a lower sequence. If it is desirable you should create them to ensure that the reservation on the related locations will use these rules first instead of this one.
</strong>
</group>
</form>
</field>