From 5e29b4182289c1a921f834658354638be04bb0fe Mon Sep 17 00:00:00 2001 From: Guewen Baconnier Date: Wed, 20 May 2020 15:21:22 +0200 Subject: [PATCH] Remove fallback location It could not work properly here as we need the "fallback" to apply even if there is no quantity at all in the stock. As we hook the reservation rules in StockMove._update_reserved_quantity(), and this method is called only if we have at least 1 product in qty, the fallback was not applied with zero qty. A new module will handle this concept: https://github.com/OCA/wms/pull/28 --- stock_reserve_rule/README.rst | 4 +- stock_reserve_rule/models/stock_move.py | 72 ++-------- .../models/stock_reserve_rule.py | 27 ---- stock_reserve_rule/readme/CONFIGURE.rst | 6 - .../static/description/index.html | 2 - stock_reserve_rule/tests/test_reserve_rule.py | 128 ------------------ .../views/stock_reserve_rule_views.xml | 1 - 7 files changed, 11 insertions(+), 229 deletions(-) diff --git a/stock_reserve_rule/README.rst b/stock_reserve_rule/README.rst index c72fbe43818d..5855725f345a 100644 --- a/stock_reserve_rule/README.rst +++ b/stock_reserve_rule/README.rst @@ -23,7 +23,7 @@ Stock Reservation Rules :target: https://runbot.odoo-community.org/runbot/153/12.0 :alt: Try me on Runbot -|badge1| |badge2| |badge3| |badge4| |badge5| +|badge1| |badge2| |badge3| |badge4| |badge5| This module adds rules for advanced reservation / removal strategies. @@ -86,8 +86,6 @@ Creation of a rule: Properties that define where the rule will be applied: * Location: Define where the rule will look for goods (a parent of the move's source location). -* Fallback Location: Define where the goods are reserved if none of the removal rule could reserve - the goods. If left empty, the goods are reserved in the move's source location / sub-locations. * Rule Domain: The rule is used only if the Stock Move matches the domain. Removal rules for the locations: diff --git a/stock_reserve_rule/models/stock_move.py b/stock_reserve_rule/models/stock_move.py index e615b9e33fba..68d29e1b91b6 100644 --- a/stock_reserve_rule/models/stock_move.py +++ b/stock_reserve_rule/models/stock_move.py @@ -103,68 +103,16 @@ def _update_reserved_quantity( break reserved = need - still_need - if rule.fallback_location_id: - quants = self.env["stock.quant"]._gather( - self.product_id, - rule.fallback_location_id, - lot_id=lot_id, - package_id=forced_package_id, - owner_id=owner_id, - strict=strict, - ) - fallback_quantity = sum(quants.mapped("quantity")) - sum( - quants.mapped("reserved_quantity") - ) - # If there is some qties to reserve in the fallback location, - # reserve them - reserved_fallback = super()._update_reserved_quantity( - still_need, - fallback_quantity, - location_id=rule.fallback_location_id, - lot_id=lot_id, - package_id=package_id, - owner_id=owner_id, - strict=strict, - ) - reserved += reserved_fallback - still_need = self.product_uom_qty - reserved - if still_need: - if not reserved: - # nothing could be reserved, however, we want to source - # the move on the specific fallback location (for - # replenishment), so update it's origin and return 0 - # reserved to leave the move confirmed - self.location_id = rule.fallback_location_id - return 0 - else: - # Then if there is still a need, we split the current move to - # get a new one targetting the fallback location with the - # remaining qties for replenishment - qty_split = self.product_uom._compute_quantity( - still_need, - self.product_id.uom_id, - rounding_method="HALF-UP", - ) - new_move_id = self._split(qty_split) - new_move = self.browse(new_move_id) - new_move.location_id = rule.fallback_location_id - # Shunt the caller '_action_assign' by telling that all - # the need has been reserved to get the current move - # updated to the state 'assigned' - return reserved + new_move.product_uom_qty - return reserved - - else: - # Implicit fallback on the original location - return reserved + super()._update_reserved_quantity( - still_need, - available_quantity - reserved, - location_id=location_id, - lot_id=lot_id, - package_id=package_id, - owner_id=owner_id, - strict=strict, - ) + # Implicit fallback on the original location + return reserved + super()._update_reserved_quantity( + still_need, + available_quantity - reserved, + location_id=location_id, + lot_id=lot_id, + package_id=package_id, + owner_id=owner_id, + strict=strict, + ) # We fall here if there is no rule or they have all been # excluded by 'rule._is_rule_applicable' diff --git a/stock_reserve_rule/models/stock_reserve_rule.py b/stock_reserve_rule/models/stock_reserve_rule.py index e794e999a43f..d4e4b962fd04 100644 --- a/stock_reserve_rule/models/stock_reserve_rule.py +++ b/stock_reserve_rule/models/stock_reserve_rule.py @@ -42,14 +42,6 @@ class StockReserveRule(models.Model): ) location_id = fields.Many2one(comodel_name="stock.location", required=True) - fallback_location_id = fields.Many2one( - comodel_name="stock.location", - help="If all removal rules are exhausted, try to reserve in this " - "location. Use it for replenishment. The source location move will be " - "changed to this location if the move is not available. If the move is " - "partially available, it is split and the unavailable quantity is sourced " - "in this location for replenishment.", - ) rule_removal_ids = fields.One2many( comodel_name="stock.reserve.rule.removal", inverse_name="rule_id" @@ -62,25 +54,6 @@ class StockReserveRule(models.Model): "rule is applicable or not.", ) - @api.constrains("fallback_location_id") - def _constraint_fallback_location_id(self): - """The fallback location has to be a child of the main location.""" - location_model = self.env["stock.location"] - for rule in self: - if rule.fallback_location_id: - is_child = location_model.search_count( - [ - ("id", "=", rule.fallback_location_id.id), - ("id", "child_of", rule.location_id.id), - ], - ) - if not is_child: - msg = _( - "Fallback location has to be a child of the location '{}'." - ).format(rule.location_id.display_name) - _logger.error("Rule '%s' - %s", rule.name, msg) - raise ValidationError(msg) - def _rules_for_location(self, location): # We'll typically have a handful of rules, so reading all of them then # checking if they are a parent location of the location is pretty diff --git a/stock_reserve_rule/readme/CONFIGURE.rst b/stock_reserve_rule/readme/CONFIGURE.rst index 03965177b96f..3052a5e99a33 100644 --- a/stock_reserve_rule/readme/CONFIGURE.rst +++ b/stock_reserve_rule/readme/CONFIGURE.rst @@ -5,12 +5,6 @@ Creation of a rule: Properties that define where the rule will be applied: * Location: Define where the rule will look for goods (a parent of the move's source location). -* Fallback Location: Define where the goods are reserved if none of the removal - rule could reserve the goods. Use it for replenishment. The source location - move will be changed to this location if the move is not available. If the - move is partially available, it is split and the unavailable quantity is - sourced in this location for replenishment. If left empty, the goods are - reserved in the move's source location / sub-locations. * Rule Domain: The rule is used only if the Stock Move matches the domain. Removal rules for the locations: diff --git a/stock_reserve_rule/static/description/index.html b/stock_reserve_rule/static/description/index.html index b410ab119841..9514a6efe3bb 100644 --- a/stock_reserve_rule/static/description/index.html +++ b/stock_reserve_rule/static/description/index.html @@ -429,8 +429,6 @@

Configuration

Properties that define where the rule will be applied:

Removal rules for the locations:

diff --git a/stock_reserve_rule/tests/test_reserve_rule.py b/stock_reserve_rule/tests/test_reserve_rule.py index cee56b1ca2fc..19611e9e5058 100644 --- a/stock_reserve_rule/tests/test_reserve_rule.py +++ b/stock_reserve_rule/tests/test_reserve_rule.py @@ -131,23 +131,6 @@ def _setup_packagings(self, product, packagings): ] ) - def test_rule_fallback_child_of_location(self): - # fallback is a child - self._create_rule( - {"fallback_location_id": self.loc_zone1.id}, - [{"location_id": self.loc_zone1.id}], - ) - # fallback is not a child - with self.assertRaises(exceptions.ValidationError): - self._create_rule( - { - "fallback_location_id": self.env.ref( - "stock.stock_location_locations" - ).id - }, - [{"location_id": self.loc_zone1.id}], - ) - def test_removal_rule_location_child_of_rule_location(self): # removal rule location is a child self._create_rule({}, [{"location_id": self.loc_zone1.id}]) @@ -157,82 +140,6 @@ def test_removal_rule_location_child_of_rule_location(self): {}, [{"location_id": self.env.ref("stock.stock_location_locations").id}] ) - def test_rule_fallback_partial_assign(self): - """Assign move partially available. - - The move should be splitted in two: - - one move assigned with reserved goods - - one move for remaining goods targetting the fallback location - """ - # Need 150 and 120 available => new move with 30 waiting qties - self._update_qty_in_location(self.loc_zone1_bin1, self.product1, 100) - self._update_qty_in_location(self.loc_zone2_bin1, self.product1, 20) - picking = self._create_picking(self.wh, [(self.product1, 150)]) - fallback = self.loc_zone2_bin1 - self._create_rule( - {"fallback_location_id": fallback.id}, - [{"location_id": self.loc_zone1_bin1.id, "sequence": 1}], - ) - self.assertEqual(len(picking.move_lines), 1) - picking.action_assign() - self.assertEqual(len(picking.move_lines), 2) - move_assigned = picking.move_lines.filtered(lambda m: m.state == "assigned") - move_unassigned = picking.move_lines.filtered(lambda m: m.state == "confirmed") - self.assertRecordValues( - move_assigned, - [ - { - "state": "assigned", - "location_id": picking.location_id.id, - "product_uom_qty": 120, - } - ], - ) - self.assertRecordValues( - move_unassigned, - [{"state": "confirmed", "location_id": fallback.id, "product_uom_qty": 30}], - ) - - def test_rule_fallback_unavailable(self): - """Assign move unavailable. - - The move source location should be changed to be the fallback location. - """ - self._update_qty_in_location(self.loc_zone1_bin1, self.product1, 100) - picking = self._create_picking(self.wh, [(self.product1, 150)]) - fallback = self.loc_zone2_bin1 - self._create_rule( - {"fallback_location_id": fallback.id}, - [ - { - "location_id": self.loc_zone1_bin1.id, - "sequence": 1, - # FIXME check if this isn't an issue? - # for the test: StockQuant._get_available_quantity should - # return something otherwise we won't enter in - # StockMove._update_reserved_quantity - # and the fallback will not be applied, so to reproduce a - # case where the quants are not allowed to be taken, - # use a domain that always resolves to false - "quant_domain": [("id", "=", 0)], - } - ], - ) - self.assertEqual(len(picking.move_lines), 1) - picking.action_assign() - self.assertEqual(len(picking.move_lines), 1) - move = picking.move_lines - self.assertRecordValues( - move, - [ - { - "state": "confirmed", - "location_id": fallback.id, - "product_uom_qty": 150, - } - ], - ) - def test_rule_take_all_in_2(self): all_locs = ( self.loc_zone1_bin1, @@ -324,41 +231,6 @@ def test_rule_remaining(self): self.assertEqual(move.state, "partially_available") self.assertEqual(move.reserved_availability, 300.0) - def test_rule_fallback(self): - reserve = self.env["stock.location"].create( - {"name": "Reserve", "location_id": self.wh.lot_stock_id.id} - ) - - self._update_qty_in_location(self.loc_zone1_bin1, self.product1, 100) - self._update_qty_in_location(self.loc_zone2_bin1, self.product1, 100) - self._update_qty_in_location(self.loc_zone3_bin1, self.product1, 100) - self._update_qty_in_location(reserve, self.product1, 300) - picking = self._create_picking(self.wh, [(self.product1, 400)]) - - self._create_rule( - {"fallback_location_id": reserve.id}, - [ - {"location_id": self.loc_zone1.id, "sequence": 3}, - {"location_id": self.loc_zone2.id, "sequence": 1}, - {"location_id": self.loc_zone3.id, "sequence": 2}, - ], - ) - - picking.action_assign() - move = picking.move_lines - ml = move.move_line_ids - self.assertRecordValues( - ml, - [ - {"location_id": self.loc_zone2_bin1.id, "product_qty": 100}, - {"location_id": self.loc_zone3_bin1.id, "product_qty": 100}, - {"location_id": self.loc_zone1_bin1.id, "product_qty": 100}, - {"location_id": reserve.id, "product_qty": 100}, - ], - ) - self.assertEqual(move.state, "assigned") - self.assertEqual(move.reserved_availability, 400.0) - def test_rule_domain(self): self._update_qty_in_location(self.loc_zone1_bin1, self.product1, 100) self._update_qty_in_location(self.loc_zone2_bin1, self.product1, 100) diff --git a/stock_reserve_rule/views/stock_reserve_rule_views.xml b/stock_reserve_rule/views/stock_reserve_rule_views.xml index 97ffd1624d3b..fb0c80148151 100644 --- a/stock_reserve_rule/views/stock_reserve_rule_views.xml +++ b/stock_reserve_rule/views/stock_reserve_rule_views.xml @@ -21,7 +21,6 @@ -