From 41d07e9d385287d31214218be89cf1e5e77daa0a Mon Sep 17 00:00:00 2001 From: MerlinGuillaume Date: Tue, 1 Aug 2023 14:23:24 +0000 Subject: [PATCH] [FIX] base: allow deletion of inherited custom field The inherited field of a custom field cannot be deleted Steps to reproduce: 1. Install Contacts and Studio 2. Go to Contacts and open any contact 3. Toggle Studio 4. Add a field of any type in the view, remove it and close Studio 5. Go to Settings > Technical > Database Structure > Fields and search for `x_studio` Solution: Mark the inherited field as manual if its parent field is manual and allow the deletion of inherited custom field if we also delete its dependency Problem: The inherited field was not marked as custom so it was impossible to delete it opw-3093581 closes odoo/odoo#133820 X-original-commit: 526f3407c7c5b4cd014a4d091ca01b9617e6e938 Signed-off-by: Raphael Collet Signed-off-by: Guillaume Merlin (megu) --- odoo/addons/base/models/ir_model.py | 89 ++++++++++++++--------- odoo/addons/base/tests/test_ir_actions.py | 36 +++++++++ 2 files changed, 89 insertions(+), 36 deletions(-) diff --git a/odoo/addons/base/models/ir_model.py b/odoo/addons/base/models/ir_model.py index 8007883fe8d..79af2e695fa 100644 --- a/odoo/addons/base/models/ir_model.py +++ b/odoo/addons/base/models/ir_model.py @@ -314,7 +314,10 @@ class IrModel(models.Model): def unlink(self): # prevent screwing up fields that depend on these models' fields - self.field_id._prepare_update() + if self.state == 'manual': + self.field_id.filtered(lambda f: f.state == 'manual')._prepare_update() + else: + self.field_id._prepare_update() # delete fields whose comodel is being removed self.env['ir.model.fields'].search([('relation', 'in', self.mapped('model'))]).unlink() @@ -815,46 +818,64 @@ 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.get(rec.model) - if model is not None: - if rec.name in model._fields: - field = model._fields[rec.name] - else: - # field hasn't been loaded (yet?) - continue - for dep in self.pool.get_dependent_fields(field): - if dep.manual: - failed_dependencies.append((field, dep)) - for inverse in model.pool.field_inverses[field]: - if inverse.manual and inverse.type == 'one2many': - failed_dependencies.append((field, inverse)) - uninstalling = self._context.get(MODULE_UNINSTALL_FLAG) - if not uninstalling 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() + if not uninstalling and any(record.state != 'manual' for record in self): + raise UserError(_("This column contains module data and cannot be removed!")) - self = self.filtered(lambda record: record.state == 'manual') - if not self: - return + records = self # all the records to delete + fields_ = OrderedSet() # all the fields corresponding to 'records' + failed_dependencies = [] # list of broken (field, dependent_field) + + for record in self: + model = self.env.get(record.model) + if model is None: + continue + field = model._fields.get(record.name) + if field is None: + continue + fields_.add(field) + for dep in self.pool.get_dependent_fields(field): + if dep.manual: + failed_dependencies.append((field, dep)) + elif dep.inherited: + fields_.add(dep) + records |= self._get(dep.model_name, dep.name) + + for field in fields_: + for inverse in model.pool.field_inverses[field]: + if inverse.manual and inverse.type == 'one2many': + failed_dependencies.append((field, inverse)) + + self = records + + if failed_dependencies: + if not uninstalling: + field, dep = failed_dependencies[0] + raise UserError(_( + "The field '%s' cannot be removed because the field '%s' depends on it.", + field, dep, + )) + else: + self = self.union(*[ + self._get(dep.model_name, dep.name) + for field, dep in failed_dependencies + ]) + + records = self.filtered(lambda record: record.state == 'manual') + if not records: + return self # remove pending write of this field # DLE P16: if there are pending updates of the field we currently try to unlink, pop them out from the cache # test `test_unlink_with_dependant` - for record in self: + for record in records: model = self.env.get(record.model) field = model and model._fields.get(record.name) if field: self.env.cache.clear_dirty_field(field) # 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) + fields = [self.env[record.model]._pop_field(record.name) for record in records] + domain = expression.OR([('arch_db', 'like', record.name)] for record in records) views = self.env['ir.ui.view'].search(domain) try: for view in views: @@ -876,18 +897,14 @@ class IrModelFields(models.Model): # the registry has been modified, restore it self.pool.setup_models(self._cr) - @api.ondelete(at_uninstall=False) - def _unlink_if_manual(self): - # Prevent manual deletion of module columns - if any(field.state != 'manual' for field in self): - raise UserError(_("This column contains module data and cannot be removed!")) + return self def unlink(self): if not self: return True # prevent screwing up fields that depend on these fields - self._prepare_update() + self = self._prepare_update() # determine registry fields corresponding to self fields = OrderedSet() diff --git a/odoo/addons/base/tests/test_ir_actions.py b/odoo/addons/base/tests/test_ir_actions.py index 5cb2699249f..7445cd49489 100644 --- a/odoo/addons/base/tests/test_ir_actions.py +++ b/odoo/addons/base/tests/test_ir_actions.py @@ -446,6 +446,17 @@ class TestCustomFields(TestCommonCustomFields): field.name = 'x_bar' self.assertIn('x_foo', self.env[self.MODEL]._fields) + def test_unlink_base(self): + """ one cannot delete a non-custom field expect for uninstallation """ + field = self.env['ir.model.fields']._get(self.MODEL, 'ref') + self.assertTrue(field) + + with self.assertRaisesRegex(UserError, 'This column contains module data'): + field.unlink() + + # but it works in the context of uninstalling a module + field.with_context(_force_unlink=True).unlink() + def test_unlink_with_inverse(self): """ create a custom o2m and then delete its m2o inverse """ model = self.env['ir.model']._get(self.MODEL) @@ -499,6 +510,31 @@ class TestCustomFields(TestCommonCustomFields): field.with_context(_force_unlink=True).unlink() self.assertFalse(dependant.exists()) + def test_unlink_inherited_custom(self): + """ Creating a field on a model automatically creates an inherited field + in the comodel, and the latter can only be removed by deleting the + "parent" field. + """ + field = self.create_field('x_foo') + self.assertEqual(field.state, 'manual') + + inherited_field = self.env['ir.model.fields']._get(self.COMODEL, 'x_foo') + self.assertTrue(inherited_field) + self.assertEqual(inherited_field.state, 'base') + + # one cannot delete the inherited field itself + with self.assertRaises(UserError): + inherited_field.unlink() + + # but the inherited field is deleted when its parent field is + field.unlink() + self.assertFalse(field.exists()) + self.assertFalse(inherited_field.exists()) + self.assertFalse(self.env['ir.model.fields'].search_count([ + ('model', 'in', [self.MODEL, self.COMODEL]), + ('name', '=', 'x_foo'), + ])) + def test_create_binary(self): """ binary custom fields should be created as attachment=True to avoid bloating the DB when creating e.g. image fields via studio