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

[FIX] web,mail: only send fields in view for onchanges

Browse Source 
The field 'display_name' is automatically added to the list of
fields to fetch when fetching a record. Before this rev., its
value was also sent alongside the fields in the view for onchange
RPCs. It shouldn't: only the fields in the view should be sent.

It produced a weird bug in the res.partner form view: open a
contact (company), add a sub-contact, save -> the display_name of
the contact was then '' (and 'Contacts' was displayed in the
breadcrumbs instead of the display_name of the opened contact).
This is because 'display_name' was sent to the onchange, which
returned '' as value (because of another bug python side). So as
it's value changed, when saving, the new display_name was sent to
the server (even if this field was actually readonly).

Before this rev., we also always fetched 'display_name' when
fetching list of records (read or search_read). This wasn't a bug
per se, but it might be a computed field, and it isn't necessary,
so we stop fetching it in that case if it isn't in the view (as
it was done in saas-15, before the new views).
This commit is contained in:
Aaron Bohy
2017-08-29 10:38:36 +02:00
parent f24c83efcc
commit d93726f9ac
5 changed files with 110 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/mail/static/tests/chatter_tests.js
+7 -7
View File
@@ -530,7 +530,7 @@ QUnit.test('form activity widget: schedule next activity', function (assert) {
return $.when();
}
if (args.method === 'read' && checkReadArgs) {
assert.deepEqual(args.args[1], ['activity_ids', 'message_ids'],
assert.deepEqual(args.args[1], ['activity_ids', 'message_ids', 'display_name'],
"should only read the mail fields");
}
return this._super.apply(this, arguments);
@@ -686,11 +686,11 @@ QUnit.test('form activity widget: mark as done and remove', function (assert) {
if (nbReads === 1) { // first read
assert.strictEqual(args.args[1].length, 4, 'should read all fiels the first time');
} else if (nbReads === 2) { // second read: after the unlink
assert.ok(_.isEqual(args.args[1], ['activity_ids']),
'should only read the activities after an unlink');
assert.ok(_.isEqual(args.args[1], ['activity_ids', 'display_name']),
'should only read the activities (+ display_name) after an unlink');
} else { // third read: after marking an activity done
assert.ok(_.isEqual(args.args[1], ['activity_ids', 'message_ids']),
'should read the activities and messages after marking an activity done');
assert.ok(_.isEqual(args.args[1], ['activity_ids', 'message_ids', 'display_name']),
'should read the activities and messages (+ display_name) after marking an activity done');
}
}
return this._super.apply(this, arguments);
@@ -809,8 +809,8 @@ QUnit.test('followers widget: follow/unfollow, edit subtypes', function (assert)
assert.strictEqual(args.args[1].length, 3,
'should read "foo", "message_follower_ids" and "display_name"');
} else { // three next reads: only read 'message_follower_ids' field
assert.ok(_.isEqual(args.args[1], ['message_follower_ids']),
'should only read "message_follower_ids"');
assert.deepEqual(args.args[1], ['message_follower_ids', 'display_name'],
'should only read "message_follower_ids" and "display_name"');
}
}
return this._super.apply(this, arguments);
addons/web/static/src/js/services/data_manager.js
-5
View File
@@ -398,11 +398,6 @@ return core.Class.extend({
*/
_processFieldsView: function (viewInfo) {
var viewFields = this._processFields(viewInfo.type, viewInfo.arch, viewInfo.fields);
// by default fetch display_name and id
if (!viewInfo.fields.display_name) {
viewInfo.fields.display_name = {type: 'char'};
viewFields.display_name = {};
}
viewInfo.fieldsInfo = {};
viewInfo.fieldsInfo[viewInfo.type] = viewFields;
utils.deepFreeze(viewInfo.fields);
addons/web/static/src/js/views/basic/basic_model.js
+12 -2
View File
@@ -1629,8 +1629,8 @@ var BasicModel = AbstractModel.extend({
*/
_fetchRecord: function (record, options) {
var self = this;
var fieldNames = options && options.fieldNames ||
_.uniq(record.getFieldNames().concat(['display_name']));
var fieldNames = options && options.fieldNames || record.getFieldNames();
fieldNames = _.uniq(fieldNames.concat(['display_name']));
return this._rpc({
model: record.model,
method: 'read',
@@ -2337,6 +2337,16 @@ var BasicModel = AbstractModel.extend({
_generateOnChangeData: function (record) {
var commands = this._generateX2ManyCommands(record, {withReadonly: true});
var data = _.extend(this.get(record.id, {raw: true}).data, commands);
// 'display_name' is automatically added to the list of fields to fetch,
// when fetching a record, even if it doesn't appear in the view. However,
// only the fields in the view must be passed to the onchange RPC, so we
// remove it from the data sent by RPC if it isn't in the view.
var hasDisplayName = _.some(record.fieldsInfo, function (fieldsInfo) {
return 'display_name' in fieldsInfo;
});
if (!hasDisplayName) {
delete data.display_name;
}
// one2many records have a parentID
if (record.parentID) {
addons/web/static/tests/fields/relational_fields_tests.js
+4 -2
View File
@@ -2521,6 +2521,7 @@ QUnit.module('relational_fields', {
arch:'<form string="Partners">' +
'<field name="p">' +
'<kanban create="0">' +
'<field name="display_name"/>' +
'<templates>' +
'<t t-name="kanban-box">' +
'<div class="oe_kanban_global_click">' +
@@ -3499,7 +3500,7 @@ QUnit.module('relational_fields', {
rpcCount++;
if (args.method === 'write') {
assert.deepEqual(args.args[1].p, [[0, false, {
display_name: false, int_field: 123, product_id: 41
int_field: 123, product_id: 41
}]]);
}
return this._super(route, args);
@@ -3736,7 +3737,7 @@ QUnit.module('relational_fields', {
if (args.method === 'default_get') {
var expected = counter === 0 ?
[[4, 2, false]] :
[[4, 2, false], [0, false, {display_name: false, turtle_foo: 'hammer'}]];
[[4, 2, false], [0, false, {turtle_foo: 'hammer'}]];
assert.deepEqual(args.kwargs.context.turtles, expected,
"should have properly evaluated turtles key in context");
counter++;
@@ -5544,6 +5545,7 @@ QUnit.module('relational_fields', {
arch:'<form string="Partners">' +
'<field name="timmy">' +
'<kanban create="0">' +
'<field name="display_name"/>' +
'<templates>' +
'<t t-name="kanban-box">' +
'<div class="oe_kanban_global_click">' +
addons/web/static/tests/views/form_tests.js
+87 -3
View File
@@ -1134,7 +1134,7 @@ QUnit.module('Views', {
mockRPC: function (route, args) {
if (args.method === "onchange") {
assert.deepEqual(args.args[3],
{"foo": "1", "p": "", "p.bar": "", "p.display_name": "", "p.product_id": "", "timmy": "", "timmy.name": ""},
{"foo": "1", "p": "", "p.bar": "", "p.product_id": "", "timmy": "", "timmy.name": ""},
"should send only the fields used in the views");
}
return this._super(route, args);
@@ -1282,7 +1282,6 @@ QUnit.module('Views', {
if (args.method === 'onchange') {
assert.deepEqual(args.args[3], {
p: '',
'p.display_name': '',
'p.foo': '1'
}, "onchangeSpec should be correct (with sub fields)");
}
@@ -2827,6 +2826,92 @@ QUnit.module('Views', {
form.destroy();
});
QUnit.test('display_name not sent for onchanges if not in view', function (assert) {
assert.expect(7);
this.data.partner.records[0].timmy = [12];
this.data.partner.onchanges = {
foo: function () {},
};
this.data.partner_type.onchanges = {
name: function () {},
};
var readInModal = false;
var form = createView({
View: FormView,
model: 'partner',
data: this.data,
arch: '<form string="Partners">' +
'<group>' +
'<field name="foo"/>' +
'<field name="timmy">' +
'<tree>' +
'<field name="name"/>' +
'</tree>' +
'<form>' +
'<field name="name"/>' +
'<field name="color"/>' +
'</form>' +
'</field>' +
'</group>' +
'</form>',
mockRPC: function (route, args) {
if (args.method === 'read' && args.model === 'partner') {
assert.deepEqual(args.args[1], ['foo', 'timmy', 'display_name'],
"should read display_name even if not in the view");
}
if (args.method === 'read' && args.model === 'partner_type') {
if (!readInModal) {
assert.deepEqual(args.args[1], ['name'],
"should not read display_name for records in the list");
} else {
assert.deepEqual(args.args[1], ['name', 'color', 'display_name'],
"should read display_name when opening the subrecord");
}
}
if (args.method === 'onchange' && args.model === 'partner') {
assert.deepEqual(args.args[1], {
id: 1,
foo: 'coucou',
timmy: [[6, false, [12]]],
}, "should only send the value of fields in the view (+ id)");
assert.deepEqual(args.args[3], {
foo: '1',
timmy: '',
'timmy.name': '1',
'timmy.color': '',
}, "only the fields in the view should be in the onchange spec");
}
if (args.method === 'onchange' && args.model === 'partner_type') {
assert.deepEqual(args.args[1], {
id: 12,
name: 'new name',
color: 2,
}, "should only send the value of fields in the view (+ id)");
assert.deepEqual(args.args[3], {
name: '1',
color: '',
}, "only the fields in the view should be in the onchange spec");
}
return this._super.apply(this, arguments);
},
res_id: 1,
viewOptions: {
mode: 'edit',
},
});
// trigger the onchange
form.$('.o_field_widget[name=foo]').val("coucou").trigger('input');
// open a subrecord and trigger an onchange
readInModal = true;
form.$('.o_data_row .o_data_cell:first').click();
$('.modal .o_field_widget[name=name]').val("new name").trigger('input');
form.destroy();
});
QUnit.test('onchanges on date(time) fields', function (assert) {
assert.expect(6);
@@ -3725,7 +3810,6 @@ QUnit.module('Views', {
if (args.method === 'create') {
var command = args.args[0].p;
assert.deepEqual(command, [[0, false, {
display_name: false,
timmy: [[6, false, [12]]],
}]], "the default partner_type_id should be equal to 12");
}