[FIX] base: reset _rec_name if x_name field is deleted

Commit 6a0028f91944b1d9e4eac026e86ca249ef5bc7ee introduced a change that
allowed field_triggers to be computed lazily per registry, this change
introduced a behavioral change that is not easy to notice, in order to
explain the behavioral change I will use the following example which was
the original bug reported:

    - Install studio
    - Create an app (create a new custom model M)
    - Uninstall studio
    -> Uninstall fails because field display_name of the custom model M
    depends on field M.x_name which has been removed due to the
    uninstall process.

To understand why it didn't happen before the aforementioned commit, we
must first understand what happens with said commit applied:

    - We trigger the uninstall of studio, this triggers the uninstall of
    any modules that depend on it, namely studio_customizations which is
    the module in which all customizations done with studio live in.

    - We gather all data belonging to the studio_customization and we
    start deleting in the following order: ...,
    ir.model.fields.selection, ir.model.fields, ..., ir.model

    - During the unlink process, we first remove the actual fields from
    the model instances before deleting their database reflections, this
    is done in the ir.model.fields._drop_column() method, it is this
    method that will delete the x_name field but **not** the
    display_name field, since it is a base field and not a custom one.

    - After the deletion of the fields in memory, we call modified() to
    mark fields that might've depended on the fields we just modified so
    that they can be recomputed later.

    - The call to modified will in turn access field_triggers, but since
    we're in a new registry and field_triggers is a lazy property, it
    will be computed right at this moment, this means that it will call
    resolve_depends on the display_name field which still exists, and
    this field has a dependency on the x_name field that we just
    deleted! This is what will trigger the crash.

With that context, we can now understand how it didn't crash before
commit 6a0028f91944b1d9e4eac026e86ca249ef5bc7ee:

    - Before the aforementioned commit, the field_triggers attribute was
    computed during the registry's setup_models(), in the case of an
    uninstall this call to setup_models was done way before the
    uninstall step of the registry (Step 3 is the last to call
    setup_models before Step 5).

    - This means that the old behavior was technically a bug, because
    right after removing x_name from the model, the field_triggers still
    contained a dependency from display_name to x_name, the former being
    no-longer present in memory.

With this commit, we simply reset the _rec_name and the dependencies of
the display_name if the x_name field is being removed, this ensures that
the computation of field_triggers won't crash and burn.

opw-2452498
opw-2478589

closes odoo/odoo#67747

Signed-off-by: Adrian Torres (adt) <adt@odoo.com>
Co-authored-by: Raphael Collet <rco@odoo.com>

2 files changed, 19 insertions, 8 deletions

diff --git a/odoo/addons/base/tests/test_ir_model.py b/odoo/addons/base/tests/test_ir_model.py
index eea989f1237..09a0180a974 100644
--- a/odoo/addons/base/tests/test_ir_model.py
+++ b/odoo/addons/base/tests/test_ir_model.py
@@ -311,6 +311,20 @@ class TestIrModel(SavepointCase):
         }]
         self.assertEqual(groups, expected, 'should include 2 empty ripeness stages')
 
+    def test_rec_name_deletion(self):
+        """Check that deleting 'x_name' does not crash."""
+        record = self.env['x_bananas'].create({'x_name': "Ifan Ben-Mezd"})
+        self.assertEqual(record._rec_name, 'x_name')
+        self.assertEqual(record._fields['display_name'].depends, ('x_name',))
+        self.assertEqual(record.display_name, "Ifan Ben-Mezd")
+
+        # unlinking x_name should fixup _rec_name and display_name
+        self.env['ir.model.fields']._get('x_bananas', 'x_name').unlink()
+        record = self.env['x_bananas'].browse(record.id)
+        self.assertEqual(record._rec_name, None)
+        self.assertEqual(record._fields['display_name'].depends, ())
+        self.assertEqual(record.display_name, f"x_bananas,{record.id}")
+
 
 @tagged('test_eval_context')
 class TestEvalContext(TransactionCase):
diff --git a/odoo/models.py b/odoo/models.py
index 6bd10fde74a..ff34e09f754 100644
--- a/odoo/models.py
+++ b/odoo/models.py
@@ -412,6 +412,10 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})):
         field = cls._fields.pop(name, None)
         if hasattr(cls, name):
             delattr(cls, name)
+        if cls._rec_name == name:
+            # fixup _rec_name and display_name's dependencies
+            cls._rec_name = None
+            cls.display_name.depends = tuple(dep for dep in cls.display_name.depends if dep != name)
         return field
 
     @api.model
@@ -2848,14 +2852,7 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})):
                 raise
 
         for name in bad_fields:
-            del cls._fields[name]
-            delattr(cls, name)
-
-        # fix up _rec_name
-        if 'x_name' in bad_fields and cls._rec_name == 'x_name':
-            cls._rec_name = None
-            field = cls._fields['display_name']
-            field.depends = tuple(name for name in field.depends if name != 'x_name')
+            self._pop_field(name)
 
     @api.model
     def _setup_complete(self):