[FIX] pivot: support sorted column with field name

Fix for an upgrade issue: the sortedColumn measure was not updated
from using fieldName to using id. If the sortedColumn measure matches
a measure fieldName in the definition, update it to use the measure's id instead
of its fieldName.

closes #6533

Task: 4816705
X-original-commit: c737e58
Signed-off-by: Pierre Rousseau (pro) <[email protected]>

Author: LucasLefevre

src/plugins/core/pivot.ts

@@ -124,7 +124,12 @@ export class PivotCorePlugin extends CorePlugin<CoreState> implements CoreState
         break;
       }
       case "UPDATE_PIVOT": {
-        this.history.update("pivots", cmd.pivotId, "definition", deepCopy(cmd.pivot));
+        this.history.update(
+          "pivots",
+          cmd.pivotId,
+          "definition",
+          this.repairSortedColumn(deepCopy(cmd.pivot))
+        );
         this.compileCalculatedMeasures(cmd.pivot.measures);
         break;
       }
@@ -214,7 +219,10 @@ export class PivotCorePlugin extends CorePlugin<CoreState> implements CoreState
     pivot: PivotCoreDefinition,
     formulaId = this.nextFormulaId.toString()
   ) {
-    this.history.update("pivots", pivotId, { definition: deepCopy(pivot), formulaId });
+    this.history.update("pivots", pivotId, {
+      definition: this.repairSortedColumn(deepCopy(pivot)),
+      formulaId,
+    });
     this.compileCalculatedMeasures(pivot.measures);
     this.history.update("formulaIds", formulaId, pivotId);
     this.history.update("nextFormulaId", this.nextFormulaId + 1);
@@ -339,6 +347,29 @@ export class PivotCorePlugin extends CorePlugin<CoreState> implements CoreState
     return CommandResult.Success;
   }
 
+  private repairSortedColumn(definition: PivotCoreDefinition) {
+    if (definition.sortedColumn) {
+      // Fix for an upgrade issue: the sortedColumn measure was not updated
+      // from using fieldName to using id. If the sortedColumn measure matches
+      // a measure fieldName in the definition, update it to use the measure's id instead
+      // of its fieldName.
+      // TODO: add an upgrade step to fix this in master and remove this code
+      const sortedMeasure = definition.measures.find(
+        (measure) => measure.fieldName === definition.sortedColumn?.measure
+      );
+      if (sortedMeasure) {
+        return {
+          ...definition,
+          sortedColumn: {
+            ...definition.sortedColumn,
+            measure: sortedMeasure.id,
+          },
+        };
+      }
+    }
+    return definition;
+  }
+
   // ---------------------------------------------------------------------
   // Import/Export
   // ---------------------------------------------------------------------

tests/pivots/pivot_sorting.test.ts

@@ -67,6 +67,49 @@ describe("Pivot sorting", () => {
     });
   });
 
+  test("sortedColumn.measure can be a field name instead of the measure id", () => {
+    // prettier-ignore
+    const grid = {
+      A1: "Customer",    B1: "Price",
+      A2: "Alice",       B2: "10",
+      A3: "Bob",         B3: "20",
+      A4: "=PIVOT(1)"
+    };
+    const model = createModelFromGrid(grid);
+    const sortedColumn: PivotSortedColumn = {
+      domain: [],
+      order: "desc",
+      measure: "Price", // should be "Price:sum" instead of "Price", but we accept it
+    };
+    addPivot(model, "A1:B3", {
+      columns: [],
+      rows: [{ fieldName: "Customer" }],
+      measures: [{ id: "Price:sum", fieldName: "Price", aggregator: "sum" }],
+      sortedColumn,
+    });
+
+    const pivotId = model.getters.getPivotIds()[0];
+    expect(model.getters.getPivot(pivotId).getTableStructure().isSorted).toBe(true);
+    // prettier-ignore
+    expect(getGrid(model)).toMatchObject({
+      A4: "(#1) Pivot",  B4: "Total",
+      A5: "",            B5: "Price",
+      A6: "Bob",         B6: 20,
+      A7: "Alice",       B7: 10,
+      A8: "Total",       B8: 30,
+    });
+    updatePivot(model, pivotId, { sortedColumn: { ...sortedColumn, order: "asc" } });
+    expect(model.getters.getPivot(pivotId).getTableStructure().isSorted).toBe(true);
+    // prettier-ignore
+    expect(getGrid(model)).toMatchObject({
+      A4: "(#1) Pivot",  B4: "Total",
+      A5: "",            B5: "Price",
+      A6: "Alice",       B6: 10,
+      A7: "Bob",         B7: 20,
+      A8: "Total",       B8: 30,
+    });
+  });
+
   test("Empty values are sorted as the smallest value", () => {
     const model = createModelWithTestPivotDataset();
     const bobColumn: PivotSortedColumn = {