From 3b7fb9934d795ff6971727089e6bbe0a77c06476 Mon Sep 17 00:00:00 2001 From: Aaron Bohy Date: Thu, 10 Aug 2017 17:24:43 +0200 Subject: [FIX] web: BasicModel: keep onchanges on unknown fields Let's assume the following situation: we have a one2many field displayed as a list in a form view. The list isn't editable, so when a subrecord is clicked, it is opened in a form view in a dialog. In this form view, there is a relational field X that is not in the list. In the main form view, there is a field with an onchange. When its value changes, the onchange returns a new value for the one2many: a command 5 (delete all), and a command 0 (create) with a dict of values containing a value for X. However, as X is unknown as long as no subrecord has been open in the dialog, the value for X can't be processed (we don't know the type of X, so we don't know how to process the value). Before this rev., when this happened, we discarded the value of X. This caused an issue for example in the Quotation form view (with website_quote installed): - create a new Quotation - set PartnerShip Contract as quotation template -> the onchange returns two 'create' commands for the order_line one2many - click on one of the two subrecords in the one2many -> error. This rev. ensures to keep changes on unknown fields. We keep them unprocessed, and once the form view is opened, we process and apply them. --- .../web/static/src/js/views/basic/basic_model.js | 44 ++++++++++++++++++++-- 1 file changed, 40 insertions(+), 4 deletions(-) (limited to 'addons/web/static/src/js/views/basic/basic_model.js') 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 8755805f993..67688c83fdc 100644 --- a/addons/web/static/src/js/views/basic/basic_model.js +++ b/addons/web/static/src/js/views/basic/basic_model.js @@ -185,6 +185,29 @@ var BasicModel = AbstractModel.extend({ return id; }); }, + /** + * Onchange RPCs may return values for fields that are not in the current + * view. Those fields might even be unknown when the onchange returns (e.g. + * in x2manys, we only know the fields that are used in the inner view, but + * not those used in the potential form view opened in a dialog when a sub- + * record is clicked). When this happens, we can't infer their type, so the + * given value can't be processed. It is instead stored in the '_rawChanges' + * key of the record, without any processing. Later on, if this record is + * displayed in another view (e.g. the user clicked on it in the x2many + * list, and the record opens in a dialog), those changes that were left + * behind must be applied. This function applies changes stored in + * '_rawChanges' for a given viewType. + * + * @param {string} id local resource id of a record + * @param {string} viewType the current viewType + * @returns {Deferred -> string} resolves to the id of the record + */ + applyRawChanges: function (recordID, viewType) { + var record = this.localData[recordID]; + return this._applyOnChange(record._rawChanges, record, viewType).then(function () { + return record.id; + }); + }, /** * Delete a list of records, then, if the records have a parent, reload it. * @@ -1093,17 +1116,29 @@ var BasicModel = AbstractModel.extend({ * @param {Object} values the result of the onchange RPC (a mapping of * fieldnames to their value) * @param {Object} record + * @param {string} [viewType] current viewType. If not set, we will assume + * main viewType from the record * @returns {Deferred} */ - _applyOnChange: function (values, record) { + _applyOnChange: function (values, record, viewType) { var self = this; var defs = []; var rec; + viewType = viewType || record.viewType; record._changes = record._changes || {}; _.each(values, function (val, name) { var field = record.fields[name]; if (!field) { - return; // ignore changes of unknown fields + // this field is unknown so we can't process it for now (it is not + // in the current view anyway, otherwise it wouldn't be unknown. + // we store its value without processing it, so that if we later + // on switch to another view in which this field is displayed, + // we could process it as we would know its type then. + // use case: an onchange sends a create command for a one2many, + // in the dict of values, there is a value for a field that is + // not in the one2many list, but that is in the one2many form. + record._rawChanges[name] = val; + return; } if (field.type === 'many2one' ) { @@ -1132,7 +1167,7 @@ var BasicModel = AbstractModel.extend({ if (listId) { list = self.localData[listId]; } else { - var fieldInfo = record.fieldsInfo[record.viewType][name]; + var fieldInfo = record.fieldsInfo[viewType][name]; if (!fieldInfo) { return; //ignore changes of x2many not in view } @@ -1142,7 +1177,7 @@ var BasicModel = AbstractModel.extend({ parentID: record.id, static: true, type: 'list', - viewtype: fieldInfo.viewType, + viewType: fieldInfo.viewType, }); } record._changes[name] = list.id; @@ -2712,6 +2747,7 @@ var BasicModel = AbstractModel.extend({ _cache: type === 'list' ? {} : undefined, _changes: null, _domains: {}, + _rawChanges: {}, aggregateValues: params.aggregateValues || {}, context: params.context || {}, count: params.count || res_ids.length, -- cgit v1.2.3 From 870aa81ad9ef26b88bff8190a71e8c805fe33beb Mon Sep 17 00:00:00 2001 From: Aaron Bohy Date: Fri, 11 Aug 2017 08:46:11 +0200 Subject: [FIX] web: BasicModel: default_get: handle false value for o2m The value for one2many fields returned by default_get may be a list of commands, or directly a list of ids. These cases were correctly handled. However, it seems that the value can also be false, and when this happened, it crashed. For instance, go to Sales > Leads (must be activated in the Settings) > in the list view, check several leads > in 'Action', click on 'Convert to Opportunities' > crash. --- addons/web/static/src/js/views/basic/basic_model.js | 1 + 1 file changed, 1 insertion(+) (limited to 'addons/web/static/src/js/views/basic/basic_model.js') 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 67688c83fdc..8bb46ea68aa 100644 --- a/addons/web/static/src/js/views/basic/basic_model.js +++ b/addons/web/static/src/js/views/basic/basic_model.js @@ -2895,6 +2895,7 @@ var BasicModel = AbstractModel.extend({ x2manyList._changes = []; var many2ones = {}; var r; + result[name] = result[name] || []; // handle false value var isCommandList = result[name].length && _.isArray(result[name][0]); if (!isCommandList) { result[name] = [[6, false, result[name]]]; -- cgit v1.2.3 From 5fce37486a707c19ba4c751b234f5620ed066aa7 Mon Sep 17 00:00:00 2001 From: Aaron Bohy Date: Fri, 11 Aug 2017 10:23:55 +0200 Subject: [FIX] web: BasicModel: default_get: false m2o inside o2m Sometimes, a default_get returns a create command for a one2many, with a dict of values containing value 'false' for a many2one. This is strictly equivalent to the case where the many2one field isn't in the dict of values, but we have to handle both cases, and before this rev., we only handled the latter one. This happens for example in Sales > Leads (must be activated in the Settings) > in the list view, check several leads > in 'Action', click on 'Forward to Partner' > crash. --- addons/web/static/src/js/views/basic/basic_model.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'addons/web/static/src/js/views/basic/basic_model.js') 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 8bb46ea68aa..72a89a23cc2 100644 --- a/addons/web/static/src/js/views/basic/basic_model.js +++ b/addons/web/static/src/js/views/basic/basic_model.js @@ -2922,7 +2922,7 @@ var BasicModel = AbstractModel.extend({ r._changes = _.defaults(value[2], r.data); for (var name in r._changes) { - if (r._changes[name] === null) { + if (!r._changes[name]) { continue; } var isFieldInView = name in r.fields; -- cgit v1.2.3 From b5b8e66b900e78dc3228946e15d517f91a214703 Mon Sep 17 00:00:00 2001 From: Aaron Bohy Date: Mon, 14 Aug 2017 12:38:14 +0200 Subject: [FIX] web: grouped views: update pager The problem occured when opening a record from a grouped list or kanban view. In the opened form view, the pager was sometimes incorrect. This was because the list of ids wasn't correctly updated when groups were opened/folded. --- .../web/static/src/js/views/basic/basic_model.js | 27 +++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) (limited to 'addons/web/static/src/js/views/basic/basic_model.js') 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 72a89a23cc2..e8409487abc 100644 --- a/addons/web/static/src/js/views/basic/basic_model.js +++ b/addons/web/static/src/js/views/basic/basic_model.js @@ -919,18 +919,23 @@ var BasicModel = AbstractModel.extend({ * @returns {Deferred -> string} resolves to the group id */ toggleGroup: function (groupId) { + var self = this; var group = this.localData[groupId]; if (group.isOpen) { group.isOpen = false; group.data = []; + group.res_ids = []; group.offset = 0; + this._updateParentResIDs(group); return $.when(groupId); } if (!group.isOpen) { group.isOpen = true; var def; if (group.count > 0) { - def = this._load(group); + def = this._load(group).then(function () { + self._updateParentResIDs(group); + }); } return $.when(def).then(function () { return groupId; @@ -3400,6 +3405,26 @@ var BasicModel = AbstractModel.extend({ }); } }, + /** + * Updates the res_ids of the parent of a given element of type list. + * + * After some operations (e.g. loading more records, folding/unfolding a + * group), the res_ids list of an element may be updated. When this happens, + * the res_ids of its ancestors need to be updated as well. This is the + * purpose of this function. + * + * @param {Object} element + */ + _updateParentResIDs: function (element) { + var self = this; + if (element.parentID) { + var parent = this.localData[element.parentID]; + parent.res_ids = _.flatten(_.map(parent.data, function (dataPointID) { + return self.localData[dataPointID].res_ids; + })); + this._updateParentResIDs(parent); + } + }, /** * Helper method. Recursively traverses the data, starting from the element * record (or list), then following all relations. This is useful when one -- cgit v1.2.3 From ab2e71d9fc2486f248fd6518237ac4b54cf45bd5 Mon Sep 17 00:00:00 2001 From: Aaron Bohy Date: Mon, 14 Aug 2017 15:58:48 +0200 Subject: [FIX] web: Kanban: notify environment of ids changes In grouped Kanban views, the list of res_ids of the displayed records may change for several reasons: 1) 'Load more' is clicked for a column 2) a record is moved from a column to another 3) a record is moved inside a column (resequence) 4) a column is moved 5) a column is (un)archived 6) a column is (un)folded 7) a record is created/destroyed directly from the Kanban. When this happens, the environment must be notified of the new list of res_ids (the order matters). Before this rev., this wasn't the case. As a consequence, when a record was opened in a Form view after one of the 5 operations above, the pager could display unexpected values. Note that before the new views, cases 2, 3 and 4 weren't handled, and case 7 already worked before this commit. --- addons/web/static/src/js/views/basic/basic_model.js | 1 + 1 file changed, 1 insertion(+) (limited to 'addons/web/static/src/js/views/basic/basic_model.js') 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 e8409487abc..d55109bde01 100644 --- a/addons/web/static/src/js/views/basic/basic_model.js +++ b/addons/web/static/src/js/views/basic/basic_model.js @@ -3360,6 +3360,7 @@ var BasicModel = AbstractModel.extend({ list.data = data; list.res_ids = ids; } + self._updateParentResIDs(list); return list; }); }, -- cgit v1.2.3 From 68d0ca03da992bb741cb7692e69fd11cbe15b29a Mon Sep 17 00:00:00 2001 From: Martin Geubelle Date: Wed, 16 Aug 2017 15:51:52 +0200 Subject: [FIX] web: edit (list editable) one2manys inside one2manys Since rev. https://github.com/odoo/odoo/commit/ebd17217c4e7e9ff06d97028de64fb2fc44d6266 the attribute `_editionViewType` has been introduced to use the modifiers from the correct one2many view type. This attribute wasn't correctly populated if multiple one2manys were imbricated. The first view type (form in the test case) was set for the second one2many even though the view type was list. This was due to the `visitChildren` function use that browses through all levels (and not only the first one). To reproduce the bug before this rev. you could try to create a new survey, add a question (one2many) then select multiple choice and add an answer (one2many editable). --- addons/web/static/src/js/views/basic/basic_model.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) (limited to 'addons/web/static/src/js/views/basic/basic_model.js') 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 d55109bde01..819eb978b48 100644 --- a/addons/web/static/src/js/views/basic/basic_model.js +++ b/addons/web/static/src/js/views/basic/basic_model.js @@ -777,14 +777,15 @@ var BasicModel = AbstractModel.extend({ if (newValue instanceof Array) { rec._savePoint = newValue.slice(0); } else { - // save the viewType of edition, so that the correct readonly modifiers - // can be evaluated when the record will be saved - for (var fieldName in (rec._changes || {})) { - rec._editionViewType[fieldName] = options.viewType; - } rec._savePoint = _.extend({}, newValue); } }); + + // save the viewType of edition, so that the correct readonly modifiers + // can be evaluated when the record will be saved + _.each((record._changes || {}), function (value, fieldName) { + record._editionViewType[fieldName] = options.viewType; + }); } var shouldReload = 'reload' in options ? options.reload : true; var method = self.isNew(record_id) ? 'create' : 'write'; -- cgit v1.2.3 From 4efa4e9d9420d0283c8d7b9683d13d3cd0907400 Mon Sep 17 00:00:00 2001 From: Martin Geubelle Date: Thu, 17 Aug 2017 18:37:25 +0200 Subject: [FIX] web: BasicModel: default_get: correctly fetch m2o inside o2m In general (e.g. read, search_read), values for many2one fields are pairs [id, display_name]. The default_get is an exception, it only returns the id, so we need to manually perform a name_get afterwards. In the case of list views, those name_get RPCs are batched. Before this rev., the code in charge of it didn't handle the case where the many2one value was false, so it crashed when it happened. This rev. deals with this use case and a test case has been added. --- addons/web/static/src/js/views/basic/basic_model.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'addons/web/static/src/js/views/basic/basic_model.js') 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 819eb978b48..3bfab5e3eb6 100644 --- a/addons/web/static/src/js/views/basic/basic_model.js +++ b/addons/web/static/src/js/views/basic/basic_model.js @@ -1576,15 +1576,17 @@ var BasicModel = AbstractModel.extend({ var self = this; var model; var records = []; + var ids = []; list = this._applyX2ManyOperations(list); - var ids = _.map(list.data, function (localId) { + _.each(list.data, function (localId) { var record = self.localData[localId]; var data = record._changes || record.data; var many2oneId = data[fieldName]; + if (!many2oneId) { return; } var many2oneRecord = self.localData[many2oneId]; records.push(many2oneRecord); + ids.push(many2oneRecord.res_id); model = many2oneRecord.model; - return many2oneRecord.res_id; }); return this._rpc({ model: model, -- cgit v1.2.3 From dbab6f74171c895ba8196653b2755361ac34a823 Mon Sep 17 00:00:00 2001 From: Martin Geubelle Date: Fri, 18 Aug 2017 15:35:17 +0200 Subject: [FIX] web: advanced use of widget `reference` The reference widget is also meant to be used on a char field, not only on a reference field. The rev. https://github.com/odoo/odoo/commit/a4e30a1b43d0b9acd76508d9e6c466cd1dbdffab removed the use of `field_utils.format.many2one` and replaced it by `_formatValue` to support this use case. The replacement wasn't complete as some occurences was remaining (in reset), which led to traceback when the widget was reset (impossible to create an External Identifier anymore). Also, the reference `_formatValue` wasn't correct as it was displaying the record `display_name` and not the reference field value ; this only makes sense for the External Identifier as its display name is the reference field value but it doesn't make any sense for other reference field. Last but not least, this commit implements the `onchange` and `default_get` for the reference field. --- .../web/static/src/js/views/basic/basic_model.js | 115 ++++++++++++++++----- 1 file changed, 88 insertions(+), 27 deletions(-) (limited to 'addons/web/static/src/js/views/basic/basic_model.js') 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 3bfab5e3eb6..0d95c292070 100644 --- a/addons/web/static/src/js/views/basic/basic_model.js +++ b/addons/web/static/src/js/views/basic/basic_model.js @@ -1146,9 +1146,9 @@ var BasicModel = AbstractModel.extend({ record._rawChanges[name] = val; return; } - - if (field.type === 'many2one' ) { - var id = false; + var id; + if (field.type === 'many2one') { + 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. @@ -1167,6 +1167,20 @@ var BasicModel = AbstractModel.extend({ id = rec.id; } record._changes[name] = id; + } else if (field.type === 'reference') { + id = false; + if (val) { + var ref = val.split(','); + rec = self._makeDataPoint({ + context: record.context, + data: {id: parseInt(ref[1])}, + modelName: ref[0], + parentID: record.id, + }); + defs.push(self._fetchNameGet(rec)); + id = rec.id; + } + record._changes[name] = id; } else if (field.type === 'one2many' || field.type === 'many2many') { var listId = record._changes[name] || record.data[name]; var list; @@ -1642,6 +1656,39 @@ var BasicModel = AbstractModel.extend({ }); }); }, + /** + * Fetch the `name_get` for a reference field. + * + * @private + * @param {Object} record + * @param {string} fieldName + * @returns {Deferred} + */ + _fetchReference: function (record, fieldName) { + var self = this; + var def; + var value = record._changes && record._changes[fieldName] || record.data[fieldName]; + var model = value && value.split(',')[0]; + var resID = value && parseInt(value.split(',')[1]); + if (model && model !== 'False' && resID) { + def = self._rpc({ + model: model, + method: 'name_get', + args: [resID], + context: record.getContext({fieldName: fieldName}), + }).then(function (result) { + return self._makeDataPoint({ + data: { + id: result[0][0], + display_name: result[0][1], + }, + modelName: model, + parentID: record.id, + }); + }); + } + return $.when(def); + }, /** * Fetch the extra data (`name_get`) for the reference fields of the record * model. @@ -1657,28 +1704,12 @@ var BasicModel = AbstractModel.extend({ _.each(fieldNames, function (fieldName) { var field = record.fields[fieldName]; if (field.type === 'reference') { - var value = record.data[fieldName]; - if (value) { - var model = value.split(',')[0]; - var resID = parseInt(value.split(',')[1]); - var def = self._rpc({ - model: model, - method: 'name_get', - args: [resID], - context: record.getContext({fieldName: fieldName}), - }).then(function (result) { - var referenceDp = self._makeDataPoint({ - data: { - id: result[0][0], - display_name: result[0][1], - }, - modelName: model, - parentID: record.id, - }); - record.data[fieldName] = referenceDp.id; - }); - defs.push(def); - } + var def = self._fetchReference(record, fieldName).then(function (dataPoint) { + if (dataPoint) { + record.data[fieldName] = dataPoint.id; + } + }); + defs.push(def); } }); return $.when.apply($, defs); @@ -1948,6 +1979,25 @@ var BasicModel = AbstractModel.extend({ context: context }); }, + /** + * Fetches the `name_get` associated to the reference widget if the field is + * a `char` (which is a supported case). + * + * @private + * @param {Object} record - an element from the localData + * @param {Object} fieldName - the name of the field + * @returns {Deferred} + */ + _fetchSpecialReference: function (record, fieldName) { + var def; + var field = record.fields[fieldName]; + if (field.type === 'char') { + // if the widget reference is set on a char field, the name_get + // needs to be fetched a posteriori + def = this._fetchReference(record, fieldName); + } + return $.when(def); + }, /** * Fetches all the m2o records associated to the given fieldName. If the * given fieldName is not a m2o field, nothing is done. The difference with @@ -2870,14 +2920,25 @@ var BasicModel = AbstractModel.extend({ var field = params.fields[name]; data[name] = null; record._changes = record._changes || {}; + var dp; if (field.type === 'many2one' && result[name]) { - var rec = self._makeDataPoint({ + dp = self._makeDataPoint({ context: record.context, data: {id: result[name]}, modelName: field.relation, parentID: record.id, }); - record._changes[name] = rec.id; + record._changes[name] = dp.id; + } else if (field.type === 'reference' && result[name]) { + var ref = result[name].split(','); + dp = self._makeDataPoint({ + context: record.context, + data: {id: parseInt(ref[1])}, + modelName: ref[0], + parentID: record.id, + }); + defs.push(self._fetchNameGet(dp)); + record._changes[name] = dp.id; } else if (field.type === 'one2many' || field.type === 'many2many') { var fieldInfo = record.fieldsInfo[record.viewType][name]; var view = fieldInfo.views && fieldInfo.views[fieldInfo.mode]; -- cgit v1.2.3