From 5b094973bf342ed233f092ae10a0cc9a43b31871 Mon Sep 17 00:00:00 2001 From: Guewen Baconnier Date: Tue, 5 Jan 2021 08:33:33 +0100 Subject: [PATCH 1/2] Modify empty bin rule to always respect fifo, lifo, ... The former implementation was sorting the quants per location and trying to take as much quantities as possible from the same locations, to limit the number of operations to do. Then, only, removal order (fifo, ...) was applied. It is more important to respect removal order than limiting the operations, so remove this "optimization". --- .../models/stock_reserve_rule.py | 26 +++--------- stock_reserve_rule/tests/test_reserve_rule.py | 40 +++++++++++++------ 2 files changed, 34 insertions(+), 32 deletions(-) diff --git a/stock_reserve_rule/models/stock_reserve_rule.py b/stock_reserve_rule/models/stock_reserve_rule.py index fc781d0e7..e7a55b7bb 100644 --- a/stock_reserve_rule/models/stock_reserve_rule.py +++ b/stock_reserve_rule/models/stock_reserve_rule.py @@ -219,28 +219,14 @@ class StockReserveRuleRemoval(models.Model): # the total quantity held in a location). quants_per_bin = quants._group_by_location() - # We want to limit the operations as much as possible. - # We'll sort the quants desc so we can fulfill as much as possible - # from as few as possible locations. The best case being an exact - # match. + # We take goods only if we empty the bin. # The original ordering (fefo, fifo, ...) must be kept. - - bins = sorted( - [ - ( - sum(quants.mapped("quantity")) - - sum(quants.mapped("reserved_quantity")), - location, - ) - for location, quants in quants_per_bin - ], - reverse=True, - ) - - # Propose the largest quants first, so we have as less operations - # as possible. We take goods only if we empty the bin. rounding = fields.first(quants).product_id.uom_id.rounding - for location_quantity, location in bins: + for location, location_quants in quants_per_bin: + location_quantity = sum(location_quants.mapped("quantity")) - sum( + location_quants.mapped("reserved_quantity") + ) + if location_quantity <= 0: continue diff --git a/stock_reserve_rule/tests/test_reserve_rule.py b/stock_reserve_rule/tests/test_reserve_rule.py index 51d5c5395..9c1e018f2 100644 --- a/stock_reserve_rule/tests/test_reserve_rule.py +++ b/stock_reserve_rule/tests/test_reserve_rule.py @@ -1,6 +1,6 @@ # Copyright 2019 Camptocamp (https://www.camptocamp.com) -from odoo import exceptions +from odoo import exceptions, fields from odoo.tests import common @@ -103,8 +103,10 @@ class TestReserveRule(common.SavepointCase): ) return picking - def _update_qty_in_location(self, location, product, quantity): - self.env["stock.quant"]._update_available_quantity(product, location, quantity) + def _update_qty_in_location(self, location, product, quantity, in_date=None): + self.env["stock.quant"]._update_available_quantity( + product, location, quantity, in_date=in_date + ) def _create_rule(self, rule_values, removal_values): rule_config = { @@ -403,10 +405,25 @@ class TestReserveRule(common.SavepointCase): ) self.assertEqual(move.state, "assigned") - def test_rule_empty_bin_largest_first(self): - self._update_qty_in_location(self.loc_zone1_bin1, self.product1, 30) - self._update_qty_in_location(self.loc_zone1_bin2, self.product1, 60) - self._update_qty_in_location(self.loc_zone2_bin1, self.product1, 50) + def test_rule_empty_bin_fifo(self): + self._update_qty_in_location( + self.loc_zone1_bin1, + self.product1, + 30, + in_date=fields.Datetime.to_datetime("2021-01-04 12:00:00"), + ) + self._update_qty_in_location( + self.loc_zone1_bin2, + self.product1, + 60, + in_date=fields.Datetime.to_datetime("2021-01-02 12:00:00"), + ) + self._update_qty_in_location( + self.loc_zone2_bin1, + self.product1, + 50, + in_date=fields.Datetime.to_datetime("2021-01-05 12:00:00"), + ) picking = self._create_picking(self.wh, [(self.product1, 80)]) self._create_rule( @@ -424,11 +441,10 @@ class TestReserveRule(common.SavepointCase): move = picking.move_lines ml = move.move_line_ids - # We expect to take 60 in zone1/bin2 as it will empty a bin, - # and we prefer to take in the largest empty bins first to minimize - # the number of operations. - # Then we cannot take in zone1/bin1 as it would not be empty afterwards - + # We expect to take 60 in zone1/bin2 as it will empty a bin and + # respecting fifo, the 60 of zone2 should be taken before the 30 of + # zone1. Then, as zone1/bin1 would not be empty, it is discarded. The + # remaining is taken in zone2 which has no rule. self.assertRecordValues( ml, [ From a003b49ba62ef0f2d9f52b5357dfff7fc1269e46 Mon Sep 17 00:00:00 2001 From: Guewen Baconnier Date: Tue, 5 Jan 2021 12:24:47 +0100 Subject: [PATCH 2/2] Modify packaging rule to always respect fifo, lifo, ... The former implementation was to take as much as possible of the largest packaging, to the smallest packacking, to have less to move. Then, only, removal order (fifo, ...) was applied for equal quantities. It is more important to respect removal order than limiting the operations, so remove this "optimization". --- .../models/stock_reserve_rule.py | 27 ++++-------- stock_reserve_rule/tests/test_reserve_rule.py | 42 +++++++++++++++++++ 2 files changed, 50 insertions(+), 19 deletions(-) diff --git a/stock_reserve_rule/models/stock_reserve_rule.py b/stock_reserve_rule/models/stock_reserve_rule.py index e7a55b7bb..42adda991 100644 --- a/stock_reserve_rule/models/stock_reserve_rule.py +++ b/stock_reserve_rule/models/stock_reserve_rule.py @@ -264,28 +264,17 @@ class StockReserveRuleRemoval(models.Model): def is_greater_eq(value, other): return float_compare(value, other, precision_rounding=rounding) >= 0 - for pack_quantity in packaging_quantities: - # Get quants quantity on each loop because they may change. - # Sort by max quant first so we have more chance to take a full - # package. But keep the original ordering for equal quantities! - bins = sorted( - [ - ( - sum(quants.mapped("quantity")) - - sum(quants.mapped("reserved_quantity")), - location, - ) - for location, quants in quants_per_bin - ], - reverse=True, + for location, location_quants in quants_per_bin: + location_quantity = sum(location_quants.mapped("quantity")) - sum( + location_quants.mapped("reserved_quantity") ) + if location_quantity <= 0: + continue - for location_quantity, location in bins: - if location_quantity <= 0: - continue + for pack_quantity in packaging_quantities: enough_for_packaging = is_greater_eq(location_quantity, pack_quantity) - asked_more_than_packaging = is_greater_eq(need, pack_quantity) - if enough_for_packaging and asked_more_than_packaging: + asked_at_least_packaging_qty = is_greater_eq(need, pack_quantity) + if enough_for_packaging and asked_at_least_packaging_qty: # compute how much packaging we can get take = (need // pack_quantity) * pack_quantity need = yield location, location_quantity, take diff --git a/stock_reserve_rule/tests/test_reserve_rule.py b/stock_reserve_rule/tests/test_reserve_rule.py index 9c1e018f2..acdb34047 100644 --- a/stock_reserve_rule/tests/test_reserve_rule.py +++ b/stock_reserve_rule/tests/test_reserve_rule.py @@ -498,6 +498,48 @@ class TestReserveRule(common.SavepointCase): ) self.assertEqual(move.state, "assigned") + def test_rule_packaging_fifo(self): + self._setup_packagings( + self.product1, + [("Pallet", 500, self.pallet), ("Retail Box", 50, self.retail_box)], + ) + self._update_qty_in_location( + self.loc_zone1_bin1, + self.product1, + 500, + in_date=fields.Datetime.to_datetime("2021-01-04 12:00:00"), + ) + self._update_qty_in_location( + self.loc_zone1_bin2, + self.product1, + 500, + in_date=fields.Datetime.to_datetime("2021-01-02 12:00:00"), + ) + self._create_rule( + {}, + [ + { + "location_id": self.loc_zone1.id, + "sequence": 1, + "removal_strategy": "packaging", + }, + ], + ) + + # take in bin2 to respect fifo + picking = self._create_picking(self.wh, [(self.product1, 50)]) + picking.action_assign() + self.assertRecordValues( + picking.move_lines.move_line_ids, + [{"location_id": self.loc_zone1_bin2.id, "product_qty": 50.0}], + ) + picking2 = self._create_picking(self.wh, [(self.product1, 50)]) + picking2.action_assign() + self.assertRecordValues( + picking2.move_lines.move_line_ids, + [{"location_id": self.loc_zone1_bin2.id, "product_qty": 50.0}], + ) + def test_rule_packaging_0_packaging(self): # a packaging mistakenly created with a 0 qty should be ignored, # not make the reservation fail