[FIX] ir_model_fields: properly unlink manual fields

Before this commit:

* Install module A that implements model M and field X.

* Create a new custom field Y whose reference model is not M, implemented in
any module other than A.

* Make this field either a one2many, related or compute field depending
on field X.

* Uninstall module A.

=> The registry crashes fatally, the only way to recover from this is to
log directly into the postgres database and manually eliminate the
infringing ir.model.fields record.

The reasons are multiple:

* When creating custom fields that are relational, char fields are used
to hold data like model name, field name, instead of properly storing
the related field, which would let postgres handle the unlinking
properly.

* Despite this, the ir.model.fields method _prepare_update tries to make
up for this by verifying that the ir.model.fields being unlinked during
an uninstallation are not needed elsewhere and drops them, however it
does so naively: It assumes that the "problematic" field will be defined
within the module's scope, i.e. its model reference will be a model
defined by the module being uninstalled, this isn't always the case.

This is solved by getting the dependants of each field by accessing the
model's field_triggers, if there are any triggers (i.e. dependants) then
we add them to a `failed_dependencies` list for later processing. The same
is done for field inverses (only in the case of a many2one).

After finding the failed_dependencies, a check is performed to see if the unlink is
being performed in the context of an uninstallation, in which case the failed
dependencies will be unlinked, otherwise an error will be shown to the user.

This commit is a "backport" + generalization of commit 4265026ee2

Task-ID: 1856263

odoo/addons/base/ir/ir_model.py

@@ -546,23 +546,29 @@ class IrModelFields(models.Model):
             This method prevents the modification/deletion of many2one fields
             that have an inverse one2many, for instance.
         """
+        failed_dependencies = []
+        for rec in self:
+            model = self.env[rec.model]
+            field = model._fields[rec.name]
+            for dependant, path in model._field_triggers.get(field, ()):
+                if dependant.manual:
+                    failed_dependencies.append((field, dependant))
+            for inverse in model._field_inverses.get(field, ()):
+                if inverse.manual and inverse.type == 'one2many':
+                    failed_dependencies.append((field, inverse))
+
+        if not self._context.get(MODULE_UNINSTALL_FLAG) and failed_dependencies:
+            msg = _("The field '%s' cannot be removed because the field '%s' depends on it.")
+            raise UserError(msg % failed_dependencies[0])
+        elif failed_dependencies:
+            dependants = {rel[1] for rel in failed_dependencies}
+            to_unlink = [self._get(field.model_name, field.name) for field in dependants]
+            self.browse().union(*to_unlink).unlink()
+
         self = self.filtered(lambda record: record.state == 'manual')
         if not self:
             return
 
-        for record in self:
-            model = self.env[record.model]
-            field = model._fields[record.name]
-            if field.type == 'many2one' and model._field_inverses.get(field):
-                if self._context.get(MODULE_UNINSTALL_FLAG):
-                    # automatically unlink the corresponding one2many field(s)
-                    inverses = self.search([('relation', '=', field.model_name),
-                                            ('relation_field', '=', field.name)])
-                    inverses.unlink()
-                    continue
-                msg = _("The field '%s' cannot be removed because the field '%s' depends on it.")
-                raise UserError(msg % (field, model._field_inverses[field][0]))
-
         # remove fields from registry, and check that views are not broken
         fields = [self.env[record.model]._pop_field(record.name) for record in self]
         domain = expression.OR([('arch_db', 'like', record.name)] for record in self)

odoo/addons/base/tests/test_ir_actions.py

@@ -229,6 +229,7 @@ class TestActionBindings(common.TransactionCase):
 
 class TestCustomFields(common.TransactionCase):
     MODEL = 'res.partner'
+    COMODEL = 'res.users'
 
     def setUp(self):
         # check that the registry is properly reset
@@ -244,9 +245,6 @@ class TestCustomFields(common.TransactionCase):
         self.registry.enter_test_mode()
         self.addCleanup(self.registry.leave_test_mode)
 
-        # do not reload the registry after removing a field
-        self.env = self.env(context={'_force_unlink': True})
-
     def create_field(self, name):
         """ create a custom field and return it """
         model = self.env['ir.model'].search([('model', '=', self.MODEL)])
@@ -331,3 +329,56 @@ class TestCustomFields(common.TransactionCase):
         with self.assertRaises(UserError):
             field.name = 'x_bar'
         self.assertIn('x_foo', self.env[self.MODEL]._fields)
+
+    def test_unlink_with_inverse(self):
+        """ create a custom o2m and then delete its m2o inverse """
+        model = self.env['ir.model']._get(self.MODEL)
+        comodel = self.env['ir.model']._get(self.COMODEL)
+
+        m2o_field = self.env['ir.model.fields'].create({
+            'model_id': comodel.id,
+            'name': 'x_my_m2o',
+            'field_description': 'my_m2o',
+            'ttype': 'many2one',
+            'relation': self.MODEL,
+        })
+
+        o2m_field = self.env['ir.model.fields'].create({
+            'model_id': model.id,
+            'name': 'x_my_o2m',
+            'field_description': 'my_o2m',
+            'ttype': 'one2many',
+            'relation': self.COMODEL,
+            'relation_field': m2o_field.name,
+        })
+
+        # normal mode: you cannot break dependencies
+        with self.assertRaises(UserError):
+            m2o_field.unlink()
+
+        # uninstall mode: unlink dependant fields
+        m2o_field.with_context(_force_unlink=True).unlink()
+        self.assertFalse(o2m_field.exists())
+
+    def test_unlink_with_dependant(self):
+        """ create a computed field, then delete its dependency """
+        # Also applies to compute fields
+        comodel = self.env['ir.model'].search([('model', '=', self.COMODEL)])
+
+        field = self.create_field('x_my_char')
+
+        dependant = self.env['ir.model.fields'].create({
+            'model_id': comodel.id,
+            'name': 'x_oh_boy',
+            'field_description': 'x_oh_boy',
+            'ttype': 'char',
+            'related': 'partner_id.x_my_char',
+        })
+
+        # normal mode: you cannot break dependencies
+        with self.assertRaises(UserError):
+            field.unlink()
+
+        # uninstall mode: unlink dependant fields
+        field.with_context(_force_unlink=True).unlink()
+        self.assertFalse(dependant.exists())