pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[PERF] spreadsheet: use web_search_read to load list currency

Browse Source 
With this commit, list data is loaded using `web_search_read`
instead of `search_read`.

The goal is to fetch the currency (symbol, decimal places, etc.) of monetary
fields in a single request, instead of 2 RPCs.

Pros:
- less code
- one evaluation saved
- one network request saved
- easier future refactoring (see below)

Cons:
- overhead of data transferred over network (from 4.5MB to 6.5MB, unzipped
  and from 711kB to 725kB gzipped to fetch a list of 20K crm leads).

Before this commit, here is what it looked like:

1. the list data is fetch (with the currency_field)
2. the cells are evaluated with the new data
3. we realize we want to format a currency amount. We already have the
   currency name but not the symbol, etc. So we fetch the currency data
4. evaluate the cells again with the new currency format

Now:
1. fetch the list data with everything we need for the currency
2. evaluate the cells

This commit also serves another goal for a future refactoring: in the hope
of avoiding throwing "loading errors", I'd like to have an easy way to know
if a data source is fully loaded or not (the data and the format).
With this commit, everything is centralized in the list data source with
a single RPC. The goal is therefore achieved with this commit.

closes odoo/odoo#153434

Task: 3730232
Related: odoo/enterprise#56253
Signed-off-by: Pierre Rousseau (pro) <pro@odoo.com>
This commit is contained in:
Lucas Lefèvre (lul)
2024-02-20 18:22:03 +00:00
parent 6d2daa0013
commit 8777973edd
8 changed files with 91 additions and 90 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/spreadsheet/models/res_currency.py
+1
View File
@@ -4,6 +4,7 @@ from odoo import api, models
class ResCurrency(models.Model):
_inherit = "res.currency"
# TODO remove this method in master. It's not used anymore.
@api.model
def get_currencies_for_spreadsheet(self, currency_names):
"""
addons/spreadsheet/static/src/currency/currency_data_source.js
-13
View File
@@ -53,17 +53,4 @@ export class CurrencyDataSource {
}
return result;
}
/**
* Get all currencies from the server
* @param {string} currencyName
* @returns {Currency}
*/
getCurrency(currencyName) {
return this.serverData.batch.get(
"res.currency",
"get_currencies_for_spreadsheet",
currencyName
);
}
}
addons/spreadsheet/static/src/currency/plugins/currency.js
+5 -17
View File
@@ -39,10 +39,7 @@ class CurrencyPlugin extends UIPlugin {
}
/**
*
* @param {Currency | undefined} currency
* @private
*
* @returns {string | undefined}
*/
computeFormatFromCurrency(currency) {
@@ -56,19 +53,6 @@ class CurrencyPlugin extends UIPlugin {
});
}
/**
* Returns the default display format of a given currency
* @param {string} currencyName
* @returns {string | undefined}
*/
getCurrencyFormat(currencyName) {
const currency =
currencyName &&
this.dataSources &&
this.dataSources.get(DATA_SOURCE_ID).getCurrency(currencyName);
return this.computeFormatFromCurrency(currency);
}
/**
* Returns the default display format of a the company currency
* @param {number|undefined} companyId
@@ -85,6 +69,10 @@ class CurrencyPlugin extends UIPlugin {
}
}
CurrencyPlugin.getters = ["getCurrencyRate", "getCurrencyFormat", "getCompanyCurrencyFormat"];
CurrencyPlugin.getters = [
"getCurrencyRate",
"computeFormatFromCurrency",
"getCompanyCurrencyFormat",
];
featurePluginRegistry.add("odooCurrency", CurrencyPlugin);
addons/spreadsheet/static/src/list/list_data_source.js
+53 -18
View File
@@ -62,16 +62,13 @@ export class ListDataSource extends OdooViewsDataSource {
return;
}
const { domain, orderBy, context } = this._searchParams;
this.data = await this._orm.searchRead(
this._metaData.resModel,
domain,
this._getFieldsToFetch(),
{
order: orderByToString(orderBy),
limit: this.maxPosition,
context,
}
);
const { records } = await this._orm.webSearchRead(this._metaData.resModel, domain, {
specification: this._getReadSpec(),
order: orderByToString(orderBy),
limit: this.maxPosition,
context,
});
this.data = records;
this.maxPositionFetched = this.maxPosition;
}
@@ -79,14 +76,33 @@ export class ListDataSource extends OdooViewsDataSource {
* Get the fields to fetch from the server.
* Automatically add the currency field if the field is a monetary field.
*/
_getFieldsToFetch() {
const fields = this._metaData.columns.filter((f) => this.getField(f));
_getReadSpec() {
const spec = {};
const fields = this._metaData.columns.map((f) => this.getField(f)).filter(Boolean);
for (const field of fields) {
if (this.getField(field).type === "monetary") {
fields.push(this.getField(field).currency_field);
switch (field.type) {
case "monetary":
spec[field.name] = {};
spec[field.currency_field] = {
fields: {
name: {}, // currency code
symbol: {},
decimal_places: {},
position: {},
},
};
break;
case "many2one":
case "many2many":
case "one2many":
spec[field.name] = { fields: { display_name: {} } };
break;
default:
spec[field.name] = field;
break;
}
}
return fields;
return spec;
}
/**
@@ -143,12 +159,12 @@ export class ListDataSource extends OdooViewsDataSource {
}
switch (field.type) {
case "many2one":
return record[fieldName].length === 2 ? record[fieldName][1] : "";
return record[fieldName].display_name ?? "";
case "one2many":
case "many2many": {
const labels = record[fieldName]
.map((id) => this._metadataRepository.getRecordDisplayName(field.relation, id))
.filter((value) => value !== undefined);
.map(({ display_name }) => display_name)
.filter((displayName) => displayName !== undefined);
return labels.join(", ");
}
case "selection": {
@@ -177,6 +193,25 @@ export class ListDataSource extends OdooViewsDataSource {
}
}
/**
* @param {number} position
* @param {string} currencyFieldName
* @returns {import("@spreadsheet/currency/currency_data_source").Currency | undefined}
*/
getListCurrency(position, currencyFieldName) {
this._assertDataIsLoaded();
const currency = this.data[position]?.[currencyFieldName];
if (!currency) {
return undefined;
}
return {
code: currency.name,
symbol: currency.symbol,
decimalPlaces: currency.decimal_places,
position: currency.position,
};
}
//--------------------------------------------------------------------------
// Private
//--------------------------------------------------------------------------
addons/spreadsheet/static/src/list/list_functions.js
+5 -6
View File
@@ -42,12 +42,11 @@ const ODOO_LIST = {
case "float":
return "#,##0.00";
case "monetary": {
const currencyName = this.getters.getListCellValue(
id,
position,
field.currency_field
);
return this.getters.getCurrencyFormat(currencyName);
const currency = this.getters.getListCurrency(id, position, field.currency_field);
if (!currency) {
return "#,##0.00";
}
return this.getters.computeFormatFromCurrency(currency);
}
case "date":
return this.locale.dateFormat;
addons/spreadsheet/static/src/list/plugins/list_ui_plugin.js
+5
View File
@@ -298,6 +298,10 @@ export class ListUIPlugin extends spreadsheet.UIPlugin {
return this.getters.getListDataSource(listId).getListCellValue(position, fieldName);
}
getListCurrency(listId, position, fieldName) {
return this.getters.getListDataSource(listId).getListCurrency(position, fieldName);
}
/**
* Get the currently selected list id
* @returns {number|undefined} Id of the list, undefined if no one is selected
@@ -338,6 +342,7 @@ export class ListUIPlugin extends spreadsheet.UIPlugin {
ListUIPlugin.getters = [
"getListComputedDomain",
"getListCurrency",
"getListHeaderValue",
"getListIdFromPosition",
"getListCellValue",
addons/spreadsheet/static/tests/lists/list_plugin_test.js
+22 -19
View File
@@ -228,18 +228,13 @@ QUnit.module("spreadsheet > list plugin", {}, () => {
mockRPC: async function (route, args, performRPC) {
if (
spreadsheetLoaded &&
args.method === "search_read" &&
args.method === "web_search_read" &&
args.model === "partner" &&
args.kwargs.fields &&
args.kwargs.fields.includes(forbiddenFieldName)
args.kwargs.specification[forbiddenFieldName]
) {
// We should not go through this condition if the forbidden fields is properly filtered
assert.ok(false, `${forbiddenFieldName} should have been ignored`);
}
if (this) {
// @ts-ignore
return this._super.apply(this, arguments);
}
},
});
const listId = model.getters.getListIds()[0];
@@ -294,7 +289,7 @@ QUnit.module("spreadsheet > list plugin", {}, () => {
assert.equal(getCellValue(model, "A1"), "Loading...");
await nextTick();
assert.equal(getCellValue(model, "A1"), 12);
assert.verifySteps(["partner/fields_get", "partner/search_read"]);
assert.verifySteps(["partner/fields_get", "partner/web_search_read"]);
});
QUnit.test("user context is combined with list context to fetch data", async function (assert) {
@@ -358,8 +353,8 @@ QUnit.module("spreadsheet > list plugin", {}, () => {
return;
}
switch (method) {
case "search_read":
assert.step("search_read");
case "web_search_read":
assert.step("web_search_read");
assert.deepEqual(
kwargs.context,
expectedFetchContext,
@@ -370,7 +365,7 @@ QUnit.module("spreadsheet > list plugin", {}, () => {
},
});
await waitForDataSourcesLoaded(model);
assert.verifySteps(["search_read"]);
assert.verifySteps(["web_search_read"]);
});
QUnit.test("rename list with empty name is refused", async (assert) => {
@@ -537,10 +532,18 @@ QUnit.module("spreadsheet > list plugin", {}, () => {
await createSpreadsheetWithList({
columns: ["pognon"],
mockRPC: async function (route, args, performRPC) {
if (args.method === "search_read" && args.model === "partner") {
assert.strictEqual(args.kwargs.fields.length, 2);
assert.strictEqual(args.kwargs.fields[0], "pognon");
assert.strictEqual(args.kwargs.fields[1], "currency_id");
if (args.method === "web_search_read" && args.model === "partner") {
const spec = args.kwargs.specification;
assert.strictEqual(Object.keys(spec).length, 2);
assert.deepEqual(spec.currency_id, {
fields: {
name: {},
symbol: {},
decimal_places: {},
position: {},
},
});
assert.deepEqual(spec.pognon, {});
}
},
});
@@ -598,9 +601,9 @@ QUnit.module("spreadsheet > list plugin", {}, () => {
const model = await createModelWithDataSource({
spreadsheetData,
mockRPC: function (route, args) {
if (args.method === "search_read") {
if (args.method === "web_search_read") {
assert.deepEqual(args.kwargs.domain, [["foo", "=", uid]]);
assert.step("search_read");
assert.step("web_search_read");
}
},
});
@@ -611,7 +614,7 @@ QUnit.module("spreadsheet > list plugin", {}, () => {
'[("foo", "=", uid)]',
"the domain is exported with the dynamic parts"
);
assert.verifySteps(["search_read"]);
assert.verifySteps(["web_search_read"]);
});
QUnit.test(
@@ -622,7 +625,7 @@ QUnit.module("spreadsheet > list plugin", {}, () => {
mockRPC: async function (route, args) {
if (
args.model === "partner" &&
args.method === "search_read" &&
args.method === "web_search_read" &&
!hasAccessRights
) {
throw makeServerError({ description: "ya done!" });
addons/spreadsheet/static/tests/utils/mock_server.js
-17
View File
@@ -4,23 +4,6 @@ import { registry } from "@web/core/registry";
registry
.category("mock_server")
.add("res.currency/get_currencies_for_spreadsheet", function (route, args) {
const currencyNames = args.args[0];
const result = [];
for (const currencyName of currencyNames) {
const curr = this.models["res.currency"].records.find(
(curr) => curr.name === currencyName
);
result.push({
code: curr.name,
symbol: curr.symbol,
decimalPlaces: curr.decimal_places || 2,
position: curr.position || "after",
});
}
return result;
})
.add("res.currency/get_company_currency_for_spreadsheet", function (route, args) {
return {
code: "EUR",