[IMP] formulas: add the SUBTOTAL function #6774

laa-odoo · 2025-07-08T14:39:26Z

Task: 4873384

robodoo · 2025-07-08T14:39:30Z

rrahir · 2025-07-26T07:13:07Z

Yeah!🎉

src/functions/module_info.ts

src/functions/module_lookup.ts

src/plugins/ui_core_views/cell_evaluation/compilation_parameters.ts

LucasLefevre · 2025-07-28T13:46:17Z

tests/xlsx/__snapshots__/xlsx_export.test.ts.snap

@@ -35962,6 +36293,11 @@ exports[`Test XLSX export formulas Multi-Sheets exportable functions 1`] = `
            &gt;25
        </t>
    </si>
+    <si>
+        <t>
+            =ROW(A234)


do you know where this comes from ?

Using ROW and COLUMN for references that do not exist on the sheet no longer works.
The test has been changed so that it continues to work on an existing reference.

LucasLefevre · 2025-07-28T13:48:33Z

src/functions/module_math.ts

+    "Returns a subtotal for a vertical range of cells using a specified aggregation function."
+  ),
+  args: [
+    arg("function_code (number)", _t("The function to use in subtotal aggregation.")),


add what are the possible codes ?

to do in another commit for all formulas that accept static parameters

done in a TOFIXUP commit

LucasLefevre · 2025-07-28T13:50:45Z

src/functions/module_math.ts

+      const bottom = top + ref[0].length - 1;
+
+      for (let row = top; row <= bottom; row++) {
+        if (this.getters.isRowFiltered(sheetId, row)) continue;


Now, all commands related to filters and hidden headers need to invalidate the evaluation

done in the commit "TOFIXUP"

src/functions/module_math.ts

Why this commit ? We would like to introduce the SUBTOTAL formula: This formula has the particularity of taking meta arguments as input, and based on certain criteria, returning the value related to the passed reference. Compared to other formulas using meta arguments, using the values belonging to the cells of the meta arguments is a new behavior. So, for this function to be viable, two things must be done: - compute the values of the passed meta references (which was not done until now) - compute all the values of the cells belonging to the matrix when a range is passed as a meta reference This last point (computing all the cell values when a meta range is passed) implies differentiating between simple meta arguments and matrix meta arguments in the code. Hence the main purpose of this commit. Two other implications of this commit: - This commit enables vectorization for meta arguments. - We revised the way arguments were defined in meta functions, resulting in a complete redefinition for certain functions, such as COLUMNS and ROWS, which no longer use meta arguments. This change involved redefining the error type for references whose sheetname doesn't exist. This also promotes consistency. Task: 4873384

LucasLefevre · 2025-08-14T14:21:05Z

src/functions/module_math.ts

+import { tokenColors } from "../components/composer/composer/abstract_composer_store";
+import { splitReference, toZone } from "../helpers";
+import { insertTokenAfterLeftParenthesis } from "../helpers/pivot/pivot_composer_helpers";
+import { autoCompleteProviders } from "../registries/auto_completes";


there's a circular dependency

LucasLefevre · 2025-08-14T14:34:29Z

src/functions/module_math.ts

+
+        for (let col = left; col <= right; col++) {
+          const cell = this.getters.getCell({ sheetId, col, row });
+          if (!cell || !cell.isFormula || !cell.content || !cell.content.includes("SUBTOTAL")) {


cell.content is expensive for formulas because it builds the entire formula string every time.
Check cell.compiledFormula.tokens instead

src/plugins/ui_core_views/cell_evaluation/evaluation_plugin.ts

src/plugins/ui_feature/subtotal_evaluation.ts

LucasLefevre · 2025-08-14T15:01:24Z

src/functions/module_math.ts

+    return functionRegistry
+      .get(subtotalFunctionAggregateByCode[code])
+      .compute.apply(this, [[evaluatedCellToKeep]]);


Suggested change

return functionRegistry

.get(subtotalFunctionAggregateByCode[code])

.compute.apply(this, [[evaluatedCellToKeep]]);

return this[subtotalFunctionAggregateByCode[code]].apply(this, [[evaluatedCellToKeep]]);

LucasLefevre · 2025-08-14T15:07:48Z

src/functions/module_math.ts

+    const subtotalFunctionCodes = Object.keys(subtotalFunctionAggregateByCode);
+    if (subtotalFunctionCodes.includes(tokenAtCursor.value)) {


this only checks 1-9 but not the 10x versions.

LucasLefevre · 2025-08-14T15:08:26Z

src/functions/module_math.ts

+      return;
+    }
+
+    const proposals: any[] = [];


Suggested change

const proposals: any[] = [];

const proposals: AutoCompleteProposal[] = [];

LucasLefevre · 2025-08-14T15:14:08Z

src/functions/module_math.ts

+  (fnName: string) => _t("%s (accept hidden cells)", fnName),
+  (fnName: string) => _t(`%s (doesn't accept hidden cells)`, fnName),


Suggested change

(fnName: string) => _t("%s (accept hidden cells)", fnName),

(fnName: string) => _t(`%s (doesn't accept hidden cells)`, fnName),

(fnName: string) => _t("%s (include manually-hidden rows)", fnName),

(fnName: string) => _t(`%s (exclude manually-hidden rows)`, fnName),

LucasLefevre · 2025-08-14T15:25:17Z

src/functions/module_math.ts

+    for (let i = 0; i < subtotalFunctionAggregateDescriptions.length; i++) {
+      for (const functionCode of subtotalFunctionCodes) {
+        const str = `${i === 1 ? Number(functionCode) + 100 : functionCode}`;
+        proposals.push({
+          text: str,
+          description: subtotalFunctionAggregateDescriptions[i](
+            subtotalFunctionAggregateByCode[functionCode]
+          ),
+          htmlContent: [{ value: str, color: tokenColors.NUMBER }],
+          fuzzySearchKey: str,
+          alwaysExpanded: true,
+        });
+      }
+    }


Something like that would be a lot easier to read IMO, with a small helper function codeToProposal

for (const code of subtotalFunctionCodes) { proposals.push(codeToProposal(code, _t("%s (accept hidden cells)", subtotalFunctionAggregateByCode[code]))); } for (const code of subtotalFunctionCodes.map((c) => `${Number(c) + 100}`)) { proposals.push(codeToProposal(code, _t("%s (doesn't accept hidden cells)", subtotalFunctionAggregateByCode[code]))); }

laa-odoo · 2025-08-18T09:37:58Z

@LucasLefevre your last review will be taken into account in #6929

src/plugins/ui_feature/subtotal_evaluation.ts

Task: 4873384

LucasLefevre

robodoo r+ rebase-ff

robodoo · 2025-08-18T11:18:13Z

Merge method set to rebase and fast-forward.

Why this commit ? We would like to introduce the SUBTOTAL formula: This formula has the particularity of taking meta arguments as input, and based on certain criteria, returning the value related to the passed reference. Compared to other formulas using meta arguments, using the values belonging to the cells of the meta arguments is a new behavior. So, for this function to be viable, two things must be done: - compute the values of the passed meta references (which was not done until now) - compute all the values of the cells belonging to the matrix when a range is passed as a meta reference This last point (computing all the cell values when a meta range is passed) implies differentiating between simple meta arguments and matrix meta arguments in the code. Hence the main purpose of this commit. Two other implications of this commit: - This commit enables vectorization for meta arguments. - We revised the way arguments were defined in meta functions, resulting in a complete redefinition for certain functions, such as COLUMNS and ROWS, which no longer use meta arguments. This change involved redefining the error type for references whose sheetname doesn't exist. This also promotes consistency. Task: 4873384 Part-of: #6774 Signed-off-by: Lucas Lefèvre (lul) <[email protected]>

closes #6774 Task: 4873384 Signed-off-by: Lucas Lefèvre (lul) <[email protected]>

laa-odoo force-pushed the master-add-subtotal-formula-laa branch 6 times, most recently from 29f0eec to 43c8aed Compare July 25, 2025 14:38

laa-odoo mentioned this pull request Jul 25, 2025

[FIX] Formulas: COLUMN/ROW spread with range #6748

Closed

14 tasks

laa-odoo force-pushed the master-add-subtotal-formula-laa branch from 43c8aed to e42c127 Compare July 28, 2025 12:28

LucasLefevre reviewed Jul 28, 2025

View reviewed changes

laa-odoo force-pushed the master-add-subtotal-formula-laa branch 2 times, most recently from 01b8dde to 5d69db4 Compare August 4, 2025 07:56

laa-odoo force-pushed the master-add-subtotal-formula-laa branch 2 times, most recently from 2e9751b to b63b9c1 Compare August 7, 2025 09:08

LucasLefevre reviewed Aug 14, 2025

View reviewed changes

laa-odoo force-pushed the master-add-subtotal-formula-laa branch from b63b9c1 to 45b5db5 Compare August 18, 2025 10:13

LucasLefevre reviewed Aug 18, 2025

View reviewed changes

src/plugins/ui_feature/subtotal_evaluation.ts Outdated Show resolved Hide resolved

[IMP] formulas: add the SUBTOTAL function

825beb1

Task: 4873384

laa-odoo force-pushed the master-add-subtotal-formula-laa branch from 45b5db5 to 825beb1 Compare August 18, 2025 10:53

LucasLefevre reviewed Aug 18, 2025

View reviewed changes

robodoo pushed a commit that referenced this pull request Aug 18, 2025

[IMP] formulas: add the SUBTOTAL function

035b5a0

closes #6774 Task: 4873384 Signed-off-by: Lucas Lefèvre (lul) <[email protected]>

robodoo temporarily deployed to merge August 18, 2025 11:24 Inactive

robodoo added the 18.5 label Aug 18, 2025

robodoo closed this Aug 18, 2025

		const subtotalFunctionCodes = Object.keys(subtotalFunctionAggregateByCode);
		if (subtotalFunctionCodes.includes(tokenAtCursor.value)) {

	const proposals: any[] = [];
	const proposals: AutoCompleteProposal[] = [];

		(fnName: string) => _t("%s (accept hidden cells)", fnName),
		(fnName: string) => _t(`%s (doesn't accept hidden cells)`, fnName),

[IMP] formulas: add the SUBTOTAL function #6774

[IMP] formulas: add the SUBTOTAL function #6774

Uh oh!

Conversation

laa-odoo commented Jul 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

robodoo commented Jul 8, 2025

Uh oh!

rrahir commented Jul 26, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LucasLefevre Jul 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

laa-odoo commented Aug 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

LucasLefevre left a comment

Choose a reason for hiding this comment

Uh oh!

robodoo commented Aug 18, 2025

Uh oh!

Uh oh!

laa-odoo commented Jul 8, 2025 •

edited

Loading

LucasLefevre Jul 28, 2025 •

edited

Loading

laa-odoo commented Aug 18, 2025 •

edited

Loading