[FIX] web: BasicModel: correctly apply onchanges result
Onchange results weren't correctly applied for many2manys, and for x2manys inside x2manys. More precisely, if an onchange RPC returned a command 4 (link to) for a many2many field, this command was ignored. If it returned nested commands (commands 0, 1 or 4 inside commands 0 or 1), they were ignored as well. The function that applies the result of an onchange is now recursive, and it handles commands 0, 1, 4 and 5.
This commit is contained in:
Aaron Bohy
committed by
Géry Debongnie
Géry Debongnie
parent
bf29aab3ba
commit
f6c20c61d6
@@ -834,7 +834,7 @@ var BasicModel = AbstractModel.extend({
|
||||
return $.when.apply($, defs).then(function () {
|
||||
var onchangeDef;
|
||||
if (onChangeFields.length) {
|
||||
onchangeDef = self._applyOnChange(record, onChangeFields).then(function (result) {
|
||||
onchangeDef = self._performOnChange(record, onChangeFields).then(function (result) {
|
||||
delete record._warning;
|
||||
return _.keys(changes).concat(Object.keys(result && result.value || {}));
|
||||
});
|
||||
@@ -899,133 +899,104 @@ var BasicModel = AbstractModel.extend({
|
||||
});
|
||||
},
|
||||
/**
|
||||
* This method is quite important: it is supposed to perform the /onchange
|
||||
* rpc and apply the result.
|
||||
* Applies the result on an onchange RPC on a record.
|
||||
*
|
||||
* @private
|
||||
* @param {Object} values the result of the onchange RPC (a mapping of
|
||||
* fieldnames to their value)
|
||||
* @param {Object} record
|
||||
* @param {string[]} fields changed fields
|
||||
* @returns {Deferred} The returned deferred can fail, in which case the
|
||||
* fail value will be the warning message received from the server
|
||||
* @returns {Deferred}
|
||||
*/
|
||||
_applyOnChange: function (record, fields) {
|
||||
_applyOnChange: function (values, record) {
|
||||
var self = this;
|
||||
var onchange_spec = this._buildOnchangeSpecs(record);
|
||||
var idList = record.data.id ? [record.data.id] : [];
|
||||
var options = {};
|
||||
if (fields.length === 1) {
|
||||
fields = fields[0];
|
||||
// if only one field changed, add its context to the RPC context
|
||||
options.fieldName = fields;
|
||||
}
|
||||
var context = this._getContext(record, options);
|
||||
var currentData = this._generateOnChangeData(record);
|
||||
var defs = [];
|
||||
var rec;
|
||||
record._changes = record._changes || {};
|
||||
_.each(values, function (val, name) {
|
||||
var field = record.fields[name];
|
||||
if (!field) {
|
||||
return; // ignore changes of unknown fields
|
||||
}
|
||||
|
||||
return self._rpc({
|
||||
model: record.model,
|
||||
method: 'onchange',
|
||||
args: [idList, currentData, fields, onchange_spec, context],
|
||||
})
|
||||
.then(function (result) {
|
||||
if (!record._changes) {
|
||||
// if the _changes key does not exist anymore, it means that
|
||||
// it was removed by discarding the changes after the rpc
|
||||
// to onchange. So, in that case, the proper response is to
|
||||
// ignore the onchange.
|
||||
return;
|
||||
}
|
||||
if (result.warning) {
|
||||
self.trigger_up('warning', {
|
||||
message: result.warning.message,
|
||||
title: result.warning.title,
|
||||
type: 'dialog',
|
||||
if (field.type === 'many2one' ) {
|
||||
var id = false;
|
||||
// in some case, the value returned by the onchange can
|
||||
// be false (no value), so we need to avoid creating a
|
||||
// local record for that.
|
||||
if (val) {
|
||||
// when the value isn't false, it can be either
|
||||
// an array [id, display_name] or just an id.
|
||||
var data = _.isArray(val) ?
|
||||
{id: val[0], display_name: val[1]} :
|
||||
{id: val};
|
||||
rec = self._makeDataPoint({
|
||||
context: record.context,
|
||||
data: data,
|
||||
modelName: field.relation,
|
||||
});
|
||||
record._warning = true;
|
||||
id = rec.id;
|
||||
}
|
||||
var defs = [];
|
||||
_.each(result.value, function (val, name) {
|
||||
var field = record.fields[name];
|
||||
if (!field) { return; } // ignore changes of unknown fields
|
||||
|
||||
var rec;
|
||||
if (field.type === 'many2one' ) {
|
||||
// in some case, the value returned by the onchange can
|
||||
// be false (no value), so we need to avoid creating a
|
||||
// local record for that.
|
||||
// FIXME: shouldn't we erase the value in that case?
|
||||
if (val) {
|
||||
// when the value isn't false, it can be either
|
||||
// an array [id, display_name] or just an id.
|
||||
var data = _.isArray(val) ?
|
||||
{id: val[0], display_name: val[1]} :
|
||||
{id: val};
|
||||
rec = self._makeDataPoint({
|
||||
context: context,
|
||||
data: data,
|
||||
modelName: field.relation,
|
||||
});
|
||||
record._changes[name] = rec.id;
|
||||
record._changes[name] = id;
|
||||
} else if (field.type === 'one2many' || field.type === 'many2many') {
|
||||
var listId = record._changes[name] || record.data[name];
|
||||
var list;
|
||||
if (listId) {
|
||||
list = self.localData[listId];
|
||||
} else {
|
||||
var fieldInfo = record.fieldsInfo[record.viewType][name];
|
||||
list = self._makeDataPoint({
|
||||
fieldsInfo: fieldInfo.fieldsInfo,
|
||||
modelName: field.relation,
|
||||
type: 'list',
|
||||
viewtype: fieldInfo.viewType,
|
||||
});
|
||||
}
|
||||
record._changes[name] = list.id;
|
||||
var shouldLoad = false;
|
||||
_.each(val, function (command) {
|
||||
if (command[0] === 0 || command[0] === 1) {
|
||||
// CREATE or UPDATE
|
||||
var params = {
|
||||
context: list.context,
|
||||
fields: list.fields,
|
||||
fieldsInfo: list.fieldsInfo,
|
||||
modelName: list.model,
|
||||
parentID: list.id,
|
||||
viewType: list.viewType,
|
||||
};
|
||||
if (command[0] === 1) {
|
||||
params.res_id = command[1];
|
||||
}
|
||||
} else if (field.type === 'one2many' || field.type === 'many2many') {
|
||||
record._changes = record._changes || {};
|
||||
var listId = record._changes[name] || record.data[name];
|
||||
var list = self.localData[listId];
|
||||
_.each(val, function (command) {
|
||||
if (command[0] === 0 || command[0] === 1) {
|
||||
// CREATE or UPDATE
|
||||
var params = {
|
||||
context: context,
|
||||
fields: list.fields,
|
||||
fieldsInfo: list.fieldsInfo,
|
||||
modelName: list.model,
|
||||
parentID: list.id,
|
||||
viewType: list.viewType,
|
||||
};
|
||||
if (command[0] === 1) {
|
||||
params.res_id = command[1];
|
||||
}
|
||||
rec = self._makeDataPoint(params);
|
||||
var data = {};
|
||||
_.each(Object.keys(command[2]), function (name) {
|
||||
var field = rec.fields[name];
|
||||
if (!field) {
|
||||
// ignore if this field is unknown as we don't
|
||||
// know if it requires a special handling
|
||||
return;
|
||||
}
|
||||
data[name] = command[2][name];
|
||||
if (field.type === 'many2one') {
|
||||
var r = self._makeDataPoint({
|
||||
modelName: field.relation,
|
||||
data: {
|
||||
id: data[name][0],
|
||||
display_name: data[name][1],
|
||||
},
|
||||
});
|
||||
data[name] = r.id;
|
||||
}
|
||||
});
|
||||
rec._changes = data;
|
||||
list._changes.push(rec.id);
|
||||
}
|
||||
if (command[0] === 5) {
|
||||
// DELETE ALL
|
||||
list._changes = [];
|
||||
}
|
||||
});
|
||||
} else if (field.type === 'date') {
|
||||
// process data: convert into a moment instance
|
||||
record._changes[name] = fieldUtils.parse.date(val, field, {isUTC: true});
|
||||
} else if (field.type === 'datetime') {
|
||||
// process datetime: convert into a moment instance
|
||||
record._changes[name] = fieldUtils.parse.datetime(val, field, {isUTC: true});
|
||||
} else {
|
||||
record._changes[name] = val;
|
||||
rec = self._makeDataPoint(params);
|
||||
list._changes.push(rec.id);
|
||||
defs.push(self._applyOnChange(command[2], rec));
|
||||
} else if (command[0] === 4) {
|
||||
// LINK TO
|
||||
list.res_ids.push(command[1]);
|
||||
list.count++;
|
||||
shouldLoad = true;
|
||||
} else if (command[0] === 5) {
|
||||
// DELETE ALL
|
||||
list._changes = [];
|
||||
}
|
||||
});
|
||||
return $.when.apply($, defs).then(function () {
|
||||
return result;
|
||||
});
|
||||
});
|
||||
if (shouldLoad) {
|
||||
var def = self._readUngroupedList(list).then(function () {
|
||||
list._changes = list.data;
|
||||
});
|
||||
defs.push(def);
|
||||
}
|
||||
} else if (field.type === 'date') {
|
||||
// process data: convert into a moment instance
|
||||
record._changes[name] = fieldUtils.parse.date(val);
|
||||
} else if (field.type === 'datetime') {
|
||||
// process datetime: convert into a moment instance
|
||||
record._changes[name] = fieldUtils.parse.datetime(val);
|
||||
} else {
|
||||
record._changes[name] = val;
|
||||
}
|
||||
});
|
||||
return $.when.apply($, defs);
|
||||
},
|
||||
/**
|
||||
* When an operation is applied to a x2many field, the field widgets
|
||||
@@ -1161,7 +1132,7 @@ var BasicModel = AbstractModel.extend({
|
||||
* An onchange spec is necessary as an argument to the /onchange route. It
|
||||
* looks like this: { field: "1", anotherField: "", relation.subField: "1"}
|
||||
*
|
||||
* @see _applyOnChange
|
||||
* @see _performOnChange
|
||||
*
|
||||
* @param {Object} record resource object of type 'record'
|
||||
* @returns {Object} an onchange spec
|
||||
@@ -2215,7 +2186,7 @@ var BasicModel = AbstractModel.extend({
|
||||
}
|
||||
}
|
||||
if (shouldApplyOnchange) {
|
||||
return self._applyOnChange(record, fields_key).then(function () {
|
||||
return self._performOnChange(record, fields_key).then(function () {
|
||||
if (record._warning) {
|
||||
return $.Deferred().reject();
|
||||
}
|
||||
@@ -2276,6 +2247,54 @@ var BasicModel = AbstractModel.extend({
|
||||
}
|
||||
});
|
||||
},
|
||||
/**
|
||||
* This method is quite important: it is supposed to perform the /onchange
|
||||
* rpc and apply the result.
|
||||
*
|
||||
* @param {Object} record
|
||||
* @param {string[]} fields changed fields
|
||||
* @returns {Deferred} The returned deferred can fail, in which case the
|
||||
* fail value will be the warning message received from the server
|
||||
*/
|
||||
_performOnChange: function (record, fields) {
|
||||
var self = this;
|
||||
var onchange_spec = this._buildOnchangeSpecs(record);
|
||||
var idList = record.data.id ? [record.data.id] : [];
|
||||
var options = {};
|
||||
if (fields.length === 1) {
|
||||
fields = fields[0];
|
||||
// if only one field changed, add its context to the RPC context
|
||||
options.fieldName = fields;
|
||||
}
|
||||
var context = this._getContext(record, options);
|
||||
var currentData = this._generateOnChangeData(record);
|
||||
|
||||
return self._rpc({
|
||||
model: record.model,
|
||||
method: 'onchange',
|
||||
args: [idList, currentData, fields, onchange_spec, context],
|
||||
})
|
||||
.then(function (result) {
|
||||
if (!record._changes) {
|
||||
// if the _changes key does not exist anymore, it means that
|
||||
// it was removed by discarding the changes after the rpc
|
||||
// to onchange. So, in that case, the proper response is to
|
||||
// ignore the onchange.
|
||||
return;
|
||||
}
|
||||
if (result.warning) {
|
||||
self.trigger_up('warning', {
|
||||
message: result.warning.message,
|
||||
title: result.warning.title,
|
||||
type: 'dialog',
|
||||
});
|
||||
record._warning = true;
|
||||
}
|
||||
return self._applyOnChange(result.value, record).then(function () {
|
||||
return result;
|
||||
});
|
||||
});
|
||||
},
|
||||
/**
|
||||
* Once a record is created and some data has been fetched, we need to do
|
||||
* quite a lot of computations to determine what needs to be fetched. This
|
||||
|
||||
@@ -2023,14 +2023,103 @@ QUnit.module('Views', {
|
||||
form.destroy();
|
||||
});
|
||||
|
||||
QUnit.test('properly apply onchange on one2many fields', function (assert) {
|
||||
assert.expect(5);
|
||||
|
||||
this.data.partner.records[0].p = [4];
|
||||
this.data.partner.onchanges = {
|
||||
foo: function (obj) {
|
||||
obj.p = [
|
||||
[5],
|
||||
[1, 4, {display_name: "updated record"}],
|
||||
[0, null, {display_name: "created record"}],
|
||||
];
|
||||
},
|
||||
};
|
||||
var form = createView({
|
||||
View: FormView,
|
||||
model: 'partner',
|
||||
data: this.data,
|
||||
arch: '<form string="Partners">' +
|
||||
'<group><field name="foo"/></group>' +
|
||||
'<field name="p">' +
|
||||
'<tree>' +
|
||||
'<field name="display_name"/>' +
|
||||
'</tree>' +
|
||||
'</field>' +
|
||||
'</form>',
|
||||
res_id: 1,
|
||||
});
|
||||
|
||||
assert.strictEqual(form.$('.o_form_field_one2many .o_data_row').length, 1,
|
||||
"there should be one one2many record linked at first");
|
||||
assert.strictEqual(form.$('.o_form_field_one2many .o_data_row td:first').text(), 'aaa',
|
||||
"the 'display_name' of the one2many record should be correct");
|
||||
|
||||
// switch to edit mode
|
||||
form.$buttons.find('.o_form_button_edit').click();
|
||||
form.$('.o_form_input').val('let us trigger an onchange').trigger('input');
|
||||
var $o2m = form.$('.o_form_field_one2many');
|
||||
assert.strictEqual($o2m.find('.o_data_row').length, 2,
|
||||
"there should be two linked record");
|
||||
assert.strictEqual($o2m.find('.o_data_row:first td:first').text(), 'updated record',
|
||||
"the 'display_name' of the first one2many record should have been updated");
|
||||
assert.strictEqual($o2m.find('.o_data_row:nth(1) td:first').text(), 'created record',
|
||||
"the 'display_name' of the second one2many record should be correct");
|
||||
|
||||
form.destroy();
|
||||
});
|
||||
|
||||
QUnit.test('update many2many value in one2many after onchange', function (assert) {
|
||||
assert.expect(2);
|
||||
|
||||
this.data.partner.records[1].p = [4];
|
||||
this.data.partner.onchanges = {
|
||||
foo: function (obj) {
|
||||
obj.p = [
|
||||
[5],
|
||||
[1, 4, {
|
||||
display_name: "gold",
|
||||
timmy: [5]
|
||||
}],
|
||||
];
|
||||
},
|
||||
};
|
||||
var form = createView({
|
||||
View: FormView,
|
||||
model: 'partner',
|
||||
data: this.data,
|
||||
arch: '<form>' +
|
||||
'<field name="foo"/>' +
|
||||
'<field name="p">' +
|
||||
'<tree editable="top">' +
|
||||
'<field name="display_name" attrs="{\'readonly\': [(\'timmy\', \'=\', false)]}"/>' +
|
||||
'<field name="timmy"/>' +
|
||||
'</tree>' +
|
||||
'</field>' +
|
||||
'</form>',
|
||||
res_id: 2,
|
||||
});
|
||||
assert.strictEqual($('div[name="p"] .o_data_row td').text().trim(), "aaaNo records",
|
||||
"should have proper initial content");
|
||||
form.$buttons.find('.o_form_button_edit').click();
|
||||
|
||||
form.$('input').val("tralala").trigger('input');
|
||||
|
||||
assert.strictEqual($('div[name="p"] .o_data_row td').text().trim(), "goldNo records",
|
||||
"should have proper initial content");
|
||||
form.destroy();
|
||||
});
|
||||
|
||||
QUnit.test('properly apply onchange on many2many fields', function (assert) {
|
||||
assert.expect(3);
|
||||
assert.expect(14);
|
||||
|
||||
this.data.partner.onchanges = {
|
||||
foo: function (obj) {
|
||||
obj.timmy = [
|
||||
[5],
|
||||
[1, 12, {display_name: "gold"}],
|
||||
[4, 12],
|
||||
[4, 14],
|
||||
];
|
||||
},
|
||||
};
|
||||
@@ -2046,6 +2135,19 @@ QUnit.module('Views', {
|
||||
'</tree>' +
|
||||
'</field>' +
|
||||
'</form>',
|
||||
mockRPC: function (route, args) {
|
||||
assert.step(args.method);
|
||||
if (args.method === 'read' && args.model === 'partner_type') {
|
||||
assert.deepEqual(args.args[0], [12, 14],
|
||||
"should read both m2m with one RPC");
|
||||
}
|
||||
if (args.method === 'write') {
|
||||
assert.deepEqual(args.args[1].timmy, [[6, false, [12, 14]]],
|
||||
"should correctly save the changed m2m values");
|
||||
|
||||
}
|
||||
return this._super.apply(this, arguments);
|
||||
},
|
||||
res_id: 2,
|
||||
});
|
||||
|
||||
@@ -2055,10 +2157,17 @@ QUnit.module('Views', {
|
||||
// switch to edit mode
|
||||
form.$buttons.find('.o_form_button_edit').click();
|
||||
form.$('.o_form_input').val('let us trigger an onchange').trigger('input');
|
||||
assert.strictEqual(form.$('.o_form_field_many2many .o_data_row').length, 1,
|
||||
"there should be one linked record");
|
||||
assert.strictEqual(form.$('.o_form_field_many2many .o_data_row td:first').text(), 'gold',
|
||||
"the 'display_name' of the many2many record should be correctly displayed");
|
||||
var $m2m = form.$('.o_form_field_many2many');
|
||||
assert.strictEqual($m2m.find('.o_data_row').length, 2,
|
||||
"there should be two linked records");
|
||||
assert.strictEqual($m2m.find('.o_data_row:first td:first').text(), 'gold',
|
||||
"the 'display_name' of the first m2m record should be correctly displayed");
|
||||
assert.strictEqual($m2m.find('.o_data_row:nth(1) td:first').text(), 'silver',
|
||||
"the 'display_name' of the second m2m record should be correctly displayed");
|
||||
|
||||
form.$buttons.find('.o_form_button_save').click();
|
||||
|
||||
assert.verifySteps(['read', 'onchange', 'read', 'write', 'read', 'read']);
|
||||
|
||||
form.destroy();
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user