[FIX] base: virtual group fields on users with new default/onchange

* override `onchange` in res.users in order to properly generate and
  send the reified group fields alongside the rest: while default_get
  sets them up, those fields then get stripped by the onchange
  machinery as they don't actually exist on the model
* add a test to check for it
* fix SSF in relation with the new default/onchange system
  - because fields may not actually exist on the record,
    `record_to_values` needs to `read` the record data rather than
    directly access the recordset: the recordset likely will not
    know about fake fields
  - after the initial onchange has run the form must be filled with
    falsy values in case the onchange has not sent defaults for
    everything
  - on creation, all fields in the form should be considered modified

Task 2341153

closes odoo/odoo#57987

Signed-off-by: Xavier Morel (xmo) <xmo@odoo.com>

3 files changed, 48 insertions, 9 deletions

diff --git a/odoo/addons/base/models/res_users.py b/odoo/addons/base/models/res_users.py
index e642e4d5c30..903007e4db0 100644
--- a/odoo/addons/base/models/res_users.py
+++ b/odoo/addons/base/models/res_users.py
@@ -1387,6 +1387,16 @@ class UsersView(models.Model):
         self._add_reified_groups(group_fields, values)
         return values
 
+    def onchange(self, values, field_name, field_onchange):
+        field_onchange['groups_id'] = ''
+        result = super().onchange(values, field_name, field_onchange)
+        if not field_name: # merged default_get
+            self._add_reified_groups(
+                filter(is_reified_group, field_onchange),
+                result.setdefault('value', {})
+            )
+        return result
+
     def read(self, fields=None, load='_classic_read'):
         # determine whether reified groups fields are required, and which ones
         fields1 = fields or list(self.fields_get())
diff --git a/odoo/addons/base/tests/test_res_users.py b/odoo/addons/base/tests/test_res_users.py
index 2e73c751481..34f21356abe 100644
--- a/odoo/addons/base/tests/test_res_users.py
+++ b/odoo/addons/base/tests/test_res_users.py
@@ -1,7 +1,7 @@
 # -*- coding: utf-8 -*-
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
-from odoo.tests.common import TransactionCase
+from odoo.tests.common import TransactionCase, Form, tagged
 
 
 class TestUsers(TransactionCase):
@@ -114,3 +114,18 @@ class TestUsers(TransactionCase):
             "On user company change, if its partner_id has already a company_id,"
             "the company_id of the partner_id shall be updated"
         )
+
+@tagged('post_install', '-at_install')
+class TestUsers2(TransactionCase):
+    def test_reified_groups(self):
+        """ The groups handler doesn't use the "real" view with pseudo-fields
+        during installation, so it always works (because it uses the normal
+        groups_id field).
+        """
+        # use the specific views which has the pseudo-fields
+        f = Form(self.env['res.users'], view='base.view_users_form')
+        f.name = "bob"
+        f.login = "bob"
+        user = f.save()
+
+        self.assertIn(self.env.ref('base.group_user'), user.groups_id)
diff --git a/odoo/tests/common.py b/odoo/tests/common.py
index 01318a64422..522de3a2434 100644
--- a/odoo/tests/common.py
+++ b/odoo/tests/common.py
@@ -1701,6 +1701,15 @@ class Form(object):
         # call onchange with an empty list of fields; this retrieves default
         # values, applies onchanges and return the result
         self._perform_onchange([])
+        # fill in whatever fields are still missing with falsy values
+        vals.update(
+            (f, _cleanup_from_default(descr['type'], False))
+            for f, descr in self._view['fields'].items()
+            if f not in vals
+        )
+        # mark all fields as modified (though maybe this should be done on
+        # save when creating for better reliability?)
+        self._changed.update(self._view['fields'])
 
     def _init_from_values(self, values):
         self._values.update(
@@ -1981,7 +1990,7 @@ class Form(object):
             _logger.getChild('onchange').warning("%(title)s %(message)s" % result.get('warning'))
         values = result.get('value', {})
         # mark onchange output as changed
-        self._changed.update(values.keys())
+        self._changed.update(values.keys() & self._view['fields'].keys())
         self._values.update(
             (k, self._cleanup_onchange(
                 self._view['fields'][k],
@@ -2385,16 +2394,21 @@ class M2MProxy(X2MProxy, collections.Sequence):
 
 def record_to_values(fields, record):
     r = {}
-    for f, descr in fields.items():
-        v = record[f]
+    # don't read the id explicitly, not sure why but if any of the "magic" hr
+    # field is read alongside `id` then it blows up e.g.
+    # james.read(['barcode']) works fine but james.read(['id', 'barcode'])
+    # triggers an ACL error on barcode, likewise km_home_work or
+    # emergency_contact or whatever. Since we always get the id anyway, just
+    # remove it from the fields to read
+    to_read = list(fields.keys() - {'id'})
+    for f, v in record.read(to_read)[0].items():
+        descr = fields[f]
         if descr['type'] == 'many2one':
-            assert v._name == descr['relation']
-            v = v.id
+            v = v and v[0]
         elif descr['type'] == 'many2many':
-            assert v._name == descr['relation']
-            v = [(6, 0, v.ids)]
+            v = [(6, 0, v or [])]
         elif descr['type'] == 'one2many':
-            v = [(1, r.id, None) for r in v]
+            v = [(1, r, None) for r in v or []]
         elif descr['type'] == 'datetime' and isinstance(v, datetime):
             v = odoo.fields.Datetime.to_string(v)
         elif descr['type'] == 'date' and isinstance(v, date):