Revert "[FIX] models: make `onchange` return a smaller diff"

This reverts commit 2054b5928574affbb7efcd8a9ab146ee4fa34938 which
already has been fixed multiple times [0].

However this change does not correctly handle successive onchange calls on
ony2many fields which alter existing rows.

i.e., in saas~11.4, an onchange has been added on move lines [1] in
order to create and/or update lines containing taxes amounts.
However, with diff-only onchange, a second call to onchange will discard
changes made by a first one as nothing change between the 2 calls.

Scenario: On an existing account.move, add a new credit line with a tax set.
The first onchange will update the tax line with new amount. Add a
second line (with debit amount balanced). The onchange will not change
the tax line, resulting as it value being reset to initial value. As a
result, the move is not balanced and cannot be saved.

[0] cd2ccc4110274e23607521c18309f07cca68476a, 5e58ac16c3cae2d5c45e7c9f746ed810b835a65d, 07023137cd17d8917673fd6ba7809b2870bd8cc0
[1] d49ab568a2cc5231ac12267a9e74eaf16ca45f9e

2 files changed, 67 insertions, 132 deletions

diff --git a/odoo/addons/test_new_api/tests/test_onchange.py b/odoo/addons/test_new_api/tests/test_onchange.py
index a1b2e4edfee..1bfdee908a7 100644
--- a/odoo/addons/test_new_api/tests/test_onchange.py
+++ b/odoo/addons/test_new_api/tests/test_onchange.py
@@ -139,10 +139,14 @@ class TestOnChange(common.TransactionCase):
         self.env.cache.invalidate()
         result = self.Discussion.onchange(values, 'name', field_onchange)
         self.assertIn('messages', result['value'])
-        self.assertEqual(result['value']['messages'], [
+        self.assertItemsEqual(result['value']['messages'], [
             (5,),
             (1, message.id, {
                 'name': "[%s] %s" % ("Foo", USER.name),
+                'body': message.body,
+                'author': message.author.name_get()[0],
+                'size': message.size,
+                'important': message.important,
             }),
             (0, 0, {
                 'name': "[%s] %s" % ("Foo", USER.name),
@@ -252,7 +256,8 @@ class TestOnChange(common.TransactionCase):
             'lines': [
                 (5,),
                 (1, line1.id, {'name': partner2.name,
-                               'partner': (partner2.id, partner2.name)}),
+                               'partner': (partner2.id, partner2.name),
+                               'tags': [(5,)]}),
                 (0, 0, {'name': partner2.name,
                         'partner': (partner2.id, partner2.name),
                         'tags': [(5,)]}),
@@ -260,17 +265,6 @@ class TestOnChange(common.TransactionCase):
         })
 
         # do it again, but this time with a new tag on the second line
-        expected_new_tag_result = {
-            'name': partner2.name,
-            'lines': [
-                (5,),
-                (1, line1.id, {'name': partner2.name,
-                               'partner': (partner2.id, partner2.name)}),
-                (0, 0, {'name': partner2.name,
-                        'partner': (partner2.id, partner2.name),
-                        'tags': [(5,), (0, 0, {'name': 'Tag'})]}),
-            ],
-        }
         values = {
             'name': partner1.name,
             'partner': partner2.id,             # this one just changed
@@ -282,13 +276,18 @@ class TestOnChange(common.TransactionCase):
         self.env.cache.invalidate()
 
         result = multi.onchange(values, 'partner', field_onchange)
-        self.assertEqual(result['value'], expected_new_tag_result)
-
-        # ensure ID is not returned when asked and a many2many record is set to be created
-        self.env.cache.invalidate()
-
-        result = multi.onchange(values, 'partner', dict(field_onchange, **{'lines.tags.id': None}))
-        self.assertEqual(result['value'], expected_new_tag_result)
+        self.assertEqual(result['value'], {
+            'name': partner2.name,
+            'lines': [
+                (5,),
+                (1, line1.id, {'name': partner2.name,
+                               'partner': (partner2.id, partner2.name),
+                               'tags': [(5,)]}),
+                (0, 0, {'name': partner2.name,
+                        'partner': (partner2.id, partner2.name),
+                        'tags': [(5,), (0, 0, {'name': 'Tag'})]}),
+            ],
+        })
 
     def test_onchange_specific(self):
         """ test the effect of field-specific onchange method """
@@ -320,7 +319,8 @@ class TestOnChange(common.TransactionCase):
         self.assertIn('participants', result['value'])
         self.assertItemsEqual(
             result['value']['participants'],
-            [(5,)] + [(4, user.id) for user in discussion.participants + demo],
+            [(5,)] + [(1, user.id, {'display_name': user.display_name})
+                      for user in discussion.participants + demo],
         )
 
     def test_onchange_default(self):
@@ -356,8 +356,6 @@ class TestOnChange(common.TransactionCase):
         self.assertEqual(len(discussion.messages), 3)
         messages = [(4, msg.id) for msg in discussion.messages]
         messages[0] = (1, messages[0][1], {'body': 'test onchange'})
-        lines = ["%s:%s" % (m.name, m.body) for m in discussion.messages]
-        lines[0] = "%s:%s" % (discussion.messages[0].name, 'test onchange')
         values = {
             'name': discussion.name,
             'moderator': demo.id,
@@ -368,7 +366,8 @@ class TestOnChange(common.TransactionCase):
         }
         result = discussion.onchange(values, 'messages', field_onchange)
         self.assertIn('message_concat', result['value'])
-        self.assertEqual(result['value']['message_concat'], "\n".join(lines))
+        self.assertEqual(result['value']['message_concat'],
+                         "\n".join(["%s:%s" % (m.name, m.body) for m in discussion.messages]))
 
     def test_onchange_one2many_with_domain_on_related_field(self):
         """ test the value of the one2many field when defined with a domain on a related field"""
@@ -414,15 +413,28 @@ class TestOnChange(common.TransactionCase):
             'categories': [(4, cat.id) for cat in discussion.categories],
             'messages': [(4, msg.id) for msg in discussion.messages],
             'participants': [(4, usr.id) for usr in discussion.participants],
+            'message_changes': 0,
             'important_messages': [(4, msg.id) for msg in discussion.important_messages],
             'important_emails': [(4, eml.id) for eml in discussion.important_emails],
         }
-        self.env.cache.invalidate()
         result = discussion.onchange(values, 'name', field_onchange)
 
+        # When one2many domain contains non-computed field, things are ok
+        self.assertEqual(result['value']['important_messages'],
+                         [(5,)] + [(4, msg.id) for msg in discussion.important_messages])
+
+        # But here with commit 5676d81, we get value of: [(2, email.id)]
         self.assertEqual(
             result['value']['important_emails'],
-            [(5,), (1, email.id, {'name': u'[Foo Bar] %s' % USER.name})],
+            [(5,),
+             (1, email.id, {
+                 'name': u'[Foo Bar] %s' % USER.name,
+                 'body': email.body,
+                 'author': USER.name_get()[0],
+                 'important': True,
+                 'email_to': demo.email,
+                 'size': email.size,
+             })]
         )
 
     def test_onchange_related(self):
diff --git a/odoo/models.py b/odoo/models.py
index a716a429f0d..964dbcbf5d9 100644
--- a/odoo/models.py
+++ b/odoo/models.py
@@ -5040,114 +5040,28 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})):
         if not all(name in self._fields for name in names):
             return {}
 
-        class PrefixTree(OrderedDict):
-            """ A prefix tree for sequences of field names. The tree is a
-                dictionary that associates each given field name to its
-                corresponding subtree (in fields order)::
-
-                    # tree corresponding to dotnames
-                    # ['name', 'line_ids.product_id', 'line_ids.tags_ids.name']
-                    {
-                        'name': {},
-                        'line_ids': {
-                            'product_id': {},
-                            'tags_ids': {
-                                'name': {},
-                            },
-                        },
-                    }
-            """
-            def __init__(self, model, dotnames):
-                super(PrefixTree, self).__init__()
-                if not dotnames:
-                    return
-                # group dotnames by prefix
-                suffixes = defaultdict(list)
-                for dotname in dotnames:
-                    names = dotname.split('.', 1)
-                    name_suffixes = suffixes[names[0]]
-                    if len(names) > 1:
-                        name_suffixes.append(names[1])
-                # fill in self in fields order
-                for name in model._fields:
-                    if name in suffixes:
-                        self[name] = PrefixTree(model[name], suffixes[name])
-
-            def dotnames(self):
-                """ Iterate over the sequences of field names. """
-                for name, subnames in self.items():
-                    yield name
-                    for dotname in subnames.dotnames():
-                        yield "%s.%s" % (name, dotname)
-
-        nametree = PrefixTree(self.browse(), field_onchange)
-        dotnames = list(nametree.dotnames())
-
-        def snapshot(record, tree=nametree):
-            """ Return a dict with the values of record, following nametree. """
-            vals = {}
-            for name, subnames in tree.items():
-                if subnames:
-                    # x2many fields as {line: snapshot(line), ...}
-                    vals[name] = OrderedDict(
-                        (line, snapshot(line, subnames))
-                        for line in record[name]
-                    )
-                else:
-                    vals[name] = record[name]
-            return vals
-
-        def diff(record, old, new, tree=nametree):
-            """ Return the values that differ between snapshots.
-                The snapshot ``old`` may be empty (for new records).
-            """
-            result = {}
-            for name, subnames in tree.items():
-                if name == 'id':
-                    continue
-                if old and old[name] == new[name]:
-                    continue
-                field = record._fields[name]
-                if not subnames:
-                    result[name] = field.convert_to_onchange(new[name], record, {})
-                    continue
-                # x2many fields: serialize value as commands
-                result[name] = commands = [(5,)]
-                old_val = old.get(name) or {}
-                for line, vals in new[name].items():
-                    vals0 = (old_val.get(line) or snapshot(line, subnames)) if line.id else {}
-                    line_diff = diff(line, vals0, vals, subnames)
-                    if not line.id:
-                        commands.append((0, line.id.ref or 0, line_diff))
-                    elif line_diff:
-                        commands.append((1, line.id, line_diff))
-                    else:
-                        commands.append((4, line.id))
-            return result
-
-        # prefetch x2many lines without data (for the initial snapshot)
-        for name, subnames in nametree.items():
-            if subnames and values.get(name):
-                # retrieve all ids in commands, and read the expected fields
-                line_ids = []
-                for cmd in values[name]:
-                    if cmd[0] in (1, 4):
-                        line_ids.append(cmd[1])
-                    elif cmd[0] == 6:
-                        line_ids.extend(cmd[2])
-                lines = self.browse()[name].browse(line_ids)
-                lines.read(list(subnames), load='_classic_write')
+        # filter out keys in field_onchange that do not refer to actual fields
+        dotnames = []
+        for dotname in field_onchange:
+            try:
+                model = self.browse()
+                for name in dotname.split('.'):
+                    model = model[name]
+                dotnames.append(dotname)
+            except Exception:
+                pass
 
         # create a new record with values, and attach ``self`` to it
         with env.do_in_onchange():
             record = self.new(values)
-            values = {name: record[name] for name in nametree}
+            values = {name: record[name] for name in record._cache}
             # attach ``self`` with a different context (for cache consistency)
             record._origin = self.with_context(__onchange=True)
 
-        # make a snapshot based on the initial values of record
+        # load fields on secondary records, to avoid false changes
         with env.do_in_onchange():
-            before = snapshot(record)
+            for dotname in dotnames:
+                record.mapped(dotname)
 
         # determine which field(s) should be triggered an onchange
         todo = list(names) or list(values)
@@ -5166,6 +5080,7 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})):
                 record[name] = value
 
         result = {}
+        dirty = set()
 
         # process names in order (or the keys of values if no name given)
         while todo:
@@ -5191,14 +5106,22 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})):
                         field.type in ('one2many', 'many2many') and newval._is_dirty()
                     ):
                         todo.append(name)
+                        dirty.add(name)
 
-        # make a snapshot based on the final values of record
-        with env.do_in_onchange():
-            after = snapshot(record)
+        # determine subfields for field.convert_to_onchange() below
+        Tree = lambda: defaultdict(Tree)
+        subnames = Tree()
+        for dotname in dotnames:
+            subtree = subnames
+            for name in dotname.split('.'):
+                subtree = subtree[name]
 
-        # determine values that have changed by comparing snapshots
-        self.invalidate_cache()
-        result['value'] = diff(record, before, after)
+        # collect values from dirty fields
+        with env.do_in_onchange():
+            result['value'] = {
+                name: self._fields[name].convert_to_onchange(record[name], record, subnames[name])
+                for name in dirty
+            }
 
         return result