pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

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

Browse Source 
*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.
This commit is contained in:
Aaron Bohy
2020-01-21 11:59:22 +00:00
parent 01e3a5996f
commit 50bf8309f8
7 changed files with 139 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/sale/static/src/js/product_configurator_widget.js
+13 -14
View File
@@ -109,24 +109,23 @@ var ProductConfiguratorWidget = relationalFields.FieldMany2One.extend({
*
* @override
* @param {OdooEvent} ev
* {boolean} ev.data.preventProductIdCheck prevent the product configurator widget
* @param {boolean} ev.data.preventProductIdCheck prevent the product configurator widget
* from looping forever when it needs to change the 'product_template_id'
*
* @private
*/
_onFieldChanged: function (ev) {
var self = this;
this._super.apply(this, arguments);
if (ev.data.changes && !ev.data.preventProductIdCheck && ev.data.changes.product_template_id) {
self._onTemplateChange(ev.data.changes.product_template_id.id, ev.data.dataPointID);
} else if (ev.data.changes && ev.data.changes.product_id) {
self._onProductChange(ev.data.changes.product_id.id, ev.data.dataPointID).then(function (wizardOpened) {
if (!wizardOpened) {
self._onLineConfigured();
}
});
reset: async function (record, ev) {
await this._super(...arguments);
if (ev.target === this) {
if (ev.data.changes && !ev.data.preventProductIdCheck && ev.data.changes.product_template_id) {
this._onTemplateChange(record.data.product_template_id.data.id, ev.data.dataPointID);
} else if (ev.data.changes && ev.data.changes.product_id) {
this._onProductChange(record.data.product_id.data.id, ev.data.dataPointID).then(wizardOpened => {
if (!wizardOpened) {
this._onLineConfigured();
}
});
}
}
},
addons/sale_product_configurator/static/src/js/product_configurator_widget.js
+5 -3
View File
@@ -33,9 +33,11 @@ ProductConfiguratorWidget.include({
* @override
* @private
*/
_onFieldChanged: function (ev) {
this.restoreProductTemplateId = this.recordData.product_template_id;
this.optionalProducts = (ev.data && ev.data.optionalProducts) || this.optionalProducts;
reset: function (record, ev) {
if (ev.target === this) {
this.restoreProductTemplateId = this.recordData.product_template_id;
this.optionalProducts = (ev.data && ev.data.optionalProducts) || this.optionalProducts;
}
this._super.apply(this, arguments);
},
addons/web/static/src/js/fields/relational_fields.js
+3 -14
View File
@@ -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();
}
addons/web/static/src/js/views/basic/basic_model.js
+44 -6
View File
@@ -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;
addons/web/static/tests/fields/relational_fields/field_many2one_tests.js
+1
View File
@@ -967,6 +967,7 @@ QUnit.module('fields', {}, function () {
},
});
var parent = new StandaloneWidget(model);
model.setParent(parent);
testUtils.mock.addMockEnvironment(parent, {
data: self.data,
mockRPC: function (route, args) {
addons/web/static/tests/fields/relational_fields/field_one2many_tests.js
+50
View File
@@ -8741,6 +8741,56 @@ QUnit.module('fields', {}, function () {
form.destroy();
});
QUnit.test('one2many reset by onchange (of another field) while being edited', async function (assert) {
// In this test, we have a many2one and a one2many. The many2one has an onchange that
// updates the value of the one2many. We set a new value to the many2one (name_create)
// such that the onchange is delayed. During the name_create, we click to add a new row
// to the one2many. After a while, we unlock the name_create, which triggers the onchange
// and resets the one2many. At the end, we want the row to be in edition.
assert.expect(3);
const prom = testUtils.makeTestPromise();
this.data.partner.onchanges = {
trululu: obj => {
obj.p = [[5]].concat(obj.p);
},
};
const form = await createView({
View: FormView,
model: 'partner',
data: this.data,
arch: `
<form>
<field name="trululu"/>
<field name="p">
<tree editable="top"><field name="product_id" required="1"/></tree>
</field>
</form>`,
mockRPC: function (route, args) {
const result = this._super.apply(this, arguments);
if (args.method === 'name_create') {
return prom.then(() => result);
}
return result;
},
});
// set a new value for trululu (will delay the onchange)
await testUtils.fields.many2one.searchAndClickItem('trululu', {search: 'new value'});
// add a row in p
await testUtils.dom.click(form.$('.o_field_x2many_list_row_add a'));
assert.containsNone(form, '.o_data_row');
// resolve the name_create to trigger the onchange, and the reset of p
prom.resolve();
await testUtils.nextTick();
assert.containsOnce(form, '.o_data_row');
assert.hasClass(form.$('.o_data_row'), 'o_selected_row');
form.destroy();
});
});
});
});
addons/web/static/tests/fields/relational_fields_tests.js
+23
View File
@@ -1914,6 +1914,29 @@ QUnit.module('relational_fields', {
form.destroy();
});
QUnit.test('fieldmany2many tags: quick create a new record', async function (assert) {
assert.expect(3);
const form = await createView({
View: FormView,
model: 'partner',
data: this.data,
arch: `<form><field name="timmy" widget="many2many_tags"/></form>`,
});
assert.containsNone(form, '.o_field_many2manytags .badge');
await testUtils.fields.many2one.searchAndClickItem('timmy', {search: 'new value'});
assert.containsOnce(form, '.o_field_many2manytags .badge');
await testUtils.form.clickSave(form);
assert.strictEqual(form.el.querySelector('.o_field_many2manytags').innerText, 'new value');
form.destroy();
});
QUnit.module('FieldRadio');
QUnit.test('fieldradio widget on a many2one in a new record', async function (assert) {