diff --git a/stock_reserve_rule/models/__init__.py b/stock_reserve_rule/models/__init__.py index 2f1eab9ba..b7b0c5f76 100644 --- a/stock_reserve_rule/models/__init__.py +++ b/stock_reserve_rule/models/__init__.py @@ -1,3 +1,4 @@ from . import stock_move +from . import stock_location from . import stock_quant from . import stock_reserve_rule diff --git a/stock_reserve_rule/models/stock_location.py b/stock_reserve_rule/models/stock_location.py new file mode 100644 index 000000000..60caf1b3f --- /dev/null +++ b/stock_reserve_rule/models/stock_location.py @@ -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) diff --git a/stock_reserve_rule/models/stock_move.py b/stock_reserve_rule/models/stock_move.py index 129c1ba88..e615b9e33 100644 --- a/stock_reserve_rule/models/stock_move.py +++ b/stock_reserve_rule/models/stock_move.py @@ -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, diff --git a/stock_reserve_rule/models/stock_reserve_rule.py b/stock_reserve_rule/models/stock_reserve_rule.py index 48c519a2b..add61e828 100644 --- a/stock_reserve_rule/models/stock_reserve_rule.py +++ b/stock_reserve_rule/models/stock_reserve_rule.py @@ -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.""" diff --git a/stock_reserve_rule/tests/test_reserve_rule.py b/stock_reserve_rule/tests/test_reserve_rule.py index 48ef9cec5..cee56b1ca 100644 --- a/stock_reserve_rule/tests/test_reserve_rule.py +++ b/stock_reserve_rule/tests/test_reserve_rule.py @@ -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") diff --git a/stock_reserve_rule/views/stock_reserve_rule_views.xml b/stock_reserve_rule/views/stock_reserve_rule_views.xml index 966dbfaed..97ffd1624 100644 --- a/stock_reserve_rule/views/stock_reserve_rule_views.xml +++ b/stock_reserve_rule/views/stock_reserve_rule_views.xml @@ -35,15 +35,11 @@ - + -
@@ -66,13 +62,6 @@
- - - 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. -