From f41c3b1d9e6383bbdfae2fc8d8b5c8c16c0c4f27 Mon Sep 17 00:00:00 2001 From: "Adrien Widart (awt)" Date: Thu, 17 Aug 2023 14:53:41 +0000 Subject: [PATCH] [FIX] base: rollback if broken transaction while importing Suppose a user who has CRM module and wants to import some leads thanks to this CSV file: ```csv name,recurring_plan Coca-Cola,Plan01 SAP,Plan02 ``` And, because the recurring plans are not yet created on his database, he enables the option "Create new values" for that field. An error will occur and here is the only info the user will have: > current transaction is aborted, commands ignored until end of > transaction block When importing, we convert the data encoded by the user ([1]). To do so, we convert the provided values, field by field ([2]). Since `recurring_plan` is a `many2one` field, we go (through `_str_to_many2one`) in `db_id_for`. In this method, we `name_search` the record and then, if it does not exist and if the feature is enabled, we `name_create` it ([3]). Back to the above use case. We start with the first line and there is not any recurring plan called "Plan01" so we try to create it. But here is the issue: we only have a name to create the record although there is another required field: `number_of_months`. Therefore, it leads to a `NotNullViolation` error. This error is caught (see [3]) and, later in the same method, we will raise a `ValueError`. This error will be caught by one of the except in [2]: we will save the error and will then continue with the convertion of next values. However, because of the `NotNullViolation`, the current SQL transaction is broken. As a result, while trying to convert the second line of the file, we will `name_search` "Plan02" and it will simply lead to a `InFailedSqlTransaction` [1] https://github.com/odoo/odoo/blob/f3d7fdce608f692ecb08498ee158edf9dfbced5e/odoo/models.py#L1170-L1182 [2] https://github.com/odoo/odoo/blob/b80d4294a000e748677a3a0c1849140bf46ddbe8/odoo/addons/base/models/ir_fields.py#L117-L118 [3] https://github.com/odoo/odoo/blob/b80d4294a000e748677a3a0c1849140bf46ddbe8/odoo/addons/base/models/ir_fields.py#L472-L475 sentry-3969379125 closes odoo/odoo#132897 X-original-commit: 28373b9d261a48154b233fbbd895880680b0aed0 Signed-off-by: Adrien Widart (awt) --- odoo/addons/base/models/ir_fields.py | 3 ++- odoo/addons/test_impex/ir.model.access.csv | 2 ++ odoo/addons/test_impex/models.py | 11 +++++++++++ odoo/addons/test_impex/tests/test_load.py | 14 ++++++++++++++ 4 files changed, 29 insertions(+), 1 deletion(-) diff --git a/odoo/addons/base/models/ir_fields.py b/odoo/addons/base/models/ir_fields.py index 38c37e7e104..89f174058f8 100644 --- a/odoo/addons/base/models/ir_fields.py +++ b/odoo/addons/base/models/ir_fields.py @@ -470,7 +470,8 @@ class IrFieldsConverter(models.AbstractModel): name_create_enabled_fields = self.env.context.get('name_create_enabled_fields') or {} if name_create_enabled_fields.get(field.name): try: - id, _name = RelatedModel.name_create(name=value) + with self.env.cr.savepoint(): + id, _name = RelatedModel.name_create(name=value) except (Exception, psycopg2.IntegrityError): error_msg = _(u"Cannot create new '%s' records from their name alone. Please create those records manually and try importing again.", RelatedModel._description) else: diff --git a/odoo/addons/test_impex/ir.model.access.csv b/odoo/addons/test_impex/ir.model.access.csv index 9dc4d082572..9c5ea893e11 100644 --- a/odoo/addons/test_impex/ir.model.access.csv +++ b/odoo/addons/test_impex/ir.model.access.csv @@ -28,3 +28,5 @@ access_export_inherits_parent,access_export_inherits_parent,model_export_inherit access_export_inherits_child,access_export_inherits_child,model_export_inherits_child,base.group_user,1,1,1,1 access_export_m2o_str,access_export_m2o_str,model_export_m2o_str,base.group_user,1,1,1,1 access_export_m2o_str_child,access_export_m2o_str_child,model_export_m2o_str_child,base.group_user,1,1,1,1 +access_export_with_required_field,access_export_with_required_field,model_export_with_required_field,base.group_user,1,1,1,1 +access_export_many2one_required_subfield,access_export_many2one_required_subfield,model_export_many2one_required_subfield,base.group_user,1,1,1,1 diff --git a/odoo/addons/test_impex/models.py b/odoo/addons/test_impex/models.py index 136798c7bea..e28faeea540 100644 --- a/odoo/addons/test_impex/models.py +++ b/odoo/addons/test_impex/models.py @@ -190,3 +190,14 @@ class ChidToString(models.Model): _name = _description = 'export.m2o.str.child' name = fields.Char() + +class WithRequiredField(models.Model): + _name = _description = 'export.with.required.field' + + name = fields.Char() + value = fields.Integer(required=True) + +class Many2OneRequiredSubfield(models.Model): + _name = _description = 'export.many2one.required.subfield' + + name = fields.Many2one('export.with.required.field') diff --git a/odoo/addons/test_impex/tests/test_load.py b/odoo/addons/test_impex/tests/test_load.py index 64c3b69e6f1..d1269d00e3f 100644 --- a/odoo/addons/test_impex/tests/test_load.py +++ b/odoo/addons/test_impex/tests/test_load.py @@ -720,6 +720,20 @@ class test_m2o(ImporterCase): self.assertFalse(result['messages']) self.assertEqual(len(result['ids']), 1) + @mute_logger('odoo.sql_db') + def test_name_create_enabled_m2o_required_field(self): + self.model = self.env['export.many2one.required.subfield'] + self.env['export.with.required.field'].create({'name': 'ipsum', 'value': 10}) + context = {'name_create_enabled_fields': {'name': True}} + result = self.import_(['name'], [['lorem'], ['ipsum']], context=context) + messages = result['messages'] + self.assertTrue(messages) + self.assertEqual(len(messages), 1) + self.assertEqual(messages[0]['message'], + "No matching record found for name 'lorem' in field 'Name' and the following error was " + "encountered when we attempted to create one: Cannot create new 'export.with.required.field' " + "records from their name alone. Please create those records manually and try importing again.") + class TestInvalidStrings(ImporterCase): model_name = 'export.m2o.str'