Merge PR #1046 into 13.0

Signed-off-by simahawk

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
 
@@ -278,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

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,
             [
@@ -482,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