[FIX] fields: `compute_sudo` defaults to True for stored fields only

The default `compute_sudo=True` makes sense for recomputing stored
fields that are indirectly related to a business operation.  This
ensures that the recomputation of the field does not break an operation
that is not aware of the fields to recompute.

However, computing non-stored fields in superuser mode is usually not
necessary.  It even leads to unexpected values: counting a partner's
sales orders does not give the same result in superuser mode as in
normal mode.  That is why non-stored fields are not computed in
superuser mode by default.

[FIX] account, delivery, event, hr_recruitment, point_of_sale, stock:
adapt the model definition to make all fields with the same compute
method have the same value for `compute_sudo`.

[FIX] sale: split the computation of `invoice_ids`, `invoice_count`
(non-stored) and `invoice_status` (stored), as no code is actually
shared.

closes odoo/odoo#39195

X-original-commit: 843fd38a97f02b49dc09d7f55919072d272fd80e
Signed-off-by: Olivier Dony (odo) <odo@openerp.com>

9 files changed, 49 insertions, 45 deletions

diff --git a/addons/account/models/account_move.py b/addons/account/models/account_move.py
index 0a207efbab6..61e56a5f10d 100644
--- a/addons/account/models/account_move.py
+++ b/addons/account/models/account_move.py
@@ -228,7 +228,7 @@ class AccountMove(models.Model):
         help="Auto-complete from a past bill.")
     invoice_source_email = fields.Char(string='Source Email', tracking=True)
     invoice_partner_display_name = fields.Char(compute='_compute_invoice_partner_display_info', store=True)
-    invoice_partner_icon = fields.Char(compute='_compute_invoice_partner_display_info', store=False)
+    invoice_partner_icon = fields.Char(compute='_compute_invoice_partner_display_info', store=False, compute_sudo=True)
 
     # ==== Cash rounding fields ====
     invoice_cash_rounding_id = fields.Many2one('account.cash.rounding', string='Cash Rounding Method',
diff --git a/addons/delivery/models/sale_order.py b/addons/delivery/models/sale_order.py
index 464b36aa726..ff3eb2c1a35 100644
--- a/addons/delivery/models/sale_order.py
+++ b/addons/delivery/models/sale_order.py
@@ -122,14 +122,14 @@ class SaleOrder(models.Model):
             post = u'\N{NO-BREAK SPACE}{symbol}'.format(symbol=self.currency_id.symbol or '')
         return u' {pre}{0}{post}'.format(amount, pre=pre, post=post)
 
-    @api.depends('state', 'order_line.invoice_status', 'order_line.invoice_lines',
-                 'order_line.is_delivery', 'order_line.is_downpayment', 'order_line.product_id.invoice_policy')
-    def _get_invoiced(self):
-        super(SaleOrder, self)._get_invoiced()
+    @api.depends('order_line.is_delivery', 'order_line.is_downpayment',
+                 'order_line.product_id.invoice_policy')
+    def _get_invoice_status(self):
+        super()._get_invoice_status()
         for order in self:
-            order_line = order.order_line.filtered(lambda x: not x.is_delivery and not x.is_downpayment and not x.display_type)
-            if all(line.product_id.invoice_policy == 'delivery' and line.invoice_status == 'no' for line in order_line):
-                order.update({'invoice_status': 'no'})
+            order_lines = order.order_line.filtered(lambda x: not x.is_delivery and not x.is_downpayment and not x.display_type)
+            if all(line.product_id.invoice_policy == 'delivery' and line.invoice_status == 'no' for line in order_lines):
+                order.invoice_status = 'no'
 
 
 class SaleOrderLine(models.Model):
diff --git a/addons/event/models/event.py b/addons/event/models/event.py
index 433aaccf7b5..2eae07a83f3 100644
--- a/addons/event/models/event.py
+++ b/addons/event/models/event.py
@@ -141,7 +141,7 @@ class EventEvent(models.Model):
         store=True, readonly=True, compute='_compute_seats')
     seats_expected = fields.Integer(
         string='Number of Expected Attendees',
-        readonly=True, compute='_compute_seats')
+        compute_sudo=True, readonly=True, compute='_compute_seats')
 
     # Registration fields
     registration_ids = fields.One2many(
diff --git a/addons/hr_recruitment/models/hr_recruitment.py b/addons/hr_recruitment/models/hr_recruitment.py
index cc3838b51ff..e7b8595092e 100644
--- a/addons/hr_recruitment/models/hr_recruitment.py
+++ b/addons/hr_recruitment/models/hr_recruitment.py
@@ -146,8 +146,8 @@ class Applicant(models.Model):
     partner_mobile = fields.Char("Mobile", size=32)
     type_id = fields.Many2one('hr.recruitment.degree', "Degree")
     department_id = fields.Many2one('hr.department', "Department", domain="['|', ('company_id', '=', False), ('company_id', '=', company_id)]")
-    day_open = fields.Float(compute='_compute_day', string="Days to Open")
-    day_close = fields.Float(compute='_compute_day', string="Days to Close")
+    day_open = fields.Float(compute='_compute_day', string="Days to Open", compute_sudo=True)
+    day_close = fields.Float(compute='_compute_day', string="Days to Close", compute_sudo=True)
     delay_close = fields.Float(compute="_compute_day", string='Delay to Close', readonly=True, group_operator="avg", help="Number of days to close", store=True)
     color = fields.Integer("Color Index", default=0)
     emp_id = fields.Many2one('hr.employee', string="Employee", help="Employee linked to the applicant.", copy=False)
diff --git a/addons/point_of_sale/models/pos_session.py b/addons/point_of_sale/models/pos_session.py
index 2092baf0f3d..b8d6bb440ba 100644
--- a/addons/point_of_sale/models/pos_session.py
+++ b/addons/point_of_sale/models/pos_session.py
@@ -51,7 +51,7 @@ class PosSession(models.Model):
     sequence_number = fields.Integer(string='Order Sequence Number', help='A sequence number that is incremented with each order', default=1)
     login_number = fields.Integer(string='Login Sequence Number', help='A sequence number that is incremented each time a user resumes the pos session', default=0)
 
-    cash_control = fields.Boolean(compute='_compute_cash_all', string='Has Cash Control')
+    cash_control = fields.Boolean(compute='_compute_cash_all', string='Has Cash Control', compute_sudo=True)
     cash_journal_id = fields.Many2one('account.journal', compute='_compute_cash_all', string='Cash Journal', store=True)
     cash_register_id = fields.Many2one('account.bank.statement', compute='_compute_cash_all', string='Cash Register', store=True)
 
diff --git a/addons/sale/models/sale.py b/addons/sale/models/sale.py
index 002762fb84f..b805f0f01f3 100644
--- a/addons/sale/models/sale.py
+++ b/addons/sale/models/sale.py
@@ -52,8 +52,19 @@ class SaleOrder(models.Model):
                 'amount_total': amount_untaxed + amount_tax,
             })
 
-    @api.depends('state', 'order_line.invoice_status', 'order_line.invoice_lines')
+    @api.depends('order_line.invoice_lines')
     def _get_invoiced(self):
+        # The invoice_ids are obtained thanks to the invoice lines of the SO
+        # lines, and we also search for possible refunds created directly from
+        # existing invoices. This is necessary since such a refund is not
+        # directly linked to the SO.
+        for order in self:
+            invoices = order.order_line.invoice_lines.move_id.filtered(lambda r: r.type in ('out_invoice', 'out_refund'))
+            order.invoice_ids = invoices
+            order.invoice_count = len(invoices)
+
+    @api.depends('state', 'order_line.invoice_status')
+    def _get_invoice_status(self):
         """
         Compute the invoice status of a SO. Possible statuses:
         - no: if the SO is not in status 'sale' or 'done', we consider that there is nothing to
@@ -61,36 +72,22 @@ class SaleOrder(models.Model):
         - to invoice: if any SO line is 'to invoice', the whole SO is 'to invoice'
         - invoiced: if all SO lines are invoiced, the SO is invoiced.
         - upselling: if all SO lines are invoiced or upselling, the status is upselling.
-
-        The invoice_ids are obtained thanks to the invoice lines of the SO lines, and we also search
-        for possible refunds created directly from existing invoices. This is necessary since such a
-        refund is not directly linked to the SO.
         """
         # Ignore the status of the deposit product
         deposit_product_id = self.env['sale.advance.payment.inv']._default_product_id()
         line_invoice_status_all = [(d['order_id'][0], d['invoice_status']) for d in self.env['sale.order.line'].read_group([('order_id', 'in', self.ids), ('product_id', '!=', deposit_product_id.id)], ['order_id', 'invoice_status'], ['order_id', 'invoice_status'], lazy=False)]
         for order in self:
-            invoice_ids = order.order_line.mapped('invoice_lines.move_id').filtered(lambda r: r.type in ['out_invoice', 'out_refund'])
-
-
             line_invoice_status = [d[1] for d in line_invoice_status_all if d[0] == order.id]
-
             if order.state not in ('sale', 'done'):
-                invoice_status = 'no'
+                order.invoice_status = 'no'
             elif any(invoice_status == 'to invoice' for invoice_status in line_invoice_status):
-                invoice_status = 'to invoice'
+                order.invoice_status = 'to invoice'
             elif line_invoice_status and all(invoice_status == 'invoiced' for invoice_status in line_invoice_status):
-                invoice_status = 'invoiced'
-            elif line_invoice_status and all(invoice_status in ['invoiced', 'upselling'] for invoice_status in line_invoice_status):
-                invoice_status = 'upselling'
+                order.invoice_status = 'invoiced'
+            elif line_invoice_status and all(invoice_status in ('invoiced', 'upselling') for invoice_status in line_invoice_status):
+                order.invoice_status = 'upselling'
             else:
-                invoice_status = 'no'
-
-            order.update({
-                'invoice_count': len(set(invoice_ids.ids)),
-                'invoice_ids': invoice_ids.ids,
-                'invoice_status': invoice_status
-            })
+                order.invoice_status = 'no'
 
     @api.model
     def get_empty_list_help(self, help):
@@ -184,7 +181,7 @@ class SaleOrder(models.Model):
         ('invoiced', 'Fully Invoiced'),
         ('to invoice', 'To Invoice'),
         ('no', 'Nothing to Invoice')
-        ], string='Invoice Status', compute='_get_invoiced', store=True, readonly=True)
+        ], string='Invoice Status', compute='_get_invoice_status', store=True, readonly=True)
 
     note = fields.Text('Terms and conditions', default=_default_note)
 
diff --git a/addons/stock/models/stock_quant.py b/addons/stock/models/stock_quant.py
index 8c78b5434bf..6bd89d968f4 100644
--- a/addons/stock/models/stock_quant.py
+++ b/addons/stock/models/stock_quant.py
@@ -661,7 +661,7 @@ class QuantPackage(models.Model):
         index=True, readonly=True, store=True)
     owner_id = fields.Many2one(
         'res.partner', 'Owner', compute='_compute_package_info', search='_search_owner',
-        index=True, readonly=True)
+        index=True, readonly=True, compute_sudo=True)
 
     @api.depends('quant_ids.package_id', 'quant_ids.location_id', 'quant_ids.company_id', 'quant_ids.owner_id', 'quant_ids.quantity', 'quant_ids.reserved_quantity')
     def _compute_package_info(self):
diff --git a/odoo/addons/test_new_api/tests/test_new_fields.py b/odoo/addons/test_new_api/tests/test_new_fields.py
index a7784cd97d1..f7a555f6e2a 100644
--- a/odoo/addons/test_new_api/tests/test_new_fields.py
+++ b/odoo/addons/test_new_api/tests/test_new_fields.py
@@ -100,10 +100,12 @@ class TestFields(common.TransactionCase):
         # by default function fields are not stored and readonly
         field = self.env['test_new_api.message']._fields['size']
         self.assertFalse(field.store)
+        self.assertFalse(field.compute_sudo)
         self.assertTrue(field.readonly)
 
         field = self.env['test_new_api.message']._fields['name']
         self.assertTrue(field.store)
+        self.assertTrue(field.compute_sudo)
         self.assertTrue(field.readonly)
 
     def test_10_computed_custom(self):
diff --git a/odoo/fields.py b/odoo/fields.py
index 720c3bcb7c0..1eaaeb4a2a1 100644
--- a/odoo/fields.py
+++ b/odoo/fields.py
@@ -153,8 +153,9 @@ class Field(MetaField('DummyField', (object,), {})):
         :param store: whether the field is stored in database (boolean, by
             default ``False`` on computed fields)
 
-        :param compute_sudo: whether the field should be recomputed as superuser
-            to bypass access rights (boolean, by default ``True``)
+        :param compute_sudo: whether the field should be computed in superuser
+            mode to bypass access rights (boolean, defaults to ``True`` for
+            stored fields and ``False`` for non-stored fields)
 
         The methods given for ``compute``, ``inverse`` and ``search`` are model
         methods. Their signature is shown in the following example::
@@ -295,7 +296,7 @@ class Field(MetaField('DummyField', (object,), {})):
         'depends_context': None,        # collection of context key dependencies
         'recursive': False,             # whether self depends on itself
         'compute': None,                # compute(recs) computes field on recs
-        'compute_sudo': True,           # whether field should be recomputed as superuser
+        'compute_sudo': False,          # whether field should be recomputed as superuser
         'inverse': None,                # inverse(recs) inverses field on recs
         'search': None,                 # search(recs, operator, value) searches on self
         'related': None,                # sequence of field names, for related fields
@@ -415,18 +416,24 @@ class Field(MetaField('DummyField', (object,), {})):
 
         # initialize ``self`` with ``attrs``
         if attrs.get('compute'):
-            # by default, computed fields are not stored, not copied and readonly
-            attrs['store'] = attrs.get('store', False)
+            # by default, computed fields are not stored, computed in superuser
+            # mode if stored, not copied and readonly
+            attrs['store'] = store = attrs.get('store', False)
+            attrs['compute_sudo'] = attrs.get('compute_sudo', store)
             attrs['copy'] = attrs.get('copy', False)
             attrs['readonly'] = attrs.get('readonly', not attrs.get('inverse'))
         if attrs.get('related'):
-            # by default, related fields are not stored and not copied
-            attrs['store'] = attrs.get('store', False)
+            # by default, related fields are not stored, computed in superuser
+            # mode, not copied and readonly
+            attrs['store'] = store = attrs.get('store', False)
+            attrs['compute_sudo'] = attrs.get('compute_sudo', attrs.get('related_sudo', True))
             attrs['copy'] = attrs.get('copy', False)
             attrs['readonly'] = attrs.get('readonly', True)
         if attrs.get('company_dependent'):
-            # by default, company-dependent fields are not stored and not copied
+            # by default, company-dependent fields are not stored, not computed
+            # in superuser mode and not copied
             attrs['store'] = False
+            attrs['compute_sudo'] = attrs.get('compute_sudo', False)
             attrs['copy'] = attrs.get('copy', False)
             attrs['default'] = attrs.get('default', self._default_company_dependent)
             attrs['compute'] = self._compute_company_dependent
@@ -439,8 +446,6 @@ class Field(MetaField('DummyField', (object,), {})):
             attrs['depends_context'] = attrs.get('depends_context', ()) + ('lang',)
         if 'depends' in attrs:
             attrs['depends'] = tuple(attrs['depends'])
-        if 'related_sudo' in attrs:
-            attrs['compute_sudo'] = attrs['related_sudo']
 
         return attrs