From f8ab35f9e18d033cb9fa2b28c6abe3689afc774a Mon Sep 17 00:00:00 2001 From: Julien Mougenot Date: Fri, 31 Jul 2020 11:46:14 +0000 Subject: [PATCH 1/4] [IMP] web: Model: small simplifications Small refactor of some aspects of the main model.js class to make it more consistent and extensible. --- addons/web/static/src/js/model.js | 113 ++++++++++++++++-------------- 1 file changed, 60 insertions(+), 53 deletions(-) diff --git a/addons/web/static/src/js/model.js b/addons/web/static/src/js/model.js index b86599eb5fc..51959450216 100644 --- a/addons/web/static/src/js/model.js +++ b/addons/web/static/src/js/model.js @@ -20,7 +20,7 @@ odoo.define("web/static/src/js/model.js", function (require) { constructor(config) { this.config = config; this.env = this.config.env; - this._shouldLoad = true; + this.shouldLoad = true; this.state = {}; } @@ -30,12 +30,23 @@ odoo.define("web/static/src/js/model.js", function (require) { /** * Used by the parent model to initiate a load action. The actual - * loading of the extension is determined by the "_shouldLoad" property. + * loading of the extension is determined by the "shouldLoad" property. + * @param {Object} params */ - async callLoad() { - if (this._shouldLoad) { - this._shouldLoad = false; - await this.load(); + async callLoad(params) { + if (this.shouldLoad) { + this.shouldLoad = false; + await this.load(params); + } + } + + /** + * @param {string} method + * @param {...any} args + */ + dispatch(method, ...args) { + if (method in this) { + this[method](...args); } } @@ -65,8 +76,8 @@ odoo.define("web/static/src/js/model.js", function (require) { * @param {Object} [state] */ importState(state) { - this._shouldLoad = !state; - if (this._shouldLoad) { + this.shouldLoad = !state; + if (this.shouldLoad) { this.prepareState(); } else { Object.assign(this.state, state); @@ -76,9 +87,10 @@ odoo.define("web/static/src/js/model.js", function (require) { /** * Called and awaited on initial model load. * @abstract + * @param {Object} params * @returns {Promise} */ - async load() { + async load(/* params */) { /* ... */ } @@ -90,15 +102,6 @@ odoo.define("web/static/src/js/model.js", function (require) { prepareState() { /* ... */ } - - /** - * Called and awaited after an action dispatch. - * @abstract - * @returns {Promise} - */ - async reloadAfterDispatch() { - /* ... */ - } } /** * The layer of an extension indicates with which other extensions this one @@ -285,40 +288,32 @@ odoo.define("web/static/src/js/model.js", function (require) { * extension. This method must be overridden if multiple extensions * return a value with a common method (and dispatchAll does not * suffice). After the dispatch of the action, all models are partially - * reloaded (@see ModelExtension.reloadAfterDispatch()) and components - * are notified afterwards. + * reloaded and components are notified afterwards. + * @param {string} method + * @param {...any} args */ dispatch(method, ...args) { const isInitialDispatch = !this.dispatching; this.dispatching = true; for (const extension of this.extensions.flat()) { - if (method in extension) { - extension[method](...args); + extension.dispatch(method, ...args); + } + if (!isInitialDispatch) { + return; + } + this.dispatching = false; + let rev = this.rev; + // Calls 'after dispatch' hooks + // Purpose: fetch updated data from the server. This is considered + // a loading action and is thus performed by groups instead of + // loading all extensions at once. + this._loadExtensions({ isInitialLoad: false }).then(() => { + // Notifies subscribed components + // Purpose: re-render components bound by 'useModel' + if (rev === this.rev) { + this._notifyComponents(); } - } - if (isInitialDispatch) { - this.dispatching = false; - (async () => { - // Calls 'after dispatch' hooks - // Purpose: fetch updated data from the server. This is - // considered a loading action and is thus performed by groups - // instead of loading all extensions at once. - for (let layer = 0; layer < this.extensions.length; layer++) { - const afterDispatchPromises = []; - for (const extension of this.extensions[layer]) { - afterDispatchPromises.push(extension.reloadAfterDispatch()); - } - await Promise.all(afterDispatchPromises); - } - // Notifies subscribed components - // Purpose: re-render components bound by 'useModel' - let rev = this.rev; - await Promise.resolve(); - if (rev === this.rev) { - this._notifyComponents(); - } - })(); - } + }); } /** @@ -378,13 +373,7 @@ odoo.define("web/static/src/js/model.js", function (require) { * @returns {Promise} */ async load() { - for (let layer = 0; layer < this.extensions.length; layer++) { - const layerPromises = []; - for (const extension of this.extensions[layer]) { - layerPromises.push(extension.callLoad()); - } - await Promise.all(layerPromises); - } + await this._loadExtensions({ isInitialLoad: true }); } //--------------------------------------------------------------------- @@ -416,6 +405,24 @@ odoo.define("web/static/src/js/model.js", function (require) { return results; } + /** + * Private handler to loop over all extension layers sequencially and + * wait for a given callback to be completed on all extensions of a + * same layer. + * @private + * @param {Object} params + * @param {boolean} params.isInitialLoad whether this call comes + * from the initial load. + * @returns {Promise} + */ + async _loadExtensions(params) { + for (let layer = 0; layer < this.extensions.length; layer++) { + await Promise.all(this.extensions[layer].map( + (extension) => extension.callLoad(params) + )); + } + } + /** * @see Context.__notifyComponents() in owl.js for explanation * @private From 6e780b2e4f69cdee247afe4be964227a90b3d738 Mon Sep 17 00:00:00 2001 From: Julien Mougenot Date: Fri, 31 Jul 2020 11:47:53 +0000 Subject: [PATCH 2/4] [IMP] lunch: Make dispatch actions synchronous Before this commit, the return value of some of the dispatch actions of the lunch model extension was wrapped inside a promise with the use of the "async" keyword. This caused the synchronous aspect of all these methods to be delayed. Now they are not longer asynchronous. --- addons/lunch/static/src/js/lunch_model_extension.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/addons/lunch/static/src/js/lunch_model_extension.js b/addons/lunch/static/src/js/lunch_model_extension.js index fccd82fdce9..aec64b9762a 100644 --- a/addons/lunch/static/src/js/lunch_model_extension.js +++ b/addons/lunch/static/src/js/lunch_model_extension.js @@ -55,9 +55,9 @@ odoo.define("lunch/static/src/js/lunch_model_extension.js", function (require) { * @param {number} locationId * @returns {Promise} */ - async setLocationId(locationId) { + setLocationId(locationId) { this.state.locationId = locationId; - await this.env.services.rpc({ + this.env.services.rpc({ route: "/lunch/user_location_set", params: { context: this.env.session.user_context, @@ -71,9 +71,9 @@ odoo.define("lunch/static/src/js/lunch_model_extension.js", function (require) { * @param {number} userId * @returns {Promise} */ - async updateUserId(userId) { + updateUserId(userId) { this.state.userId = userId; - await this._updateLocationId(); + this.shouldLoad = true; } //--------------------------------------------------------------------- From cc6a60bec935a42e19bac0bef5771dfcbad81f83 Mon Sep 17 00:00:00 2001 From: Julien Mougenot Date: Fri, 31 Jul 2020 11:50:56 +0000 Subject: [PATCH 3/4] [IMP] web: Optimize action model loading The loading of the action model extensions have been split in this commit: - when calling load, a first promise is returned. It is meant to be resolved as soon as all the query informations are available. - a second promise returned by a new method "isReady" is resolved when all of the extension data have been fetched. This difference allow to not wait for server calls irrelevant for the query. By default "isReady" will always wait for a full load call, but this is meant to be overridden in case optimisations can be made. --- .../src/js/views/abstract_controller.js | 1 - .../web/static/src/js/views/abstract_view.js | 11 ++-- .../web/static/src/js/views/action_model.js | 52 +++++++++++++++++-- 3 files changed, 56 insertions(+), 8 deletions(-) diff --git a/addons/web/static/src/js/views/abstract_controller.js b/addons/web/static/src/js/views/abstract_controller.js index 5128fa6d9a8..2f819bff7a5 100644 --- a/addons/web/static/src/js/views/abstract_controller.js +++ b/addons/web/static/src/js/views/abstract_controller.js @@ -222,7 +222,6 @@ var AbstractController = mvc.Controller.extend(ActionMixin, { reload: async function (params = {}) { if (params.controllerState) { this.importState(params.controllerState); - await this.searchModel.load(); Object.assign(params, this.searchModel.get('query')); } return this.update(params, {}); diff --git a/addons/web/static/src/js/views/abstract_view.js b/addons/web/static/src/js/views/abstract_view.js index 04b1b781848..129cb6817b3 100644 --- a/addons/web/static/src/js/views/abstract_view.js +++ b/addons/web/static/src/js/views/abstract_view.js @@ -306,13 +306,16 @@ var AbstractView = Factory.extend({ */ getController: async function () { const _super = this._super.bind(this); - await this.controllerParams.searchModel.load(); - const query = this.controllerParams.searchModel.get('query'); - this._updateMVCParams(query); + const { searchModel } = this.controllerParams; + await searchModel.load(); + this._updateMVCParams(searchModel.get("query")); // get the parent of the model if it already exists, as _super will // set the new controller as parent, which we don't want const modelParent = this.model && this.model.getParent(); - const controller = await _super(...arguments); + const [controller] = await Promise.all([ + _super(...arguments), + searchModel.isReady(), + ]); if (modelParent) { // if we already add a model, restore its parent this.model.setParent(modelParent); diff --git a/addons/web/static/src/js/views/action_model.js b/addons/web/static/src/js/views/action_model.js index 382aa30531b..c3b69271a07 100644 --- a/addons/web/static/src/js/views/action_model.js +++ b/addons/web/static/src/js/views/action_model.js @@ -16,6 +16,29 @@ odoo.define("web/static/src/js/views/action_model.js", function (require) { */ class ActionModelExtension extends Model.Extension { + //--------------------------------------------------------------------- + // Public + //--------------------------------------------------------------------- + + /** + * Initiates the asynchronous tasks of the extension and returns a + * promise resolved as soon as all the informations necessary to build + * the search query are ready. + * @returns {Promise} + */ + async callLoad() { + this.loadPromise = super.callLoad(...arguments); + await this.loadPromise; + } + + /** + * Returns a promise resolved when the extension is completely ready. + * @returns {Promise} + */ + async isReady() { + await this.loadPromise; + } + //--------------------------------------------------------------------- // Static //--------------------------------------------------------------------- @@ -51,10 +74,28 @@ odoo.define("web/static/src/js/views/action_model.js", function (require) { return super.get(...arguments); } + /** + * Returns a promise resolved when all extensions are completely ready. + * @returns {Promise} + */ + async isReady() { + await this._awaitExtensions(); + } + //--------------------------------------------------------------------- // Private //--------------------------------------------------------------------- + /** + * @private + * @returns {Promise} + */ + async _awaitExtensions() { + await Promise.all(this.extensions.flat().map( + (extension) => extension.isReady() + )); + } + /** * @override */ @@ -140,11 +181,16 @@ odoo.define("web/static/src/js/views/action_model.js", function (require) { } /** + * Overridden to trigger a "search" event as soon as the query data + * are ready. * @override */ - _notifyComponents() { - this.trigger("search", this.get("query")); - super._notifyComponents(...arguments); + async _loadExtensions({ isInitialLoad }) { + await super._loadExtensions(...arguments); + if (!isInitialLoad) { + this.trigger("search", this.get("query")); + await this._awaitExtensions(); + } } //--------------------------------------------------------------------- From 2524c7aaace5ce314eace5517285bf13586fb176 Mon Sep 17 00:00:00 2001 From: Julien Mougenot Date: Fri, 31 Jul 2020 11:52:25 +0000 Subject: [PATCH 4/4] [IMP] web: Do not store search panel categories This commit handles two things: 1) Move the local storage logic of the search panel into the documents module. 2) This allows for a variety of optimisations now that we do not have to constantly wait for a potential default value hidden in the storage. This is why this commit also introduces a way for the search panel model extension to sometimes return its query informations sooner than after being completely loaded. Task 2284207 --- .../js/chrome/action_manager_act_window.js | 1 - .../web/static/src/js/views/abstract_view.js | 9 - .../js/views/search_panel_model_extension.js | 317 ++++++++------ .../tests/chrome/action_manager_tests.js | 3 - .../static/tests/views/search_panel_tests.js | 412 ++++++++---------- 5 files changed, 357 insertions(+), 385 deletions(-) diff --git a/addons/web/static/src/js/chrome/action_manager_act_window.js b/addons/web/static/src/js/chrome/action_manager_act_window.js index 749feacb3d6..ee39d7cb7a7 100644 --- a/addons/web/static/src/js/chrome/action_manager_act_window.js +++ b/addons/web/static/src/js/chrome/action_manager_act_window.js @@ -693,7 +693,6 @@ ActionManager.include({ if (config.device.isMobile && actionData.mobile) { options = Object.assign({}, options, actionData.mobile); } - action.flags = _.extend({}, action.flags, {searchPanelDefaultNoFilter: true}); return self.doAction(action, options).then(ev.data.on_success, ev.data.on_fail); }); }, diff --git a/addons/web/static/src/js/views/abstract_view.js b/addons/web/static/src/js/views/abstract_view.js index 129cb6817b3..127b2a10c45 100644 --- a/addons/web/static/src/js/views/abstract_view.js +++ b/addons/web/static/src/js/views/abstract_view.js @@ -260,17 +260,8 @@ var AbstractView = Factory.extend({ if (this.withSearchPanel) { // Search panel (Model) const SearchPanelComponent = this.config.SearchPanel; - const defaultValues = {}; - Object.keys(this.loadParams.context).forEach((key) => { - let match = /^searchpanel_default_(.*)$/.exec(key); - if (match) { - defaultValues[match[1]] = this.loadParams.context[key]; - } - }); extensions[SearchPanelComponent.modelExtension] = { archNodes: searchPanelInfo.children, - defaultNoFilter: params.searchPanelDefaultNoFilter, - defaultValues, }; this.controllerParams.withSearchPanel = true; this.rendererParams.withSearchPanel = true; diff --git a/addons/web/static/src/js/views/search_panel_model_extension.js b/addons/web/static/src/js/views/search_panel_model_extension.js index 3641c425ba7..3aca92029f3 100644 --- a/addons/web/static/src/js/views/search_panel_model_extension.js +++ b/addons/web/static/src/js/views/search_panel_model_extension.js @@ -13,6 +13,14 @@ odoo.define("web/static/src/js/views/search_panel_model_extension.js", function const DEFAULT_LIMIT = 200; let nextSectionId = 1; + /** + * @param {Filter} filter + * @returns {boolean} + */ + function hasDomain(filter) { + return filter.domain !== "[]"; + } + /** * @param {Section} section * @returns {boolean} @@ -68,14 +76,44 @@ odoo.define("web/static/src/js/views/search_panel_model_extension.js", function constructor() { super(...arguments); - this.searchDomain = null; - this.DEFAULT_VALUE_INDEX = 0; + this.categoriesToLoad = []; + this.defaultValues = {}; + this.filtersToLoad = []; + this.initialStateImport = false; + this.searchDomain = []; + for (const key in this.config.context) { + const match = /^searchpanel_default_(.*)$/.exec(key); + if (match) { + this.defaultValues[match[1]] = this.config.context[key]; + } + } } //--------------------------------------------------------------------- // Public //--------------------------------------------------------------------- + /** + * @override + */ + async callLoad(params) { + const searchDomain = this._getExternalDomain(); + params.searchDomainChanged = ( + JSON.stringify(this.searchDomain) !== JSON.stringify(searchDomain) + ); + if (!this.shouldLoad && !this.initialStateImport) { + const isFetchable = (section) => section.enableCounters || + (params.searchDomainChanged && !section.expand); + this.categoriesToLoad = this.categories.filter(isFetchable); + this.filtersToLoad = this.filters.filter(isFetchable); + this.shouldLoad = params.searchDomainChanged || + Boolean(this.categoriesToLoad.length + this.filtersToLoad.length); + } + this.searchDomain = searchDomain; + this.initialStateImport = false; + await super.callLoad(params); + } + /** * @override */ @@ -109,6 +147,7 @@ odoo.define("web/static/src/js/views/search_panel_model_extension.js", function * @override */ importState(importedState) { + this.initialStateImport = Boolean(importedState && !this.state.sections); super.importState(...arguments); if (importedState) { this.state.sections = new Map(this.state.sections); @@ -127,8 +166,18 @@ odoo.define("web/static/src/js/views/search_panel_model_extension.js", function /** * @override */ - async load() { - await this._fetchSections({ initialLoad: true }); + async isReady() { + await this.sectionsPromise; + } + + /** + * @override + */ + async load(params) { + this.sectionsPromise = this._fetchSections(params.isInitialLoad); + if (this._shouldWaitForData(params)) { + await this.sectionsPromise; + } } /** @@ -139,13 +188,6 @@ odoo.define("web/static/src/js/views/search_panel_model_extension.js", function this._createSectionsFromArch(); } - /** - * @override - */ - async reloadAfterDispatch() { - await this._fetchSections(); - } - //--------------------------------------------------------------------- // Actions / Getters //--------------------------------------------------------------------- @@ -180,18 +222,13 @@ odoo.define("web/static/src/js/views/search_panel_model_extension.js", function } /** - * Sets the active value id of a given category. This will also update - * the local storage value. + * Sets the active value id of a given category. * @param {number} sectionId * @param {number} valueId */ toggleCategoryValue(sectionId, valueId) { const category = this.state.sections.get(sectionId); category.activeValueId = valueId; - const storageKey = `searchpanel_${this.config.modelName}_${category.fieldName}`; - this.env.services.local_storage.setItem(storageKey, valueId); - // FIXME this should be removed (in another commit): should no longer (re)store - // active category in localStorage } /** @@ -240,7 +277,7 @@ odoo.define("web/static/src/js/views/search_panel_model_extension.js", function */ _applyDefaultFilterValues() { for (const { fieldName, values } of this.filters) { - const defaultValues = this.config.defaultValues[fieldName] || []; + const defaultValues = this.defaultValues[fieldName] || []; for (const valueId of defaultValues) { const value = values.get(valueId); if (value) { @@ -266,15 +303,6 @@ odoo.define("web/static/src/js/views/search_panel_model_extension.js", function if (category.hierarchize) { category.parentField = parentField; } - - const allItem = { - childrenIds: [], - display_name: this.env._t("All"), - id: false, - bold: true, - parentId: false, - }; - category.values = new Map([[false, allItem]]); for (const value of values) { category.values.set( value.id, @@ -299,7 +327,8 @@ odoo.define("web/static/src/js/views/search_panel_model_extension.js", function } } // Set active value from context - category.activeValueId = this._getCategoryDefaultValue(values, category); + const valueIds = [false, ...values.map((val) => val.id)]; + this._ensureCategoryValue(category, valueIds); } /** @@ -318,8 +347,8 @@ odoo.define("web/static/src/js/views/search_panel_model_extension.js", function // restore checked property values.forEach((value) => { - const oldValue = filter.values && filter.values.get(value.id); - value.checked = (oldValue && oldValue.checked) || false; + const oldValue = filter.values.get(value.id); + value.checked = oldValue ? oldValue.checked : false; }); filter.values = new Map(); @@ -390,12 +419,21 @@ odoo.define("web/static/src/js/views/search_panel_model_extension.js", function index, limit: pyUtils.py_eval(attrs.limit || String(DEFAULT_LIMIT)), type, + values: new Map(), }; if (type === "category") { + section.activeValueId = this.defaultValues[attrs.name]; section.icon = section.icon || "fa-folder"; section.hierarchize = !!pyUtils.py_eval( attrs.hierarchize || "1" ); + section.values.set(false, { + childrenIds: [], + display_name: this.env._t("All"), + id: false, + bold: true, + parentId: false, + }); } else { section.domain = attrs.domain || "[]"; section.groupBy = attrs.groupby; @@ -405,104 +443,97 @@ odoo.define("web/static/src/js/views/search_panel_model_extension.js", function }); } + /** + * Ensures that the active value of a category is one of its own + * existing values. + * @private + * @param {Category} category + * @param {number[]} valueIds + */ + _ensureCategoryValue(category, valueIds) { + if (!valueIds.includes(category.activeValueId)) { + category.activeValueId = valueIds[0]; + } + } + /** * Fetches values for each category at startup. At reload a category is * only fetched if needed. * @private - * @param {{ - * initialLoad: boolean, - * searchDomainHasChanged: boolean - * }} options + * @param {Category[]} categories * @returns {Promise} resolved when all categories have been fetched */ - async _fetchCategories({ initialLoad, searchDomainHasChanged }) { - const proms = []; - for (const category of this.categories) { - const { enableCounters, expand, hierarchize, limit } = category; - if (initialLoad || enableCounters || (searchDomainHasChanged && !expand)) { - const prom = this.env.services.rpc({ - method: "search_panel_select_range", - model: this.config.modelName, - args: [category.fieldName], - kwargs: { - category_domain: this._getCategoryDomain( - category.id - ), - enable_counters: enableCounters, - expand, - filter_domain: this._getFilterDomain(), - hierarchize, - limit, - search_domain: this.searchDomain, - }, - }).then(result => this._createCategoryTree(category.id, result)); - proms.push(prom); - } - } - await Promise.all(proms); + async _fetchCategories(categories) { + const filterDomain = this._getFilterDomain(); + await Promise.all(categories.map(async (category) => { + const result = await this.env.services.rpc({ + method: "search_panel_select_range", + model: this.config.modelName, + args: [category.fieldName], + kwargs: { + category_domain: this._getCategoryDomain(category.id), + enable_counters: category.enableCounters, + expand: category.expand, + filter_domain: filterDomain, + hierarchize: category.hierarchize, + limit: category.limit, + search_domain: this.searchDomain, + }, + }); + this._createCategoryTree(category.id, result); + })); } /** * Fetches values for each filter. This is done at startup and at each * reload if needed. * @private - * @param {{ - * initialLoad: boolean, - * searchDomainHasChanged: boolean - * }} options + * @param {Filter[]} filters * @returns {Promise} resolved when all filters have been fetched */ - async _fetchFilters({ initialLoad, searchDomainHasChanged }) { + async _fetchFilters(filters) { const evalContext = {}; for (const category of this.categories) { evalContext[category.fieldName] = category.activeValueId; } const categoryDomain = this._getCategoryDomain(); - const proms = []; - for (const filter of this.filters) { - const { enableCounters, expand, groupBy, limit } = filter; - if (initialLoad || enableCounters || (searchDomainHasChanged && !expand)) { - const prom = this.env.services.rpc({ - method: "search_panel_select_multi_range", - model: this.config.modelName, - args: [filter.fieldName], - kwargs: { - category_domain: categoryDomain, - comodel_domain: Domain.prototype.stringToArray( - filter.domain, - evalContext - ), - enable_counters: enableCounters, - filter_domain: this._getFilterDomain(filter.id), - expand, - group_by: groupBy || false, - group_domain: this._getGroupDomain(filter), - limit, - search_domain: this.searchDomain, - }, - }).then(result => this._createFilterTree(filter.id, result)); - proms.push(prom); - } - } - await Promise.all(proms); + await Promise.all(filters.map(async (filter) => { + const result = await this.env.services.rpc({ + method: "search_panel_select_multi_range", + model: this.config.modelName, + args: [filter.fieldName], + kwargs: { + category_domain: categoryDomain, + comodel_domain: Domain.prototype.stringToArray( + filter.domain, + evalContext + ), + enable_counters: filter.enableCounters, + filter_domain: this._getFilterDomain(filter.id), + expand: filter.expand, + group_by: filter.groupBy || false, + group_domain: this._getGroupDomain(filter), + limit: filter.limit, + search_domain: this.searchDomain, + }, + }); + this._createFilterTree(filter.id, result); + })); } /** - * Fetches categories and filters at startup or if needed. * @private - * @param {Object} [options={}] - * @param {boolean} [options.initialLoad=false] + * @param {boolean} isInitialLoad + * @returns {Promise} */ - async _fetchSections(options = {}) { - const searchDomain = this._getExternalDomain(); - if (!options.initialLoad) { - options.searchDomainHasChanged = - JSON.stringify(this.searchDomain) !== JSON.stringify(searchDomain); - } - this.searchDomain = searchDomain; - await this._fetchCategories(options); - await this._fetchFilters(options); - if (options.initialLoad) { + async _fetchSections(isInitialLoad) { + await this._fetchCategories( + isInitialLoad ? this.categories : this.categoriesToLoad + ); + await this._fetchFilters( + isInitialLoad ? this.filters : this.filtersToLoad + ); + if (isInitialLoad) { this._applyDefaultFilterValues(); } } @@ -520,53 +551,22 @@ odoo.define("web/static/src/js/views/search_panel_model_extension.js", function for (const category of this.categories) { if ( category.id === excludedCategoryId || - category.activeValueId === false + !category.activeValueId ) { continue; } - if (category.activeValueId) { - const field = this.config.fields[category.fieldName]; - const operator = - field.type === "many2one" && category.parentField ? "child_of" : "="; - domain.push([ - category.fieldName, - operator, - category.activeValueId, - ]); - } + const field = this.config.fields[category.fieldName]; + const operator = + field.type === "many2one" && category.parentField ? "child_of" : "="; + domain.push([ + category.fieldName, + operator, + category.activeValueId, + ]); } return domain; } - /** - * @private - * @param {Object[]} values - * @param {Category} category - * @returns {number | false} - */ - _getCategoryDefaultValue(values, { fieldName }) { - const validValues = [false, ...values.map((val) => val.id)]; - const defaultValue = this.config.defaultValues[fieldName]; - if (validValues.includes(defaultValue)) { - return defaultValue; - } - if (this.config.defaultNoFilter) { - return false; - } - // If not set in context, or set to an unknown value, set active value - // from localStorage - const storageKey = `searchpanel_${this.config.modelName}_${fieldName}`; - const storageValue = this.env.services.local_storage.getItem(storageKey); - if (validValues.includes(storageValue)) { - return storageValue; - } - // If still not a valid value, get the search panel default value - // from the given valid values. - return validValues[ - Math.min(validValues.length - 1, this.DEFAULT_VALUE_INDEX) - ]; - } - /** * Returns the domain retrieved from the other model extensions. * @private @@ -617,7 +617,7 @@ odoo.define("web/static/src/js/views/search_panel_model_extension.js", function for (const group of groups.values()) { addCondition(fieldName, group.values); } - } else if (values) { + } else { addCondition(fieldName, values); } } @@ -694,6 +694,33 @@ odoo.define("web/static/src/js/views/search_panel_model_extension.js", function return groupDomain; } + /** + * Returns whether the query informations should be considered as ready + * before or after having (re-)fetched the sections data. + * @private + * @param {Object} params + * @param {boolean} params.isInitialLoad + * @param {boolean} params.searchDomainChanged + * @returns {boolean} + */ + _shouldWaitForData({ isInitialLoad, searchDomainChanged }) { + if (isInitialLoad && Object.keys(this.defaultValues).length) { + // Default values need to be checked on initial load + return true; + } + if (this.categories.length && this.filters.some(hasDomain)) { + // Selected category value might affect the filter values + return true; + } + if (!this.searchDomain.length) { + // No search domain -> no need to check for expand + return false; + } + return [...this.state.sections.values()].some( + (section) => !section.expand && searchDomainChanged + ); + } + //--------------------------------------------------------------------- // Static //--------------------------------------------------------------------- diff --git a/addons/web/static/tests/chrome/action_manager_tests.js b/addons/web/static/tests/chrome/action_manager_tests.js index 971c5effb98..02c6fbda8e4 100644 --- a/addons/web/static/tests/chrome/action_manager_tests.js +++ b/addons/web/static/tests/chrome/action_manager_tests.js @@ -3591,9 +3591,6 @@ QUnit.module('ActionManager', { active_id: 1, active_ids: [1], }, - flags: { - searchPanelDefaultNoFilter: true, - }, }); var checkSessionStorage = false; var actionManager = await createActionManager({ diff --git a/addons/web/static/tests/views/search_panel_tests.js b/addons/web/static/tests/views/search_panel_tests.js index f9004d562c4..387cc8dc0e5 100644 --- a/addons/web/static/tests/views/search_panel_tests.js +++ b/addons/web/static/tests/views/search_panel_tests.js @@ -1,10 +1,8 @@ odoo.define("web/static/tests/views/search_panel_tests.js", function (require) { "use strict"; -const AbstractStorageService = require('web.AbstractStorageService'); const FormView = require('web.FormView'); const KanbanView = require('web.KanbanView'); -const RamStorage = require('web.RamStorage'); const testUtils = require('web.test_utils'); const SearchPanel = require("web/static/src/js/views/search_panel.js"); @@ -115,13 +113,6 @@ QUnit.module('Views', { `, }; - - var RamStorageService = AbstractStorageService.extend({ - storage: new RamStorage(), - }); - this.services = { - local_storage: RamStorageService, - }; }, }, function () { @@ -138,7 +129,6 @@ QUnit.module('Views', { assert.step((args.method || route) + (args.model ? (' on ' + args.model) : '')); return this._super.apply(this, arguments); }, - services: this.services, arch: ` @@ -198,7 +188,6 @@ QUnit.module('Views', { View: KanbanView, model: 'partner', data: this.data, - services: this.services, arch: ` @@ -237,7 +226,6 @@ QUnit.module('Views', { View: KanbanView, model: 'partner', data: this.data, - services: this.services, arch: ` @@ -279,7 +267,6 @@ QUnit.module('Views', { View: KanbanView, model: 'partner', data: this.data, - services: this.services, arch: ` @@ -320,7 +307,6 @@ QUnit.module('Views', { View: KanbanView, model: 'partner', data: this.data, - services: this.services, arch: ` @@ -368,7 +354,6 @@ QUnit.module('Views', { } return this._super.apply(this, arguments); }, - services: this.services, arch: ` @@ -434,7 +419,6 @@ QUnit.module('Views', { } return this._super.apply(this, arguments); }, - services: this.services, arch: ` @@ -486,132 +470,6 @@ QUnit.module('Views', { kanban.destroy(); }); - QUnit.test('store and retrieve active category value', async function (assert) { - assert.expect(9); - - var Storage = RamStorage.extend({ - getItem: function (key) { - assert.step(`getItem ${key}`); - const item = this._super(...arguments); - return item || 3; // default: 'asustek' - }, - setItem: function (key, value) { - assert.step(`setItem ${key} to ${value}`); - this._super(...arguments); - }, - }); - var RamStorageService = AbstractStorageService.extend({ - storage: new Storage(), - }); - - var expectedActiveId = 3; - var kanban = await createView({ - View: KanbanView, - model: 'partner', - data: this.data, - services: { - local_storage: RamStorageService, - }, - arch: ` - - - -
- -
- - - `, - archs: { - 'partner,false,search': ` - - - - - `, - }, - mockRPC: function (route, args) { - if (route === '/web/dataset/search_read') { - assert.deepEqual(args.domain, [['company_id', 'child_of', expectedActiveId]]); - } - return this._super.apply(this, arguments); - }, - }); - - // 'asustek' should be selected by default - assert.containsOnce(kanban, '.o_search_panel_category_value .active'); - assert.containsOnce(kanban, '.o_search_panel_category_value:nth(1) .active'); - assert.containsN(kanban, '.o_kanban_record:not(.o_kanban_ghost)', 2); - - // select 'agrolait' - expectedActiveId = 5; - await testUtils.dom.click(kanban.$('.o_search_panel_category_value:nth(2) header')); - - assert.verifySteps([ - 'getItem searchpanel_partner_company_id', // get on start - 'setItem searchpanel_partner_company_id to 5', - 'getItem searchpanel_partner_company_id', // get on reload - ]); - - kanban.destroy(); - }); - - QUnit.test('retrieved category value does not exist', async function (assert) { - assert.expect(6); - - var Storage = RamStorage.extend({ - getItem: function (key) { - assert.step('getItem ' + key); - return 343; // this value doesn't exist - }, - }); - var RamStorageService = AbstractStorageService.extend({ - storage: new Storage(), - }); - - var kanban = await createView({ - View: KanbanView, - model: 'partner', - data: this.data, - services: { - local_storage: RamStorageService, - }, - arch: ` - - - -
- -
- - - `, - archs: { - 'partner,false,search': ` - - - - - `, - }, - mockRPC: function (route, args) { - if (route === '/web/dataset/search_read') { - assert.deepEqual(args.domain, []); - } - return this._super.apply(this, arguments); - }, - }); - - // 'All' should be selected by default as the retrieved value does not exist - assert.containsOnce(kanban, '.o_search_panel_category_value .active'); - assert.containsOnce(kanban, '.o_search_panel_category_value:first .active'); - assert.containsN(kanban, '.o_kanban_record:not(.o_kanban_ghost)', 4); - - assert.verifySteps(['getItem searchpanel_partner_company_id']); - - kanban.destroy(); - }); - QUnit.test('category has been archived', async function (assert) { assert.expect(2); @@ -688,7 +546,6 @@ QUnit.module('Views', { } return this._super.apply(this, arguments); }, - services: this.services, arch: ` @@ -778,7 +635,6 @@ QUnit.module('Views', { } return this._super.apply(this, arguments); }, - services: this.services, arch: ` @@ -876,7 +732,6 @@ QUnit.module('Views', { } return this._super.apply(this, arguments); }, - services: this.services, arch: ` @@ -929,7 +784,6 @@ QUnit.module('Views', { View: KanbanView, model: 'partner', data: this.data, - services: this.services, arch: ` @@ -982,7 +836,6 @@ QUnit.module('Views', { View: KanbanView, model: 'partner', data: this.data, - services: this.services, arch: ` @@ -1034,7 +887,6 @@ QUnit.module('Views', { } return result; }, - services: this.services, arch: ` @@ -1104,6 +956,187 @@ QUnit.module('Views', { kanban.destroy(); }); + QUnit.test("concurrency: single category", async function (assert) { + assert.expect(12); + + let prom = testUtils.makeTestPromise(); + const kanbanPromise = createView({ + arch: ` + + +
+ +
+ + `, + archs: { + "partner,false,search": ` + + + + + + `, + }, + context: { + searchpanel_default_company_id: [5], + }, + data: this.data, + async mockRPC(route, args) { + const _super = this._super.bind(this); + if (route !== "/web/dataset/search_read") { + await prom; + } + assert.step(args.method || route); + return _super(...arguments); + }, + model: 'partner', + View: KanbanView, + }); + + // Case 1: search panel is awaited to build the query with search defaults + await testUtils.nextTick(); + assert.verifySteps([]); + + prom.resolve(); + const kanban = await kanbanPromise; + + assert.verifySteps([ + "search_panel_select_range", + "/web/dataset/search_read", + ]); + + // Case 2: search domain changed so we wait for the search panel once again + prom = testUtils.makeTestPromise(); + + await testUtils.controlPanel.toggleFilterMenu(kanban); + await testUtils.controlPanel.toggleMenuItem(kanban, 0); + + assert.verifySteps([]); + + prom.resolve(); + await testUtils.nextTick(); + + assert.verifySteps([ + "search_panel_select_range", + "/web/dataset/search_read", + ]); + + // Case 3: search domain is the same and default values do not matter anymore + prom = testUtils.makeTestPromise(); + + await testUtils.dom.click(kanban.$('.o_search_panel_category_value:nth(1) header')); + + // The search read is executed right away in this case + assert.verifySteps(["/web/dataset/search_read"]); + + prom.resolve(); + await testUtils.nextTick(); + + assert.verifySteps(["search_panel_select_range"]); + + kanban.destroy(); + }); + + QUnit.test("concurrency: category and filter", async function (assert) { + assert.expect(5); + + let prom = testUtils.makeTestPromise(); + const kanbanPromise = createView({ + arch: ` + + +
+ +
+ + `, + archs: { + "partner,false,search": ` + + + + + + `, + }, + data: this.data, + async mockRPC(route, args) { + const _super = this._super.bind(this); + if (route !== "/web/dataset/search_read") { + await prom; + } + assert.step(args.method || route); + return _super(...arguments); + }, + model: 'partner', + View: KanbanView, + }); + + await testUtils.nextTick(); + assert.verifySteps(["/web/dataset/search_read"]); + + prom.resolve(); + const kanban = await kanbanPromise; + + assert.verifySteps([ + "search_panel_select_range", + "search_panel_select_multi_range", + ]); + + kanban.destroy(); + }); + + QUnit.test("concurrency: category and filter with a domain", async function (assert) { + assert.expect(5); + + let prom = testUtils.makeTestPromise(); + const kanbanPromise = createView({ + arch: ` + + +
+ +
+ + `, + archs: { + "partner,false,search": ` + + + + + + `, + }, + data: this.data, + async mockRPC(route, args) { + const _super = this._super.bind(this); + if (route !== "/web/dataset/search_read") { + await prom; + } + assert.step(args.method || route); + return _super(...arguments); + }, + model: 'partner', + View: KanbanView, + }); + + await testUtils.nextTick(); + assert.verifySteps([]); + + prom.resolve(); + const kanban = await kanbanPromise; + + assert.verifySteps([ + "search_panel_select_range", + "search_panel_select_multi_range", + "/web/dataset/search_read", + ]); + + kanban.destroy(); + }); + QUnit.test('concurrency: misordered get_filters', async function (assert) { assert.expect(15); @@ -1119,7 +1152,6 @@ QUnit.module('Views', { } return result; }, - services: this.services, arch: ` @@ -1154,7 +1186,7 @@ QUnit.module('Views', { // 'All' should still be selected, and there should still be 4 records assert.containsOnce(kanban, '.o_search_panel_category_value .active'); assert.containsOnce(kanban, '.o_search_panel_category_value:nth(1) .active'); - assert.containsN(kanban, '.o_kanban_record:not(.o_kanban_ghost)', 4); + assert.containsN(kanban, '.o_kanban_record:not(.o_kanban_ghost)', 1); // select 'ghi' (delay the reload) def = testUtils.makeTestPromise(); @@ -1164,7 +1196,7 @@ QUnit.module('Views', { // 'All' should still be selected, and there should still be 4 records assert.containsOnce(kanban, '.o_search_panel_category_value .active'); assert.containsOnce(kanban, '.o_search_panel_category_value:nth(3) .active'); - assert.containsN(kanban, '.o_kanban_record:not(.o_kanban_ghost)', 4); + assert.containsN(kanban, '.o_kanban_record:not(.o_kanban_ghost)', 2); // unlock ghi search ghiDef.resolve(); @@ -1198,7 +1230,6 @@ QUnit.module('Views', { } return result; }, - services: this.services, arch: ` @@ -1264,7 +1295,6 @@ QUnit.module('Views', { } return result; }, - services: this.services, arch: ` @@ -1371,7 +1401,6 @@ QUnit.module('Views', { } return result; }, - services: this.services, arch: ` @@ -1463,7 +1492,6 @@ QUnit.module('Views', { assert.step(args.method || route); return this._super.apply(this, arguments); }, - services: this.services, arch: ` @@ -1533,7 +1561,6 @@ QUnit.module('Views', { assert.step(args.method || route); return this._super.apply(this, arguments); }, - services: this.services, arch: ` @@ -1606,7 +1633,6 @@ QUnit.module('Views', { } return this._super.apply(this, arguments); }, - services: this.services, arch: ` @@ -1711,7 +1737,6 @@ QUnit.module('Views', { } return this._super.apply(this, arguments); }, - services: this.services, arch: ` @@ -1823,7 +1848,6 @@ QUnit.module('Views', { } return result; }, - services: this.services, arch: ` @@ -1952,7 +1976,6 @@ QUnit.module('Views', { } return result; }, - services: this.services, arch: ` @@ -2002,7 +2025,6 @@ QUnit.module('Views', { } return result; }, - services: this.services, arch: ` @@ -2073,7 +2095,6 @@ QUnit.module('Views', { View: KanbanView, model: 'partner', data: this.data, - services: this.services, arch: ` @@ -2123,7 +2144,6 @@ QUnit.module('Views', { View: KanbanView, model: 'partner', data: this.data, - services: this.services, arch: ` @@ -2165,7 +2185,6 @@ QUnit.module('Views', { View: KanbanView, model: 'partner', data: this.data, - services: this.services, arch: ` @@ -2215,7 +2234,6 @@ QUnit.module('Views', { View: KanbanView, model: 'partner', data: this.data, - services: this.services, arch: `
@@ -2261,7 +2279,6 @@ QUnit.module('Views', { View: KanbanView, model: 'partner', data: this.data, - services: this.services, arch: ` @@ -2358,7 +2375,6 @@ QUnit.module('Views', { actions: this.actions, archs: this.archs, data: this.data, - services: this.services, mockRPC: function (route, args) { if (route === '/web/dataset/search_read') { assert.step(JSON.stringify(args.domain)); @@ -2407,7 +2423,6 @@ QUnit.module('Views', { actions: this.actions, archs: this.archs, data: this.data, - services: this.services, mockRPC: function (route, args) { if (route === '/web/dataset/search_read') { assert.step(JSON.stringify(args.domain)); @@ -2488,26 +2503,10 @@ QUnit.module('Views', { QUnit.test('after onExecuteAction, selects "All" as default category value', async function (assert) { assert.expect(4); - var Storage = RamStorage.extend({ - getItem: function (key) { - assert.step('getItem ' + key); - return 3; // 'asustek' - }, - setItem: function (key, value) { - assert.step('setItem ' + key + ' to ' + value); - }, - }); - var RamStorageService = AbstractStorageService.extend({ - storage: new Storage(), - }); - var actionManager = await createActionManager({ actions: this.actions, archs: this.archs, data: this.data, - services: { - local_storage: RamStorageService, - }, }); await actionManager.doAction(2); @@ -2530,7 +2529,6 @@ QUnit.module('Views', { actions: this.actions, archs: this.archs, data: this.data, - services: this.services, }); await actionManager.doAction(2); @@ -2549,7 +2547,6 @@ QUnit.module('Views', { actions: this.actions, archs: this.archs, data: this.data, - services: this.services, mockRPC: function (route, args) { assert.step(args.method || route); return this._super.apply(this, arguments); @@ -2587,7 +2584,6 @@ QUnit.module('Views', { actions: this.actions, archs: this.archs, data: this.data, - services: this.services, }); actionManager.$el.css('max-height', 300); @@ -2669,7 +2665,6 @@ QUnit.module('Views', { assert.step(args.method || route); return this._super.apply(this, arguments); }, - services: this.services, arch: ` @@ -2694,7 +2689,7 @@ QUnit.module('Views', { assert.verifySteps([ 'search_panel_select_range', "search_panel_select_multi_range", - '/web/dataset/search_read' + '/web/dataset/search_read', ]); assert.deepEqual(getCounters(kanban), [ @@ -2712,7 +2707,7 @@ QUnit.module('Views', { assert.verifySteps([ 'search_panel_select_range', "search_panel_select_multi_range", - '/web/dataset/search_read' + '/web/dataset/search_read', ]); kanban.destroy(); @@ -2745,7 +2740,6 @@ QUnit.module('Views', { }, data: this.data, model: 'partner', - services: this.services, View: KanbanView, }); @@ -2788,7 +2782,6 @@ QUnit.module('Views', { }, data: this.data, model: 'partner', - services: this.services, View: KanbanView, }); @@ -2831,7 +2824,6 @@ QUnit.module('Views', { }, data: this.data, model: 'partner', - services: this.services, View: KanbanView, }); @@ -2869,7 +2861,6 @@ QUnit.module('Views', { }, data: this.data, model: 'partner', - services: this.services, View: KanbanView, }); @@ -2907,7 +2898,6 @@ QUnit.module('Views', { }, data: this.data, model: 'partner', - services: this.services, View: KanbanView, }); @@ -2950,7 +2940,6 @@ QUnit.module('Views', { }, data: this.data, model: 'partner', - services: this.services, View: KanbanView, }); @@ -2993,7 +2982,6 @@ QUnit.module('Views', { }, data: this.data, model: 'partner', - services: this.services, View: KanbanView, }); @@ -3031,7 +3019,6 @@ QUnit.module('Views', { }, data: this.data, model: 'partner', - services: this.services, View: KanbanView, }); @@ -3067,7 +3054,6 @@ QUnit.module('Views', { }, data: this.data, model: 'partner', - services: this.services, View: KanbanView, }); @@ -3103,7 +3089,6 @@ QUnit.module('Views', { }, data: this.data, model: 'partner', - services: this.services, View: KanbanView, }); @@ -3139,7 +3124,6 @@ QUnit.module('Views', { }, data: this.data, model: 'partner', - services: this.services, View: KanbanView, }); @@ -3175,7 +3159,6 @@ QUnit.module('Views', { }, data: this.data, model: 'partner', - services: this.services, View: KanbanView, }); @@ -3211,7 +3194,6 @@ QUnit.module('Views', { }, data: this.data, model: 'partner', - services: this.services, View: KanbanView, }); @@ -3247,7 +3229,6 @@ QUnit.module('Views', { }, data: this.data, model: 'partner', - services: this.services, View: KanbanView, }); @@ -3283,7 +3264,6 @@ QUnit.module('Views', { }, data: this.data, model: 'partner', - services: this.services, View: KanbanView, }); @@ -3319,7 +3299,6 @@ QUnit.module('Views', { }, data: this.data, model: 'partner', - services: this.services, View: KanbanView, }); @@ -3355,7 +3334,6 @@ QUnit.module('Views', { }, data: this.data, model: 'partner', - services: this.services, View: KanbanView, }); @@ -3391,7 +3369,6 @@ QUnit.module('Views', { }, data: this.data, model: 'partner', - services: this.services, View: KanbanView, }); @@ -3427,7 +3404,6 @@ QUnit.module('Views', { }, data: this.data, model: 'partner', - services: this.services, View: KanbanView, }); @@ -3463,7 +3439,6 @@ QUnit.module('Views', { }, data: this.data, model: 'partner', - services: this.services, View: KanbanView, }); @@ -3499,7 +3474,6 @@ QUnit.module('Views', { }, data: this.data, model: 'partner', - services: this.services, View: KanbanView, }); @@ -3535,7 +3509,6 @@ QUnit.module('Views', { }, data: this.data, model: 'partner', - services: this.services, View: KanbanView, }); @@ -3571,7 +3544,6 @@ QUnit.module('Views', { }, data: this.data, model: 'partner', - services: this.services, View: KanbanView, }); @@ -3607,7 +3579,6 @@ QUnit.module('Views', { }, data: this.data, model: 'partner', - services: this.services, View: KanbanView, }); @@ -3643,7 +3614,6 @@ QUnit.module('Views', { }, data: this.data, model: 'partner', - services: this.services, View: KanbanView, }); @@ -3679,7 +3649,6 @@ QUnit.module('Views', { }, data: this.data, model: 'partner', - services: this.services, View: KanbanView, }); @@ -3715,7 +3684,6 @@ QUnit.module('Views', { }, data: this.data, model: 'partner', - services: this.services, View: KanbanView, }); @@ -3751,7 +3719,6 @@ QUnit.module('Views', { }, data: this.data, model: 'partner', - services: this.services, View: KanbanView, }); @@ -3787,7 +3754,6 @@ QUnit.module('Views', { }, data: this.data, model: 'partner', - services: this.services, View: KanbanView, }); @@ -3823,7 +3789,6 @@ QUnit.module('Views', { }, data: this.data, model: 'partner', - services: this.services, View: KanbanView, }); @@ -3859,7 +3824,6 @@ QUnit.module('Views', { }, data: this.data, model: 'partner', - services: this.services, View: KanbanView, }); @@ -3895,7 +3859,6 @@ QUnit.module('Views', { }, data: this.data, model: 'partner', - services: this.services, View: KanbanView, }); @@ -3936,7 +3899,6 @@ QUnit.module('Views', { }, data: this.data, model: 'partner', - services: this.services, View: KanbanView, }); @@ -3980,7 +3942,6 @@ QUnit.module('Views', { }, data: this.data, model: 'partner', - services: this.services, View: KanbanView, }); @@ -4018,7 +3979,6 @@ QUnit.module('Views', { }, data: this.data, model: 'partner', - services: this.services, View: KanbanView, }); @@ -4043,7 +4003,6 @@ QUnit.module('Views', { assert.step(args.method || route); return this._super.apply(this, arguments); }, - services: this.services, arch: ` @@ -4115,7 +4074,6 @@ QUnit.module('Views', { assert.step(args.method || route); return this._super.apply(this, arguments); }, - services: this.services, arch: `