From ef16e96255e4e362053ca7b2e4c90ef7ed1908a2 Mon Sep 17 00:00:00 2001 From: Denis Ledoux Date: Fri, 16 Sep 2022 13:34:10 +0000 Subject: [PATCH] [FIX] base, web: graph and pivot views requires more field descriptions Following odoo/odoo@4636620004f0cc0e504cd6694fe8ddb6758ba811 `get_views` only pass the model fields included in the view architecture, except for the main model when the search views is requested. Because the search views requires all fields for the user to be able to make advanced filters and advanced group by using any fields of the model. However, other views requires more field descriptions as well than just the fields included in their architecture: - the graph view requires all integer and float fields, to automatically add suggestions of measures in the measures dropdown menu. It's a bit like the search view, the user should be able to choose any measure available in the model (as long as this is integer or float fields) - the pivot view requires all groupable fields, so the user can group by any groupable fields of the model. The JS MockServer `getViews` is adapted to include the changes added by the above revision as well as the current revision, for the qunit tests suite to be able to reflect these API changes from the server side. closes odoo/odoo#100376 Related: odoo/enterprise#31427 Signed-off-by: Denis Ledoux (dle) --- .../mail/static/tests/helpers/mock_server.js | 2 +- .../web/static/tests/helpers/mock_server.js | 88 +++++++++++++++---- addons/web/static/tests/views/view_tests.js | 12 +-- odoo/models.py | 31 +++++-- 4 files changed, 103 insertions(+), 30 deletions(-) diff --git a/addons/mail/static/tests/helpers/mock_server.js b/addons/mail/static/tests/helpers/mock_server.js index 744990ab00f..31f61d9bbb9 100644 --- a/addons/mail/static/tests/helpers/mock_server.js +++ b/addons/mail/static/tests/helpers/mock_server.js @@ -1992,7 +1992,7 @@ patch(MockServer.prototype, 'mail', { * Simulates `_message_track` on `mail.thread` */ _mockMailThread_MessageTrack(modelName, trackedFieldNames, initialTrackedFieldValuesByRecordId) { - const trackFieldNamesToField = this.mockFieldsGet(modelName, [trackedFieldNames]); + const trackFieldNamesToField = this.mockFieldsGet(modelName, trackedFieldNames); const tracking = {}; const records = this.models[modelName].records; for (const record of records) { diff --git a/addons/web/static/tests/helpers/mock_server.js b/addons/web/static/tests/helpers/mock_server.js index 19ee46f7ec2..005d415f5f2 100644 --- a/addons/web/static/tests/helpers/mock_server.js +++ b/addons/web/static/tests/helpers/mock_server.js @@ -147,6 +147,40 @@ export class MockServer { // def.abort = abort; } + _getViewFields(modelName, viewType, models) { + if (["kanban", "list", "form"].includes(viewType)) { + for (const fieldNames of Object.values(models)) { + fieldNames.add("id"); + fieldNames.add("__last_update"); + } + } else if (viewType === "search") { + models[modelName] = Object.keys(this.models[modelName].fields); + } else if (viewType === "graph") { + for (const [fieldName, field] of Object.entries(this.models[modelName].fields)) { + if (["integer", "float"].includes(field.type)) { + models[modelName].add(fieldName); + } + } + } else if (viewType === "pivot") { + for (const [fieldName, field] of Object.entries(this.models[modelName].fields)) { + if ( + [ + "many2one", + "many2many", + "char", + "boolean", + "selection", + "date", + "datetime", + ].includes(field.type) + ) { + models[modelName].add(fieldName); + } + } + } + return models; + } + getView(modelName, args, kwargs) { if (!(modelName in this.models)) { throw new Error(`Model ${modelName} was not defined in mock server data`); @@ -208,7 +242,7 @@ export class MockServer { const onchanges = params.models[modelName].onchanges || {}; const fieldNodes = {}; const groupbyNodes = {}; - const relatedModels = new Set([modelName]); + const relatedModels = { [modelName]: new Set() }; let doc; if (typeof arch === "string") { doc = domParser.parseFromString(arch, "text/xml").documentElement; @@ -316,6 +350,7 @@ export class MockServer { } return !isField; }); + Object.keys(fieldNodes).forEach((field) => relatedModels[modelName].add(field)); let relModel, relFields; Object.entries(fieldNodes).forEach(([name, { node, isInvisible }]) => { const field = fields[name]; @@ -327,7 +362,6 @@ export class MockServer { } if (field.type === "one2many" || field.type === "many2many") { relModel = field.relation; - relatedModels.add(relModel); // inline subviews: in forms if field is visible and has no widget (1st level only) if (inFormView && level === 0 && !node.getAttribute("widget") && !isInvisible) { const inlineViewTypes = Array.from(node.children).map((c) => c.tagName); @@ -368,7 +402,10 @@ export class MockServer { processedNodes, level: level + 1, }); - [...models].forEach((modelName) => relatedModels.add(modelName)); + Object.entries(models).forEach(([modelName, fields]) => { + relatedModels[modelName] = relatedModels[modelName] || new Set(); + fields.forEach((field) => relatedModels[modelName].add(field)); + }); } }); } @@ -384,7 +421,6 @@ export class MockServer { } field.views = {}; relModel = field.relation; - relatedModels.add(relModel); relFields = Object.assign({}, params.models[relModel].fields); processedNodes.push(node); // postprocess simulation @@ -396,7 +432,10 @@ export class MockServer { context, processedNodes, }); - [...models].forEach((modelName) => relatedModels.add(modelName)); + Object.entries(models).forEach(([modelName, fields]) => { + relatedModels[modelName] = relatedModels[modelName] || new Set(); + fields.forEach((field) => relatedModels[modelName].add(field)); + }); }); const processedArch = xmlSerializer.serializeToString(doc); const fieldsInView = {}; @@ -405,11 +444,12 @@ export class MockServer { fieldsInView[fname] = field; } }); + const viewType = doc.tagName === "tree" ? "list" : doc.tagName; return { arch: processedArch, model: modelName, - type: doc.tagName === "tree" ? "list" : doc.tagName, - models: relatedModels, + type: viewType, + models: this._getViewFields(modelName, viewType, relatedModels), }; } @@ -492,7 +532,7 @@ export class MockServer { case "create": return this.mockCreate(args.model, args.args[0], args.kwargs); case "fields_get": - return this.mockFieldsGet(args.model); + return this.mockFieldsGet(args.model, args.fields); case "get_views": return this.mockGetViews(args.model, args.kwargs); case "name_create": @@ -607,8 +647,12 @@ export class MockServer { return result; } - mockFieldsGet(modelName) { - return this.models[modelName].fields; + mockFieldsGet(modelName, fieldNames) { + let fields = this.models[modelName].fields; + if (fieldNames) { + fields = _.pick(this.models[modelName].fields, fieldNames); + } + return fields; } mockLoadAction(kwargs) { @@ -637,16 +681,26 @@ export class MockServer { mockGetViews(modelName, kwargs) { const views = {}; const models = {}; - models[modelName] = this.mockFieldsGet(modelName); + + // Determine all the models/fields used in the views + // modelFields = {modelName: Set([...fieldNames])} + const modelFields = {}; kwargs.views.forEach(([viewId, viewType]) => { views[viewType] = this.getView(modelName, [viewId, viewType], kwargs); - if (kwargs.options.load_filters && viewType === "search") { - views[viewType].filters = this.models[modelName].filters || []; - } - for (const modelName of views[viewType].models) { - models[modelName] = models[modelName] || this.mockFieldsGet(modelName); - } + Object.entries(views[viewType].models).forEach(([modelName, fields]) => { + modelFields[modelName] = modelFields[modelName] || new Set(); + fields.forEach((field) => modelFields[modelName].add(field)); + }); }); + + // For each model, fetch the information of the fields used in the views only + Object.entries(modelFields).forEach(([modelName, fields]) => { + models[modelName] = this.mockFieldsGet(modelName, [...fields]); + }); + + if (kwargs.options.load_filters && "search" in views) { + views["search"].filters = this.models[modelName].filters || []; + } return { models, views }; } diff --git a/addons/web/static/tests/views/view_tests.js b/addons/web/static/tests/views/view_tests.js index 5f098514f12..887e93e11c0 100644 --- a/addons/web/static/tests/views/view_tests.js +++ b/addons/web/static/tests/views/view_tests.js @@ -124,7 +124,7 @@ QUnit.module("Views", (hooks) => { this._super(); const { arch, fields, info } = this.props; assert.strictEqual(arch, serverData.views["animal,false,toy"]); - assert.deepEqual(fields, serverData.models.animal.fields); + assert.deepEqual(fields, {}); assert.strictEqual(info.actionMenus, undefined); assert.strictEqual(this.env.config.viewId, false); }, @@ -162,7 +162,7 @@ QUnit.module("Views", (hooks) => { this._super(); const { arch, fields, info } = this.props; assert.strictEqual(arch, serverData.views["animal,1,toy"]); - assert.deepEqual(fields, serverData.models.animal.fields); + assert.deepEqual(fields, {}); assert.strictEqual(info.actionMenus, undefined); assert.strictEqual(this.env.config.viewId, 1); }, @@ -199,7 +199,7 @@ QUnit.module("Views", (hooks) => { this._super(); const { arch, fields, info } = this.props; assert.strictEqual(arch, serverData.views["animal,1,toy"]); - assert.deepEqual(fields, serverData.models.animal.fields); + assert.deepEqual(fields, {}); assert.strictEqual(info.actionMenus, undefined); assert.strictEqual(this.env.config.viewId, 1); }, @@ -240,7 +240,7 @@ QUnit.module("Views", (hooks) => { this._super(); const { arch, fields, info } = this.props; assert.strictEqual(arch, serverData.views["animal,false,toy"]); - assert.deepEqual(fields, serverData.models.animal.fields); + assert.deepEqual(fields, {}); assert.strictEqual(info.actionMenus, undefined); assert.strictEqual(this.env.config.viewId, false); }, @@ -283,7 +283,7 @@ QUnit.module("Views", (hooks) => { this._super(); const { arch, fields, info } = this.props; assert.strictEqual(arch, serverData.views["animal,1,toy"]); - assert.deepEqual(fields, serverData.models.animal.fields); + assert.deepEqual(fields, {}); assert.strictEqual(info.actionMenus, undefined); assert.strictEqual(this.env.config.viewId, 1); }, @@ -362,7 +362,7 @@ QUnit.module("Views", (hooks) => { this._super(); const { arch, fields, info } = this.props; assert.strictEqual(arch, serverData.views["animal,false,toy"]); - assert.deepEqual(fields, serverData.models.animal.fields); + assert.deepEqual(fields, {}); assert.deepEqual(info.actionMenus, {}); assert.strictEqual(this.env.config.viewId, false); }, diff --git a/odoo/models.py b/odoo/models.py index 65c9163cafe..69cde2ea819 100644 --- a/odoo/models.py +++ b/odoo/models.py @@ -1659,15 +1659,10 @@ class BaseModel(metaclass=MetaModel): models = {} for view in result['views'].values(): for model, model_fields in view.pop('models').items(): - models.setdefault(model, {'id', self.CONCURRENCY_CHECK_FIELD}).update(model_fields) + models.setdefault(model, set()).update(model_fields) result['models'] = {} - if 'search' in result['views']: - # If the search view is requested, all fields of the main model must be passed. - result['models'][self._name] = self.fields_get(attributes=self._get_view_field_attributes()) - models.pop(self._name) - for model, model_fields in models.items(): result['models'][model] = self.env[model].fields_get( allfields=model_fields, attributes=self._get_view_field_attributes() @@ -1797,6 +1792,7 @@ class BaseModel(metaclass=MetaModel): # Apply post processing, groups and modifiers etc... arch, models = view.postprocess_and_fields(arch, model=self._name, **options) + models = self._get_view_fields(view_type or view.type, models) result = { 'arch': arch, # TODO: only `web_studio` seems to require this. I guess this is acceptable to keep it. @@ -1840,6 +1836,29 @@ class BaseModel(metaclass=MetaModel): return result + @api.model + def _get_view_fields(self, view_type, models): + """ Returns the field names required by the web client to load the views according to the view type. + + The method is meant to be overridden by modules extending web client features and requiring additional + fields. + + :param string view_type: type of the view + :param dict models: dict holding the models and fields used in the view architecture. + :return: dict holding the models and field required by the web client given the view type. + :rtype: list + """ + if view_type in ('kanban', 'list', 'form'): + for model_fields in models.values(): + model_fields.update({'id', self.CONCURRENCY_CHECK_FIELD}) + elif view_type == 'search': + models[self._name] = list(self._fields.keys()) + elif view_type == 'graph': + models[self._name].union(fname for fname, field in self._fields.items() if field.type in ('integer', 'float')) + elif view_type == 'pivot': + models[self._name].union(fname for fname, field in self._fields.items() if field.groupable) + return models + @api.model def _get_view_field_attributes(self): """ Returns the field attributes required by the web client to load the views.