[FIX] spreadsheet: autoresize pivot table

Before this revision, the pivot table was not autoresized when group bys
contains a many2one or a many2many field as it required an extra
request to get the name_get of the values.

With this revision, the values of the relational fields are extracted
from the pivot model in order to save an extra request.

Note: this fix is not ideal as it introduce a dependency between the
pivot model and the server data source. However, it is the best
solution in a stable version.

Task-id 2928601

closes odoo/odoo#108527

X-original-commit: d6a1aa63702132ffc59d2412c2a009912eba797a
Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com>
This commit is contained in:
Pierre Rousseau
2022-12-22 14:49:29 +01:00
parent 0fa9d984f3
commit 180ddd6da2
5 changed files with 281 additions and 34 deletions
@@ -0,0 +1,200 @@
/** @odoo-module */
import { Deferred } from "@web/core/utils/concurrency";
import { LoadingDataError } from "../o_spreadsheet/errors";
import BatchEndpoint, { Request } from "./server_data";
/**
* @typedef PendingDisplayName
* @property {"PENDING"} state
* @property {Deferred<string>} deferred
*
* @typedef ErrorDisplayName
* @property {"ERROR"} state
* @property {Deferred<string>} deferred
* @property {Error} error
*
* @typedef CompletedDisplayName
* @property {"COMPLETED"} state
* @property {Deferred<string>} deferred
* @property {string|undefined} value
*
* @typedef {PendingDisplayName | ErrorDisplayName | CompletedDisplayName} DisplayNameResult
*
* @typedef {[number, string]} BatchedNameGetRPCResult
*/
/**
* This class is responsible for fetching the display names of records. It
* caches the display names of records that have already been fetched.
* It also provides a way to wait for the display name of a record to be
* fetched.
*/
export class DisplayNameRepository {
/**
*
* @param {import("@web/core/orm_service").ORM} orm
* @param {Object} params
* @param {function} params.whenDataIsFetched Callback to call when the
* display name of a record is fetched.
*/
constructor(orm, { whenDataIsFetched }) {
this.dataFetchedCallback = whenDataIsFetched;
/**
* Contains the display names of records. It's organized in the following way:
* {
* "res.country": {
* 1: {
* "value": "Belgium",
* "deferred": Deferred<"Belgium">,
* },
* }
*/
/** @type {Object.<string, Object.<number, DisplayNameResult>>}*/
this._displayNames = {};
this._orm = orm;
this._endpoints = {};
}
/**
* Get the display name of the given record.
*
* @param {string} model
* @param {number} id
* @returns {Promise<string>}
*/
async getDisplayNameAsync(model, id) {
const displayNameResult = this._displayNames[model] && this._displayNames[model][id];
if (!displayNameResult) {
return this._fetchDisplayName(model, id);
}
return displayNameResult.deferred;
}
/**
* Set the display name of the given record. This will prevent the display name
* from being fetched in the background.
*
* @param {string} model
* @param {number} id
* @param {string} displayName
*/
setDisplayName(model, id, displayName) {
if (!this._displayNames[model]) {
this._displayNames[model] = {};
}
this._displayNames[model][id] = {
state: "COMPLETED",
deferred: new Deferred().resolve(displayName),
value: displayName,
};
}
/**
* Get the display name of the given record. If the record does not exist,
* it will throw a LoadingDataError and fetch the display name in the background.
*
* @param {string} model
* @param {number} id
* @returns {string}
*/
getDisplayName(model, id) {
const displayNameResult = this._displayNames[model] && this._displayNames[model][id];
if (!displayNameResult) {
// Catch the error to prevent the error from being thrown in the
// background.
this._fetchDisplayName(model, id).catch(() => {});
throw new LoadingDataError();
}
switch (displayNameResult.state) {
case "ERROR":
throw displayNameResult.error;
case "COMPLETED":
return displayNameResult.value;
default:
throw new LoadingDataError();
}
}
/**
* Get the batch endpoint for the given model. If it does not exist, it will
* be created.
*
* @param {string} model
* @returns {BatchEndpoint}
*/
_getEndpoint(model) {
if (!this._endpoints[model]) {
this._endpoints[model] = new BatchEndpoint(this._orm, model, "name_get", {
whenDataIsFetched: () => this.dataFetchedCallback(),
successCallback: this._assignResult.bind(this),
failureCallback: this._assignError.bind(this),
});
}
return this._endpoints[model];
}
/**
* This method is called when the display name of a record is successfully
* fetched. It updates the cache and resolves the deferred of the record.
*
* @param {Request} request
* @param {BatchedNameGetRPCResult} result
*
* @private
*/
_assignResult(request, result) {
const deferred = this._displayNames[request.resModel][request.args[0]].deferred;
deferred.resolve(result && result[1]);
this._displayNames[request.resModel][request.args[0]] = {
state: "COMPLETED",
deferred,
value: result && result[1],
};
}
/**
* This method is called when the display name of a record could not be
* fetched. It updates the cache and rejects the deferred of the record.
*
* @param {Request} request
* @param {Error} error
*
* @private
*/
_assignError(request, error) {
const deferred = this._displayNames[request.resModel][request.args[0]].deferred;
deferred.reject(error);
this._displayNames[request.resModel][request.args[0]] = {
state: "ERROR",
deferred,
error,
};
}
/**
* This method is called when the display name of a record is not in the
* cache. It creates a deferred and fetches the display name in the
* background.
*
* @param {string} model
* @param {number} id
*
* @private
* @returns {Deferred<string>}
*/
async _fetchDisplayName(model, id) {
const deferred = new Deferred();
if (!this._displayNames[model]) {
this._displayNames[model] = {};
}
this._displayNames[model][id] = {
state: "PENDING",
deferred,
};
const endpoint = this._getEndpoint(model);
const request = new Request(model, "name_get", [id]);
endpoint.call(request);
return deferred;
}
}
@@ -0,0 +1,54 @@
/** @odoo-module */
/**
* This class is responsible for keeping track of the labels of records. It
* caches the labels of records that have already been fetched.
* This class will not fetch the labels of records, it is the responsibility of
* the caller to fetch the labels and insert them in this repository.
*/
export class LabelsRepository {
constructor() {
/**
* Contains the labels of records. It's organized in the following way:
* {
* "crm.lead": {
* "city": {
* "bruxelles": "Bruxelles",
* }
* },
* }
*/
this._labels = {};
}
/**
* Get the label of a record.
* @param {string} model technical name of the model
* @param {string} field name of the field
* @param {any} value value of the field
*
* @returns {string|undefined} label of the record
*/
getLabel(model, field, value) {
return (
this._labels[model] && this._labels[model][field] && this._labels[model][field][value]
);
}
/**
* Set the label of a record.
* @param {string} model
* @param {string} field
* @param {string|number} value
* @param {string|undefined} label
*/
setLabel(model, field, value, label) {
if (!this._labels[model]) {
this._labels[model] = {};
}
if (!this._labels[model][field]) {
this._labels[model][field] = {};
}
this._labels[model][field][value] = label;
}
}
@@ -5,6 +5,8 @@ import { sprintf } from "@web/core/utils/strings";
import { ServerData } from "../data_sources/server_data";
import { LoadingDataError } from "../o_spreadsheet/errors";
import { DisplayNameRepository } from "./display_name_repository";
import { LabelsRepository } from "./labels_repository";
const { EventBus } = owl;
@@ -28,7 +30,7 @@ const { EventBus } = owl;
* for different entities that are defined on the same model.
*
* Implementation note:
* For the labels, when someone is asking for a label which is not loaded yet,
* For the labels, when someone is asking for a display name which is not loaded yet,
* the proxy returns directly (undefined) and a request for a name_get will
* be triggered. All the requests created are batched and send, with only one
* request per model, after a clock cycle.
@@ -38,21 +40,16 @@ export class MetadataRepository extends EventBus {
constructor(orm) {
super();
this.orm = orm.silent;
/**
* Contains the labels of records. It's organized in the following way:
* {
* "crm.lead": {
* "city": {
* "bruxelles": "Bruxelles",
* }
* },
* }
*/
this._labels = {};
this.serverData = new ServerData(this.orm, {
whenDataIsFetched: () => this.trigger("labels-fetched"),
});
this.labelsRepository = new LabelsRepository();
this.displayNameRepository = new DisplayNameRepository(this.orm, {
whenDataIsFetched: () => this.trigger("labels-fetched"),
});
}
/**
@@ -85,13 +82,7 @@ export class MetadataRepository extends EventBus {
* @param {string} label
*/
registerLabel(model, field, value, label) {
if (!this._labels[model]) {
this._labels[model] = {};
}
if (!this._labels[model][field]) {
this._labels[model][field] = {};
}
this._labels[model][field][value] = label;
this.labelsRepository.setLabel(model, field, value, label);
}
/**
@@ -103,9 +94,14 @@ export class MetadataRepository extends EventBus {
* @returns {string}
*/
getLabel(model, field, value) {
return (
this._labels[model] && this._labels[model][field] && this._labels[model][field][value]
);
return this.labelsRepository.getLabel(model, field, value);
}
/**
* Save the result of a name_get request in the cache
*/
setDisplayName(model, id, result) {
this.displayNameRepository.setDisplayName(model, id, result);
}
/**
@@ -119,8 +115,7 @@ export class MetadataRepository extends EventBus {
*/
getRecordDisplayName(model, id) {
try {
const result = this.serverData.batch.get(model, "name_get", id);
return result[1];
return this.displayNameRepository.getDisplayName(model, id);
} catch (e) {
if (e instanceof LoadingDataError) {
throw e;
@@ -414,6 +414,12 @@ export class SpreadsheetPivotModel extends PivotModel {
group.values[i],
group.labels[i]
);
} else {
metadataRepository.setDisplayName(
field.relation,
group.values[i],
group.labels[i]
);
}
}
}
@@ -396,7 +396,6 @@ QUnit.module("spreadsheet > pivot plugin", {}, () => {
QUnit.test("display loading while data is not fully available", async function (assert) {
const metadataPromise = makeDeferred();
const dataPromise = makeDeferred();
const namePromise = makeDeferred();
const spreadsheetData = {
sheets: [
{
@@ -437,8 +436,7 @@ QUnit.module("spreadsheet > pivot plugin", {}, () => {
await dataPromise;
}
if (model === "product" && method === "name_get") {
assert.step(`${model}/${method}`);
await namePromise;
assert.ok(false, "should not be called because data is put in cache");
}
return result;
},
@@ -456,15 +454,9 @@ QUnit.module("spreadsheet > pivot plugin", {}, () => {
await nextTick();
setCellContent(model, "A10", "2");
assert.strictEqual(getCellValue(model, "A1"), "Probability");
assert.strictEqual(getCellValue(model, "A2"), "Loading...");
assert.strictEqual(getCellValue(model, "A3"), 131);
namePromise.resolve();
await nextTick();
setCellContent(model, "A10", "3");
assert.strictEqual(getCellValue(model, "A1"), "Probability");
assert.strictEqual(getCellValue(model, "A2"), "xphone");
assert.strictEqual(getCellValue(model, "A3"), 131);
assert.verifySteps(["partner/fields_get", "partner/read_group", "product/name_get"]);
assert.verifySteps(["partner/fields_get", "partner/read_group"]);
});
QUnit.test("relational PIVOT.HEADER with missing id", async function (assert) {