[FIX] stock_picking_batch: remove done picking

With the option "Reception Report" enabled, a batch may contains
done and ready pickings, which is inconsistent

To reproduce the issue:
1. In Settings, enable Reception Report
2. Operations Types, edit Receipts:
   - Show Reception Report: True
   - Automatic Batches: True
   - Group by: Contact
3. Create two products P1, P2
4. Confirm a delivery with P1
5. Confirm two receipts, one for each product, both with partner P
   - It should create a batch
6. Partially process the receipt with P1, with backorder
   - It should open the reception report because of the delivery
7. Open the batch

Error: The first receipt is done but is still part of the batch and
the backorder is not attached to it. Worst: if the user now tries to
confirm a third receipt with the same partner, he will got an error
because of an inconsistency with the batch

[1] is incorrect. The code assumes that if `res` is not `True`, there
was an issue with the picking, so it's better to immediately return
`res` without analyzing the batch. However, sometimes `res` can be a
`dict` even when the picking has been processed correctly. This is the
case here: the receipt is marked as done, but we return an action to
open the reception report.

As a result, in the above case, we directly return the report action,
and we don't analyze the batch consistency (i.e., we don't remove
the done receipt and add the backorder). Then, because of the batch
inconsistency, the user is blocked and can not validate another
receipt.

[1] 0c7122d416a3669d1d42f66bef5b397e8b36a91b

OPW-3346255

closes odoo/odoo#138690

X-original-commit: d92c5a594e57cb2574bcf87ab398919c07c9a738
Signed-off-by: William Henrotin (whe) <whe@odoo.com>
Signed-off-by: Adrien Widart (awt) <awt@odoo.com>

addons/stock_picking_batch/models/stock_picking.py

@@ -118,14 +118,14 @@ class StockPicking(models.Model):
 
     def button_validate(self):
         res = super().button_validate()
-        if res is not True:
-            return res
         to_assign_ids = set()
         if self and self.env.context.get('pickings_to_detach'):
             self.env['stock.picking'].browse(self.env.context['pickings_to_detach']).batch_id = False
             to_assign_ids.update(self.env.context['pickings_to_detach'])
 
         for picking in self:
+            if picking.state != 'done':
+                continue
             # Avoid inconsistencies in states of the same batch when validating a single picking in a batch.
             if picking.batch_id and any(p.state != 'done' for p in picking.batch_id.picking_ids):
                 picking.batch_id = None

addons/stock_picking_batch/tests/test_batch_picking.py

@@ -15,7 +15,9 @@ class TestBatchPicking(TransactionCase):
         """ Create a picking batch with two pickings from stock to customer """
         super().setUpClass()
         cls.stock_location = cls.env.ref('stock.stock_location_stock')
+        cls.supplier_location = cls.env.ref('stock.stock_location_suppliers')
         cls.customer_location = cls.env.ref('stock.stock_location_customers')
+        cls.picking_type_in = cls.env['ir.model.data']._xmlid_to_res_id('stock.picking_type_in')
         cls.picking_type_out = cls.env['ir.model.data']._xmlid_to_res_id('stock.picking_type_out')
         cls.env['stock.picking.type'].browse(cls.picking_type_out).reservation_method = 'manual'
         cls.productA = cls.env['product.product'].create({
@@ -608,6 +610,81 @@ class TestBatchPicking(TransactionCase):
             {'product_id': self.productB.id, 'product_uom_qty': 10.0, 'quantity_done': 7.0, 'state': 'done'},
         ])
 
+    def test_process_picking_with_reception_report(self):
+        """"
+        Auto batch by partner + Reception report enabled
+        Batch with two pickings
+        Process the first one with backorder:
+        - Initial picking should be removed from the batch
+        - Backorder should be added to the bach
+        Create a third picking with same partner
+        - Should be added to the batch
+        """
+        self.env.user.groups_id = [(4, self.ref('stock.group_reception_report'))]
+        self.env['stock.picking.type'].browse(self.picking_type_in).write({
+            'auto_show_reception_report': True,
+            'auto_batch': True,
+            'batch_group_by_partner': True,
+        })
+
+        partner = self.env['res.partner'].create({'name': 'Super Partner'})
+
+        pickings = self.env['stock.picking'].create([{
+            'partner_id': partner_id,
+            'picking_type_id': type_id,
+            'location_id': from_loc.id,
+            'location_dest_id': to_loc.id,
+            'move_ids': [(0, 0, {
+                'name': '/',
+                'product_id': product.id,
+                'product_uom': product.uom_id.id,
+                'product_uom_qty': 1,
+                'location_id': from_loc.id,
+                'location_dest_id': to_loc.id,
+            })],
+        } for partner_id, product, type_id, from_loc, to_loc in [
+            # delivery
+            (False, self.productA, self.picking_type_out, self.stock_location, self.customer_location),
+            # receipts
+            (partner.id, self.productA, self.picking_type_in, self.supplier_location, self.stock_location),
+            (partner.id, self.productB, self.picking_type_in, self.supplier_location, self.stock_location),
+        ]])
+        pickings.action_confirm()
+        _delivery, receipt01, receipt02 = pickings
+
+        batch = receipt01.batch_id
+        self.assertTrue(batch)
+        self.assertEqual(batch.picking_ids, receipt01 | receipt02)
+
+        receipt01.move_ids.quantity_done = 0.75
+        action = receipt01.button_validate()
+        wizard = Form(self.env[action.get('res_model')].with_context(action['context'])).save()
+        res = wizard.process()
+        self.assertEqual(receipt01.state, 'done')
+        self.assertIsInstance(res, dict)
+        self.assertEqual(res.get('res_model'), 'report.stock.report_reception')
+
+        backorder = receipt01.backorder_ids
+        self.assertTrue(backorder)
+        self.assertEqual(batch.picking_ids, receipt02 | backorder)
+
+        receipt03 = self.env['stock.picking'].create({
+            'partner_id': partner.id,
+            'picking_type_id': self.picking_type_in,
+            'location_id': self.supplier_location.id,
+            'location_dest_id': self.stock_location.id,
+            'move_ids': [(0, 0, {
+                'name': '/',
+                'product_id': self.productA.id,
+                'product_uom': self.productA.uom_id.id,
+                'product_uom_qty': 1,
+                'location_id': self.supplier_location.id,
+                'location_dest_id': self.stock_location.id,
+            })],
+        })
+        receipt03.action_confirm()
+        self.assertEqual(batch.picking_ids, backorder | receipt02 | receipt03)
+
 @tagged('-at_install', 'post_install')
 class TestBatchPicking02(TransactionCase):