[FIX] pivot: Ensure computed measure range adaptation

src/helpers/pivot/pivot_registry.ts

@@ -28,7 +28,6 @@ type PivotDefinitionConstructor = new (
 export interface PivotRegistryItem {
   ui: PivotUIConstructor;
   definition: PivotDefinitionConstructor;
-  externalData: boolean;
   dateGranularities: string[];
   datetimeGranularities: string[];
   isMeasureCandidate: (field: PivotField) => boolean;
@@ -51,7 +50,6 @@ const dateGranularities = [
 pivotRegistry.add("SPREADSHEET", {
   ui: SpreadsheetPivot,
   definition: SpreadsheetPivotRuntimeDefinition,
-  externalData: false,
   dateGranularities: [...dateGranularities],
   datetimeGranularities: [...dateGranularities, "hour_number", "minute_number", "second_number"],
   isMeasureCandidate: (field: PivotField) => !["datetime", "boolean"].includes(field.type),

src/plugins/core/pivot.ts

@@ -314,18 +314,19 @@ export class PivotCorePlugin extends CorePlugin<CoreState> implements CoreState
       if (!pivot) {
         continue;
       }
-      for (const measure of pivot.definition.measures) {
+      const def = deepCopy(pivot.definition);
+
+      for (const measure of def.measures) {
         if (measure.computedBy?.formula === formulaString) {
-          const measureIndex = pivot.definition.measures.indexOf(measure);
-          this.history.update(
-            "pivots",
-            pivotId,
-            "definition",
-            "measures",
-            measureIndex,
-            "computedBy",
-            { formula: newFormulaString, sheetId }
-          );
+          const measureIndex = def.measures.indexOf(measure);
+          if (measureIndex !== -1) {
+            def.measures[measureIndex].computedBy = {
+              formula: newFormulaString,
+              sheetId,
+            };
+          }
+
+          this.dispatch("UPDATE_PIVOT", { pivotId, pivot: def });
         }
       }
     }

src/plugins/core/spreadsheet_pivot.ts

@@ -48,6 +48,10 @@ export class SpreadsheetPivotCorePlugin extends CorePlugin {
         const range = this.getters.getRangeFromZone(sheetId, zone);
         const adaptedRange = adaptPivotRange(range, applyChange);
 
+        if (adaptedRange === range) {
+          return;
+        }
+
         const dataSet = adaptedRange && {
           sheetId: adaptedRange.sheetId,
           zone: adaptedRange.zone,

src/plugins/ui_core_views/pivot_ui.ts

@@ -2,6 +2,7 @@ import { Token } from "../../formulas";
 import { astToFormula } from "../../formulas/parser";
 import { toScalar } from "../../functions/helper_matrices";
 import { toBoolean } from "../../functions/helpers";
+import { deepCopy } from "../../helpers";
 import {
   getFirstPivotFunction,
   getNumberOfPivotFunctions,
@@ -65,9 +66,7 @@ export class PivotUIPlugin extends UIPlugin {
   handle(cmd: Command) {
     if (invalidateEvaluationCommands.has(cmd.type)) {
       for (const pivotId of this.getters.getPivotIds()) {
-        if (!pivotRegistry.get(this.getters.getPivotCoreDefinition(pivotId).type).externalData) {
-          this.setupPivot(pivotId, { recreate: true });
-        }
+        this.setupPivot(pivotId, { recreate: true });
       }
     }
     switch (cmd.type) {
@@ -302,7 +301,7 @@ export class PivotUIPlugin extends UIPlugin {
   }
 
   setupPivot(pivotId: UID, { recreate } = { recreate: false }) {
-    const definition = this.getters.getPivotCoreDefinition(pivotId);
+    const definition = deepCopy(this.getters.getPivotCoreDefinition(pivotId));
     if (!(pivotId in this.pivots)) {
       const Pivot = withPivotPresentationLayer(pivotRegistry.get(definition.type).ui);
       this.pivots[pivotId] = new Pivot(this.custom, { definition, getters: this.getters });

src/types/commands.ts

@@ -132,7 +132,6 @@ export const invalidateEvaluationCommands = new Set<CommandTypes>([
   "REDO",
   "ADD_MERGE",
   "REMOVE_MERGE",
-  "DUPLICATE_SHEET",
   "UPDATE_LOCALE",
   "ADD_PIVOT",
   "UPDATE_PIVOT",

tests/collaborative/collaborative_history.test.ts

@@ -1,7 +1,6 @@
 import { Model } from "../../src";
 import { DEFAULT_REVISION_ID, MESSAGE_VERSION } from "../../src/constants";
 import { toZone } from "../../src/helpers";
-import { pivotRegistry } from "../../src/helpers/pivot/pivot_registry";
 import { CommandResult, UpdateCellCommand } from "../../src/types";
 import { LineChartDefinition } from "../../src/types/chart/line_chart";
 import { StateUpdateMessage } from "../../src/types/collaborative/transport_service";
@@ -1023,7 +1022,6 @@ describe("Collaborative local history", () => {
   });
 
   test("remove pivot, new user joins, then undo", () => {
-    pivotRegistry.get("SPREADSHEET").externalData = true; // simulate external pivot
     const network = new MockTransportService();
     const data = {
       revisionId: DEFAULT_REVISION_ID,
@@ -1071,7 +1069,6 @@ describe("Collaborative local history", () => {
     const bob = new Model(data, configBob, messages);
     undo(alice);
     expect(getEvaluatedCell(bob, "B3").value).toEqual(10);
-    pivotRegistry.get("SPREADSHEET").externalData = false;
   });
 
   test("Concurrently undo a command on which another is based", () => {

tests/pivots/pivot_calculated_measure.test.ts

@@ -1,10 +1,12 @@
 import {
   activateSheet,
-  addColumns,
+  addRows,
   createSheet,
   deleteSheet,
+  redo,
   setCellContent,
   setFormat,
+  undo,
 } from "../test_helpers/commands_helpers";
 import { getEvaluatedCell, getEvaluatedGrid } from "../test_helpers/getters_helpers";
 import { createModelFromGrid } from "../test_helpers/helpers";
@@ -1063,16 +1065,21 @@ describe("Pivot calculated measure", () => {
       ],
     });
     expect(getEvaluatedCell(model, "A4").value).toEqual(42);
-    addColumns(model, "before", "A", 1);
+    addRows(model, "before", 2, 1);
     expect(model.getters.getPivotCoreDefinition("1").measures).toEqual([
       {
         id: "calculated",
         fieldName: "calculated",
         aggregator: "sum",
-        computedBy: { formula: "=B3", sheetId },
+        computedBy: { formula: "=A4", sheetId },
       },
     ]);
-    expect(getEvaluatedCell(model, "B4").value).toEqual(42);
+    expect(getEvaluatedCell(model, "A5").value).toEqual(42);
+
+    undo(model);
+    expect(getEvaluatedCell(model, "A4").value).toEqual(42);
+    redo(model);
+    expect(getEvaluatedCell(model, "A5").value).toEqual(42);
   });
 
   test("references becomes invalid when sheet is deleted", () => {

tests/pivots/spreadsheet_pivot/spreadsheet_pivot.test.ts

@@ -2,6 +2,7 @@ import { CellErrorType, FunctionResultObject, Model } from "../../../src";
 import { resetMapValueDimensionDate } from "../../../src/helpers/pivot/spreadsheet_pivot/date_spreadsheet_pivot";
 import { DEFAULT_LOCALES } from "../../../src/types/locale";
 import {
+  addRows,
   createSheet,
   deleteContent,
   deleteSheet,
@@ -650,6 +651,22 @@ describe("Spreadsheet Pivot", () => {
     );
   });
 
+  test("Modifying a sheet structure adapts the pivot range", () => {
+    const model = createModelWithPivot("A1:I5");
+    setCellContent(model, "A26", `=pivot(1)`);
+    expect(model.getters.getPivot("1").isValid()).toBeTruthy();
+    expect(getEvaluatedCell(model, "A26").value).toEqual("(#1) My pivot");
+    addRows(model, "before", 0, 1);
+    expect(model.getters.getPivot("1").isValid()).toBeTruthy();
+    expect(getEvaluatedCell(model, "A27").value).toEqual("(#1) My pivot");
+    undo(model);
+    expect(model.getters.getPivot("1").isValid()).toBeTruthy();
+    expect(getEvaluatedCell(model, "A26").value).toEqual("(#1) My pivot");
+    redo(model);
+    expect(model.getters.getPivot("1").isValid()).toBeTruthy();
+    expect(getEvaluatedCell(model, "A27").value).toEqual("(#1) My pivot");
+  });
+
   test("Sum with a field that contains a string should work", () => {
     const model = createModelWithPivot("A1:I5");
     updatePivot(model, "1", {

tests/pivots/spreadsheet_pivot/spreadsheet_pivot_side_panel.test.ts

@@ -636,11 +636,16 @@ describe("Spreadsheet pivot side panel", () => {
   test("Invalid pivot dimensions are displayed as such in the side panel", async () => {
     setCellContent(model, "A1", "ValidDimension");
     setCellContent(model, "A2", "10");
-    addPivot(model, "A1:A2", {
-      columns: [{ fieldName: "ValidDimension" }],
-      rows: [{ fieldName: "InvalidDimension" }],
-    });
-    env.openSidePanel("PivotSidePanel", { pivotId: "1" });
+    addPivot(
+      model,
+      "A1:A2",
+      {
+        columns: [{ fieldName: "ValidDimension" }],
+        rows: [{ fieldName: "InvalidDimension" }],
+      },
+      "2"
+    );
+    env.openSidePanel("PivotSidePanel", { pivotId: "2" });
     await nextTick();
     const pivotDimensionEls = fixture.querySelectorAll<HTMLElement>(".pivot-dimension")!;
     const validDimensionEl = pivotDimensionEls[0];