From 3b3f68030d29e74d3c2486921ed1b8868b1ba9c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Thu, 7 Dec 2017 15:33:52 +0100 Subject: [FIX] web: send proper context when fetching m2o namegets When fetching batched namegets (typically after a default_get), we need to evaluate the context that we send to the server. This may be important for models with a custom name_get method. (note: overriding namegets should be avoided if possible) With the new views refactoring, this was not done properly and the 'view' context was sent instead. This is not really critical, the proof is that it took a few months before getting some complaints, but this is definitely a bug. To reproduce, simply add a context attribute to a m2o field in a form view, with a non empty value in a default get, and look at the context sent to the server. --- .../web/static/src/js/views/basic/basic_model.js | 26 ++++++++++++---------- 1 file changed, 14 insertions(+), 12 deletions(-) (limited to 'addons/web/static/src/js/views/basic') 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 e8f4a3993fa..416bc33f90f 100644 --- a/addons/web/static/src/js/views/basic/basic_model.js +++ b/addons/web/static/src/js/views/basic/basic_model.js @@ -1724,22 +1724,21 @@ var BasicModel = AbstractModel.extend({ /** * Fetch all name_gets for the many2ones in a group * - * @param {Object} group a valid resource object + * @param {Object[]} groups a list of object with context and record sub keys * @returns {Deferred} */ - _fetchMany2OneGroup: function (group) { - var ids = _.uniq(_.pluck(group, 'res_id')); - + _fetchMany2OneGroup: function (groups) { + var ids = _.uniq(_.pluck(_.pluck(groups, 'record'), 'res_id')); return this._rpc({ - model: group[0].model, + model: groups[0].record.model, method: 'name_get', args: [ids], - context: group[0].context + context: groups[0].context }) .then(function (name_gets) { - _.each(group, function (record) { - var nameGet = _.find(name_gets, function (n) { return n[0] === record.res_id;}); - record.data.display_name = nameGet[1]; + _.each(groups, function (obj) { + var nameGet = _.find(name_gets, function (n) { return n[0] === obj.record.res_id;}); + obj.record.data.display_name = nameGet[1]; }); }); }, @@ -2000,14 +1999,17 @@ var BasicModel = AbstractModel.extend({ if (!relatedRecord) { return; } - toBeFetched.push(relatedRecord); + toBeFetched.push({ + context: record.getContext({fieldName: name, viewType: record.viewType}), + record: relatedRecord + }); } }); // group them by model and context. Using the context as key is // necessary to make sure the correct context is used for the rpc; - var groups = _.groupBy(toBeFetched, function (rec) { - return [rec.model, JSON.stringify(rec.context)].join(); + var groups = _.groupBy(toBeFetched, function (elem) { + return [elem.record.model, JSON.stringify(elem.context)].join(); }); return $.when.apply($, _.map(groups, this._fetchMany2OneGroup.bind(this))); -- cgit v1.2.3 From 6c024d1ef0c9e828b09eea17f55748784232a0c9 Mon Sep 17 00:00:00 2001 From: "Lucas Perais (lpe)" Date: Thu, 7 Dec 2017 09:10:39 +0100 Subject: [FIX] web: give context to fetch x2m batched Before this commit M2Mtags widget did not translate. This was because no context was given to the rpc that fetched the fields Hence, the lang was not passed. After this commit, M2Mtags translate well OPW 784396 --- addons/web/static/src/js/views/basic/basic_model.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'addons/web/static/src/js/views/basic') 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 416bc33f90f..1562bffdc42 100644 --- a/addons/web/static/src/js/views/basic/basic_model.js +++ b/addons/web/static/src/js/views/basic/basic_model.js @@ -1480,7 +1480,8 @@ var BasicModel = AbstractModel.extend({ def = this._rpc({ model: list.model, method: 'read', - args: [_.pluck(data, 'id'), fieldNames] + args: [_.pluck(data, 'id'), fieldNames], + context: record.context, }).then(function (records) { _.each(records, function (record) { list_records[record.id].data = record; @@ -2399,7 +2400,7 @@ var BasicModel = AbstractModel.extend({ model: field.relation, method: 'read', args: [ids, fieldNames], - context: {}, // FIXME + context: list.getContext() || {}, }); } else { def = $.when(_.map(ids, function (id) { -- cgit v1.2.3 From dbdd742d2046f208c0057fdc60b359e5c88814ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Fri, 15 Dec 2017 10:06:03 +0100 Subject: [FIX] web: better mode handling, prevent useless work in some cases Before this commit, the way mode was handled was not optimal. The end result was correct (the widget were properly created/displayed in edit/readonly as required), but useless work was done. In particular, if we have list/x2manys with widgets and modifiers, it could happen that widgets were created in edit mode, then destroyed and recreated in readonly mode. This could happen in some important views, for example, any view with a field many2many_tags with a readonly modifiers (pretty much all views handling taxes) In this commit, we add a 'editable' attribute to list renderer. This is necessary, because the list renderer is not actually editable when it is in a x2many in a form view which is in readonly mode. Also, we need to manage the distinction between the fact that a list view is editable, and the fact that some widgets are created in edit and other in readonly mode, depending on modifiers, and on which line is being edited. We also add a benchmark (courtesy of CHM) to be able to measure the performance impact of edit/readonly/onchange in a larger form view. Before this commit, running this form benchmark on my machine gave a number of 0.4 op/sec, and after, it increases to 1.26 op/sec, so a pretty large improvement. --- .../static/src/js/views/basic/basic_renderer.js | 47 ++++++++++++---------- 1 file changed, 26 insertions(+), 21 deletions(-) (limited to 'addons/web/static/src/js/views/basic') diff --git a/addons/web/static/src/js/views/basic/basic_renderer.js b/addons/web/static/src/js/views/basic/basic_renderer.js index 21fabbc2274..64b90c33022 100644 --- a/addons/web/static/src/js/views/basic/basic_renderer.js +++ b/addons/web/static/src/js/views/basic/basic_renderer.js @@ -251,13 +251,12 @@ var BasicRenderer = AbstractRenderer.extend({ function _apply(element) { // If the view is in edit mode and that a widget have to switch // its "readonly" state, we have to re-render it completely - if ('readonly' in modifiers - && self.mode === "edit" - && element.widget - && (element.widget.mode === 'readonly') !== modifiers.readonly) - { - self._rerenderFieldWidget(element.widget, record); - return; // Rerendering already applied the modifiers, no need to go further + if ('readonly' in modifiers && element.widget) { + var mode = modifiers.readonly ? 'readonly' : modifiersData.baseMode; + if (mode !== element.widget.mode) { + self._rerenderFieldWidget(element.widget, record, mode); + return; // Rerendering already applied the modifiers, no need to go further + } } // Toggle modifiers CSS classes if necessary @@ -429,6 +428,12 @@ var BasicRenderer = AbstractRenderer.extend({ this.allModifiersData.push(modifiersData); } } + // we register here the base mode of the node. This is a field widget + // specific settings which represents the generic mode for the widget, + // regardless of its modifiers. The interesting case is the list view: + // all widgets are supposed to be in the baseMode 'readonly', except the + // ones that are in the line that is currently being edited. + modifiersData.baseMode = (options && options.mode) || this.mode; // Evaluate if necessary if (!modifiersData.evaluatedModifiers[record.id]) { @@ -454,7 +459,7 @@ var BasicRenderer = AbstractRenderer.extend({ } modifiersData.elementsByRecord[record.id].push(newElement); - this._applyModifiers(modifiersData, record, newElement); + this._applyModifiers(modifiersData, record, newElement, options); } return modifiersData.evaluatedModifiers[record.id]; @@ -490,22 +495,20 @@ var BasicRenderer = AbstractRenderer.extend({ * @param {Object} node * @param {Object} record * @param {Object} [options] - * @param {Object} [modifiersOptions] * @returns {AbstractField} */ - _renderFieldWidget: function (node, record, options, modifiersOptions) { + _renderFieldWidget: function (node, record, options) { var fieldName = node.attrs.name; - // Register the node-associated modifiers - var modifiers = this._registerModifiers(node, record); - + var mode = options && options.mode || this.mode; + var modifiers = this._registerModifiers(node, record, null, options); // Initialize and register the widget // Readonly status is known as the modifiers have just been registered var Widget = record.fieldsInfo[this.viewType][fieldName].Widget; - var widget = new Widget(this, fieldName, record, _.extend({ - mode: modifiers.readonly ? 'readonly' : this.mode, + var widget = new Widget(this, fieldName, record, { + mode: modifiers.readonly ? 'readonly' : mode, viewType: this.viewType, - }, options || {})); + }); // Register the widget so that it can easily be found again if (this.allFieldWidgets[record.id] === undefined) { @@ -526,15 +529,16 @@ var BasicRenderer = AbstractRenderer.extend({ // associated to new widget) var self = this; def.then(function () { - self._registerModifiers(node, record, widget, _.extend({ + self._registerModifiers(node, record, widget, { callback: function (element, modifiers, record) { element.$el.toggleClass('o_field_empty', !!( record.data.id - && (modifiers.readonly || self.mode === 'readonly') + && (modifiers.readonly || mode === 'readonly') && !element.widget.isSet() )); }, - }, modifiersOptions || {})); + mode: mode + }); self._postProcessField(widget, node); }); @@ -602,11 +606,12 @@ var BasicRenderer = AbstractRenderer.extend({ * @private * @param {Widget} widget * @param {Object} record + * @param {string} mode either 'readonly' or 'edit' * @returns {AbstractField} */ - _rerenderFieldWidget: function (widget, record) { + _rerenderFieldWidget: function (widget, record, mode) { // Render the new field widget - var newWidget = this._renderFieldWidget(widget.__node, record); + var newWidget = this._renderFieldWidget(widget.__node, record, {mode: mode}); widget.$el.replaceWith(newWidget.$el); // Destroy the old widget and position the new one at the old one's -- cgit v1.2.3