Skip to content

Commit 7efcbdf

Browse files
committed
[FIX] pivot: unused pivot detection with calculated measure
If a pivot has a calculated measure that refers to a second pivot, the second pivot is not detected as used. This commit fixes that. Note: the fix does not fix 100% of the issues. A pivot only referenced in other places (eg. in a CF rule formula) will still not be detected as used. But checking every other place the pivot can be referenced is expensive and error-prone. That may be done in master. Task: 6105894 X-original-commit: 3ae4353 Part-of: #8739 Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com> Signed-off-by: Adrien Minne (adrm) <adrm@odoo.com>
1 parent b98b445 commit 7efcbdf

2 files changed

Lines changed: 62 additions & 8 deletions

File tree

src/plugins/ui_core_views/pivot_ui.ts

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
FunctionResultObject,
2121
PivotCoreMeasure,
2222
PivotTableCell,
23+
RangeCompiledFormula,
2324
UID,
2425
UpdatePivotCommand,
2526
invalidateEvaluationCommands,
@@ -127,14 +128,16 @@ export class PivotUIPlugin extends CoreViewPlugin {
127128
getPivotIdFromPosition(position: CellPosition) {
128129
const cell = this.getters.getCorrespondingFormulaCell(position);
129130
if (cell && cell.isFormula) {
130-
const pivotFunction = this.getFirstPivotFunction(
131-
position.sheetId,
132-
cell.compiledFormula.tokens
133-
);
134-
if (pivotFunction) {
135-
const pivotId = pivotFunction.args[0]?.toString();
136-
return pivotId && this.getters.getPivotId(pivotId);
137-
}
131+
return this.getPivotIdFromFormula(position.sheetId, cell.compiledFormula);
132+
}
133+
return undefined;
134+
}
135+
136+
private getPivotIdFromFormula(sheetId: UID, formula: RangeCompiledFormula) {
137+
const pivotFunction = this.getFirstPivotFunction(sheetId, formula.tokens);
138+
if (pivotFunction) {
139+
const pivotId = pivotFunction.args[0]?.toString();
140+
return pivotId && this.getters.getPivotId(pivotId);
138141
}
139142
return undefined;
140143
}
@@ -325,6 +328,25 @@ export class PivotUIPlugin extends CoreViewPlugin {
325328
}
326329
}
327330
}
331+
332+
for (const pivotId of this.getters.getPivotIds()) {
333+
const pivot = this.getters.getPivot(pivotId);
334+
for (const measure of pivot.definition.measures) {
335+
if (measure.computedBy) {
336+
const { sheetId } = measure.computedBy;
337+
const formula = this.getters.getMeasureCompiledFormula(pivotId, measure);
338+
const relatedPivotId = this.getPivotIdFromFormula(sheetId, formula);
339+
if (relatedPivotId) {
340+
unusedPivots.delete(relatedPivotId);
341+
if (!unusedPivots.size) {
342+
this.unusedPivots = [];
343+
return [];
344+
}
345+
}
346+
}
347+
}
348+
}
349+
328350
this.unusedPivots = [...unusedPivots];
329351
return this.unusedPivots;
330352
}

tests/pivots/pivot_plugin.test.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -405,4 +405,36 @@ describe("Pivot plugin", () => {
405405
EMPTY_PIVOT_CELL
406406
);
407407
});
408+
409+
describe("isPivotUnused getter", () => {
410+
test("Can use getter to detect unused pivot", () => {
411+
const model = createModelWithPivot("A1:I5");
412+
setCellContent(model, "A40", "=PIVOT(1)");
413+
addPivot(model, "A1:I5", { name: "Unused Pivot" }, "2");
414+
expect(model.getters.isPivotUnused("1")).toBe(false);
415+
expect(model.getters.isPivotUnused("2")).toBe(true);
416+
});
417+
418+
test("Pivot used in another pivot calculated measure is marked as used", () => {
419+
const model = createModelWithPivot("A1:I5");
420+
setCellContent(model, "A40", "=PIVOT(1)");
421+
addPivot(model, "A1:I5", { name: "Unused Pivot" }, "2");
422+
updatePivot(model, "1", {
423+
measures: [
424+
{
425+
id: "Calculated",
426+
fieldName: "Calculated",
427+
aggregator: "sum",
428+
computedBy: {
429+
formula: "=PIVOT(2)",
430+
sheetId: model.getters.getActiveSheetId(),
431+
},
432+
},
433+
],
434+
});
435+
436+
expect(model.getters.isPivotUnused("1")).toBe(false);
437+
expect(model.getters.isPivotUnused("2")).toBe(false);
438+
});
439+
});
408440
});

0 commit comments

Comments
 (0)