[PERF] spreadsheet: use web_search_read to load list currency

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>

addons/spreadsheet/models/res_currency.py

@@ -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

@@ -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

@@ -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

@@ -62,16 +62,13 @@ export class ListDataSource extends OdooViewsDataSource {
             return;
         }
         const { domain, orderBy, context } = this._searchParams;
-        this.data = await this._orm.searchRead(
+        const { records } = await this._orm.webSearchRead(this._metaData.resModel, domain, {
-            this._metaData.resModel,
+            specification: this._getReadSpec(),
-            domain,
+            order: orderByToString(orderBy),
-            this._getFieldsToFetch(),
+            limit: this.maxPosition,
-            {
+            context,
-                order: orderByToString(orderBy),
+        });
-                limit: this.maxPosition,
+        this.data = records;
-                context,
-            }
-        );
         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() {
+    _getReadSpec() {
-        const fields = this._metaData.columns.filter((f) => this.getField(f));
+        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") {
+            switch (field.type) {
-                fields.push(this.getField(field).currency_field);
+                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))
+                    .map(({ display_name }) => display_name)
-                    .filter((value) => value !== undefined);
+                    .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

@@ -42,12 +42,11 @@ const ODOO_LIST = {
             case "float":
                 return "#,##0.00";
             case "monetary": {
-                const currencyName = this.getters.getListCellValue(
+                const currency = this.getters.getListCurrency(id, position, field.currency_field);
-                    id,
+                if (!currency) {
-                    position,
+                    return "#,##0.00";
-                    field.currency_field
+                }
-                );
+                return this.getters.computeFormatFromCurrency(currency);
-                return this.getters.getCurrencyFormat(currencyName);
             }
             case "date":
                 return this.locale.dateFormat;

addons/spreadsheet/static/src/list/plugins/list_ui_plugin.js

@@ -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

@@ -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.specification[forbiddenFieldName]
-                    args.kwargs.fields.includes(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":
+                    case "web_search_read":
-                        assert.step("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") {
+                if (args.method === "web_search_read" && args.model === "partner") {
-                    assert.strictEqual(args.kwargs.fields.length, 2);
+                    const spec = args.kwargs.specification;
-                    assert.strictEqual(args.kwargs.fields[0], "pognon");
+                    assert.strictEqual(Object.keys(spec).length, 2);
-                    assert.strictEqual(args.kwargs.fields[1], "currency_id");
+                    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

@@ -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",