From 838718cc7631d1bdd5a7de4f398f8c7b2ac366b0 Mon Sep 17 00:00:00 2001 From: "Adrien Widart (awt)" Date: Fri, 13 Oct 2023 11:43:53 +0200 Subject: [PATCH] [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) Signed-off-by: Adrien Widart (awt) --- .../models/stock_picking.py | 4 +- .../tests/test_batch_picking.py | 77 +++++++++++++++++++ 2 files changed, 79 insertions(+), 2 deletions(-) diff --git a/addons/stock_picking_batch/models/stock_picking.py b/addons/stock_picking_batch/models/stock_picking.py index 8523dbf5b74..3b2600142ae 100644 --- a/addons/stock_picking_batch/models/stock_picking.py +++ b/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 diff --git a/addons/stock_picking_batch/tests/test_batch_picking.py b/addons/stock_picking_batch/tests/test_batch_picking.py index 09b87b70e37..4ecc9cb88e7 100644 --- a/addons/stock_picking_batch/tests/test_batch_picking.py +++ b/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):