From e55dad2d3096ab05a0398f17cba61bb37fb2c7ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lucas=20Lef=C3=A8vre=20=28lul=29?= Date: Fri, 20 Oct 2023 14:56:03 +0200 Subject: [PATCH] [REF] spreadsheet: factorize pivot value format This commit factorizes how the format is computed for ODOO.PIVOT and ODOO.PIVOT.HEADER functions. It was essentially duplicated. Also move the date(time) format responsibilty to each time adapters, instead of handling the different aggregate cases separatly. This commit also prepares the next commit which fixes a formatting bug. Task: 3570281 Part-of: odoo/odoo#139295 --- .../static/src/pivot/pivot_functions.js | 42 ++----------------- .../static/src/pivot/pivot_model.js | 2 +- .../static/src/pivot/pivot_time_adapters.js | 38 ++++++++++++----- .../src/pivot/plugins/pivot_ui_plugin.js | 31 ++++++++++++++ .../static/tests/pivots/pivot_helpers_test.js | 22 +++++----- 5 files changed, 75 insertions(+), 60 deletions(-) diff --git a/addons/spreadsheet/static/src/pivot/pivot_functions.js b/addons/spreadsheet/static/src/pivot/pivot_functions.js index 9f63fd2e797..71933b4336f 100644 --- a/addons/spreadsheet/static/src/pivot/pivot_functions.js +++ b/addons/spreadsheet/static/src/pivot/pivot_functions.js @@ -73,20 +73,10 @@ const ODOO_PIVOT = { computeFormat: function (pivotId, measureName, ...domain) { pivotId = toString(pivotId.value); const measure = toString(measureName.value); - const field = this.getters.getPivotDataSource(pivotId).getField(measure); - if (!field) { - return undefined; - } - switch (field.type) { - case "integer": - return "0"; - case "float": - return "#,##0.00"; - case "monetary": - return this.getters.getCompanyCurrencyFormat() || "#,##0.00"; - default: - return undefined; + if (measure === "__count") { + return "0"; } + return this.getters.getPivotFieldFormat(pivotId, measure); }, returns: ["NUMBER", "STRING"], }; @@ -108,7 +98,6 @@ const ODOO_PIVOT_HEADER = { }, computeFormat: function (pivotId, ...domain) { pivotId = toString(pivotId.value); - const pivot = this.getters.getPivotDataSource(pivotId); const len = domain.length; if (!len) { return undefined; @@ -118,30 +107,7 @@ const ODOO_PIVOT_HEADER = { if (fieldName === "measure" || value === "false") { return undefined; } - const { aggregateOperator, field } = pivot.parseGroupField(fieldName); - switch (field.type) { - case "integer": - return "0"; - case "float": - case "monetary": - return "#,##0.00"; - case "date": - case "datetime": - switch (aggregateOperator) { - case "day": - return this.locale.dateFormat; - case "month": - return "mmmm yyyy"; - case "year": - return "0"; - case "week": - case "quarter": - return undefined; - } - break; - default: - return undefined; - } + return this.getters.getPivotFieldFormat(pivotId, fieldName); }, returns: ["NUMBER", "STRING"], }; diff --git a/addons/spreadsheet/static/src/pivot/pivot_model.js b/addons/spreadsheet/static/src/pivot/pivot_model.js index 6f6ff6363d1..c45419e91f0 100644 --- a/addons/spreadsheet/static/src/pivot/pivot_model.js +++ b/addons/spreadsheet/static/src/pivot/pivot_model.js @@ -312,7 +312,7 @@ export class SpreadsheetPivotModel extends PivotModel { return toNumber(value, DEFAULT_LOCALE); } const adapter = pivotTimeAdapter(aggregateOperator); - return adapter.format(value, locale); + return adapter.formatValue(value, locale); } if (field.relation) { const label = this.metadataRepository.getRecordDisplayName(field.relation, value); diff --git a/addons/spreadsheet/static/src/pivot/pivot_time_adapters.js b/addons/spreadsheet/static/src/pivot/pivot_time_adapters.js index eace10d4601..1231fc51ce3 100644 --- a/addons/spreadsheet/static/src/pivot/pivot_time_adapters.js +++ b/addons/spreadsheet/static/src/pivot/pivot_time_adapters.js @@ -69,7 +69,8 @@ export function pivotTimeAdapter(groupAggregate) { * @property {(groupBy: string, field: string, readGroupResult: object) => string} normalizeServerValue * @property {(value: string) => string} normalizeFunctionValue * @property {(normalizedValue: string, step: number) => string} increment - * @property {(normalizedValue: string, locale: Object) => string} format + * @property {(normalizedValue: string, locale: Object) => string} formatValue + * @property {(locale: Object) => string} getFormat */ /** @@ -94,9 +95,12 @@ const dayAdapter = { const date = DateTime.fromFormat(normalizedValue, "MM/dd/yyyy"); return date.plus({ days: step }).toFormat("MM/dd/yyyy"); }, - format(normalizedValue, locale) { + getFormat(locale) { + return locale.dateFormat; + }, + formatValue(normalizedValue, locale) { const value = toNumber(normalizedValue, DEFAULT_LOCALE); - return formatValue(value, { locale, format: locale.dateFormat }); + return formatValue(value, { locale, format: this.getFormat(locale) }); }, }; @@ -122,7 +126,10 @@ const weekAdapter = { const nextWeek = date.plus({ weeks: step }); return `${nextWeek.weekNumber}/${nextWeek.weekYear}`; }, - format(normalizedValue, locale) { + getFormat(locale) { + return undefined; + }, + formatValue(normalizedValue, locale) { const [week, year] = normalizedValue.split("/"); return sprintf(_t("W%(week)s %(year)s"), { week, year }); }, @@ -148,9 +155,12 @@ const monthAdapter = { .plus({ months: step }) .toFormat("MM/yyyy"); }, - format(normalizedValue, locale) { + getFormat(locale) { + return "mmmm yyyy"; + }, + formatValue(normalizedValue, locale) { const value = toNumber(normalizedValue, DEFAULT_LOCALE); - return formatValue(value, { locale, format: "mmmm yyyy" }); + return formatValue(value, { locale, format: this.getFormat(locale) }); }, }; @@ -175,7 +185,10 @@ const quarterAdapter = { const nextQuarter = date.plus({ quarters: step }); return `${nextQuarter.quarter}/${nextQuarter.year}`; }, - format(normalizedValue, locale) { + getFormat(locale) { + return undefined; + }, + formatValue(normalizedValue, locale) { const [quarter, year] = normalizedValue.split("/"); return sprintf(_t("Q%(quarter)s %(year)s"), { quarter, year }); }, @@ -193,7 +206,10 @@ const yearAdapter = { increment(normalizedValue, step) { return normalizedValue + step; }, - format(normalizedValue, locale) { + getFormat(locale) { + return "0"; + }, + formatValue(normalizedValue, locale) { return formatValue(normalizedValue, { locale, format: "0" }); }, }; @@ -201,6 +217,7 @@ const yearAdapter = { /** * Decorate adapter functions to handle the empty value "false" * @param {PivotTimeAdapter} adapter + * @returns {PivotTimeAdapter} */ function falseHandlerDecorator(adapter) { return { @@ -222,11 +239,12 @@ function falseHandlerDecorator(adapter) { } return adapter.increment(normalizedValue, step); }, - format(normalizedValue, locale) { + getFormat: adapter.getFormat.bind(adapter), + formatValue(normalizedValue, locale) { if (normalizedValue === false) { return _t("None"); } - return adapter.format(normalizedValue, locale); + return adapter.formatValue(normalizedValue, locale); }, }; } diff --git a/addons/spreadsheet/static/src/pivot/plugins/pivot_ui_plugin.js b/addons/spreadsheet/static/src/pivot/plugins/pivot_ui_plugin.js index 1f2824eb2bb..dffddb39f52 100644 --- a/addons/spreadsheet/static/src/pivot/plugins/pivot_ui_plugin.js +++ b/addons/spreadsheet/static/src/pivot/plugins/pivot_ui_plugin.js @@ -7,6 +7,7 @@ import { Domain } from "@web/core/domain"; import { NO_RECORD_AT_THIS_POSITION } from "../pivot_model"; import { globalFiltersFieldMatchers } from "@spreadsheet/global_filters/plugins/global_filters_core_plugin"; import { PivotDataSource } from "../pivot_data_source"; +import { pivotTimeAdapter } from "../pivot_time_adapters"; const { astToFormula } = spreadsheet; const { DateTime } = luxon; @@ -316,6 +317,12 @@ export class PivotUIPlugin extends spreadsheet.UIPlugin { return dataSource.computeOdooPivotHeaderValue(domainArgs); } + getPivotFieldFormat(pivotId, fieldName) { + const dataSource = this.getPivotDataSource(pivotId); + const { field, aggregateOperator } = dataSource.parseGroupField(fieldName); + return this._getFieldFormat(field, aggregateOperator); + } + /** * Get the value for a pivot cell * @@ -438,6 +445,29 @@ export class PivotUIPlugin extends spreadsheet.UIPlugin { // Private // --------------------------------------------------------------------- + /** + * @param {import("../../data_sources/metadata_repository").Field} field + * @param {"day" | "week" | "month" | "quarter" | "year"} aggregateOperator + * @returns {string | undefined} + */ + _getFieldFormat(field, aggregateOperator) { + switch (field.type) { + case "integer": + return "0"; + case "float": + return "#,##0.00"; + case "monetary": + return this.getters.getCompanyCurrencyFormat() || "#,##0.00"; + case "date": + case "datetime": { + const timeAdapter = pivotTimeAdapter(aggregateOperator); + return timeAdapter.getFormat(this.getters.getLocale()); + } + default: + return undefined; + } + } + /** * Refresh the cache of a pivot * @@ -517,6 +547,7 @@ PivotUIPlugin.getters = [ "getSelectedPivotId", "getPivotComputedDomain", "computeOdooPivotHeaderValue", + "getPivotFieldFormat", "getPivotIdFromPosition", "getPivotCellValue", "getPivotGroupByValues", diff --git a/addons/spreadsheet/static/tests/pivots/pivot_helpers_test.js b/addons/spreadsheet/static/tests/pivots/pivot_helpers_test.js index 6696b34776d..b7e804642d1 100644 --- a/addons/spreadsheet/static/tests/pivots/pivot_helpers_test.js +++ b/addons/spreadsheet/static/tests/pivots/pivot_helpers_test.js @@ -181,32 +181,32 @@ QUnit.module("spreadsheet > toNormalizedPivotValue", {}, () => { QUnit.module("spreadsheet > pivot time adapters formatted value", {}, () => { QUnit.test("Day adapter", (assert) => { const adapter = pivotTimeAdapter("day"); - assert.strictEqual(adapter.format("11/12/2020", DEFAULT_LOCALE), "11/12/2020"); - assert.strictEqual(adapter.format("01/11/2020", DEFAULT_LOCALE), "1/11/2020"); - assert.strictEqual(adapter.format("12/05/2020", DEFAULT_LOCALE), "12/5/2020"); + assert.strictEqual(adapter.formatValue("11/12/2020", DEFAULT_LOCALE), "11/12/2020"); + assert.strictEqual(adapter.formatValue("01/11/2020", DEFAULT_LOCALE), "1/11/2020"); + assert.strictEqual(adapter.formatValue("12/05/2020", DEFAULT_LOCALE), "12/5/2020"); }); QUnit.test("Week adapter", (assert) => { const adapter = pivotTimeAdapter("week"); - assert.strictEqual(adapter.format("5/2024", DEFAULT_LOCALE), "W5 2024"); - assert.strictEqual(adapter.format("51/2020", DEFAULT_LOCALE), "W51 2020"); + assert.strictEqual(adapter.formatValue("5/2024", DEFAULT_LOCALE), "W5 2024"); + assert.strictEqual(adapter.formatValue("51/2020", DEFAULT_LOCALE), "W51 2020"); }); QUnit.test("Month adapter", (assert) => { const adapter = pivotTimeAdapter("month"); - assert.strictEqual(adapter.format("12/2020", DEFAULT_LOCALE), "December 2020"); - assert.strictEqual(adapter.format("02/2020", DEFAULT_LOCALE), "February 2020"); + assert.strictEqual(adapter.formatValue("12/2020", DEFAULT_LOCALE), "December 2020"); + assert.strictEqual(adapter.formatValue("02/2020", DEFAULT_LOCALE), "February 2020"); }); QUnit.test("Quarter adapter", (assert) => { const adapter = pivotTimeAdapter("quarter"); - assert.strictEqual(adapter.format("1/2022", DEFAULT_LOCALE), "Q1 2022"); - assert.strictEqual(adapter.format("3/1998", DEFAULT_LOCALE), "Q3 1998"); + assert.strictEqual(adapter.formatValue("1/2022", DEFAULT_LOCALE), "Q1 2022"); + assert.strictEqual(adapter.formatValue("3/1998", DEFAULT_LOCALE), "Q3 1998"); }); QUnit.test("Year adapter", (assert) => { const adapter = pivotTimeAdapter("year"); - assert.strictEqual(adapter.format("2020", DEFAULT_LOCALE), "2020"); - assert.strictEqual(adapter.format("1997", DEFAULT_LOCALE), "1997"); + assert.strictEqual(adapter.formatValue("2020", DEFAULT_LOCALE), "2020"); + assert.strictEqual(adapter.formatValue("1997", DEFAULT_LOCALE), "1997"); }); });