[FIX] web,*: fix race condition with m2o quick create

*sale,sale_product_configurator

Let's assume a form view containing a many2one with an onchange
that updates the value of a one2many. Do a quick create in the
many2one. While the name_create request is pending, add a row to
the one2many but do not leave it. When the name_create returns, and
the onchange has been performed, the one2many is reset, and the row
is no longer in edition (worse, it could be invalid, i.e. in a state
that the user could not have reached in a normal situation).

This commit fixes this issue by considering the whole [name_create +
onchange] operation as one. This operation is executed in the mutex
of the model, so the other request (adding a row to the one2many) is
delayed until the many2one value has been correctly set.

This fixes an issue with the sale and rental tours (on sale_order),
that had been deactivated for a while.

2 files changed, 47 insertions, 20 deletions

diff --git a/addons/web/static/src/js/fields/relational_fields.js b/addons/web/static/src/js/fields/relational_fields.js
index c1f116784fa..deff2fbebcf 100644
--- a/addons/web/static/src/js/fields/relational_fields.js
+++ b/addons/web/static/src/js/fields/relational_fields.js
@@ -489,23 +489,12 @@ var FieldMany2One = AbstractField.extend({
                 dialog.on('closed', self, createDone);
             };
             if (self.nodeOptions.quick_create) {
-                var nameCreateDef = self._rpc({
-                    model: self.field.relation,
-                    method: 'name_create',
-                    args: [name],
-                    context: self.record.getContext(self.recordParams),
-                }).guardedCatch(function (reason) {
+                const prom = self.reinitialize({id: false, display_name: name});
+                prom.guardedCatch(reason => {
                     reason.event.preventDefault();
                     slowCreate();
                 });
-                self.dp.add(nameCreateDef)
-                    .then(function (result) {
-                        if (self.mode === "edit") {
-                            self.reinitialize({id: result[0], display_name: result[1]});
-                        }
-                        createDone();
-                    })
-                    .guardedCatch(reject);
+                self.dp.add(prom).then(createDone).guardedCatch(reject);
             } else {
                 slowCreate();
             }
diff --git a/addons/web/static/src/js/views/basic/basic_model.js b/addons/web/static/src/js/views/basic/basic_model.js
index a4c6d0e8795..e43c7f2a0e6 100644
--- a/addons/web/static/src/js/views/basic/basic_model.js
+++ b/addons/web/static/src/js/views/basic/basic_model.js
@@ -1472,7 +1472,7 @@ var BasicModel = AbstractModel.extend({
             if (field && (field.type === 'one2many' || field.type === 'many2many')) {
                 defs.push(this._applyX2ManyChange(record, fieldName, changes[fieldName], options));
             } else if (field && (field.type === 'many2one' || field.type === 'reference')) {
-                defs.push(this._applyX2OneChange(record, fieldName, changes[fieldName]));
+                defs.push(this._applyX2OneChange(record, fieldName, changes[fieldName], options));
             } else {
                 record._changes[fieldName] = changes[fieldName];
             }
@@ -1523,18 +1523,39 @@ var BasicModel = AbstractModel.extend({
      * onchange modifies a many2one field. For this reason, we need (sometimes)
      * to do a /name_get to fetch a display_name.
      *
+     * Moreover, for the many2one case, a new value can sometimes be set (i.e.
+     * a display_name is given, but no id). When this happens, we first do a
+     * name_create.
+     *
      * @param {Object} record
      * @param {string} fieldName
      * @param {Object} [data]
+     * @param {Object} [options]
+     * @param {string} [options.viewType] current viewType. If not set, we will
+     *   assume main viewType from the record
      * @returns {Promise}
      */
-    _applyX2OneChange: function (record, fieldName, data) {
+    _applyX2OneChange: async function (record, fieldName, data, options) {
+        options = options || {};
         var self = this;
-        if (!data || !data.id) {
+        if (!data || (!data.id && !data.display_name)) {
             record._changes[fieldName] = false;
             return Promise.resolve();
         }
 
+        const field = record.fields[fieldName];
+        const coModel = field.type === 'reference' ? data.model : field.relation;
+        if (field.type === 'many2one' && !data.id && data.display_name) {
+            // only display_name given -> do a name_create
+            const result = await this._rpc({
+                model: coModel,
+                method: 'name_create',
+                args: [data.display_name],
+                context: this._getContext(record, {fieldName: fieldName, viewType: options.viewType}),
+            });
+            data = {id: result[0], display_name: result[1]};
+        }
+
         // here, we check that the many2one really changed. If the res_id is the
         // same, we do not need to do any extra work. It can happen when the
         // user edited a manyone (with the small form view button) with an
@@ -1551,11 +1572,9 @@ var BasicModel = AbstractModel.extend({
             return Promise.resolve();
         }
         var rel_data = _.pick(data, 'id', 'display_name');
-        var field = record.fields[fieldName];
 
         // the reference field doesn't store its co-model in its field metadata
         // but directly in the data (as the co-model isn't fixed)
-        var coModel = field.type === 'reference' ? data.model : field.relation;
         var def;
         if (rel_data.display_name === undefined) {
             // TODO: refactor this to use _fetchNameGet
@@ -1802,7 +1821,7 @@ var BasicModel = AbstractModel.extend({
      *   (only supported by the 'CREATE' command.operation)
      * @returns {Promise}
      */
-    _applyX2ManyChange: function (record, fieldName, command, options) {
+    _applyX2ManyChange: async function (record, fieldName, command, options) {
         if (command.operation === 'TRIGGER_ONCHANGE') {
             // the purpose of this operation is to trigger an onchange RPC, so
             // there is no need to apply any change on the record (the changes
@@ -1848,6 +1867,25 @@ var BasicModel = AbstractModel.extend({
                 // record added) or an array of dict of values
                 var data = _.isArray(command.ids) ? command.ids : [command.ids];
 
+                // name_create records for which there is no id (typically, could
+                // be the case of a quick_create in a many2many_tags, so data.length
+                // is 1)
+                for (const r of data) {
+                    if (!r.id && r.display_name) {
+                        const prom = this._rpc({
+                            model: field.relation,
+                            method: 'name_create',
+                            args: [r.display_name],
+                            context: this._getContext(record, {fieldName: fieldName, viewType: options.viewType}),
+                        }).then(result => {
+                            r.id = result[0];
+                            r.display_name = result[1];
+                        });
+                        defs.push(prom);
+                    }
+                }
+                await Promise.all(defs);
+
                 // Ensure the local data repository (list) boundaries can handle incoming records (data)
                 if (data.length + list.res_ids.length > list.limit) {
                     list.limit = data.length + list.res_ids.length;