Skip to content

Commit

Permalink
[FIX] pivot: running total display with multiple measures
Browse files Browse the repository at this point in the history
The running total computation was wrong in pivot if there was multiple
measures: the pivot cells were not filtered by the measure on which
the running total was computed, which led to the same cell being
counted multiple times.

The display rank ascending/descending had theoretically the same issue,
but since pivot cells with the exact same value have the same rank, it
didn't end up impacting the final result.

closes #5037

Task: 4207244
X-original-commit: d85d4bc
Signed-off-by: Lucas Lefèvre (lul) <[email protected]>
  • Loading branch information
hokolomopo committed Sep 30, 2024
1 parent 6c0b276 commit dea1a76
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 4 deletions.
8 changes: 4 additions & 4 deletions src/helpers/pivot/pivot_presentation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,7 @@ export default function (PivotClass: PivotUIConstructor) {
const mainDimension = getFieldDimensionType(this, fieldNameWithGranularity);
const secondaryDimension = mainDimension === "row" ? "column" : "row";

let pivotCells = this.getPivotValueCells();
let pivotCells = this.getPivotValueCells(measure.id);

if (mainDimension === "column") {
// Transpose the pivot cells so we can do the same operations on the columns as on the rows
Expand Down Expand Up @@ -597,7 +597,7 @@ export default function (PivotClass: PivotUIConstructor) {
const mainDimension = getFieldDimensionType(this, fieldNameWithGranularity);
const secondaryDimension = mainDimension === "row" ? "column" : "row";

let pivotCells = this.getPivotValueCells();
let pivotCells = this.getPivotValueCells(measure.id);

if (mainDimension === "column") {
// Transpose the pivot cells so we can do the same operations on the columns as on the rows
Expand Down Expand Up @@ -703,10 +703,10 @@ export default function (PivotClass: PivotUIConstructor) {
return comparedValueNumber;
}

private getPivotValueCells(): PivotValueCell[][] {
private getPivotValueCells(measureId: string): PivotValueCell[][] {
return this.getTableStructure()
.getPivotCells()
.map((col) => col.filter((cell) => cell.type === "VALUE"))
.map((col) => col.filter((cell) => cell.type === "VALUE" && cell.measure === measureId))
.filter((col) => col.length > 0) as PivotValueCell[][];
}

Expand Down
33 changes: 33 additions & 0 deletions tests/pivots/pivot_measure/pivot_measure_display_model.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1387,6 +1387,39 @@ describe("Measure display", () => {
A25: "Total", B25: "", C25: "", D25: "",
});
});

test("Running total with multiple measures", () => {
const model = createModelWithTestPivot({
measures: [
{
fieldName: "Expected Revenue",
aggregator: "sum",
userDefinedName: "m1",
id: "m1",
},
{
fieldName: "Expected Revenue",
aggregator: "sum",
id: "m2",
userDefinedName: "m2",
display: {
type: "running_total",
fieldNameWithGranularity: "Salesperson",
},
},
],
});

// prettier-ignore
expect(getFormattedGrid(model)).toMatchObject({
A20:"(#1) Pivot", B20: "Alice", C20: "", D20: "Bob", E20: "", F20: "Total", G20: "",
A21: "", B21: "m1", C21: "m2", D21: "m1", E21: "m2", F21: "m1", G21: "m2",
A22: "February", B22: "22500", C22: "22500", D22: "", E22: "22500", F22: "22500", G22: "",
A23: "March", B23: "125400", C23: "125400", D23: "64000", E23: "189400", F23: "189400", G23: "",
A24: "April", B24: "82300", C24: "82300", D24: "26000", E24: "108300", F24: "108300", G24: "",
A25: "Total", B25: "230200", C25: "230200", D25: "90000", E25: "320200", F25: "320200", G25: "",
});
});
});

describe("%_running_total", () => {
Expand Down

0 comments on commit dea1a76

Please sign in to comment.