From 2e747cedf4faa194699456c13c48313c14daf8f8 Mon Sep 17 00:00:00 2001 From: Arnold Moyaux Date: Thu, 28 Nov 2019 15:25:52 +0000 Subject: [PATCH] [IMP] stock, purchase_stock, mrp: Procurement exception Currently when a procurement fail it will raise a user error. Some function as run in stock_rule catch the errors and merge them in a single error. Also since commit a2a6b099a4c1c99f4aa9e741442944e588a8f048 procurements are executed in batch and in some cases, we would like to have more information about the error. For example in orderpoint we would like to know which orderpoint is responssible for which error. It would also be possible to return an object with the details but procurements pass through a lot of different method that already have their own return signature and a simple exception is easier to manage. Task: 1849912 --- addons/mrp/models/stock_rule.py | 8 ++-- addons/purchase_stock/models/stock_rule.py | 4 +- addons/stock/models/stock_move.py | 2 +- addons/stock/models/stock_rule.py | 48 +++++++++++++++------- 4 files changed, 41 insertions(+), 21 deletions(-) diff --git a/addons/mrp/models/stock_rule.py b/addons/mrp/models/stock_rule.py index 36646667497..7ff8ec09d7a 100644 --- a/addons/mrp/models/stock_rule.py +++ b/addons/mrp/models/stock_rule.py @@ -5,8 +5,8 @@ from collections import defaultdict from dateutil.relativedelta import relativedelta from odoo import api, fields, models, _ -from odoo.exceptions import UserError from odoo.osv import expression +from odoo.addons.stock.models.stock_rule import ProcurementException class StockRule(models.Model): @@ -41,7 +41,7 @@ class StockRule(models.Model): bom = self._get_matching_bom(procurement.product_id, procurement.company_id, procurement.values) if not bom: msg = _('There is no Bill of Material of type manufacture or kit found for the product %s. Please define a Bill of Material for this product.') % (procurement.product_id.display_name,) - raise UserError(msg) + raise ProcurementException([(procurement, msg)]) productions_values_by_company[procurement.company_id.id].append(rule._prepare_mo_vals(*procurement, bom)) @@ -115,7 +115,7 @@ class ProcurementGroup(models.Model): _inherit = 'procurement.group' @api.model - def run(self, procurements): + def run(self, procurements, raise_user_error=True): """ If 'run' is called on a kit, this override is made in order to call the original 'run' method with the values of the components of that kit. """ @@ -138,7 +138,7 @@ class ProcurementGroup(models.Model): procurement.origin, procurement.company_id, values)) else: procurements_without_kit.append(procurement) - return super(ProcurementGroup, self).run(procurements_without_kit) + return super(ProcurementGroup, self).run(procurements_without_kit, raise_user_error=raise_user_error) def _get_moves_to_assign_domain(self): domain = super(ProcurementGroup, self)._get_moves_to_assign_domain() diff --git a/addons/purchase_stock/models/stock_rule.py b/addons/purchase_stock/models/stock_rule.py index 0a5a7b85c23..05ef9e451d9 100644 --- a/addons/purchase_stock/models/stock_rule.py +++ b/addons/purchase_stock/models/stock_rule.py @@ -6,7 +6,7 @@ from dateutil.relativedelta import relativedelta from itertools import groupby from odoo import api, fields, models, _ -from odoo.exceptions import UserError +from odoo.addons.stock.models.stock_rule import ProcurementException class StockRule(models.Model): @@ -53,7 +53,7 @@ class StockRule(models.Model): if not supplier: msg = _('There is no matching vendor price to generate the purchase order for product %s (no vendor defined, minimum quantity not reached, dates not valid, ...). Go on the product form and complete the list of vendors.') % (procurement.product_id.display_name) - raise UserError(msg) + raise ProcurementException([(procurement, msg)]) partner = supplier.name # we put `supplier_info` in values for extensibility purposes diff --git a/addons/stock/models/stock_move.py b/addons/stock/models/stock_move.py index 1c84c3ca3b3..8b903222069 100644 --- a/addons/stock/models/stock_move.py +++ b/addons/stock/models/stock_move.py @@ -1023,7 +1023,7 @@ class StockMove(models.Model): move.product_id, move.product_uom_qty, move.product_uom, move.location_id, move.rule_id and move.rule_id.name or "/", origin, move.company_id, values)) - self.env['procurement.group'].run(procurement_requests) + self.env['procurement.group'].run(procurement_requests, raise_user_error=not self.env.context.get('from_orderpoint')) move_to_confirm.write({'state': 'confirmed'}) (move_waiting | move_create_proc).write({'state': 'waiting'}) diff --git a/addons/stock/models/stock_rule.py b/addons/stock/models/stock_rule.py index 129266e640a..7782ebf8709 100644 --- a/addons/stock/models/stock_rule.py +++ b/addons/stock/models/stock_rule.py @@ -9,13 +9,24 @@ from psycopg2 import OperationalError from odoo import api, fields, models, registry, SUPERUSER_ID, _ from odoo.osv import expression from odoo.tools import DEFAULT_SERVER_DATETIME_FORMAT, float_compare, float_round - from odoo.exceptions import UserError import logging _logger = logging.getLogger(__name__) +class ProcurementException(Exception): + """An exception raised by ProcurementGroup `run` containing all the faulty + procurements. + """ + def __init__(self, procurement_exceptions): + """:param procurement_exceptions: a list of tuples containing the faulty + procurement and their error messages + :type procurement_exceptions: list + """ + self.procurement_exceptions = procurement_exceptions + + class StockRule(models.Model): """ A rule describe what a procurement should do; produce, buy, move, ... """ _name = 'stock.rule' @@ -210,7 +221,7 @@ class StockRule(models.Model): for procurement, rule in procurements: if not rule.location_src_id: msg = _('No source location defined on stock rule: %s!') % (rule.name, ) - raise UserError(msg) + raise ProcurementException([(procurement, msg)]) if rule.procure_method == 'mts_else_mto': mtso_products_by_locations[rule.location_src_id].append(procurement.product_id.id) @@ -352,40 +363,49 @@ class ProcurementGroup(models.Model): required=True) @api.model - def run(self, procurements): + def run(self, procurements, raise_user_error=True): """ Method used in a procurement case. The purpose is to supply the product passed as argument in the location also given as an argument. In order to be able to find a suitable location that provide the product it will search among stock.rule. """ + + def raise_exception(procurement_errors): + if raise_user_error: + dummy, errors = zip(*procurement_errors) + raise UserError('\n'.join(errors)) + else: + raise ProcurementException(procurement_errors) + actions_to_run = defaultdict(list) - errors = [] + procurement_errors = [] for procurement in procurements: procurement.values.setdefault('company_id', self.env.company) procurement.values.setdefault('priority', '1') procurement.values.setdefault('date_planned', fields.Datetime.now()) rule = self._get_rule(procurement.product_id, procurement.location_id, procurement.values) if not rule: - errors.append(_('No rule has been found to replenish "%s" in "%s".\nVerify the routes configuration on the product.') % - (procurement.product_id.display_name, procurement.location_id.display_name)) + error = _('No rule has been found to replenish "%s" in "%s".\nVerify the routes configuration on the product.') %\ + (procurement.product_id.display_name, procurement.location_id.display_name) + procurement_errors.append((procurement, error)) else: action = 'pull' if rule.action == 'pull_push' else rule.action actions_to_run[action].append((procurement, rule)) - if errors: - raise UserError('\n'.join(errors)) + if procurement_errors: + raise_exception(procurement_errors) for action, procurements in actions_to_run.items(): if hasattr(self.env['stock.rule'], '_run_%s' % action): try: getattr(self.env['stock.rule'], '_run_%s' % action)(procurements) - except UserError as e: - errors.append(e.name) + except ProcurementException as e: + procurement_errors += e.procurement_exceptions else: _logger.error("The method _run_%s doesn't exist on the procurement rules" % action) - if errors: - raise UserError('\n'.join(errors)) + if procurement_errors: + raise_exception(procurement_errors) return True @api.model @@ -579,10 +599,10 @@ class ProcurementGroup(models.Model): try: with self._cr.savepoint(): #TODO: make it batch - self.env['procurement.group'].run([self.env['procurement.group'].Procurement( + self.env['procurement.group'].with_context(from_orderpoint=True).run([self.env['procurement.group'].Procurement( orderpoint.product_id, qty_rounded, orderpoint.product_uom, orderpoint.location_id, orderpoint.name, orderpoint.name, - orderpoint.company_id, values)]) + orderpoint.company_id, values)], raise_user_error=False) except UserError as error: self.env['stock.rule']._log_next_activity(orderpoint.product_id, error.name) self._procurement_from_orderpoint_post_process([orderpoint.id])