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".

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
 

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