[FIX] core: various issues & deviations from client's behaviour in Form

* Form would entirely mis-interpret the server's response to some
  results: for unmodified o2m records the server sends a (4), which was
  un-interpreted by Form, leading to the o2m record being lost entirely
  (as it rebuilds the entire o2m on setting)
* on record creation, every field (except id) is considered modified not
  just the fields set by default_get
* Form diverged from actual client in that it would send (1, id, {}) for
  unmodified o2m records, actual client sends (4, id, False)
* datetimes should be sent stringified
* onchange should not be triggered when none of the modified fields is
  flagged as an onchange trigger
* the default value for numerical fields is 0 not False
* convert m2m onchange results to (6, False, ids) instead of (6, 0, ids)
  for ease of diffing with actual client's data

There are still a few odd divergences in the actual invocation of
onchange when attempted on an SO, but they look to be of low relevance
/ risk.

Note: making integer fields "required" has pretty much no effect
      anymore. This is in line with (my understanding of) the client's
      behaviour, but required the alteration of test models as the
      specific test on required... was using an integer field.

closes odoo/odoo#31064

4 files changed, 74 insertions, 31 deletions

diff --git a/odoo/addons/test_testing_utilities/menu.xml b/odoo/addons/test_testing_utilities/menu.xml
index 99e409708c2..3ca18ef2a50 100644
--- a/odoo/addons/test_testing_utilities/menu.xml
+++ b/odoo/addons/test_testing_utilities/menu.xml
@@ -10,6 +10,17 @@
               name="O2M onchange test thing"
               action="action_test_o2m_onchange"/>
 
+    <record id="action_test_m2o_onchange" model="ir.actions.act_window">
+        <field name="name">test_default_and_onchange</field>
+        <field name="type">ir.actions.act_window</field>
+        <field name="res_model">test_testing_utilities.d</field>
+        <field name="view_mode">tree,form</field>
+    </record>
+
+    <menuitem id="menu_m2o" parent="base.menu_tests"
+              name="test_default_and_onchange"
+              action="action_test_m2o_onchange"/>
+
     <record id="o2m_parent" model="ir.ui.view">
         <field name="name">Parent External Form</field>
         <field name="priority">1</field>
@@ -98,7 +109,7 @@
         <field name="arch" type="xml">
             <form>
                 <field name="f1"/>
-                <field name="f2" attrs="{'readonly': [['f1', '=', 63], ['f3', '=', 5]]}"/>
+                <field name="f2" attrs="{'readonly': [['f1', '=', '63'], ['f3', '=', 5]]}"/>
                 <field name="f3"/>
                 <field name="f4"/>
                 <field name="f5" readonly="1"/>
diff --git a/odoo/addons/test_testing_utilities/models.py b/odoo/addons/test_testing_utilities/models.py
index 98a9fea7e7f..ffb27cd08d8 100644
--- a/odoo/addons/test_testing_utilities/models.py
+++ b/odoo/addons/test_testing_utilities/models.py
@@ -6,7 +6,7 @@ class A(models.Model):
     _name = 'test_testing_utilities.a'
     _description = 'Testing Utilities A'
 
-    f1 = fields.Integer(required=True)
+    f1 = fields.Char(required=True)
     f2 = fields.Integer(default=42)
     f3 = fields.Integer()
     f4 = fields.Integer(compute='_compute_f4')
@@ -22,7 +22,7 @@ class A(models.Model):
     @api.depends('f1', 'f2')
     def _compute_f4(self):
         for r in self:
-            r.f4 = r.f2 / (r.f1 or 1)
+            r.f4 = r.f2 / (int(r.f1) or 1)
 
 class B(models.Model):
     _name = 'test_testing_utilities.readonly'
@@ -154,7 +154,6 @@ class O2MSub(models.Model):
 
     @api.onchange('has_parent')
     def _onchange_has_parent(self):
-        self.has_parent = bool(self.parent_id)
         if self.has_parent:
             self.value = self.parent_id.value
 
diff --git a/odoo/addons/test_testing_utilities/tests/test_form_impl.py b/odoo/addons/test_testing_utilities/tests/test_form_impl.py
index e2f0d0d89d3..344ae62f4d2 100644
--- a/odoo/addons/test_testing_utilities/tests/test_form_impl.py
+++ b/odoo/addons/test_testing_utilities/tests/test_form_impl.py
@@ -24,7 +24,7 @@ class TestBasic(TransactionCase):
         self.assertEqual(f.f3, 21)
         self.assertEqual(f.f4, 42)
 
-        f.f1 = 4
+        f.f1 = '4'
         self.assertEqual(f.f2, 42)
         self.assertEqual(f.f3, 21)
         self.assertEqual(f.f4, 10)
@@ -36,7 +36,7 @@ class TestBasic(TransactionCase):
         r = f.save()
         self.assertEqual(
             (r.f1, r.f2, r.f3, r.f4),
-            (4, 8, 4, 2),
+            ('4', 8, 4, 2),
         )
 
     def test_required(self):
@@ -45,12 +45,12 @@ class TestBasic(TransactionCase):
         with self.assertRaisesRegexp(AssertionError, 'f1 is a required field'):
             f.save()
         # set f1 and unset f2 => should work
-        f.f1 = 1
+        f.f1 = '1'
         f.f2 = False
         r = f.save()
         self.assertEqual(
             (r.f1, r.f2, r.f3, r.f4),
-            (1, 0, 0, 0)
+            ('1', 0, 0, 0)
         )
 
     def test_readonly(self):
@@ -61,7 +61,7 @@ class TestBasic(TransactionCase):
         f = Form(self.env['test_testing_utilities.readonly'])
 
         with self.assertRaises(AssertionError):
-            f.f1 = 5
+            f.f1 = '5'
         with self.assertRaises(AssertionError):
             f.f2 = 42
 
@@ -70,7 +70,7 @@ class TestBasic(TransactionCase):
         """
         f = Form(self.env['test_testing_utilities.a'], view='test_testing_utilities.non_normalized_attrs')
 
-        f.f1 = 1
+        f.f1 = '1'
         f.f2 = 987
         self.assertEqual(f.f5, 987)
         self.assertEqual(f.f6, 987)
@@ -86,7 +86,7 @@ class TestBasic(TransactionCase):
         # not readonly yet, should work
         f.f2 = 5
         # make f2 readonly
-        f.f1 = 63
+        f.f1 = '63'
         f.f3 = 5
         with self.assertRaises(AssertionError):
             f.f2 = 6
@@ -101,9 +101,9 @@ class TestM2O(TransactionCase):
 
         f = Form(self.env['test_testing_utilities.d'])
 
-        self.assertEqual(
-            f.f, a,
-            "The default value for the m2o should be the first Sub record"
+        self.assertFalse(
+            f.f,
+            "The default value gets overridden by the onchange"
         )
         f.f2 = "B"
         self.assertEqual(
@@ -271,6 +271,7 @@ class TestO2M(TransactionCase):
             [get(s) for s in r.subs],
             [("2", 2, 2), ("2", 2, 2)]
         )
+        self.assertEqual(r.v, 5)
 
         with Form(r, view='test_testing_utilities.o2m_parent') as f:
             with f.subs.new() as sub:
@@ -286,6 +287,7 @@ class TestO2M(TransactionCase):
             [get(s) for s in r.subs],
             [("2", 2, 2), ("5", 5, 5), ("2", 2, 2)]
         )
+        self.assertEqual(r.v, 10)
 
         with Form(r, view='test_testing_utilities.o2m_parent') as f, \
             f.subs.edit(index=0) as sub,\
@@ -439,12 +441,12 @@ class TestEdition(TransactionCase):
     """
     def test_trivial(self):
         r = self.env['test_testing_utilities.a'].create({
-            'f1': 5,
+            'f1': '5',
         })
 
         with Form(r) as f:
             self.assertEqual(f.id, r.id)
-            self.assertEqual(f.f1, 5)
+            self.assertEqual(f.f1, '5')
             self.assertEqual(f.f4, 8)
 
             f.f2 = 5
diff --git a/odoo/tests/common.py b/odoo/tests/common.py
index b735a63c816..d6ba11d17ac 100644
--- a/odoo/tests/common.py
+++ b/odoo/tests/common.py
@@ -1200,6 +1200,11 @@ class Form(object):
             if fields[k]['type'] == 'one2many':
                 # o2m default gets a (6) at the start, makes no sense
                 return [c for c in v if c[0] != 6]
+            elif fields[k]['type'] == 'datetime' and isinstance(v, datetime):
+                return odoo.fields.Datetime.to_string(v)
+            elif fields[k]['type'] == 'date' and isinstance(v, date):
+                return odoo.fields.Datetime.to_string(v)
+
             return v
         defaults = {
             k: cleanup(k, v)
@@ -1215,9 +1220,12 @@ class Form(object):
                     vals[k] = [(6, False, [])]
                 elif type_ == 'one2many':
                     vals[k] = []
+                elif type_ in ('integer', 'float'):
+                    vals[k] = 0
 
-        # TODO: check that only fields with default values should be sent
-        self._perform_onchange(list(defaults.keys()))
+        # on creation, every field is considered changed by the client
+        # apparently
+        self._perform_onchange(list(vals.keys() - {'id'}))
 
     def _init_from_values(self, values):
         self._values.update(
@@ -1371,20 +1379,22 @@ class Form(object):
                 node = self._get_node(f)
                 if not node.get('force_save'):
                     continue
-            # TODO: filter out (1, _, {}) from o2m values
+
             values[f] = v
         return values
 
     def _perform_onchange(self, fields):
         assert isinstance(fields, list)
-
-        # marks any onchange source as changed (default_get or explicit set)
+        # marks any onchange source as changed
         self._changed.update(fields)
-        result = self._model.onchange(
-            self._onchange_values(),
-            fields,
-            self._view['onchange'],
-        )
+
+        # skip calling onchange() if there's no trigger on any of the changed
+        # fields
+        spec = self._view['onchange']
+        if not any(spec[f] for f in fields):
+            return
+
+        result = self._model.onchange(self._onchange_values(), fields, spec)
         if result.get('warning'):
             _logger.getChild('onchange').warn("%(title)s %(message)s" % result.get('warning'))
         values = result.get('value', {})
@@ -1400,7 +1410,18 @@ class Form(object):
         )
 
     def _onchange_values(self):
-        return dict(self._values)
+        f = self._view['fields']
+        values = {}
+        for k, v in self._values.items():
+            if f[k]['type'] == 'one2many':
+                # web client sends a 4 for unmodified o2m rows
+                values[k] = [
+                    (4, rid, False) if (c == 1 and not vs) else (c, rid, vs)
+                    for (c, rid, vs) in v
+                ]
+            else:
+                values[k] = v
+        return values
 
     def _cleanup_onchange(self, descr, value, current):
         if descr['type'] == 'many2one':
@@ -1414,6 +1435,7 @@ class Form(object):
                 return []
 
             v = []
+            c = {t[1]: t[2] for t in current if t[0] == 1}
             # which view should this be???
             subfields = descr['views']['edition']['fields']
             for command in value:
@@ -1426,7 +1448,10 @@ class Form(object):
                         for k, v in command[2].items()
                         if k in subfields
                     }))
-                    # TODO: should reuse existing values if not 5?
+                elif command[0] == 4:
+                    v.append((1, command[1], c.get(command[1], {})))
+                elif command[0] == 5:
+                    v = []
             return v
         elif descr['type'] == 'many2many':
             # onchange result is a bunch of commands, normalize to single 6
@@ -1446,7 +1471,7 @@ class Form(object):
                 else:
                     raise ValueError(
                         "Unsupported M2M command %d" % command[0])
-            return [(6, 0, ids)]
+            return [(6, False, ids)]
 
         return value
 
@@ -1510,7 +1535,6 @@ class O2MForm(Form):
                 continue
             # if self._get_modifier(f, 'readonly'):
             #     continue
-            # TODO: filter out (1, _, {}) from o2m values
             values[f] = v
         return values
 
@@ -1543,7 +1567,10 @@ class O2MProxy(X2MProxy):
             elif command == 2:
                 pass
             else:
-                raise AssertionError("O2M proxy only supports commands 0, 1 and 2")
+                raise AssertionError("O2M proxy only supports commands 0, 1 and 2, found %s" % command)
+
+    def __len__(self):
+        return len(self._records)
 
     @property
     def _model(self):
@@ -1705,6 +1732,10 @@ def record_to_values(fields, record):
             v = [(6, 0, v.ids)]
         elif descr['type'] == 'one2many':
             v = [(1, r.id, {}) for r in v]
+        elif descr['type'] == 'datetime' and isinstance(v, datetime):
+            v = odoo.fields.Datetime.to_string(v)
+        elif descr['type'] == 'date' and isinstance(v, date):
+            v = odoo.fields.Date.to_string(v)
         r[f] = v
     return r