[IMP] fields: forbid required m2o ondelete set null

A many2one field that is required means that its column cannot contain
null values, however the default ondelete policy for all m2o fields is
`set null`, which contradicts `required=True`.

This patch will, by default, apply the `restrict` policy to required m2o
fields unless the ondelete attribute is explicitly specified. However,
if the policy specified is `set null`, a ValueError will be raised
whenever the registry registers the field as this makes no sense.

The default ondelete policy remains unchanged for non-required m2o
fields (set null)

closes odoo/odoo#30122

addons/google_drive/models/google_drive.py

@@ -176,7 +176,7 @@ class GoogleDrive(models.Model):
         return config_values
 
     name = fields.Char('Template Name', required=True)
-    model_id = fields.Many2one('ir.model', 'Model', ondelete='set null', required=True)
+    model_id = fields.Many2one('ir.model', 'Model', required=True)
     model = fields.Char('Related Model', related='model_id.model', readonly=True)
     filter_id = fields.Many2one('ir.filters', 'Filter', domain="[('model_id', '=', model)]")
     google_drive_template_url = fields.Char('Template URL', required=True)

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

@@ -28,3 +28,4 @@ access_test_new_api_monetary_related,access_test_new_api_monetary_related,model_
 access_test_new_api_monetary_custom,access_test_new_api_monetary_custom,model_test_new_api_monetary_custom,,1,1,1,1
 access_test_new_api_monetary_inherits,access_test_new_api_monetary_inherits,model_test_new_api_monetary_inherits,,1,1,1,1
 access_test_new_api_field_with_caps,access_test_new_api_field_with_caps,model_test_new_api_field_with_caps,,1,1,1,1
+access_test_new_api_req_m2o,access_test_new_api_req_m2o,model_test_new_api_req_m2o,,1,1,1,1

odoo/addons/test_new_api/models.py

@@ -516,3 +516,11 @@ class FieldWithCaps(models.Model):
     _description = 'Model with field defined with capital letters'
 
     pArTneR_321_id = fields.Many2one('res.partner')
+
+
+class RequiredM2O(models.Model):
+    _name = 'test_new_api.req_m2o'
+    _description = 'Required Many2one'
+
+    foo = fields.Many2one('res.currency', required=True, ondelete='cascade')
+    bar = fields.Many2one('res.country', required=True)

odoo/addons/test_new_api/tests/test_new_fields.py

@@ -1447,3 +1447,25 @@ class TestParentStore(common.TransactionCase):
         """ Move multiple nodes to create a cycle. """
         with self.assertRaises(UserError):
             self.cats(1, 3).write({'parent': self.cats(9).id})
+
+
+class TestRequiredMany2one(common.TransactionCase):
+
+    def test_explicit_ondelete(self):
+        field = self.env['test_new_api.req_m2o']._fields['foo']
+        self.assertEqual(field.ondelete, 'cascade')
+
+    def test_implicit_ondelete(self):
+        field = self.env['test_new_api.req_m2o']._fields['bar']
+        self.assertEqual(field.ondelete, 'restrict')
+
+    def test_explicit_set_null(self):
+        Model = self.env['test_new_api.req_m2o']
+        field = Model._fields['foo']
+
+        # invalidate registry to redo the setup afterwards
+        self.registry.registry_invalidated = True
+        self.patch(field, 'ondelete', 'set null')
+
+        with self.assertRaises(ValueError):
+            field._setup_regular_base(Model)

odoo/fields.py

@@ -2052,7 +2052,7 @@ class Many2one(_Relational):
     type = 'many2one'
     column_type = ('int4', 'int4')
     _slots = {
-        'ondelete': 'set null',         # what to do when value is deleted
+        'ondelete': None,               # what to do when value is deleted
         'auto_join': False,             # whether joins are generated upon search
         'delegate': False,              # whether self implements delegation
     }
@@ -2066,6 +2066,22 @@ class Many2one(_Relational):
         if not self.delegate:
             self.delegate = name in model._inherits.values()
 
+    def _setup_regular_base(self, model):
+        super()._setup_regular_base(model)
+        # 3 cases:
+        # 1) The ondelete attribute is not defined, we assign it a sensible default
+        # 2) The ondelete attribute is defined and its definition makes sense
+        # 3) The ondelete attribute is explicitly defined as 'set null' for a required m2o,
+        #    this is considered a programming error.
+        if not self.ondelete:
+            self.ondelete = 'restrict' if self.required else 'set null'
+        if self.ondelete == 'set null' and self.required:
+            raise ValueError(
+                "The m2o field %s of model %s is required but declares its ondelete policy "
+                "as being 'set null'. Only 'restrict' and 'cascade' make sense."
+                % (self.name, model._name)
+            )
+
     def update_db(self, model, columns):
         comodel = model.env[self.comodel_name]
         if not model.is_transient() and comodel.is_transient():