From 08d4273737cf294b8fa6f45ad94248f4002b88f0 Mon Sep 17 00:00:00 2001
From: Guewen Baconnier <guewen.baconnier@camptocamp.com>
Date: Tue, 5 Jan 2021 12:24:47 +0100
Subject: [PATCH] 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