[FIX] stock: non-tracked products should follow FIFO removal strategy too

In a warehouse, mostly most of the products have no lot.  It stays
important however to avoid having old products in stock, certainly if
we can distinguish them by pack or location.

As now in stable, in_date is not put on the quant for non-tracked products,
databases might mix quants with and without in_date of the same product.
We should see those without in_date as the oldest.
That is why we need to be able to apply a ASC NULLS FIRST (ASC is NULLS LAST by default) for FIFO
or DESC NULLS LAST for LIFO.

In order to do that, we copied the code from the _search method of the ORM to
be able to do this special order as the orm itself is restrictive as it
supposes the order itself has no spaces.

We adapted and added some tests to check it handles the combination of NULL/NOT NULL well.

addons/stock/models/stock_quant.py

@@ -79,12 +79,6 @@ class StockQuant(models.Model):
             if float_compare(quant.quantity, 1, precision_rounding=quant.product_uom_id.rounding) > 0 and quant.lot_id and quant.product_id.tracking == 'serial':
                 raise ValidationError(_('A serial number should only be linked to a single product.'))
 
-    @api.constrains('in_date', 'lot_id')
-    def check_in_date(self):
-        for quant in self:
-            if quant.in_date and not quant.lot_id:
-                raise ValidationError(_('An incoming date cannot be set to an untracked product.'))
-
     @api.constrains('location_id')
     def check_location_id(self):
         for quant in self:
@@ -109,9 +103,9 @@ class StockQuant(models.Model):
     @api.model
     def _get_removal_strategy_order(self, removal_strategy):
         if removal_strategy == 'fifo':
-            return 'in_date, id'
+            return 'in_date ASC NULLS FIRST, id'
         elif removal_strategy == 'lifo':
-            return 'in_date desc, id desc'
+            return 'in_date DESC NULLS LAST, id desc'
         raise UserError(_('Removal strategy %s not implemented.') % (removal_strategy,))
 
     def _gather(self, product_id, location_id, lot_id=None, package_id=None, owner_id=None, strict=False):
@@ -134,7 +128,17 @@ class StockQuant(models.Model):
             domain = expression.AND([[('owner_id', '=', owner_id and owner_id.id or False)], domain])
             domain = expression.AND([[('location_id', '=', location_id.id)], domain])
 
-        return self.search(domain, order=removal_strategy_order)
+        # Copy code of _search for special NULLS FIRST/LAST order
+        self.sudo(self._uid).check_access_rights('read')
+        query = self._where_calc(domain)
+        self._apply_ir_rules(query, 'read')
+        from_clause, where_clause, where_clause_params = query.get_sql()
+        where_str = where_clause and (" WHERE %s" % where_clause) or ''
+        query_str = 'SELECT "%s".id FROM ' % self._table + from_clause + where_str + " ORDER BY "+ removal_strategy_order
+        self._cr.execute(query_str, where_clause_params)
+        res = self._cr.fetchall()
+        # No uniquify list necessary as auto_join is not applied anyways...
+        return self.browse([x[0] for x in res])
 
     @api.model
     def _get_available_quantity(self, product_id, location_id, lot_id=None, package_id=None, owner_id=None, strict=False, allow_negative=False):
@@ -196,17 +200,16 @@ class StockQuant(models.Model):
         quants = self._gather(product_id, location_id, lot_id=lot_id, package_id=package_id, owner_id=owner_id, strict=True)
         rounding = product_id.uom_id.rounding
 
-        if lot_id:
-            incoming_dates = quants.mapped('in_date')  # `mapped` already filtered out falsy items
-            incoming_dates = [fields.Datetime.from_string(incoming_date) for incoming_date in incoming_dates]
-            if in_date:
-                incoming_dates += [in_date]
-            # If multiple incoming dates are available for a given lot_id/package_id/owner_id, we
-            # consider only the oldest one as being relevant.
-            if incoming_dates:
-                in_date = fields.Datetime.to_string(min(incoming_dates))
-            else:
-                in_date = fields.Datetime.now()
+        incoming_dates = [d for d in quants.mapped('in_date') if d]
+        incoming_dates = [fields.Datetime.from_string(incoming_date) for incoming_date in incoming_dates]
+        if in_date:
+            incoming_dates += [in_date]
+        # If multiple incoming dates are available for a given lot_id/package_id/owner_id, we
+        # consider only the oldest one as being relevant.
+        if incoming_dates:
+            in_date = fields.Datetime.to_string(min(incoming_dates))
+        else:
+            in_date = fields.Datetime.now()
 
         for quant in quants:
             try:

addons/stock/tests/test_quant.py

@@ -629,7 +629,24 @@ class StockQuant(TransactionCase):
         })
         quantity, in_date = self.env['stock.quant']._update_available_quantity(product1, stock_location, 1.0)
         self.assertEqual(quantity, 1)
-        self.assertEqual(in_date, None)
+        self.assertNotEqual(in_date, None)
+
+
+    def test_in_date_1b(self):
+        stock_location = self.env.ref('stock.stock_location_stock')
+        product1 = self.env['product.product'].create({
+            'name': 'Product A',
+            'type': 'product',
+        })
+        self.env['stock.quant'].create({
+            'product_id': product1.id,
+            'location_id': stock_location.id,
+            'quantity': 1.0,
+        })
+        quantity, in_date = self.env['stock.quant']._update_available_quantity(product1, stock_location, 2.0)
+        self.assertEqual(quantity, 3)
+        self.assertNotEqual(in_date, None)
+
 
     def test_in_date_2(self):
         """ Check that an incoming date is correctly set when updating the quantity of a tracked
@@ -707,6 +724,34 @@ class StockQuant(TransactionCase):
         # Removal strategy is LIFO, so lot1 should be received as it was received later.
         self.assertEqual(quants[0][0].lot_id.id, lot1.id)
 
+    def test_in_date_4b(self):
+        """ Check for LIFO and max with/without in_date that it handles the LIFO NULLS LAST well
+        """
+        stock_location = self.env.ref('stock.stock_location_stock')
+        stock_location1 = self.env.ref('stock.stock_location_components')
+        stock_location2 = self.env.ref('stock.stock_location_14')
+        lifo_strategy = self.env['product.removal'].search([('method', '=', 'lifo')])
+        stock_location.removal_strategy_id = lifo_strategy
+        product1 = self.env['product.product'].create({
+            'name': 'Product A',
+            'type': 'product',
+            'tracking': 'serial',
+        })
+
+        self.env['stock.quant'].create({
+            'product_id': product1.id,
+            'location_id': stock_location1.id,
+            'quantity': 1.0,
+        })
+
+        in_date_location2 = datetime.now()
+        self.env['stock.quant']._update_available_quantity(product1, stock_location2, 1.0, in_date=in_date_location2)
+
+        quants = self.env['stock.quant']._update_reserved_quantity(product1, stock_location, 1)
+
+        # Removal strategy is LIFO, so the one with date is the most recent one and should be selected
+        self.assertEqual(quants[0][0].location_id.id, stock_location2.id)
+
     def test_in_date_5(self):
         """ Receive the same lot at different times, once they're in the same location, the quants
         are merged and only the earliest incoming date is kept.