Move save of M2O from _save_values to save()

Closes #11381

trytond/CHANGELOG

@@ -1,3 +1,4 @@
+* Warn about implicit save() of unsaved relation
 * Raise error message with record name and field in check_xml_record
 * Cache RPC call for class method selection
 

trytond/trytond/model/modelstorage.py

@@ -7,6 +7,7 @@ import datetime
 import decimal
 import random
 import time
+import warnings
 from collections import defaultdict
 from decimal import Decimal
 from functools import lru_cache, wraps
@@ -84,6 +85,12 @@ class TimeFormatValidationError(ValidationError):
     pass
 
 
+class _UnsavedRecordError(ValueError):
+
+    def __init__(self, record):
+        self.record = record
+
+
 def without_check_access(func):
     @wraps(func)
     def wrapper(*args, **kwargs):
@@ -1833,7 +1840,7 @@ class ModelStorage(Model):
                 if value:
                     if ((value.id is None or value.id < 0)
                             and field._type != 'reference'):
-                        value.save()
+                        raise _UnsavedRecordError(value)
                     if field._type == 'reference':
                         value = str(value)
                     else:
@@ -1924,7 +1931,14 @@ class ModelStorage(Model):
                         or context != record._context):
                     latter.append(record)
                     continue
-                save_values[record] = record._save_values
+                while True:
+                    try:
+                        save_values[record] = record._save_values
+                        break
+                    except _UnsavedRecordError as e:
+                        warnings.warn(
+                            f"Using an unsaved relation record: {e.record}")
+                        e.record.save()
                 values[record] = record._values
                 record._values = None
                 if record.id is None or record.id < 0:

trytond/trytond/tests/modelstorage.py

@@ -20,6 +20,28 @@ class ModelStorageRequired(ModelSQL):
     name = fields.Char('Name', required=True)
 
 
+class ModelStorageSaveMany2One(ModelSQL):
+    "ModelStorage to test save with Many2One"
+    __name__ = 'test.modelstorage.save_m2o'
+    target = fields.Many2One('test.modelstorage.save_m2o.target', "Target")
+    targets = fields.One2Many(
+        'test.modelstorage.save_m2o.target', 'parent', "Targets")
+
+
+class ModelStorageSaveMany2OneTarget(ModelSQL):
+    "ModelStorage to test save with Many2One - Target"
+    __name__ = 'test.modelstorage.save_m2o.target'
+
+    parent = fields.Many2One('test.modelstorage.save_m2o', "Parent")
+    target = fields.Many2One(
+        'test.modelstorage.save_m2o.o2m_target.target', "Target")
+
+
+class ModelStorageSaveMany2OneTargetTarget(ModelSQL):
+    "ModelStorage to test save with Many2One - Target of Target"
+    __name__ = 'test.modelstorage.save_m2o.o2m_target.target'
+
+
 class ModelStorageSave2Many(ModelSQL):
     "Model Storage to test save with xxx2many"
     __name__ = 'test.modelstorage.save2many'
@@ -176,6 +198,9 @@ def register(module):
     Pool.register(
         ModelStorage,
         ModelStorageRequired,
+        ModelStorageSaveMany2One,
+        ModelStorageSaveMany2OneTarget,
+        ModelStorageSaveMany2OneTargetTarget,
         ModelStorageSave2Many,
         ModelStorageSave2ManyTarget,
         ModelStorageSave2ManyRelation,

trytond/trytond/tests/test_modelstorage.py

@@ -6,6 +6,7 @@ import unittest
 from trytond.model import EvalEnvironment
 from trytond.model.exceptions import (
     AccessError, DomainValidationError, RequiredValidationError)
+from trytond.model.modelstorage import _UnsavedRecordError
 from trytond.pool import Pool
 from trytond.tests.test_tryton import activate_module, with_transaction
 from trytond.transaction import Transaction
@@ -151,6 +152,78 @@ class ModelStorageTestCase(unittest.TestCase):
         with self.assertRaises(RequiredValidationError):
             ModelStorage.save([foo, bar])
 
+    @with_transaction()
+    def test_many2one_save_raise_exception(self):
+        "Test _save_values raises _UnsavedRecordError on unsaved record"
+        pool = Pool()
+        ModelStorage = pool.get('test.modelstorage.save_m2o')
+        Target = pool.get('test.modelstorage.save_m2o.target')
+
+        record = ModelStorage()
+        record.target = target = Target()
+
+        with self.assertRaises(_UnsavedRecordError) as cm:
+            record._save_values
+
+        self.assertEqual(cm.exception.record, target)
+
+    @with_transaction()
+    def test_save_many2one_create(self):
+        "Test saving a record while creating a Many2One"
+        pool = Pool()
+        ModelStorage = pool.get('test.modelstorage.save_m2o')
+        Target = pool.get('test.modelstorage.save_m2o.target')
+
+        record = ModelStorage()
+        record.target = target = Target()
+        with self.assertWarnsRegex(
+                UserWarning, r"Using an unsaved relation record:"):
+            record.save()
+
+        self.assertIsNotNone(record.id)
+        self.assertIsNotNone(target.id)
+
+    @with_transaction()
+    def test_save_many2one_create_multi(self):
+        "Test saving multiple records while creating a M2O"
+        pool = Pool()
+        ModelStorage = pool.get('test.modelstorage.save_m2o')
+        Target = pool.get('test.modelstorage.save_m2o.target')
+
+        record1 = ModelStorage()
+        record1.target = target = Target()
+        record1.save()
+
+        record2 = ModelStorage()
+        record2.target = target
+        record2.save()
+
+        ModelStorage.save([record1, record2])
+
+        self.assertIsNotNone(record1.id)
+        self.assertIsNotNone(record2.id)
+        self.assertEqual(record1.target.id, record2.target.id)
+        self.assertNotEqual(record2.id, record1.id)
+
+    @with_transaction()
+    def test_save_many2one_in_one2many_create(self):
+        "Test saving a record while creating a M2O contained in a O2M"
+        pool = Pool()
+        ModelStorage = pool.get('test.modelstorage.save_m2o')
+        Target = pool.get('test.modelstorage.save_m2o.target')
+        M2OTarget = pool.get('test.modelstorage.save_m2o.o2m_target.target')
+
+        target = Target()
+        target.target = m2o_target = M2OTarget()
+        record = ModelStorage()
+        record.targets = [target]
+        record.save()
+
+        self.assertIsNotNone(record.id)
+        self.assertIsNotNone(m2o_target.id)
+        self.assertEqual(len(record.targets), 1)
+        self.assertEqual(record.targets[0].target, m2o_target)
+
     @with_transaction()
     def test_save_one2many_create(self):
         "Test save one2many create"