From 8ff484574a654bc6de290244f7e77e2aa100bb6e Mon Sep 17 00:00:00 2001 From: "Adrien Widart (awt)" Date: Wed, 24 May 2023 10:46:55 +0000 Subject: [PATCH] [FIX] stock, *: write on WH with deleted routes *: mrp{,_subcontracting{,_dropshipping}}, purchase_stock When a route is deleted, it is no more possible to write on a warehouse To reproduce the issue: 1. In Settings, enable "Multi Steps Route" 2. Delete the route "Buy" 3. Edit the warehouse: - Receipt: 3 steps Error: a UserError is displayed because of the missing buy route, which does not make sense, the warehouse receipt should be updated Even worse: step 3, try to install `mrp_subcontracting` and it will lead to a parse error. It comes from: https://github.com/odoo/odoo/blob/3f389b4769d947b52985c0a15eae5be59b1e28f7/addons/mrp_subcontracting/data/mrp_subcontracting_data.xml#L10-L13 Again, we try to write on the warehouse -> not possible When writing on a warehouse, we will check if we have to create/update some rules: https://github.com/odoo/odoo/blob/3b801a6d48f1feffd1b87a7d54731ab58e8d63e9/addons/stock/models/stock_warehouse.py#L202-L206 We will then try to get the buy route https://github.com/odoo/odoo/blob/1b525febfab839fdb9d6ff66107a9c0c833b92be/addons/purchase_stock/models/stock.py#L35 Which will lead to the user error https://github.com/odoo/odoo/blob/3b801a6d48f1feffd1b87a7d54731ab58e8d63e9/addons/stock/models/stock_warehouse.py#L379 sentry-4128085959 closes odoo/odoo#122743 X-original-commit: 4323e2ec5d316fe83a0184c97230d21ea122ef99 Signed-off-by: William Henrotin (whe) Signed-off-by: Adrien Widart (awt) --- addons/mrp/models/stock_warehouse.py | 12 ++++++------ .../mrp_subcontracting/models/stock_warehouse.py | 8 ++++---- .../models/stock_warehouse.py | 6 +++--- addons/purchase_stock/models/stock.py | 6 +++--- addons/purchase_stock/tests/test_routes.py | 15 +++++++++++++++ addons/stock/models/stock_warehouse.py | 12 +++++++++--- 6 files changed, 40 insertions(+), 19 deletions(-) diff --git a/addons/mrp/models/stock_warehouse.py b/addons/mrp/models/stock_warehouse.py index ccbe1ae90b2..2876be71cb0 100644 --- a/addons/mrp/models/stock_warehouse.py +++ b/addons/mrp/models/stock_warehouse.py @@ -104,8 +104,8 @@ class StockWarehouse(models.Model): else: return super(StockWarehouse, self)._get_route_name(route_type) - def _get_global_route_rules_values(self): - rules = super(StockWarehouse, self)._get_global_route_rules_values() + def _generate_global_route_rules_values(self): + rules = super()._generate_global_route_rules_values() location_src = self.manufacture_steps == 'mrp_one_step' and self.lot_stock_id or self.pbm_loc_id production_location = self._get_production_location() location_dest_id = self.manufacture_steps == 'pbm_sam' and self.sam_loc_id or self.lot_stock_id @@ -117,7 +117,7 @@ class StockWarehouse(models.Model): 'procure_method': 'make_to_order', 'company_id': self.company_id.id, 'picking_type_id': self.manu_type_id.id, - 'route_id': self._find_global_route('mrp.route_warehouse0_manufacture', _('Manufacture')).id + 'route_id': self._find_global_route('mrp.route_warehouse0_manufacture', _('Manufacture'), raise_if_not_found=False).id }, 'update_values': { 'active': self.manufacture_to_resupply, @@ -133,7 +133,7 @@ class StockWarehouse(models.Model): 'company_id': self.company_id.id, 'action': 'pull', 'auto': 'manual', - 'route_id': self._find_global_route('stock.route_warehouse0_mto', _('Replenish on Order (MTO)')).id, + 'route_id': self._find_global_route('stock.route_warehouse0_mto', _('Replenish on Order (MTO)'), raise_if_not_found=False).id, 'location_dest_id': production_location.id, 'location_src_id': location_src.id, 'picking_type_id': self.manu_type_id.id @@ -150,7 +150,7 @@ class StockWarehouse(models.Model): 'company_id': self.company_id.id, 'action': 'pull', 'auto': 'manual', - 'route_id': self._find_global_route('stock.route_warehouse0_mto', _('Replenish on Order (MTO)')).id, + 'route_id': self._find_global_route('stock.route_warehouse0_mto', _('Replenish on Order (MTO)'), raise_if_not_found=False).id, 'name': self._format_rulename(self.lot_stock_id, self.pbm_loc_id, 'MTO'), 'location_dest_id': self.pbm_loc_id.id, 'location_src_id': self.lot_stock_id.id, @@ -173,7 +173,7 @@ class StockWarehouse(models.Model): 'company_id': self.company_id.id, 'action': 'pull', 'auto': 'manual', - 'route_id': self._find_global_route('mrp.route_warehouse0_manufacture', _('Manufacture')).id, + 'route_id': self._find_global_route('mrp.route_warehouse0_manufacture', _('Manufacture'), raise_if_not_found=False).id, 'name': self._format_rulename(self.sam_loc_id, self.lot_stock_id, False), 'location_dest_id': self.lot_stock_id.id, 'location_src_id': self.sam_loc_id.id, diff --git a/addons/mrp_subcontracting/models/stock_warehouse.py b/addons/mrp_subcontracting/models/stock_warehouse.py index 7a1dbf12b0b..5fd3e6c6b1b 100644 --- a/addons/mrp_subcontracting/models/stock_warehouse.py +++ b/addons/mrp_subcontracting/models/stock_warehouse.py @@ -86,8 +86,8 @@ class StockWarehouse(models.Model): }) return routes - def _get_global_route_rules_values(self): - rules = super(StockWarehouse, self)._get_global_route_rules_values() + def _generate_global_route_rules_values(self): + rules = super()._generate_global_route_rules_values() subcontract_location_id = self._get_subcontracting_location() production_location_id = self._get_production_location() rules.update({ @@ -98,7 +98,7 @@ class StockWarehouse(models.Model): 'company_id': self.company_id.id, 'action': 'pull', 'auto': 'manual', - 'route_id': self._find_global_route('stock.route_warehouse0_mto', _('Replenish on Order (MTO)')).id, + 'route_id': self._find_global_route('stock.route_warehouse0_mto', _('Replenish on Order (MTO)'), raise_if_not_found=False).id, 'name': self._format_rulename(self.lot_stock_id, subcontract_location_id, 'MTO'), 'location_dest_id': subcontract_location_id.id, 'location_src_id': self.lot_stock_id.id, @@ -116,7 +116,7 @@ class StockWarehouse(models.Model): 'action': 'pull', 'auto': 'manual', 'route_id': self._find_global_route('mrp_subcontracting.route_resupply_subcontractor_mto', - _('Resupply Subcontractor on Order')).id, + _('Resupply Subcontractor on Order'), raise_if_not_found=False).id, 'name': self._format_rulename(subcontract_location_id, production_location_id, False), 'location_dest_id': production_location_id.id, 'location_src_id': subcontract_location_id.id, diff --git a/addons/mrp_subcontracting_dropshipping/models/stock_warehouse.py b/addons/mrp_subcontracting_dropshipping/models/stock_warehouse.py index 04498fb55f6..5f7592a2b16 100644 --- a/addons/mrp_subcontracting_dropshipping/models/stock_warehouse.py +++ b/addons/mrp_subcontracting_dropshipping/models/stock_warehouse.py @@ -67,8 +67,8 @@ class StockWarehouse(models.Model): route_id.active = bool(all_rules.filtered(lambda r: r.action == 'pull')) - def _get_global_route_rules_values(self): - rules = super()._get_global_route_rules_values() + def _generate_global_route_rules_values(self): + rules = super()._generate_global_route_rules_values() subcontract_location_id = self._get_subcontracting_location() production_location_id = self._get_production_location() rules.update({ @@ -80,7 +80,7 @@ class StockWarehouse(models.Model): 'action': 'pull', 'auto': 'manual', 'route_id': self._find_global_route('mrp_subcontracting_dropshipping.route_subcontracting_dropshipping', - _('Dropship Subcontractor on Order')).id, + _('Dropship Subcontractor on Order'), raise_if_not_found=False).id, 'name': self._format_rulename(subcontract_location_id, production_location_id, False), 'location_dest_id': production_location_id.id, 'location_src_id': subcontract_location_id.id, diff --git a/addons/purchase_stock/models/stock.py b/addons/purchase_stock/models/stock.py index b77048c64bf..b8d0b7a8a11 100644 --- a/addons/purchase_stock/models/stock.py +++ b/addons/purchase_stock/models/stock.py @@ -21,8 +21,8 @@ class StockWarehouse(models.Model): help="When products are bought, they can be delivered to this warehouse") buy_pull_id = fields.Many2one('stock.rule', 'Buy rule') - def _get_global_route_rules_values(self): - rules = super(StockWarehouse, self)._get_global_route_rules_values() + def _generate_global_route_rules_values(self): + rules = super()._generate_global_route_rules_values() location_id = self.in_type_id.default_location_dest_id rules.update({ 'buy_pull_id': { @@ -32,7 +32,7 @@ class StockWarehouse(models.Model): 'picking_type_id': self.in_type_id.id, 'group_propagation_option': 'none', 'company_id': self.company_id.id, - 'route_id': self._find_global_route('purchase_stock.route_warehouse0_buy', _('Buy')).id, + 'route_id': self._find_global_route('purchase_stock.route_warehouse0_buy', _('Buy'), raise_if_not_found=False).id, 'propagate_cancel': self.reception_steps != 'one_step', }, 'update_values': { diff --git a/addons/purchase_stock/tests/test_routes.py b/addons/purchase_stock/tests/test_routes.py index 760e7fe8db8..9d3a66b5646 100644 --- a/addons/purchase_stock/tests/test_routes.py +++ b/addons/purchase_stock/tests/test_routes.py @@ -51,3 +51,18 @@ class TestRoutes(TransactionCase): line.name = 'second rule' line.action = 'buy' line.picking_type_id = receipt_2 + + def test_delete_buy_route(self): + """ + The user should be able to write on a warehouse even if the buy route + does not exist anymore + """ + wh = self.env['stock.warehouse'].search([], limit=1) + + buy_routes = self.env['stock.route'].search([('name', 'ilike', 'buy')]) + self.assertTrue(buy_routes) + + buy_routes.unlink() + + wh.reception_steps = 'two_steps' + self.assertEqual(wh.reception_steps, 'two_steps') diff --git a/addons/stock/models/stock_warehouse.py b/addons/stock/models/stock_warehouse.py index 2f94eb59b1a..77ffa6bd102 100644 --- a/addons/stock/models/stock_warehouse.py +++ b/addons/stock/models/stock_warehouse.py @@ -370,12 +370,12 @@ class Warehouse(models.Model): self[rule_field] = self.env['stock.rule'].create(values) return True - def _find_global_route(self, xml_id, route_name): + def _find_global_route(self, xml_id, route_name, raise_if_not_found=True): """ return a route record set from an xml_id or its name. """ route = self.env.ref(xml_id, raise_if_not_found=False) if not route: route = self.env['stock.route'].search([('name', 'like', route_name)], limit=1) - if not route: + if not route and raise_if_not_found: raise UserError(_('Can\'t find any generic route %s.') % (route_name)) return route @@ -391,6 +391,12 @@ class Warehouse(models.Model): -update_values: values used to update the route when a field in depends is modify on the warehouse. """ + vals = self._generate_global_route_rules_values() + # `route_id` might be `False` if the user has deleted it, in such case we + # should simply ignore the rule + return {k: v for k, v in vals.items() if v.get('create_values', {}).get('route_id', True) and v.get('update_values', {}).get('route_id', True)} + + def _generate_global_route_rules_values(self): # We use 0 since routing are order from stock to cust. If the routing # order is modify, the mto rule will be wrong. rule = self.get_rules_dict()[self.id][self.delivery_steps] @@ -408,7 +414,7 @@ class Warehouse(models.Model): 'action': 'pull', 'auto': 'manual', 'propagate_carrier': True, - 'route_id': self._find_global_route('stock.route_warehouse0_mto', _('Replenish on Order (MTO)')).id + 'route_id': self._find_global_route('stock.route_warehouse0_mto', _('Replenish on Order (MTO)'), raise_if_not_found=False).id }, 'update_values': { 'name': self._format_rulename(location_id, location_dest_id, 'MTO'),