[FIX] core: don't send readonly fields in o2m in SSF

The SSF would correctly filter out readonly fields when saving a
toplevel form, however it could not remove readonly values when saving
o2m pseudo-records to the parent form (as these would be expected to
remain available for reading upon the next edition and whatnot), so
these values would get sent in 0/1 commands.

Filter out these fields during the parent / toplevel save call.

Complexity notes:

* evaluating readonly modifiers requires the entire record, so
  unchanged fields still have to be written back to the parent form
  and be filtered out when *it* is set up for save, an alternative
  would be to store the `changed` and `readonly` flags alongside the
  record dict, and have the post-process only override the
  pre-computed readonly flag using force_save
* had to fix a test to match the new behaviour, the post-edition
  states turns out to be in line with the client's behaviour (or how
  it looks anyway)

Fixes #32019

odoo/addons/test_testing_utilities/ir.model.access.csv

@@ -17,3 +17,5 @@ access_test_testing_utilities_onchange_parent,access_test_testing_utilities_onch
 access_test_testing_utilities_onchange_line,access_test_testing_utilities_onchange_line,model_test_testing_utilities_onchange_line,,1,0,0,0
 access_model_test_testing_utilities_onchange_count,access_model_test_testing_utilities_onchange_count,model_test_testing_utilities_onchange_count,,1,1,1,1
 access_test_testing_utilities_onchange_count_sub,access_test_testing_utilities_onchange_count_sub,model_test_testing_utilities_onchange_count_sub,,1,1,1,1
+access_o2m_readonly_subfield_parent,access_o2m_readonly_subfield_parent,model_o2m_readonly_subfield_parent,,1,1,1,1
+access_o2m_readonly_subfield_child,access_o2m_readonly_subfield_child,model_o2m_readonly_subfield_child,,1,1,1,1

odoo/addons/test_testing_utilities/models.py

@@ -240,3 +240,24 @@ class O2MChangeSub(models.Model):
 
     parent = fields.Many2one('test_testing_utilities.onchange_count')
     name = fields.Char()
+
+class O2MReadonlySubfield(models.Model):
+    _name = 'o2m_readonly_subfield_parent'
+    _description = _name
+
+    line_ids = fields.One2many('o2m_readonly_subfield_child', 'parent_id')
+
+class O2MReadonlySubfieldChild(models.Model):
+    _name = _description = 'o2m_readonly_subfield_child'
+
+    name = fields.Char()
+    parent_id = fields.Many2one('o2m_readonly_subfield_parent')
+    f = fields.Integer(compute='_compute_f', inverse='_inverse_f', readonly=True)
+
+    @api.depends('name')
+    def _compute_f(self):
+        for r in self:
+            r.f = len(r.name) if r.name else 0
+
+    def _inverse_f(self):
+        raise AssertionError("Inverse of f should not be called")

odoo/addons/test_testing_utilities/tests/test_form_impl.py

@@ -359,7 +359,7 @@ class TestO2M(TransactionCase):
 
         self.assertEqual(
             [get(s) for s in r.subs],
-            [("5", 2, 5)]
+            [("5", 0, 5)]
         )
 
     def test_o2m_inner_default(self):
@@ -409,7 +409,10 @@ class TestO2M(TransactionCase):
             self.assertEqual(sub.value, 1)
             self.assertEqual(sub.v, 1)
 
-    def test_m2o_readonly(self):
+    def test_readonly_o2m(self):
+        """ Tests that o2m fields flagged as readonly (readonly="1" in the
+        view) can't be written to
+        """
         r = self.env['test_testing_utilities.parent'].create({
             'subs': [(0, 0, {})]
         })
@@ -422,6 +425,20 @@ class TestO2M(TransactionCase):
         with self.assertRaises(AssertionError):
             f.subs.remove(index=0)
 
+    def test_o2m_readonly_subfield(self):
+        """ Tests that readonly is applied to the field of the o2m = not sent
+        as part of the create / write values
+        """
+        f = Form(self.env['o2m_readonly_subfield_parent'])
+        with f.line_ids.new() as new_line:
+            new_line.name = "ok"
+            self.assertEqual(new_line.f, 2)
+        r = f.save()
+        self.assertEqual(
+            (r.line_ids.name, r.line_ids.f),
+            ('ok', 2)
+        )
+
     def test_o2m_dyn_onchange(self):
         f = Form(self.env['test_testing_utilities.onchange_parent'], view='test_testing_utilities.m2o_onchange_view')
 

odoo/tests/common.py

@@ -1149,14 +1149,6 @@ class Form(object):
         self._process_fvg(submodel, edition, o2m=False)
         descr['views']['edition'] = edition
 
-    def _get_node(self, f):
-        """ Find etree node for the field ``f`` in the current arch
-        """
-        return next(
-            n for n in self._view['tree'].iter('field')
-            if n.get('name') == f
-        )
-
     def __str__(self):
         return "<%s %s(%s)>" % (
             type(self).__name__,
@@ -1250,12 +1242,13 @@ class Form(object):
             return O2MProxy(self, field)
         return v
 
-    def _get_modifier(self, field, modifier, default=False):
-        d = self._view['modifiers'][field].get(modifier, default)
+    def _get_modifier(self, field, modifier, default=False, modmap=None, vals=None):
+        d = (modmap or self._view['modifiers'])[field].get(modifier, default)
         if isinstance(d, bool):
             return d
 
-        vals = self._values
+        if vals is None:
+            vals = self._values
         stack = []
         for it in reversed(d):
             if it == '!':
@@ -1369,7 +1362,8 @@ class Form(object):
         load/save
         """
         values = {}
-        for f in self._view['fields']:
+        fields = self._view['fields']
+        for f in fields:
             v = self._values[f]
             if self._get_modifier(f, 'required'):
                 assert v is not False, "{} is a required field".format(f)
@@ -1379,10 +1373,31 @@ class Form(object):
                 continue
 
             if self._get_modifier(f, 'readonly'):
-                node = self._get_node(f)
+                node = _get_node(self._view, f)
                 if not node.get('force_save'):
                     continue
 
+            if fields[f]['type'] == 'one2many':
+                view = fields[f]['views']['edition']
+                modifiers = view['modifiers']
+                oldvals = v
+                v = []
+
+                nodes = {
+                    n.get('name'): n
+                    for n in view['tree'].iter('field')
+                }
+                nodes['id'] = etree.Element('field', attrib={'name': 'id'})
+
+                for (c, rid, vs) in oldvals:
+                    if c in (0, 1):
+                        items = getattr(vs, 'changed_items', vs.items)
+                        vs = {
+                            k: v for k, v in items()
+                            if nodes[k].get('force_save') or not self._get_modifier(k, 'readonly', modmap=modifiers, vals=vs)
+                        }
+                    v.append((c, rid, vs))
+
             values[f] = v
         return values
 
@@ -1417,11 +1432,15 @@ class Form(object):
         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
-                ]
+                it = values[k] = []
+                for (c, rid, vs) in v:
+                    if c == 1 and not vs:
+                        # web client sends a 4 for unmodified o2m rows
+                        it.append((4, rid, False))
+                    elif c == 1 and isinstance(vs, UpdateDict):
+                        it.append((1, rid, dict(vs.changed_items())))
+                    else:
+                        it.append((c, rid, vs))
             else:
                 values[k] = v
         return values
@@ -1520,9 +1539,16 @@ class O2MForm(Form):
         if self._index is None:
             commands.append((0, 0, values))
         else:
-            (c, _, vs) = commands[proxy._command_index(self._index)]
-            assert c in (0, 1)
-            vs.update(values)
+            index = proxy._command_index(self._index)
+            (c, id_, vs) = commands[index]
+            if c == 0:
+                vs.update(values)
+            elif c == 1:
+                vs = UpdateDict(vs)
+                vs.update(values)
+                commands[index] = (1, id_, vs)
+            else:
+                raise AssertionError("Expected command type 0 or 1, found %s" % c)
 
         # FIXME: should be called when performing on change => value needs to be serialised into parent every time?
         proxy._parent._perform_onchange([proxy._field])
@@ -1531,20 +1557,31 @@ class O2MForm(Form):
         """ Validates values and returns only fields modified since
         load/save
         """
-        values = {}
-        for f in self._view['fields']:
-            v = self._values[f]
-            if self._get_modifier(f, 'required'):
-                assert v is not False, "{} is a required field".format(f)
+        values = UpdateDict(self._values)
+        values._changed.update(self._changed)
+
+        for f in self._view['fields']:
+            if self._get_modifier(f, 'required'):
+                assert self._values[f] is not False, "{} is a required field".format(f)
 
-            # skip unmodified fields
-            if f not in self._changed:
-                continue
-            # if self._get_modifier(f, 'readonly'):
-            #     continue
-            values[f] = v
         return values
 
+class UpdateDict(dict):
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self._changed = set()
+
+    def changed_items(self):
+        return (
+            (k, v) for k, v in self.items()
+            if k in self._changed
+        )
+
+    def update(self, *args, **kw):
+        super().update(*args, **kw)
+        if args and isinstance(args[0], UpdateDict):
+            self._changed.update(args[0]._changed)
+
 class X2MProxy(object):
     _parent = None
     _field = None
@@ -1746,6 +1783,13 @@ def record_to_values(fields, record):
         r[f] = v
     return r
 
+def _get_node(view, f, *arg):
+    """ Find etree node for the field ``f`` in the view's arch
+    """
+    return next((
+        n for n in view['tree'].iter('field')
+        if n.get('name') == f
+    ), *arg)
 
 def tagged(*tags):
     """