diff --git a/addons/spreadsheet/static/src/data_sources/data_source.js b/addons/spreadsheet/static/src/data_sources/data_source.js index adc580f80d2..68dfe64175f 100644 --- a/addons/spreadsheet/static/src/data_sources/data_source.js +++ b/addons/spreadsheet/static/src/data_sources/data_source.js @@ -15,10 +15,11 @@ import { KeepLast } from "@web/core/utils/concurrency"; * particular data. */ export class LoadableDataSource { - constructor(services) { - this._orm = services.orm; - this._metadataRepository = services.metadataRepository; - this._notify = services.notify; + constructor(params) { + this._orm = params.orm; + this._metadataRepository = params.metadataRepository; + this._notifyWhenPromiseResolves = params.notifyWhenPromiseResolves; + this._cancelPromise = params.cancelPromise; /** * Last time that this dataSource has been updated @@ -44,6 +45,7 @@ export class LoadableDataSource { */ async load(params) { if (params && params.reload) { + this._cancelPromise(this._loadPromise); this._loadPromise = undefined; } if (!this._loadPromise) { @@ -59,8 +61,8 @@ export class LoadableDataSource { .finally(() => { this._lastUpdate = Date.now(); this._isFullyLoaded = true; - this._notify(); }); + await this._notifyWhenPromiseResolves(this._loadPromise); } return this._loadPromise; } diff --git a/addons/spreadsheet/static/src/data_sources/data_sources.js b/addons/spreadsheet/static/src/data_sources/data_sources.js index a98de615c12..b9d0633adb9 100644 --- a/addons/spreadsheet/static/src/data_sources/data_sources.js +++ b/addons/spreadsheet/static/src/data_sources/data_sources.js @@ -25,6 +25,7 @@ export class DataSources extends EventBus { this._metadataRepository.addEventListener("labels-fetched", () => this.notify()); /** @type {Object.} */ this._dataSources = {}; + this.pendingPromises = new Set(); } /** @@ -41,6 +42,8 @@ export class DataSources extends EventBus { orm: this._orm, metadataRepository: this._metadataRepository, notify: () => this.notify(), + notifyWhenPromiseResolves: this.notifyWhenPromiseResolves.bind(this), + cancelPromise: (promise) => this.pendingPromises.delete(promise), }, params ); @@ -89,11 +92,41 @@ export class DataSources extends EventBus { return id in this._dataSources; } + /** + * @private + * @param {Promise} promise + */ + async notifyWhenPromiseResolves(promise) { + this.pendingPromises.add(promise); + await promise + .then(() => { + this.pendingPromises.delete(promise); + this.notify(); + }) + .catch(() => { + this.pendingPromises.delete(promise); + this.notify(); + }); + } + /** * Notify that a data source has been updated. Could be useful to * request a re-evaluation. */ notify() { + if (this.pendingPromises.size) { + if (!this.nextTriggerTimeOutId) { + // evaluates at least every 10 seconds, even if there are pending promises + // to avoid blocking everything if there is a really long request + this.nextTriggerTimeOutId = setTimeout(() => { + this.nextTriggerTimeOutId = undefined; + if (this.pendingPromises.size) { + this.trigger("data-source-updated"); + } + }, 10000); + } + return; + } this.trigger("data-source-updated"); } diff --git a/addons/spreadsheet/static/tests/data_fetching/data_source_test.js b/addons/spreadsheet/static/tests/data_fetching/data_source_test.js index 9e44f3babf2..32adb9e8af8 100644 --- a/addons/spreadsheet/static/tests/data_fetching/data_source_test.js +++ b/addons/spreadsheet/static/tests/data_fetching/data_source_test.js @@ -31,17 +31,22 @@ QUnit.module("spreadsheet data source", {}, () => { } } const dataSource = new TestDataSource({ - notify: () => {}, + notify: () => assert.step("notify"), + notifyWhenPromiseResolves: () => assert.step("notify-from-promise"), + cancelPromise: () => assert.step("cancel-promise"), }); dataSource.load(); + assert.verifySteps(["notify-from-promise"]); dataSource.load({ reload: true }); assert.strictEqual(dataSource.isReady(), false); def1.resolve(); await nextTick(); + assert.verifySteps(["cancel-promise", "notify-from-promise"]); assert.strictEqual(dataSource.isReady(), false); def2.resolve(); await nextTick(); assert.strictEqual(dataSource.isReady(), true); + assert.verifySteps([]); } ); @@ -57,7 +62,9 @@ QUnit.module("spreadsheet data source", {}, () => { } const dataSource = new TestDataSource({ - notify: () => {}, + notify: () => assert.step("notify"), + notifyWhenPromiseResolves: () => assert.step("notify-from-promise"), + cancelPromise: () => assert.step("cancel-promise"), orm: { call: () => { throw makeServerError({ description: "Ya done!" }); @@ -65,6 +72,7 @@ QUnit.module("spreadsheet data source", {}, () => { }, }); await dataSource.load(); + assert.verifySteps(["notify-from-promise"]); assert.ok(dataSource._isFullyLoaded); assert.notOk(dataSource._isValid); assert.equal(dataSource._loadErrorMessage, "Ya done!"); diff --git a/addons/spreadsheet/static/tests/pivots/model/pivot_plugin_test.js b/addons/spreadsheet/static/tests/pivots/model/pivot_plugin_test.js index c483f664d44..a53534711ae 100644 --- a/addons/spreadsheet/static/tests/pivots/model/pivot_plugin_test.js +++ b/addons/spreadsheet/static/tests/pivots/model/pivot_plugin_test.js @@ -388,6 +388,70 @@ QUnit.module("spreadsheet > pivot plugin", {}, () => { assert.equal(getCellValue(model, "A1"), 131); }); + QUnit.test("evaluates only once when two pivots are loading", async function (assert) { + const spreadsheetData = { + sheets: [{ id: "sheet1" }], + pivots: { + 1: { + id: 1, + colGroupBys: ["foo"], + domain: [], + measures: [{ field: "probability", operator: "avg" }], + model: "partner", + rowGroupBys: ["bar"], + }, + 2: { + id: 2, + colGroupBys: ["foo"], + domain: [], + measures: [{ field: "probability", operator: "avg" }], + model: "partner", + rowGroupBys: ["bar"], + }, + }, + }; + const model = await createModelWithDataSource({ + spreadsheetData, + }); + model.config.custom.dataSources.addEventListener("data-source-updated", () => + assert.step("data-source-notified") + ); + setCellContent(model, "A1", '=ODOO.PIVOT("1", "probability")'); + setCellContent(model, "A2", '=ODOO.PIVOT("2", "probability")'); + assert.equal(getCellValue(model, "A1"), "Loading..."); + assert.equal(getCellValue(model, "A2"), "Loading..."); + await nextTick(); + assert.equal(getCellValue(model, "A1"), 131); + assert.equal(getCellValue(model, "A2"), 131); + assert.verifySteps(["data-source-notified"], "evaluation after both pivots are loaded"); + }); + + QUnit.test("concurrently load the same pivot twice", async function (assert) { + const spreadsheetData = { + sheets: [{ id: "sheet1" }], + pivots: { + 1: { + id: 1, + colGroupBys: ["foo"], + domain: [], + measures: [{ field: "probability", operator: "avg" }], + model: "partner", + rowGroupBys: ["bar"], + }, + }, + }; + const model = await createModelWithDataSource({ + spreadsheetData, + }); + // the data loads first here, when we insert the first pivot function + setCellContent(model, "A1", '=ODOO.PIVOT("1", "probability")'); + assert.equal(getCellValue(model, "A1"), "Loading..."); + // concurrently reload the same pivot + model.dispatch("REFRESH_PIVOT", { id: 1 }); + await nextTick(); + assert.equal(getCellValue(model, "A1"), 131); + }); + QUnit.test("display loading while data is not fully available", async function (assert) { const metadataPromise = makeDeferred(); const dataPromise = makeDeferred();