[FIX] spreadsheet: preload list record limit
Steps to reproduce: - Insert a list view in a spreadsheet - Cut & Paste the entire list to a new sheet - On the first sheet, insert a formula, `=SUM()` which sum many records of the list - Click on `Clear history` in the file menu (to force a snapshot and a reload) => There are one RPC by number of records in the SUM function, with the limit increased by 1 each time. The record limit (the bigger record index) is computed during the evaluation. With a normal case, the evaluation goes through all the cells of the viewport, collect the biggest record index, and trigger the rpc after that. However, in our usecase, the evaluation is not passed on the second sheet (the one which contains the list view), so the limit is not computed. When we evaluate the arguments of the `SUM` function, we evaluate each argument one by one. So we evaluate a `ODOO.LIST` formula, the limit is lower than the previous limit, so the function returns an error. But when evaluating the arguments of a function, we stop the evaluation as soon as we find an error. So the evaluation of the `SUM` function is stopped, and the limit is not entirely computed. So the first RPC will have a limit 1, then the evaluation is re-run, a second RPC is triggered with a limit 2, and so on. This commit fixes the issue by preloading the limit during the import of the data and during an UPDATE_CELL. opw-3165458 closes odoo/odoo#113789 X-original-commit: 1c5a05198542b3a40c97443b2a9ecf5fb32eb5f2 Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com> Signed-off-by: Pierre Rousseau (pro) <pro@odoo.com>
This commit is contained in:
Pierre Rousseau
@@ -35,13 +35,22 @@ export default class ListDataSource extends OdooViewsDataSource {
|
||||
*/
|
||||
constructor(services, params) {
|
||||
super(services, params);
|
||||
this.limit = params.limit;
|
||||
this.maxPosition = params.limit;
|
||||
this.maxPositionFetched = 0;
|
||||
this.data = [];
|
||||
}
|
||||
|
||||
/**
|
||||
* Increase the max position of the list
|
||||
* @param {number} position
|
||||
*/
|
||||
increaseMaxPosition(position) {
|
||||
this.maxPosition = Math.max(this.maxPosition, position);
|
||||
}
|
||||
|
||||
async _load() {
|
||||
await super._load();
|
||||
if (this.limit === 0) {
|
||||
if (this.maxPosition === 0) {
|
||||
this.data = [];
|
||||
return;
|
||||
}
|
||||
@@ -52,10 +61,11 @@ export default class ListDataSource extends OdooViewsDataSource {
|
||||
this._getFieldsToFetch(),
|
||||
{
|
||||
order: orderByToString(orderBy),
|
||||
limit: this.limit,
|
||||
limit: this.maxPosition,
|
||||
context,
|
||||
}
|
||||
);
|
||||
this.maxPositionFetched = this.maxPosition;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -99,8 +109,8 @@ export default class ListDataSource extends OdooViewsDataSource {
|
||||
*/
|
||||
getListCellValue(position, fieldName) {
|
||||
this._assertDataIsLoaded();
|
||||
if (position >= this.limit) {
|
||||
this.limit = position + 1;
|
||||
if (position >= this.maxPositionFetched) {
|
||||
this.increaseMaxPosition(position + 1);
|
||||
// A reload is needed because the asked position is not already loaded.
|
||||
this._triggerFetching();
|
||||
throw new LoadingDataError();
|
||||
|
||||
@@ -9,6 +9,7 @@ import { _t } from "@web/core/l10n/translation";
|
||||
import { globalFiltersFieldMatchers } from "@spreadsheet/global_filters/plugins/global_filters_core_plugin";
|
||||
import { sprintf } from "@web/core/utils/strings";
|
||||
import { checkFilterFieldMatching } from "@spreadsheet/global_filters/helpers";
|
||||
import { getFirstListFunction, getNumberOfListFormulas } from "../list_helpers";
|
||||
|
||||
/**
|
||||
* @typedef {Object} ListDefinition
|
||||
@@ -143,9 +144,51 @@ export default class ListCorePlugin extends CorePlugin {
|
||||
case "REMOVE_GLOBAL_FILTER":
|
||||
this._onFilterDeletion(cmd.id);
|
||||
break;
|
||||
|
||||
case "START":
|
||||
for (const sheetId of this.getters.getSheetIds()) {
|
||||
const cells = this.getters.getCells(sheetId);
|
||||
for (const cell of Object.values(cells)) {
|
||||
if (cell.isFormula) {
|
||||
this._addListPositionToDataSource(cell.content);
|
||||
}
|
||||
}
|
||||
}
|
||||
break;
|
||||
case "UPDATE_CELL":
|
||||
if (cmd.content) {
|
||||
this._addListPositionToDataSource(cmd.content);
|
||||
}
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Extract the position of the records asked in the given formula and
|
||||
* increase the max position of the corresponding data source.
|
||||
*
|
||||
* @param {string} content Odoo list formula
|
||||
*/
|
||||
_addListPositionToDataSource(content) {
|
||||
if (getNumberOfListFormulas(content) !== 1) {
|
||||
return;
|
||||
}
|
||||
const { functionName, args } = getFirstListFunction(content);
|
||||
if (functionName !== "ODOO.LIST") {
|
||||
return;
|
||||
}
|
||||
const [listId, positionArg] = args.map((arg) => arg.value.toString());
|
||||
if (!(listId in this.lists)) {
|
||||
return;
|
||||
}
|
||||
const position = parseInt(positionArg, 10);
|
||||
if (isNaN(position)) {
|
||||
return;
|
||||
}
|
||||
const dataSourceId = this.lists[listId].dataSourceId;
|
||||
this.dataSources.get(dataSourceId).increaseMaxPosition(position);
|
||||
}
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
// Getters
|
||||
// -------------------------------------------------------------------------
|
||||
|
||||
@@ -137,7 +137,7 @@ QUnit.module("spreadsheet > list plugin", {}, () => {
|
||||
const { model } = await createSpreadsheetWithList({ linesNumber: 2 });
|
||||
const [listId] = model.getters.getListIds();
|
||||
const dataSource = model.getters.getListDataSource(listId);
|
||||
assert.strictEqual(dataSource.limit, 2);
|
||||
assert.strictEqual(dataSource.maxPosition, 2);
|
||||
});
|
||||
|
||||
QUnit.test("can select a List from cell formula within a formula", async function (assert) {
|
||||
@@ -246,13 +246,9 @@ QUnit.module("spreadsheet > list plugin", {}, () => {
|
||||
model.dispatch("ACTIVATE_SHEET", { sheetIdFrom: "sheet1", sheetIdTo: "sheet2" });
|
||||
/*
|
||||
* Ask a first time the value => It will trigger a loading of the data source.
|
||||
* As the list index limit is to 0, there is loading.
|
||||
*/
|
||||
assert.equal(getCellValue(model, "A1"), "Loading...");
|
||||
await nextTick();
|
||||
// Ask a second time the value => The list index limit is raised => reload
|
||||
assert.equal(getCellValue(model, "A1"), "Loading...");
|
||||
await nextTick();
|
||||
assert.equal(getCellValue(model, "A1"), 12);
|
||||
assert.verifySteps(["partner/fields_get", "partner/search_read"]);
|
||||
});
|
||||
@@ -508,6 +504,42 @@ QUnit.module("spreadsheet > list plugin", {}, () => {
|
||||
});
|
||||
});
|
||||
|
||||
QUnit.test(
|
||||
"List record limit is computed during the import and UPDATE_CELL",
|
||||
async function (assert) {
|
||||
const spreadsheetData = {
|
||||
sheets: [
|
||||
{
|
||||
id: "sheet1",
|
||||
cells: {
|
||||
A1: { content: `=ODOO.LIST("1", "1", "foo")` },
|
||||
},
|
||||
},
|
||||
],
|
||||
lists: {
|
||||
1: {
|
||||
id: 1,
|
||||
columns: ["foo", "contact_name"],
|
||||
domain: [],
|
||||
model: "partner",
|
||||
orderBy: [],
|
||||
context: {},
|
||||
},
|
||||
},
|
||||
};
|
||||
const model = await createModelWithDataSource({ spreadsheetData });
|
||||
const ds = model.getters.getListDataSource("1");
|
||||
assert.strictEqual(ds.maxPosition, 1);
|
||||
assert.strictEqual(ds.maxPositionFetched, 0);
|
||||
setCellContent(model, "A1", `=ODOO.LIST("1", "42", "foo", 2)`);
|
||||
assert.strictEqual(ds.maxPosition, 42);
|
||||
assert.strictEqual(ds.maxPositionFetched, 0);
|
||||
await waitForDataSourcesLoaded(model);
|
||||
assert.strictEqual(ds.maxPosition, 42);
|
||||
assert.strictEqual(ds.maxPositionFetched, 42);
|
||||
}
|
||||
);
|
||||
|
||||
QUnit.test(
|
||||
"Load list spreadsheet with models that cannot be accessed",
|
||||
async function (assert) {
|
||||
|
||||
Reference in New Issue
Block a user