From dc10774d52a3e8a326470ce21ba3441382a47b57 Mon Sep 17 00:00:00 2001 From: yhu-odoo Date: Mon, 20 Jan 2020 13:01:30 +0000 Subject: [PATCH] [FIX] stock, mrp: inter company transit chained moves Let's say we have a chain of move wh1 - intercomp transit -> intercomp transit - wh2 The second move will be reserved according to what the first move brought since they are chained. This behavior resulted in rev[0] which tries to work around the ir.rule limiting the access of stock.move and stock.move.line records in multi-company environment. This patch wasn't perfect since, if the first move brought a lot, the second move will reserve this lot and it will result in another access error since the lot will still have the company of the first move. We fix this by implementing the following logic: receiving from another company should behave the same as receiving from the supplier, no reservation is applied. We fix this by marking the inter company transit as `_should_bypass_reservation` and we break the move chain if the pull/push rule create an intercompany chain. [0] 6ff34073153767d449804669f31e26c04de0a670 closes odoo/odoo#45601 Task: 2160847 X-original-commit: 67b45da9d877a5a1bb9adb3ce14051e01f67045f Signed-off-by: Simon Lejeune (sle) --- addons/mrp/models/stock_move.py | 4 + addons/mrp/tests/test_workorder_operation.py | 2 + addons/stock/models/stock_location.py | 2 +- addons/stock/models/stock_move.py | 7 +- addons/stock/models/stock_picking.py | 3 - addons/stock/models/stock_production_lot.py | 4 +- addons/stock/models/stock_rule.py | 12 +- addons/stock/security/stock_security.xml | 4 +- addons/stock/tests/test_move.py | 48 +---- addons/stock/tests/test_multicompany.py | 210 +++++++++++++++++++ 10 files changed, 238 insertions(+), 58 deletions(-) diff --git a/addons/mrp/models/stock_move.py b/addons/mrp/models/stock_move.py index e9a32f38f2e..1d3cb98a94f 100644 --- a/addons/mrp/models/stock_move.py +++ b/addons/mrp/models/stock_move.py @@ -252,6 +252,10 @@ class StockMove(models.Model): res = super(StockMove, self)._should_be_assigned() return bool(res and not (self.production_id or self.raw_material_production_id)) + def _should_bypass_reservation(self): + res = super(StockMove, self)._should_bypass_reservation() + return bool(res and not self.production_id) + def _key_assign_picking(self): keys = super(StockMove, self)._key_assign_picking() return keys + (self.created_production_id,) diff --git a/addons/mrp/tests/test_workorder_operation.py b/addons/mrp/tests/test_workorder_operation.py index d6acdf1db76..c1125633bf1 100644 --- a/addons/mrp/tests/test_workorder_operation.py +++ b/addons/mrp/tests/test_workorder_operation.py @@ -1766,6 +1766,7 @@ class TestRoutingAndKits(SavepointCase): self.bom_finished1.consumption = 'flexible' add_product = self.env['product.product'].create({ 'name': 'Additional', + 'type': 'product', }) mo_form = Form(self.env['mrp.production']) mo_form.product_id = self.finished1 @@ -1799,6 +1800,7 @@ class TestRoutingAndKits(SavepointCase): self.bom_finished1.consumption = 'flexible' add_product = self.env['product.product'].create({ 'name': 'Additional', + 'type': 'product', }) mo_form = Form(self.env['mrp.production']) mo_form.product_id = self.finished1 diff --git a/addons/stock/models/stock_location.py b/addons/stock/models/stock_location.py index 822773beb01..f0189addc91 100644 --- a/addons/stock/models/stock_location.py +++ b/addons/stock/models/stock_location.py @@ -157,7 +157,7 @@ class Location(models.Model): def should_bypass_reservation(self): self.ensure_one() - return self.usage in ('supplier', 'customer', 'inventory', 'production') or self.scrap_location + return self.usage in ('supplier', 'customer', 'inventory', 'production') or self.scrap_location or (self.usage == 'transit' and not self.company_id) class Route(models.Model): diff --git a/addons/stock/models/stock_move.py b/addons/stock/models/stock_move.py index dd93a25a484..8aa76e8d2d3 100644 --- a/addons/stock/models/stock_move.py +++ b/addons/stock/models/stock_move.py @@ -1016,9 +1016,12 @@ class StockMove(models.Model): moves._assign_picking() self._push_apply() self._check_company() + moves = self if merge: - return self._merge_moves(merge_into=merge_into) - return self + moves = self._merge_moves(merge_into=merge_into) + # call `_action_assign` on every confirmed move which location_id bypasses the reservation + moves.filtered(lambda move: move._should_bypass_reservation() and move.state == 'confirmed')._action_assign() + return moves def _prepare_procurement_values(self): """ Prepare specific key for moves or other componenets that will be created from a stock rule diff --git a/addons/stock/models/stock_picking.py b/addons/stock/models/stock_picking.py index de8e9fa144a..b18920d4435 100644 --- a/addons/stock/models/stock_picking.py +++ b/addons/stock/models/stock_picking.py @@ -657,9 +657,6 @@ class Picking(models.Model): self.mapped('move_lines')\ .filtered(lambda move: move.state == 'draft')\ ._action_confirm() - # call `_action_assign` on every confirmed move which location_id bypasses the reservation - self.filtered(lambda picking: not picking.immediate_transfer and picking.location_id.usage in ('supplier', 'inventory', 'production') and picking.state == 'confirmed')\ - .mapped('move_lines')._action_assign() return True def action_assign(self): diff --git a/addons/stock/models/stock_production_lot.py b/addons/stock/models/stock_production_lot.py index 2ac426d33f5..40df922447c 100644 --- a/addons/stock/models/stock_production_lot.py +++ b/addons/stock/models/stock_production_lot.py @@ -7,7 +7,7 @@ from odoo.exceptions import UserError class ProductionLot(models.Model): _name = 'stock.production.lot' - _inherit = ['mail.thread','mail.activity.mixin'] + _inherit = ['mail.thread', 'mail.activity.mixin'] _description = 'Lot/Serial' _check_company_auto = True @@ -85,7 +85,7 @@ class ProductionLot(models.Model): def _product_qty(self): for lot in self: # We only care for the quants in internal or transit locations. - quants = lot.quant_ids.filtered(lambda q: q.location_id.usage in ['internal', 'transit']) + quants = lot.quant_ids.filtered(lambda q: q.location_id.usage == 'internal' or (q.location_id.usage == 'transit' and q.location_id.company_id)) lot.product_qty = sum(quants.mapped('quantity')) def action_lot_open_quants(self): diff --git a/addons/stock/models/stock_rule.py b/addons/stock/models/stock_rule.py index 899343db755..b4bbbb07ef1 100644 --- a/addons/stock/models/stock_rule.py +++ b/addons/stock/models/stock_rule.py @@ -189,7 +189,10 @@ class StockRule(models.Model): else: new_move_vals = self._push_prepare_move_copy_values(move, new_date) new_move = move.sudo().copy(new_move_vals) - move.write({'move_dest_ids': [(4, new_move.id)]}) + if new_move._should_bypass_reservation(): + new_move.write({'procure_method': 'make_to_stock'}) + if not new_move.location_id.should_bypass_reservation(): + move.write({'move_dest_ids': [(4, new_move.id)]}) new_move._action_confirm() def _push_prepare_move_copy_values(self, move_to_copy, new_date): @@ -283,6 +286,11 @@ class StockRule(models.Model): # it is possible that we've already got some move done, so check for the done qty and create # a new move with the correct qty qty_left = product_qty + + move_dest_ids = [] + if not self.location_id.should_bypass_reservation(): + move_dest_ids = values.get('move_dest_ids', False) and [(4, x.id) for x in values['move_dest_ids']] or [] + move_values = { 'name': name[:2000], 'company_id': self.company_id.id or self.location_src_id.company_id.id or self.location_id.company_id.id or company_id.id, @@ -292,7 +300,7 @@ class StockRule(models.Model): 'partner_id': self.partner_address_id.id or (values.get('group_id', False) and values['group_id'].partner_id.id) or False, 'location_id': self.location_src_id.id, 'location_dest_id': location_id.id, - 'move_dest_ids': values.get('move_dest_ids', False) and [(4, x.id) for x in values['move_dest_ids']] or [], + 'move_dest_ids': move_dest_ids, 'rule_id': self.id, 'procure_method': self.procure_method, 'origin': origin, diff --git a/addons/stock/security/stock_security.xml b/addons/stock/security/stock_security.xml index bbf0a2714f5..e6364e02c36 100644 --- a/addons/stock/security/stock_security.xml +++ b/addons/stock/security/stock_security.xml @@ -116,14 +116,14 @@ stock_move multi-company - ['|', ('company_id', 'in', company_ids), '&', ('location_dest_id.company_id', '=', False), ('location_dest_id.usage', '=', 'transit')] + [('company_id', 'in', company_ids)] stock_move_line multi-company - ['|','|',('company_id','=',False),('company_id', 'in', company_ids),'&',('location_dest_id.company_id', '=', False), ('location_dest_id.usage', '=', 'transit')] + ['|',('company_id','=',False),('company_id', 'in', company_ids)] diff --git a/addons/stock/tests/test_move.py b/addons/stock/tests/test_move.py index 2ea2047baf0..cb0583a4101 100644 --- a/addons/stock/tests/test_move.py +++ b/addons/stock/tests/test_move.py @@ -67,10 +67,6 @@ class StockMove(SavepointCase): # confirmation move1._action_confirm() - self.assertEqual(move1.state, 'confirmed') - - # assignment - move1._action_assign() self.assertEqual(move1.state, 'assigned') self.assertEqual(len(move1.move_line_ids), 1) @@ -109,10 +105,6 @@ class StockMove(SavepointCase): # confirmation move1._action_confirm() - self.assertEqual(move1.state, 'confirmed') - - # assignment - move1._action_assign() self.assertEqual(move1.state, 'assigned') self.assertEqual(len(move1.move_line_ids), 1) move_line = move1.move_line_ids[0] @@ -154,10 +146,6 @@ class StockMove(SavepointCase): # confirmation move1._action_confirm() - self.assertEqual(move1.state, 'confirmed') - - # assignment - move1._action_assign() self.assertEqual(move1.state, 'assigned') self.assertEqual(len(move1.move_line_ids), 5) move_line = move1.move_line_ids[0] @@ -260,10 +248,6 @@ class StockMove(SavepointCase): # confirmation move1._action_confirm() - self.assertEqual(move1.state, 'confirmed') - - # assignment - move1._action_assign() self.assertEqual(move1.state, 'assigned') self.assertEqual(len(move1.move_line_ids), 1) self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product, self.stock_location), 0.0) @@ -714,10 +698,6 @@ class StockMove(SavepointCase): 'product_uom_qty': 100.0, }) move1._action_confirm() - self.assertEqual(move1.state, 'confirmed') - - # assignment - move1._action_assign() self.assertEqual(move1.state, 'assigned') self.assertEqual(len(move1.move_line_ids), 1) @@ -755,10 +735,6 @@ class StockMove(SavepointCase): 'product_uom_qty': 100.0, }) move1._action_confirm() - self.assertEqual(move1.state, 'confirmed') - - # assignment - move1._action_assign() self.assertEqual(move1.state, 'assigned') self.assertEqual(len(move1.move_line_ids), 1) @@ -809,10 +785,6 @@ class StockMove(SavepointCase): 'product_uom_qty': 100.0, }) move1._action_confirm() - self.assertEqual(move1.state, 'confirmed') - - # assignment - move1._action_assign() self.assertEqual(move1.state, 'assigned') self.assertEqual(len(move1.move_line_ids), 1) @@ -865,10 +837,6 @@ class StockMove(SavepointCase): 'product_uom_qty': 100.0, }) move1._action_confirm() - self.assertEqual(move1.state, 'confirmed') - - # assignment - move1._action_assign() self.assertEqual(move1.state, 'assigned') self.assertEqual(len(move1.move_line_ids), 1) @@ -908,10 +876,6 @@ class StockMove(SavepointCase): 'product_uom_qty': 100.0, }) move1._action_confirm() - self.assertEqual(move1.state, 'confirmed') - - # assignment - move1._action_assign() self.assertEqual(move1.state, 'assigned') self.assertEqual(len(move1.move_line_ids), 1) @@ -967,10 +931,6 @@ class StockMove(SavepointCase): 'product_uom_qty': 100.0, }) move1._action_confirm() - self.assertEqual(move1.state, 'confirmed') - - # assignment - move1._action_assign() self.assertEqual(move1.state, 'assigned') self.assertEqual(len(move1.move_line_ids), 1) @@ -1020,10 +980,6 @@ class StockMove(SavepointCase): 'product_uom_qty': 100.0, }) move1._action_confirm() - self.assertEqual(move1.state, 'confirmed') - - # assignment - move1._action_assign() self.assertEqual(move1.state, 'assigned') self.assertEqual(len(move1.move_line_ids), 1) @@ -1921,7 +1877,7 @@ class StockMove(SavepointCase): (move_supp_stock_1 + move_supp_stock_2 + move_stock_stock_1 + move_stock_stock_2)._action_confirm() move_supp_stock_1._action_assign() self.assertEqual(move_supp_stock_1.state, 'assigned') - self.assertEqual(move_supp_stock_2.state, 'confirmed') + self.assertEqual(move_supp_stock_2.state, 'assigned') self.assertEqual(move_stock_stock_1.state, 'waiting') self.assertEqual(move_stock_stock_2.state, 'waiting') self.assertEqual(self.env['stock.quant']._get_available_quantity(self.product, self.stock_location), 0.0) @@ -1931,7 +1887,7 @@ class StockMove(SavepointCase): move_supp_stock_1.move_line_ids.qty_done = 3.0 move_supp_stock_1._action_done() self.assertEqual(move_supp_stock_1.state, 'done') - self.assertEqual(move_supp_stock_2.state, 'confirmed') + self.assertEqual(move_supp_stock_2.state, 'assigned') self.assertEqual(move_stock_stock_1.state, 'assigned') self.assertEqual(move_stock_stock_2.state, 'waiting') diff --git a/addons/stock/tests/test_multicompany.py b/addons/stock/tests/test_multicompany.py index 3ebc0f0c6f9..44f981f26eb 100644 --- a/addons/stock/tests/test_multicompany.py +++ b/addons/stock/tests/test_multicompany.py @@ -386,3 +386,213 @@ class TestMultiCompany(SavepointCase): }) with self.assertRaises(UserError): move._action_confirm() + + def test_intercom_lot_push(self): + """ Create a push rule to transfer products received in inter company + transit location to company b. Move a lot product from company a to the + transit location. Check the move created by the push rule is not chained + with previous move, and no product are reserved from inter-company + transit. """ + supplier_location = self.env.ref('stock.stock_location_suppliers') + intercom_location = self.env.ref('stock.stock_location_inter_wh') + intercom_location.write({'active': True}) + + product_lot = self.env['product.product'].create({ + 'type': 'product', + 'tracking': 'lot', + 'name': 'product lot', + }) + + picking_type_to_transit = self.env['stock.picking.type'].create({ + 'name': 'To Transit', + 'sequence_code': 'TRANSIT', + 'code': 'outgoing', + 'company_id': self.company_a.id, + 'warehouse_id': False, + 'default_location_src_id': self.stock_location_a.id, + 'default_location_dest_id': intercom_location.id, + 'sequence_id': self.env['ir.sequence'].create({ + 'code': 'transit', + 'name': 'transit sequence', + 'company_id': self.company_a.id, + }).id, + }) + + route = self.env['stock.location.route'].create({ + 'name': 'Push', + 'company_id': False, + 'rule_ids': [(0, False, { + 'name': 'create a move to company b', + 'company_id': self.company_b.id, + 'location_src_id': intercom_location.id, + 'location_id': self.stock_location_b.id, + 'action': 'push', + 'auto': 'manual', + 'picking_type_id': self.warehouse_b.in_type_id.id, + })], + }) + + move_from_supplier = self.env['stock.move'].create({ + 'company_id': self.company_a.id, + 'name': 'test_from_supplier', + 'location_id': supplier_location.id, + 'location_dest_id': self.stock_location_a.id, + 'product_id': product_lot.id, + 'product_uom': product_lot.uom_id.id, + 'product_uom_qty': 1.0, + 'picking_type_id': self.warehouse_a.in_type_id.id, + }) + move_from_supplier._action_confirm() + move_line_1 = move_from_supplier.move_line_ids[0] + move_line_1.lot_name = 'lot 1' + move_line_1.qty_done = 1.0 + move_from_supplier._action_done() + lot_1 = move_line_1.lot_id + + move_to_transit = self.env['stock.move'].create({ + 'company_id': self.company_a.id, + 'name': 'test_to_transit', + 'location_id': self.stock_location_a.id, + 'location_dest_id': intercom_location.id, + 'product_id': product_lot.id, + 'product_uom': product_lot.uom_id.id, + 'product_uom_qty': 1.0, + 'picking_type_id': picking_type_to_transit.id, + 'route_ids': [(4, route.id)], + }) + move_to_transit._action_confirm() + move_to_transit._action_assign() + move_line_2 = move_to_transit.move_line_ids[0] + self.assertTrue(move_line_2.lot_id, move_line_1.lot_id) + move_line_2.qty_done = 1.0 + move_to_transit._action_done() + + move_push = self.env['stock.move'].search([('location_id', '=', intercom_location.id), + ('product_id', '=', product_lot.id)]) + self.assertTrue(move_push, 'No move created from push rules') + self.assertEqual(move_push.state, "assigned") + self.assertTrue(move_push.move_line_ids, "No move line created for the move") + self.assertFalse(move_push in move_to_transit.move_dest_ids, + "Chained move created in transit location") + self.assertNotEqual(move_push.move_line_ids.lot_id, move_line_2.lot_id, + "Reserved from transit location") + picking_receipt = move_push.picking_id + with self.assertRaises(UserError): + picking_receipt.button_validate() + + move_line_3 = move_push.move_line_ids[0] + move_line_3.lot_name = 'lot 2' + move_line_3.qty_done = 1.0 + picking_receipt.button_validate() + lot_2 = move_line_3.lot_id + self.assertEqual(lot_1.company_id, self.company_a) + self.assertEqual(lot_1.name, 'lot 1') + self.assertEqual(self.env['stock.quant']._get_available_quantity(product_lot, intercom_location, lot_1), 1.0) + self.assertEqual(lot_2.company_id, self.company_b) + self.assertEqual(lot_2.name, 'lot 2') + self.assertEqual(self.env['stock.quant']._get_available_quantity(product_lot, self.stock_location_b, lot_2), 1.0) + + def test_intercom_lot_pull(self): + """Use warehouse of comany a to resupply warehouse of company b. Check + pull rule works correctly in two companies and moves are unchained from + inter-company transit location.""" + customer_location = self.env.ref('stock.stock_location_customers') + supplier_location = self.env.ref('stock.stock_location_suppliers') + intercom_location = self.env.ref('stock.stock_location_inter_wh') + intercom_location.write({'active': True}) + partner = self.env['res.partner'].create({'name': 'Deco Addict'}) + self.warehouse_a.resupply_wh_ids = [(6, 0, [self.warehouse_b.id])] + resupply_route = self.env['stock.location.route'].search([('supplier_wh_id', '=', self.warehouse_b.id), + ('supplied_wh_id', '=', self.warehouse_a.id)]) + self.assertTrue(resupply_route, "Resupply route not found") + + product_lot = self.env['product.product'].create({ + 'type': 'product', + 'tracking': 'lot', + 'name': 'product lot', + 'route_ids': [(4, resupply_route.id), (4, self.env.ref('stock.route_warehouse0_mto').id)], + }) + + move_sup_to_whb = self.env['stock.move'].create({ + 'company_id': self.company_b.id, + 'name': 'from_supplier_to_whb', + 'location_id': supplier_location.id, + 'location_dest_id': self.warehouse_b.lot_stock_id.id, + 'product_id': product_lot.id, + 'product_uom': product_lot.uom_id.id, + 'product_uom_qty': 1.0, + 'picking_type_id': self.warehouse_b.in_type_id.id, + }) + move_sup_to_whb._action_confirm() + move_line_1 = move_sup_to_whb.move_line_ids[0] + move_line_1.lot_name = 'lot b' + move_line_1.qty_done = 1.0 + move_sup_to_whb._action_done() + lot_b = move_line_1.lot_id + + picking_out = self.env['stock.picking'].create({ + 'company_id': self.company_a.id, + 'partner_id': partner.id, + 'picking_type_id': self.warehouse_a.out_type_id.id, + 'location_id': self.stock_location_a.id, + 'location_dest_id': customer_location.id, + }) + move_wha_to_cus = self.env['stock.move'].create({ + 'name': "WH_A to Customer", + 'product_id': product_lot.id, + 'product_uom_qty': 1, + 'product_uom': product_lot.uom_id.id, + 'picking_id': picking_out.id, + 'location_id': self.stock_location_a.id, + 'location_dest_id': customer_location.id, + 'warehouse_id': self.warehouse_a.id, + 'procure_method': 'make_to_order', + 'company_id': self.company_a.id, + }) + picking_out.action_confirm() + + move_whb_to_transit = self.env['stock.move'].search([('location_id', '=', self.stock_location_b.id), + ('product_id', '=', product_lot.id)]) + move_transit_to_wha = self.env['stock.move'].search([('location_id', '=', intercom_location.id), + ('product_id', '=', product_lot.id)]) + self.assertTrue(move_whb_to_transit, "No move created by pull rule") + self.assertTrue(move_transit_to_wha, "No move created by pull rule") + self.assertTrue(move_wha_to_cus in move_transit_to_wha.move_dest_ids, + "Moves are not chained") + self.assertFalse(move_transit_to_wha in move_whb_to_transit.move_dest_ids, + "Chained move created in transit location") + self.assertEqual(move_wha_to_cus.state, "waiting") + self.assertEqual(move_transit_to_wha.state, "waiting") + self.assertEqual(move_whb_to_transit.state, "confirmed") + + (move_wha_to_cus + move_whb_to_transit + move_transit_to_wha).picking_id.action_assign() + self.assertEqual(move_wha_to_cus.state, "waiting") + self.assertEqual(move_transit_to_wha.state, "assigned") + self.assertEqual(move_whb_to_transit.state, "assigned") + + res_dict = move_whb_to_transit.picking_id.button_validate() + self.assertEqual(res_dict.get('res_model'), 'stock.immediate.transfer') + wizard = Form(self.env[res_dict['res_model']].with_context(res_dict['context'])).save() + wizard.process() + self.assertEqual(self.env['stock.quant']._get_available_quantity(product_lot, intercom_location, lot_b), 1.0) + with self.assertRaises(UserError): + move_transit_to_wha.picking_id.button_validate() + + move_line_2 = move_transit_to_wha.move_line_ids[0] + move_line_2.lot_name = 'lot a' + move_line_2.qty_done = 1.0 + move_transit_to_wha._action_done() + lot_a = move_line_2.lot_id + + move_wha_to_cus._action_assign() + self.assertEqual(move_wha_to_cus.state, "assigned") + res_dict = move_wha_to_cus.picking_id.button_validate() + self.assertEqual(res_dict.get('res_model'), 'stock.immediate.transfer') + wizard = Form(self.env[res_dict['res_model']].with_context(res_dict['context'])).save() + wizard.process() + self.assertEqual(self.env['stock.quant']._get_available_quantity(product_lot, customer_location, lot_a), 1.0) + + self.assertEqual(lot_a.company_id, self.company_a) + self.assertEqual(lot_a.name, 'lot a') + self.assertEqual(lot_b.company_id, self.company_b) + self.assertEqual(lot_b.name, 'lot b')