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