From b970bf865a7fa1dfbef0b8a365e39210fb5b75ce Mon Sep 17 00:00:00 2001 From: amoyaux Date: Wed, 16 Aug 2017 16:40:05 +0200 Subject: [PATCH] [FIX] stock: unable to return SN in chained moves Use case to reproduce the bug: - Set multi location in settings - Set warehouse's deliery method as pick/pack/ship - Deliver a product to customer - Process a return from customer to output. - Create a return from output to pack -> Unable to reserver product. It happens because return moves where not linked together but have the picking that created the return as origin. Thus action_assign does not detect the return moves and can't guess the real quantity available. This commit add a behavior that will create links between return move. It can be guess from parent move(the picking's move from which the return is created) and its move dest/orig that have already created other return. It also correct action assign because the group by could retourn same line with return moves. It add a test that check returns and links between them. --- addons/stock/models/stock_traceability.py | 2 + addons/stock/tests/common.py | 1 + addons/stock/tests/test_move2.py | 228 ++++++++++++++++++++ addons/stock/wizard/stock_picking_return.py | 13 +- 4 files changed, 243 insertions(+), 1 deletion(-) diff --git a/addons/stock/models/stock_traceability.py b/addons/stock/models/stock_traceability.py index ccd6c5b67dc..73ffdf3736b 100644 --- a/addons/stock/models/stock_traceability.py +++ b/addons/stock/models/stock_traceability.py @@ -84,6 +84,7 @@ class MrpStockReport(models.TransientModel): ('lot_id', '=', context.get('active_id')), ('location_id.usage', '!=', 'internal'), ('state', '=', 'done'), + ('move_id.returned_move_ids', '=', False), ]) res += self._lines(line_id, model_id=model_id, model='stock.move.line', level=level, parent_quant=parent_quant, stream=stream, obj_ids=move_ids) @@ -99,6 +100,7 @@ class MrpStockReport(models.TransientModel): ('lot_id', '=', context.get('active_id')), ('location_dest_id.usage', '!=', 'internal'), ('state', '=', 'done'), + ('move_id.returned_move_ids', '=', False), ]) res += self._lines(line_id, model_id=model_id, model='stock.move.line', level=level, parent_quant=parent_quant, stream=stream, obj_ids=move_ids) diff --git a/addons/stock/tests/common.py b/addons/stock/tests/common.py index f10c42db152..6a695af9363 100644 --- a/addons/stock/tests/common.py +++ b/addons/stock/tests/common.py @@ -28,6 +28,7 @@ class TestStockCommon(common.TransactionCase): self.supplier_location = self.ModelDataObj.xmlid_to_res_id('stock.stock_location_suppliers') self.stock_location = self.ModelDataObj.xmlid_to_res_id('stock.stock_location_stock') self.pack_location = self.ModelDataObj.xmlid_to_res_id('stock.location_pack_zone') + self.output_location = self.ModelDataObj.xmlid_to_res_id('stock.stock_location_output') self.customer_location = self.ModelDataObj.xmlid_to_res_id('stock.stock_location_customers') self.categ_unit = self.ModelDataObj.xmlid_to_res_id('product.product_uom_categ_unit') self.categ_kgm = self.ModelDataObj.xmlid_to_res_id('product.product_uom_categ_kgm') diff --git a/addons/stock/tests/test_move2.py b/addons/stock/tests/test_move2.py index 704f3015bb5..b9fcd4f0dfc 100644 --- a/addons/stock/tests/test_move2.py +++ b/addons/stock/tests/test_move2.py @@ -45,6 +45,62 @@ class TestPickShip(TestStockCommon): }) return picking_pick, picking_client + def create_pick_pack_ship(self): + picking_ship = self.env['stock.picking'].create({ + 'location_id': self.pack_location, + 'location_dest_id': self.customer_location, + 'partner_id': self.partner_delta_id, + 'picking_type_id': self.picking_type_out, + }) + + ship = self.MoveObj.create({ + 'name': self.productA.name, + 'product_id': self.productA.id, + 'product_uom_qty': 1, + 'product_uom': self.productA.uom_id.id, + 'picking_id': picking_ship.id, + 'location_id': self.output_location, + 'location_dest_id': self.customer_location, + }) + + picking_pack = self.env['stock.picking'].create({ + 'location_id': self.stock_location, + 'location_dest_id': self.pack_location, + 'partner_id': self.partner_delta_id, + 'picking_type_id': self.picking_type_out, + }) + + pack = self.MoveObj.create({ + 'name': self.productA.name, + 'product_id': self.productA.id, + 'product_uom_qty': 1, + 'product_uom': self.productA.uom_id.id, + 'picking_id': picking_pack.id, + 'location_id': self.pack_location, + 'location_dest_id': self.output_location, + 'move_dest_ids': [(4, ship.id)], + }) + + picking_pick = self.env['stock.picking'].create({ + 'location_id': self.stock_location, + 'location_dest_id': self.pack_location, + 'partner_id': self.partner_delta_id, + 'picking_type_id': self.picking_type_out, + }) + + self.MoveObj.create({ + 'name': self.productA.name, + 'product_id': self.productA.id, + 'product_uom_qty': 1, + 'product_uom': self.productA.uom_id.id, + 'picking_id': picking_pick.id, + 'location_id': self.stock_location, + 'location_dest_id': self.pack_location, + 'move_dest_ids': [(4, pack.id)], + 'state': 'confirmed', + }) + return picking_pick, picking_pack, picking_ship + def test_mto_moves(self): """ 10 in stock, do pick->ship and check ship is assigned when pick is done, then backorder of ship @@ -279,6 +335,178 @@ class TestPickShip(TestStockCommon): self.assertEqual(picking_client.state, 'assigned') self.assertEqual(picking_client.move_lines.reserved_availability, 5.0) + def test_pick_ship_return(self): + """ Create pick and ship. Bring it ot the customer and then return + it to stock. This test check the state and the quantity after each move in + order to ensure that it is correct. + """ + picking_pick, picking_ship = self.create_pick_ship() + stock_location = self.env['stock.location'].browse(self.stock_location) + pack_location = self.env['stock.location'].browse(self.pack_location) + customer_location = self.env['stock.location'].browse(self.customer_location) + self.productA.tracking = 'lot' + lot = self.env['stock.production.lot'].create({ + 'product_id': self.productA.id, + 'name': '123456789' + }) + self.env['stock.quant']._update_available_quantity(self.productA, stock_location, 10.0, lot_id=lot) + + picking_pick.action_assign() + picking_pick.move_lines[0].move_line_ids[0].qty_done = 10.0 + picking_pick.action_done() + self.assertEqual(picking_pick.state, 'done') + self.assertEqual(picking_ship.state, 'assigned') + + picking_ship.action_assign() + picking_ship.move_lines[0].move_line_ids[0].qty_done = 10.0 + picking_ship.action_done() + + customer_quantity = self.env['stock.quant']._get_available_quantity(self.productA, customer_location, lot_id=lot) + self.assertEqual(customer_quantity, 10, 'It should be one product in customer') + + """ First we create the return picking for pick pinking. + Since we do not have created the return between customer and + output. This return should not be available and should only have + picking pick as origin move. + """ + stock_return_picking = self.env['stock.return.picking']\ + .with_context(active_ids=picking_pick.ids, active_id=picking_pick.ids[0])\ + .create({}) + stock_return_picking.product_return_moves.quantity = 10.0 + stock_return_picking_action = stock_return_picking.create_returns() + return_pick_picking = self.env['stock.picking'].browse(stock_return_picking_action['res_id']) + + self.assertEqual(return_pick_picking.state, 'waiting') + + stock_return_picking = self.env['stock.return.picking']\ + .with_context(active_ids=picking_ship.ids, active_id=picking_ship.ids[0])\ + .create({}) + stock_return_picking.product_return_moves.quantity = 10.0 + stock_return_picking_action = stock_return_picking.create_returns() + return_ship_picking = self.env['stock.picking'].browse(stock_return_picking_action['res_id']) + + self.assertEqual(return_ship_picking.state, 'assigned', 'Return ship picking should automatically be assigned') + """ We created the return for ship picking. The origin/destination + link between return moves should have been created during return creation. + """ + self.assertTrue(return_ship_picking.move_lines in return_pick_picking.move_lines.mapped('move_orig_ids'), + 'The pick return picking\'s moves should have the ship return picking\'s moves as origin') + + self.assertTrue(return_pick_picking.move_lines in return_ship_picking.move_lines.mapped('move_dest_ids'), + 'The ship return picking\'s moves should have the pick return picking\'s moves as destination') + + return_ship_picking.move_lines[0].move_line_ids[0].write({ + 'qty_done': 10.0, + 'lot_id': lot.id, + }) + return_ship_picking.action_done() + self.assertEqual(return_ship_picking.state, 'done') + self.assertEqual(return_pick_picking.state, 'assigned') + + customer_quantity = self.env['stock.quant']._get_available_quantity(self.productA, customer_location, lot_id=lot) + self.assertEqual(customer_quantity, 0, 'It should be one product in customer') + + pack_quantity = self.env['stock.quant']._get_available_quantity(self.productA, pack_location, lot_id=lot) + self.assertEqual(pack_quantity, 0, 'It should be one product in pack location but is reserved') + + # Should use previous move lot. + return_pick_picking.move_lines[0].move_line_ids[0].qty_done = 10.0 + return_pick_picking.action_done() + self.assertEqual(return_pick_picking.state, 'done') + + stock_quantity = self.env['stock.quant']._get_available_quantity(self.productA, stock_location, lot_id=lot) + self.assertEqual(stock_quantity, 10, 'The product is not back in stock') + + def test_pick_pack_ship_return(self): + """ This test do a pick pack ship delivery to customer and then + return it to stock. Once everything is done, this test will check + if all the link orgini/destination between moves are correct. + """ + picking_pick, picking_pack, picking_ship = self.create_pick_pack_ship() + stock_location = self.env['stock.location'].browse(self.stock_location) + self.productA.tracking = 'serial' + lot = self.env['stock.production.lot'].create({ + 'product_id': self.productA.id, + 'name': '123456789' + }) + self.env['stock.quant']._update_available_quantity(self.productA, stock_location, 1.0, lot_id=lot) + + picking_pick.action_assign() + picking_pick.move_lines[0].move_line_ids[0].qty_done = 1.0 + picking_pick.action_done() + + picking_pack.action_assign() + picking_pack.move_lines[0].move_line_ids[0].qty_done = 1.0 + picking_pack.action_done() + + picking_ship.action_assign() + picking_ship.move_lines[0].move_line_ids[0].qty_done = 1.0 + picking_ship.action_done() + + stock_return_picking = self.env['stock.return.picking']\ + .with_context(active_ids=picking_ship.ids, active_id=picking_ship.ids[0])\ + .create({}) + stock_return_picking.product_return_moves.quantity = 1.0 + stock_return_picking_action = stock_return_picking.create_returns() + return_ship_picking = self.env['stock.picking'].browse(stock_return_picking_action['res_id']) + + return_ship_picking.move_lines[0].move_line_ids[0].write({ + 'qty_done': 1.0, + 'lot_id': lot.id, + }) + return_ship_picking.action_done() + + stock_return_picking = self.env['stock.return.picking']\ + .with_context(active_ids=picking_pack.ids, active_id=picking_pack.ids[0])\ + .create({}) + stock_return_picking.product_return_moves.quantity = 1.0 + stock_return_picking_action = stock_return_picking.create_returns() + return_pack_picking = self.env['stock.picking'].browse(stock_return_picking_action['res_id']) + + return_pack_picking.move_lines[0].move_line_ids[0].qty_done = 1.0 + return_pack_picking.action_done() + + stock_return_picking = self.env['stock.return.picking']\ + .with_context(active_ids=picking_pick.ids, active_id=picking_pick.ids[0])\ + .create({}) + stock_return_picking.product_return_moves.quantity = 1.0 + stock_return_picking_action = stock_return_picking.create_returns() + return_pick_picking = self.env['stock.picking'].browse(stock_return_picking_action['res_id']) + + return_pick_picking.move_lines[0].move_line_ids[0].qty_done = 1.0 + return_pick_picking.action_done() + + # Now that everything is returned we will check if the return moves are correctly linked between them. + # +--------------------------------------------------------------------------------------------------------+ + # | -- picking_pick(1) --> -- picking_pack(2) --> -- picking_ship(3) --> + # | Stock Pack Output Customer + # | <--- return pick(6) -- <--- return pack(5) -- <--- return ship(4) -- + # +--------------------------------------------------------------------------------------------------------+ + # Recaps of final link (MO = move_orig_ids, MD = move_dest_ids) + # picking_pick(1) : MO = (), MD = (2,6) + # picking_pack(2) : MO = (1), MD = (3,5) + # picking ship(3) : MO = (2), MD = (4) + # return ship(4) : MO = (3), MD = (5) + # return pack(5) : MO = (2, 4), MD = (6) + # return pick(6) : MO = (1, 5), MD = () + + self.assertEqual(len(picking_pick.move_lines.move_orig_ids), 0, 'Picking pick should not have origin moves') + self.assertEqual(set(picking_pick.move_lines.move_dest_ids.ids), set((picking_pack.move_lines | return_pick_picking.move_lines).ids)) + + self.assertEqual(set(picking_pack.move_lines.move_orig_ids.ids), set(picking_pick.move_lines.ids)) + self.assertEqual(set(picking_pack.move_lines.move_dest_ids.ids), set((picking_ship.move_lines | return_pack_picking.move_lines).ids)) + + self.assertEqual(set(picking_ship.move_lines.move_orig_ids.ids), set(picking_pack.move_lines.ids)) + self.assertEqual(set(picking_ship.move_lines.move_dest_ids.ids), set(return_ship_picking.move_lines.ids)) + + self.assertEqual(set(return_ship_picking.move_lines.move_orig_ids.ids), set(picking_ship.move_lines.ids)) + self.assertEqual(set(return_ship_picking.move_lines.move_dest_ids.ids), set(return_pack_picking.move_lines.ids)) + + self.assertEqual(set(return_pack_picking.move_lines.move_orig_ids.ids), set((picking_pack.move_lines | return_ship_picking.move_lines).ids)) + self.assertEqual(set(return_pack_picking.move_lines.move_dest_ids.ids), set(return_pick_picking.move_lines.ids)) + + self.assertEqual(set(return_pick_picking.move_lines.move_orig_ids.ids), set((picking_pick.move_lines | return_pack_picking.move_lines).ids)) + self.assertEqual(len(return_pick_picking.move_lines.move_dest_ids), 0) class TestSinglePicking(TestStockCommon): def test_backorder_1(self): diff --git a/addons/stock/wizard/stock_picking_return.py b/addons/stock/wizard/stock_picking_return.py index 9afd8026193..e82f63d1070 100644 --- a/addons/stock/wizard/stock_picking_return.py +++ b/addons/stock/wizard/stock_picking_return.py @@ -110,8 +110,19 @@ class ReturnPicking(models.TransientModel): returned_lines += 1 vals = self._prepare_move_default_values(return_line, new_picking) r = return_line.move_id.copy(vals) - r.write({'move_orig_ids': [(4, return_line.move_id.id, False)]}) + vals = {} + # +--------------------------------------------------------------------------------------------------------+ + # | picking_pick <--Move Orig-- picking_pack --Move Dest--> picking_ship + # | | returned_move_ids ↑ | returned_move_ids + # | ↓ | return_line.move_id ↓ + # | return pick(Add as dest) return toLink return ship(Add as orig) + # +--------------------------------------------------------------------------------------------------------+ + move_orig_to_link = return_line.move_id.move_dest_ids.mapped('returned_move_ids') + move_dest_to_link = return_line.move_id.move_orig_ids.mapped('returned_move_ids') + vals['move_orig_ids'] = [(4, m.id) for m in move_orig_to_link | return_line.move_id] + vals['move_dest_ids'] = [(4, m.id) for m in move_dest_to_link] + r.write(vals) if not returned_lines: raise UserError(_("Please specify at least one non-zero quantity."))