[FIX] core: computed fields can be copied if editable

Purpose of this commit is to let computed stored editable fields being
copied if their field class allows it.

Indeed the value of those fields is computed based on some triggers but
can also be updated manually by users.  When copying a record, it makes
sense to consider that this value is what the user expects and allow its
copy, if the original field allows it.  Either it was computed, and
copied value will be correct without having to call computation again
(well, provided all dependencies have been copied, too), or it was
updated and the copied value will be the one the user entered.

Without this fix, an edited field is not copied, and will be recomputed,
which may look like an inconsistent value.

Task ID 2209163

closes odoo/odoo#48383

X-original-commit: 4b274d3b4101fbae154a572cdf40d23838899773
Signed-off-by: Olivier Dony (odo) <odo@openerp.com>
Signed-off-by: Raphael Collet (rco) <rco@openerp.com>
Co-authored-by: Raphael Collet <rco@odoo.com>

addons/sale_timesheet/tests/test_sale_service.py

@@ -590,12 +590,12 @@ class TestSaleService(TestCommonSaleTimesheetNoChart):
 
         # copy the project
         project_copy = project.copy()
-        self.assertFalse(project_copy.sale_line_id, "Duplicatinga project should erase its Sale line")
-        self.assertFalse(project_copy.sale_order_id, "Duplicatinga project should erase its Sale order")
-        self.assertEqual(project_copy.billable_type, 'no', "Duplicatinga project should reset its billable type to none billable")
+        self.assertFalse(project_copy.sale_line_id, "Duplicating project should erase its Sale line")
+        self.assertFalse(project_copy.sale_order_id, "Duplicating project should erase its Sale order")
+        self.assertEqual(project_copy.billable_type, 'no', "Duplicating project should reset its billable type to none billable")
         self.assertEqual(len(project.tasks), len(project_copy.tasks), "Copied project must have the same number of tasks")
         self.assertFalse(project_copy.tasks.mapped('sale_line_id'), "The tasks of the duplicated project should not have a Sale Line set.")
 
         # copy the task
         task_copy = task.copy()
-        self.assertFalse(task_copy.sale_line_id, "Duplicatinga task should not keep its Sale line")
+        self.assertEqual(task_copy.sale_line_id, task.sale_line_id, "Duplicating task should keep its Sale line")

odoo/addons/test_new_api/models/test_new_api.py

@@ -505,6 +505,14 @@ class ComputeOnchange(models.Model):
     foo = fields.Char()
     bar = fields.Char(compute='_compute_bar', store=True)
     baz = fields.Char(compute='_compute_baz', store=True, readonly=False)
+    line_ids = fields.One2many(
+        'test_new_api.compute.onchange.line', 'record_id',
+        compute='_compute_line_ids', store=True, readonly=False
+    )
+    tag_ids = fields.Many2many(
+        'test_new_api.multi.tag',
+        compute='_compute_tag_ids', store=True, readonly=False,
+    )
 
     @api.depends('foo')
     def _compute_bar(self):
@@ -517,6 +525,36 @@ class ComputeOnchange(models.Model):
             if record.active:
                 record.baz = record.foo
 
+    @api.depends('foo')
+    def _compute_line_ids(self):
+        for record in self:
+            if not record.foo:
+                continue
+            if any(line.foo == record.foo for line in record.line_ids):
+                continue
+            # add a line with the same value as 'foo'
+            record.line_ids = [(0, 0, {'foo': record.foo})]
+
+    @api.depends('foo')
+    def _compute_tag_ids(self):
+        Tag = self.env['test_new_api.multi.tag']
+        for record in self:
+            if record.foo:
+                record.tag_ids = Tag.search([('name', '=', record.foo)])
+
+    def copy(self, default=None):
+        default = dict(default or {}, foo="%s (copy)" % (self.foo or ""))
+        return super().copy(default)
+
+
+class ComputeOnchangeLine(models.Model):
+    _name = 'test_new_api.compute.onchange.line'
+    _description = "Line-like model for test_new_api.compute.onchange"
+
+    foo = fields.Char()
+    record_id = fields.Many2one('test_new_api.compute.onchange',
+                                required=True, ondelete='cascade')
+
 
 class ModelBinary(models.Model):
     _name = 'test_new_api.model_binary'

odoo/addons/test_new_api/security/ir.model.access.csv

@@ -24,6 +24,7 @@ access_test_new_api_multi_compute_inverse,access_test_new_api_multi_compute_inve
 access_test_new_api_recursive,access_test_new_api_recursive,model_test_new_api_recursive,,1,1,1,1
 access_test_new_api_cascade,access_test_new_api_cascade,model_test_new_api_cascade,,1,1,1,1
 access_test_new_api_compute_onchange,access_test_new_api_compute_onchange,model_test_new_api_compute_onchange,,1,1,1,1
+access_test_new_api_compute_onchange_line,access_test_new_api_compute_onchange_line,model_test_new_api_compute_onchange_line,,1,1,1,1
 access_test_new_api_binary_svg,access_test_new_api_binary_svg,model_test_new_api_binary_svg,,1,1,1,1
 access_test_new_api_monetary_base,access_test_new_api_monetary_base,model_test_new_api_monetary_base,,1,1,1,1
 access_test_new_api_monetary_related,access_test_new_api_monetary_related,model_test_new_api_monetary_related,,1,1,1,1

odoo/addons/test_new_api/tests/test_new_fields.py

@@ -110,16 +110,37 @@ class TestFields(TransactionCaseWithUserDemo):
 
     def test_10_computed(self):
         """ check definition of computed fields """
-        # by default function fields are not stored and readonly
+        # by default function fields are not stored, readonly, not copied
         field = self.env['test_new_api.message']._fields['size']
         self.assertFalse(field.store)
         self.assertFalse(field.compute_sudo)
         self.assertTrue(field.readonly)
+        self.assertFalse(field.copy)
 
         field = self.env['test_new_api.message']._fields['name']
         self.assertTrue(field.store)
         self.assertTrue(field.compute_sudo)
         self.assertTrue(field.readonly)
+        self.assertFalse(field.copy)
+
+        # stored editable computed fields are copied according to their type
+        field = self.env['test_new_api.compute.onchange']._fields['baz']
+        self.assertTrue(field.store)
+        self.assertTrue(field.compute_sudo)
+        self.assertFalse(field.readonly)
+        self.assertTrue(field.copy)
+
+        field = self.env['test_new_api.compute.onchange']._fields['line_ids']
+        self.assertTrue(field.store)
+        self.assertTrue(field.compute_sudo)
+        self.assertFalse(field.readonly)
+        self.assertFalse(field.copy)  # like a regular one2many field
+
+        field = self.env['test_new_api.compute.onchange']._fields['tag_ids']
+        self.assertTrue(field.store)
+        self.assertTrue(field.compute_sudo)
+        self.assertFalse(field.readonly)
+        self.assertTrue(field.copy)  # like a regular many2many field
 
     def test_10_computed_custom(self):
         """ check definition of custom computed fields """

odoo/addons/test_new_api/tests/test_onchange.py

@@ -552,6 +552,41 @@ class TestComputeOnchange(common.TransactionCase):
         self.assertEqual(record.bar, "foo")
         self.assertEqual(record.baz, "baz")
 
+    def test_copy(self):
+        Model = self.env['test_new_api.compute.onchange']
+
+        # create tags
+        tag_foo, tag_bar = self.env['test_new_api.multi.tag'].create([
+            {'name': 'foo1'},
+            {'name': 'bar1'},
+        ])
+
+        # compute 'bar', 'baz', 'line_ids' and 'tag_ids'
+        record = Model.create({'active': True, 'foo': "foo1"})
+        self.assertEqual(record.bar, "foo1")
+        self.assertEqual(record.baz, "foo1")
+        self.assertEqual(record.line_ids.mapped('foo'), ['foo1'])
+        self.assertEqual(record.tag_ids, tag_foo)
+
+        # manually update 'baz' and 'lines' to test copy attribute
+        record.write({
+            'baz': "baz1",
+            'line_ids': [(0, 0, {'foo': 'bar'})],
+            'tag_ids': [(4, tag_bar.id)],
+        })
+        self.assertEqual(record.bar, "foo1")
+        self.assertEqual(record.baz, "baz1")
+        self.assertEqual(record.line_ids.mapped('foo'), ['foo1', 'bar'])
+        self.assertEqual(record.tag_ids, tag_foo + tag_bar)
+
+        # copy the record, and check results
+        copied = record.copy()
+        self.assertEqual(copied.foo, "foo1 (copy)")   # copied and modified
+        self.assertEqual(copied.bar, "foo1 (copy)")   # computed
+        self.assertEqual(copied.baz, "baz1")          # copied
+        self.assertEqual(record.line_ids.mapped('foo'), ['foo1', 'bar'])  # copied
+        self.assertEqual(record.tag_ids, tag_foo + tag_bar)  # copied
+
     def test_write(self):
         model = self.env['test_new_api.compute.onchange']
         record = model.create({'active': True, 'foo': "foo"})

odoo/fields.py

@@ -365,10 +365,12 @@ class Field(MetaField('DummyField', (object,), {})):
         # initialize ``self`` with ``attrs``
         if attrs.get('compute'):
             # by default, computed fields are not stored, computed in superuser
-            # mode if stored, not copied and readonly
+            # mode if stored, not copied (unless stored and explicitly not
+            # readonly), and readonly (unless inversible)
             attrs['store'] = store = attrs.get('store', False)
             attrs['compute_sudo'] = attrs.get('compute_sudo', store)
-            attrs['copy'] = attrs.get('copy', False)
+            if not (attrs['store'] and not attrs.get('readonly', True)):
+                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, computed in superuser